You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by mr...@apache.org on 2006/08/21 16:10:20 UTC

svn commit: r433248 - in /jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state: ChildPropertyEntry.java ItemState.java NodeState.java PropertyReference.java PropertyState.java SessionItemStateManager.java

Author: mreutegg
Date: Mon Aug 21 07:10:18 2006
New Revision: 433248

URL: http://svn.apache.org/viewvc?rev=433248&view=rev
Log:
- Introduce ChildPropertyEntry.isAvailable()
- Add ItemState.collectTransientStates() which traverses the ItemState hierarchy and collects the transiently modified ItemStates.
- Change various methods of NodeState to return only valid Property- and Node-States.
- Change SessionItemStateManager.getChangeLog() to use new method ItemState.collectTransientStates() and remove unused methods.
- Prepare removal of ZombieHierarchyManager :)

Modified:
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ChildPropertyEntry.java
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyReference.java
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java
    jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/SessionItemStateManager.java

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ChildPropertyEntry.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ChildPropertyEntry.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ChildPropertyEntry.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ChildPropertyEntry.java Mon Aug 21 07:10:18 2006
@@ -42,4 +42,14 @@
      * <code>PropertyState</code>.
      */
     public PropertyState getPropertyState() throws NoSuchItemStateException, ItemStateException;
