From: Robert Varga Date: Wed, 30 Jan 2019 12:49:40 +0000 (+0100) Subject: Eliminate no-op MandatoryLeafEnforcer X-Git-Tag: v2.0.17~33 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=e9e1269514ffbb04e52e0125c195ed9623da18e8;p=yangtools.git Eliminate no-op MandatoryLeafEnforcer Running through a no-op enforcer, while efficient, still requires a bimorphic invocation, which results most of the time in a no-op. Eliminate the no-op implementation and refactor its two users, so that they provide dedicated subclasses which use enforcer. This has the benefit of not overriding SchemaAwareApplyOperation just to run a noop invocation. JIRA: YANGTOOLS-945 Change-Id: Iff44eb6efa048bb86a71b0d7753d3e58425b5e4d Signed-off-by: Robert Varga (cherry picked from commit 8dbd4934c7ed38b62a36a27322c663c6ddb45159) --- diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractMapModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractMapModificationStrategy.java index b764890a1b..40b808e9c1 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractMapModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractMapModificationStrategy.java @@ -30,7 +30,7 @@ abstract class AbstractMapModificationStrategy extends AbstractNodeContainerModi AbstractMapModificationStrategy(final Class nodeClass, final ListSchemaNode schema, final DataTreeConfiguration treeConfig) { super(nodeClass, treeConfig); - entryStrategy = Optional.of(new ListEntryModificationStrategy(schema, treeConfig)); + entryStrategy = Optional.of(ListEntryModificationStrategy.of(schema, treeConfig)); } @Override diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/CaseEnforcer.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/CaseEnforcer.java index b3703dfabc..c12b677452 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/CaseEnforcer.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/CaseEnforcer.java @@ -7,12 +7,13 @@ */ package org.opendaylight.yangtools.yang.data.impl.schema.tree; -import com.google.common.base.Preconditions; +import static java.util.Objects.requireNonNull; + import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.Sets; -import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier; @@ -26,17 +27,30 @@ import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; import org.opendaylight.yangtools.yang.model.api.CaseSchemaNode; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; -final class CaseEnforcer implements Immutable { - private final Map children; - private final Map augmentations; - private final MandatoryLeafEnforcer enforcer; +class CaseEnforcer implements Immutable { + private static final class EnforcingMandatory extends CaseEnforcer { + private final MandatoryLeafEnforcer enforcer; + + EnforcingMandatory(final ImmutableMap children, + final ImmutableMap augmentations, + final MandatoryLeafEnforcer enforcer) { + super(children, augmentations); + this.enforcer = requireNonNull(enforcer); + } + + @Override + void enforceOnTreeNode(final NormalizedNode normalizedNode) { + enforcer.enforceOnData(normalizedNode); + } + } - private CaseEnforcer(final Map children, - final Map augmentations, - final MandatoryLeafEnforcer enforcer) { - this.children = Preconditions.checkNotNull(children); - this.augmentations = Preconditions.checkNotNull(augmentations); - this.enforcer = Preconditions.checkNotNull(enforcer); + private final ImmutableMap children; + private final ImmutableMap augmentations; + + CaseEnforcer(final ImmutableMap children, + final ImmutableMap augmentations) { + this.children = requireNonNull(children); + this.augmentations = requireNonNull(augmentations); } static CaseEnforcer forTree(final CaseSchemaNode schema, final DataTreeConfiguration treeConfig) { @@ -57,33 +71,37 @@ final class CaseEnforcer implements Immutable { } } - final Map children = childrenBuilder.build(); - final Map augmentations = augmentationsBuilder.build(); - return children.isEmpty() ? null - : new CaseEnforcer(children, augmentations, MandatoryLeafEnforcer.forContainer(schema, treeConfig)); + final ImmutableMap children = childrenBuilder.build(); + if (children.isEmpty()) { + return null; + } + final ImmutableMap augmentations = augmentationsBuilder.build(); + final Optional enforcer = MandatoryLeafEnforcer.forContainer(schema, treeConfig); + return enforcer.isPresent() ? new EnforcingMandatory(children, augmentations, enforcer.get()) + : new CaseEnforcer(children, augmentations); } - Set> getChildEntries() { + final Set> getChildEntries() { return children.entrySet(); } - Set getChildIdentifiers() { + final Set getChildIdentifiers() { return children.keySet(); } - Set> getAugmentationEntries() { + final Set> getAugmentationEntries() { return augmentations.entrySet(); } - Set getAugmentationIdentifiers() { + final Set getAugmentationIdentifiers() { return augmentations.keySet(); } - Set getAllChildIdentifiers() { + final Set getAllChildIdentifiers() { return Sets.union(children.keySet(), augmentations.keySet()); } void enforceOnTreeNode(final NormalizedNode normalizedNode) { - enforcer.enforceOnData(normalizedNode); + // Default is no-op } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java index 5b988ed169..361cfcf8f7 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java @@ -8,24 +8,81 @@ package org.opendaylight.yangtools.yang.data.impl.schema.tree; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; import com.google.common.base.Preconditions; +import java.util.Optional; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeConfiguration; +import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version; import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode; /** - * General container modification strategy. Used by {@link PresenceContainerModificationStrategy} via subclassing - * and by {@link StructuralContainerModificationStrategy} as a delegate. + * General container modification strategy. This is used by {@link EnforcingMandatory} in case of presence containers + * with mandatory nodes, as it needs to tap into {@link SchemaAwareApplyOperation}'s operations, or subclassed + * for the purposes of {@link StructuralContainerModificationStrategy}'s automatic lifecycle. */ class ContainerModificationStrategy extends AbstractDataNodeContainerModificationStrategy { + private static final class EnforcingMandatory extends ContainerModificationStrategy { + private final MandatoryLeafEnforcer enforcer; + + EnforcingMandatory(final ContainerSchemaNode schemaNode, final DataTreeConfiguration treeConfig, + final MandatoryLeafEnforcer enforcer) { + super(schemaNode, treeConfig); + this.enforcer = requireNonNull(enforcer); + } + + @Override + void verifyStructure(final NormalizedNode writtenValue, final boolean verifyChildren) { + super.verifyStructure(writtenValue, verifyChildren); + if (verifyChildren) { + enforcer.enforceOnData(writtenValue); + } + } + + @Override + protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, + final Version version) { + final TreeNode ret = super.applyMerge(modification, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } + + @Override + protected TreeNode applyWrite(final ModifiedNode modification, final NormalizedNode newValue, + final Optional currentMeta, final Version version) { + final TreeNode ret = super.applyWrite(modification, newValue, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } + + @Override + protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, + final Version version) { + final TreeNode ret = super.applyTouch(modification, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } + } + ContainerModificationStrategy(final ContainerSchemaNode schemaNode, final DataTreeConfiguration treeConfig) { super(schemaNode, ContainerNode.class, treeConfig); } + static ModificationApplyOperation of(final ContainerSchemaNode schema, final DataTreeConfiguration treeConfig) { + if (schema.isPresenceContainer()) { + final Optional enforcer = MandatoryLeafEnforcer.forContainer(schema, treeConfig); + return enforcer.isPresent() ? new EnforcingMandatory(schema, treeConfig, enforcer.get()) + : new ContainerModificationStrategy(schema, treeConfig); + } + + return new StructuralContainerModificationStrategy(schema, treeConfig); + } + @Override @SuppressWarnings("rawtypes") protected final DataContainerNodeBuilder createBuilder(final NormalizedNode original) { diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java index 9b44410f08..ff7d147090 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java @@ -8,6 +8,7 @@ package org.opendaylight.yangtools.yang.data.impl.schema.tree; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; import java.util.Optional; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; @@ -19,42 +20,57 @@ import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContaine import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableMapEntryNodeBuilder; import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; -final class ListEntryModificationStrategy extends AbstractDataNodeContainerModificationStrategy { - private final MandatoryLeafEnforcer enforcer; +class ListEntryModificationStrategy extends AbstractDataNodeContainerModificationStrategy { + private static final class EnforcingMandatory extends ListEntryModificationStrategy { + private final MandatoryLeafEnforcer enforcer; - ListEntryModificationStrategy(final ListSchemaNode schema, final DataTreeConfiguration treeConfig) { - super(schema, MapEntryNode.class, treeConfig); - enforcer = MandatoryLeafEnforcer.forContainer(schema, treeConfig); - } + EnforcingMandatory(final ListSchemaNode schemaNode, final DataTreeConfiguration treeConfig, + final MandatoryLeafEnforcer enforcer) { + super(schemaNode, treeConfig); + this.enforcer = requireNonNull(enforcer); + } - @Override - void verifyStructure(final NormalizedNode writtenValue, final boolean verifyChildren) { - super.verifyStructure(writtenValue, verifyChildren); - if (verifyChildren) { - enforcer.enforceOnData(writtenValue); + @Override + void verifyStructure(final NormalizedNode writtenValue, final boolean verifyChildren) { + super.verifyStructure(writtenValue, verifyChildren); + if (verifyChildren) { + enforcer.enforceOnData(writtenValue); + } } - } - @Override - protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { - final TreeNode ret = super.applyMerge(modification, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; + @Override + protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, + final Version version) { + final TreeNode ret = super.applyMerge(modification, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } + + @Override + protected TreeNode applyWrite(final ModifiedNode modification, final NormalizedNode newValue, + final Optional currentMeta, final Version version) { + final TreeNode ret = super.applyWrite(modification, newValue, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } + + @Override + protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, + final Version version) { + final TreeNode ret = super.applyTouch(modification, currentMeta, version); + enforcer.enforceOnTreeNode(ret); + return ret; + } } - @Override - protected TreeNode applyWrite(final ModifiedNode modification, final NormalizedNode newValue, - final Optional currentMeta, final Version version) { - final TreeNode ret = super.applyWrite(modification, newValue, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; + ListEntryModificationStrategy(final ListSchemaNode schema, final DataTreeConfiguration treeConfig) { + super(schema, MapEntryNode.class, treeConfig); } - @Override - protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { - final TreeNode ret = super.applyTouch(modification, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; + static ListEntryModificationStrategy of(final ListSchemaNode schema, final DataTreeConfiguration treeConfig) { + final Optional enforcer = MandatoryLeafEnforcer.forContainer(schema, treeConfig); + return enforcer.isPresent() ? new EnforcingMandatory(schema, treeConfig, enforcer.get()) + : new ListEntryModificationStrategy(schema, treeConfig); } @Override diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java index 354aa1bb76..1ff166d6c7 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MandatoryLeafEnforcer.java @@ -7,10 +7,11 @@ */ package org.opendaylight.yangtools.yang.data.impl.schema.tree; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableCollection.Builder; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + import com.google.common.collect.ImmutableList; -import java.util.Collection; +import com.google.common.collect.ImmutableList.Builder; import java.util.Optional; import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; @@ -29,38 +30,38 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; // TODO: would making this Serializable be useful (for Functions and similar?) -abstract class MandatoryLeafEnforcer implements Immutable { - private static final class Strict extends MandatoryLeafEnforcer { - private final Collection mandatoryNodes; +final class MandatoryLeafEnforcer implements Immutable { + private static final Logger LOG = LoggerFactory.getLogger(MandatoryLeafEnforcer.class); - Strict(final Collection mandatoryNodes) { - this.mandatoryNodes = Preconditions.checkNotNull(mandatoryNodes); - } + private final ImmutableList mandatoryNodes; - @Override - void enforceOnData(final NormalizedNode data) { - for (final YangInstanceIdentifier id : mandatoryNodes) { - final Optional> descandant = NormalizedNodes.findNode(data, id); - Preconditions.checkArgument(descandant.isPresent(), "Node %s is missing mandatory descendant %s", - data.getIdentifier(), id); - } + private MandatoryLeafEnforcer(final ImmutableList mandatoryNodes) { + this.mandatoryNodes = requireNonNull(mandatoryNodes); + } + + static Optional forContainer(final DataNodeContainer schema, + final DataTreeConfiguration treeConfig) { + if (!treeConfig.isMandatoryNodesValidationEnabled()) { + return Optional.empty(); } + + final Builder builder = ImmutableList.builder(); + findMandatoryNodes(builder, YangInstanceIdentifier.EMPTY, schema, treeConfig.getTreeType()); + final ImmutableList mandatoryNodes = builder.build(); + return mandatoryNodes.isEmpty() ? Optional.empty() : Optional.of(new MandatoryLeafEnforcer(mandatoryNodes)); } - private static final Logger LOG = LoggerFactory.getLogger(MandatoryLeafEnforcer.class); - private static final MandatoryLeafEnforcer NOOP_ENFORCER = new MandatoryLeafEnforcer() { - @Override - void enforceOnData(final NormalizedNode normalizedNode) { - // Intentional no-op + void enforceOnData(final NormalizedNode data) { + for (final YangInstanceIdentifier id : mandatoryNodes) { + checkArgument(NormalizedNodes.findNode(data, id).isPresent(), + "Node %s is missing mandatory descendant %s", data.getIdentifier(), id); } - }; + } - final void enforceOnTreeNode(final TreeNode tree) { + void enforceOnTreeNode(final TreeNode tree) { enforceOnData(tree.getData()); } - abstract void enforceOnData(NormalizedNode normalizedNode); - private static void findMandatoryNodes(final Builder builder, final YangInstanceIdentifier id, final DataNodeContainer schema, final TreeType type) { for (final DataSchemaNode child : schema.getChildNodes()) { @@ -88,15 +89,4 @@ abstract class MandatoryLeafEnforcer implements Immutable { } } } - - static MandatoryLeafEnforcer forContainer(final DataNodeContainer schema, final DataTreeConfiguration treeConfig) { - if (!treeConfig.isMandatoryNodesValidationEnabled()) { - return NOOP_ENFORCER; - } - - final Builder builder = ImmutableList.builder(); - findMandatoryNodes(builder, YangInstanceIdentifier.EMPTY, schema, treeConfig.getTreeType()); - final Collection mandatoryNodes = builder.build(); - return mandatoryNodes.isEmpty() ? NOOP_ENFORCER : new Strict(mandatoryNodes); - } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java deleted file mode 100644 index 5be8745cad..0000000000 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/PresenceContainerModificationStrategy.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright (c) 2014 Cisco Systems, Inc. and others. All rights reserved. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License v1.0 which accompanies this distribution, - * and is available at http://www.eclipse.org/legal/epl-v10.html - */ - -package org.opendaylight.yangtools.yang.data.impl.schema.tree; - -import java.util.Optional; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeConfiguration; -import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; -import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version; -import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode; - -/** - * Presence container modification strategy. In addition to {@link ContainerModificationStrategy} it also enforces - * presence of mandatory leaves. - */ -final class PresenceContainerModificationStrategy extends ContainerModificationStrategy { - private final MandatoryLeafEnforcer enforcer; - - PresenceContainerModificationStrategy(final ContainerSchemaNode schemaNode, - final DataTreeConfiguration treeConfig) { - super(schemaNode, treeConfig); - enforcer = MandatoryLeafEnforcer.forContainer(schemaNode, treeConfig); - } - - @Override - void verifyStructure(final NormalizedNode writtenValue, final boolean verifyChildren) { - super.verifyStructure(writtenValue, verifyChildren); - if (verifyChildren) { - enforcer.enforceOnData(writtenValue); - } - } - - @Override - protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { - final TreeNode ret = super.applyMerge(modification, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; - } - - @Override - protected TreeNode applyWrite(final ModifiedNode modification, final NormalizedNode newValue, - final Optional currentMeta, final Version version) { - final TreeNode ret = super.applyWrite(modification, newValue, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; - } - - @Override - protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { - final TreeNode ret = super.applyTouch(modification, currentMeta, version); - enforcer.enforceOnTreeNode(ret); - return ret; - } -} diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java index 43bb4bc5c6..c1546a9bbf 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java @@ -43,12 +43,7 @@ abstract class SchemaAwareApplyOperation extends ModificationApplyOperation { "Supplied %s does not belongs to configuration tree.", schemaNode.getPath()); } if (schemaNode instanceof ContainerSchemaNode) { - final ContainerSchemaNode containerSchema = (ContainerSchemaNode) schemaNode; - if (containerSchema.isPresenceContainer()) { - return new PresenceContainerModificationStrategy(containerSchema, treeConfig); - } - - return new StructuralContainerModificationStrategy(containerSchema, treeConfig); + return ContainerModificationStrategy.of((ContainerSchemaNode) schemaNode, treeConfig); } else if (schemaNode instanceof ListSchemaNode) { return fromListSchemaNode((ListSchemaNode) schemaNode, treeConfig); } else if (schemaNode instanceof ChoiceSchemaNode) {