jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject 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/...
Date Tue, 05 Apr 2011 13:33:57 GMT
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);
         }
 
         /**



Mime
View raw message