+
+    /**
+     * Returns <code>true</code> if the referenced <code>PropertyState</code> is
+     * available. That is, the referenced <code>PropertyState</code> is already
+     * cached and ready to be returned by {@link #getPropertyState()}.
+     *
+     * @return <code>true</code> if the <code>PropertyState</code> is available;
+     *         otherwise <code>false</code>.
+     */
+    public boolean isAvailable();
 }

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/ItemState.java Mon Aug 21 07:10:18 2006
@@ -485,6 +485,19 @@
     public abstract void revert(Set affectedItemStates);
 
     /**
+     * Checks if this <code>ItemState</code> is transiently modified or new and
+     * adds itself to the <code>Set</code> of <code>transientStates</code> if
+     * that is the case. It this <code>ItemState</code> has children it will
+     * call the method {@link #collectTransientStates(java.util.Set)} on those
+     * <code>ItemState</code>s.
+     *
+     * @param transientStates the <code>Set</code> of transient <code>ItemState</code>,
+     *                        collected while the <code>ItemState</code>
+     *                        hierarchy is traversed.
+     */
+    public abstract void collectTransientStates(Set transientStates);
+
+    /**
      * Add an <code>ItemStateListener</code>
      *
      * @param listener the new listener to be informed on modifications

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/NodeState.java Mon Aug 21 07:10:18 2006
@@ -325,7 +325,7 @@
      *         <code>false</code> otherwise.
      */
     public boolean hasChildNodeEntries() {
-        return !childNodeEntries.isEmpty();
+        return containsValidChildNodeEntry(childNodeEntries);
     }
 
     /**
@@ -337,7 +337,7 @@
      *         the specified <code>name</code>.
      */
     public synchronized boolean hasChildNodeEntry(QName name) {
-        return !childNodeEntries.get(name).isEmpty();
+        return containsValidChildNodeEntry(childNodeEntries.get(name));
     }
 
     /**
@@ -620,6 +620,45 @@
     }
 
     /**
+     * @inheritDoc
+     * @see ItemState#collectTransientStates(Set)
+     */
+    public void collectTransientStates(Set transientStates) {
+        switch (status) {
+            case STATUS_EXISTING_MODIFIED:
+            case STATUS_EXISTING_REMOVED:
+            case STATUS_NEW:
+            case STATUS_STALE_DESTROYED:
+            case STATUS_STALE_MODIFIED:
+                transientStates.add(this);
+        }
+        // call available property states
+        for (Iterator it = properties.values().iterator(); it.hasNext(); ) {
+            PropertyReference ref = (PropertyReference) it.next();
+            if (ref.isAvailable()) {
+                try {
+                    ref.getPropertyState().collectTransientStates(transientStates);
+                } catch (ItemStateException e) {
+                    // should not happen because ref is available
+                }
+            }
+        }
+        // add all properties in attic
+        transientStates.addAll(propertiesInAttic.values());
+        // call available child node states
+        for (Iterator it = childNodeEntries.iterator(); it.hasNext(); ) {
+            ChildNodeEntry cne = (ChildNodeEntry) it.next();
+            if (cne.isAvailable()) {
+                try {
+                    cne.getNodeState().collectTransientStates(transientStates);
+                } catch (ItemStateException e) {
+                    // should not happen because cne is available
+                }
+            }
+        }
+    }
+
+    /**
      * Determines if there is a property entry with the specified
      * <code>QName</code>.
      *
@@ -628,7 +667,18 @@
      *         <code>QName</code>.
      */
     public synchronized boolean hasPropertyName(QName propName) {
-        return properties.containsKey(propName);
+        PropertyReference ref = (PropertyReference) properties.get(propName);
+        if (ref.isResolved()) {
+            try {
+                return ref.getPropertyState().isValid();
+            } catch (ItemStateException e) {
+                // probably deleted in the meantime
+                return false;
+            }
+        } else {
+            // then it must be valid
+            return true;
+        }
     }
 
     /**
@@ -639,7 +689,17 @@
      * @see #addPropertyName
      */
     public synchronized Collection getPropertyNames() {
-        return Collections.unmodifiableSet(properties.keySet());
+        Collection names;
+        if (status == STATUS_EXISTING_MODIFIED) {
+            names = new ArrayList();
+            for (Iterator it = getPropertyEntries().iterator(); it.hasNext(); ) {
+                names.add(((ChildPropertyEntry) it.next()).getName());
+            }
+        } else {
+            // this node state is unmodified, return all
+            names = properties.keySet();
+        }
+        return Collections.unmodifiableCollection(names);
     }
 
     /**
@@ -649,7 +709,30 @@
      * @see #addPropertyName
      */
     public synchronized Collection getPropertyEntries() {
-        return Collections.unmodifiableCollection(properties.values());
+        Collection props;
+        if (status == STATUS_EXISTING_MODIFIED) {
+            // filter out removed properties
+            props = new ArrayList();
+            for (Iterator it = properties.values().iterator(); it.hasNext(); ) {
+                ChildPropertyEntry propEntry = (ChildPropertyEntry) it.next();
+                if (propEntry.isAvailable()) {
+                    try {
+                        if (propEntry.getPropertyState().isValid()) {
+                            props.add(propEntry);
+                        }
+                    } catch (ItemStateException e) {
+                        // removed in the meantime -> ignore
+                    }
+                } else {
+                    // never been accessed before, assume valid
+                    props.add(propEntry);
+                }
+            }
+        } else {
+            // no need to filter out properties, there are no removed properties
+            props = properties.values();
+        }
+        return Collections.unmodifiableCollection(props);
     }
 
     /**
@@ -731,6 +814,26 @@
      */
     public synchronized PropertyState getPropertyState(QName propertyName)
             throws NoSuchItemStateException, ItemStateException {
+        PropertyState propState = getAnyPropertyState(propertyName);
+        if (propState.isValid()) {
+            return propState;
+        } else {
+            throw new NoSuchItemStateException(idFactory.createPropertyId(getNodeId(), propertyName).toString());
+        }
+    }
+
+    /**
+     * Returns the property state with the given name and also takes removed
+     * property states into account.
+     *
+     * @param propertyName the name of the property state to return.
+     * @throws NoSuchItemStateException if there is no property state with the
+     *                                  given name.
+     * @throws ItemStateException       if an error occurs while retrieving the
+     *                                  property state.
+     */
+    public synchronized PropertyState getAnyPropertyState(QName propertyName)
+            throws NoSuchItemStateException, ItemStateException {
         PropertyReference propRef = (PropertyReference) properties.get(propertyName);
         if (propRef == null) {
             throw new NoSuchItemStateException(idFactory.createPropertyId(getNodeId(), propertyName).toString());
@@ -873,6 +976,9 @@
         } else {
             throw new RepositoryException("Unexpected error: Child state to be renamed does not exist.");
         }
+        // mark both this and newParent modified
+        markModified();
+        newParent.markModified();
     }
 
     /**
@@ -913,8 +1019,57 @@
      *         found in this <code>NodeState</code>.
      */
     int getChildNodeIndex(QName name, ChildNodeEntry cne) {
+        if (!this.getDefinition().allowsSameNameSiblings()) {
+            return Path.INDEX_DEFAULT;
+        }
         List sns = childNodeEntries.get(name);
-        return sns.indexOf(cne) + 1;
+        // index is one based
+        int index = 1;
+        for (Iterator it = sns.iterator(); it.hasNext(); ) {
+            ChildNodeEntry e = (ChildNodeEntry) it.next();
+            if (e == cne) {
+                return index;
+            }
+            // skip removed entries
+            try {
+                if (e.isAvailable() && e.getNodeState().isValid()) {
+                    index++;
+                }
+            } catch (ItemStateException ex) {
+                // probably removed or stale
+            }
+        }
+        // not found
+        return 0;
+    }
+
+    //-------------------------------< internal >-------------------------------
+
+    /**
+     * Returns <code>true</code> if the collection of child node
+     * <code>entries</code> contains at least one valid <code>ChildNodeEntry</code>.
+     *
+     * @param entries the collection to check.
+     * @return <code>true</code> if one of the entries is valid; otherwise
+     *         <code>false</code>.
+     */
+    private static boolean containsValidChildNodeEntry(Collection entries) {
+        for (Iterator it = entries.iterator(); it.hasNext(); ) {
+            ChildNodeEntry cne = (ChildNodeEntry) it.next();
+            if (cne.isAvailable()) {
+                try {
+                    if (cne.getNodeState().isValid()) {
+                        return true;
+                    }
+                } catch (ItemStateException e) {
+                    // probably removed in the meantime, check next
+                }
+            } else {
+                // then it has never been accessed and must exist
+                return true;
+            }
+        }
+        return false;
     }
 
     //------------------------------------------------------< inner classes >---
