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 re...@apache.org on 2012/05/09 17:52:35 UTC
svn commit: r1336250 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/
oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
oak-jcr/src/main/java/org/apache/ja...
Author: reschke
Date: Wed May 9 15:52:35 2012
New Revision: 1336250
URL: http://svn.apache.org/viewvc?rev=1336250&view=rev
Log:
OAK-6: add test content for property tests; add value conversion for dates, handle "." paths in path-typed properties
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java Wed May 9 15:52:35 2012
@@ -143,6 +143,11 @@ public class NamePathMapperImpl implemen
}
}
+ // empty path: map to "."
+ if (oakPath.length() == 0) {
+ return ".";
+ }
+
// root path is special-cased early on so it does not need to
// be considered here
oakPath.deleteCharAt(oakPath.length() - 1);
@@ -228,6 +233,11 @@ public class NamePathMapperImpl implemen
}
}
+ // empty path: map to "."
+ if (jcrPath.length() == 0) {
+ return ".";
+ }
+
jcrPath.deleteCharAt(jcrPath.length() - 1);
return jcrPath.toString();
}
Modified: jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java?rev=1336250&r1=1336249&r2=1336250&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java (original)
+++ jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java Wed May 9 15:52:35 2012
@@ -16,16 +16,22 @@
*/
package org.apache.jackrabbit.oak.jcr;
+import java.util.Calendar;
+
import javax.jcr.Node;
import javax.jcr.PathNotFoundException;
+import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import javax.jcr.ValueFactory;
public class TestContentLoader {
public void loadTestContent(Session session) throws RepositoryException {
- getOrAddNode(session.getRootNode(), "testdata");
+ Node data = getOrAddNode(session.getRootNode(), "testdata");
+ addPropertyTestData(getOrAddNode(data, "property"));
+
session.save();
}
@@ -36,4 +42,20 @@ public class TestContentLoader {
return node.addNode(name);
}
}
+
+ /**
+ * Creates a boolean, double, long, calendar and a path property at the
+ * given node.
+ */
+ private void addPropertyTestData(Node node) throws RepositoryException {
+ node.setProperty("boolean", true);
+ node.setProperty("double", Math.PI);
+ node.setProperty("long", 90834953485278298l);
+ Calendar c = Calendar.getInstance();
+ c.set(2005, 6, 18, 17, 30);
+ node.setProperty("calendar", c);
+ ValueFactory factory = node.getSession().getValueFactory();
+ node.setProperty("path", factory.createValue("/", PropertyType.PATH));
+ node.setProperty("multi", new String[] { "one", "two", "three" });
+ }
}
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Wed May 9 15:52:35 2012
@@ -365,6 +365,12 @@ public class NodeImpl extends ItemImpl i
checkStatus();
String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
+
+ // TODO: hack
+ if (".".equals(oakPath)) {
+ return this;
+ }
+
NodeDelegate nd = dlg.getChild(oakPath);
if (nd == null) {
throw new PathNotFoundException(relPath);
Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java Wed May 9 15:52:35 2012
@@ -113,7 +113,13 @@ class ValueImpl implements Value {
@Override
public BigDecimal getDecimal() throws RepositoryException {
try {
- return value.getDecimal();
+ switch (getType()) {
+ case PropertyType.DATE:
+ Calendar cal = getDate();
+ return BigDecimal.valueOf(cal.getTimeInMillis());
+ default:
+ return value.getDecimal();
+ }
} catch (NumberFormatException e) {
throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
}
@@ -125,7 +131,13 @@ class ValueImpl implements Value {
@Override
public double getDouble() throws RepositoryException {
try {
- return value.getDouble();
+ switch (getType()) {
+ case PropertyType.DATE:
+ Calendar cal = getDate();
+ return cal.getTimeInMillis();
+ default:
+ return value.getDouble();
+ }
} catch (NumberFormatException e) {
throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
}
@@ -137,7 +149,13 @@ class ValueImpl implements Value {
@Override
public long getLong() throws RepositoryException {
try {
- return value.getLong();
+ switch (getType()) {
+ case PropertyType.DATE:
+ Calendar cal = getDate();
+ return cal.getTimeInMillis();
+ default:
+ return value.getLong();
+ }
} catch (NumberFormatException e) {
throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
}
Re: value conversions, and relative paths in Oak
Posted by Michael Dürig <md...@apache.org>.
Hi,
See attached patch for how I'd solve the issue for the current path.
Currently this causes
testGetNode(org.apache.jackrabbit.test.api.PathPropertyTest) to fail
which is due to the path value not being stored correctly (i.e. as oak
path instead of as jcr path). We'd need to complete the Value related
implementations first to support this.
For paths containing .. I think we could do something along the lines of:
absPath = PathUtils.concat(this.path, relPath);
normalizedAbsPath = getOakPath(absPath)
normalizedRelPath = PathUtils.relativize(this.path, normalizedAbsPath);
Michael
On 9.5.12 17:00, Julian Reschke wrote:
> Hi,
>
> I just discovered that we have tests that do not do anything meaningful
> because we didn't have test data in the repo. I just fixed that for
> ./property (see TestContentLoader).
>
> This exposed three issues:
>
> 1) missing Date->long/double/decimal value conversions. I have added
> those for now in ValueImpl (not core); is this correct, Angela?
>
> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
> sure I got the mapping correct. What is a same-node reference in Oak
> path syntax? "." or empty string? I also fear that we need to be able to
> round-trip relative paths like "../foo/bar" in path values, which we
> currently do not. Do we need a "normalize" flag???
>
> 3) Node.getNode(".") failed because it looked for a child called ".". If
> have added a hack to make this work, but I'm wondering how to do this
> properly.
>
> Best regards, Julian
>
>
>
> On 2012-05-09 17:52, reschke@apache.org wrote:
>> Author: reschke
>> Date: Wed May 9 15:52:35 2012
>> New Revision: 1336250
>>
>> URL: http://svn.apache.org/viewvc?rev=1336250&view=rev
>> Log:
>> OAK-6: add test content for property tests; add value conversion for
>> dates, handle "." paths in path-typed properties
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>>
>> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>>
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>>
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>>
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>>
>> URL:
>> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>>
>> ==============================================================================
>>
>> ---
>> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>> (original)
>> +++
>> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>> Wed May 9 15:52:35 2012
>> @@ -143,6 +143,11 @@ public class NamePathMapperImpl implemen
>> }
>> }
>>
>> + // empty path: map to "."
>> + if (oakPath.length() == 0) {
>> + return ".";
>> + }
>> +
>> // root path is special-cased early on so it does not need to
>> // be considered here
>> oakPath.deleteCharAt(oakPath.length() - 1);
>> @@ -228,6 +233,11 @@ public class NamePathMapperImpl implemen
>> }
>> }
>>
>> + // empty path: map to "."
>> + if (jcrPath.length() == 0) {
>> + return ".";
>> + }
>> +
>> jcrPath.deleteCharAt(jcrPath.length() - 1);
>> return jcrPath.toString();
>> }
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>>
>> URL:
>> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>>
>> ==============================================================================
>>
>> ---
>> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>> (original)
>> +++
>> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>> Wed May 9 15:52:35 2012
>> @@ -16,16 +16,22 @@
>> */
>> package org.apache.jackrabbit.oak.jcr;
>>
>> +import java.util.Calendar;
>> +
>> import javax.jcr.Node;
>> import javax.jcr.PathNotFoundException;
>> +import javax.jcr.PropertyType;
>> import javax.jcr.RepositoryException;
>> import javax.jcr.Session;
>> +import javax.jcr.ValueFactory;
>>
>> public class TestContentLoader {
>>
>> public void loadTestContent(Session session) throws RepositoryException {
>>
>> - getOrAddNode(session.getRootNode(), "testdata");
>> + Node data = getOrAddNode(session.getRootNode(), "testdata");
>> + addPropertyTestData(getOrAddNode(data, "property"));
>> +
>> session.save();
>> }
>>
>> @@ -36,4 +42,20 @@ public class TestContentLoader {
>> return node.addNode(name);
>> }
>> }
>> +
>> + /**
>> + * Creates a boolean, double, long, calendar and a path property at the
>> + * given node.
>> + */
>> + private void addPropertyTestData(Node node) throws
>> RepositoryException {
>> + node.setProperty("boolean", true);
>> + node.setProperty("double", Math.PI);
>> + node.setProperty("long", 90834953485278298l);
>> + Calendar c = Calendar.getInstance();
>> + c.set(2005, 6, 18, 17, 30);
>> + node.setProperty("calendar", c);
>> + ValueFactory factory = node.getSession().getValueFactory();
>> + node.setProperty("path", factory.createValue("/", PropertyType.PATH));
>> + node.setProperty("multi", new String[] { "one", "two", "three" });
>> + }
>> }
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>>
>> URL:
>> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>>
>> ==============================================================================
>>
>> ---
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>> (original)
>> +++
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>> Wed May 9 15:52:35 2012
>> @@ -365,6 +365,12 @@ public class NodeImpl extends ItemImpl i
>> checkStatus();
>>
>> String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
>> +
>> + // TODO: hack
>> + if (".".equals(oakPath)) {
>> + return this;
>> + }
>> +
>> NodeDelegate nd = dlg.getChild(oakPath);
>> if (nd == null) {
>> throw new PathNotFoundException(relPath);
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>>
>> URL:
>> http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>>
>> ==============================================================================
>>
>> ---
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>> (original)
>> +++
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>> Wed May 9 15:52:35 2012
>> @@ -113,7 +113,13 @@ class ValueImpl implements Value {
>> @Override
>> public BigDecimal getDecimal() throws RepositoryException {
>> try {
>> - return value.getDecimal();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return BigDecimal.valueOf(cal.getTimeInMillis());
>> + default:
>> + return value.getDecimal();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " +
>> PropertyType.nameFromValue(getType()));
>> }
>> @@ -125,7 +131,13 @@ class ValueImpl implements Value {
>> @Override
>> public double getDouble() throws RepositoryException {
>> try {
>> - return value.getDouble();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return cal.getTimeInMillis();
>> + default:
>> + return value.getDouble();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " +
>> PropertyType.nameFromValue(getType()));
>> }
>> @@ -137,7 +149,13 @@ class ValueImpl implements Value {
>> @Override
>> public long getLong() throws RepositoryException {
>> try {
>> - return value.getLong();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return cal.getTimeInMillis();
>> + default:
>> + return value.getLong();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " +
>> PropertyType.nameFromValue(getType()));
>> }
>>
>>
>>
>
Re: value conversions, and relative paths in Oak
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, May 10, 2012 at 2:14 PM, Julian Reschke <ju...@gmx.de> wrote:
> I think this means we need a variant of getOakPath() that doesn't make the
> path absolute. Any preferences about whether this should be a flag or a new
> method?
I'd ideally keep these tasks clearly separate. Have a PathMapper that
only takes care of mapping namespace prefixes and URIs to respective
Oak prefixes, and a PathResolver that takes such a mapped path and
resolves it against a given context.
BR,
Jukka Zitting
Re: value conversions, and relative paths in Oak
Posted by Julian Reschke <ju...@gmx.de>.
On 2012-05-10 14:14, Julian Reschke wrote:
> ...
> OK, trying to summarize:
>
> - we allow relative paths inside path values; but we need to oakify them
> with respect to prefixes
>
> - open question: we allow relative paths such as ".." but does that
> imply we do not normalize at all?
>
> - relative paths in other methods (such as getNode()) are resolved to an
> absolute Oak path in oak-jcr
>
> I think this means we need a variant of getOakPath() that doesn't make
> the path absolute. Any preferences about whether this should be a flag
> or a new method?
>
> Best regards, Julian
I just chatted with Michael, and the revised plan is to let getOakPath()
to normalize relative paths as much as possible.
Which means that an attempt to set a path value to
a/../b
would actually persist
b
I believe that's ok for real-world use. I also checked the spec and it
appears to be silent on that matter...
Best regards, Julian
Re: value conversions, and relative paths in Oak
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
>>My thinking was to make this opaque to oak-core. That is, set the value
>> of the path property to the jcr path and let oak-jcr handle it. Unless
>> there is a case (which might well be) where we need to dereference such
>> paths properties from within oak-core this should work.
I don't think this will work for the query engine. Similar to namespace
mapping, node types, the query engine in oak-core will probably have to
resolve relative path as well, maybe even at execution time.
Regards,
Thomas
Re: value conversions, and relative paths in Oak
Posted by Julian Reschke <ju...@gmx.de>.
On 2012-05-10 11:00, Michael Dürig wrote:
> On 10.5.12 9:43, Julian Reschke wrote:
>> On 2012-05-10 10:29, Michael Dürig wrote:
>>>>>> 2) we didn't round-trip a relative path of ".". Now we do, but I'm
>>>>>> not
>>>>>> sure I got the mapping correct. What is a same-node reference in Oak
>>>>>> path syntax? "." or empty string?
>>>>>
>>>>> well, i would expect "."
>>>>
>>>> Indeed.
>>>
>>> Currently there is no notion for .. and . in Oak paths and I would leave
>>> it at that. I.e. oak-jcr needs to take care to normalize paths before it
>>> passes them to the Oak API. But of course we should use the
>>> JcrPathParser to do that.
>>
>> So how do you express a reference to the parent node in a path property
>> inside Oak Core?
>
> My thinking was to make this opaque to oak-core. That is, set the value
> of the path property to the jcr path and let oak-jcr handle it. Unless
> there is a case (which might well be) where we need to dereference such
> paths properties from within oak-core this should work.
I don't think that's going to work as then the persisted path depends on
session namespace mappings.
>>>>>> I also fear that we need to be able to
>>>>>> round-trip relative paths like "../foo/bar" in path values, which we
>>>>>> currently do not. Do we need a "normalize" flag???
>>>>>
>>>>> i would expect that we need it sooner or later.
>>>>
>>>> +1
>>>
>>> The JcrPathParser does already normalise. What it can't do is
>>> normalising "negative" paths like ../foo or foo/../..
>>
>> Yup. What I meant is that we may need a way to switch off normalizing.
>
> Ah ok. That shouldn't be difficult.
>
>>
>>> See my other mail how that could be handled with the current setup.
>>> Alternatively we could also have the JcrPathParser accept a prefix path
>>> against which normalisation is done.
>>
>> This might break assumptions on stability of path property values. If
>> the client sets a relative path it probably expects it to stay relative,
>> so that the path continues to work when the node is moved.
>
> I was just talking of resolving it (under the above opaqueness
> assumption). I didn't mean to save that dereferenced and normalized path
> to the path property. That surely doesn't work.
OK, trying to summarize:
- we allow relative paths inside path values; but we need to oakify them
with respect to prefixes
- open question: we allow relative paths such as ".." but does that
imply we do not normalize at all?
- relative paths in other methods (such as getNode()) are resolved to an
absolute Oak path in oak-jcr
I think this means we need a variant of getOakPath() that doesn't make
the path absolute. Any preferences about whether this should be a flag
or a new method?
Best regards, Julian
Re: value conversions, and relative paths in Oak
Posted by Michael Dürig <md...@apache.org>.
On 10.5.12 9:43, Julian Reschke wrote:
> On 2012-05-10 10:29, Michael Dürig wrote:
>>>>> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
>>>>> sure I got the mapping correct. What is a same-node reference in Oak
>>>>> path syntax? "." or empty string?
>>>>
>>>> well, i would expect "."
>>>
>>> Indeed.
>>
>> Currently there is no notion for .. and . in Oak paths and I would leave
>> it at that. I.e. oak-jcr needs to take care to normalize paths before it
>> passes them to the Oak API. But of course we should use the
>> JcrPathParser to do that.
>
> So how do you express a reference to the parent node in a path property
> inside Oak Core?
My thinking was to make this opaque to oak-core. That is, set the value
of the path property to the jcr path and let oak-jcr handle it. Unless
there is a case (which might well be) where we need to dereference such
paths properties from within oak-core this should work.
>
>>>>> I also fear that we need to be able to
>>>>> round-trip relative paths like "../foo/bar" in path values, which we
>>>>> currently do not. Do we need a "normalize" flag???
>>>>
>>>> i would expect that we need it sooner or later.
>>>
>>> +1
>>
>> The JcrPathParser does already normalise. What it can't do is
>> normalising "negative" paths like ../foo or foo/../..
>
> Yup. What I meant is that we may need a way to switch off normalizing.
Ah ok. That shouldn't be difficult.
>
>> See my other mail how that could be handled with the current setup.
>> Alternatively we could also have the JcrPathParser accept a prefix path
>> against which normalisation is done.
>
> This might break assumptions on stability of path property values. If
> the client sets a relative path it probably expects it to stay relative,
> so that the path continues to work when the node is moved.
I was just talking of resolving it (under the above opaqueness
assumption). I didn't mean to save that dereferenced and normalized path
to the path property. That surely doesn't work.
Michael
>
>>>>> 3) Node.getNode(".") failed because it looked for a child called
>>>>> ".". If
>>>>> have added a hack to make this work, but I'm wondering how to do this
>>>>> properly.
>>>>
>>>> imo oak paths should always be normalized except for path values.
>>>
>>> Which means that we need to expose that in the path mapping API, right?
>>
>> In the sense I described above yes.
>>
>> Michael
>>
>
Re: value conversions, and relative paths in Oak
Posted by Julian Reschke <ju...@gmx.de>.
On 2012-05-10 10:29, Michael Dürig wrote:
>>>> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
>>>> sure I got the mapping correct. What is a same-node reference in Oak
>>>> path syntax? "." or empty string?
>>>
>>> well, i would expect "."
>>
>> Indeed.
>
> Currently there is no notion for .. and . in Oak paths and I would leave
> it at that. I.e. oak-jcr needs to take care to normalize paths before it
> passes them to the Oak API. But of course we should use the
> JcrPathParser to do that.
So how do you express a reference to the parent node in a path property
inside Oak Core?
>>>> I also fear that we need to be able to
>>>> round-trip relative paths like "../foo/bar" in path values, which we
>>>> currently do not. Do we need a "normalize" flag???
>>>
>>> i would expect that we need it sooner or later.
>>
>> +1
>
> The JcrPathParser does already normalise. What it can't do is
> normalising "negative" paths like ../foo or foo/../..
Yup. What I meant is that we may need a way to switch off normalizing.
> See my other mail how that could be handled with the current setup.
> Alternatively we could also have the JcrPathParser accept a prefix path
> against which normalisation is done.
This might break assumptions on stability of path property values. If
the client sets a relative path it probably expects it to stay relative,
so that the path continues to work when the node is moved.
>>>> 3) Node.getNode(".") failed because it looked for a child called
>>>> ".". If
>>>> have added a hack to make this work, but I'm wondering how to do this
>>>> properly.
>>>
>>> imo oak paths should always be normalized except for path values.
>>
>> Which means that we need to expose that in the path mapping API, right?
>
> In the sense I described above yes.
>
> Michael
>
Re: value conversions, and relative paths in Oak
Posted by Michael Dürig <md...@apache.org>.
>>> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
>>> sure I got the mapping correct. What is a same-node reference in Oak
>>> path syntax? "." or empty string?
>>
>> well, i would expect "."
>
> Indeed.
Currently there is no notion for .. and . in Oak paths and I would leave
it at that. I.e. oak-jcr needs to take care to normalize paths before it
passes them to the Oak API. But of course we should use the
JcrPathParser to do that.
>
>>> I also fear that we need to be able to
>>> round-trip relative paths like "../foo/bar" in path values, which we
>>> currently do not. Do we need a "normalize" flag???
>>
>> i would expect that we need it sooner or later.
>
> +1
The JcrPathParser does already normalise. What it can't do is
normalising "negative" paths like ../foo or foo/../..
See my other mail how that could be handled with the current setup.
Alternatively we could also have the JcrPathParser accept a prefix path
against which normalisation is done.
>
>>> 3) Node.getNode(".") failed because it looked for a child called ".". If
>>> have added a hack to make this work, but I'm wondering how to do this
>>> properly.
>>
>> imo oak paths should always be normalized except for path values.
>
> Which means that we need to expose that in the path mapping API, right?
In the sense I described above yes.
Michael
Re: value conversions, and relative paths in Oak
Posted by Julian Reschke <ju...@gmx.de>.
On 2012-05-10 08:46, Angela Schreiber wrote:
> hi julian
>
>> 1) missing Date->long/double/decimal value conversions. I have added
>> those for now in ValueImpl (not core); is this correct, Angela?
>
> as long as we don't need it in the core, i would add it in oak-jcr.
> we can still push i down to the CoreValue later on.
That's where I put it for now.
> the basic question is again what part of the validation should be
> in which layer... that's a matter of definition rather than right
> or wrong.
>
>> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
>> sure I got the mapping correct. What is a same-node reference in Oak
>> path syntax? "." or empty string?
>
> well, i would expect "."
Indeed.
>> I also fear that we need to be able to
>> round-trip relative paths like "../foo/bar" in path values, which we
>> currently do not. Do we need a "normalize" flag???
>
> i would expect that we need it sooner or later.
+1
>> 3) Node.getNode(".") failed because it looked for a child called ".". If
>> have added a hack to make this work, but I'm wondering how to do this
>> properly.
>
> imo oak paths should always be normalized except for path values.
Which means that we need to expose that in the path mapping API, right?
Re: value conversions, and relative paths in Oak
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
>>1) missing Date->long/double/decimal value conversions. I have added
>> those for now in ValueImpl (not core); is this correct, Angela?
>
>as long as we don't need it in the core, i would add it in oak-jcr.
>we can still push i down to the CoreValue later on.
Value conversions are (or at least will be) used in the query engine, in
oak-core.
>the basic question is again what part of the validation should be
>in which layer... that's a matter of definition rather than right
>or wrong.
I thought it's about converting not validation
Regards,
Thomas
Re: value conversions, and relative paths in Oak
Posted by Angela Schreiber <an...@adobe.com>.
hi julian
> 1) missing Date->long/double/decimal value conversions. I have added
> those for now in ValueImpl (not core); is this correct, Angela?
as long as we don't need it in the core, i would add it in oak-jcr.
we can still push i down to the CoreValue later on.
the basic question is again what part of the validation should be
in which layer... that's a matter of definition rather than right
or wrong.
> 2) we didn't round-trip a relative path of ".". Now we do, but I'm not
> sure I got the mapping correct. What is a same-node reference in Oak
> path syntax? "." or empty string?
well, i would expect "."
> I also fear that we need to be able to
> round-trip relative paths like "../foo/bar" in path values, which we
> currently do not. Do we need a "normalize" flag???
i would expect that we need it sooner or later.
> 3) Node.getNode(".") failed because it looked for a child called ".". If
> have added a hack to make this work, but I'm wondering how to do this
> properly.
imo oak paths should always be normalized except for path values.
kind regards
angela
> Best regards, Julian
>
>
>
> On 2012-05-09 17:52, reschke@apache.org wrote:
>> Author: reschke
>> Date: Wed May 9 15:52:35 2012
>> New Revision: 1336250
>>
>> URL: http://svn.apache.org/viewvc?rev=1336250&view=rev
>> Log:
>> OAK-6: add test content for property tests; add value conversion for dates, handle "." paths in path-typed properties
>>
>> Modified:
>> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>>
>> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
>> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>> ==============================================================================
>> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java (original)
>> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java Wed May 9 15:52:35 2012
>> @@ -143,6 +143,11 @@ public class NamePathMapperImpl implemen
>> }
>> }
>>
>> + // empty path: map to "."
>> + if (oakPath.length() == 0) {
>> + return ".";
>> + }
>> +
>> // root path is special-cased early on so it does not need to
>> // be considered here
>> oakPath.deleteCharAt(oakPath.length() - 1);
>> @@ -228,6 +233,11 @@ public class NamePathMapperImpl implemen
>> }
>> }
>>
>> + // empty path: map to "."
>> + if (jcrPath.length() == 0) {
>> + return ".";
>> + }
>> +
>> jcrPath.deleteCharAt(jcrPath.length() - 1);
>> return jcrPath.toString();
>> }
>>
>> Modified: jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
>> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>> ==============================================================================
>> --- jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java (original)
>> +++ jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java Wed May 9 15:52:35 2012
>> @@ -16,16 +16,22 @@
>> */
>> package org.apache.jackrabbit.oak.jcr;
>>
>> +import java.util.Calendar;
>> +
>> import javax.jcr.Node;
>> import javax.jcr.PathNotFoundException;
>> +import javax.jcr.PropertyType;
>> import javax.jcr.RepositoryException;
>> import javax.jcr.Session;
>> +import javax.jcr.ValueFactory;
>>
>> public class TestContentLoader {
>>
>> public void loadTestContent(Session session) throws RepositoryException {
>>
>> - getOrAddNode(session.getRootNode(), "testdata");
>> + Node data = getOrAddNode(session.getRootNode(), "testdata");
>> + addPropertyTestData(getOrAddNode(data, "property"));
>> +
>> session.save();
>> }
>>
>> @@ -36,4 +42,20 @@ public class TestContentLoader {
>> return node.addNode(name);
>> }
>> }
>> +
>> + /**
>> + * Creates a boolean, double, long, calendar and a path property at the
>> + * given node.
>> + */
>> + private void addPropertyTestData(Node node) throws RepositoryException {
>> + node.setProperty("boolean", true);
>> + node.setProperty("double", Math.PI);
>> + node.setProperty("long", 90834953485278298l);
>> + Calendar c = Calendar.getInstance();
>> + c.set(2005, 6, 18, 17, 30);
>> + node.setProperty("calendar", c);
>> + ValueFactory factory = node.getSession().getValueFactory();
>> + node.setProperty("path", factory.createValue("/", PropertyType.PATH));
>> + node.setProperty("multi", new String[] { "one", "two", "three" });
>> + }
>> }
>>
>> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
>> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>> ==============================================================================
>> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
>> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Wed May 9 15:52:35 2012
>> @@ -365,6 +365,12 @@ public class NodeImpl extends ItemImpl i
>> checkStatus();
>>
>> String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
>> +
>> + // TODO: hack
>> + if (".".equals(oakPath)) {
>> + return this;
>> + }
>> +
>> NodeDelegate nd = dlg.getChild(oakPath);
>> if (nd == null) {
>> throw new PathNotFoundException(relPath);
>>
>> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
>> ==============================================================================
>> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java (original)
>> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java Wed May 9 15:52:35 2012
>> @@ -113,7 +113,13 @@ class ValueImpl implements Value {
>> @Override
>> public BigDecimal getDecimal() throws RepositoryException {
>> try {
>> - return value.getDecimal();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return BigDecimal.valueOf(cal.getTimeInMillis());
>> + default:
>> + return value.getDecimal();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
>> }
>> @@ -125,7 +131,13 @@ class ValueImpl implements Value {
>> @Override
>> public double getDouble() throws RepositoryException {
>> try {
>> - return value.getDouble();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return cal.getTimeInMillis();
>> + default:
>> + return value.getDouble();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
>> }
>> @@ -137,7 +149,13 @@ class ValueImpl implements Value {
>> @Override
>> public long getLong() throws RepositoryException {
>> try {
>> - return value.getLong();
>> + switch (getType()) {
>> + case PropertyType.DATE:
>> + Calendar cal = getDate();
>> + return cal.getTimeInMillis();
>> + default:
>> + return value.getLong();
>> + }
>> } catch (NumberFormatException e) {
>> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
>> }
>>
>>
>>
>
value conversions, and relative paths in Oak
Posted by Julian Reschke <ju...@gmx.de>.
Hi,
I just discovered that we have tests that do not do anything meaningful
because we didn't have test data in the repo. I just fixed that for
./property (see TestContentLoader).
This exposed three issues:
1) missing Date->long/double/decimal value conversions. I have added
those for now in ValueImpl (not core); is this correct, Angela?
2) we didn't round-trip a relative path of ".". Now we do, but I'm not
sure I got the mapping correct. What is a same-node reference in Oak
path syntax? "." or empty string? I also fear that we need to be able to
round-trip relative paths like "../foo/bar" in path values, which we
currently do not. Do we need a "normalize" flag???
3) Node.getNode(".") failed because it looked for a child called ".". If
have added a hack to make this work, but I'm wondering how to do this
properly.
Best regards, Julian
On 2012-05-09 17:52, reschke@apache.org wrote:
> Author: reschke
> Date: Wed May 9 15:52:35 2012
> New Revision: 1336250
>
> URL: http://svn.apache.org/viewvc?rev=1336250&view=rev
> Log:
> OAK-6: add test content for property tests; add value conversion for dates, handle "." paths in path-typed properties
>
> Modified:
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
> jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
> jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java Wed May 9 15:52:35 2012
> @@ -143,6 +143,11 @@ public class NamePathMapperImpl implemen
> }
> }
>
> + // empty path: map to "."
> + if (oakPath.length() == 0) {
> + return ".";
> + }
> +
> // root path is special-cased early on so it does not need to
> // be considered here
> oakPath.deleteCharAt(oakPath.length() - 1);
> @@ -228,6 +233,11 @@ public class NamePathMapperImpl implemen
> }
> }
>
> + // empty path: map to "."
> + if (jcrPath.length() == 0) {
> + return ".";
> + }
> +
> jcrPath.deleteCharAt(jcrPath.length() - 1);
> return jcrPath.toString();
> }
>
> Modified: jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java?rev=1336250&r1=1336249&r2=1336250&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java (original)
> +++ jackrabbit/oak/trunk/oak-it/jcr/src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java Wed May 9 15:52:35 2012
> @@ -16,16 +16,22 @@
> */
> package org.apache.jackrabbit.oak.jcr;
>
> +import java.util.Calendar;
> +
> import javax.jcr.Node;
> import javax.jcr.PathNotFoundException;
> +import javax.jcr.PropertyType;
> import javax.jcr.RepositoryException;
> import javax.jcr.Session;
> +import javax.jcr.ValueFactory;
>
> public class TestContentLoader {
>
> public void loadTestContent(Session session) throws RepositoryException {
>
> - getOrAddNode(session.getRootNode(), "testdata");
> + Node data = getOrAddNode(session.getRootNode(), "testdata");
> + addPropertyTestData(getOrAddNode(data, "property"));
> +
> session.save();
> }
>
> @@ -36,4 +42,20 @@ public class TestContentLoader {
> return node.addNode(name);
> }
> }
> +
> + /**
> + * Creates a boolean, double, long, calendar and a path property at the
> + * given node.
> + */
> + private void addPropertyTestData(Node node) throws RepositoryException {
> + node.setProperty("boolean", true);
> + node.setProperty("double", Math.PI);
> + node.setProperty("long", 90834953485278298l);
> + Calendar c = Calendar.getInstance();
> + c.set(2005, 6, 18, 17, 30);
> + node.setProperty("calendar", c);
> + ValueFactory factory = node.getSession().getValueFactory();
> + node.setProperty("path", factory.createValue("/", PropertyType.PATH));
> + node.setProperty("multi", new String[] { "one", "two", "three" });
> + }
> }
>
> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Wed May 9 15:52:35 2012
> @@ -365,6 +365,12 @@ public class NodeImpl extends ItemImpl i
> checkStatus();
>
> String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
> +
> + // TODO: hack
> + if (".".equals(oakPath)) {
> + return this;
> + }
> +
> NodeDelegate nd = dlg.getChild(oakPath);
> if (nd == null) {
> throw new PathNotFoundException(relPath);
>
> Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java?rev=1336250&r1=1336249&r2=1336250&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java (original)
> +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueImpl.java Wed May 9 15:52:35 2012
> @@ -113,7 +113,13 @@ class ValueImpl implements Value {
> @Override
> public BigDecimal getDecimal() throws RepositoryException {
> try {
> - return value.getDecimal();
> + switch (getType()) {
> + case PropertyType.DATE:
> + Calendar cal = getDate();
> + return BigDecimal.valueOf(cal.getTimeInMillis());
> + default:
> + return value.getDecimal();
> + }
> } catch (NumberFormatException e) {
> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
> }
> @@ -125,7 +131,13 @@ class ValueImpl implements Value {
> @Override
> public double getDouble() throws RepositoryException {
> try {
> - return value.getDouble();
> + switch (getType()) {
> + case PropertyType.DATE:
> + Calendar cal = getDate();
> + return cal.getTimeInMillis();
> + default:
> + return value.getDouble();
> + }
> } catch (NumberFormatException e) {
> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
> }
> @@ -137,7 +149,13 @@ class ValueImpl implements Value {
> @Override
> public long getLong() throws RepositoryException {
> try {
> - return value.getLong();
> + switch (getType()) {
> + case PropertyType.DATE:
> + Calendar cal = getDate();
> + return cal.getTimeInMillis();
> + default:
> + return value.getLong();
> + }
> } catch (NumberFormatException e) {
> throw new ValueFormatException("Incompatible type " + PropertyType.nameFromValue(getType()));
> }
>
>
>