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 md...@apache.org on 2013/03/27 14:30:14 UTC

svn commit: r1461570 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: ItemImpl.java NodeImpl.java PropertyImpl.java

Author: mduerig
Date: Wed Mar 27 13:30:14 2013
New Revision: 1461570

URL: http://svn.apache.org/r1461570
Log:
OAK-672: Avoid JCR APIs calling other JCR APIs
OAK-662: Reduce boilerplate code in JCR impl methods
Factor property state creation into common method

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.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/PropertyImpl.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java?rev=1461570&r1=1461569&r2=1461570&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java Wed Mar 27 13:30:14 2013
@@ -16,6 +16,8 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import java.util.Arrays;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.jcr.AccessDeniedException;
@@ -23,21 +25,28 @@ import javax.jcr.Item;
 import javax.jcr.ItemNotFoundException;
 import javax.jcr.Node;
 import javax.jcr.PathNotFoundException;
+import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.Value;
 import javax.jcr.ValueFactory;
+import javax.jcr.ValueFormatException;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.ItemDefinition;
 import javax.jcr.nodetype.NodeTypeManager;
+import javax.jcr.nodetype.PropertyDefinition;
 import javax.jcr.version.VersionManager;
 
+import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.jcr.delegate.ItemDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.SessionDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.SessionOperation;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
 import org.apache.jackrabbit.oak.plugins.nodetype.DefinitionProvider;
 import org.apache.jackrabbit.oak.plugins.nodetype.EffectiveNodeTypeProvider;
+import org.apache.jackrabbit.value.ValueHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -333,4 +342,104 @@ abstract class ItemImpl<T extends ItemDe
         return sessionContext.getVersionManager();
     }
 
+    /**
+     * Determine property type from property definition and default to
+     * {@code defaultType} in the case of {@link javax.jcr.PropertyType#UNDEFINED}.
+     *
+     * @param definition
+     * @param defaultType
+     * @return the type determined from the property definition
+     */
+    private static int getType(PropertyDefinition definition, int defaultType) {
+        if (definition.getRequiredType() == PropertyType.UNDEFINED) {
+            return defaultType == PropertyType.UNDEFINED ? PropertyType.STRING : defaultType;
+        } else {
+            return definition.getRequiredType();
+        }
+    }
+
+    /**
+     * Removes all {@code null} values from the given array.
+     *
+     * @param values value array
+     * @return value array without {@code null} entries
+     */
+    private static Value[] compact(Value[] values) {
+        int n = 0;
+        for (Value value : values) {
+            if (value != null) {
+                n++;
+            }
+        }
+        if (n == values.length) {
+            return values;
+        } else {
+            Value[] nonNullValues = new Value[n];
+            for (int i = 0, j = 0; i < values.length; i++) {
+                if (values[i] != null) {
+                    nonNullValues[j++] = values[i];
+                }
+            }
+            return nonNullValues;
+        }
+    }
+
+    /**
+     * Create a single value property state given a {@code name}, a {@code value} and a
+     * property {@code definition}. If the value does not match the property definition
+     * it is converted accordingly
+     *
+     * @param name
+     * @param value
+     * @param definition
+     * @return  single valued property state
+     * @throws ValueFormatException  if {@code definition} defines a multi valued property
+     * @throws RepositoryException  if value conversion fails
+     */
+    PropertyState createSingleState(String name, Value value, PropertyDefinition definition)
+            throws RepositoryException {
+        if (definition.isMultiple()) {
+            throw new ValueFormatException("Cannot set single value to multivalued property");
+        }
+
+        int targetType = getType(definition, value.getType());
+        if (targetType == value.getType()) {
+            return PropertyStates.createProperty(name, value);
+        }
+        else {
+            return PropertyStates.createProperty(name, ValueHelper.convert(
+                    value, targetType, getValueFactory()));
+        }
+    }
+
+    /**
+     * Create a single value property state given a {@code name}, a {@code type}, a
+     * {@code value} and a property {@code definition}. If the type does not match the
+     * property definition it is converted accordingly
+     *
+     * @param name
+     * @param type
+     * @param values
+     * @param definition
+     * @return  array valued property state
+     * @throws ValueFormatException  if {@code definition} does not define a multi valued property
+     * @throws RepositoryException  if value conversion fails
+     */
+    PropertyState createMultiState(String name, int type, Value[] values, PropertyDefinition definition)
+            throws RepositoryException {
+        if (!definition.isMultiple()) {
+            throw new ValueFormatException("Cannot set value array to single value property");
+        }
+
+        Value[] nonNullValues = compact(values);
+        int targetType = getType(definition, type);
+        if (targetType == type) {
+            return PropertyStates.createProperty(name, Arrays.asList(nonNullValues));
+        }
+        else {
+            return PropertyStates.createProperty(name, Arrays.asList(ValueHelper.convert(
+                    values, targetType, getValueFactory())));
+        }
+    }
+
 }

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=1461570&r1=1461569&r2=1461570&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 Mar 27 13:30:14 2013
@@ -23,7 +23,6 @@ import static javax.jcr.PropertyType.UND
 
 import java.io.InputStream;
 import java.math.BigDecimal;
