jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: rev 55004 - in incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core: . observation
Date Mon, 18 Oct 2004 13:12:33 GMT
Author: stefan
Date: Mon Oct 18 06:12:33 2004
New Revision: 55004

Modified:
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java
   incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/observation/EventStateCollection.java
Log:
fixing http://issues.apache.org/jira/browse/JCR-10
(Item#save() did under special circumstances trigger
transient modifications that were not persisted in 
the same call)

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java	(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/ItemImpl.java	Mon Oct 18
06:12:33 2004
@@ -319,6 +319,127 @@
         return session.getHierarchyManager().getPath(id);
     }
 
+    /**
+     * Builds a list of transient (i.e. new or modified) item states that are
+     * within the scope of <code>this.{@link #save()}</code>.
+     *
+     * @return list of transient item states
+     * @throws InvalidItemStateException
+     * @throws RepositoryException
+     */
+    private Collection getTransientStates()
+            throws InvalidItemStateException, RepositoryException {
+        // list of transient states that should be persisted
+        ArrayList dirty = new ArrayList();
+        ItemState transientState;
+
+        // check status of this item's state
+        if (isTransient()) {
+            switch (state.getStatus()) {
+                case ItemState.STATUS_EXISTING_MODIFIED:
+                    // add this item's state to the list
+                    dirty.add(state);
+                    break;
+
+                case ItemState.STATUS_NEW:
+                    {
+                        String msg = safeGetJCRPath() + ": cannot save a new item.";
+                        log.error(msg);
+                        throw new RepositoryException(msg);
+                    }
+
+                case ItemState.STATUS_STALE_MODIFIED:
+                    {
+                        String msg = safeGetJCRPath() + ": the item cannot be saved because
it has been modified externally.";
+                        log.error(msg);
+                        throw new InvalidItemStateException(msg);
+                    }
+
+                case ItemState.STATUS_STALE_DESTROYED:
+                    {
+                        String msg = safeGetJCRPath() + ": the item cannot be saved because
it has been deleted externally.";
+                        log.error(msg);
+                        throw new InvalidItemStateException(msg);
+                    }
+
+                default:
+                    log.debug("unexpected state status (" + state.getStatus() + ")");
+                    // ignore
+                    break;
+            }
+        }
+
+        if (isNode()) {
+            // build list of 'new' or 'modified' descendents
+            Iterator iter = itemStateMgr.getDescendantTransientItemStates(id);
+            while (iter.hasNext()) {
+                transientState = (ItemState) iter.next();
+                switch (transientState.getStatus()) {
+                    case ItemState.STATUS_NEW:
+                    case ItemState.STATUS_EXISTING_MODIFIED:
+                        // add modified state to the list
+                        dirty.add(transientState);
+                        break;
+
+                    case ItemState.STATUS_STALE_MODIFIED:
+                        {
+                            String msg = transientState.getId() + ": the item cannot be saved
because it has been modified externally.";
+                            log.error(msg);
+                            throw new InvalidItemStateException(msg);
+                        }
+
+                    case ItemState.STATUS_STALE_DESTROYED:
+                        {
+                            String msg = transientState.getId() + ": the item cannot be saved
because it has been deleted externally.";
+                            log.error(msg);
+                            throw new InvalidItemStateException(msg);
+                        }
+
+                    default:
+                        log.debug("unexpected state status (" + transientState.getStatus()
+ ")");
+                        // ignore
+                        break;
+                }
+            }
+        }
+        return dirty;
+    }
+
+    /**
+     * Builds a list of transient descendant item states in the attic
+     * (i.e. those marked as 'removed') that are within the scope of
+     * <code>this.{@link #save()}</code>.
+     *
+     * @return list of transient item states
+     * @throws InvalidItemStateException
+     * @throws RepositoryException
+     */
+    private Collection getRemovedStates()
+            throws InvalidItemStateException, RepositoryException {
+        ArrayList removed = new ArrayList();
+        ItemState transientState;
+
+        if (isNode()) {
+            Iterator iter = itemStateMgr.getDescendantTransientItemStatesInAttic(id);
+            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.error(msg);
+                    throw new InvalidItemStateException(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.error(msg);
+                    throw new InvalidItemStateException(msg);
+                }
+                removed.add(transientState);
+            }
+        }
+        return removed;
+    }
+
     private void validateTransientItems(Iterator iter)
             throws AccessDeniedException, ConstraintViolationException, RepositoryException
{
         /**
@@ -646,7 +767,7 @@
      * @throws RepositoryException
      */
     private void initVersionHistories(Iterator iter) throws RepositoryException {
-        // todo centralize version history creation code (currently in NodeImpl.addMixin
& ItemImpl.initVersionHistories
+        // todo consolidate version history creation code (currently in NodeImpl.addMixin
& ItemImpl.initVersionHistories
         // walk through list of transient items and
         // search for new versionable nodes
         while (iter.hasNext()) {
@@ -841,98 +962,20 @@
         // check state of this instance
         checkItemState();
 
-        // list of transient states that should be persisted
-        ArrayList dirty = new ArrayList();
-        ItemState transientState;
-
-        // list of events that are generated by saved changes
-        WorkspaceImpl wsp = (WorkspaceImpl) session.getWorkspace();
-        ObservationManagerFactory obsFactory = rep.getObservationManagerFactory(wsp.getName());
-        EventStateCollection events = obsFactory.createEventStateCollection(session,
-                session.getItemStateManager(), session.getHierarchyManager());
-
-        // check status of this item's state
-        if (isTransient()) {
-            switch (state.getStatus()) {
-                case ItemState.STATUS_EXISTING_MODIFIED:
-                    // add this item's state to the list
-                    dirty.add(state);
-                    // create event
-                    events.createEventStates(state);
-                    break;
-
-                case ItemState.STATUS_NEW:
-                    {
-                        String msg = safeGetJCRPath() + ": cannot save a new item.";
-                        log.error(msg);
-                        throw new RepositoryException(msg);
-                    }
-
-                case ItemState.STATUS_STALE_MODIFIED:
-                    {
-                        String msg = safeGetJCRPath() + ": the item cannot be saved because
it has been modified externally.";
-                        log.error(msg);
-                        throw new InvalidItemStateException(msg);
-                    }
-
-                case ItemState.STATUS_STALE_DESTROYED:
-                    {
-                        String msg = safeGetJCRPath() + ": the item cannot be saved because
it has been deleted externally.";
-                        log.error(msg);
-                        throw new InvalidItemStateException(msg);
-                    }
-
-                default:
-                    log.debug("unexpected state status (" + state.getStatus() + ")");
-                    // ignore
-                    break;
-            }
-        }
-
-        if (isNode()) {
-            // build list of 'new' or 'modified' descendents
-            Iterator iter = itemStateMgr.getDescendantTransientItemStates(id);
-            while (iter.hasNext()) {
-                transientState = (ItemState) iter.next();
-                switch (transientState.getStatus()) {
-                    case ItemState.STATUS_NEW:
-                    case ItemState.STATUS_EXISTING_MODIFIED:
-                        // add modified state to the list
-                        dirty.add(transientState);
-                        // create events
-                        events.createEventStates(transientState);
-                        break;
-
-                    case ItemState.STATUS_STALE_MODIFIED:
-                        {
-                            String msg = transientState.getId() + ": the item cannot be saved
because it has been modified externally.";
-                            log.error(msg);
-                            throw new InvalidItemStateException(msg);
-                        }
-
-                    case ItemState.STATUS_STALE_DESTROYED:
-                        {
-                            String msg = transientState.getId() + ": the item cannot be saved
because it has been deleted externally.";
-                            log.error(msg);
-                            throw new InvalidItemStateException(msg);
-                        }
-
-                    default:
-                        log.debug("unexpected state status (" + transientState.getStatus()
+ ")");
-                        // ignore
-                        break;
-                }
-            }
-        }
-
+        // build list of transient states that should be persisted
+        Collection dirty = getTransientStates();
         if (dirty.size() == 0) {
             // no transient items, nothing to do here
             return;
         }
 
-        // 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)
+        ItemState transientState;
+
+        /**
+         * 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)
+         */
         Iterator iter = dirty.iterator();
         while (iter.hasNext()) {
             transientState = (ItemState) iter.next();
@@ -966,45 +1009,48 @@
             }
         }
 
-        // validate access and node type constraints
-        // (this will also validate child removals)
+        /**
+         * validate access and node type constraints
+         * (this will also validate child removals)
+         */
         validateTransientItems(dirty.iterator());
 
-        // we need to make sure that we are not interrupted while
-        // verifying/persisting node references
+        WorkspaceImpl wsp = (WorkspaceImpl) session.getWorkspace();
+
+        // list of events that are generated by saved changes
+        ObservationManagerFactory obsFactory = rep.getObservationManagerFactory(wsp.getName());
+        EventStateCollection events = obsFactory.createEventStateCollection(session,
+                session.getItemStateManager(), session.getHierarchyManager());
+
+        /**
+         * we need to make sure that we are not interrupted while
+         * verifying/persisting node references
+         */
         ReferenceManager refMgr = wsp.getReferenceManager();
         synchronized (refMgr) {
 
-            // build list of transient descendents in the attic
-            // (i.e. those marked as 'removed')
-            ArrayList removed = new ArrayList();
-            if (isNode()) {
-                iter = itemStateMgr.getDescendantTransientItemStatesInAttic(id);
-                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.error(msg);
-                        throw new InvalidItemStateException(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.error(msg);
-                        throw new InvalidItemStateException(msg);
-                    }
-                    removed.add(transientState);
-                    events.createEventStates(transientState);
-                }
-            }
-
-            // referential integrity checks:
-            // make sure that a referenced node cannot be removed and
-            // that all references are updated and persisted
+            /**
+             * build list of transient descendents in the attic
+             * (i.e. those marked as 'removed')
+             */
+            Collection removed = getRemovedStates();
+
+            /**
+             * referential integrity checks:
+             * make sure that a referenced node cannot be removed and
+             * that all references are updated and persisted
+             */
             checkReferences(dirty.iterator(), removed.iterator(), refMgr);
 
-            // prepare for event dispatch
-            // this will check access rights on items that will be removed
+            /**
+             * create event states for the affected item states and
+             * prepare them for event dispatch (this step is necessary in order
+             * to check access rights on items that will be removed)
+             *
+             * todo consolidate event generating and dispatching code (ideally one method
call after save has succeeded)
+             */
+            events.createEventStates(dirty);
+            events.createEventStates(removed);
             events.prepare();
 
             // definitively remove transient items marked as 'removed'
@@ -1014,14 +1060,19 @@
             iter = removed.iterator();
             while (iter.hasNext()) {
                 transientState = (ItemState) iter.next();
-                // dispose the transient state, it is no longer used
-                // this will indirectly (through stateDiscarded listener method)
-                // permanently invalidate the wrapping Item instance
+                /**
+                 * dispose the transient state, it is no longer used
+                 * this will indirectly (through stateDiscarded listener method)
+                 * permanently invalidate the wrapping Item instance
+                 */
                 itemStateMgr.disposeTransientItemStateInAttic(transientState);
             }
 
-            // initialize version histories for new nodes
+            // initialize version histories for new nodes (might create new transient state)
             initVersionHistories(dirty.iterator());
+
+            // re-build the list of transient states (might have changed by now)
+            dirty = getTransientStates();
 
             // persist 'new' or 'modified' transient states
             persistTransientItems(dirty.iterator());

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java	(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/NodeImpl.java	Mon Oct 18
06:12:33 2004
@@ -17,10 +17,10 @@
 
 import org.apache.jackrabbit.core.nodetype.*;
 import org.apache.jackrabbit.core.state.*;
-import org.apache.jackrabbit.core.version.*;
-import org.apache.jackrabbit.core.util.uuid.UUID;
 import org.apache.jackrabbit.core.util.ChildrenCollector;
 import org.apache.jackrabbit.core.util.IteratorHelper;
+import org.apache.jackrabbit.core.util.uuid.UUID;
+import org.apache.jackrabbit.core.version.*;
 import org.apache.log4j.Logger;
 
 import javax.jcr.*;
@@ -109,7 +109,7 @@
                 genValues = new InternalValue[]{InternalValue.create(((NodeState) state).getUUID())};
             }
 /*
-	todo centralize version history creation code (currently in NodeImpl.addMixin & ItemImpl.initVersionHistories
+	todo consolidate version history creation code (currently in NodeImpl.addMixin & ItemImpl.initVersionHistories
 	} else if (nt.getQName().equals(NodeTypeRegistry.MIX_VERSIONABLE)) {
 	    // mix:versionable node type
 	    VersionHistory hist = rep.getVersionManager().getOrCreateVersionHistory(this);
@@ -1681,7 +1681,7 @@
 
         // build effective node type of mixin's & primary type in order to detect conflicts
         NodeTypeRegistry ntReg = ntMgr.getNodeTypeRegistry();
-        EffectiveNodeType entExisting;
+        EffectiveNodeType entExisting, entNew;
         try {
             // existing mixin's
             HashSet set = new HashSet(((NodeState) state).getMixinTypeNames());
@@ -1695,7 +1695,7 @@
             // add new mixin
             set.add(ntName);
             // try to build new effective node type (will throw in case of conflicts)
-            ntReg.buildEffectiveNodeType((QName[]) set.toArray(new QName[set.size()]));
+            entNew = ntReg.buildEffectiveNodeType((QName[]) set.toArray(new QName[set.size()]));
         } catch (NodeTypeConflictException ntce) {
             throw new ConstraintViolationException(ntce.getMessage());
         }
@@ -1738,8 +1738,10 @@
             }
 
             // check for special node types
-            // todo centralize version history creation code (currently in NodeImpl.addMixin
& ItemImpl.initVersionHistories
-            if (ntName.equals(NodeTypeRegistry.MIX_VERSIONABLE)) {
+            // todo consolidate version history creation code (currently in NodeImpl.addMixin
& ItemImpl.initVersionHistories
+            if (!entExisting.includesNodeType(NodeTypeRegistry.MIX_VERSIONABLE) &&
+                    entNew.includesNodeType(NodeTypeRegistry.MIX_VERSIONABLE)) {
+                // node has become 'versionable', initialize version history
                 VersionHistory hist = rep.getVersionManager().createVersionHistory(this);
                 internalSetProperty(VersionManager.PROPNAME_VERSION_HISTORY, InternalValue.create(new
UUID(hist.getUUID())));
                 internalSetProperty(VersionManager.PROPNAME_BASE_VERSION, InternalValue.create(new
UUID(hist.getRootVersion().getUUID())));
@@ -2668,7 +2670,7 @@
         // check uuid
         if (isNodeType(NodeTypeRegistry.MIX_REFERENCEABLE)) {
             String uuid = freeze.getFrozenUUID();
-            if (uuid!=null && !uuid.equals(getUUID())) {
+            if (uuid != null && !uuid.equals(getUUID())) {
                 throw new ItemExistsException("Unable to restore version of " + safeGetJCRPath()
+ ". UUID changed.");
             }
         }

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/observation/EventStateCollection.java
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/observation/EventStateCollection.java
(original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/observation/EventStateCollection.java
Mon Oct 18 06:12:33 2004
@@ -24,6 +24,7 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Collection;
 
 /**
  * The <code>EventStateCollection</code> class implements how {@link EventState}
@@ -75,6 +76,22 @@
         this.session = session;
         this.provider = provider;
         this.hmgr = hmgr;
+    }
+
+    /**
+     * Creates {@link EventState}s for the {@link org.apache.jackrabbit.core.state.ItemState
state}
+     * instances contained in the specified collection.
+     *
+     * @param states collection of transient <code>ItemState</code> for whom
+     *               to create {@link EventState}s.
+     * @see #createEventStates(org.apache.jackrabbit.core.state.ItemState)
+     */
+    public void createEventStates(Collection states)
+            throws RepositoryException {
+        Iterator iter = states.iterator();
+        while (iter.hasNext()) {
+            createEventStates((ItemState) iter.next());
+        }
     }
 
     /**

Mime
View raw message