diff --git a/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugValidatorTest.java b/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugValidatorTest.java index c9f158a..78b0aab 100644 --- a/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugValidatorTest.java +++ b/oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugValidatorTest.java @@ -48,8 +48,10 @@ public class CugValidatorTest extends AbstractCugTest { @Test public void testChangePrimaryType() { + node = new NodeUtil(root.getTree(SUPPORTED_PATH2)); try { node.setName(JcrConstants.JCR_PRIMARYTYPE, NT_REP_CUG_POLICY); + node.setStrings(REP_PRINCIPAL_NAMES, EveryonePrincipal.NAME); root.commit(); fail(); } catch (CommitFailedException e) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java index f733066..91c1aff 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java @@ -36,6 +36,7 @@ import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NO import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_IS_ABSTRACT; import static org.apache.jackrabbit.oak.plugins.nodetype.constraint.Constraints.valueConstraint; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -44,7 +45,9 @@ import javax.annotation.Nonnull; import javax.jcr.PropertyType; import javax.jcr.Value; +import com.google.common.base.Objects; import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; @@ -55,6 +58,7 @@ import org.apache.jackrabbit.oak.spi.commit.DefaultEditor; import org.apache.jackrabbit.oak.spi.commit.Editor; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.spi.state.NodeStateUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -75,7 +79,7 @@ class TypeEditor extends DefaultEditor { private final Set typesToCheck; - private final boolean checkThisNode; + private boolean checkThisNode; private final TypeEditor parent; @@ -87,6 +91,8 @@ class TypeEditor extends DefaultEditor { private final NodeBuilder builder; + private final boolean validate; + TypeEditor( boolean strict, Set typesToCheck, NodeState types, String primary, Iterable mixins, NodeBuilder builder) @@ -102,11 +108,13 @@ class TypeEditor extends DefaultEditor { this.types = checkNotNull(types); this.effective = createEffectiveType(null, null, primary, mixins); this.builder = checkNotNull(builder); + this.validate = false; } private TypeEditor( @Nonnull TypeEditor parent, @Nonnull String name, - @CheckForNull String primary, @Nonnull Iterable mixins, @Nonnull NodeBuilder builder) + @CheckForNull String primary, @Nonnull Iterable mixins, @Nonnull NodeBuilder builder, + boolean validate) throws CommitFailedException { this.strict = parent.strict; this.typesToCheck = parent.typesToCheck; @@ -119,6 +127,7 @@ class TypeEditor extends DefaultEditor { this.types = parent.types; this.effective = createEffectiveType(parent.effective, name, primary, mixins); this.builder = checkNotNull(builder); + this.validate = validate; } /** @@ -133,6 +142,7 @@ class TypeEditor extends DefaultEditor { this.types = EMPTY_NODE; this.effective = checkNotNull(effective); this.builder = EMPTY_NODE.builder(); + this.validate = false; } /** @@ -174,23 +184,7 @@ class TypeEditor extends DefaultEditor { public void propertyChanged(PropertyState before, PropertyState after) throws CommitFailedException { if (checkThisNode) { - NodeState definition = effective.getDefinition(after); - if (definition == null) { - constraintViolation(4, "No matching property definition found for " + after); - } else if (JCR_UUID.equals(after.getName()) - && effective.isNodeType(MIX_REFERENCEABLE)) { - // special handling for the jcr:uuid property of mix:referenceable - // TODO: this should be done in a pluggable extension - if (!isValidUUID(after.getValue(Type.STRING))) { - constraintViolation(12, "Invalid UUID value in the jcr:uuid property"); - } - } else { - int requiredType = getRequiredType(definition); - if (requiredType != PropertyType.UNDEFINED) { - checkRequiredType(after, requiredType); - checkValueConstraints(definition, after, requiredType); - } - } + checkPropertyTypeConstraints(after); } } @@ -205,31 +199,20 @@ class TypeEditor extends DefaultEditor { } @Override - public Editor childNodeAdded(String name, NodeState after) - throws CommitFailedException { - TypeEditor editor = childNodeChanged(name, MISSING_NODE, after); - - if (editor != null && editor.checkThisNode) { - // TODO: add any auto-created items that are still missing - - // verify the presence of all mandatory items - for (String property : editor.getEffective().getMandatoryProperties()) { - if (!after.hasProperty(property)) { - editor.constraintViolation( - 21, "Mandatory property " + property - + " not found in a new node"); - } - } - for (String child : editor.getEffective().getMandatoryChildNodes()) { - if (!after.hasChildNode(child)) { - editor.constraintViolation( - 25, "Mandatory child node " + child - + " not found in a new node"); - } - } + public void enter(NodeState before, NodeState after) throws CommitFailedException { + if (checkThisNode && validate) { + // when adding a new node, or changing node type on an existing + // node, we need to reverify type constraints + checkNodeTypeConstraints(after); + checkThisNode = false; } + } - return editor; + @Override + public Editor childNodeAdded(String name, NodeState after) + throws CommitFailedException { + // TODO: add any auto-created items that are still missing + return childNodeChanged(name, MISSING_NODE, after); } @Override @@ -239,7 +222,6 @@ class TypeEditor extends DefaultEditor { String primary = after.getName(JCR_PRIMARYTYPE); Iterable mixins = after.getNames(JCR_MIXINTYPES); - NodeBuilder childBuilder = builder.getChildNode(name); if (primary == null && effective != null) { // no primary type defined, find and apply a default type primary = effective.getDefaultType(name); @@ -252,8 +234,11 @@ class TypeEditor extends DefaultEditor { } } - TypeEditor editor = new TypeEditor(this, name, primary, mixins, childBuilder); - if (checkThisNode && !getEffective().isValidChildNode(name, editor.getEffective())) { + // if node type didn't change no need to validate child node + boolean validate = primaryChanged(before, primary) || mixinsChanged(before, mixins); + NodeBuilder childBuilder = builder.getChildNode(name); + TypeEditor editor = new TypeEditor(this, name, primary, mixins, childBuilder, validate); + if (checkThisNode && validate && !effective.isValidChildNode(name, editor.getEffective())) { constraintViolation( 1, "No matching definition found for child node " + name + " with effective type " + editor.getEffective()); @@ -380,4 +365,79 @@ class TypeEditor extends DefaultEditor { constraintViolation(5, "Value constraint violation in " + property); } + private static boolean primaryChanged(NodeState before, String after) { + String pre = before.getName(JCR_PRIMARYTYPE); + return !Objects.equal(pre, after); + } + + private static boolean mixinsChanged(NodeState before, Iterable after) { + List pre = Lists.newArrayList(before.getNames(JCR_MIXINTYPES)); + Collections.sort(pre); + List post = Lists.newArrayList(after); + Collections.sort(post); + if (pre.isEmpty() && post.isEmpty()) { + return false; + } else if (pre.isEmpty() || post.isEmpty()) { + return true; + } else { + return !Iterables.elementsEqual(pre, post); + } + } + + private void checkNodeTypeConstraints(NodeState after) throws CommitFailedException { + EffectiveType effective = getEffective(); + + Set properties = effective.getMandatoryProperties(); + for (PropertyState ps : after.getProperties()) { + properties.remove(ps.getName()); + checkPropertyTypeConstraints(ps); + } + // verify the presence of all mandatory items + if (!properties.isEmpty()) { + constraintViolation(21, "Mandatory property " + properties.iterator().next() + " not found in a new node"); + } + + List names = Lists.newArrayList(after.getChildNodeNames()); + for (String child : effective.getMandatoryChildNodes()) { + if (!names.remove(child)) { + constraintViolation(25, "Mandatory child node " + child + " not found in a new node"); + } + } + if (!names.isEmpty()) { + for (String name : names) { + NodeState child = after.getChildNode(name); + String primary = child.getName(JCR_PRIMARYTYPE); + Iterable mixins = child.getNames(JCR_MIXINTYPES); + NodeBuilder childBuilder = builder.getChildNode(name); + TypeEditor editor = new TypeEditor(this, name, primary, mixins, childBuilder, false); + if (!effective.isValidChildNode(name, editor.getEffective())) { + constraintViolation(25, "Unexpected child node " + names + " found in a new node"); + } + } + } + } + + private void checkPropertyTypeConstraints(PropertyState after) + throws CommitFailedException { + if (NodeStateUtils.isHidden(after.getName())) { + return; + } + NodeState definition = effective.getDefinition(after); + if (definition == null) { + constraintViolation(4, "No matching property definition found for " + after); + } else if (JCR_UUID.equals(after.getName()) && effective.isNodeType(MIX_REFERENCEABLE)) { + // special handling for the jcr:uuid property of mix:referenceable + // TODO: this should be done in a pluggable extension + if (!isValidUUID(after.getValue(Type.STRING))) { + constraintViolation(12, "Invalid UUID value in the jcr:uuid property"); + } + } else { + int requiredType = getRequiredType(definition); + if (requiredType != PropertyType.UNDEFINED) { + checkRequiredType(after, requiredType); + checkValueConstraints(definition, after, requiredType); + } + } + } + } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorTest.java index 6087328..36e02ed 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditorTest.java @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugins.nodetype; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.JcrConstants.NT_FOLDER; +import static org.apache.jackrabbit.JcrConstants.MIX_REFERENCEABLE; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; import static org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent.INITIAL_CONTENT; import static org.easymock.EasyMock.createControl; @@ -225,4 +226,98 @@ public class TypeEditorTest { builder.setProperty("any", 134.34, Type.DOUBLE); hook.processCommit(after, builder.getNodeState(), CommitInfo.EMPTY); } + + @Test + public void changeNodeTypeWExtraNodes() throws CommitFailedException { + EditorHook hook = new EditorHook(new TypeEditorProvider()); + + NodeState root = INITIAL_CONTENT; + NodeBuilder builder = root.builder(); + + NodeState before = builder.getNodeState(); + + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:unstructured", Type.NAME); + builder.child("testcontent").child("unstructured_child").setProperty(JCR_PRIMARYTYPE, "nt:unstructured", + Type.NAME); + NodeState after = builder.getNodeState(); + root = hook.processCommit(before, after, CommitInfo.EMPTY); + + builder = root.builder(); + before = builder.getNodeState(); + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:folder", Type.NAME); + try { + hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY); + fail("should not be able to change node type due to extra nodes"); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + } + } + + @Test + public void changeNodeTypeWExtraProps() throws CommitFailedException { + EditorHook hook = new EditorHook(new TypeEditorProvider()); + + NodeState root = INITIAL_CONTENT; + NodeBuilder builder = root.builder(); + + NodeState before = builder.getNodeState(); + + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:unstructured", Type.NAME); + builder.child("testcontent").setProperty("extra", "information"); + + NodeState after = builder.getNodeState(); + root = hook.processCommit(before, after, CommitInfo.EMPTY); + + builder = root.builder(); + before = builder.getNodeState(); + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:folder", Type.NAME); + try { + hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY); + fail("should not be able to change node type due to extra properties"); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + } + } + + @Test + public void changeNodeTypeNewBroken() throws CommitFailedException { + EditorHook hook = new EditorHook(new TypeEditorProvider()); + + NodeState root = INITIAL_CONTENT; + NodeBuilder builder = root.builder(); + + NodeState before = builder.getNodeState(); + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:folder", Type.NAME); + builder.child("testcontent").setProperty("extra", "information"); + try { + hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY); + fail("should not be able to change node type due to extra properties"); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + } + } + + @Test + public void malformedUUID() throws CommitFailedException { + EditorHook hook = new EditorHook(new TypeEditorProvider()); + + NodeState root = INITIAL_CONTENT; + NodeBuilder builder = root.builder(); + + NodeState before = builder.getNodeState(); + builder.child("testcontent").setProperty(JCR_PRIMARYTYPE, "nt:unstructured", Type.NAME); + builder.child("testcontent").setProperty("jcr:uuid", "not-a-uuid"); + NodeState after = builder.getNodeState(); + root = hook.processCommit(before, after, CommitInfo.EMPTY); + + builder = root.builder(); + before = builder.getNodeState(); + builder.child("testcontent").setProperty(JCR_MIXINTYPES, ImmutableList.of(MIX_REFERENCEABLE), Type.NAMES); + try { + hook.processCommit(before, builder.getNodeState(), CommitInfo.EMPTY); + fail("should not be able to change mixin due to illegal uuid format"); + } catch (CommitFailedException e) { + assertTrue(e.isConstraintViolation()); + } + } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java index 4ce6f4e..f747697 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java @@ -24,6 +24,7 @@ import static com.google.common.collect.Iterators.transform; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Sets.newHashSet; import static com.google.common.collect.Sets.newLinkedHashSet; +import static org.apache.jackrabbit.JcrConstants.JCR_HASORDERABLECHILDNODES; import static org.apache.jackrabbit.JcrConstants.JCR_ISMIXIN; import static org.apache.jackrabbit.JcrConstants.JCR_LOCKISDEEP; import static org.apache.jackrabbit.JcrConstants.JCR_LOCKOWNER; @@ -38,6 +39,7 @@ import static org.apache.jackrabbit.JcrConstants.JCR_UUID; import static org.apache.jackrabbit.JcrConstants.MIX_LOCKABLE; import static org.apache.jackrabbit.JcrConstants.NT_BASE; import static org.apache.jackrabbit.oak.api.Type.BOOLEAN; +import static org.apache.jackrabbit.oak.api.Type.NAME; import static org.apache.jackrabbit.oak.api.Type.NAMES; import static org.apache.jackrabbit.oak.api.Type.UNDEFINED; import static org.apache.jackrabbit.oak.api.Type.UNDEFINEDS; @@ -377,6 +379,29 @@ public class NodeDelegate extends ItemDelegate { } } + public void setPrimaryType(String typeName) throws RepositoryException { + Tree typeRoot = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH); + Tree type = typeRoot.getChild(typeName); + if (!type.exists()) { + throw new NoSuchNodeTypeException("Node type " + typeName + " does not exist"); + } else if (getBoolean(type, JCR_IS_ABSTRACT)) { + throw new ConstraintViolationException("Node type " + typeName + " is abstract"); + } else if (getBoolean(type, JCR_ISMIXIN)) { + throw new ConstraintViolationException("Node type " + typeName + " is a mixin type"); + } + String oldType = TreeUtil.getName(tree, JCR_PRIMARYTYPE); + if (typeName.equals(oldType)) { + return; + } + PropertyState state = PropertyStates.createProperty(JCR_PRIMARYTYPE, typeName, NAME); + setProperty(state, true, true); + setOrderableChildren(getBoolean(type, JCR_HASORDERABLECHILDNODES)); + TreeUtil.autoCreateItems(getTree(), type, typeRoot, getUserID()); + if (oldType != null) { + purgeNode(typeRoot, newHashSet(oldType)); + } + } + public boolean canAddMixin(String typeName) throws RepositoryException { Tree type = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH).getChild(typeName); if (type.exists()) { @@ -416,11 +441,13 @@ public class NodeDelegate extends ItemDelegate { } } - public void updateMixins(Set addMixinNames, Set removedOakMixinNames) throws RepositoryException { + Tree tree = getTree(); + Tree typeRoot = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH); + String userId = getUserID(); // 1. set all new mixin types including validation for (String oakMixinName : addMixinNames) { - addMixin(oakMixinName); + TreeUtil.addMixin(tree, oakMixinName, typeRoot, userId); } if (!removedOakMixinNames.isEmpty()) { @@ -431,61 +458,77 @@ public class NodeDelegate extends ItemDelegate { mixinNames.addAll(addMixinNames); tree.setProperty(JCR_MIXINTYPES, mixinNames, NAMES); } - - // 3. deal with locked nodes - boolean wasLockable = isNodeType(MIX_LOCKABLE); - boolean isLockable = isNodeType(MIX_LOCKABLE); - if (wasLockable && !isLockable && holdsLock(false)) { - // TODO: This should probably be done in a commit hook - unlock(); - sessionDelegate.refresh(true); - } - - // 4. clean up set of properties and child nodes such that all child items + // 3. clean up set of properties and child nodes such that all child items // have a valid item definition according to the effective node type present // after having updated the mixin property. this includes removing all // protected properties and child nodes associated with the removed mixin // type(s), as there's no way for the client to do that. Other items // defined in this mixin type might also need to be removed if there // is no longer a matching item definition available. - Tree typeRoot = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH); - List removed = new ArrayList(); - for (String name : removedOakMixinNames) { - removed.add(typeRoot.getChild(name)); - } - List remaining = getNodeTypes(tree, typeRoot); - - for (PropertyState property : tree.getProperties()) { - String name = property.getName(); - Type type = property.getType(); - - Tree oldDefinition = findMatchingPropertyDefinition(removed, name, type, true); - if (oldDefinition != null) { - Tree newDefinition = findMatchingPropertyDefinition(remaining, name, type, true); - if (newDefinition == null - || (getBoolean(oldDefinition, JCR_PROTECTED) - && !getBoolean(newDefinition, JCR_PROTECTED))) { - tree.removeProperty(name); - } + purgeNode(typeRoot, removedOakMixinNames); + } + } + + /** + * Clean up the set of properties and child nodes such that all child items + * have a valid item definition according to the effective node type present + * after having updated the mixin property or the primary type. this + * includes removing all protected properties and child nodes associated + * with the removed mixin type(s), as there's no way for the client to do + * that. Other items defined in this mixin type might also need to be + * removed if there is no longer a matching item definition available. + * @param typeRoot + * @param removedNames + */ + private void purgeNode(Tree typeRoot, Set removedNames) throws RepositoryException { + if (removedNames.isEmpty()) { + return; + } + + // deal with locked nodes + boolean wasLockable = isNodeType(MIX_LOCKABLE); + boolean isLockable = isNodeType(MIX_LOCKABLE); + if (wasLockable && !isLockable && holdsLock(false)) { + // TODO: This should probably be done in a commit hook + unlock(); + sessionDelegate.refresh(true); + } + List removed = new ArrayList(); + for (String name : removedNames) { + removed.add(typeRoot.getChild(name)); + } + List remaining = getNodeTypes(tree, typeRoot); + + for (PropertyState property : tree.getProperties()) { + String name = property.getName(); + Type type = property.getType(); + + Tree oldDefinition = findMatchingPropertyDefinition(removed, name, type, true); + if (oldDefinition != null) { + Tree newDefinition = findMatchingPropertyDefinition(remaining, name, type, true); + if (newDefinition == null + || (getBoolean(oldDefinition, JCR_PROTECTED) + && !getBoolean(newDefinition, JCR_PROTECTED))) { + tree.removeProperty(name); } } + } - for (Tree child : tree.getChildren()) { - String name = child.getName(); - Set typeNames = newLinkedHashSet(); - for (Tree type : getNodeTypes(child, typeRoot)) { - typeNames.add(TreeUtil.getName(type, JCR_NODETYPENAME)); - addAll(typeNames, getNames(type, REP_SUPERTYPES)); - } + for (Tree child : tree.getChildren()) { + String name = child.getName(); + Set typeNames = newLinkedHashSet(); + for (Tree type : getNodeTypes(child, typeRoot)) { + typeNames.add(TreeUtil.getName(type, JCR_NODETYPENAME)); + addAll(typeNames, getNames(type, REP_SUPERTYPES)); + } - Tree oldDefinition = findMatchingChildNodeDefinition(removed, name, typeNames); - if (oldDefinition != null) { - Tree newDefinition = findMatchingChildNodeDefinition(remaining, name, typeNames); - if (newDefinition == null - || (getBoolean(oldDefinition, JCR_PROTECTED) - && !getBoolean(newDefinition, JCR_PROTECTED))) { - child.remove(); - } + Tree oldDefinition = findMatchingChildNodeDefinition(removed, name, typeNames); + if (oldDefinition != null) { + Tree newDefinition = findMatchingChildNodeDefinition(remaining, name, typeNames); + if (newDefinition == null + || (getBoolean(oldDefinition, JCR_PROTECTED) + && !getBoolean(newDefinition, JCR_PROTECTED))) { + child.remove(); } } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java index 15ffa7a..a6e4c50 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java @@ -934,6 +934,7 @@ public class NodeImpl extends ItemImpl implements Nod @Override public void setPrimaryType(final String nodeTypeName) throws RepositoryException { + final String oakTypeName = getOakName(checkNotNull(nodeTypeName)); sessionDelegate.performVoid(new ItemWriteOperation("setPrimaryType") { @Override public void checkPreconditions() throws RepositoryException { @@ -945,7 +946,7 @@ public class NodeImpl extends ItemImpl implements Nod @Override public void performVoid() throws RepositoryException { - internalSetPrimaryType(nodeTypeName); + dlg.setPrimaryType(oakTypeName); } }); } @@ -1341,20 +1342,6 @@ public class NodeImpl extends ItemImpl implements Nod } } - private void internalSetPrimaryType(final String nodeTypeName) throws RepositoryException { - // TODO: figure out the right place for this check - NodeType nt = getNodeTypeManager().getNodeType(nodeTypeName); // throws on not found - if (nt.isAbstract() || nt.isMixin()) { - throw new ConstraintViolationException(getNodePath()); - } - // TODO: END - - PropertyState state = PropertyStates.createProperty( - JCR_PRIMARYTYPE, getOakName(nodeTypeName), NAME); - dlg.setProperty(state, true, true); - dlg.setOrderableChildren(nt.hasOrderableChildNodes()); - } - private Property internalSetProperty( final String jcrName, final Value value, final boolean exactTypeMatch) throws RepositoryException { diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java index b9c3977..2311452 100644 --- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java +++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/RepositoryTest.java @@ -1872,7 +1872,37 @@ public class RepositoryTest extends AbstractRepositoryTest { } @Test - @Ignore("OAK-5229") + public void setPrimaryTypeCleanup() throws RepositoryException { + String path = TEST_PATH + "/" + "testType"; + getAdminSession().getWorkspace().copy("/jcr:system/jcr:nodeTypes/oak:Unstructured/jcr:propertyDefinition", + path); + + Node testNode = getNode(path); + assertEquals("nt:propertyDefinition", testNode.getPrimaryNodeType().getName()); + + List protectedProps = Arrays.asList("jcr:valueConstraints", "jcr:isQueryOrderable", + "jcr:isFullTextSearchable", "jcr:requiredType", "jcr:protected", "jcr:multiple", "jcr:onParentVersion", + "jcr:availableQueryOperators", "jcr:defaultValues", "jcr:mandatory", "jcr:autoCreated"); + + Set pnames = new HashSet<>(); + PropertyIterator pi = testNode.getProperties(); + while (pi.hasNext()) { + pnames.add(pi.nextProperty().getName()); + } + assertTrue(pnames.containsAll(protectedProps)); + + testNode.setPrimaryType("nt:unstructured"); + getAdminSession().save(); + + pnames = new HashSet<>(); + pi = testNode.getProperties(); + while (pi.hasNext()) { + pnames.add(pi.nextProperty().getName()); + } + assertTrue(Collections.disjoint(protectedProps, pnames)); + } + + @Test public void setPrimaryTypeShouldFail() throws RepositoryException { Node testNode = getNode(TEST_PATH); assertEquals("nt:unstructured", testNode.getPrimaryNodeType().getName()); @@ -1883,18 +1913,32 @@ public class RepositoryTest extends AbstractRepositoryTest { getAdminSession().save(); testNode.setPrimaryType("nt:folder"); - try { - getAdminSession().save(); - fail("Changing the primary type to nt:folder should fail."); - } catch (RepositoryException e) { - // ok - } - + // try { + // getAdminSession().save(); + // fail("Changing the primary type to nt:folder should fail."); + // } catch (RepositoryException e) { + // getAdminSession().refresh(false); + // } + // + // Session session2 = createAnonymousSession(); + // try { + // testNode = session2.getNode(TEST_PATH); + // assertEquals("nt:unstructured", + // testNode.getPrimaryNodeType().getName()); + // assertEquals("nt:unstructured", + // testNode.getProperty("jcr:primaryType").getString()); + // } finally { + // session2.logout(); + // } + + // purge enabled + getAdminSession().save(); Session session2 = createAnonymousSession(); try { testNode = session2.getNode(TEST_PATH); - assertEquals("nt:unstructured", testNode.getPrimaryNodeType().getName()); - assertEquals("nt:unstructured", testNode.getProperty("jcr:primaryType").getString()); + assertEquals("nt:folder", testNode.getPrimaryNodeType().getName()); + // child node is gone + assertEquals(0, testNode.getNodes().getSize()); } finally { session2.logout(); } diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java index ad01663..99eae00 100644 --- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java +++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java @@ -115,7 +115,7 @@ public class NodeTypeManagementTest extends AbstractEvaluationTest { String ntName = child.getPrimaryNodeType().getName(); try { - childNode.setPrimaryType("nt:folder"); + childNode.setPrimaryType("oak:Unstructured"); testSession.save(); fail("TestSession does not have sufficient privileges to change the primary type."); } catch (AccessDeniedException e) { @@ -134,7 +134,7 @@ public class NodeTypeManagementTest extends AbstractEvaluationTest { Node child = (Node) superuser.getItem(childNode.getPath()); String ntName = child.getPrimaryNodeType().getName(); - String changedNtName = "nt:folder"; + String changedNtName = "oak:Unstructured"; child.setPrimaryType(changedNtName); testSession.save();