-import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.Iterator;
@@ -49,7 +48,6 @@ import javax.jcr.RepositoryException;
 import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.Value;
 import javax.jcr.ValueFactory;
-import javax.jcr.ValueFormatException;
 import javax.jcr.Workspace;
 import javax.jcr.lock.Lock;
 import javax.jcr.nodetype.ConstraintViolationException;
@@ -1445,59 +1443,32 @@ public class NodeImpl<T extends NodeDele
         return perform(new ItemWriteOperation<Property>() {
             @Override
             public Property perform() throws RepositoryException {
-                PropertyState state =
-                        PropertyStates.createProperty(oakName, value);
-
                 // TODO: Avoid extra JCR method calls (OAK-672)
-                PropertyDefinition definition = getDefinitionProvider().getDefinition(
-                        dlg.getTree(), state, exactTypeMatch);
+                PropertyDefinition definition = getEffectiveNodeType()
+                        .getPropertyDefinition(oakName, false, value.getType(), exactTypeMatch);
 
                 checkProtected(definition);
-                if (definition.isMultiple()) {
-                    throw new ValueFormatException(
-                            "Cannot set single value to multivalued property");
-                }
-
-                int targetType = PropertyImpl.getType(definition, value.getType());
-                if (targetType != value.getType()) {
-                    state = PropertyStates.createProperty(
-                            oakName, ValueHelper.convert(
-                            value, targetType, getValueFactory()));
-                }
-
+                PropertyState state = createSingleState(oakName, value, definition);
                 return new PropertyImpl(dlg.setProperty(state), sessionContext);
             }
         });
     }
 
     private Property internalSetProperty(
-            String jcrName, Value[] values,
+            String jcrName, final Value[] values,
             final int type, final boolean exactTypeMatch)
             throws RepositoryException {
         final String oakName = getOakPathOrThrow(checkNotNull(jcrName));
-        final Value[] nonNullValues = PropertyImpl.compact(checkNotNull(values));
+        checkNotNull(values);
         return perform(new ItemWriteOperation<Property>() {
             @Override
             public Property perform() throws RepositoryException {
-                PropertyState state = PropertyStates.createProperty(
-                        oakName, Arrays.asList(nonNullValues));
-
                 // TODO: Avoid extra JCR method calls (OAK-672)
-                PropertyDefinition definition = getDefinitionProvider().getDefinition(
-                        dlg.getTree(), state, exactTypeMatch);
-                checkProtected(definition);
-                if (!definition.isMultiple()) {
-                    throw new ValueFormatException(
-                            "Cannot set value array to single value property");
-                }
-
-                int targetType = PropertyImpl.getType(definition, type);
-                if (targetType != type) {
-                    state = PropertyStates.createProperty(
-                            oakName, Arrays.asList(ValueHelper.convert(
-                                    nonNullValues, targetType, getValueFactory())));
-                }
+                PropertyDefinition definition = getEffectiveNodeType()
+                        .getPropertyDefinition(oakName, true, type, exactTypeMatch);
 
+                checkProtected(definition);
+                PropertyState state = createMultiState(oakName, type, values, definition);
                 return new PropertyImpl(dlg.setProperty(state), sessionContext);
             }
         });

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java?rev=1461570&r1=1461569&r2=1461570&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java Wed Mar 27 13:30:14 2013
@@ -21,7 +21,6 @@ import static javax.jcr.PropertyType.UND
 
 import java.io.InputStream;
 import java.math.BigDecimal;
-import java.util.Arrays;
 import java.util.Calendar;
 import java.util.List;
 
@@ -43,7 +42,6 @@ import org.apache.jackrabbit.oak.api.Pro
 import org.apache.jackrabbit.oak.api.Tree.Status;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
 import org.apache.jackrabbit.oak.jcr.delegate.PropertyDelegate;
-import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
 import org.apache.jackrabbit.oak.plugins.value.ValueFactoryImpl;
 import org.apache.jackrabbit.value.ValueHelper;
 import org.slf4j.Logger;
@@ -381,8 +379,7 @@ public class PropertyImpl extends ItemIm
         return perform(new ItemReadOperation<PropertyDefinition>() {
             @Override
             protected PropertyDefinition perform() throws RepositoryException {
-                return getDefinitionProvider().getDefinition(
-                        dlg.getParent().getTree(), dlg.getPropertyState(), true);
+                return getPropertyDefinition();
             }
         });
     }
