jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: r961769 - in /jackrabbit/branches/1.6/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/ test/java/org/apache/jackrabbit/core/
Date Thu, 08 Jul 2010 13:53:07 GMT
Author: stefan
Date: Thu Jul  8 13:53:06 2010
New Revision: 961769

URL: http://svn.apache.org/viewvc?rev=961769&view=rev
Log:
JCR-2659: Fails to remove a previously assigned mixin

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

Modified: jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java?rev=961769&r1=961768&r2=961769&view=diff
==============================================================================
--- jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
(original)
+++ jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
Thu Jul  8 13:53:06 2010
@@ -572,11 +572,19 @@ public abstract class ItemImpl implement
         while (removedIter.hasNext()) {
             ItemState itemState = (ItemState) removedIter.next();
             ItemDefinition def;
-            if (itemState.isNode()) {
-                def = itemMgr.getDefinition((NodeState) itemState);
-            } else {
-                def = itemMgr.getDefinition((PropertyState) itemState);
-            }
+            try {
+                 if (itemState.isNode()) {
+                     def = itemMgr.getDefinition((NodeState) itemState);
+                 } else {
+                     def = itemMgr.getDefinition((PropertyState) itemState);
+                 }
+             } 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 (!def.isProtected()) {
                 Path path = stateMgr.getAtticAwareHierarchyMgr().getPath(itemState.getId());
                 // check REMOVE permission

Modified: jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java?rev=961769&r1=961768&r2=961769&view=diff
==============================================================================
--- jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
(original)
+++ jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
Thu Jul  8 13:53:06 2010
@@ -214,6 +214,19 @@ public class ItemManager implements Dump
 
     PropertyDefinitionImpl getDefinition(PropertyState state)
             throws RepositoryException {
+        // this is a bit ugly
+        // there might be cases where otherwise protected items turn into
+        // non-protected items because a mixin has been removed from the parent
+        // node state.
+        // see also: JCR-2408
+        if (state.getStatus() == ItemState.STATUS_EXISTING_REMOVED
+                && state.getName().equals(NameConstants.JCR_UUID)) {
+            NodeTypeRegistry ntReg = session.getNodeTypeManager().getNodeTypeRegistry();
+            PropDef def = ntReg.getEffectiveNodeType(
+                    NameConstants.MIX_REFERENCEABLE).getApplicablePropertyDef(
+                    state.getName(), state.getType());
+            return session.getNodeTypeManager().getPropertyDefinition(def);
+        }
         try {
             NodeImpl parent = (NodeImpl) getItem(state.getParentId());
             return parent.getApplicablePropertyDefinition(

Modified: jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=961769&r1=961768&r2=961769&view=diff
==============================================================================
--- jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
(original)
+++ jackrabbit/branches/1.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
Thu Jul  8 13:53:06 2010
@@ -99,9 +99,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Calendar;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -1132,6 +1134,219 @@ public class NodeImpl extends ItemImpl i
         // check state of this instance
         sanityCheck();
 
+        int options = ItemValidator.CHECK_LOCK | ItemValidator.CHECK_VERSIONING
+                | ItemValidator.CHECK_CONSTRAINTS | ItemValidator.CHECK_HOLD;
+        int permissions = Permission.NODE_TYPE_MNGMT;
+        session.getValidator().checkModify(this, options, permissions);
+
+        // check if mixin is assigned
+        final NodeState state = data.getNodeState();
+        if (!state.getMixinTypeNames().contains(mixinName)) {
+            throw new NoSuchNodeTypeException();
+        }
+
+        NodeTypeManagerImpl ntMgr = session.getNodeTypeManager();
+        NodeTypeRegistry ntReg = ntMgr.getNodeTypeRegistry();
+
+        // build effective node type of remaining mixin's & primary type
+        Set remainingMixins = new HashSet(state.getMixinTypeNames());
+        // remove name of target mixin
+        remainingMixins.remove(mixinName);
+        EffectiveNodeType entResulting;
+        try {
+            // build effective node type representing primary type including remaining mixin's
+            entResulting = ntReg.getEffectiveNodeType(
+                    state.getNodeTypeName(), remainingMixins);
+        } catch (NodeTypeConflictException e) {
+            throw new ConstraintViolationException(e.getMessage(), e);
+        }
+
+        /**
+         * mix:referenceable needs special handling because it has
+         * special semantics:
+         * it can only be removed if there no more references to this node
+         */
+        NodeTypeImpl mixin = ntMgr.getNodeType(mixinName);
+        if ((NameConstants.MIX_REFERENCEABLE.equals(mixinName)
+                || mixin.isDerivedFrom(NameConstants.MIX_REFERENCEABLE))
+                && !entResulting.includesNodeType(NameConstants.MIX_REFERENCEABLE))
{
+            // removing this mixin would effectively remove mix:referenceable:
+            // make sure no references exist
+            PropertyIterator iter = getReferences();
+            if (iter.hasNext()) {
+                throw new ConstraintViolationException(mixinName + " can not be removed:
the node is being referenced"
+                        + " through at least one property of type REFERENCE");
+            }
+        }
+
+        /*
+         * mix:lockable: the mixin cannot be removed if the node is currently
+         * locked even if the editing session is the lock holder.
+         */
+        if ((NameConstants.MIX_LOCKABLE.equals(mixinName)
+                || mixin.isDerivedFrom(NameConstants.MIX_LOCKABLE))
+                && !entResulting.includesNodeType(NameConstants.MIX_LOCKABLE)
+                && isLocked()) {
+            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 affectedProps = new HashMap();
+        Map affectedNodes = new HashMap();
+        try {
+            Set names = thisState.getPropertyNames();
+            for (Iterator it = names.iterator(); it.hasNext();) {
+                Name propName = (Name) it.next();
+                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 entries = thisState.getChildNodeEntries();
+            for (Iterator it = entries.iterator(); it.hasNext();) {
+                ChildNodeEntry entry = (ChildNodeEntry) it.next();
+                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
+        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
+        boolean success = false;
+        try {
+            for (Iterator it = affectedProps.keySet().iterator(); it.hasNext();) {
+                PropertyId id = (PropertyId) it.next();
+                PropertyImpl prop = (PropertyImpl) itemMgr.getItem(id);
+                PropertyDefinition oldDef = (PropertyDefinition) 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);
+                        } 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);
+                        }
+                    } else {
+                        // redefine property
+                        prop.onRedefine(newDef.unwrap());
+                    }
+                } 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 (Iterator it = affectedNodes.keySet().iterator(); it.hasNext();) {
+                ChildNodeEntry entry = (ChildNodeEntry) it.next();
+                NodeState nodeState = (NodeState) stateMgr.getItemState(entry.getId());
+                NodeImpl node = (NodeImpl) itemMgr.getItem(entry.getId());
+                NodeDefinition oldDef = (NodeDefinition) affectedNodes.get(entry);
+
+                if (oldDef.isProtected()) {
+                    // remove 'orphaned' protected child node immediately
+                    removeChildNode(entry.getName(), entry.getIndex());
+                    continue;
+                }
+
+                // 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());
+                }
+            }
+            success = true;
+        } catch (ItemStateException e) {
+            throw new RepositoryException("Failed to clean up child items defined by removed
mixin " + session.getJCRName(mixinName), e);
+        } finally {
+            if (!success) {
+                // TODO JCR-1914: revert any changes made so far
+            }
+        }
+    }
+
+    /**
+     * Same as {@link Node#removeMixin(String)} except that it takes a
+     * <code>Name</code> instead of a <code>String</code>.
+     *
+     * @see Node#removeMixin(String)
+     */
+    public void removeMixinOld(Name mixinName)
+            throws NoSuchNodeTypeException, VersionException,
+            ConstraintViolationException, LockException, RepositoryException {
+        // check state of this instance
+        sanityCheck();
+
         int options = ItemValidator.CHECK_LOCK | ItemValidator.CHECK_VERSIONING |
                 ItemValidator.CHECK_CONSTRAINTS | ItemValidator.CHECK_HOLD;
         int permissions = Permission.NODE_TYPE_MNGMT;
@@ -1205,7 +1420,7 @@ public class NodeImpl extends ItemImpl i
         }
 
         // walk through properties and child nodes and remove those that aren't
-        // accomodated by the resulting new effective node type (see JCR-2130)
+        // accommodated by the resulting new effective node type (see JCR-2130)
         boolean success = false;
         try {
             // use temp set to avoid ConcurrentModificationException

Modified: jackrabbit/branches/1.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/1.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java?rev=961769&r1=961768&r2=961769&view=diff
==============================================================================
--- jackrabbit/branches/1.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
(original)
+++ jackrabbit/branches/1.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/NodeImplTest.java
Thu Jul  8 13:53:06 2010
@@ -96,6 +96,38 @@ public class NodeImplTest extends Abstra
     }
 
     /**
+     * Test case for JCR-2130 and JCR-2659.
+     *
+     * @throws RepositoryException
+     */
+    public void testAddRemoveMixin() throws RepositoryException {
+        // add mix:title to a nt:folder node and set jcr:title property
+        Node n = testRootNode.addNode(nodeName1, "nt:folder");
+        n.addMixin("mix:referenceable");
+        testRootNode.getSession().save();
+        assertTrue(n.hasProperty("jcr:uuid"));
+
+        // remove mix:title, jcr:title should be gone as there's no matching
+        // definition in nt:folder
+        n.removeMixin("mix:referenceable");
+        testRootNode.getSession().save();
+        assertFalse(n.hasProperty("jcr:uuid"));
+
+        // add mix:referenceable to a nt:unstructured node, jcr:uuid is
+        // automatically added
+        Node n2 = testRootNode.addNode(nodeName3, "nt:unstructured");
+        n2.addMixin(mixReferenceable);
+        testRootNode.getSession().save();
+        assertTrue(n2.hasProperty("jcr:uuid"));
+
+        // remove mix:referenceable, jcr:uuid should always get removed
+        // since it is a protcted property
+        n2.removeMixin(mixReferenceable);
+        testRootNode.getSession().save();
+        assertFalse(n2.hasProperty("jcr:uuid"));
+    }
+
+    /**
      * Test case for #JCR-1729. Note, that test will only be executable with
      * a security configurations that allows to set Deny-ACEs.
      *



Mime
View raw message