Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1518118 - in
 /jackrabbit/branches/2.4/jackrabbit-core/src:
 main/java/org/apache/jackrabbit/core/
 main/java/org/apache/jackrabbit/core/persistence/util/
 main/java/org/apache/jackrabbit/core/version/
 test/java/org/apache/jackrabbit/core/pers...
Date: Wed, 28 Aug 2013 09:21:19 -0000
To: commits@jackrabbit.apache.org
From: thomasm@apache.org
Message-Id: <20130828092119.BF0A72388900@eris.apache.org>

Author: thomasm
Date: Wed Aug 28 09:21:18 2013
New Revision: 1518118

URL: http://svn.apache.org/r1518118
Log:
JCR-3652 Bundle serialization broken

Added:
    jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/BundleBindingRandomizedTest.java
    jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/NodeCorruptionTest.java
Modified:
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleWriter.java
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/NodePropBundle.java
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
    jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/TestAll.java

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=1518118&r1=1518117&r2=1518118&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java Wed Aug 28 09:21:18 2013
@@ -2043,6 +2043,18 @@ public class NodeImpl extends ItemImpl i
                     removeChildProperty(name);
                 }
                 throw e; // rethrow
+            } catch (RuntimeException e) {
+                if (status.get(CREATED)) {
+                    // setting value failed, get rid of newly created property
+                    removeChildProperty(name);
+                }
+                throw e; // rethrow
+            } catch (Error e) {
+                if (status.get(CREATED)) {
+                    // setting value failed, get rid of newly created property
+                    removeChildProperty(name);
+                }
+                throw e; // rethrow
             }
             return property;
         }
@@ -2836,7 +2848,13 @@ public class NodeImpl extends ItemImpl i
             PropertyId id = new PropertyId(state.getNodeId(), JCR_ISCHECKEDOUT);
             PropertyState ps =
                 (PropertyState) sessionContext.getItemStateManager().getItemState(id);
