You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bval.apache.org by Matt Benson <gu...@gmail.com> on 2010/09/28 00:08:46 UTC
Urgent problem with PathImpl
Hi guys,
I believe I have found a serious issue. In section 4.2, the structure of a set of Path.Nodes is described almost as a footnote to the section describing ConstraintViolation. On rereading this section my interpretation is that bval-jsr303 currently implements association traversals precisely backward to the letter of the specification. I am quite shocked that we can pass the TCK like this, but hopefully this means the TCK tests are simply string-based, since the alternative situation would be that the RI, which presumably passes the TCK, is flawed in a similar way to the Apache implementation. I'd also be glad to be educated on why I am mistaken. My issue is this:
The spec says that a constraint on the fourth author (i.e. "authors[3]") would be represented by a not-in-iterable "authors" node followed by a nameless node with index 3. PathImpl would represent this as a single "authors" node with index 3.
Likewise, the spec says that a constraint on the first author's company ("authors[0].company") would be represented by a not-in-iterable "authors" node followed by a "company" node with index 0. PathImpl represents this as an "authors" node with index 3, followed by a not-in-iterable "company" node.
I wholeheartedly believe I have discovered a bona fide problem here and will begin working to fix it. Unless I am looking at the wrong specification I can't imagine how I could be mistaken here, but please review and let me know if I am in fact mistaken.
Thanks,
Matt
Re: Urgent problem with PathImpl
Posted by Matt Benson <gu...@gmail.com>.
On Sep 28, 2010, at 8:14 PM, Matt Benson wrote:
>
> On Sep 28, 2010, at 4:52 PM, Carlos Vara wrote:
>
>> Hi again Matt,
>>
>> this is the issue in which I added the changes to pass the tests:
>> https://issues.apache.org/jira/browse/BVAL-29
>>
>> From its message:
>> "Call to inIterable() should modify the previous sibling node instead of the
>> "current" node, and in case a null named non iterable node is left in the
>> leaf of the path, it should never be added.
>> This behavior is not documented in the JSR-303 spec document, but
>> ConstraintValidatorContext Javadoc gives information on how it should work."
>>
>> You can get the description of this behaviour here:
>> http://download.oracle.com/javaee/6/api/javax/validation/ConstraintValidatorContext.html#buildConstraintViolationWithTemplate%28java.lang.String%29(take
>> care of the typo with city/country).
>>
>
> Hi Carlos,
> My local changes still pass our local tests of this nature, despite the TCK failures. I've just discovered the debugging profile in the tck-runner module, so I'll look into that and report back.
>
I'm relatively Maven-illiterate, but I'm learning. I was able to debug into the console-based TCK run using -Dmaven.surefire.debug . However, it wasn't rocket science to see that the RI PathImpl is basically the same as bval's, so it suffers from the same shortcomings and functions identically contrariwise to the specification, from my reading.
Gerhard/Niall, can either of you shed any light on the origins of the specification as applies to Path? When I apply my changes to make PathImpl work according to my interpretation of the spec, the TCK tests that fail appear to have been authored almost wholly by the same person as the RI Path implementation itself (and that person not being an EG member, I would venture to say it is possible he simply misinterpreted the spec). Maybe they should have had the tests and the implementation developed by separate people. It has to be true either I am illiterate or that the RI and spec oppose one another; I'm betting on the latter and it is thus my opinion that the EG needs to rule which is in error so we can all move on with our lives.
-Matt
> -Matt
>
>
>> I don't have much time to look into it today, but I can say that I remember
>> that behaviour as being a little counter-intuitive, but at that stage I
>> centered on making the tests pass.
>>
>> Regards,
>> Carlos
>>
>> On Tue, Sep 28, 2010 at 8:49 PM, Matt Benson <gu...@gmail.com> wrote:
>>
>>>
>>> On Sep 28, 2010, at 2:36 PM, Carlos Vara wrote:
>>>
>>>> Hi Matt,
>>>>
>>>> I remember having special trouble when working on the Path creation API
>>> to
>>>> make the tests pass. I also remember thinking the TCK was weird in the
>>> way
>>>> the nodes had to be created for the tests to pass.
>>>>
>>>> This night I will try to get time to take a look and update this thread.
>>>>
>>>
>>> Actually I have already finished a cut of the changes to make PathImpl et
>>> al spec-compliant and they do then fail the TCK. It would seem that either
>>> the spec or the TCK needs to change to come into alignment with the other.
>>> I am going to finish my changes up and attach them to a patch in JIRA so I
>>> don't lose them. :/
>>>
>>> -Matt
>>>
>>>> Regards,
>>>> Carlos
>>>>
>>>> On Mon, Sep 27, 2010 at 11:08 PM, Matt Benson <gu...@gmail.com>
>>> wrote:
>>>>
>>>>> Hi guys,
>>>>> I believe I have found a serious issue. In section 4.2, the structure
>>> of
>>>>> a set of Path.Nodes is described almost as a footnote to the section
>>>>> describing ConstraintViolation. On rereading this section my
>>> interpretation
>>>>> is that bval-jsr303 currently implements association traversals
>>> precisely
>>>>> backward to the letter of the specification. I am quite shocked that we
>>> can
>>>>> pass the TCK like this, but hopefully this means the TCK tests are
>>> simply
>>>>> string-based, since the alternative situation would be that the RI,
>>> which
>>>>> presumably passes the TCK, is flawed in a similar way to the Apache
>>>>> implementation. I'd also be glad to be educated on why I am mistaken.
>>> My
>>>>> issue is this:
>>>>>
>>>>> The spec says that a constraint on the fourth author (i.e. "authors[3]")
>>>>> would be represented by a not-in-iterable "authors" node followed by a
>>>>> nameless node with index 3. PathImpl would represent this as a single
>>>>> "authors" node with index 3.
>>>>>
>>>>> Likewise, the spec says that a constraint on the first author's company
>>>>> ("authors[0].company") would be represented by a not-in-iterable
>>> "authors"
>>>>> node followed by a "company" node with index 0. PathImpl represents
>>> this as
>>>>> an "authors" node with index 3, followed by a not-in-iterable "company"
>>>>> node.
>>>>>
>>>>> I wholeheartedly believe I have discovered a bona fide problem here and
>>>>> will begin working to fix it. Unless I am looking at the wrong
>>>>> specification I can't imagine how I could be mistaken here, but please
>>>>> review and let me know if I am in fact mistaken.
>>>>>
>>>>> Thanks,
>>>>> Matt
>>>
>>>
>
Re: Urgent problem with PathImpl
Posted by Matt Benson <gu...@gmail.com>.
On Sep 28, 2010, at 4:52 PM, Carlos Vara wrote:
> Hi again Matt,
>
> this is the issue in which I added the changes to pass the tests:
> https://issues.apache.org/jira/browse/BVAL-29
>
> From its message:
> "Call to inIterable() should modify the previous sibling node instead of the
> "current" node, and in case a null named non iterable node is left in the
> leaf of the path, it should never be added.
> This behavior is not documented in the JSR-303 spec document, but
> ConstraintValidatorContext Javadoc gives information on how it should work."
>
> You can get the description of this behaviour here:
> http://download.oracle.com/javaee/6/api/javax/validation/ConstraintValidatorContext.html#buildConstraintViolationWithTemplate%28java.lang.String%29(take
> care of the typo with city/country).
>
Hi Carlos,
My local changes still pass our local tests of this nature, despite the TCK failures. I've just discovered the debugging profile in the tck-runner module, so I'll look into that and report back.
-Matt
> I don't have much time to look into it today, but I can say that I remember
> that behaviour as being a little counter-intuitive, but at that stage I
> centered on making the tests pass.
>
> Regards,
> Carlos
>
> On Tue, Sep 28, 2010 at 8:49 PM, Matt Benson <gu...@gmail.com> wrote:
>
>>
>> On Sep 28, 2010, at 2:36 PM, Carlos Vara wrote:
>>
>>> Hi Matt,
>>>
>>> I remember having special trouble when working on the Path creation API
>> to
>>> make the tests pass. I also remember thinking the TCK was weird in the
>> way
>>> the nodes had to be created for the tests to pass.
>>>
>>> This night I will try to get time to take a look and update this thread.
>>>
>>
>> Actually I have already finished a cut of the changes to make PathImpl et
>> al spec-compliant and they do then fail the TCK. It would seem that either
>> the spec or the TCK needs to change to come into alignment with the other.
>> I am going to finish my changes up and attach them to a patch in JIRA so I
>> don't lose them. :/
>>
>> -Matt
>>
>>> Regards,
>>> Carlos
>>>
>>> On Mon, Sep 27, 2010 at 11:08 PM, Matt Benson <gu...@gmail.com>
>> wrote:
>>>
>>>> Hi guys,
>>>> I believe I have found a serious issue. In section 4.2, the structure
>> of
>>>> a set of Path.Nodes is described almost as a footnote to the section
>>>> describing ConstraintViolation. On rereading this section my
>> interpretation
>>>> is that bval-jsr303 currently implements association traversals
>> precisely
>>>> backward to the letter of the specification. I am quite shocked that we
>> can
>>>> pass the TCK like this, but hopefully this means the TCK tests are
>> simply
>>>> string-based, since the alternative situation would be that the RI,
>> which
>>>> presumably passes the TCK, is flawed in a similar way to the Apache
>>>> implementation. I'd also be glad to be educated on why I am mistaken.
>> My
>>>> issue is this:
>>>>
>>>> The spec says that a constraint on the fourth author (i.e. "authors[3]")
>>>> would be represented by a not-in-iterable "authors" node followed by a
>>>> nameless node with index 3. PathImpl would represent this as a single
>>>> "authors" node with index 3.
>>>>
>>>> Likewise, the spec says that a constraint on the first author's company
>>>> ("authors[0].company") would be represented by a not-in-iterable
>> "authors"
>>>> node followed by a "company" node with index 0. PathImpl represents
>> this as
>>>> an "authors" node with index 3, followed by a not-in-iterable "company"
>>>> node.
>>>>
>>>> I wholeheartedly believe I have discovered a bona fide problem here and
>>>> will begin working to fix it. Unless I am looking at the wrong
>>>> specification I can't imagine how I could be mistaken here, but please
>>>> review and let me know if I am in fact mistaken.
>>>>
>>>> Thanks,
>>>> Matt
>>
>>
Re: Urgent problem with PathImpl
Posted by Carlos Vara <ba...@gmail.com>.
Hi again Matt,
this is the issue in which I added the changes to pass the tests:
https://issues.apache.org/jira/browse/BVAL-29
>From its message:
"Call to inIterable() should modify the previous sibling node instead of the
"current" node, and in case a null named non iterable node is left in the
leaf of the path, it should never be added.
This behavior is not documented in the JSR-303 spec document, but
ConstraintValidatorContext Javadoc gives information on how it should work."
You can get the description of this behaviour here:
http://download.oracle.com/javaee/6/api/javax/validation/ConstraintValidatorContext.html#buildConstraintViolationWithTemplate%28java.lang.String%29(take
care of the typo with city/country).
I don't have much time to look into it today, but I can say that I remember
that behaviour as being a little counter-intuitive, but at that stage I
centered on making the tests pass.
Regards,
Carlos
On Tue, Sep 28, 2010 at 8:49 PM, Matt Benson <gu...@gmail.com> wrote:
>
> On Sep 28, 2010, at 2:36 PM, Carlos Vara wrote:
>
> > Hi Matt,
> >
> > I remember having special trouble when working on the Path creation API
> to
> > make the tests pass. I also remember thinking the TCK was weird in the
> way
> > the nodes had to be created for the tests to pass.
> >
> > This night I will try to get time to take a look and update this thread.
> >
>
> Actually I have already finished a cut of the changes to make PathImpl et
> al spec-compliant and they do then fail the TCK. It would seem that either
> the spec or the TCK needs to change to come into alignment with the other.
> I am going to finish my changes up and attach them to a patch in JIRA so I
> don't lose them. :/
>
> -Matt
>
> > Regards,
> > Carlos
> >
> > On Mon, Sep 27, 2010 at 11:08 PM, Matt Benson <gu...@gmail.com>
> wrote:
> >
> >> Hi guys,
> >> I believe I have found a serious issue. In section 4.2, the structure
> of
> >> a set of Path.Nodes is described almost as a footnote to the section
> >> describing ConstraintViolation. On rereading this section my
> interpretation
> >> is that bval-jsr303 currently implements association traversals
> precisely
> >> backward to the letter of the specification. I am quite shocked that we
> can
> >> pass the TCK like this, but hopefully this means the TCK tests are
> simply
> >> string-based, since the alternative situation would be that the RI,
> which
> >> presumably passes the TCK, is flawed in a similar way to the Apache
> >> implementation. I'd also be glad to be educated on why I am mistaken.
> My
> >> issue is this:
> >>
> >> The spec says that a constraint on the fourth author (i.e. "authors[3]")
> >> would be represented by a not-in-iterable "authors" node followed by a
> >> nameless node with index 3. PathImpl would represent this as a single
> >> "authors" node with index 3.
> >>
> >> Likewise, the spec says that a constraint on the first author's company
> >> ("authors[0].company") would be represented by a not-in-iterable
> "authors"
> >> node followed by a "company" node with index 0. PathImpl represents
> this as
> >> an "authors" node with index 3, followed by a not-in-iterable "company"
> >> node.
> >>
> >> I wholeheartedly believe I have discovered a bona fide problem here and
> >> will begin working to fix it. Unless I am looking at the wrong
> >> specification I can't imagine how I could be mistaken here, but please
> >> review and let me know if I am in fact mistaken.
> >>
> >> Thanks,
> >> Matt
>
>
Re: Urgent problem with PathImpl
Posted by Matt Benson <gu...@gmail.com>.
On Sep 28, 2010, at 2:36 PM, Carlos Vara wrote:
> Hi Matt,
>
> I remember having special trouble when working on the Path creation API to
> make the tests pass. I also remember thinking the TCK was weird in the way
> the nodes had to be created for the tests to pass.
>
> This night I will try to get time to take a look and update this thread.
>
Actually I have already finished a cut of the changes to make PathImpl et al spec-compliant and they do then fail the TCK. It would seem that either the spec or the TCK needs to change to come into alignment with the other. I am going to finish my changes up and attach them to a patch in JIRA so I don't lose them. :/
-Matt
> Regards,
> Carlos
>
> On Mon, Sep 27, 2010 at 11:08 PM, Matt Benson <gu...@gmail.com> wrote:
>
>> Hi guys,
>> I believe I have found a serious issue. In section 4.2, the structure of
>> a set of Path.Nodes is described almost as a footnote to the section
>> describing ConstraintViolation. On rereading this section my interpretation
>> is that bval-jsr303 currently implements association traversals precisely
>> backward to the letter of the specification. I am quite shocked that we can
>> pass the TCK like this, but hopefully this means the TCK tests are simply
>> string-based, since the alternative situation would be that the RI, which
>> presumably passes the TCK, is flawed in a similar way to the Apache
>> implementation. I'd also be glad to be educated on why I am mistaken. My
>> issue is this:
>>
>> The spec says that a constraint on the fourth author (i.e. "authors[3]")
>> would be represented by a not-in-iterable "authors" node followed by a
>> nameless node with index 3. PathImpl would represent this as a single
>> "authors" node with index 3.
>>
>> Likewise, the spec says that a constraint on the first author's company
>> ("authors[0].company") would be represented by a not-in-iterable "authors"
>> node followed by a "company" node with index 0. PathImpl represents this as
>> an "authors" node with index 3, followed by a not-in-iterable "company"
>> node.
>>
>> I wholeheartedly believe I have discovered a bona fide problem here and
>> will begin working to fix it. Unless I am looking at the wrong
>> specification I can't imagine how I could be mistaken here, but please
>> review and let me know if I am in fact mistaken.
>>
>> Thanks,
>> Matt
Re: Urgent problem with PathImpl
Posted by Carlos Vara <ba...@gmail.com>.
Hi Matt,
I remember having special trouble when working on the Path creation API to
make the tests pass. I also remember thinking the TCK was weird in the way
the nodes had to be created for the tests to pass.
This night I will try to get time to take a look and update this thread.
Regards,
Carlos
On Mon, Sep 27, 2010 at 11:08 PM, Matt Benson <gu...@gmail.com> wrote:
> Hi guys,
> I believe I have found a serious issue. In section 4.2, the structure of
> a set of Path.Nodes is described almost as a footnote to the section
> describing ConstraintViolation. On rereading this section my interpretation
> is that bval-jsr303 currently implements association traversals precisely
> backward to the letter of the specification. I am quite shocked that we can
> pass the TCK like this, but hopefully this means the TCK tests are simply
> string-based, since the alternative situation would be that the RI, which
> presumably passes the TCK, is flawed in a similar way to the Apache
> implementation. I'd also be glad to be educated on why I am mistaken. My
> issue is this:
>
> The spec says that a constraint on the fourth author (i.e. "authors[3]")
> would be represented by a not-in-iterable "authors" node followed by a
> nameless node with index 3. PathImpl would represent this as a single
> "authors" node with index 3.
>
> Likewise, the spec says that a constraint on the first author's company
> ("authors[0].company") would be represented by a not-in-iterable "authors"
> node followed by a "company" node with index 0. PathImpl represents this as
> an "authors" node with index 3, followed by a not-in-iterable "company"
> node.
>
> I wholeheartedly believe I have discovered a bona fide problem here and
> will begin working to fix it. Unless I am looking at the wrong
> specification I can't imagine how I could be mistaken here, but please
> review and let me know if I am in fact mistaken.
>
> Thanks,
> Matt