jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mdue...@apache.org
Subject svn commit: r747839 - in /jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi: ItemImpl.java hierarchy/HierarchyEntryImpl.java state/ItemState.java state/NodeState.java state/PropertyState.java
Date Wed, 25 Feb 2009 15:55:46 GMT
Author: mduerig
Date: Wed Feb 25 15:55:46 2009
New Revision: 747839

URL: http://svn.apache.org/viewvc?rev=747839&view=rev
Log:
JCR-1963: Determination of property state difference should skip binary values

Property states are now only diffed when necessary

Modified:
    jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/ItemImpl.java
    jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/hierarchy/HierarchyEntryImpl.java
    jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
    jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
    jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java

Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/ItemImpl.java?rev=747839&r1=747838&r2=747839&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/ItemImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/ItemImpl.java
Wed Feb 25 15:55:46 2009
@@ -16,22 +16,8 @@
  */
 package org.apache.jackrabbit.jcr2spi;
 
-import org.apache.commons.collections.map.ReferenceMap;
-import org.apache.jackrabbit.jcr2spi.config.CacheBehaviour;
-import org.apache.jackrabbit.jcr2spi.hierarchy.HierarchyEntry;
-import org.apache.jackrabbit.jcr2spi.hierarchy.NodeEntry;
-import org.apache.jackrabbit.jcr2spi.operation.Operation;
-import org.apache.jackrabbit.jcr2spi.operation.Remove;
-import org.apache.jackrabbit.jcr2spi.state.ItemState;
-import org.apache.jackrabbit.jcr2spi.state.ItemStateLifeCycleListener;
-import org.apache.jackrabbit.jcr2spi.state.ItemStateValidator;
-import org.apache.jackrabbit.jcr2spi.state.NodeState;
-import org.apache.jackrabbit.jcr2spi.state.Status;
-import org.apache.jackrabbit.jcr2spi.util.LogUtil;
-import org.apache.jackrabbit.spi.Name;
-import org.apache.jackrabbit.spi.Path;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import java.util.Collections;
+import java.util.Map;
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.InvalidItemStateException;
@@ -48,8 +34,23 @@
 import javax.jcr.lock.LockException;
 import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.version.VersionException;
-import java.util.Collections;
-import java.util.Map;
+
+import org.apache.commons.collections.map.ReferenceMap;
+import org.apache.jackrabbit.jcr2spi.config.CacheBehaviour;
+import org.apache.jackrabbit.jcr2spi.hierarchy.HierarchyEntry;
+import org.apache.jackrabbit.jcr2spi.hierarchy.NodeEntry;
+import org.apache.jackrabbit.jcr2spi.operation.Operation;
+import org.apache.jackrabbit.jcr2spi.operation.Remove;
+import org.apache.jackrabbit.jcr2spi.state.ItemState;
+import org.apache.jackrabbit.jcr2spi.state.ItemStateLifeCycleListener;
+import org.apache.jackrabbit.jcr2spi.state.ItemStateValidator;
+import org.apache.jackrabbit.jcr2spi.state.NodeState;
+import org.apache.jackrabbit.jcr2spi.state.Status;
+import org.apache.jackrabbit.jcr2spi.util.LogUtil;
+import org.apache.jackrabbit.spi.Name;
+import org.apache.jackrabbit.spi.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * <code>ItemImpl</code>...
@@ -58,7 +59,7 @@
 
     private static Logger log = LoggerFactory.getLogger(ItemImpl.class);
 
-    private ItemState state;
+    private final ItemState state;
 
     /**
      * The session that created this item.
@@ -263,7 +264,7 @@
                 session.getCacheBehaviour() != CacheBehaviour.OBSERVATION) {
                 // merge current transient modifications with latest changes
                 // from the 'server'.
-                // Note, that with Observation-CacheBehaviour no manuel refresh
+                // Note, that with Observation-CacheBehaviour no manual refresh
                 // is required. changes get pushed automatically.
                 state.getHierarchyEntry().invalidate(true);
             }
@@ -276,7 +277,7 @@
             }
 
             /*
-            Reset all transient modifications from this item and its decendants.
+            Reset all transient modifications from this item and its descendants.
             */
             session.getSessionItemStateManager().undo(state);
 

Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/hierarchy/HierarchyEntryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/hierarchy/HierarchyEntryImpl.java?rev=747839&r1=747838&r2=747839&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/hierarchy/HierarchyEntryImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/hierarchy/HierarchyEntryImpl.java
Wed Feb 25 15:55:46 2009
@@ -16,20 +16,22 @@
  */
 package org.apache.jackrabbit.jcr2spi.hierarchy;
 
+import java.lang.ref.Reference;
+import java.lang.ref.SoftReference;
+
+import javax.jcr.InvalidItemStateException;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.RepositoryException;
+
 import org.apache.jackrabbit.jcr2spi.state.ItemState;
-import org.apache.jackrabbit.jcr2spi.state.Status;
 import org.apache.jackrabbit.jcr2spi.state.ItemStateFactory;
+import org.apache.jackrabbit.jcr2spi.state.Status;
+import org.apache.jackrabbit.jcr2spi.state.ItemState.MergeResult;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.spi.Path;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.InvalidItemStateException;
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.RepositoryException;
-import java.lang.ref.SoftReference;
-import java.lang.ref.Reference;
-
 /**
  * <code>HierarchyEntryImpl</code> implements base functionality for child node
  * and property references.
@@ -222,10 +224,10 @@
             // with the passed state.
             int currentStatus = currentState.getStatus();
             boolean keepChanges = Status.isTransient(currentStatus) || Status.isStale(currentStatus);
-            boolean modified = currentState.merge(state, keepChanges);
+            MergeResult mergeResult = currentState.merge(state, keepChanges);
             if (currentStatus == Status.INVALIDATED) {
                 currentState.setStatus(Status.EXISTING);
-            } else if (modified) {
+            } else if (mergeResult.modified()) {
                 currentState.setStatus(Status.MODIFIED);
             } // else: not modified. just leave status as it is.
         }

Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java?rev=747839&r1=747838&r2=747839&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
(original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
Wed Feb 25 15:55:46 2009
@@ -16,6 +16,14 @@
  */
 package org.apache.jackrabbit.jcr2spi.state;
 
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+
+import javax.jcr.InvalidItemStateException;
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.RepositoryException;
+
 import org.apache.jackrabbit.jcr2spi.hierarchy.HierarchyEntry;
 import org.apache.jackrabbit.jcr2spi.hierarchy.NodeEntry;
 import org.apache.jackrabbit.jcr2spi.nodetype.ItemDefinitionProvider;
@@ -26,13 +34,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.InvalidItemStateException;
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.RepositoryException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-
 /**
  * <code>ItemState</code> represents the state of an <code>Item</code>.
  */
@@ -51,7 +52,7 @@
     /**
      * The hierarchy entry this state belongs to.
      */
-    private HierarchyEntry hierarchyEntry;
+    private final HierarchyEntry hierarchyEntry;
 
     /**
      * Listeners (weak references)
@@ -264,9 +265,9 @@
      *
      * @param another
      * @param keepChanges
-     * @return true if this state has been modified
+     * @return a MergeResult instance which represent the result of the merge operation
      */
-    public abstract boolean merge(ItemState another, boolean keepChanges);
+    public abstract MergeResult merge(ItemState another, boolean keepChanges);
 
     /**
      * Revert all transient modifications made to this ItemState.
@@ -333,4 +334,40 @@
                 throw new InvalidItemStateException(msg);
         }
     }
+
+    // -----------------------------------------------------< MergeResult >---
+
+    /**
+     * A MergeResult represents the result of a {@link ItemState#merge(ItemState, boolean)}
+     * operation.
+     */
+    public interface MergeResult {
+
+        /**
+         * @return  true iff the target state of {@link ItemState#merge(ItemState, boolean)}
+         * was modified.
+         */
+        public boolean modified();
+    }
+
+    /**
+     * A SimpleMergeResult is just a holder for a modification status.
+     * The {@link #modified()} method just returns the modification status passed
+     * to the constructor.
+     */
+    protected class SimpleMergeResult implements MergeResult {
+        private final boolean modified;
+
+        /**
+         * @param modified  modification status
+         */
+        public SimpleMergeResult(boolean modified) {
+            this.modified = modified;
+        }
+
+        public boolean modified() {
+            return modified;
+        }
+    }
+
 }

Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java?rev=747839&r1=747838&r2=747839&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
(original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
Wed Feb 25 15:55:46 2009
@@ -16,6 +16,12 @@
  */
 package org.apache.jackrabbit.jcr2spi.state;
 
+import java.util.Arrays;
+import java.util.List;
+
+import javax.jcr.ItemNotFoundException;
+import javax.jcr.RepositoryException;
+
 import org.apache.jackrabbit.jcr2spi.hierarchy.NodeEntry;
 import org.apache.jackrabbit.jcr2spi.hierarchy.PropertyEntry;
 import org.apache.jackrabbit.jcr2spi.nodetype.ItemDefinitionProvider;
@@ -31,11 +37,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.ItemNotFoundException;
-import javax.jcr.RepositoryException;
-import java.util.Arrays;
-import java.util.List;
-
 /**
  * <code>NodeState</code> represents the state of a <code>Node</code>.
  */
@@ -122,31 +123,30 @@
     /**
      * @see ItemState#merge(ItemState, boolean)
      */
-    public boolean merge(ItemState another, boolean keepChanges) {
-        if (another == null || another == this) {
-            return false;
-        }
-        if (!another.isNode()) {
-            throw new IllegalArgumentException("Attempt to merge node state with property
state.");
-        }
+    public MergeResult merge(ItemState another, boolean keepChanges) {
         boolean modified = false;
-        synchronized (another) {
-            NodeState nState = (NodeState) another;
-
-            if (nState.definition != null && !nState.definition.equals(definition))
{
-                definition = nState.definition;
-                modified = true;
+        if (another != null && another != this) {
+            if (!another.isNode()) {
+                throw new IllegalArgumentException("Attempt to merge node state with property
state.");
             }
+            synchronized (another) {
+                NodeState nState = (NodeState) another;
 
-            // since 'mixinTypeNames' are modified upon save only, no special
-            // merging is required here. just reset the mixinTypeNames.
-            List mixN = Arrays.asList(nState.mixinTypeNames);
-            if (mixN.size() != mixinTypeNames.length || !mixN.containsAll(Arrays.asList(mixinTypeNames)))
{
-                setMixinTypeNames(nState.mixinTypeNames);
-                modified = true;
+                if (nState.definition != null && !nState.definition.equals(definition))
{
+                    definition = nState.definition;
+                    modified = true;
+                }
+
+                // since 'mixinTypeNames' are modified upon save only, no special
+                // merging is required here. just reset the mixinTypeNames.
+                List mixN = Arrays.asList(nState.mixinTypeNames);
+                if (mixN.size() != mixinTypeNames.length || !mixN.containsAll(Arrays.asList(mixinTypeNames)))
{
+                    setMixinTypeNames(nState.mixinTypeNames);
+                    modified = true;
+                }
             }
         }
-        return modified;
+        return new SimpleMergeResult(modified);
     }
 
     /**
@@ -254,7 +254,7 @@
      * TODO: clarify usage
      * In case the status of the given node state is not {@link Status#EXISTING}
      * the transiently added mixin types are taken into account as well.
-     * 
+     *
      * @return
      */
     public synchronized Name[] getAllNodeTypeNames() {

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=747839&r1=747838&r2=747839&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
Wed Feb 25 15:55:46 2009
@@ -16,6 +16,11 @@
  */
 package org.apache.jackrabbit.jcr2spi.state;
 
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.ValueFormatException;
+import javax.jcr.nodetype.ConstraintViolationException;
+
 import org.apache.jackrabbit.jcr2spi.hierarchy.PropertyEntry;
 import org.apache.jackrabbit.jcr2spi.nodetype.ItemDefinitionProvider;
 import org.apache.jackrabbit.spi.ItemId;
@@ -26,11 +31,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.ValueFormatException;
-import javax.jcr.nodetype.ConstraintViolationException;
-
 /**
  * <code>PropertyState</code> represents the state of a <code>Property</code>.
  */
@@ -130,28 +130,28 @@
      *
      * @see ItemState#merge(ItemState, boolean)
      */
-    public boolean merge(ItemState another, boolean keepChanges) {
-        if (another == null || another == this) {
-            return false;
-        }
-        if (another.isNode()) {
-            throw new IllegalArgumentException("Attempt to merge property state with node
state.");
-        }
-        // calculate if the persistent values of this state differ from the
-        // other state.
-        boolean diff = diff(data, ((PropertyState) another).data);
-        // reset the pInfo to point to the pInfo of another state.
-        this.data = ((PropertyState) another).data;
-        // if transient changes should be preserved OR if there are not
-        // transient changes, simply return diff to indicate if this state
-        // was internally changed.
-        if (keepChanges || transientData == null) {
-            return diff;
-        } else {
-            transientData.discardValues();
-            transientData = null;
-            return true;
+    public MergeResult merge(ItemState another, boolean keepChanges) {
+        boolean modified = false;
+        if (another != null && another != this) {
+            if (another.isNode()) {
+                throw new IllegalArgumentException("Attempt to merge property state with
node state.");
+            }
+            PropertyDiffer result = new PropertyDiffer(data, ((PropertyState) another).data);
+
+            // reset the pInfo to point to the pInfo of another state.
+            this.data = ((PropertyState) another).data;
+            // if transient changes should be preserved OR if there are not
+            // transient changes, simply return diff to indicate if this state
+            // was internally changed.
+            if (keepChanges || transientData == null) {
+                return result;
+            } else {
+                transientData.discardValues();
+                transientData = null;
+                modified = true;
+            }
         }
+        return new SimpleMergeResult(modified);
     }
 
     /**
@@ -340,9 +340,9 @@
      * Inner class storing property values an their type.
      */
     private class PropertyData {
-
         private int type;
         private QValue[] values;
+        private boolean discarded;
 
         private PropertyData(PropertyInfo pInfo) {
             this.type = pInfo.getType();
@@ -365,6 +365,7 @@
         }
 
         private void discardValues() {
+            discarded = true;
             if (values != null) {
                 for (int i = 0; i < values.length; i++) {
                     if (values[i] != null) {
@@ -376,4 +377,27 @@
             }
         }
     }
+
+    /**
+     * Helper class for delayed determination of property differences.
+     */
+    private class PropertyDiffer implements MergeResult {
+        private final PropertyData thisData;
+        private final PropertyData thatData;
+
+        PropertyDiffer(PropertyData thisData, PropertyData thatData) {
+            super();
+            this.thisData = thisData;
+            this.thatData = thatData;
+        }
+
+        public boolean modified() {
+            if (thisData.discarded || thatData.discarded) {
+                log.warn("Property data has been discarded");
+                return true;
+            }
+            return diff(thisData, thatData);
+        }
+    }
+
 }



Mime
View raw message