You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by th...@apache.org on 2008/01/29 17:38:09 UTC
svn commit: r616442 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/value/
test/java/org/apache/jackrabbit/core/data/
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;
}