@@ -986,7 +1141,8 @@
 
         /**
          * Returns a <code>List</code> of <code>ChildNodeEntry</code>s for the
-         * given <code>nodeName</code>.
+         * given <code>nodeName</code>. This method does <b>not</b> filter out
+         * removed <code>ChildNodeEntry</code>s!
          *
          * @param nodeName the child node name.
          * @return same name sibling nodes with the given <code>nodeName</code>.
@@ -1037,7 +1193,8 @@
 
         /**
          * Returns the <code>ChildNodeEntry</code> with the given
-         * <code>nodeName</code> and <code>index</code>.
+         * <code>nodeName</code> and <code>index</code>. This method ignores
+         * <code>ChildNodeEntry</code>s which are marked removed!
          *
          * @param nodeName name of the child node entry.
          * @param index    the index of the child node entry.
@@ -1056,8 +1213,27 @@
             if (obj instanceof List) {
                 // map entry is a list of siblings
                 List siblings = (List) obj;
-                if (index <= siblings.size()) {
-                    return ((LinkedEntries.LinkNode) siblings.get(index - 1)).getChildNodeEntry();
+                // filter out removed states
+                for (Iterator it = siblings.iterator(); it.hasNext(); ) {
+                    ChildNodeEntry cne = ((LinkedEntries.LinkNode) it.next()).getChildNodeEntry();
+                    if (cne.isAvailable()) {
+                        try {
+                            if (cne.getNodeState().isValid()) {
+                                index--;
+                            } else {
+                                // child node removed
+                            }
+                        } catch (ItemStateException e) {
+                            // should never happen, cne.isAvailable() returned true
+                        }
+                    } else {
+                        // then this child node entry has never been accessed
+                        // before and is assumed valid
+                        index--;
+                    }
+                    if (index == 0) {
+                        return cne;
+                    }
                 }
             } else {
                 // map entry is a single child node entry

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyReference.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyReference.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyReference.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyReference.java Mon Aug 21 07:10:18 2006
@@ -89,4 +89,12 @@
     public PropertyState getPropertyState() throws NoSuchItemStateException, ItemStateException {
         return (PropertyState) resolve();
     }
+
+    /**
+     * @inheritDoc
+     * @see ChildPropertyEntry#isAvailable()
+     */
+    public boolean isAvailable() {
+        return isResolved();
+    }
 }

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/PropertyState.java Mon Aug 21 07:10:18 2006
@@ -147,6 +147,21 @@
     }
 
     /**
+     * @inheritDoc
+     * @see ItemState#collectTransientStates(Set)
+     */
+    public void collectTransientStates(Set transientStates) {
+        switch (status) {
+            case STATUS_EXISTING_MODIFIED:
+            case STATUS_EXISTING_REMOVED:
+            case STATUS_NEW:
+            case STATUS_STALE_DESTROYED:
+            case STATUS_STALE_MODIFIED:
+                transientStates.add(this);
+        }
+    }
+
+    /**
      * {@inheritDoc}
      */
     protected synchronized void copy(ItemState state) {

Modified: jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/SessionItemStateManager.java?rev=433248&r1=433247&r2=433248&view=diff
==============================================================================
--- jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/SessionItemStateManager.java (original)
+++ jackrabbit/trunk/contrib/spi/jcr2spi/src/main/java/org/apache/jackrabbit/jcr2spi/state/SessionItemStateManager.java Mon Aug 21 07:10:18 2006
@@ -18,7 +18,6 @@
 
 import org.apache.commons.collections.iterators.IteratorChain;
 import org.apache.jackrabbit.jcr2spi.HierarchyManager;
-import org.apache.jackrabbit.jcr2spi.ZombieHierarchyManager;
 import org.apache.jackrabbit.jcr2spi.HierarchyManagerImpl;
 import org.apache.jackrabbit.jcr2spi.util.ReferenceChangeTracker;
 import org.apache.jackrabbit.jcr2spi.util.LogUtil;
@@ -75,7 +74,6 @@
 import javax.jcr.nodetype.NoSuchNodeTypeException;
 import javax.jcr.lock.LockException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -346,172 +344,8 @@
         // make sure all entries are removed
         refTracker.clear();
     }
