From 7d6b8c7235c4576dc36bc76b3d51721c901e6609 Mon Sep 17 00:00:00 2001 From: Robert Munteanu Date: Thu, 7 Sep 2017 18:23:37 +0300 Subject: [PATCH] OAK-6580 - Ensure mounts are consistent with the node type registry API draft for making the logic out of the TypeEditor public. --- .../oak/plugins/nodetype/EffectiveType.java | 2 +- .../oak/plugins/nodetype/EffectiveTypeFactory.java | 76 ++++++++++++++++++++++ .../oak/plugins/nodetype/TypeEditor.java | 60 +++++++---------- 3 files changed, 102 insertions(+), 36 deletions(-) create mode 100644 oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveTypeFactory.java diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java index d66d166d65..08eee85055 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java @@ -50,7 +50,7 @@ import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.REP_R import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.REP_RESIDUAL_PROPERTY_DEFINITIONS; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.REP_SUPERTYPES; -class EffectiveType { +public class EffectiveType { private final List types; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveTypeFactory.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveTypeFactory.java new file mode 100644 index 0000000000..233aa35664 --- /dev/null +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveTypeFactory.java @@ -0,0 +1,76 @@ +package org.apache.jackrabbit.oak.plugins.nodetype; + +import static org.apache.jackrabbit.JcrConstants.JCR_ISMIXIN; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_IS_ABSTRACT; + +import java.util.List; + +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; + +import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Lists; + +public class EffectiveTypeFactory { + + private static final Logger log = LoggerFactory.getLogger(EffectiveTypeFactory.class); + + private final NodeState types; + + public EffectiveTypeFactory(NodeState types) { + this.types = types; + } + + public EffectiveType newEffectiveType(@CheckForNull EffectiveType parent, @CheckForNull String path, + @CheckForNull String primary, @Nonnull Iterable mixins, @Nonnull ConstraintViolationCallback callback) { + + List list = Lists.newArrayList(); + String name = PathUtils.getName(path); + + NodeState type = (primary == null) ? null : types.getChildNode(primary); + if (type == null || !type.exists()) { + callback.onConstraintViolation(1, "The primary type " + primary + " does not exist"); + } else if (type.getBoolean(JCR_ISMIXIN)) { + callback.onConstraintViolation(2, "Mixin type " + primary + " used as the primary type"); + } else { + if (type.getBoolean(JCR_IS_ABSTRACT)) { + if (parent != null && name != null && primary.equals(parent.getDefaultType(name))) { + // OAK-1013: Allow (with a warning) an abstract primary + // type if it's the default type implied by the parent node + log.warn("Abstract type " + primary + + " used as the default primary type of node " + + path); + } else { + callback.onConstraintViolation(2, "Abstract type " + primary + " used as the primary type"); + } + } + list.add(type); + } + + // mixin types + for (String mixin : mixins) { + type = types.getChildNode(mixin); + if (!type.exists()) { + callback.onConstraintViolation(5, "The mixin type " + mixin + " does not exist"); + } else if (!type.getBoolean(JCR_ISMIXIN)) { + callback.onConstraintViolation(6, "Primary type " + mixin + " used as a mixin type"); + } else if (type.getBoolean(JCR_IS_ABSTRACT)) { + callback.onConstraintViolation(7, "Abstract type " + mixin + " used as a mixin type"); + } else { + list.add(type); + } + } + + return new EffectiveType(list); + + } + + static interface ConstraintViolationCallback { + + void onConstraintViolation(int code, String message); + } +} 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 1fcc04078a..6abc64252f 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 @@ -19,7 +19,6 @@ package org.apache.jackrabbit.oak.plugins.nodetype; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.in; import static com.google.common.collect.Iterables.any; -import static org.apache.jackrabbit.JcrConstants.JCR_ISMIXIN; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.JcrConstants.JCR_REQUIREDTYPE; @@ -33,7 +32,6 @@ import static org.apache.jackrabbit.oak.api.Type.STRINGS; import static org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager.isValidUUID; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; -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; @@ -53,6 +51,7 @@ import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.plugins.nodetype.EffectiveTypeFactory.ConstraintViolationCallback; import org.apache.jackrabbit.oak.plugins.value.jcr.ValueFactoryImpl; import org.apache.jackrabbit.oak.spi.commit.DefaultEditor; import org.apache.jackrabbit.oak.spi.commit.Editor; @@ -260,43 +259,34 @@ class TypeEditor extends DefaultEditor { @CheckForNull EffectiveType parent, @CheckForNull String name, @CheckForNull String primary, @Nonnull Iterable mixins) throws CommitFailedException { - List list = Lists.newArrayList(); - - NodeState type = (primary == null) ? null : types.getChildNode(primary); - if (type == null || !type.exists()) { - constraintViolation(1, "The primary type " + primary + " does not exist"); - } else if (type.getBoolean(JCR_ISMIXIN)) { - constraintViolation(2, "Mixin type " + primary + " used as the primary type"); - } else { - if (type.getBoolean(JCR_IS_ABSTRACT)) { - if (parent != null && name != null && primary.equals(parent.getDefaultType(name))) { - // OAK-1013: Allow (with a warning) an abstract primary - // type if it's the default type implied by the parent node - log.warn("Abstract type " + primary - + " used as the default primary type of node " - + getPath()); - } else { - constraintViolation(2, "Abstract type " + primary + " used as the primary type"); - } + + // TODO - pass this around + List violations = Lists.newArrayList(); + EffectiveTypeFactory factory = new EffectiveTypeFactory(types); + EffectiveType effectiveType = factory.newEffectiveType(parent, getPath(), primary, mixins, new ConstraintViolationCallback() { + + @Override + public void onConstraintViolation(int code, String message){ + violations.add(new ConstraintViolation(code, message)); + } - list.add(type); + }); + + for (ConstraintViolation violation: violations ) { + constraintViolation(violation.code, violation.message); } + + return effectiveType; + } - // mixin types - for (String mixin : mixins) { - type = types.getChildNode(mixin); - if (!type.exists()) { - constraintViolation(5, "The mixin type " + mixin + " does not exist"); - } else if (!type.getBoolean(JCR_ISMIXIN)) { - constraintViolation(6, "Primary type " + mixin + " used as a mixin type"); - } else if (type.getBoolean(JCR_IS_ABSTRACT)) { - constraintViolation(7, "Abstract type " + mixin + " used as a mixin type"); - } else { - list.add(type); - } + static class ConstraintViolation { + private int code; + private String message; + + private ConstraintViolation(int code, String message) { + this.code = code; + this.message = message; } - - return new EffectiveType(list); } @Nonnull -- 2.14.1