jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r616442 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/value/ test/java/org/apache/jackrabbit/core/data/
Date Tue, 29 Jan 2008 16:38:09 GMT
Author: thomasm
Date: Tue Jan 29 08:38:02 2008
New Revision: 616442

URL: http://svn.apache.org/viewvc?rev=616442&view=rev
Log:
JCR-1346 InternalValue.createCopy for binary properties (jcr:data) causes problems

Added:
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java
  (with props)
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInMemory.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInResource.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInTempFile.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBValue.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBFileValue.java
Tue Jan 29 08:38:02 2008
@@ -69,6 +69,14 @@
     public abstract void delete(boolean pruneEmptyParentDirs);
 
     /**
+     * Checks if this object is immutable.
+     * Immutable objects can not change and can safely copied.
+     *
+     * @return true if the object is immutable
+     */
+    public abstract boolean isImmutable();
+
+    /**
      * {@inheritDoc}
      */
     public abstract boolean equals(Object obj);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInDataStore.java
Tue Jan 29 08:38:02 2008
@@ -61,6 +61,13 @@
         // do nothing
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public boolean isImmutable() {
+        return true;
+    }
+
     public boolean equals(Object obj) {
         if (!(obj instanceof BLOBInDataStore) || obj == null) {
             return false;

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInMemory.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInMemory.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInMemory.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInMemory.java
Tue Jan 29 08:38:02 2008
@@ -119,16 +119,25 @@
      * {@inheritDoc}
      */
     public void delete(boolean pruneEmptyParentDirs) {
-        // TODO avoid: this makes the value mutable
-        data = EMPTY_BYTE_ARRAY;
+        // do nothing
+        // this object could still be referenced
+        // the data will be garbage collected
     }
 
     /**
      * {@inheritDoc}
      */
     public void discard() {
-        // TODO avoid: this makes the value mutable
-        data = EMPTY_BYTE_ARRAY;
+        // do nothing
+        // this object could still be referenced
+        // the data will be garbage collected
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public boolean isImmutable() {
+        return true;
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInResource.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInResource.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInResource.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInResource.java
Tue Jan 29 08:38:02 2008
@@ -103,6 +103,14 @@
     /**
      * {@inheritDoc}
      */
+    public boolean isImmutable() {
+        // delete will modify the state.
+        return false;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
     public long getLength() {
         return length;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInTempFile.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInTempFile.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInTempFile.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBInTempFile.java
Tue Jan 29 08:38:02 2008
@@ -133,6 +133,14 @@
     /**
      * {@inheritDoc}
      */
+    public boolean isImmutable() {
+        // discard and delete can modify the state.
+        return false;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
     public long getLength() {
         return length;
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBValue.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBValue.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/BLOBValue.java
Tue Jan 29 08:38:02 2008
@@ -237,7 +237,7 @@
      * @see #delete(boolean)
      */
     public void discard() {
-        if (!temp) {
+        if (!temp){
             // do nothing if this instance is not backed by temporarily
             // allocated resource/buffer
             return;
@@ -280,6 +280,14 @@
             // this instance is backed by an in-memory buffer
             buffer = EMPTY_BYTE_ARRAY;
         }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public boolean isImmutable() {
+        // delete will modify the state
+        return false;
     }
 
     //-------------------------------------------< java.lang.Object overrides >

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/InternalValue.java
Tue Jan 29 08:38:02 2008
@@ -121,7 +121,7 @@
         switch (value.getType()) {
             case PropertyType.BINARY:
                 if (USE_DATA_STORE) {
-                    return new InternalValue(getBLOBFileValue(store, value.getStream()));
+                    return new InternalValue(getBLOBFileValue(store, value.getStream(), true));
                 }
                 if (value instanceof BLOBFileValue) {
                     return new InternalValue((BLOBFileValue) value);
@@ -226,7 +226,7 @@
      */
     public static InternalValue createTemporary(InputStream value) throws RepositoryException
{
         if (USE_DATA_STORE) {
-            return new InternalValue(getBLOBFileValue(null, value));
+            return new InternalValue(getBLOBFileValue(null, value, true));
         }
         try {
             return new InternalValue(new BLOBValue(value, true));
@@ -246,7 +246,7 @@
      */
     public static InternalValue createTemporary(InputStream value, DataStore store) throws
RepositoryException {
         if (USE_DATA_STORE) {
-            return new InternalValue(getBLOBFileValue(store, value));
+            return new InternalValue(getBLOBFileValue(store, value, true));
         }
         try {
             return new InternalValue(new BLOBValue(value, true));
@@ -263,7 +263,7 @@
      */
     public static InternalValue create(InputStream value) throws RepositoryException {
         if (USE_DATA_STORE) {
-            return new InternalValue(getBLOBFileValue(null, value));
+            return new InternalValue(getBLOBFileValue(null, value, false));
         }
         try {
             return new InternalValue(new BLOBValue(value, false));
@@ -440,6 +440,8 @@
     }
 
     /**
+     * Get the type of this value.
+     *
      * @return the type
      */
     public int getType() {
@@ -447,29 +449,34 @@
     }
 
     /**
-     * @return
+     * Create a copy of this object. Immutable values will return itself,
+     * while mutable values will return a copy.
+     *
+     * @return itself or a copy
      * @throws RepositoryException
      */
     public InternalValue createCopy() throws RepositoryException {
-        if (USE_DATA_STORE) {
+        if (type != PropertyType.BINARY) {
+            // for all types except BINARY it's safe to return 'this' because the
+            // wrapped value is immutable (and therefore this instance as well)
             return this;
         }
-        if (type == PropertyType.BINARY) {
-            // return a copy since the wrapped BLOBFileValue instance is mutable
-            InputStream stream = ((BLOBFileValue) val).getStream();
+        BLOBFileValue v = (BLOBFileValue) val;
+        if (USE_DATA_STORE) {
+            if (v.isImmutable()) {
+                return this;
+            }
+        }
+        // return a copy since the wrapped BLOBFileValue instance is mutable
+        InputStream stream = v.getStream();
+        try {
+            return createTemporary(stream);
+        } finally {
             try {
-                return createTemporary(stream);
-            } finally {
-                try {
-                    stream.close();
-                } catch (IOException e) {
-                    // ignore
-                }
+                stream.close();
+            } catch (IOException e) {
+                // ignore
             }
-        } else {
-            // for all other types it's safe to return 'this' because the
-            // wrapped value is immutable (and therefore this instance as well)
-            return this;
         }
     }
 
@@ -595,7 +602,16 @@
         type = PropertyType.REFERENCE;
     }
 
-    private static BLOBFileValue getBLOBFileValue(DataStore store, InputStream in) throws
RepositoryException {
+    /**
+     * Create a BLOB value from in input stream. Small objects will create an in-memory object,
+     * while large objects are stored in the data store or in a temp file (if the store parameter
is not set).
+     *
+     * @param store the data store (optional)
+     * @param in the input stream
+     * @param temporary if the file should be deleted when discard is called (ignored if
a data store is used)
+     * @return the value
+     */
+    private static BLOBFileValue getBLOBFileValue(DataStore store, InputStream in, boolean
temporary) throws RepositoryException {
         int maxMemorySize;
         if (store != null) {
             maxMemorySize = store.getMinRecordLength() - 1;
@@ -628,7 +644,7 @@
             if (store != null) {
                 return BLOBInDataStore.getInstance(store, in);
             } else {
-                return BLOBInTempFile.getInstance(in, true);
+                return BLOBInTempFile.getInstance(in, temporary);
             }
         }
     }
@@ -643,6 +659,13 @@
         }
     }
 
+    /**
+     * Store a value in the data store. This will store temporary files or in-memory objects
+     * in the data store.
+     *
+     * @param dataStore the data store
+     * @throws RepositoryException
+     */
     public void store(DataStore dataStore) throws RepositoryException, IOException {
         assert USE_DATA_STORE;
         assert dataStore != null;
@@ -658,7 +681,7 @@
             }
         }
         // store the temp file to the data store, or (theoretically) load it in memory
-        val = getBLOBFileValue(dataStore, v.getStream());
+        val = getBLOBFileValue(dataStore, v.getStream(), false);
     }
 
 }

Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java?rev=616442&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java
(added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java
Tue Jan 29 08:38:02 2008
@@ -0,0 +1,168 @@
+/*
+ * 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.data;
+
+import org.apache.jackrabbit.test.AbstractJCRTest;
+
+import java.io.ByteArrayInputStream;
+import java.util.Random;
+
+import javax.jcr.Node;
+import javax.jcr.Property;
+import javax.jcr.Session;
+
+/**
+ * Tests copying binary values from one node to another.
+ * See also {@link https://issues.apache.org/jira/browse/JCR-1351}
+ * and {@link https://issues.apache.org/jira/browse/JCR-1346}
+ */
+public class CopyValueTest extends AbstractJCRTest {
+
+    static {
+        // to test without data store, enable the following line:
+        // System.setProperty("org.jackrabbit.useDataStore", "false");
+    }
+
+    /**
+     * Tests the Workspace.copy() operation with various lengths.
+     */
+    public void testCopyStream() throws Exception {
+        doTestCopy(100000);
+        doTestCopy(0);
+        doTestCopy(1);
+        doTestCopy(10);
+        doTestCopy(100);
+        doTestCopy(1000);
+    }
+
+    private void doTestCopy(int length) throws Exception {
+        Session session = helper.getReadWriteSession();
+        Node root = session.getRootNode();
+        if(root.hasNode("testCopy")) {
+            root.getNode("testCopy").remove();
+            session.save();
+        }
+        Node testRoot = root.addNode("testCopy");
+        Node n = testRoot.addNode("a");
+        session.save();
+        byte[] data = new byte[length + 1];
+        n.setProperty("data", new ByteArrayInputStream(data));
+        session.save();
+        data = new byte[length];
+        n.setProperty("data", new ByteArrayInputStream(data));
+        Property p = testRoot.getNode("a").getProperty("data");
+        assertEquals(length, p.getLength());
+        session.getWorkspace().copy("/testCopy/a", "/testCopy/b");
+        assertEquals(length, p.getLength());
+        p = testRoot.getNode("b").getProperty("data");
+        assertEquals(length + 1, p.getLength());
+        testRoot.remove();
+        session.save();
+    }
+
+    /**
+     * Test random operations:
+     * create, delete, move, and verify node; save and refresh a session.
+     * The test always runs the same sequence of operations.
+     */
+    public void testRandomOperations() throws Exception {
+        Random random = new Random(1);
+        Session session = helper.getReadWriteSession();
+        Node root = session.getRootNode();
+        if(root.hasNode("testRandom")) {
+            root.getNode("testRandom").remove();
+            session.save();
+        }
+        Node testRoot = root.addNode("testRandom");
+        int len = 1000;
+        int[] opCounts = new int[6];
+        for (int i = 0; i < len; i++) {
+            String node1 = "node" + random.nextInt(len / 20);
+            boolean hasNode1 = testRoot.hasNode(node1);
+            String node2 = "node" + random.nextInt(len / 20);
+            boolean hasNode2 = testRoot.hasNode(node2);
+            int op = random.nextInt(6);
+            switch (op) {
+            case 0: {
+                if (hasNode1) {
+                    log(node1 + " remove");
+                    testRoot.getNode(node1).remove();
+                }
+                opCounts[op]++;
+                Node n = testRoot.addNode(node1);
+                int dataLength = Math.abs(Math.min(10000, (int) (10000 * random
+                        .nextGaussian())));
+                byte[] data = new byte[dataLength];
+                log(node1 + " add len:" + dataLength);
+                n.setProperty("data", new ByteArrayInputStream(data));
+                n.setProperty("len", dataLength);
+                break;
+            }
+            case 1:
+                opCounts[op]++;
+                log("save");
+                session.save();
+                break;
+            case 2:
+                opCounts[op]++;
+                log("refresh");
+                session.refresh(false);
+                break;
+            case 3:
+                if (hasNode1) {
+                    opCounts[op]++;
+                    log(node1 + " remove");
+                    testRoot.getNode(node1).remove();
+                }
+                break;
+            case 4: {
+                if (hasNode1) {
+                    opCounts[op]++;
+                    Node n = testRoot.getNode(node1);
+                    long dataLength = n.getProperty("len").getLong();
+                    long l = n.getProperty("data").getLength();
+                    log(node1 + " verify len: " + dataLength);
+                    assertEquals(dataLength, l);
+                }
+                break;
+            }
+            case 5:
+                if (hasNode1 && !hasNode2) {
+                    opCounts[op]++;
+                    log(node1 + " copy " + node2);
+                    // todo: why is save required?
+                    session.save();
+                    session.getWorkspace().copy("/testRandom/" + node1,
+                            "/testRandom/" + node2);
+                }
+                break;
+            }
+        }
+        session.save();
+        for(int i=0; i<opCounts.length; i++) {
+            log(i + ": " + opCounts[i]);
+        }
+        testRoot.remove();
+        session.save();
+    }
+
+    private void log(String s) {
+        // to log operations, enable the following line:
+        // System.out.println(s);
+    }
+
+}

Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/CopyValueTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java?rev=616442&r1=616441&r2=616442&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
Tue Jan 29 08:38:02 2008
@@ -36,6 +36,7 @@
         suite.addTestSuite(ExportImportTest.class);
         suite.addTestSuite(GarbageCollectorTest.class);
         suite.addTestSuite(PersistenceManagerIteratorTest.class);
+        suite.addTestSuite(CopyValueTest.class);
         return suite;
     }
 



Mime
View raw message