You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bval.apache.org by "Chris Ey (JIRA)" <ji...@apache.org> on 2010/11/17 16:25:13 UTC

[jira] Created: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

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


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.java@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.


[jira] Assigned: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Donald Woods reassigned BVAL-88:
--------------------------------

    Assignee: Donald Woods

> 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
>
>
> 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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12971798#action_12971798 ] 

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

Thanks Kevan for the instructions on how to run the TCK. 
For the record: the command is 
mvn -P tck
(with a space between P and tck)

I ran it and I too saw the problem in one of the tests with my fix in place. I don't have a solution yet, though, but I wonder why it passes the TCK with this bug here, I would consider too many constraint violations a problem with the TCK?


> 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-unittest-cey.patch, 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.java@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.


[jira] Resolved: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Donald Woods resolved BVAL-88.
------------------------------

    Resolution: Fixed

Closing for now, as the included patch still passes the TCK.
Please open a new issue if you want to us to try and challenge the TCK or create an updated patch that passes with the current TCK.


> 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-unittest-cey.patch, 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.java@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.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12968489#action_12968489 ] 

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

If you have any question regarding my fix feel free to comment here (I'm watching this bug).
The gist of it is, that if we are in "reportAsSingleViolation" mode, and already found a violation while in this mode, we should not add another composed violation on top of it.
This condition is reached when a validated bean has a violation on one of its properties, and there also is a child bean which may or may not have a violation by itself.
The error is: It will always add a violation for the child bean, whether or not it has an actual violation. My patch fixes this and all tests pass.

> 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
>         Attachments: BVAL-88.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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969613#action_12969613 ] 

Donald Woods commented on BVAL-88:
----------------------------------

I'm seeing one TCK failure when I add the if failed then return at/before the try block.  Will have to look at it more tomorrow.

> 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-unittest-cey.patch, 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.java@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.


[jira] Work started: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on BVAL-88 started by Donald Woods.

> 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
>
>
> 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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933819#action_12933819 ] 

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

I just applied my proposed fix locally and our tests pass again. In short, the fix may be valid.

> 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
>
> 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.java@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.


[jira] Updated: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Ey updated BVAL-88:
-------------------------

    Fix Version/s: 0.3-incubating

> 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
>             Fix For: 0.3-incubating
>
>         Attachments: BVAL-88.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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Kevan Miller (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969626#action_12969626 ] 

Kevan Miller commented on BVAL-88:
----------------------------------

Not sure how well this is documented and since Christian might want to investigate, to run the TCK:

cd bval-tck
mvn -Ptck 

(hope I got that right).

> 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-unittest-cey.patch, 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.java@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.


[jira] Updated: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Ey updated BVAL-88:
-------------------------

    Attachment: BVAL-88-unittest-cey.patch

This patch contains a unit test to reproduce the problem.

Writing this test, I learned more about the problem: It was cause by the NotEmpty annotation we still used from hibernate validation. While we can and will switch to Apache Bean Validation's NotEmpty annotation, Hibernate's is written with Javax annotations only and therefore should be implementation independent.

I included Hibernate's NotEmpty annotation to the test source to show the problem. After applying my other patch, this problem is fixed and the test passes.

> 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-unittest-cey.patch, 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.java@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.


[jira] Updated: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Ey updated BVAL-88:
-------------------------

    Attachment: BVAL-88.patch

Added patch.

> 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
>         Attachments: BVAL-88.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.java@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.


[jira] Updated: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Donald Woods updated BVAL-88:
-----------------------------

    Attachment: BVAL-88drw.patch

Modified patch applied as r1043662.

> 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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12933017#action_12933017 ] 

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

I debugged a bit more and I think I found the problem:

When listener.beginReportAsSingle() is called on a listener that already has a composite error, it will add another error, because "failed" will be set to true yet another time. Maybe it makes sense to stop evaluating constraints once a composite error has been found? Proposed solution:

in ConstraintValidation.java@168 (first line in the try block):
>        if (listener.hasViolations()) {
>           return;
>        }

This way, endReportAsSingle() will be called and no additional composite error will be added.

Chris

> 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
>
> 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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ 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 ConstraintViolation@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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Donald Woods (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969472#action_12969472 ] 

Donald Woods commented on BVAL-88:
----------------------------------

Chris, can you try out the latest code.  I made a slight modification in the logic of when/where listener.hasViolations() is called.


> 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.java@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.


[jira] Commented: (BVAL-88) Cascaded validation adds a constraint violation for valid child property

Posted by "Chris Ey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/BVAL-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969537#action_12969537 ] 

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

I am working on a junit test case to reproduce this issue.

> 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.java@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.