jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1461107 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
Date Tue, 26 Mar 2013 13:07:43 GMT
Author: jukka
Date: Tue Mar 26 13:07:43 2013
New Revision: 1461107

URL: http://svn.apache.org/r1461107
Log:
OAK-702: Optimize access to node type information

Remove another getDefinition() signature from DefinitionProvider.
No need to check for previous single/multi-valuedness of the property
anymore in AuthorizablePropertiesImpl.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/DefinitionProvider.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/DefinitionProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/DefinitionProvider.java?rev=1461107&r1=1461106&r2=1461107&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/DefinitionProvider.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/DefinitionProvider.java
Tue Mar 26 13:07:43 2013
@@ -91,27 +91,8 @@ public interface DefinitionProvider {
      * @throws RepositoryException If another error occurs.
      */
     @Nonnull
-    PropertyDefinition getDefinition(Tree parent, PropertyState propertyState)
+    PropertyDefinition getDefinition(
+            Tree parent, PropertyState propertyState, boolean exactTypeMatch)
             throws ConstraintViolationException,RepositoryException;
 
-    /**
-     * Calculates the applicable definition for the property with the specified
-     * characteristics under the given parent tree.
-     *
-     * @param parent The parent tree.
-     * @param propertyName The internal oak name of the property for which the
-     * definition should be retrieved.
-     * @param isMultiple {@code true} if the target property is multi-valued.
-     * @param type The target type of the property.
-     * @param exactTypeMatch {@code true} if the required type of the definition
-     * must exactly match the type of the target property.
-     * @return the applicable definition for the target property.
-     * @throws ConstraintViolationException If no matching definition can be found.
-     * @throws RepositoryException If another error occurs.
-     */
-    @Nonnull
-    PropertyDefinition getDefinition(Tree parent, String propertyName, boolean isMultiple,
-                                     int type, boolean exactTypeMatch)
-            throws ConstraintViolationException, RepositoryException;
-
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java?rev=1461107&r1=1461106&r2=1461107&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/ReadOnlyNodeTypeManager.java
Tue Mar 26 13:07:43 2013
@@ -424,15 +424,13 @@ public abstract class ReadOnlyNodeTypeMa
 
     @Nonnull
     @Override
-    public PropertyDefinition getDefinition(Tree parent, PropertyState propertyState) throws
RepositoryException {
-        return getDefinition(parent, propertyState.getName(), propertyState.isArray(), propertyState.getType().tag(),
true);
-    }
-
-    @Nonnull
-    @Override
-    public PropertyDefinition getDefinition(Tree parent, String propertyName, boolean isMultiple,
int type, boolean exactTypeMatch) throws RepositoryException {
+    public PropertyDefinition getDefinition(
+            Tree parent, PropertyState property, boolean exactTypeMatch)
+            throws RepositoryException {
+        Type<?> type = property.getType();
         EffectiveNodeType effective = getEffectiveNodeType(parent);
-        return effective.getPropertyDefinition(propertyName, isMultiple, type, exactTypeMatch);
+        return effective.getPropertyDefinition(
+                property.getName(), type.isArray(), type.tag(), exactTypeMatch);
     }
 
     //-----------------------------------------------------------< internal >---

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java?rev=1461107&r1=1461106&r2=1461107&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizablePropertiesImpl.java
Tue Mar 26 13:07:43 2013
@@ -22,7 +22,6 @@ import java.util.Iterator;
 import java.util.List;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
-import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.nodetype.ConstraintViolationException;
@@ -129,18 +128,13 @@ class AuthorizablePropertiesImpl impleme
             checkRelativePath(relPath);
 
             String name = Text.getName(relPath);
+            PropertyState propertyState =
+                    PropertyStates.createProperty(name, value);
+
             String intermediate = (relPath.equals(name)) ? null : Text.getRelativeParent(relPath,
1);
             Tree parent = getOrCreateTargetTree(intermediate);
-            checkProtectedProperty(parent, name, false, value.getType());
+            checkProtectedProperty(parent, propertyState);
 
-            // check if the property has already been created as multi valued
-            // property before -> in this case remove in order to avoid
-            // ValueFormatException.
-            PropertyState p = parent.getProperty(name);
-            if (p != null && p.isArray()) {
-                parent.removeProperty(name);
-            }
-            PropertyState propertyState = PropertyStates.createProperty(name, value);
             parent.setProperty(propertyState);
         }
     }
@@ -156,19 +150,13 @@ class AuthorizablePropertiesImpl impleme
             checkRelativePath(relPath);
 
             String name = Text.getName(relPath);
+            PropertyState propertyState =
+                    PropertyStates.createProperty(name, Arrays.asList(values));
+
             String intermediate = (relPath.equals(name)) ? null : Text.getRelativeParent(relPath,
1);
             Tree parent = getOrCreateTargetTree(intermediate);
-            int targetType = (values.length == 0) ? PropertyType.UNDEFINED : values[0].getType();
-            checkProtectedProperty(parent, name, true, targetType);
+            checkProtectedProperty(parent, propertyState);
 
