You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2011/04/05 15:33:57 UTC
svn commit: r1089032 - in /jackrabbit/trunk:
jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/
jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/
jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/...
Author: angela
Date: Tue Apr 5 13:33:57 2011
New Revision: 1089032
URL: http://svn.apache.org/viewvc?rev=1089032&view=rev
Log:
JCR-2880 : Save fails after setting a binary property twice
Modified:
jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/SetPropertyValue.java
jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java
jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/BinaryTest.java
jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java
Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/SetPropertyValue.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/SetPropertyValue.java?rev=1089032&r1=1089031&r2=1089032&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/SetPropertyValue.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/operation/SetPropertyValue.java Tue Apr 5 13:33:57 2011
@@ -42,6 +42,8 @@ public class SetPropertyValue extends Ab
private final QValue[] values;
private final int valueType;
+ private final QValue[] oldValues;
+
private SetPropertyValue(PropertyState propertyState, int valueType, QValue[] values)
throws RepositoryException {
this.propertyState = propertyState;
@@ -50,6 +52,9 @@ public class SetPropertyValue extends Ab
this.valueType = valueType;
this.values = values;
+ // remember original values
+ oldValues = propertyState.getValues();
+
addAffectedItemState(propertyState);
}
@@ -70,7 +75,14 @@ public class SetPropertyValue extends Ab
public void persisted() throws RepositoryException {
assert status == STATUS_PENDING;
status = STATUS_PERSISTED;
- propertyState.getHierarchyEntry().complete(this);
+ try {
+ propertyState.getHierarchyEntry().complete(this);
+ } finally {
+ // dispose the original values
+ for (QValue v : oldValues) {
+ v.discard();
+ }
+ }
}
/**
@@ -81,6 +93,9 @@ public class SetPropertyValue extends Ab
assert status == STATUS_PENDING;
status = STATUS_UNDO;
propertyState.getHierarchyEntry().complete(this);
+
+ // NOTE: new values don't need to be disposed as the transient change
+ // has been reverted with implicit value disposal.
}
//----------------------------------------< Access Operation Parameters >---
Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java?rev=1089032&r1=1089031&r2=1089032&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java Tue Apr 5 13:33:57 2011
@@ -206,6 +206,7 @@ public class PropertyState extends ItemS
* this <code>PropertyState</code>.
*
* @return definition of this state
+ * @throws RepositoryException If an error occurs.
*/
public QPropertyDefinition getDefinition() throws RepositoryException {
if (definition == null) {
@@ -237,7 +238,7 @@ public class PropertyState extends ItemS
/**
* Convenience method for single valued property states.
*
- * @return
+ * @return the value of a single valued property.
* @throws ValueFormatException if {@link #isMultiValued()} returns true.
*/
public QValue getValue() throws ValueFormatException {
@@ -256,6 +257,8 @@ public class PropertyState extends ItemS
* Sets the value(s) of this property.
*
* @param values the new values
+ * @param type the value type
+ * @throws RepositoryException If an error occurs.
*/
void setValues(QValue[] values, int type) throws RepositoryException {
if (getStatus() == Status.NEW) {
@@ -363,8 +366,8 @@ public class PropertyState extends ItemS
// make sure the arguments are consistent and do not violate the
// given property definition.
validate(values, type, definition);
- // free old values if existing
- discardValues();
+ // note: discarding original values is deferred to operation completion
+ // -> see JCR-2880
this.type = type;
this.values = (values == null) ? QValue.EMPTY_ARRAY : values;
Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/BinaryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/BinaryTest.java?rev=1089032&r1=1089031&r2=1089032&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/BinaryTest.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/BinaryTest.java Tue Apr 5 13:33:57 2011
@@ -23,7 +23,10 @@ import javax.jcr.Node;
import javax.jcr.Session;
import javax.jcr.Property;
import javax.jcr.Binary;
+import javax.jcr.ValueFormatException;
+import org.apache.jackrabbit.jcr2spi.state.PropertyState;
+import org.apache.jackrabbit.spi.QValue;
import org.apache.jackrabbit.test.AbstractJCRTest;
/**
@@ -31,11 +34,29 @@ import org.apache.jackrabbit.test.Abstra
*/
public class BinaryTest extends AbstractJCRTest {
- public void testStreamBinary() throws Exception {
+ private static ByteArrayInputStream generateValue() {
byte[] data = new byte[1024 * 1024];
new Random().nextBytes(data);
+
+ return new ByteArrayInputStream(data);
+ }
+
+ private static QValue getQValue(Property p) throws ValueFormatException {
+ return ((PropertyState) ((PropertyImpl) p).getItemState()).getValue();
+ }
+
+ private static void assertDisposed(QValue v) {
+ try {
+ v.getStream();
+ fail("Value should have been disposed.");
+ } catch (Exception e) {
+ // success (interpret this as value was disposed)
+ }
+ }
+
+ public void testStreamBinary() throws Exception {
Node test = testRootNode.addNode("test");
- Property p = test.setProperty("prop", new ByteArrayInputStream(data));
+ Property p = test.setProperty("prop", generateValue());
// check before save
checkBinary(p);
superuser.save();
@@ -52,6 +73,79 @@ public class BinaryTest extends Abstract
}
}
+ public void testBinaryTwiceNewProperty() throws Exception {
+ Node test = testRootNode.addNode("test");
+ Property p = test.setProperty("prop", generateValue());
+ QValue qv1 = getQValue(p);
+ test.setProperty("prop", generateValue());
+ QValue qv2 = getQValue(p);
+
+ assertFalse(qv1.equals(qv2));
+
+ superuser.save();
+
+ assertEquals(qv2, getQValue(p));
+ assertDisposed(qv1);
+ }
+
+ public void testBinaryTwiceModifiedProperty() throws Exception {
+ Node test = testRootNode.addNode("test");
+ Property p = test.setProperty("prop", generateValue());
+ superuser.save();
+
+ // modify twice
+ test.setProperty("prop", generateValue());
+ QValue qv1 = getQValue(p);
+ test.setProperty("prop", generateValue());
+ QValue qv2 = getQValue(p);
+
+ assertFalse(qv1.equals(qv2));
+
+ superuser.save();
+
+ assertEquals(qv2, getQValue(p));
+ assertDisposed(qv1);
+ }
+
+ public void testBinaryTwiceIntermediateSave() throws Exception {
+ Node test = testRootNode.addNode("test");
+ Property p = test.setProperty("prop", generateValue());
+ QValue qv1 = getQValue(p);
+ superuser.save();
+
+ test.setProperty("prop", generateValue());
+ QValue qv2 = getQValue(p);
+
+ assertFalse(qv1.equals(qv2));
+
+ superuser.save();
+
+ assertEquals(qv2, getQValue(p));
+ assertDisposed(qv1);
+ }
+
+ public void testRevertSettingExistingBinary() throws Exception {
+ Node test = testRootNode.addNode("test");
+
+ Binary b = superuser.getValueFactory().createBinary(generateValue());
+ Property p = test.setProperty("prop", b);
+ QValue qv1 = getQValue(p);
+ superuser.save();
+
+ Binary b2 = superuser.getValueFactory().createBinary(generateValue());
+ test.setProperty("prop", b2);
+ QValue qv2 = getQValue(p);
+
+ assertFalse(qv1.equals(qv2));
+
+ superuser.refresh(false);
+
+ assertEquals(qv1, getQValue(p));
+ assertSame(qv1, getQValue(p));
+
+ assertFalse(qv2.equals(getQValue(p)));
+ }
+
protected void checkBinary(Property p) throws Exception {
for (int i = 0; i < 3; i++) {
Binary bin = p.getBinary();
Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java?rev=1089032&r1=1089031&r2=1089032&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2davex/RepositoryServiceImpl.java Tue Apr 5 13:33:57 2011
@@ -564,7 +564,7 @@ public class RepositoryServiceImpl exten
public void setValue(PropertyId propertyId, QValue value) throws RepositoryException {
assertMethod();
Path p = getPath(propertyId, sessionInfo);
- setProperty(p, value, false);
+ setProperty(p, value, true);
}
/**
@@ -573,7 +573,7 @@ public class RepositoryServiceImpl exten
public void setValue(PropertyId propertyId, QValue[] values) throws RepositoryException {
assertMethod();
Path p = getPath(propertyId, sessionInfo);
- setProperty(p, values, false);
+ setProperty(p, values, true);
}
/**