Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-6132

AbstractChoice#getChoices() should be final

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 7.2.0
    • Fix Version/s: 8.0.0-M1
    • Component/s: None
    • Labels:
      None

      Description

      Not a bug, but a welcome improvement. The AbstractChoice#getChoices() method is non-final (the setter is though), which can cause serious application faults migrating from Wicket 6.

      I attached a Wicket 6 quickstart as well as a Wicket 7 version to reproduce the issue using two DropDownChoices.

      In Wicket 6, you could do the following with no problem. First you construct a DropDownChoice, supplying your collection of choices. Apparently, in Wicket 6, this initial collection is not directly used when converting user input. You could override the getChoices function and supply a totally different collection without any problem. You could exploit this to create a dynamic DropDownChoice that has different select options, depending on the outcome of getChoices().

      In Wicket 7 however, while converting user input, the list of choices you supplied initially is directly accessed, circumventing the getChoices method. The dynamic DropDownChoice which was working fine in Wicket 6 will most probably still work, but with surprising results...

      To reproduce, fire up supplied Wicket 6 quickstart. You'll get two DropDownChoices, make and model of cars. Check that the model drop down is showing models for both Ferrari and Chevrolet. Now, choose Ferrari as the make and for example F355 as the model. A label has been updated to show what has been selected. Now, switch the make to Chevrolet and pick the El Camino. The label will have updated accordingly and will say "Chevrolet El Camino".

      Now, try the same reproduction path in the Wicket 7 quickstart. There is no error, but the label will say "Chevrolet Testarossa". As much as I would love to see how such a mash-up of iconic cars will work out, this is obviously not what the user expected.

      Migrating from Wicket 6 to 7 can get you unexpected behavior which could've been prevented if AbstractChoice#getChoices() had been made final. The resulting compile error is much easier to fix than having to fix potentially broken data a few weeks in production...

      Note: I know that this is a totally wrong approach to creating dynamic form input components, you should always update components through their models. It's just that you've always got that piece of legacy code, written by that someone that didn't quite grasp Wicket's way. It might even have been myself as a junior developer

        Attachments

        1. cars-wicket6.zip
          9 kB
          Rens Verhage
        2. cars-wicket7.zip
          10 kB
          Rens Verhage

          Issue Links

            Activity

              People

              • Assignee:
                mgrigorov Martin Grigorov
                Reporter:
                verhage Rens Verhage
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: