jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: r884602 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/ItemImpl.java main/java/org/apache/jackrabbit/core/NodeImpl.java test/java/org/apache/jackrabbit/core/NodeImplTest.java
Date Thu, 26 Nov 2009 15:45:32 GMT
Author: stefan
Date: Thu Nov 26 15:45:31 2009
New Revision: 884602

URL: http://svn.apache.org/viewvc?rev=884602&view=rev
Log:
reverting commits r884535 & r884562

there are still isssues/failing test cases (regression of JCR-2170)

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java?rev=884602&r1=884601&r2=884602&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
Thu Nov 26 15:45:31 2009
@@ -606,18 +606,10 @@
         // walk through list of removed transient items and check REMOVE permission
         for (ItemState itemState : removed) {
             QItemDefinition def;
-            try {
-                if (itemState.isNode()) {
-                    def = itemMgr.getDefinition((NodeState) itemState).unwrap();
-                } else {
-                    def = itemMgr.getDefinition((PropertyState) itemState).unwrap();
-                }
-            } catch (ConstraintViolationException e) {
-                // since identifier of assigned definition is not stored anymore
-                // with item state (see JCR-2170), correct definition cannot be
-                // determined for items which have been removed due to removal
-                // of a mixin (see also JCR-2130 & JCR-2408)
-                continue;
+            if (itemState.isNode()) {
+                def = itemMgr.getDefinition((NodeState) itemState).unwrap();
+            } else {
+                def = itemMgr.getDefinition((PropertyState) itemState).unwrap();
             }
             if (!def.isProtected()) {
                 Path path = stateMgr.getAtticAwareHierarchyMgr().getPath(itemState.getId());

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=884602&r1=884601&r2=884602&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
Thu Nov 26 15:45:31 2009
@@ -28,8 +28,6 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
-import java.util.Map;
-import java.util.HashMap;
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.Binary;
@@ -86,7 +84,6 @@
 import org.apache.jackrabbit.core.state.NodeReferences;
 import org.apache.jackrabbit.core.state.NodeState;
 import org.apache.jackrabbit.core.state.PropertyState;
-import org.apache.jackrabbit.core.state.NoSuchItemStateException;
 import org.apache.jackrabbit.core.value.InternalValue;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.spi.Path;
@@ -1019,143 +1016,127 @@
             throw new ConstraintViolationException(mixinName + " can not be removed: the
node is locked.");
         }
 
-        NodeState thisState = (NodeState) getOrCreateTransientItemState();
-
-        // collect information about properties and nodes which require
-        // further action as a result of the mixin removal;
-        // we need to do this *before* actually changing the assigned the mixin types,
-        // otherwise we wouldn't be able to retrieve the current definition
-        // of an item.
-        Map<PropertyId, PropertyDefinition> affectedProps = new HashMap<PropertyId,
PropertyDefinition>();
-        Map<ChildNodeEntry, NodeDefinition> affectedNodes = new HashMap<ChildNodeEntry,
NodeDefinition>();
-        try {
-            Set<Name> names = thisState.getPropertyNames();
-            for (Name propName : names) {
-                PropertyId propId = new PropertyId(thisState.getNodeId(), propName);
-                PropertyState propState = (PropertyState) stateMgr.getItemState(propId);
-                PropertyDefinition oldDef = itemMgr.getDefinition(propState);
-                // check if property has been defined by mixin type (or one of its supertypes)
-                NodeTypeImpl declaringNT = (NodeTypeImpl) oldDef.getDeclaringNodeType();
-                if (!entResulting.includesNodeType(declaringNT.getQName())) {
-                    // the resulting effective node type doesn't include the
-                    // node type that declared this property
-                    affectedProps.put(propId, oldDef);
-                }
-            }
-
-            List<ChildNodeEntry> entries = thisState.getChildNodeEntries();
-            for (ChildNodeEntry entry : entries) {
-                NodeState nodeState = (NodeState) stateMgr.getItemState(entry.getId());
-                NodeDefinition oldDef = itemMgr.getDefinition(nodeState);
-                // check if node has been defined by mixin type (or one of its supertypes)
-                NodeTypeImpl declaringNT = (NodeTypeImpl) oldDef.getDeclaringNodeType();
-                if (!entResulting.includesNodeType(declaringNT.getQName())) {
-                    // the resulting effective node type doesn't include the
-                    // node type that declared this child node
-                    affectedNodes.put(entry, oldDef);
-                }
-            }
-        } catch (ItemStateException e) {
-            throw new RepositoryException("Internal Error: Failed to determine effect of
removing mixin " + session.getJCRName(mixinName), e);
-        }
 
         // modify the state of this node
+        NodeState thisState = (NodeState) getOrCreateTransientItemState();
         thisState.setMixinTypeNames(remainingMixins);
+
         // set jcr:mixinTypes property
         setMixinTypesProperty(remainingMixins);
 
-        // process affected nodes & properties:
-        // 1. try to redefine item based on the resulting
-        //    new effective node type (see JCR-2130)
-        // 2. remove item if 1. fails
+        // shortcut
+        if (mixin.getChildNodeDefinitions().length == 0
+                && mixin.getPropertyDefinitions().length == 0) {
+            // the node type has neither property nor child node definitions,
+            // i.e. we're done
+            return;
+        }
+
+        // walk through properties and child nodes and remove those that aren't
+        // accomodated by the resulting new effective node type (see JCR-2130)
         boolean success = false;
         try {
-            for (PropertyId id : affectedProps.keySet()) {
-                PropertyImpl prop = (PropertyImpl) itemMgr.getItem(id);
-                PropertyDefinition oldDef = affectedProps.get(id);
-
-                if (oldDef.isProtected()) {
-                    // remove 'orphaned' protected properties immediately
-                    removeChildProperty(id.getName());
-                    continue;
-                }
-                // try to find new applicable definition first and
-                // redefine property if possible (JCR-2130)
-                try {
-                    PropertyDefinitionImpl newDef = getApplicablePropertyDefinition(
-                            id.getName(), prop.getType(),
-                            oldDef.isMultiple(), false);
-                    if (newDef.getRequiredType() != PropertyType.UNDEFINED
-                            && newDef.getRequiredType() != prop.getType()) {
-                        // value conversion required
-                        if (oldDef.isMultiple()) {
-                            // convert value
-                            Value[] values =
-                                    ValueHelper.convert(
-                                            prop.getValues(),
-                                            newDef.getRequiredType(),
-                                            session.getValueFactory());
-                            // redefine property
-                            prop.onRedefine(newDef.unwrap());
-                            // set converted values
-                            prop.setValue(values);
+            // use temp set to avoid ConcurrentModificationException
+            HashSet<Name> set = new HashSet<Name>(thisState.getPropertyNames());
+            for (Name propName : set) {
+                PropertyState propState = (PropertyState) stateMgr.getItemState(new PropertyId(thisState.getNodeId(),
propName));
+                // check if property has been defined by mixin type (or one of its supertypes)
+                PropertyDefinition def = itemMgr.getDefinition(propState);
+                NodeTypeImpl declaringNT = (NodeTypeImpl) def.getDeclaringNodeType();
+                if (!entResulting.includesNodeType(declaringNT.getQName())) {
+                    // the resulting effective node type doesn't include the
+                    // node type that declared this property
+
+                    // try to find new applicable definition first and
+                    // redefine property if possible (JCR-2130)
+                    try {
+                        PropertyImpl prop = (PropertyImpl) itemMgr.getItem(propState.getId());
+                        if (prop.getDefinition().isProtected()) {
+                            // remove 'orphaned' protected properties immediately
+                            removeChildProperty(propName);
+                            continue;
+                        }
+                        PropertyDefinitionImpl pdi = getApplicablePropertyDefinition(
+                                propName, propState.getType(),
+                                propState.isMultiValued(), false);
+                        if (pdi.getRequiredType() != PropertyType.UNDEFINED
+                                && pdi.getRequiredType() != propState.getType())
{
+                            // value conversion required
+                            if (propState.isMultiValued()) {
+                                // convert value
+                                Value[] values =
+                                        ValueHelper.convert(
+                                                prop.getValues(),
+                                                pdi.getRequiredType(),
+                                                session.getValueFactory());
+                                // redefine property
+                                prop.onRedefine(pdi.unwrap());
+                                // set converted values
+                                prop.setValue(values);
+                            } else {
+                                // convert value
+                                Value value =
+                                        ValueHelper.convert(
+                                                prop.getValue(),
+                                                pdi.getRequiredType(),
+                                                session.getValueFactory());
+                                // redefine property
+                                prop.onRedefine(pdi.unwrap());
+                                // set converted values
+                                prop.setValue(value);
+                            }
                         } else {
-                            // convert value
-                            Value value =
-                                    ValueHelper.convert(
-                                            prop.getValue(),
-                                            newDef.getRequiredType(),
-                                            session.getValueFactory());
                             // redefine property
-                            prop.onRedefine(newDef.unwrap());
-                            // set converted values
-                            prop.setValue(value);
+                            prop.onRedefine(pdi.unwrap());
                         }
-                    } else {
-                        // redefine property
-                        prop.onRedefine(newDef.unwrap());
+                    } catch (ValueFormatException vfe) {
+                        // value conversion failed, remove it
+                        removeChildProperty(propName);
+                    } catch (ConstraintViolationException cve) {
+                        // no suitable definition found for this property,
+                        // remove it
+                        removeChildProperty(propName);
                     }
-                } catch (ValueFormatException vfe) {
-                    // value conversion failed, remove it
-                    removeChildProperty(id.getName());
-                } catch (ConstraintViolationException cve) {
-                    // no suitable definition found for this property,
-                    // remove it
-                    removeChildProperty(id.getName());
                 }
             }
-
-            for (ChildNodeEntry entry : affectedNodes.keySet()) {
+            // use temp array to avoid ConcurrentModificationException
+            ArrayList<ChildNodeEntry> list = new ArrayList<ChildNodeEntry>(thisState.getChildNodeEntries());
+            // start from tail to avoid problems with same-name siblings
+            for (int i = list.size() - 1; i >= 0; i--) {
+                ChildNodeEntry entry = list.get(i);
                 NodeState nodeState = (NodeState) stateMgr.getItemState(entry.getId());
-                NodeImpl node = (NodeImpl) itemMgr.getItem(entry.getId());
-                NodeDefinition oldDef = affectedNodes.get(entry);
-
-                if (oldDef.isProtected()) {
-                    // remove 'orphaned' protected child node immediately
-                    removeChildNode(entry.getName(), entry.getIndex());
-                    continue;
-                }
+                NodeDefinition def = itemMgr.getDefinition(nodeState);
+                // check if node has been defined by mixin type (or one of its supertypes)
+                NodeTypeImpl declaringNT = (NodeTypeImpl) def.getDeclaringNodeType();
+                if (!entResulting.includesNodeType(declaringNT.getQName())) {
+                    // the resulting effective node type doesn't include the
+                    // node type that declared this child node
 
-                // try to find new applicable definition first and
-                // redefine node if possible (JCR-2130)
-                try {
-                    NodeDefinitionImpl newDef = getApplicableChildNodeDefinition(
-                            entry.getName(),
-                            nodeState.getNodeTypeName());
-                    // redefine node
-                    node.onRedefine(newDef.unwrap());
-                } catch (ConstraintViolationException cve) {
-                    // no suitable definition found for this child node,
-                    // remove it
-                    removeChildNode(entry.getName(), entry.getIndex());
+                    try {
+                        NodeImpl node = (NodeImpl) itemMgr.getItem(nodeState.getId());
+                        if (node.getDefinition().isProtected()) {
+                            // remove 'orphaned' protected child node immediately
+                            removeChildNode(entry.getName(), entry.getIndex());
+                            continue;
+                        }
+                        NodeDefinitionImpl ndi = getApplicableChildNodeDefinition(
+                                entry.getName(),
+                                nodeState.getNodeTypeName());
+                        // redefine node
+                        node.onRedefine(ndi.unwrap());
+                    } catch (ConstraintViolationException cve) {
+                        // no suitable definition found for this child node,
+                        // remove it
+                        removeChildNode(entry.getName(), entry.getIndex());
+                    }
                 }
             }
             success = true;
         } catch (ItemStateException e) {
-            throw new RepositoryException("Failed to clean up child items defined by removed
mixin " + session.getJCRName(mixinName), e);
+            throw new RepositoryException("Failed to clean up child items defined by removed
mixin " + session.getJCRName(mixinName));
         } finally {
             if (!success) {
-                // TODO JCR-1914: revert any changes made so far
+                // TODO JCR-1914: revert changes made to jcr:mixinTypes
             }
         }
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java?rev=884602&r1=884601&r2=884602&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
Thu Nov 26 15:45:31 2009
@@ -200,36 +200,4 @@
         assertEquals(PropertyType.nameFromValue(PropertyType.LONG),
                 PropertyType.nameFromValue(p.getType()));
     }
-
-    /**
-     * Test case for JCR-2130 and JCR-2408.
-     *
-     * @throws RepositoryException
-     */
-    public void testAddRemoveMixin() throws RepositoryException {
-        Node n = testRootNode.addNode(nodeName1, "nt:folder");
-        n.addMixin("mix:title");
-        n.setProperty("jcr:title", "blah blah");
-        testRootNode.getSession().save();
-
-        n.removeMixin("mix:title");
-        testRootNode.getSession().save();
-        assertFalse(n.hasProperty("jcr:title"));
-
-        Node n1 = testRootNode.addNode(nodeName2, ntUnstructured);
-        n1.addMixin("mix:title");
-        n1.setProperty("jcr:title", "blah blah");
-        assertEquals(
-                n1.getProperty("jcr:title").getDefinition().getDeclaringNodeType().getName(),
-                "mix:title");
-        testRootNode.getSession().save();
-
-        n1.removeMixin("mix:title");
-        testRootNode.getSession().save();
-        assertTrue(n1.hasProperty("jcr:title"));
-
-        assertEquals(
-                n1.getProperty("jcr:title").getDefinition().getDeclaringNodeType().getName(),
-                ntUnstructured);
-    }
 }



Mime
View raw message