[ 
https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969496#action_12969496
 ] 

Chris Ey commented on BVAL-88:
------------------------------

Hey Donald

thanks for applying your patch. Unfortunately, this doesn't fix the problem 
(our tests fail).

The key to the fix is that no additional composed constraint violation is added 
(line 191 of constraintviolat...@1043662) in case listener.hasViolation is true 
before entering the try block in line 176.

In your solution, you do not loop over composingValidation in case 
listener.hasViolation is true, yet you still add a composed violation in line 
191 because failed == true.

That's why in my patch I "return" the try block and the method as soon as I 
detect there was a composed violation already (listener.hasViolation == true) 
and don't even check for the value of "failed" - because if there was a 
composed violation already, no need to add another one for this 
"reportAsSingleViolation" block.

Does this make sense? Sounds confusing to me already and I just wrote it. ;)

> Cascaded validation adds a constraint violation for valid child property
> ------------------------------------------------------------------------
>
>                 Key: BVAL-88
>                 URL: https://issues.apache.org/jira/browse/BVAL-88
>             Project: BeanValidation
>          Issue Type: Bug
>    Affects Versions: 0.2-incubating
>            Reporter: Chris Ey
>            Assignee: Donald Woods
>             Fix For: 0.3-incubating
>
>         Attachments: BVAL-88.patch, BVAL-88drw.patch
>
>
> Precondition: 
> Parent bean (call it 'Department') has an invalid property, say, 
> "description" annotated with @NotEmpty and value is null. 
> Department also has a valid child element, say, Person manager, which itself 
> has a name. Manager is annotated with @Valid and @NotNull, and name is 
> annotated with @NotEmpty. Both manager and name are correctly populated with 
> non null values:
> Department {
>     @NotEmpty
>     String description;
>     @NotNull;
>     @Valid
>     Person manager;
> }
> Person {
>     @NotEmpty
>     String name;
> }
> Values:
> Department.description: Intentionally left empty (to cause a constraint 
> violation)
> Department.manager: new Person()
> Person.name: "Valid Value"
> Action:
> Department is validated using bean validation.
> Expected:
> 1 Constraint violation:
> empty Department.description
> Actual:
> 2 Constraint violations:
> empty Department.description
> empty Person.name
> I debugged a bit and it seems like when traversing down to Person's 
> properties, it somehow switches into (isReportsAsSingleViolation() == true) 
> path (constraintviolation.j...@162), then it sets failed = true in line 171, 
> because the context does have violations (caused previously by 
> Department.description). But failed should apparently stay false, because 
> there was no /new/ error caused by Person.
> Nevertheless, a new ConstraintViolation gets added in line 183 because failed 
> was true (while it shouldn't be).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to