You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by st...@apache.org on 2010/07/08 15:53:07 UTC
svn commit: r961769 - in /jackrabbit/branches/1.6/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/ test/java/org/apache/jackrabbit/core/
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.
*