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 ju...@apache.org on 2013/03/26 14:07:43 UTC
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/
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;
+ }
+ }
+
}