-    //-------------------------------------------< Transient state handling >---
-    /**
-     * Returns an iterator over those transient item state instances that are
-     * direct or indirect descendents of the item state with the given
-     * <code>parentId</code>. The transient item state instance with the given
-     * <code>parentId</code> itself (if there is such) will not be included.
-     * <p/>
-     * The instances are returned in depth-first tree traversal order.
-     *
-     * @param parent the common parent state of the transient item state
-     * instances to be returned.
-     * @return an iterator over descendant transient item state instances
-     */
-    private Iterator getDescendantTransientItemStates(NodeState parent) {
-        if (!transientStateMgr.hasPendingChanges()) {
-            return Collections.EMPTY_LIST.iterator();
-        }
-
-        // build ordered collection of descendant transient states
-        // sorted by decreasing relative depth
-
-        // use an array of lists to group the descendants by relative depth;
-        // the depth is used as array index
-        List[] la = new List[10];
-        try {
-            Iterator iter = transientStateMgr.getModifiedOrAddedItemStates();
-            while (iter.hasNext()) {
-                ItemState state = (ItemState) iter.next();
-                // determine relative depth: > 0 means it's a descendant
-                int depth;
-                try {
-                    depth = hierMgr.getRelativeDepth(parent, state);
-                } catch (ItemNotFoundException infe) {
-                    /**
-                     * one of the parents of the specified item has been
-                     * removed externally; as we don't know its path,
-                     * we can't determine if it is a descendant;
-                     * InvalidItemStateException should only be thrown if
-                     * a descendant is affected;
-                     * => throw InvalidItemStateException for now
-                     * todo FIXME
-                     */
-                    // unable to determine relative depth, assume that the item
-                    // (or any of its ancestors) has been removed externally
-                    String msg = state.getId()
-                            + ": the item seems to have been removed externally.";
-                    log.debug(msg);
-                    throw new InvalidItemStateException(msg);
-                }
-
-                if (depth < 1) {
-                    // not a descendant
-                    continue;
-                }
-
-                // ensure capacity
-                if (depth > la.length) {
-                    List old[] = la;
-                    la = new List[depth + 10];
-                    System.arraycopy(old, 0, la, 0, old.length);
-                }
-
-                List list = la[depth - 1];
-                if (list == null) {
-                    list = new ArrayList();
-                    la[depth - 1] = list;
-                }
-                list.add(state);
-            }
-        } catch (RepositoryException re) {
-            log.warn("inconsistent hierarchy state", re);
-        }
-        // create an iterator over the collected descendants
-        // in decreasing depth order
-        IteratorChain resultIter = new IteratorChain();
-        for (int i = la.length - 1; i >= 0; i--) {
-            List list = la[i];
-            if (list != null) {
-                resultIter.addIterator(list.iterator());
-            }
-        }
-        /**
-         * if the resulting iterator chain is empty return
-         * EMPTY_LIST.iterator() instead because older versions
-         * of IteratorChain (pre Commons Collections 3.1)
-         * would throw UnsupportedOperationException in this
-         * situation
-         */
-        if (resultIter.getIterators().isEmpty()) {
-            return Collections.EMPTY_LIST.iterator();
-        }
-        return resultIter;
-    }
-
-    /**
-     * Same as <code>{@link #getDescendantTransientItemStates(NodeState)}</code>
-     * except that item state instances in the attic are returned.
-     *
-     * @param parent the common parent of the transient item state
-     * instances to be returned.
-     * @return an iterator over descendant transient item state instances in the attic
-     */
-    private Iterator getDescendantTransientItemStatesInAttic(NodeState parent) {
-        if (!transientStateMgr.hasDeletedItemStates()) {
-            return Collections.EMPTY_LIST.iterator();
-        }
 
-        // build ordered collection of descendant transient states in attic
-        // sorted by decreasing relative depth
-
-        // use a special attic-aware hierarchy manager
-        ZombieHierarchyManager zombieHierMgr =
-                new ZombieHierarchyManager(this, transientStateMgr.getAttic(), nsResolver);
-
-        // use an array of lists to group the descendants by relative depth;
-        // the depth is used as array index
-        List[] la = new List[10];
-        try {
-            Iterator iter = transientStateMgr.getDeletedItemStates();
-            while (iter.hasNext()) {
-                ItemState state = (ItemState) iter.next();
-                // determine relative depth: > 0 means it's a descendant
-                int depth = zombieHierMgr.getRelativeDepth(parent, state);
-                if (depth < 1) {
-                    // not a descendant
-                    continue;
-                }
-
-                // ensure capacity
-                if (depth > la.length) {
-                    List old[] = la;
-                    la = new List[depth + 10];
-                    System.arraycopy(old, 0, la, 0, old.length);
-                }
-
-                List list = la[depth - 1];
-                if (list == null) {
-                    list = new ArrayList();
-                    la[depth - 1] = list;
-                }
-                list.add(state);
-            }
-        } catch (RepositoryException re) {
-            log.warn("inconsistent hierarchy state", re);
-        }
-        // create an iterator over the collected descendants
-        // in decreasing depth order
-        IteratorChain resultIter = new IteratorChain();
-        for (int i = la.length - 1; i >= 0; i--) {
-            List list = la[i];
-            if (list != null) {
-                resultIter.addIterator(list.iterator());
-            }
-        }
-        /**
-         * if the resulting iterator chain is empty return
-         * EMPTY_LIST.iterator() instead because older versions
-         * of IteratorChain (pre Commons Collections 3.1)
-         * would throw UnsupportedOperationException in this
-         * situation
-         */
-        if (resultIter.getIterators().isEmpty()) {
-            return Collections.EMPTY_LIST.iterator();
-        }
-        return resultIter;
-    }
+    //-------------------------------------------< Transient state handling >---
 
     /**
      *
@@ -521,173 +355,113 @@
      * @throws ItemStateException
      */
     private ChangeLog getChangeLog(ItemState itemState) throws StaleItemStateException, ItemStateException {
-
         ChangeLog changeLog = new ChangeLog();
-        if (itemState.getParent() == null) {
-            // root state -> get all item states
-            for (Iterator it = transientStateMgr.addedStates(); it.hasNext(); ) {
-                changeLog.added((ItemState) it.next());
-            }
-            for (Iterator it = transientStateMgr.modifiedStates(); it.hasNext(); ) {
-                changeLog.modified((ItemState) it.next());
-            }
-            for (Iterator it = transientStateMgr.deletedStates(); it.hasNext(); ) {
-                changeLog.deleted((ItemState) it.next());
-            }
-            for (Iterator it = transientStateMgr.getOperations(); it.hasNext(); ) {
-                changeLog.addOperation((Operation) it.next());
-            }
-        } else {
-            // build changelog for affected and decendant states only
-            collectTransientStates(itemState, changeLog);
-            collectRemovedStates(itemState, changeLog);
-
-            /**
-             * build set of item id's which are within the scope of
-             * (i.e. affected by) this save operation
-             */
-            Iterator it = new IteratorChain(changeLog.modifiedStates(), changeLog.deletedStates());
-            Set affectedStates = new HashSet();
-            while (it.hasNext()) {
-                affectedStates.add(it.next());
-            }
 
-            checkIsSelfContained(affectedStates, changeLog);
-            collectOperations(affectedStates, changeLog);
-        }
-        return changeLog;
-    }
+        // build changelog for affected and decendant states only
+        collectTransientStates(itemState, changeLog);
 
