BVal
  1. BVal
  2. BVAL-38

Groups and payload values must be part of the ConstraintDescriptor attributes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.1-incubating
    • Fix Version/s: 0.1-incubating
    • Component/s: jsr303
    • Labels:
      None

      Description

      When querying the validation metadata, the returned ConstraintDescriptor must have 'groups' and 'payload' attributes.

      1. bval-jsr303-38-setters.patch
        0.7 kB
        Carlos Vara
      2. bval-jsr303-38-extra-test.patch
        3 kB
        Carlos Vara
      3. bval-jsr303-38.patch
        4 kB
        Carlos Vara

        Activity

        Carlos Vara created issue -
        Hide
        Carlos Vara added a comment -

        Patch and 1 test.

        1 more test passes.

        Show
        Carlos Vara added a comment - Patch and 1 test. 1 more test passes.
        Carlos Vara made changes -
        Field Original Value New Value
        Attachment bval-jsr303-38.patch [ 12443696 ]
        Hide
        Roman Stumm added a comment -

        Whether groups and payload are expected to appear in the attributes was a question, for which I had AnnotationConstraintBuilder.buildFromAnnotation in mind. Thanks for clarifing this! I decided to fix this in a different way, so that the setters in ConstraintValidation stay simple and the AnnotationConstraintBuilder handles the map of attributes.

        Show
        Roman Stumm added a comment - Whether groups and payload are expected to appear in the attributes was a question, for which I had AnnotationConstraintBuilder.buildFromAnnotation in mind. Thanks for clarifing this! I decided to fix this in a different way, so that the setters in ConstraintValidation stay simple and the AnnotationConstraintBuilder handles the map of attributes.
        Roman Stumm made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Roman Stumm made changes -
        Fix Version/s 0.1-incubating [ 12314849 ]
        Hide
        Carlos Vara added a comment -

        Ah, I missed your comment in that class. It's indeed a better idea to have a cleaner setter. Only thing I'm wondering now is whether it will work properly when overriding the groups (when composing constraints for example), as they are set with that setter after the constraint has been built.

        Show
        Carlos Vara added a comment - Ah, I missed your comment in that class. It's indeed a better idea to have a cleaner setter. Only thing I'm wondering now is whether it will work properly when overriding the groups (when composing constraints for example), as they are set with that setter after the constraint has been built.
        Hide
        Roman Stumm added a comment -

        Good point, then maybe your patch should be still neccessary... I reopen the issue.

        Show
        Roman Stumm added a comment - Good point, then maybe your patch should be still neccessary... I reopen the issue.
        Hide
        Roman Stumm added a comment -

        see comment of Carlos about composing constraints

        Show
        Roman Stumm added a comment - see comment of Carlos about composing constraints
        Roman Stumm made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Carlos Vara added a comment -

        A test to check if 'groups' attribute value is correctly updated when inheriting groups.

        Show
        Carlos Vara added a comment - A test to check if 'groups' attribute value is correctly updated when inheriting groups.
        Carlos Vara made changes -
        Attachment bval-jsr303-38-extra-test.patch [ 12443700 ]
        Hide
        Carlos Vara added a comment -

        Test fails with current codebase. It can be fixed changing the setter again, or modifying the attributes hashmap when inheriting. I think changing the setter is a better idea, but both ways are fine with me.

        Also, you decide whether it's you or me the one who fixes it

        Show
        Carlos Vara added a comment - Test fails with current codebase. It can be fixed changing the setter again, or modifying the attributes hashmap when inheriting. I think changing the setter is a better idea, but both ways are fine with me. Also, you decide whether it's you or me the one who fixes it
        Hide
        Carlos Vara added a comment -

        Maybe removing the groups field from ConstraintValidation and using always the attributes map is the cleaner way, so that we don't have to maintain two references for the same data.

        Show
        Carlos Vara added a comment - Maybe removing the groups field from ConstraintValidation and using always the attributes map is the cleaner way, so that we don't have to maintain two references for the same data.
        Hide
        Carlos Vara added a comment -

        Patch that adds functionality to the setters to keep the attributes map up to date.

        I decided to go with that route because removing groups and payload field involves converting types in a method that is probably used a lot, as the groups/payloads are stored as arrays in the attribute map and as sets in the individual fields.

        Test uploaded in this issue passes with this patch applied.

        Of course, if you prefer to fix it any other way, just ignore this patch.

        Btw, I have another patch to fix implicit groups lined up for when this issue is closed

        Show
        Carlos Vara added a comment - Patch that adds functionality to the setters to keep the attributes map up to date. I decided to go with that route because removing groups and payload field involves converting types in a method that is probably used a lot, as the groups/payloads are stored as arrays in the attribute map and as sets in the individual fields. Test uploaded in this issue passes with this patch applied. Of course, if you prefer to fix it any other way, just ignore this patch. Btw, I have another patch to fix implicit groups lined up for when this issue is closed
        Carlos Vara made changes -
        Attachment bval-jsr303-38-setters.patch [ 12443708 ]
        Hide
        Roman Stumm added a comment -

        used your changes, reverted my change in AnnotationConstraintBuilder. sorry for the delay, I was busy and out-of-office for some days.

        Show
        Roman Stumm added a comment - used your changes, reverted my change in AnnotationConstraintBuilder. sorry for the delay, I was busy and out-of-office for some days.
        Roman Stumm made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Carlos Vara added a comment - - edited

        No problem at all, I have also been a little busier this week.

        I will shortly upload the next fix.

        Show
        Carlos Vara added a comment - - edited No problem at all, I have also been a little busier this week. I will shortly upload the next fix.
        Hide
        Matt Benson added a comment -

        closing issues associated with existing releases

        Show
        Matt Benson added a comment - closing issues associated with existing releases
        Matt Benson made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        15m 20s 1 Roman Stumm 05/May/10 10:13
        Resolved Resolved Reopened Reopened
        19m 19s 1 Roman Stumm 05/May/10 10:32
        Reopened Reopened Resolved Resolved
        5d 4h 26m 1 Roman Stumm 10/May/10 14:58
        Resolved Resolved Closed Closed
        697d 8h 30m 1 Matt Benson 06/Apr/12 23:29

          People

          • Assignee:
            Unassigned
            Reporter:
            Carlos Vara
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development