@@ -396,7 +393,12 @@ public class PropertyImpl extends ItemIm
                     Value[] values = getValues();
                     if (values.length == 0) {
                         // retrieve the type from the property definition
-                        return getType(getDefinition(), PropertyType.UNDEFINED);
+                        PropertyDefinition definition = getPropertyDefinition();
+                        if (definition.getRequiredType() == PropertyType.UNDEFINED) {
+                            return PropertyType.STRING;
+                        } else {
+                            return definition.getRequiredType();
+                        }
                     } else {
                         return values[0].getType();
                     }
@@ -444,48 +446,6 @@ public class PropertyImpl extends ItemIm
     }
 
     /**
-     * Determine property type from property definition and default to
-     * {@code defaultType} in the case of {@link PropertyType#UNDEFINED}.
-     *
-     * @param definition
-     * @param defaultType
-     * @return the type determined from the property definition
-     */
-    static int getType(PropertyDefinition definition, int defaultType) {
-        if (definition.getRequiredType() == PropertyType.UNDEFINED) {
-            return defaultType == PropertyType.UNDEFINED ? PropertyType.STRING : defaultType;
-        } else {
-            return definition.getRequiredType();
-        }
-    }
-
-    /**
-     * Removes all {@code null} values from the given array.
-     *
-     * @param values value array
-     * @return value array without {@code null} entries
-     */
-    static Value[] compact(Value[] values) {
-        int n = 0;
-        for (Value value : values) {
-            if (value != null) {
-                n++;
-            }
-        }
-        if (n == values.length) {
-            return values;
-        } else {
-            Value[] nonNullValues = new Value[n];
-            for (int i = 0, j = 0; i < values.length; i++) {
-                if (values[i] != null) {
-                    nonNullValues[j++] = values[i];
-                }
-            }
-            return nonNullValues;
-        }
-    }
-
-    /**
      * Return the length of the specified JCR value object.
      *
      * @param value The value.
@@ -510,6 +470,11 @@ public class PropertyImpl extends ItemIm
         });
     }
 
+    private PropertyDefinition getPropertyDefinition() throws RepositoryException {
+        return getDefinitionProvider().getDefinition(
+                dlg.getParent().getTree(), dlg.getPropertyState(), true);
+    }
+
     private void internalSetValue(@Nonnull final Value value)
             throws RepositoryException {
         checkNotNull(value);
@@ -517,23 +482,8 @@ public class PropertyImpl extends ItemIm
             @Override
             protected Void perform() throws RepositoryException {
                 // TODO: Avoid extra JCR method calls (OAK-672)
-                PropertyDefinition definition = getDefinitionProvider().getDefinition(
-                        dlg.getParent().getTree(), dlg.getPropertyState(), true);
-
-                if (definition.isMultiple()) {
-                    throw new ValueFormatException("Cannot set single value to multivalued property");
-                }
-
-                PropertyState state;
-                int targetType = getType(definition, value.getType());
-                if (targetType != value.getType()) {
-                    state = PropertyStates.createProperty(
-                            dlg.getName(), ValueHelper.convert(
-                            value, targetType, getValueFactory()));
-                } else {
-                    state = PropertyStates.createProperty(dlg.getName(), value);
-                }
-
+                PropertyDefinition definition = getPropertyDefinition();
+                PropertyState state = createSingleState(dlg.getName(), value, definition);
                 dlg.setState(state);
                 return null;
             }
@@ -541,28 +491,13 @@ public class PropertyImpl extends ItemIm
     }
 
     private void internalSetValues(@Nonnull final Value[] values, final int type) throws RepositoryException {
-        final Value[] nonNullValues = compact(checkNotNull(values));
+        checkNotNull(values);
         perform(new ItemWriteOperation<Void>() {
             @Override
             protected Void perform() throws RepositoryException {
                 // TODO: Avoid extra JCR method calls (OAK-672)
-                PropertyDefinition definition = getDefinitionProvider().getDefinition(
-                        dlg.getParent().getTree(), dlg.getPropertyState(), true);
-
-                if (!definition.isMultiple()) {
-                    throw new ValueFormatException("Cannot set value array to single value property");
-                }
-
-                PropertyState state;
-                int targetType = getType(definition, type);
-                if (targetType != type) {
-                    state = PropertyStates.createProperty(
-                            dlg.getName(), Arrays.asList(ValueHelper.convert(
-                            nonNullValues, targetType, getValueFactory())));
-                } else {
-                    state = PropertyStates.createProperty(dlg.getName(), Arrays.asList(nonNullValues));
-                }
-
+                PropertyDefinition definition = getPropertyDefinition();
+                PropertyState state = createMultiState(dlg.getName(), type, values, definition);
                 dlg.setState(state);
                 return null;
             }