-            // check if the property has already been created as single valued
-            // property before -> in this case remove in order to avoid
-            // ValueFormatException.
-            PropertyState p = parent.getProperty(name);
-            if (p != null && !p.isArray()) {
-                parent.removeProperty(name);
-            }
-            PropertyState propertyState = PropertyStates.createProperty(name, Arrays.asList(values));
             parent.setProperty(propertyState);
         }
     }
@@ -255,7 +243,7 @@ class AuthorizablePropertiesImpl impleme
             log.debug("Unable to determine definition of authorizable property at " + propertyLocation.getPath());
             return null;
         }
-        PropertyDefinition def = nodeTypeManager.getDefinition(parent, property);
+        PropertyDefinition def = nodeTypeManager.getDefinition(parent, property, true);
         if (def.isProtected() || (authorizablePath.equals(parent.getPath())
                 && !def.getDeclaringNodeType().isNodeType(UserConstants.NT_REP_AUTHORIZABLE)))
{
             return null;
@@ -264,10 +252,12 @@ class AuthorizablePropertiesImpl impleme
         return property;
     }
 
-    private void checkProtectedProperty(Tree parent, String propertyName, boolean isArray,
int type) throws RepositoryException {
-        PropertyDefinition def = nodeTypeManager.getDefinition(parent, propertyName, isArray,
type, false);
+    private void checkProtectedProperty(Tree parent, PropertyState property) throws RepositoryException
{
+        PropertyDefinition def =
+                nodeTypeManager.getDefinition(parent, property, false);
         if (def.isProtected()) {
-            throw new ConstraintViolationException("Attempt to set an protected property
" + propertyName);
+            throw new ConstraintViolationException(
+                    "Attempt to set an protected property " + property.getName());
         }
     }
 

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=1461107&r1=1461106&r2=1461107&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
Tue Mar 26 13:07:43 2013
@@ -60,7 +60,6 @@ import javax.jcr.version.VersionManager;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
-import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
@@ -362,7 +361,7 @@ public class NodeImpl<T extends NodeDele
     public Property setProperty(String name, Value value)
             throws RepositoryException {
         if (value != null) {
-            return internalSetProperty(name, value, value.getType(), false);
+            return internalSetProperty(name, value, false);
         } else {
             return internalRemoveProperty(name);
         }
@@ -374,7 +373,10 @@ public class NodeImpl<T extends NodeDele
         if (type == UNDEFINED) {
             return setProperty(name, value);
         } else if (value != null) {
-            return internalSetProperty(name, value, type, true);
+            if (value.getType() != type) {
+                value = ValueHelper.convert(value, type, getValueFactory());
+            }
+            return internalSetProperty(name, value, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -384,15 +386,19 @@ public class NodeImpl<T extends NodeDele
     public Property setProperty(String name, Value[] values)
             throws RepositoryException {
         if (values != null) {
+            int type = UNDEFINED;
             for (int i = 0; i < values.length; i++) {
                 if (values[i] != null) {
-                    // use the type of the first non-null value
-                    return internalSetProperty(
-                            name, values, values[i].getType(), false);
+                    if (type == UNDEFINED) {
+                        type = values[i].getType();
+                    } else if (values[i].getType() != type) {
+                        throw new ValueFormatException(
+                                "All values of a multi-valued property"
+                                + " must be of the same type");
+                    }
                 }
             }
-            // unknown value type
-            return internalSetProperty(name, values, UNDEFINED, false);
+            return internalSetProperty(name, values, type, false);
         } else {
             return internalRemoveProperty(name);
         }
@@ -404,6 +410,7 @@ public class NodeImpl<T extends NodeDele
         if (type == UNDEFINED) {
             return setProperty(jcrName, values);
         } else if (values != null) {
+            values = ValueHelper.convert(values, type, getValueFactory());
             return internalSetProperty(jcrName, values, type, true);
         } else {
             return internalRemoveProperty(jcrName);
@@ -440,7 +447,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (value != null) {
             Value v = getValueFactory().createValue(value);
-            return internalSetProperty(name, v, UNDEFINED, false);
+            return internalSetProperty(name, v, false);
         } else {
             return internalRemoveProperty(name);
         }
@@ -453,7 +460,7 @@ public class NodeImpl<T extends NodeDele
             return setProperty(name, value);
         } else if (value != null) {
             Value v = getValueFactory().createValue(value, type);
-            return internalSetProperty(name, v, type, true);
+            return internalSetProperty(name, v, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -467,7 +474,7 @@ public class NodeImpl<T extends NodeDele
             Binary binary = factory.createBinary(value);
             try {
                 Value v = factory.createValue(binary);
-                return internalSetProperty(name, v, PropertyType.BINARY, true);
+                return internalSetProperty(name, v, true);
             } finally {
                 binary.dispose();
             }
@@ -481,7 +488,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (value != null) {
             Value v = getValueFactory().createValue(value);
-            return internalSetProperty(name, v, PropertyType.BINARY, true);
+            return internalSetProperty(name, v, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -491,14 +498,14 @@ public class NodeImpl<T extends NodeDele
     public Property setProperty(String name, boolean value)
             throws RepositoryException {
         Value v = getValueFactory().createValue(value);
-        return internalSetProperty(name, v, PropertyType.BOOLEAN, true);
+        return internalSetProperty(name, v, true);
     }
 
     @Override @Nonnull
     public Property setProperty(String name, double value)
             throws RepositoryException {
         Value v = getValueFactory().createValue(value);
-        return internalSetProperty(name, v, PropertyType.DOUBLE, true);
+        return internalSetProperty(name, v, true);
     }
 
     @Override @Nonnull
@@ -506,7 +513,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (value != null) {
             Value v = getValueFactory().createValue(value);
-            return internalSetProperty(name, v, PropertyType.DECIMAL, true);
+            return internalSetProperty(name, v, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -516,7 +523,7 @@ public class NodeImpl<T extends NodeDele
     public Property setProperty(String name, long value)
             throws RepositoryException {
         Value v = getValueFactory().createValue(value);
-        return internalSetProperty(name, v, PropertyType.LONG, true);
+        return internalSetProperty(name, v, true);
     }
 
     @Override @Nonnull
@@ -524,7 +531,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (value != null) {
             Value v = getValueFactory().createValue(value);
-            return internalSetProperty(name, v, PropertyType.DATE, true);
+            return internalSetProperty(name, v, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -535,7 +542,7 @@ public class NodeImpl<T extends NodeDele
             throws RepositoryException {
         if (value != null) {
             Value v = getValueFactory().createValue(value);
-            return internalSetProperty(name, v, PropertyType.REFERENCE, true);
+            return internalSetProperty(name, v, true);
         } else {
             return internalRemoveProperty(name);
         }
@@ -1591,11 +1598,10 @@ public class NodeImpl<T extends NodeDele
     }
 
     private Property internalSetProperty(
-            final String jcrName, final Value value,
-            final int type, final boolean exactTypeMatch)
+            String jcrName, final Value value, final boolean exactTypeMatch)
             throws RepositoryException {
-        checkNotNull(value);
         final String oakName = getOakPath(checkNotNull(jcrName));
+        checkNotNull(value);
         return perform(new SessionOperation<Property>() {
             @Override
             protected void checkPreconditions() throws RepositoryException {
@@ -1606,9 +1612,12 @@ public class NodeImpl<T extends NodeDele
 
             @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(), oakName, false, type, exactTypeMatch);
+                        dlg.getTree(), state, exactTypeMatch);
                 checkProtected(definition);
                 if (definition.isMultiple()) {
                     throw new ValueFormatException(
@@ -1616,22 +1625,23 @@ public class NodeImpl<T extends NodeDele
                 }
 
                 int targetType = getTargetType(value, definition);
-                Value targetValue = ValueHelper.convert(
-                        value, targetType, getValueFactory());
+                if (targetType != value.getType()) {
+                    state = PropertyStates.createProperty(
+                            oakName, ValueHelper.convert(
+                                    value, targetType, getValueFactory()));
+                }
 
-                PropertyState state =
-                        PropertyStates.createProperty(oakName, targetValue);
                 return new PropertyImpl(dlg.setProperty(state), sessionContext);
             }
         });
     }
 
     private Property internalSetProperty(
-            final String jcrName, final Value[] values,
+            String jcrName, Value[] values,
             final int type, final boolean exactTypeMatch)
             throws RepositoryException {
-        checkNotNull(values);
         final String oakName = getOakPath(checkNotNull(jcrName));
+        final Value[] nonNullValues = compact(checkNotNull(values));
         return perform(new SessionOperation<Property>() {
             @Override
             protected void checkPreconditions() throws RepositoryException {
@@ -1642,23 +1652,25 @@ public class NodeImpl<T extends NodeDele
 
             @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(), oakName, true, type, exactTypeMatch);
+                        dlg.getTree(), state, exactTypeMatch);
                 checkProtected(definition);
                 if (!definition.isMultiple()) {
                     throw new ValueFormatException(
                             "Cannot set value array to single value property");
                 }
 
-                int targetType = getTargetType(values, definition);
-                Value[] targetValues = ValueHelper.convert(
-                        values, targetType, getValueFactory());
+                int targetType = getTargetType(nonNullValues, definition);
+                if (targetType != type) {
+                    state = PropertyStates.createProperty(
+                            oakName, Arrays.asList(ValueHelper.convert(
+                                    nonNullValues, targetType, getValueFactory())));
+                }
 
-                Iterable<Value> nonNullValues = Iterables.filter(
-                        Arrays.asList(targetValues), Predicates.notNull());
-                PropertyState state =
-                        PropertyStates.createProperty(oakName, nonNullValues);
                 return new PropertyImpl(dlg.setProperty(state), sessionContext);
             }
         });
@@ -1693,4 +1705,30 @@ public class NodeImpl<T extends NodeDele
         });
     }
 
+    /**
+     * 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 (int i = 0; i < values.length; i++) {
+            if (values[i] != 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;
+        }
+    }
+
 }



Mime
View raw message