[dna-commits] DNA SVN: r755 - in trunk/dna-jcr/src: test/java/org/jboss/dna/jcr and 1 other directory.

dna-commits at lists.jboss.org dna-commits at lists.jboss.org
Wed Mar 4 16:58:43 EST 2009


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));
     }
 




More information about the dna-commits mailing list