From d64deb11d8ff8312cd03227f3b90de7606feff3d Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Tue, 14 May 2013 13:51:51 +0300 Subject: [PATCH] OAK-815: Wrong type for empty multi-valued property Avoid losing type information when storing empty non-string arrays in the MicroKernel. Also drop the PropertyDelegate.isArray() method in favor of the more generic getPropertyState(). --- .../org/apache/jackrabbit/oak/kernel/JsopDiff.java | 12 +++++++++--- .../jackrabbit/oak/kernel/KernelNodeState.java | 8 ++++++++ .../org/apache/jackrabbit/oak/jcr/NodeImpl.java | 9 ++++++--- .../apache/jackrabbit/oak/jcr/PropertyImpl.java | 22 ++-------------------- .../oak/jcr/delegate/PropertyDelegate.java | 4 ---- 5 files changed, 25 insertions(+), 30 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java index bf8c1a4..03f1976 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java @@ -28,6 +28,7 @@ import javax.jcr.PropertyType; import org.apache.jackrabbit.mk.json.JsopBuilder; import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.NodeState; @@ -134,9 +135,14 @@ class JsopDiff implements NodeStateDiff { private void toJson(PropertyState propertyState, JsopBuilder jsop) { if (propertyState.isArray()) { - jsop.array(); - toJsonValue(propertyState, jsop); - jsop.endArray(); + Type type = propertyState.getType(); + if (type == STRINGS || propertyState.count() > 0) { + jsop.array(); + toJsonValue(propertyState, jsop); + jsop.endArray(); + } else { + jsop.value("[0]:" + PropertyType.nameFromValue(type.tag())); + } } else { toJsonValue(propertyState, jsop); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java index 5f077bc..6bcc6ab 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.kernel; import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.Collections.emptyList; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; @@ -55,6 +56,7 @@ import org.apache.jackrabbit.oak.plugins.memory.BinaryPropertyState; import org.apache.jackrabbit.oak.plugins.memory.BooleanPropertyState; import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState; import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState; import org.apache.jackrabbit.oak.plugins.value.Conversions; import org.apache.jackrabbit.oak.spi.state.AbstractChildNodeEntry; @@ -627,6 +629,12 @@ public final class KernelNodeState extends AbstractNodeState { return BooleanPropertyState.booleanProperty(name, false); } else if (reader.matches(JsopReader.STRING)) { String jsonString = reader.getToken(); + if (jsonString.startsWith("[0]:")) { + int type = PropertyType.valueFromName( + jsonString.substring("[0]:".length())); + return PropertyStates.createProperty( + name, emptyList(), Type.fromTag(type, true)); + } int split = TypeCodes.split(jsonString); if (split != -1) { int type = TypeCodes.decodeType(split, jsonString); diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java index 93db64a..c28c629 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java @@ -87,6 +87,7 @@ import static javax.jcr.Property.JCR_LOCK_OWNER; import static javax.jcr.PropertyType.UNDEFINED; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; +import static org.apache.jackrabbit.oak.api.Type.BOOLEAN; import static org.apache.jackrabbit.oak.api.Type.NAME; import static org.apache.jackrabbit.oak.api.Type.NAMES; @@ -1059,9 +1060,11 @@ public class NodeImpl extends ItemImpl implements Nod NodeDelegate parent = dlg.getParent(); while (parent != null) { if (parent.getPropertyOrNull(lockOwner) != null) { - PropertyDelegate isDeep = parent.getPropertyOrNull(lockIsDeep); - if (isDeep != null && !isDeep.isArray()) { - if (isDeep.getBoolean()) { + PropertyDelegate isDeep = + parent.getPropertyOrNull(lockIsDeep); + if (isDeep != null) { + PropertyState state = isDeep.getPropertyState(); + if (!state.isArray() && state.getValue(BOOLEAN)) { return true; } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java index 4663a61..6dcc15d 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/PropertyImpl.java @@ -389,25 +389,7 @@ public class PropertyImpl extends ItemImpl implements Property return perform(new ItemReadOperation() { @Override public Integer perform() throws RepositoryException { - if (isMultiple()) { - Value[] values = getValues(); - if (values.length == 0) { - // retrieve the type from the property definition - // do not require exact match (see OAK-815) - PropertyDefinition definition = getDefinitionProvider() - .getDefinition(dlg.getParent().getTree(), - dlg.getPropertyState(), false); - if (definition.getRequiredType() == PropertyType.UNDEFINED) { - return PropertyType.STRING; - } else { - return definition.getRequiredType(); - } - } else { - return values[0].getType(); - } - } else { - return getValue().getType(); - } + return dlg.getPropertyState().getType().tag(); } }); } @@ -417,7 +399,7 @@ public class PropertyImpl extends ItemImpl implements Property return perform(new ItemReadOperation() { @Override public Boolean perform() throws RepositoryException { - return dlg.isArray(); + return dlg.getPropertyState().isArray(); } }); } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java index e793426..a55031a 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java @@ -106,10 +106,6 @@ public class PropertyDelegate extends ItemDelegate { return p; } - public boolean isArray() throws InvalidItemStateException { - return getPropertyState().isArray(); - } - @Nonnull public PropertyState getSingleState() throws InvalidItemStateException, ValueFormatException { PropertyState p = getPropertyState(); -- 1.8.0.msysgit.0