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/05/15 14:35:49 UTC

svn commit: r1482792 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/

Author: jukka
Date: Wed May 15 12:35:49 2013
New Revision: 1482792

URL: http://svn.apache.org/r1482792
Log:
OAK-510: Multivalued properties with array size 0 forget their type

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().

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/TypeCodes.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
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java?rev=1482792&r1=1482791&r2=1482792&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/JsopDiff.java Wed May 15 12:35:49 2013
@@ -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,15 @@ 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(TypeCodes.EMPTY_ARRAY
+                        + PropertyType.nameFromValue(type.tag()));
+            }
         } else {
             toJsonValue(propertyState, jsop);
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java?rev=1482792&r1=1482791&r2=1482792&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java Wed May 15 12:35:49 2013
@@ -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
 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 exten
             return BooleanPropertyState.booleanProperty(name, false);
         } else if (reader.matches(JsopReader.STRING)) {
             String jsonString = reader.getToken();
+            if (jsonString.startsWith(TypeCodes.EMPTY_ARRAY)) {
+                int type = PropertyType.valueFromName(
+                        jsonString.substring(TypeCodes.EMPTY_ARRAY.length()));
+                return PropertyStates.createProperty(
+                        name, emptyList(), Type.fromTag(type, true));
+            }
             int split = TypeCodes.split(jsonString);
             if (split != -1) {
                 int type = TypeCodes.decodeType(split, jsonString);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/TypeCodes.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/TypeCodes.java?rev=1482792&r1=1482791&r2=1482792&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/TypeCodes.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/TypeCodes.java Wed May 15 12:35:49 2013
@@ -29,6 +29,9 @@ import javax.jcr.PropertyType;
  * its json serialisation.
  */
 public final class TypeCodes {
+
+    public static final String EMPTY_ARRAY = "[0]:";
+
     private static final Map<Integer, String> TYPE2CODE = new HashMap<Integer, String>();
     private static final Map<String, Integer> CODE2TYPE = new HashMap<String, Integer>();
 

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=1482792&r1=1482791&r2=1482792&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 May 15 12:35:49 2013
@@ -87,6 +87,7 @@ import static javax.jcr.Property.JCR_LOC
 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<T extends NodeDele
                 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;
                             }
                         }

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=1482792&r1=1482791&r2=1482792&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 May 15 12:35:49 2013
@@ -389,25 +389,7 @@ public class PropertyImpl extends ItemIm
         return perform(new ItemReadOperation<Integer>() {
             @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 ItemIm
         return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
-                return dlg.isArray();
+                return dlg.getPropertyState().isArray();
             }
         });
     }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java?rev=1482792&r1=1482791&r2=1482792&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java Wed May 15 12:35:49 2013
@@ -106,10 +106,6 @@ public class PropertyDelegate extends It
         return p;
     }
 
-    public boolean isArray() throws InvalidItemStateException {
-        return getPropertyState().isArray();
-    }
-
     @Nonnull
     public PropertyState getSingleState() throws InvalidItemStateException, ValueFormatException {
         PropertyState p = getPropertyState();



Re: svn commit: r1482792 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/

Posted by Michael Dürig <md...@apache.org>.

On 21.5.13 13:32, Jukka Zitting wrote:
> Only the KernelNodeStore should be dealing with the JSOP
> representation, so I wouldn't worry about JsopUtil. Nowadays the class
> is only used for initializing content in a few query test cases. Those
> cases should be refactored to use the NodeStore API and then we can
> drop the JsopUtil class.

See https://issues.apache.org/jira/browse/OAK-838

Michael

Re: svn commit: r1482792 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, May 21, 2013 at 3:20 PM, Michael Dürig <md...@apache.org> wrote:
> Note that we need to duplicate these changes to the respective method in
> org.apache.jackrabbit.oak.query.JsopUtil. There was a common utility earlier
> for this decoding, which was removed at some point however. I suggest to put
> such an utility back in place since this is not the first time these are
> getting out of sync.

Only the KernelNodeStore should be dealing with the JSOP
representation, so I wouldn't worry about JsopUtil. Nowadays the class
is only used for initializing content in a few query test cases. Those
cases should be refactored to use the NodeStore API and then we can
drop the JsopUtil class.

BR,

Jukka Zitting

Re: svn commit: r1482792 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/

Posted by Michael Dürig <md...@apache.org>.

On 15.5.13 13:35, jukka@apache.org wrote:
> Author: jukka
> Date: Wed May 15 12:35:49 2013
> New Revision: 1482792

[...]

> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java?rev=1482792&r1=1482791&r2=1482792&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java Wed May 15 12:35:49 2013
> @@ -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
>   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 exten
>               return BooleanPropertyState.booleanProperty(name, false);
>           } else if (reader.matches(JsopReader.STRING)) {
>               String jsonString = reader.getToken();
> +            if (jsonString.startsWith(TypeCodes.EMPTY_ARRAY)) {
> +                int type = PropertyType.valueFromName(
> +                        jsonString.substring(TypeCodes.EMPTY_ARRAY.length()));
> +                return PropertyStates.createProperty(
> +                        name, emptyList(), Type.fromTag(type, true));
> +            }
>               int split = TypeCodes.split(jsonString);
>               if (split != -1) {
>                   int type = TypeCodes.decodeType(split, jsonString);
>

Note that we need to duplicate these changes to the respective method in 
org.apache.jackrabbit.oak.query.JsopUtil. There was a common utility 
earlier for this decoding, which was removed at some point however. I 
suggest to put such an utility back in place since this is not the first 
time these are getting out of sync.

Michael