-            return ps.getValues()[0].getBoolean();
+            InternalValue[] values = ps.getValues();
+            if (values == null || values.length != 1) {
+                // the property is not fully set, or it is a multi-valued property
+                // in which case it's probably not mix:versionable
+                return true;
+            }
+            return values[0].getBoolean();
         } catch (ItemStateException e) {
             throw new RepositoryException(e);
         }

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleWriter.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleWriter.java?rev=1518118&r1=1518117&r2=1518118&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleWriter.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleWriter.java Wed Aug 28 09:21:18 2013
@@ -184,7 +184,9 @@ class BundleWriter {
         InternalValue[] values = state.getValues();
 
         int type = state.getType();
-        assert 0 <= type && type <= 0x0f;
+        if (type < 0 || type > 0xf) {
+            throw new IOException("Illegal property type " + type);
+        }
         if (state.isMultiValued()) {
             int len = values.length + 1;
             if (len < 0x0f) {
@@ -194,7 +196,11 @@ class BundleWriter {
                 writeVarInt(len - 0x0f);
             }
         } else {
-            assert values.length == 1;
+            if (values.length != 1) {
+                throw new IOException(
+                        "Single values property with " + values.length + " values: " + 
+                        state.getName());
+            }
             out.writeByte(type);
         }
 
@@ -203,7 +209,7 @@ class BundleWriter {
         // values
         for (int i = 0; i < values.length; i++) {
             InternalValue val = values[i];
-            switch (state.getType()) {
+            switch (type) {
                 case PropertyType.BINARY:
                     try {
                         long size = val.getLength();
@@ -324,9 +330,13 @@ class BundleWriter {
                         throw new IOException("Unexpected error while writing DATE value.");
                     }
                     break;
-                default:
+                case PropertyType.STRING:
+                case PropertyType.PATH:
+                case PropertyType.URI:
                     writeString(val.toString());
                     break;
+                default:
+                    throw new IOException("Inknown property type: " + type);
             }
         }
     }
@@ -449,6 +459,9 @@ class BundleWriter {
             }
 
             String local = name.getLocalName();
+            if (local.length() == 0) {
+                throw new IOException("Attempt to write an empty local name: " + name);
+            }
             byte[] bytes = local.getBytes("UTF-8");
             int len = Math.min(bytes.length - 1, 0x0f);
 

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/NodePropBundle.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/NodePropBundle.java?rev=1518118&r1=1518117&r2=1518118&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/NodePropBundle.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/NodePropBundle.java Wed Aug 28 09:21:18 2013
@@ -434,6 +434,7 @@ public class NodePropBundle {
 
     //--------------------------------------------------------------< Object >
 
+    @Override
     public String toString() {
         StringBuilder builder = new StringBuilder();
         builder.append(id);
@@ -459,20 +460,27 @@ public class NodePropBundle {
         return builder.toString();
     }
 
+    @Override
     public boolean equals(Object object) {
         if (object instanceof NodePropBundle) {
             NodePropBundle that = (NodePropBundle) object;
-            return id.equals(that.id)
-                && parentId.equals(that.parentId)
-                && nodeTypeName.equals(that.nodeTypeName)
-                && mixinTypeNames.equals(that.mixinTypeNames)
+            return equalNullSafe(id, that.id)
+                && equalNullSafe(parentId, that.parentId)
+                && equalNullSafe(nodeTypeName, that.nodeTypeName)
+                && equalNullSafe(mixinTypeNames, that.mixinTypeNames)
                 && isReferenceable == that.isReferenceable
-                && sharedSet.equals(that.sharedSet)
-                && properties.equals(that.properties)
-                && childNodeEntries.equals(that.childNodeEntries);
-        } else {
-            return false;
+                && equalNullSafe(sharedSet, that.sharedSet)
+                && equalNullSafe(properties, that.properties)
+                && equalNullSafe(childNodeEntries, that.childNodeEntries);
         }
+        return false;
+    }
+    
+    private static boolean equalNullSafe(Object a, Object b) {
+        if (a == null || b == null) {
+            return a == b;
+        }
+        return a.equals(b);
     }
 
     //-----------------------------------------------------< ChildNodeEntry >---
@@ -590,6 +598,9 @@ public class NodePropBundle {
             values = state.getValues();
             type = state.getType();
             multiValued = state.isMultiValued();
+            if (!multiValued && values.length != 1) {
+                throw new IllegalArgumentException("Non-multi-valued property with values.length " + values.length);
+            }
             modCount = state.getModCount();
             if (type == PropertyType.BINARY) {
                 blobIds = new String[values.length];

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java?rev=1518118&r1=1518117&r2=1518118&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java Wed Aug 28 09:21:18 2013
@@ -279,6 +279,7 @@ public class NodeStateEx {
                     nodeState.setStatus(ItemState.STATUS_EXISTING_MODIFIED);
                 }
                 propState.setType(type);
+                propState.setMultiValued(multiple);
                 propState.setValues(values);
                 return propState;
             } catch (ItemStateException e) {

Added: jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/BundleBindingRandomizedTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/BundleBindingRandomizedTest.java?rev=1518118&view=auto
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/BundleBindingRandomizedTest.java (added)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/BundleBindingRandomizedTest.java Wed Aug 28 09:21:18 2013
@@ -0,0 +1,231 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.persistence.util;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Random;
+import java.util.Set;
+
+import javax.jcr.PropertyType;
+
+import junit.framework.TestCase;
+
+import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.id.PropertyId;
+import org.apache.jackrabbit.core.persistence.util.NodePropBundle.PropertyEntry;
+import org.apache.jackrabbit.core.util.StringIndex;
+import org.apache.jackrabbit.core.value.InternalValue;
+import org.apache.jackrabbit.spi.Name;
+import org.apache.jackrabbit.spi.NameFactory;
+import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
+
+/**
+ * A randomized test for the BundleBinding writer and reader classes.
+ */
+public class BundleBindingRandomizedTest extends TestCase {
+
+    private static final NameFactory factory = NameFactoryImpl.getInstance();
+
+    /**
+     * Tests serialization of a complex bundle.
+     */
+    public void testRandomBundle() throws Exception {
+        int seed = 0;
+        for (int i=0; i<100;) {
+            Random r = new Random(seed++);
+            NodePropBundle bundle;
+            try {
+                bundle = randomBundle(r);
+            } catch (IllegalArgumentException e) {
+                continue;
+            }
+            try {
+                if (tryBundleRoundtrip(bundle)) {
+                    i++;
+                }
+            } catch (Exception e) {
+                throw new Exception(
+                        "Error round-tripping bundle with seed " + seed, e);
+                
+            }
+        }
+    }
+    
+    private static NodePropBundle randomBundle(Random r) {
+        NodeId id = randomNodeId(r);
+        NodePropBundle bundle = new NodePropBundle(id);
+        bundle.setModCount((short) randomSize(r)); 
+        if (r.nextInt(10) > 0) {
+            bundle.setParentId(randomNodeId(r));
+        }
+        if (r.nextInt(100) > 0) {
+            bundle.setNodeTypeName(randomName(r));
+        }
+        if (r.nextInt(100) > 0) {
+            bundle.setMixinTypeNames(randomNameSet(r));
+        }
+        if (r.nextInt(10) > 0) {
+            bundle.setReferenceable(r.nextBoolean());
+        }
+        if (r.nextInt(10) > 0) {
+            bundle.setSharedSet(randomNodeIdSet(r));
+        }
+        int count = randomSize(r);
+        for (int i=0; i<count; i++) {
+            PropertyEntry p = randomProperty(id, r);
+            bundle.addProperty(p);
+        }
+        count = randomSize(r);
+        for (int i=0; i<count; i++) {
+            bundle.addChildNodeEntry(randomName(r), randomNodeId(r));
+        }
+        return bundle;
+    }
+    
+    private static boolean tryBundleRoundtrip(NodePropBundle bundle)
+            throws Exception {
+        BundleBinding binding;
+        StringIndex index = new StringIndex() {
+            @Override
+            public int stringToIndex(String string) {
+                return 0;
+            }
+            @Override
+            public String indexToString(int idx) {
+                return "";
+            }
+        };
+        binding = new BundleBinding(null, null, index, index, null);
+        ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+        try {
+            binding.writeBundle(buffer, bundle);
+        } catch (Exception e) {
+            // ignore - error found
+            return false;
+        }
+        byte[] bytes = buffer.toByteArray();
+        NodePropBundle b2 = binding.readBundle(
+                new ByteArrayInputStream(bytes), bundle.getId());
+        if (!bundle.equals(b2)) {
+            // if debugging is needed:
+            // buffer = new ByteArrayOutputStream();
+            // binding.writeBundle(buffer, bundle);
+            // bytes = buffer.toByteArray();
+            // b2 = binding.readBundle(
+            // new ByteArrayInputStream(bytes), bundle.getId());
+            assertEquals(bundle.toString(), b2.toString());
+        }
+        return true;
+    }
+    
+    private static PropertyEntry randomProperty(NodeId nodeId, Random r) {
+        PropertyEntry p = new PropertyEntry(
+                new PropertyId(nodeId, randomName(r)));        
+        int type = PropertyType.STRING;
+        if (r.nextInt(10) == 0) {
+            type = r.nextInt() + 15;
+        }
+        p.setType(type);
+        p.setModCount((short) randomSize(r)); 
+        boolean multiValued = r.nextBoolean();
+        p.setMultiValued(multiValued);
+        int size;
+        if (multiValued && r.nextInt(10) > 0) {
+            size = 1;
+        } else {
+            size = randomSize(r);
+        }
+        InternalValue[] values = new InternalValue[size];
+        for (int i = 0; i < size; i++) {
+            values[i] = randomValue(r);
+        }
+        p.setValues(values);
+        return p;
+    }
+    
+    private static InternalValue randomValue(Random r) {
+        if (r.nextInt(50) == 0) {
+            return null;
+        }
+        // TODO currently only string values
+        return InternalValue.create(randomString(r));
+    }
+
+    private static int randomSize(Random r) {
+        if (r.nextInt(20) == 0) {
+            return 0;
+        } else if (r.nextInt(20) == 0) {
+            return 1;
+        } else if (r.nextInt(20) == 0) {
+            return r.nextInt(10000);
+        }
+        return r.nextInt(5) + 1;
+    }
+    
+    private static Set<Name> randomNameSet(Random r) {
+        if (r.nextInt(100) == 0) {
+            return null;
+        }
+        int size = randomSize(r);
+        HashSet<Name> set = new HashSet<Name>();
+        for(int i=0; i<size; i++) {
+            set.add(randomName(r));
+        }
+        return set;
+    }
+    
+    private static Set<NodeId> randomNodeIdSet(Random r) {
+        if (r.nextInt(100) == 0) {
+            return null;
+        } else if (r.nextInt(10) == 0) {
+            return Collections.emptySet();
+        }
+        int size = randomSize(r);
+        HashSet<NodeId> set = new HashSet<NodeId>();
+        for(int i=0; i<size; i++) {
+            set.add(randomNodeId(r));
+        }
+        return set;
+    }
+    
+    private static NodeId randomNodeId(Random r) {
+        if (r.nextInt(100) == 0) {
+            return null;
+        }
+        return new NodeId(r.nextLong(), r.nextLong());
+    }
+    
+    private static Name randomName(Random r) {
+        if (r.nextInt(100) == 0) {
+            return null;
+        }
+        return factory.create(randomString(r), randomString(r));
+    }
+    
+    private static String randomString(Random r) {
+        int size = randomSize(r);
+        StringBuilder buff = new StringBuilder();
+        for (int i=0; i<size; i++) {
+            buff.append("abcd".charAt(r.nextInt(4)));
+        }
+        return buff.toString();
+    }
+    
+}

Added: jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/NodeCorruptionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/NodeCorruptionTest.java?rev=1518118&view=auto
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/NodeCorruptionTest.java (added)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/NodeCorruptionTest.java Wed Aug 28 09:21:18 2013
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.persistence.util;
+
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
+
+import org.apache.jackrabbit.test.AbstractJCRTest;
+import org.apache.jackrabbit.test.NotExecutableException;
+
+/**
+ * Tests that a node or a node bundle can not get internally corrupt.
+ */
+public class NodeCorruptionTest extends AbstractJCRTest {
+    
+    public void testSetIsCheckedOut() throws Exception {
+        Node root = superuser.getRootNode();
+        String nodeName = "test" + System.currentTimeMillis();
+        if (root.hasNode(nodeName)) {
+            root.getNode(nodeName).remove();
+            superuser.save();
+        }
+        Node node = root.addNode(nodeName);
+        superuser.save();
+        node.setProperty("jcr:isCheckedOut", true);
+        assertTrue(node.getProperty("jcr:isCheckedOut").getValue().getBoolean());
+    }
+    
+    public void testCopyMultiSingleValue() throws Exception {
+        Node root = superuser.getRootNode();
+        String nodeName = "testCopyMulti" + System.currentTimeMillis();
+        if (root.hasNode(nodeName)) {
+            root.getNode(nodeName).remove();
+            superuser.save();
+        }
+        Node test = root.addNode(nodeName);
+        test.setProperty("x", "Hi");
+        superuser.save();
+
+        String wsp = superuser.getWorkspace().getName();
+        String workspace2 = getAlternativeWorkspaceName();
+        if (workspace2 == null) {
+            throw new NotExecutableException();
+        }
+        Session s2 = getHelper().getSuperuserSession(workspace2);
+        s2.getWorkspace().clone(wsp, "/" + nodeName, "/" + nodeName, true);
+        
+        Node test2 = s2.getRootNode().getNode(nodeName);
+        test2.setProperty("x", (Value) null);
+        test2.setProperty("x", new String[]{});
+        s2.save();
+        
+        test.update(workspace2);
+
+        try {
+            Value[] values = test.getProperty("x").getValues();
+            assertEquals(0, values.length);
+        } catch (RepositoryException e) {
+            // if we get here, it's a bug, as it is a multi-valued property now
+            // anyway, let's see what happens if we try to read it as a single valued property
+            test.getProperty("x").getValue();
+            // even if that works: it's still a bug
+            throw e;
+        }
+        
+    }
+    
+    private String getAlternativeWorkspaceName() throws RepositoryException {
+        String altWsp = null;
+        String[] wsps = superuser.getWorkspace().getAccessibleWorkspaceNames();
+        if (wsps.length == 1) {
+            superuser.getWorkspace().createWorkspace("tmp");
+            altWsp = "tmp";
+        } else {
+            for (String name : wsps) {
+                if (!name.equals(superuser.getWorkspace().getName())) {
+                    altWsp = name;
+                    break;
+                }
+            }
+        }
+        return altWsp;
+    }
+    
+}

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/TestAll.java?rev=1518118&r1=1518117&r2=1518118&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/TestAll.java (original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/persistence/util/TestAll.java Wed Aug 28 09:21:18 2013
@@ -27,6 +27,8 @@ public class TestAll extends TestCase {
 
         suite.addTestSuite(HashMapIndexTest.class);
         suite.addTestSuite(BundleBindingTest.class);
+        suite.addTestSuite(NodeCorruptionTest.class);
+        suite.addTestSuite(BundleBindingRandomizedTest.class);
 
         return suite;
     }