[jira] [Commented] (OAK-4818) Version and Version History nodes don't implement the proper interfaces when found via Node.getNodes()
[ https://issues.apache.org/jira/browse/OAK-4818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16466337#comment-16466337 ] Csaba Varga commented on OAK-4818: -- I've created a pull request to help with the progress on this ticket: [https://github.com/apache/jackrabbit-oak/pull/87] The only difference compared to the patch attached here is that the code is throwing an IllegalStateException when an error occurs instead of falling back to the old behavior. Throwing an error when unexpected things happen is a lot cleaner than trying to continue with an incorrect solution. Please let me know if I should provide backport PRs for the 1.8 and 1.6 branches as well. > Version and Version History nodes don't implement the proper interfaces when > found via Node.getNodes() > -- > > Key: OAK-4818 > URL: https://issues.apache.org/jira/browse/OAK-4818 > Project: Jackrabbit Oak > Issue Type: Bug > Components: jcr >Affects Versions: 1.0.33, 1.2.19, 1.5.10, 1.6.0 >Reporter: Csaba Varga >Priority: Minor > Attachments: version-traversal-issue.patch > > > The JCR spec says in section 15.6: > When an nt:versionHistory or nt:version node is acquired through a query or > directly through a getNode, the actual Java type of the returned object must > be VersionHistory (in the case nt:versionHistory nodes) or Version (in the > case of nt:version nodes). > While this doesn't explicitly mention the getNodes() method, I believe it's a > reasonable expectation that this method should work the same way, i.e. make > the actual Java type of the returned child object Version or VersionHistory > where applicable. Oak currently doesn't work this way; the iterator returned > by either getNodes() overload will always yield NodeImpl instances. > I will attach a suggested patch to fix this, including addition to the unit > tests to catch any regressions in the future. The patch is against the latest > trunk, but the same behavior seems to be present in all past versions as > well, so if it's not a big problem, could you also backport this to older > stable versions as well? (My employer is using AEM 6.1 currently, so we would > be interested in getting this backported to 1.2 at least.) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OAK-4818) Version and Version History nodes don't implement the proper interfaces when found via Node.getNodes()
[
https://issues.apache.org/jira/browse/OAK-4818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15502873#comment-15502873
]
Michael Dürig commented on OAK-4818:
Most of those exceptions should never actually occur on that code path. The
others are {{IllegalItemStateException}} instances indicating the iterator is
not valid any more. This suggest that we could as well throw an
{{IllegaleStateException}} instead of logging and carrying on (like we already
do e.g. in {{NodeImpl#getNodes()}}).
But I agree, there is not silver bullet here. This is one of the places where
the JCR API puts us into a hard place.
> Version and Version History nodes don't implement the proper interfaces when
> found via Node.getNodes()
> --
>
> Key: OAK-4818
> URL: https://issues.apache.org/jira/browse/OAK-4818
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: jcr
>Affects Versions: 1.6, 1.0.33, 1.2.19, 1.5.10
>Reporter: Csaba Varga
>Priority: Minor
> Attachments: version-traversal-issue.patch
>
>
> The JCR spec says in section 15.6:
> When an nt:versionHistory or nt:version node is acquired through a query or
> directly through a getNode, the actual Java type of the returned object must
> be VersionHistory (in the case nt:versionHistory nodes) or Version (in the
> case of nt:version nodes).
> While this doesn't explicitly mention the getNodes() method, I believe it's a
> reasonable expectation that this method should work the same way, i.e. make
> the actual Java type of the returned child object Version or VersionHistory
> where applicable. Oak currently doesn't work this way; the iterator returned
> by either getNodes() overload will always yield NodeImpl instances.
> I will attach a suggested patch to fix this, including addition to the unit
> tests to catch any regressions in the future. The patch is against the latest
> trunk, but the same behavior seems to be present in all past versions as
> well, so if it's not a big problem, could you also backport this to older
> stable versions as well? (My employer is using AEM 6.1 currently, so we would
> be interested in getting this backported to 1.2 at least.)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (OAK-4818) Version and Version History nodes don't implement the proper interfaces when found via Node.getNodes()
[ https://issues.apache.org/jira/browse/OAK-4818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15502834#comment-15502834 ] Marcel Reutegger commented on OAK-4818: --- Thanks for reporting this issue. In general, the patch looks good to me. Handling checked exceptions in functions is indeed a problem and falling back to NodeImpl might be the best we can do here. [~mduerig], WDYT? > Version and Version History nodes don't implement the proper interfaces when > found via Node.getNodes() > -- > > Key: OAK-4818 > URL: https://issues.apache.org/jira/browse/OAK-4818 > Project: Jackrabbit Oak > Issue Type: Bug > Components: jcr >Affects Versions: 1.6, 1.0.33, 1.2.19, 1.5.10 >Reporter: Csaba Varga >Priority: Minor > Attachments: version-traversal-issue.patch > > > The JCR spec says in section 15.6: > When an nt:versionHistory or nt:version node is acquired through a query or > directly through a getNode, the actual Java type of the returned object must > be VersionHistory (in the case nt:versionHistory nodes) or Version (in the > case of nt:version nodes). > While this doesn't explicitly mention the getNodes() method, I believe it's a > reasonable expectation that this method should work the same way, i.e. make > the actual Java type of the returned child object Version or VersionHistory > where applicable. Oak currently doesn't work this way; the iterator returned > by either getNodes() overload will always yield NodeImpl instances. > I will attach a suggested patch to fix this, including addition to the unit > tests to catch any regressions in the future. The patch is against the latest > trunk, but the same behavior seems to be present in all past versions as > well, so if it's not a big problem, could you also backport this to older > stable versions as well? (My employer is using AEM 6.1 currently, so we would > be interested in getting this backported to 1.2 at least.) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
