You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by tr...@apache.org on 2013/11/26 14:17:34 UTC
svn commit: r1545647 - in /jackrabbit/oak/trunk/oak-jcr/src:
main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Author: tripod
Date: Tue Nov 26 13:17:34 2013
New Revision: 1545647
URL: http://svn.apache.org/r1545647
Log:
OAK-1227 Node.hasNode("foo[2]") must not throw PathNotFoundException
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java?rev=1545647&r1=1545646&r2=1545647&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java Tue Nov 26 13:17:34 2013
@@ -797,24 +797,32 @@ public class NodeImpl<T extends NodeDele
@Override
public boolean hasNode(String relPath) throws RepositoryException {
- final String oakPath = getOakPathOrThrow(relPath);
- return perform(new NodeOperation<Boolean>(dlg) {
- @Override
- public Boolean perform() throws RepositoryException {
- return node.getChild(oakPath) != null;
- }
- });
+ try {
+ final String oakPath = getOakPathOrThrow(relPath);
+ return perform(new NodeOperation<Boolean>(dlg) {
+ @Override
+ public Boolean perform() throws RepositoryException {
+ return node.getChild(oakPath) != null;
+ }
+ });
+ } catch (PathNotFoundException e) {
+ return false;
+ }
}
@Override
public boolean hasProperty(String relPath) throws RepositoryException {
- final String oakPath = getOakPathOrThrow(relPath);
- return perform(new NodeOperation<Boolean>(dlg) {
- @Override
- public Boolean perform() throws RepositoryException {
- return node.getPropertyOrNull(oakPath) != null;
- }
- });
+ try {
+ final String oakPath = getOakPathOrThrow(relPath);
+ return perform(new NodeOperation<Boolean>(dlg) {
+ @Override
+ public Boolean perform() throws RepositoryException {
+ return node.getPropertyOrNull(oakPath) != null;
+ }
+ });
+ } catch (PathNotFoundException e) {
+ return false;
+ }
}
@Override
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java?rev=1545647&r1=1545646&r2=1545647&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java Tue Nov 26 13:17:34 2013
@@ -307,8 +307,12 @@ public class RepositoryTest extends Abst
}
assertTrue(getAdminSession().nodeExists("/foo[1]/bar[1]"));
+ assertTrue(node.hasNode("bar[1]"));
+ assertTrue(node.hasProperty("bar[1]/jcr:primaryType"));
assertTrue(getAdminSession().propertyExists("/foo[1]/bar[1]/jcr:primaryType"));
assertFalse(getAdminSession().nodeExists("/foo[1]/bar[2]"));
+ assertFalse(node.hasNode("bar[2]"));
+ assertFalse(node.hasProperty("bar[2]/jcr:primaryType"));
assertFalse(getAdminSession().propertyExists("/foo[1]/bar[2]/jcr:primaryType"));
}
Re: svn commit: r1545647 - in /jackrabbit/oak/trunk/oak-jcr/src:
main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Tue, Nov 26, 2013 at 9:59 AM, Tobias Bocanegra <tr...@apache.org> wrote:
> I partially agree. if oak does not support SNS at all
We actually do need at least limited SNS support, as the spec requires
it for node type definitions. What we do there is just store the
affected nodes with the SNS suffixes included in the node names.
Thus it would be nice if the path mapper didn't eagerly throw
exceptions for SNS, and the presense or absense of SNS content would
be simply determined by whether matching content exists in the
repository.
> On Tue, Nov 26, 2013 at 5:54 AM, Michael Dürig <md...@apache.org> wrote:
> > Another way to look at this is to take it as more evidence that we need
> > better path handling aka OAK-1179.
> right. so let's keep it that way and fix it together with the new path handling.
Sounds good.
BR,
Jukka Zitting
Re: svn commit: r1545647 - in /jackrabbit/oak/trunk/oak-jcr/src:
main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Tobias Bocanegra <tr...@apache.org>.
On Tue, Nov 26, 2013 at 5:54 AM, Michael Dürig <md...@apache.org> wrote:
>
> On 26.11.13 2:17 , tripod@apache.org wrote:
>>
>> Author: tripod
>> Date: Tue Nov 26 13:17:34 2013
>> New Revision: 1545647
>>
>> URL:http://svn.apache.org/r1545647
>> Log:
>> OAK-1227 Node.hasNode("foo[2]") must not throw PathNotFoundException
>
>
> [...]
>
>
>> Modified:
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
>>
>> URL:http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java?rev=1545647&r1=1545646&r2=1545647&view=diff
>>
>> ==============================================================================
>> ---
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
>> (original)
>> +++
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
>> Tue Nov 26 13:17:34 2013
>> @@ -797,24 +797,32 @@ public class NodeImpl<T extends NodeDele
>>
>> @Override
>> public boolean hasNode(String relPath) throws RepositoryException {
>> - final String oakPath = getOakPathOrThrow(relPath);
>> - return perform(new NodeOperation<Boolean>(dlg) {
>> - @Override
>> - public Boolean perform() throws RepositoryException {
>> - return node.getChild(oakPath) != null;
>> - }
>> - });
>> + try {
>> + final String oakPath = getOakPathOrThrow(relPath);
>> + return perform(new NodeOperation<Boolean>(dlg) {
>> + @Override
>> + public Boolean perform() throws RepositoryException {
>> + return node.getChild(oakPath) != null;
>> + }
>> + });
>> + } catch (PathNotFoundException e) {
>> + return false;
>> + }
>> }
>
>
> TBH I don't like this. We had long discussions on OAK-89 and related to
> avoid try catch flow control. So I think we should aim for a more general
> fix here. AFAICS the problems have already been introduce with
> http://svn.apache.org/r1545598. Throwing a PathNotFoundException in
> SessionContext#getOakPathOrThrow() stretches that method's contract and is
> clearly against its intent, which is mapping JCR path onto Oak path if
> possible. Not deciding whether such a path exists at all.
I partially agree. if oak does not support SNS at all, then the path
parser can be optimized to throw a PathNotFoundException, if a path
element contains a SNS index>1. OR, all consumers of the path need to
handle a possible SNS indexed name.
> Another way to look at this is to take it as more evidence that we need
> better path handling aka OAK-1179.
right. so let's keep it that way and fix it together with the new path handling.
regards, toby
Re: svn commit: r1545647 - in /jackrabbit/oak/trunk/oak-jcr/src:
main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java
Posted by Michael Dürig <md...@apache.org>.
On 26.11.13 2:17 , tripod@apache.org wrote:
> Author: tripod
> Date: Tue Nov 26 13:17:34 2013
> New Revision: 1545647
>
> URL:http://svn.apache.org/r1545647
> Log:
> OAK-1227 Node.hasNode("foo[2]") must not throw PathNotFoundException
[...]
> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
> URL:http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java?rev=1545647&r1=1545646&r2=1545647&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java (original)
> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java Tue Nov 26 13:17:34 2013
> @@ -797,24 +797,32 @@ public class NodeImpl<T extends NodeDele
>
> @Override
> public boolean hasNode(String relPath) throws RepositoryException {
> - final String oakPath = getOakPathOrThrow(relPath);
> - return perform(new NodeOperation<Boolean>(dlg) {
> - @Override
> - public Boolean perform() throws RepositoryException {
> - return node.getChild(oakPath) != null;
> - }
> - });
> + try {
> + final String oakPath = getOakPathOrThrow(relPath);
> + return perform(new NodeOperation<Boolean>(dlg) {
> + @Override
> + public Boolean perform() throws RepositoryException {
> + return node.getChild(oakPath) != null;
> + }
> + });
> + } catch (PathNotFoundException e) {
> + return false;
> + }
> }
TBH I don't like this. We had long discussions on OAK-89 and related to
avoid try catch flow control. So I think we should aim for a more
general fix here. AFAICS the problems have already been introduce with
http://svn.apache.org/r1545598. Throwing a PathNotFoundException in
SessionContext#getOakPathOrThrow() stretches that method's contract and
is clearly against its intent, which is mapping JCR path onto Oak path
if possible. Not deciding whether such a path exists at all.
Another way to look at this is to take it as more evidence that we need
better path handling aka OAK-1179.
Michael