[hibernate-issues] [Hibernate-JIRA] Commented: (HV-452) Incorrect Paths when using fluent API to add constraints to a context

[Glen Hilton (JIRA)]

    [ http://opensource.atlassian.com/projects/hibernate/browse/HV-452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=40235#action_40235 ] 

Glen Hilton commented on HV-452:
--------------------------------

Glad my analysis was helpful, thanks for working on it.  The paths look better.  Though it looks like the most recent changes introduce another, smaller bug.  

If we did this test: {{context.buildConstraintViolationWithTemplate("Test").addNode("Level1").addNode("Level2").addNode("Level3").addConstraintViolation();}}

I would expect that the path would be Level1.Level2.Level3  but it comes out Level1.Level3.Level2

I think this stems from the fact that {{InIterableNodeBuilderImpl.addNode(String name)}} returns a reference to {{this}} then within {{InIterableNodeBuilderImpl.addConstraintViolation()}} the {{leafNodeName}} is added, thereby reversing Level2 and Level3.


Then I have two more possible problems.  I need to take sometime to fully digest the BV spec and TCK, before I could fully prove or dismiss these.  Maybe these ramblings can inspire you in the mean time:
1.  I wonder if there's a way that {{propertyPath.addNode( leafNodeName );}} within {{InIterablePropertiesBuilderImpl.addConstraintViolation()}} can actually cause a problem too.

2. I see that the copying of the path only occurs once in all of {{ConstraintValidatorContextImpl}}, within {{buildConstraintViolationWithTemplate}} I'm wonder if there's a scenario wherein the lack of a copy in one of the other inner classes causes a problem.

> Incorrect Paths when using fluent API to add constraints to a context
> ---------------------------------------------------------------------
>
>                 Key: HV-452
>                 URL: http://opensource.atlassian.com/projects/hibernate/browse/HV-452
>             Project: Hibernate Validator
>          Issue Type: Bug
>          Components: engine
>    Affects Versions: 4.2.0.Beta1, 4.2.0.Beta2
>            Reporter: Glen Hilton
>            Assignee: Hardy Ferentschik
>             Fix For: 4.2.0.CR1
>
>
> The bug actually should be found by existing test cases in ConstraintValidatorContextTest, but there happens to be a bug there too.
> First to the test case bug:
> In ConstraintValidatorContextTest, within the testing utility method assertMessageAndPath there's a call to TestUtil.assertEqualPaths. TestUtil.assertEqualPaths doesn't assert anything it returns a boolean, this fact causes line 156: 
> {{assertEqualPaths( messageAndPath.getPath(), PathImpl.createPathFromString( expectedPath ) );}}
> to not actually test anything.  Given this testing bug, we don't know that ConstraintValidatorContextTest.testMultipleMessages line 144:
> {{assertMessageAndPath( messageAndPathList.get( 1 ), message2, "" );}}
> should actually fail. As a quick and dirty fix to the test cases, we could wrap the existing line 144 with assertTrue, so it would end up:
> {{assertTrue(assertMessageAndPath( messageAndPathList.get( 1 ), message2, "" ))}}
> Now two of the tests in ConstraintValidatorContextTest will fail.
> These test cases appear to fail because within ConstraintValidatorContextImpl, all of the subclasses (ErrorBuilderImpl, NodeBuilderImpl, InIterableNodeBuilderImpl) are getting instantiated with a reference to the propertyPath at the "base" level (ConstraintValidatorContextImpl.propertyPath).  Meaning when the subclasses update the propertyPath they are actually all updating the same underlying object, leading to incorrect paths being created.
> This bug appears to have been introduced with HV-395 as before there was code to create a copy of the path when ErrorBuilderImpl.addNode was called.  Sorry I don't have an actual fix to provide, as I don't fully grasp HV-395.  p.s. thanks for all the hard work!

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira