jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: r157745 - in incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core: ItemImpl.java SessionImpl.java state/NodeState.java
Date Wed, 16 Mar 2005 13:30:50 GMT
Author: stefan
Date: Wed Mar 16 05:30:49 2005
New Revision: 157745

URL: http://svn.apache.org/viewcvs?view=rev&rev=157745
Log:
- fixed (JCR-69) removing source parent node after session move throws on save
  http://issues.apache.org/jira/browse/JCR-69
- fixed Session.move(): lock on source parent was not checked
  

Modified:
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/SessionImpl.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/NodeState.java

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java?view=diff&r1=157744&r2=157745
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java (original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java Wed Mar 16
05:30:49 2005
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.core;
 
 import org.apache.commons.collections.ReferenceMap;
+import org.apache.commons.collections.iterators.IteratorChain;
 import org.apache.jackrabbit.core.nodetype.ChildNodeDef;
 import org.apache.jackrabbit.core.nodetype.EffectiveNodeType;
 import org.apache.jackrabbit.core.nodetype.NodeTypeImpl;
@@ -56,8 +57,10 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * <code>ItemImpl</code> implements the <code>Item</code> interface.
@@ -1132,46 +1135,77 @@
                  */
                 stateMgr.enablePathCaching(true);
 
-                // build list of transient states that should be persisted
+                /**
+                 * build list of transient (i.e. new & modified) states that
+                 * should be persisted
+                 */
                 Collection dirty = getTransientStates();
                 if (dirty.size() == 0) {
                     // no transient items, nothing to do here
                     return;
                 }
 
-                ItemState transientState;
+                /**
+                 * build list of transient descendents in the attic
+                 * (i.e. those marked as 'removed')
+                 */
+                Collection removed = getRemovedStates();
 
                 /**
-                 * check that parent node is also included in the dirty items list
-                 * if dirty node was removed or added (adding/removing a parent/child
-                 * link requires that both parent and child are saved)
+                 * build set of item id's who are within the scope of
+                 * (i.e. affected by) this save operation
                  */
-                Iterator iter = dirty.iterator();
-                while (iter.hasNext()) {
-                    transientState = (ItemState) iter.next();
+                Set affectedIds = new HashSet(dirty.size() + removed.size());
+                for (Iterator it =
+                        new IteratorChain(dirty.iterator(), removed.iterator());
+                     it.hasNext();) {
+                    affectedIds.add(((ItemState) it.next()).getId());
+                }
+
+                /**
+                 * make sure that this save operation is totally 'self-contained'
+                 * and independant; items within the scope of this save operation
+                 * must not have 'external' dependencies;
+                 * (e.g. adding/removing a parent/child link requires that both
+                 * parent and child are saved)
+                 */
+                for (Iterator it =
+                        new IteratorChain(dirty.iterator(), removed.iterator());
+                     it.hasNext();) {
+                    ItemState transientState = (ItemState) it.next();
                     if (transientState.isNode()) {
                         NodeState nodeState = (NodeState) transientState;
-                        ArrayList dirtyParents = new ArrayList();
+                        Set dependentUUIDs = new HashSet();
                         // removed parents
-                        dirtyParents.addAll(nodeState.getRemovedParentUUIDs());
+                        dependentUUIDs.addAll(nodeState.getRemovedParentUUIDs());
                         // added parents
-                        dirtyParents.addAll(nodeState.getAddedParentUUIDs());
-                        Iterator parentsIter = dirtyParents.iterator();
-                        while (parentsIter.hasNext()) {
-                            NodeId id = new NodeId((String) parentsIter.next());
-                            NodeState parentState;
-                            try {
-                                parentState = (NodeState) stateMgr.getTransientItemState(id);
-                            } catch (ItemStateException ise) {
-                                // should never get here...
-                                String msg = "inconsistency: failed to retrieve transient
state for " + itemMgr.safeGetJCRPath(id);
-                                log.debug(msg);
-                                throw new RepositoryException(msg);
-                            }
-                            // check if parent is also going to be saved
-                            if (!dirty.contains(parentState)) {
-                                // need to save the parent too
-                                String msg = itemMgr.safeGetJCRPath(id) + " needs to be saved
also.";
+                        dependentUUIDs.addAll(nodeState.getAddedParentUUIDs());
+                        // removed child node entries
+                        for (Iterator cneIt =
+                                nodeState.getRemovedChildNodeEntries().iterator();
+                             cneIt.hasNext();) {
+                            NodeState.ChildNodeEntry cne =
+                                    (NodeState.ChildNodeEntry) cneIt.next();
+                            dependentUUIDs.add(cne.getUUID());
+                        }
+                        // added child node entries
+                        for (Iterator cneIt =
+                                nodeState.getAddedChildNodeEntries().iterator();
+                             cneIt.hasNext();) {
+                            NodeState.ChildNodeEntry cne =
+                                    (NodeState.ChildNodeEntry) cneIt.next();
+                            dependentUUIDs.add(cne.getUUID());
+                        }
+
+                        // now walk through dependencies and check whether they
+                        // are within the scope of this save operation
+                        Iterator depIt = dependentUUIDs.iterator();
+                        while (depIt.hasNext()) {
+                            NodeId id = new NodeId((String) depIt.next());
+                            if (!affectedIds.contains(id)) {
+                                // need to save the parent also
+                                String msg = itemMgr.safeGetJCRPath(id)
+                                        + " needs to be saved also.";
                                 log.debug(msg);
                                 throw new RepositoryException(msg);
                             }
@@ -1180,12 +1214,6 @@
                 }
 
                 /**
-                 * build list of transient descendents in the attic
-                 * (i.e. those marked as 'removed')
-                 */
-                Collection removed = getRemovedStates();
-
-                /**
                  * validate access and node type constraints
                  * (this will also validate child removals)
                  */
@@ -1227,9 +1255,8 @@
                     // transient item states must be removed now. otherwise
                     // the session item state provider will return an orphaned
                     // item state which is not referenced by any node instance.
-                    iter = dirty.iterator();
-                    while (iter.hasNext()) {
-                        transientState = (ItemState) iter.next();
+                    for (Iterator it = dirty.iterator(); it.hasNext();) {
+                        ItemState transientState = (ItemState) it.next();
                         // dispose the transient state, it is no longer used
                         stateMgr.disposeTransientItemState(transientState);
                     }
@@ -1259,9 +1286,8 @@
                 // item states in attic are removed after store, because
                 // the observation mechanism needs to build paths of removed
                 // items in store().
-                iter = removed.iterator();
-                while (iter.hasNext()) {
-                    transientState = (ItemState) iter.next();
+                for (Iterator it = removed.iterator(); it.hasNext();) {
+                    ItemState transientState = (ItemState) it.next();
                     // dispose the transient state, it is no longer used
                     stateMgr.disposeTransientItemStateInAttic(transientState);
                 }

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/SessionImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/SessionImpl.java?view=diff&r1=157744&r2=157745
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/SessionImpl.java (original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/SessionImpl.java Wed Mar
16 05:30:49 2005
@@ -898,6 +898,8 @@
         }
 
         // check lock status
+
+        srcParentNode.checkLock();
         destParentNode.checkLock();
 
         // do move operation
@@ -906,7 +908,8 @@
         // add target to new parent
         destParentNode.createChildNodeLink(destName.getName(), targetUUID);
         // remove target from old parent
-        srcParentNode.removeChildNode(srcName.getName(), srcName.getIndex() == 0 ? 1 : srcName.getIndex());
+        srcParentNode.removeChildNode(srcName.getName(),
+                srcName.getIndex() == 0 ? 1 : srcName.getIndex());
         // change definition of target if necessary
         NodeDefImpl oldTargetDef = (NodeDefImpl) targetNode.getDefinition();
         NodeDefId oldTargetDefId = new NodeDefId(oldTargetDef.unwrap());

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/NodeState.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/NodeState.java?view=diff&r1=157744&r2=157745
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/NodeState.java (original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/state/NodeState.java Wed
Mar 16 05:30:49 2005
@@ -369,15 +369,7 @@
      * @see #removeChildNodeEntry
      */
     public synchronized List getChildNodeEntries(String uuid) {
-        ArrayList list = new ArrayList();
-        Iterator iter = childNodeEntries.iterator();
-        while (iter.hasNext()) {
-            ChildNodeEntry entry = (ChildNodeEntry) iter.next();
-            if (entry.getUUID().equals(uuid)) {
-                list.add(entry);
-            }
-        }
-        return Collections.unmodifiableList(list);
+        return childNodeEntries.get(uuid);
     }
 
     /**
@@ -389,15 +381,7 @@
      * @see #removeChildNodeEntry
      */
     public synchronized List getChildNodeEntries(QName nodeName) {
-        ArrayList list = new ArrayList();
-        Iterator iter = childNodeEntries.iterator();
-        while (iter.hasNext()) {
-            ChildNodeEntry entry = (ChildNodeEntry) iter.next();
-            if (entry.getName().equals(nodeName)) {
-                list.add(entry);
-            }
-        }
-        return Collections.unmodifiableList(list);
+        return childNodeEntries.get(nodeName);
     }
 
     /**
@@ -517,14 +501,9 @@
             return Collections.EMPTY_LIST;
         }
 
-        ArrayList list = new ArrayList(parentUUIDs);
-
         NodeState other = (NodeState) getOverlayedState();
-        Iterator i = other.parentUUIDs.iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
+        ArrayList list = new ArrayList(parentUUIDs);
+        list.removeAll(other.parentUUIDs);
         return list;
     }
 
@@ -539,14 +518,9 @@
             return Collections.unmodifiableList(propertyEntries);
         }
 
-        ArrayList list = new ArrayList(propertyEntries);
-
         NodeState other = (NodeState) getOverlayedState();
-        Iterator i = other.propertyEntries.iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
+        ArrayList list = new ArrayList(propertyEntries);
+        list.removeAll(other.propertyEntries);
         return list;
     }
 
@@ -561,15 +535,8 @@
             return Collections.unmodifiableList(childNodeEntries.entries());
         }
 
-        ArrayList list = new ArrayList(childNodeEntries.entries());
-
         NodeState other = (NodeState) getOverlayedState();
-        Iterator i = other.childNodeEntries.entries().iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
-        return list;
+        return childNodeEntries.removeAll(other.childNodeEntries);
     }
 
     /**
@@ -585,12 +552,7 @@
 
         NodeState other = (NodeState) getOverlayedState();
         ArrayList list = new ArrayList(other.parentUUIDs);
-
-        Iterator i = parentUUIDs.iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
+        list.removeAll(parentUUIDs);
         return list;
     }
 
@@ -607,12 +569,7 @@
 
         NodeState other = (NodeState) getOverlayedState();
         ArrayList list = new ArrayList(other.propertyEntries);
-
-        Iterator i = propertyEntries.iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
+        list.removeAll(propertyEntries);
         return list;
     }
 
@@ -628,26 +585,20 @@
         }
 
         NodeState other = (NodeState) getOverlayedState();
-        ArrayList list = new ArrayList(other.childNodeEntries.entries());
-
-        Iterator i = childNodeEntries.entries().iterator();
-        while (i.hasNext()) {
-            list.remove(i.next());
-        }
-
-        return list;
+        return other.childNodeEntries.removeAll(childNodeEntries);
     }
 
     //--------------------------------------------------< ItemState overrides >
     /**
      * Sets the UUID of the parent <code>NodeState</code>.
      *
-     * @param parentUUID the parent <code>NodeState</code>'s UUID or <code>null</code>
-     *                   if either this item state should represent the root node or this
item state
-     *                   should be 'free floating', i.e. detached from the repository's hierarchy.
+     * @param parentUUID the parent <code>NodeState</code>'s UUID or
+     *                   <code>null</code> if either this item state should
+     *                   represent the root node or this item state should
+     *                   be 'free floating', i.e. detached from the repository's
+     *                   hierarchy.
      */
     public synchronized void setParentUUID(String parentUUID) {
-        // @todo is this correct?
         if (parentUUID != null && !parentUUIDs.contains(parentUUID)) {
             parentUUIDs.add(parentUUID);
         }
@@ -660,7 +611,8 @@
         out.defaultWriteObject();
     }
 
-    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException
{
+    private void readObject(ObjectInputStream in)
+            throws IOException, ClassNotFoundException {
         // delegate to default implementation
         in.defaultReadObject();
     }
@@ -752,12 +704,84 @@
             return true;
         }
 
+        List get(QName nodeName) {
+            List siblings = (List) names.get(nodeName);
+            if (siblings == null) {
+                return Collections.EMPTY_LIST;
+            } else {
+                return Collections.unmodifiableList(siblings);
+            }
+        }
+
+        boolean remove(QName nodeName, String uuid) {
+            List siblings = (List) names.get(nodeName);
+            if (siblings == null || siblings.isEmpty()) {
+                return false;
+            }
+
+            Iterator iter = siblings.iterator();
+            while (iter.hasNext()) {
+                ChildNodeEntry entry = (ChildNodeEntry) iter.next();
+                if (entry.getUUID().equals(uuid)) {
+                    return remove(entry);
+                }
+            }
+            return false;
+        }
+
+        List get(String uuid) {
+            if (entries.isEmpty()) {
+                return Collections.EMPTY_LIST;
+            }
+            ArrayList list = new ArrayList();
+            Iterator iter = entries.iterator();
+            while (iter.hasNext()) {
+                ChildNodeEntry entry = (ChildNodeEntry) iter.next();
+                if (entry.getUUID().equals(uuid)) {
+                    list.add(entry);
+                }
+            }
+            return Collections.unmodifiableList(list);
+        }
+
         Iterator iterator() {
             return entries.iterator();
         }
 
         List entries() {
             return Collections.unmodifiableList(entries);
+        }
+
+        /**
+         * Returns a list of <code>ChildNodeEntry</code>s who do only exist in
+         * <code>this</code> but not in <code>other</code>
+         * <p/>
+         * Note that two entries are considered identical in this context if
+         * they have the same name and uuid, i.e. the index is disregarded,
+         * whereas <code>ChildNodeEntry.equals(Object)</code> also compares
+         * the index.
+         *
+         * @param other entries to be removed
+         * @return a new list of entries who do only exist in <code>this</code>
+         *         but not in <code>other</code>
+         */
+        List removeAll(ChildNodeEntries other) {
+            if (entries.isEmpty()) {
+                return Collections.EMPTY_LIST;
+            }
+            if (other.entries.isEmpty()) {
+                return Collections.unmodifiableList(entries);
+            }
+
+            ChildNodeEntries result = new ChildNodeEntries();
+            result.addAll(entries);
+
+            Iterator iter = other.entries.iterator();
+            while (iter.hasNext()) {
+                ChildNodeEntry entry = (ChildNodeEntry) iter.next();
+                result.remove(entry.getName(), entry.getUUID());
+            }
+            return result.entries;
         }
     }
 



Mime
View raw message