Author: rhauch
Date: 2009-03-04 16:58:43 -0500 (Wed, 04 Mar 2009)
New Revision: 755
Modified:
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrProperty.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrMultiValueProperty.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSingleValueProperty.java
trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrValue.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/AbstractJcrPropertyTest.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JackrabbitJcrTckTest.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrMultiValuePropertyTest.java
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSingleValuePropertyTest.java
Log:
DNA-293 JcrSession creates Property instances that use the PropertyType.UNDEFINED, which
is not allowed
Changed JcrSession.populateNode(...) to never use PropertyType.UNDEFINED for the actual
property type on a property. This means that even though UNDEFINED is allows on
PropertyDefinition, it is not allowed on an actual Property instance. Therefore, I added
the actual property type as an int field to JcrAbstractProperty, and modified the
subclasses and the code where the constructors are called (mostly JcrSession and unit
tests).
The current choice is to use PropertyType.STRING, since all property values can be
converted to a String. See the issue for more details and a discussion of this logic.
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrProperty.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrProperty.java 2009-03-04
19:40:01 UTC (rev 754)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/AbstractJcrProperty.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -47,10 +47,12 @@
private final ExecutionContext executionContext;
private final org.jboss.dna.graph.property.Property dnaProperty;
private final PropertyDefinition jcrPropertyDefinition;
+ private final int propertyType;
AbstractJcrProperty( Node node,
ExecutionContext executionContext,
PropertyDefinition definition,
+ int propertyType,
org.jboss.dna.graph.property.Property dnaProperty ) {
assert node != null;
assert executionContext != null;
@@ -60,6 +62,7 @@
this.executionContext = executionContext;
this.dnaProperty = dnaProperty;
this.jcrPropertyDefinition = definition;
+ this.propertyType = propertyType;
}
/**
@@ -91,7 +94,7 @@
* @see javax.jcr.Property#getType()
*/
public final int getType() {
- return jcrPropertyDefinition.getRequiredType();
+ return propertyType;
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrMultiValueProperty.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrMultiValueProperty.java 2009-03-04
19:40:01 UTC (rev 754)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrMultiValueProperty.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -44,8 +44,9 @@
JcrMultiValueProperty( Node node,
ExecutionContext executionContext,
PropertyDefinition definition,
+ int propertyType,
Property dnaProperty ) {
- super(node, executionContext, definition, dnaProperty);
+ super(node, executionContext, definition, propertyType, dnaProperty);
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-03-04 19:40:01 UTC
(rev 754)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSession.java 2009-03-04 21:58:43 UTC
(rev 755)
@@ -633,7 +633,7 @@
case DATE:
return PropertyType.DATE;
case DECIMAL:
- return PropertyType.UNDEFINED;
+ return PropertyType.STRING; // better than losing information
case DOUBLE:
return PropertyType.DOUBLE;
case BINARY:
@@ -745,7 +745,8 @@
if (referenceable) {
if (uuidProperty == null) uuidProperty =
executionContext.getPropertyFactory().create(JcrLexicon.UUID, uuid);
PropertyDefinition propertyDefinition =
propertyDefinitionsByPropertyName.get(JcrLexicon.UUID);
- properties.add(new JcrSingleValueProperty(node, executionContext,
propertyDefinition, uuidProperty));
+ properties.add(new JcrSingleValueProperty(node, executionContext,
propertyDefinition, PropertyType.STRING,
+ uuidProperty));
}
// Now create the JCR property object wrappers around the other properties ...
@@ -805,11 +806,26 @@
continue;
}
+ // Figure out the property type ...
+ int propertyType = propertyDefinition.getRequiredType();
+ if (propertyType == PropertyType.UNDEFINED) {
+ // See DNA-293 for discussion. Best option is to use PropertyType.STRING,
since values can always
+ // be converted to Strings (and because clients might use
Property.getType() to decide how to
+ // manipulate or set values).
+ propertyType = PropertyType.STRING;
+
+ // // Or, we can choose the property type for the first value (or the
default value, if there is one) ...
+ // Object value = dnaProp.getFirstValue();
+ // org.jboss.dna.graph.property.PropertyType dnaType =
+ // org.jboss.dna.graph.property.PropertyType.discoverType(value);
+ // propertyType = jcrPropertyTypeFor(dnaType);
+ }
+
// Create the appropriate JCR property wrapper ...
if (isMultiple) {
- properties.add(new JcrMultiValueProperty(node, executionContext,
propertyDefinition, dnaProp));
+ properties.add(new JcrMultiValueProperty(node, executionContext,
propertyDefinition, propertyType, dnaProp));
} else {
- properties.add(new JcrSingleValueProperty(node, executionContext,
propertyDefinition, dnaProp));
+ properties.add(new JcrSingleValueProperty(node, executionContext,
propertyDefinition, propertyType, dnaProp));
}
}
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSingleValueProperty.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSingleValueProperty.java 2009-03-04
19:40:01 UTC (rev 754)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrSingleValueProperty.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -43,10 +43,11 @@
final class JcrSingleValueProperty extends AbstractJcrProperty {
JcrSingleValueProperty( Node node,
- ExecutionContext executionContext,
- PropertyDefinition definition,
- Property dnaProperty ) {
- super(node, executionContext, definition, dnaProperty);
+ ExecutionContext executionContext,
+ PropertyDefinition definition,
+ int propertyType,
+ Property dnaProperty ) {
+ super(node, executionContext, definition, propertyType, dnaProperty);
}
/**
Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrValue.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrValue.java 2009-03-04 19:40:01 UTC
(rev 754)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrValue.java 2009-03-04 21:58:43 UTC
(rev 755)
@@ -49,8 +49,7 @@
assert valueFactories != null;
assert type == PropertyType.BINARY || type == PropertyType.BOOLEAN || type ==
PropertyType.DATE
|| type == PropertyType.DOUBLE || type == PropertyType.LONG || type ==
PropertyType.NAME
- || type == PropertyType.PATH || type == PropertyType.REFERENCE || type ==
PropertyType.STRING
- || type == PropertyType.UNDEFINED;
+ || type == PropertyType.PATH || type == PropertyType.REFERENCE || type ==
PropertyType.STRING;
assert value != null;
this.valueFactories = valueFactories;
this.type = type;
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/AbstractJcrPropertyTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/AbstractJcrPropertyTest.java 2009-03-04
19:40:01 UTC (rev 754)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/AbstractJcrPropertyTest.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -234,7 +234,7 @@
ExecutionContext executionContext,
PropertyDefinition propertyDefinition,
org.jboss.dna.graph.property.Property dnaProperty ) {
- super(node, executionContext, propertyDefinition, dnaProperty);
+ super(node, executionContext, propertyDefinition,
propertyDefinition.getRequiredType(), dnaProperty);
}
/**
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JackrabbitJcrTckTest.java
===================================================================
--- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JackrabbitJcrTckTest.java 2009-03-04
19:40:01 UTC (rev 754)
+++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JackrabbitJcrTckTest.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -85,7 +85,7 @@
addTestSuite(org.apache.jackrabbit.test.api.PathPropertyTest.class);
addTestSuite(org.apache.jackrabbit.test.api.ReferencePropertyTest.class);
addTestSuite(org.apache.jackrabbit.test.api.StringPropertyTest.class);
- // addTestSuite(org.apache.jackrabbit.test.api.UndefinedPropertyTest.class);
+ addTestSuite(org.apache.jackrabbit.test.api.UndefinedPropertyTest.class);
addTestSuite(org.apache.jackrabbit.test.api.NamespaceRegistryReadMethodsTest.class);
addTestSuite(org.apache.jackrabbit.test.api.NamespaceRemappingTest.class);
addTestSuite(org.apache.jackrabbit.test.api.NodeIteratorTest.class);
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrMultiValuePropertyTest.java
===================================================================
---
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrMultiValuePropertyTest.java 2009-03-04
19:40:01 UTC (rev 754)
+++
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrMultiValuePropertyTest.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -59,7 +59,7 @@
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
true);
stub(definition.getRequiredType()).toReturn(PropertyType.BOOLEAN);
stub(definition.isMultiple()).toReturn(true);
- prop = new JcrMultiValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrMultiValueProperty(node, executionContext, definition,
definition.getRequiredType(), dnaProperty);
}
@Test
@@ -138,7 +138,7 @@
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
value);
stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
stub(definition.isMultiple()).toReturn(true);
- prop = new JcrMultiValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrMultiValueProperty(node, executionContext, definition,
definition.getRequiredType(), dnaProperty);
lengths = prop.getLengths();
assertThat(lengths, notNullValue());
assertThat(lengths.length, is(1));
@@ -149,7 +149,7 @@
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
value);
stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
stub(definition.isMultiple()).toReturn(true);
- prop = new JcrMultiValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrMultiValueProperty(node, executionContext, definition,
definition.getRequiredType(), dnaProperty);
lengths = prop.getLengths();
assertThat(lengths, notNullValue());
assertThat(lengths.length, is(1));
@@ -159,7 +159,7 @@
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
(Object[])values);
stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
stub(definition.isMultiple()).toReturn(true);
- prop = new JcrMultiValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrMultiValueProperty(node, executionContext, definition,
definition.getRequiredType(), dnaProperty);
lengths = prop.getLengths();
assertThat(lengths, notNullValue());
assertThat(lengths.length, is(values.length));
Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSingleValuePropertyTest.java
===================================================================
---
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSingleValuePropertyTest.java 2009-03-04
19:40:01 UTC (rev 754)
+++
trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrSingleValuePropertyTest.java 2009-03-04
21:58:43 UTC (rev 755)
@@ -65,16 +65,14 @@
MockitoAnnotations.initMocks(this);
executionContext = new ExecutionContext();
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
"text/plain");
- stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
stub(definition.isMultiple()).toReturn(false);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.STRING, dnaProperty);
}
@Test
public void shouldProvideBoolean() throws Exception {
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
"true");
- stub(definition.getRequiredType()).toReturn(PropertyType.BOOLEAN);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.BOOLEAN, dnaProperty);
assertThat(prop.getBoolean(), is(true));
assertThat(prop.getType(), is(PropertyType.BOOLEAN));
}
@@ -90,8 +88,7 @@
public void shouldProvideDate() throws Exception {
DateTime dnaDate =
executionContext.getValueFactories().getDateFactory().create();
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
dnaDate);
- stub(definition.getRequiredType()).toReturn(PropertyType.DATE);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.DATE, dnaProperty);
assertThat(prop.getDate(), is(dnaDate.toCalendar()));
assertThat(prop.getLong(), is(dnaDate.getMilliseconds()));
assertThat(prop.getType(), is(PropertyType.DATE));
@@ -102,12 +99,11 @@
public void shouldProvideNode() throws Exception {
UUID referencedUuid = UUID.randomUUID();
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
referencedUuid);
- stub(definition.getRequiredType()).toReturn(PropertyType.REFERENCE);
Node referencedNode = mock(Node.class);
JcrSession session = mock(JcrSession.class);
stub(node.getSession()).toReturn(session);
stub(session.getNode(referencedUuid)).toReturn(referencedNode);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.REFERENCE, dnaProperty);
assertThat(prop.getNode(), is(referencedNode));
assertThat(prop.getType(), is(PropertyType.REFERENCE));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -116,13 +112,12 @@
@Test
public void shouldProvideDouble() throws Exception {
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
1.0);
- stub(definition.getRequiredType()).toReturn(PropertyType.DOUBLE);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.DOUBLE, dnaProperty);
assertThat(prop.getDouble(), is(1.0));
assertThat(prop.getType(), is(PropertyType.DOUBLE));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
1.0F);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.DOUBLE, dnaProperty);
assertThat(prop.getDouble(), is(1.0));
assertThat(prop.getType(), is(PropertyType.DOUBLE));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -131,15 +126,13 @@
@Test
public void shouldProvideLong() throws Exception {
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
1);
- stub(definition.getRequiredType()).toReturn(PropertyType.DOUBLE);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.DOUBLE, dnaProperty);
assertThat(prop.getLong(), is(1L));
assertThat(prop.getType(), is(PropertyType.DOUBLE));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
1L);
- stub(definition.getRequiredType()).toReturn(PropertyType.DOUBLE);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.DOUBLE, dnaProperty);
assertThat(prop.getLong(), is(1L));
assertThat(prop.getString(), is("1"));
assertThat(prop.getType(), is(PropertyType.DOUBLE));
@@ -149,8 +142,7 @@
@Test
public void shouldProvideStream() throws Exception {
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
new Object());
- stub(definition.getRequiredType()).toReturn(PropertyType.BINARY);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.BINARY, dnaProperty);
assertThat(prop.getType(), is(PropertyType.BINARY));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
InputStream stream = prop.getStream();
@@ -166,8 +158,7 @@
@Test
public void shouldProvideString() throws Exception {
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
"value");
- stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.STRING, dnaProperty);
assertThat(prop.getString(), is("value"));
assertThat(prop.getType(), is(PropertyType.STRING));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -177,8 +168,7 @@
public void shouldAllowReferenceValue() throws Exception {
UUID uuid = UUID.randomUUID();
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
uuid);
- stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.STRING, dnaProperty);
assertThat(prop.getString(), is(uuid.toString()));
assertThat(prop.getType(), is(PropertyType.STRING));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -189,8 +179,7 @@
executionContext.getNamespaceRegistry().register("acme",
"http://example.com");
Name path =
executionContext.getValueFactories().getNameFactory().create("acme:something");
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
path);
- stub(definition.getRequiredType()).toReturn(PropertyType.NAME);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.NAME, dnaProperty);
assertThat(prop.getString(), is("acme:something"));
assertThat(prop.getType(), is(PropertyType.NAME));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -204,8 +193,7 @@
executionContext.getNamespaceRegistry().register("acme",
"http://example.com");
Path path =
executionContext.getValueFactories().getPathFactory().create("/a/b/acme:c");
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
(Object)path);
- stub(definition.getRequiredType()).toReturn(PropertyType.PATH);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.PATH, dnaProperty);
assertThat(prop.getString(), is("/a/b/acme:c"));
assertThat(prop.getType(), is(PropertyType.PATH));
assertThat(prop.getName(),
is(dnaProperty.getName().getString(executionContext.getNamespaceRegistry())));
@@ -232,15 +220,13 @@
dnaValue = "some other value";
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
dnaValue);
- stub(definition.getRequiredType()).toReturn(PropertyType.STRING);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.STRING, dnaProperty);
assertThat(prop.getLength(), is((long)dnaValue.length()));
Object obj = new Object();
long binaryLength =
executionContext.getValueFactories().getBinaryFactory().create(obj).getSize();
dnaProperty = executionContext.getPropertyFactory().create(JcrLexicon.MIMETYPE,
obj);
- stub(definition.getRequiredType()).toReturn(PropertyType.BINARY);
- prop = new JcrSingleValueProperty(node, executionContext, definition,
dnaProperty);
+ prop = new JcrSingleValueProperty(node, executionContext, definition,
PropertyType.BINARY, dnaProperty);
assertThat(prop.getLength(), is(binaryLength));
}