From: Robert Varga Date: Sun, 2 Feb 2020 08:55:20 +0000 (+0100) Subject: Move copy operation execution X-Git-Tag: v4.0.7~27 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F00%2F87400%2F1;p=yangtools.git Move copy operation execution Statements should be reporting the requirements of their effective statement implementations, such that reactor can efficiently shuffle StatementContextBase implementations to reflect StmtContext view of the world. JIRA: YANGTOOLS-694 Change-Id: Ida8eff88276be523766999f4f3528afbc2a38d59 Signed-off-by: Robert Varga --- diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java index 32f546d618..ca97cfd04e 100644 --- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java +++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java @@ -64,6 +64,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.NamespaceKeyCriterion; import org.opendaylight.yangtools.yang.parser.spi.meta.NamespaceNotAvailableException; import org.opendaylight.yangtools.yang.parser.spi.meta.StatementNamespace; import org.opendaylight.yangtools.yang.parser.spi.meta.StatementSupport; +import org.opendaylight.yangtools.yang.parser.spi.meta.StatementSupport.CopyPolicy; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; @@ -769,7 +770,31 @@ public abstract class StatementContextBase, E public Optional> copyAsChildOf(final Mutable parent, final CopyType type, final QNameModule targetModule) { checkEffectiveModelCompleted(this); - return definition.support().copyAsChildOf(this, parent, type, targetModule); + + final StatementSupport support = definition.support(); + final CopyPolicy policy = support.applyCopyPolicy(this, parent, type, targetModule); + switch (policy) { + case CONTEXT_INDEPENDENT: + // FIXME: YANGTOOLS-652: we need isEmpty() here for performance reasons + if (allSubstatementsStream().findAny().isEmpty()) { + // This statement is context-independent and has no substatements -- hence it can be freely shared. + return Optional.of(this); + } + // FIXME: YANGTOOLS-694: filter out all context-independent substatements, eliminate fall-through + // fall through + case DECLARED_COPY: + // FIXME: YANGTOOLS-694: this is still to eager, we really want to copy as a lazily-instantiated + // context, so that we can support building an effective statement without copying + // anything -- we will typically end up not being inferred against. In that case, + // this slim context should end up dealing with differences at buildContext() + // time. This is a YANGTOOLS-1067 prerequisite (which will deal with what can and + // cannot be shared across instances). + return Optional.of(parent.childCopyOf(this, type, targetModule)); + case IGNORE: + return Optional.empty(); + default: + throw new IllegalStateException("Unhandled policy " + policy); + } } @Override diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java index 9c7584009e..807fc49406 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/uses/UsesStatementSupport.java @@ -8,7 +8,7 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.uses; import com.google.common.base.Verify; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableMap; import java.util.ArrayList; import java.util.Collection; import java.util.Optional; @@ -159,12 +159,28 @@ public final class UsesStatementSupport usesNode.addAsEffectOfStatement(buffer); } - // FIXME: YANGTOOLS-652: this map looks very much like InferredStatementContext.REUSED_DEF_SET - // FIXME: YANGTOOLS-694: we should take advantage of CopyPolicy - private static final ImmutableSet TOP_REUSED_DEF_SET = ImmutableSet.of( - YangStmtMapping.TYPE, YangStmtMapping.TYPEDEF); - private static final ImmutableSet DOCUMENTATION_STATEMENTS = ImmutableSet.of( - YangStmtMapping.DESCRIPTION, YangStmtMapping.REFERENCE, YangStmtMapping.STATUS); + // Note: do not put CopyPolicy.DECLARED_COPY, we treat non-presence as such + private static final ImmutableMap GROUPING_TO_TARGET_POLICY = + ImmutableMap.builder() + // FIXME: YANGTOOLS-652: this map looks very much like InferredStatementContext.REUSED_DEF_SET + // a grouping's type/typedef statements are fully defined at the grouping, we want the same effective + // statement + .put(YangStmtMapping.TYPE, CopyPolicy.CONTEXT_INDEPENDENT) + .put(YangStmtMapping.TYPEDEF, CopyPolicy.CONTEXT_INDEPENDENT) + + // We do not want to propagate description/reference/status statements, as they are the source + // grouping's documentation, not target statement's + .put(YangStmtMapping.DESCRIPTION, CopyPolicy.IGNORE) + .put(YangStmtMapping.REFERENCE, CopyPolicy.IGNORE) + .put(YangStmtMapping.STATUS, CopyPolicy.IGNORE) + + // Do not propagate uses, as their effects have been accounted for in effective statements + // FIXME: YANGTOOLS-652: this check is different from InferredStatementContext. Why is that? We should + // express a common condition in our own implementation of applyCopyPolicy() -- + // most notably reactor performs the equivalent of CopyPolicy.CONTEXT_INDEPENDENT. + // FIXME: YANGTOOLS-694: note that if the above is true, why are we propagating grouping statements? + .put(YangStmtMapping.USES, CopyPolicy.IGNORE) + .build(); private static void copyStatement(final Mutable original, final StatementContextBase targetCtx, final QNameModule targetModule, @@ -181,27 +197,16 @@ public final class UsesStatementSupport // buildEffective() I think) is actually a SchemaTreeEffectiveStatement -- i.e. if it // is not a SchemaTreeEffectiveStatement, it just cannot be added to target's tree and // hence it should not be copied. - - final StatementDefinition def = original.getPublicDefinition(); - if (DOCUMENTATION_STATEMENTS.contains(def)) { - // We do not want to propagate description/reference/status statements, as they are the source grouping's - // documentation, not target statement's - return; - } - if (TOP_REUSED_DEF_SET.contains(def)) { - // a grouping's type/typedef statements are fully defined at the grouping, we want the same effective - // statement + final CopyPolicy policy = GROUPING_TO_TARGET_POLICY.get(original.getPublicDefinition()); + if (policy == null) { + original.copyAsChildOf(targetCtx, CopyType.ADDED_BY_USES, targetModule).ifPresent(buffer::add); + } else if (policy == CopyPolicy.CONTEXT_INDEPENDENT) { buffer.add(original); - return; - } - if (YangStmtMapping.USES.equals(def)) { - // Do not propagate uses, as their effects have been accounted for in effective statements - // FIXME: YANGTOOLS-652: this check is different from InferredStatementContext. Why is that? We should - // express a common condition in our own implementation of applyCopyPolicy() - // FIXME: YANGTOOLS-694: note that if the above is true, why are we propagating grouping statements? - return; + } else if (policy == CopyPolicy.IGNORE) { + // No-op + } else { + throw new IllegalStateException("Unhandled policy " + policy); } - original.copyAsChildOf(targetCtx, CopyType.ADDED_BY_USES, targetModule).ifPresent(buffer::add); } private static QNameModule getNewQNameModule(final StmtContext targetCtx, diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java index 1318af957b..d8e9c42ee3 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java @@ -10,14 +10,11 @@ package org.opendaylight.yangtools.yang.parser.spi.meta; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; -import java.util.Optional; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; -import org.opendaylight.yangtools.yang.common.QNameModule; import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; -import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; /** * Class providing necessary support for processing a YANG statement. This class is intended to be subclassed @@ -110,21 +107,6 @@ public abstract class AbstractStatementSupport return null; } - @Override - public Optional> copyAsChildOf(final Mutable stmt, - final Mutable parent, final CopyType copyType, final QNameModule targetModule) { - // Most of statement supports will just want to copy the statement - // FIXME: YANGTOOLS-694: that is not strictly true. Subclasses of this should indicate if they are themselves - // copy-sensitive: - // 1) if they are not and cannot be targeted by inference, and all their current - // substatements are also non-sensitive, we want to return the same context. - // 2) if they are not and their current substatements are sensitive, we want to copy - // as a lazily-instantiated interceptor to let it deal with substatements when needed - // (YANGTOOLS-1067 prerequisite) - // 3) otherwise perform this eager copy - return Optional.of(parent.childCopyOf(stmt, copyType, targetModule)); - } - /** * Returns corresponding substatement validator of a statement support. * diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java index 45ab5b20d6..b8972cf686 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java @@ -9,7 +9,6 @@ package org.opendaylight.yangtools.yang.parser.spi.meta; import com.google.common.annotations.Beta; import com.google.common.collect.ForwardingObject; -import java.util.Optional; import org.opendaylight.yangtools.yang.common.QNameModule; import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; @@ -89,8 +88,8 @@ public abstract class ForwardingStatementSupport> copyAsChildOf(final Mutable stmt, + public CopyPolicy applyCopyPolicy(final Mutable stmt, final Mutable parent, final CopyType type, final QNameModule targetModule) { - return delegate().copyAsChildOf(stmt, parent, type, targetModule); + return delegate().applyCopyPolicy(stmt, parent, type, targetModule); } } diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java index fb30a4a612..7aee38118c 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java @@ -141,23 +141,31 @@ public interface StatementSupport, E extends E @Nullable StatementSupport getSupportSpecificForArgument(String argument); /** - * Create an optional copy of specified statement as a substatement of parent. - * - *

- * Note that while it may be tempting to return the same context, this is not safe in general case. It is only safe - * if the entire subtree is unaffected by changes to parent/namespace/history. This includes the semantics of this - * statement (it cannot be a target of any inference effects) as well as any substatements -- an extension statement - * is allowed pretty much anywhere and if its semantics are context-dependent, a simple instance reuse will not - * work. + * Determine reactor copy behavior of a statement instance. Statement support classes are required to determine + * their operations with regard to their statements being replicated into different contexts, so that + * {@link Mutable} instances are not created when it is evident they are superfluous. * * @param stmt Context of statement to be copied statement. * @param parent Parent statement context * @param type Type of copy being performed * @param targetModule Target module, if present - * @return Empty if the statement should be ignored, or present with an instance that should be copied into parent. + * @return Policy that needs to be applied to the copy operation of this statement. */ - @NonNull Optional> copyAsChildOf(Mutable stmt, Mutable parent, - CopyType type, @Nullable QNameModule targetModule); + // FIXME: YANGTOOLS-694: clarify targetModule semantics (does null mean 'same as declared'?) + default @NonNull CopyPolicy applyCopyPolicy(final Mutable stmt, final Mutable parent, + final CopyType type, @Nullable final QNameModule targetModule) { + // Most of statement supports will just want to copy the statement + // FIXME: YANGTOOLS-694: that is not strictly true. Subclasses of this should indicate if they are themselves + // copy-sensitive: + // 1) if they are not and cannot be targeted by inference, and all their current + // substatements are also non-sensitive, we want to return the same context. + // 2) if they are not and their current substatements are sensitive, we want to copy + // as a lazily-instantiated interceptor to let it deal with substatements when needed + // (YANGTOOLS-1067 prerequisite) + // 3) otherwise perform this eager copy + // return Optional.of(parent.childCopyOf(stmt, copyType, targetModule)); + return CopyPolicy.DECLARED_COPY; + } /** * Given a raw string representation of an argument, try to use a shared representation. @@ -225,4 +233,41 @@ public interface StatementSupport, E extends E default Class> getEffectiveRepresentationClass() { return getPublicView().getEffectiveRepresentationClass(); } + + /** + * Statement context copy policy, indicating how should reactor handle statement copy operations. Every statement + * copied by the reactor is subject to policy check done by + * {@link StatementSupport#applyCopyPolicy(Mutable, Mutable, CopyType, QNameModule)}. + * + */ + enum CopyPolicy { + /** + * Reuse the source statement context in the new place, as it cannot be affected by any further operations. This + * implies that the semantics of the effective statement are not affected by any of its substatements. Each + * of the substatements is free to make its own policy. + * + *

+ * This policy is typically used by static constant statements such as {@code description} or {@code length}, + * where the baseline RFC7950 does not allow any impact. A {@code description} could hold an extension statement + * in which case this interaction would come into play. Normal YANG will see empty substatements, so the reactor + * will be free to complete reuse the context. + * + *

+ * In case any substatement is of stronger policy, it is up to the reactor to handle correct handling of + * resulting subobjects. + */ + // TODO: does this mean source must have transitioned to ModelProcessingPhase.EFFECTIVE_MODEL? + CONTEXT_INDEPENDENT, + /** + * Create a copy sharing declared instance, but otherwise having a separate disconnected lifecycle. + */ + // TODO: will the copy transition to ModelProcessingPhase.FULL_DECLARATION or which phase? + DECLARED_COPY, + /** + * Ignore this statement's existence for the purposes of the new place -- it is not impacted. This guidance + * is left here for completeness, as it can have justifiable uses (but I can't think of any). Any substatements + * need to be ignored, too. + */ + IGNORE; + } }