-    /**
-     * DIFF JACKRABBIT: copied and adapted from ItemImpl.getRemovedStates()
-     * <p/>
-     * Builds a list of transient descendant item states in the attic
-     * (i.e. those marked as 'removed') that are within the scope of
-     * <code>root</code>.
-     *
-     * @throws StaleItemStateException
-     */
-    private void collectRemovedStates(ItemState root, ChangeLog changeLog)
-        throws StaleItemStateException {
-        ItemState transientState;
-        if (root.isNode()) {
-            Iterator iter = getDescendantTransientItemStatesInAttic((NodeState)root);
-            while (iter.hasNext()) {
-                transientState = (ItemState) iter.next();
-                // check if stale
-                if (transientState.getStatus() == ItemState.STATUS_STALE_MODIFIED) {
-                    String msg = transientState.getId()
-                        + ": the item cannot be removed because it has been modified externally.";
-                    log.debug(msg);
-                    throw new StaleItemStateException(msg);
-                }
-                if (transientState.getStatus() == ItemState.STATUS_STALE_DESTROYED) {
-                    String msg = transientState.getId()
-                        + ": the item cannot be removed because it has already been deleted externally.";
-                    log.debug(msg);
-                    throw new StaleItemStateException(msg);
-                }
-                changeLog.deleted(transientState);
-            }
+        /**
+         * build set of item id's which are within the scope of
+         * (i.e. affected by) this save operation
+         */
+        Iterator it = new IteratorChain(changeLog.modifiedStates(), changeLog.deletedStates());
+        Set affectedStates = new HashSet();
+        while (it.hasNext()) {
+            affectedStates.add(it.next());
         }
+
+        checkIsSelfContained(affectedStates, changeLog);
+        collectOperations(affectedStates, changeLog);
+
+        return changeLog;
     }
 
     /**
      * DIFF JACKRABBIT: copied and adapted from ItemImpl.getTransientStates()
      * <p/>
-     * Builds a list of transient (i.e. new or modified) item states that are
-     * within the scope of <code>state</code>.
+     * Builds a <code>ChangeLog</code> of transient (i.e. new, modified or
+     * deleted) item states that are within the scope of <code>state</code>.
      *
-     * @throws StaleItemStateException
-     * @throws ItemStateException
+     * @throws StaleItemStateException if a stale <code>ItemState</code> is
+     *                                 encountered while traversing the state
+     *                                 hierarchy. The <code>changeLog</code>
+     *                                 might have been populated with some
+     *                                 transient item states. A client should
+     *                                 therefore not reuse the <code>changeLog</code>
+     *                                 if such an exception is thrown.
+     * @throws ItemStateException if <code>state</code> is a new item state.
      */
     private void collectTransientStates(ItemState state, ChangeLog changeLog)
             throws StaleItemStateException, ItemStateException {
-        // list of transient states that should be persisted
-        ItemState transientState;
-
         // fail-fast test: check status of this item's state
         if (state.isTransient()) {
             switch (state.getStatus()) {
-                case ItemState.STATUS_EXISTING_MODIFIED:
-                    // add this item's state to the list
-                    changeLog.modified(state);
-                    break;
-
                 case ItemState.STATUS_NEW:
                     {
                         String msg = LogUtil.safeGetJCRPath(state, nsResolver, hierMgr) + ": cannot save a new item.";
                         log.debug(msg);
                         throw new ItemStateException(msg);
                     }
-
                 case ItemState.STATUS_STALE_MODIFIED:
                     {
                         String msg = LogUtil.safeGetJCRPath(state, nsResolver, hierMgr) + ": the item cannot be saved because it has been modified externally.";
                         log.debug(msg);
                         throw new StaleItemStateException(msg);
                     }
-
                 case ItemState.STATUS_STALE_DESTROYED:
                     {
                         String msg = LogUtil.safeGetJCRPath(state, nsResolver, hierMgr) + ": the item cannot be saved because it has been deleted externally.";
                         log.debug(msg);
                         throw new StaleItemStateException(msg);
                     }
-
                 case ItemState.STATUS_UNDEFINED:
                     {
                         String msg = LogUtil.safeGetJCRPath(state, nsResolver, hierMgr) + ": the item cannot be saved; it seems to have been removed externally.";
                         log.debug(msg);
                         throw new StaleItemStateException(msg);
                     }
+            }
+        }
+
+        // Set of transient states that should be persisted
+        Set transientStates = new HashSet();
+        state.collectTransientStates(transientStates);
 
+        for (Iterator it = transientStates.iterator(); it.hasNext();) {
+            ItemState transientState = (ItemState) it.next();
+            // fail-fast test: check status of transient state
+            switch (transientState.getStatus()) {
+                case ItemState.STATUS_NEW:
+                    changeLog.added(transientState);
+                    break;
+                case ItemState.STATUS_EXISTING_MODIFIED:
+                    changeLog.modified(transientState);
+                    break;
+                case ItemState.STATUS_EXISTING_REMOVED:
+                    changeLog.deleted(transientState);
+                    break;
+                case ItemState.STATUS_STALE_MODIFIED:
+                    {
+                        String msg = transientState.getId() + ": the item cannot be saved because it has been modified externally.";
+                        log.debug(msg);
+                        throw new StaleItemStateException(msg);
+                    }
+                case ItemState.STATUS_STALE_DESTROYED:
+                    {
+                        String msg = transientState.getId() + ": the item cannot be saved because it has been deleted externally.";
+                        log.debug(msg);
+                        throw new StaleItemStateException(msg);
+                    }
+                case ItemState.STATUS_UNDEFINED:
+                    {
+                        String msg = transientState.getId() + ": the item cannot be saved; it seems to have been removed externally.";
+                        log.debug(msg);
+                        throw new StaleItemStateException(msg);
+                    }
                 default:
-                    log.debug("unexpected state status (" + state.getStatus() + ")");
+                    log.debug("unexpected state status (" + transientState.getStatus() + ")");
                     // ignore
                     break;
-            }
-        }
-
-        if (state.isNode()) {
-            // build list of 'new' or 'modified' descendants
-            Iterator iter = getDescendantTransientItemStates((NodeState) state);
-            while (iter.hasNext()) {
-                transientState = (ItemState) iter.next();
-                // fail-fast test: check status of transient state
-                switch (transientState.getStatus()) {
-                    case ItemState.STATUS_NEW:
-                    case ItemState.STATUS_EXISTING_MODIFIED:
-                        // add modified state to the list
-                        changeLog.modified(transientState);
-                        break;
-
-                    case ItemState.STATUS_STALE_MODIFIED:
-                        {
-                            String msg = transientState.getId() + ": the item cannot be saved because it has been modified externally.";
-                            log.debug(msg);
-                            throw new StaleItemStateException(msg);
-                        }
-
-                    case ItemState.STATUS_STALE_DESTROYED:
-                        {
-                            String msg = transientState.getId() + ": the item cannot be saved because it has been deleted externally.";
-                            log.debug(msg);
-                            throw new StaleItemStateException(msg);
-                        }
-
-                    case ItemState.STATUS_UNDEFINED:
-                        {
-                            String msg = transientState.getId() + ": the item cannot be saved; it seems to have been removed externally.";
-                            log.debug(msg);
-                            throw new StaleItemStateException(msg);
-                        }
-
-                    default:
-                        log.debug("unexpected state status (" + transientState.getStatus() + ")");
-                        // ignore
-                        break;
-                }
             }
         }
     }