From 15184a255f23d9ce0387fd000235e781d9d02d94 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 8 Apr 2023 19:49:57 +0200 Subject: [PATCH] Move substatement ordering to yang-parser-reactor Substatement ordering should be something reactor handles, hence move it out to make sure StatementFactory does not have to deal with this, making the API towards StatementFactory more consistent. Change-Id: I9106e04c22227f054be23cf4ace492ffe416ff98 Signed-off-by: Robert Varga --- .../reactor/AbstractResumedStatement.java | 110 +++++++++++++++--- .../AbstractAugmentStatementSupport.java | 9 +- .../AbstractImplicitStatementSupport.java | 5 +- .../stmt/module/ModuleStatementSupport.java | 3 +- .../type/AbstractTypeStatementSupport.java | 5 +- .../spi/meta/AbstractStatementSupport.java | 95 ++------------- .../spi/meta/ForwardingStatementSupport.java | 6 +- .../parser/spi/meta/StatementFactory.java | 8 +- 8 files changed, 115 insertions(+), 126 deletions(-) diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java index 501eb2bcc0..5d219160f6 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java @@ -8,10 +8,14 @@ package org.opendaylight.yangtools.yang.parser.stmt.reactor; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Verify.verify; import static com.google.common.base.Verify.verifyNotNull; +import com.google.common.base.VerifyException; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -45,10 +49,10 @@ abstract class AbstractResumedStatement, E ext // Copy constructor AbstractResumedStatement(final AbstractResumedStatement original) { super(original); - this.rawArgument = original.rawArgument; - this.substatements = original.substatements; - this.declaredInstance = original.declaredInstance; - this.fullyDefined = original.fullyDefined; + rawArgument = original.rawArgument; + substatements = original.substatements; + declaredInstance = original.declaredInstance; + fullyDefined = original.fullyDefined; } AbstractResumedStatement(final StatementDefinitionContext def, final StatementSourceReference ref, @@ -74,10 +78,12 @@ abstract class AbstractResumedStatement, E ext } private @NonNull D loadDeclared() { - final ModelProcessingPhase phase = getCompletedPhase(); - checkState(phase == ModelProcessingPhase.FULL_DECLARATION || phase == ModelProcessingPhase.EFFECTIVE_MODEL, - "Cannot build declared instance after phase %s", phase); - return declaredInstance = definition().getFactory().createDeclared(this, substatementsAsDeclared()); + final var phase = getCompletedPhase(); + return switch (phase) { + case FULL_DECLARATION, EFFECTIVE_MODEL -> + declaredInstance = definition().getFactory().createDeclared(this, substatementsAsDeclared()); + default -> throw new IllegalStateException("Cannot build declared instance after phase " + phase); + }; } @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -128,7 +134,72 @@ abstract class AbstractResumedStatement, E ext // the substatements may be gone by the time the factory attempts to acquire the declared statement. ctx.declared(); - return factory.createEffective(ctx, declared, effective); + return factory.createEffective(ctx, orderSubstatements(declared, effective)); + } + + private static @NonNull Stream> orderSubstatements( + final Stream> declaredSubstatements, + final Stream> effectiveSubstatements) { + /* + * This dance is required to ensure that effects of 'uses' nodes are applied in the same order as + * the statements were defined -- i.e. if we have something like this: + * + * container foo { + * uses bar; + * uses baz; + * } + * + * grouping baz { + * leaf baz { + * type string; + * } + * } + * + * grouping bar { + * leaf bar { + * type string; + * } + * } + * + * The reactor would first inline 'uses baz' as that definition is the first one completely resolved and then + * inline 'uses bar'. Here we are iterating in declaration order re-inline the statements. + * + * FIXME: 7.0.0: this really should be handled by UsesStatementSupport such that 'uses baz' would have a + * prerequisite of a resolved 'uses bar'. + */ + final var declaredInit = declaredSubstatements + .filter(StmtContext::isSupportedByFeatures) + .collect(Collectors.toList()); + + final var substatementsInit = new ArrayList>(); + Set> filteredStatements = null; + for (var declaredSubstatement : declaredInit) { + substatementsInit.add(declaredSubstatement); + + // FIXME: YANGTOOLS-1161: we need to integrate this functionality into the reactor, so that this + // transformation is something reactor's declared statements already take into + // account. + final Collection> effect = declaredSubstatement.getEffectOfStatement(); + if (!effect.isEmpty()) { + if (filteredStatements == null) { + filteredStatements = new HashSet<>(); + } + filteredStatements.addAll(effect); + // Note: we need to filter here to exclude unsupported statements + effect.stream().filter(StmtContext::isSupportedToBuildEffective).forEach(substatementsInit::add); + } + } + + final Stream> effective; + if (filteredStatements != null) { + final var filtered = filteredStatements; + effective = effectiveSubstatements.filter(stmt -> !filtered.contains(stmt)); + } else { + effective = effectiveSubstatements; + } + + substatementsInit.addAll(effective.collect(Collectors.toList())); + return substatementsInit.stream(); } @Override @@ -155,7 +226,7 @@ abstract class AbstractResumedStatement, E ext AbstractResumedStatement createSubstatement(final int offset, final StatementDefinitionContext def, final StatementSourceReference ref, final String argument) { - final ModelProcessingPhase inProgressPhase = getRoot().getSourceContext().getInProgressPhase(); + final var inProgressPhase = getRoot().getSourceContext().getInProgressPhase(); checkState(inProgressPhase != ModelProcessingPhase.EFFECTIVE_MODEL, "Declared statement cannot be added in effective phase at: %s", sourceReference()); @@ -217,8 +288,11 @@ abstract class AbstractResumedStatement, E ext private static @NonNull AbstractResumedStatement unmaskUndeclared(final ReactorStmtCtx stmt) { var ret = stmt; while (!(ret instanceof AbstractResumedStatement resumed)) { - verify(ret instanceof UndeclaredStmtCtx, "Unexpectred statement %s", ret); - ret = ((UndeclaredStmtCtx) ret).getResumedSubstatement(); + if (ret instanceof UndeclaredStmtCtx undeclared) { + ret = undeclared.getResumedSubstatement(); + } else { + throw new VerifyException("Unexpected statement " + ret); + } } return resumed; } @@ -238,19 +312,21 @@ abstract class AbstractResumedStatement, E ext var ret = verifyParent(parent); // Unwind all undeclared statements - while (!(ret instanceof AbstractResumedStatement)) { + while (!(ret instanceof AbstractResumedStatement resumed)) { ret.finishDeclaration(phase); ret = verifyParent(ret.getParentContext()); } - return (AbstractResumedStatement) ret; + return resumed; } // FIXME: AbstractResumedStatement should only ever have OriginalStmtCtx parents, which would remove the need for // this method. In ordered to do that we need to untangle SubstatementContext's users and do not allow it // being reparent()ed. private static OriginalStmtCtx verifyParent(final StatementContextBase parent) { - verify(parent instanceof OriginalStmtCtx, "Unexpected parent context %s", parent); - return (OriginalStmtCtx) parent; + if (parent instanceof OriginalStmtCtx original) { + return original; + } + throw new VerifyException("Unexpected parent context " + parent); } final void resizeSubstatements(final int expectedSize) { diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java index 2de5f0f638..7e53180ae2 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java @@ -8,9 +8,8 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.augment; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; -import java.util.List; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.opendaylight.yangtools.yang.common.Empty; import org.opendaylight.yangtools.yang.model.api.Status; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; @@ -118,14 +117,14 @@ abstract class AbstractAugmentStatementSupport } @Override - protected final List> statementsToBuild( + protected final Stream> statementsToBuild( final Current stmt, - final List> substatements) { + final Stream> substatements) { // Pick up the marker left by onFullDefinitionDeclared() inference action. If it is present we need to pass our // children through target's implicit wrapping. final var implicitDef = stmt.namespaceItem(AugmentImplicitHandlingNamespace.INSTANCE, Empty.value()); return implicitDef == null ? substatements - : Lists.transform(substatements, subCtx -> implicitDef.wrapWithImplicit(subCtx)); + : substatements.map(subCtx -> implicitDef.wrapWithImplicit(subCtx)); } @Override diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/AbstractImplicitStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/AbstractImplicitStatementSupport.java index 45bc4aad89..0597cea5aa 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/AbstractImplicitStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/AbstractImplicitStatementSupport.java @@ -10,7 +10,6 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.meta; import static java.util.Objects.requireNonNull; import com.google.common.collect.ImmutableList; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.QName; @@ -44,9 +43,7 @@ abstract class AbstractImplicitStatementSupport stmt, final @NonNull Stream> effectiveSubstatements) { return createUndeclaredEffective(stmt, buildEffectiveSubstatements(stmt, - statementsToBuild(stmt, effectiveSubstatements - .filter(StmtContext::isSupportedToBuildEffective) - .collect(Collectors.toUnmodifiableList())))); + statementsToBuild(stmt, effectiveSubstatements.filter(StmtContext::isSupportedToBuildEffective)))); } abstract @NonNull E createUndeclaredEffective(@NonNull UndeclaredCurrent stmt, diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStatementSupport.java index 44e7705723..b93e461898 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStatementSupport.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.Empty; import org.opendaylight.yangtools.yang.common.QNameModule; @@ -184,7 +185,7 @@ public final class ModuleStatementSupport @Override protected ImmutableList> buildEffectiveSubstatements( final Current stmt, - final List> substatements) { + final Stream> substatements) { final ImmutableList> local = super.buildEffectiveSubstatements(stmt, substatements); final Collection> submodules = submoduleContexts(stmt); diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/type/AbstractTypeStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/type/AbstractTypeStatementSupport.java index 0ce6317223..5458b668fe 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/type/AbstractTypeStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/type/AbstractTypeStatementSupport.java @@ -12,7 +12,6 @@ import static com.google.common.base.Verify.verifyNotNull; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.util.Collection; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.Decimal64; @@ -188,9 +187,7 @@ abstract class AbstractTypeStatementSupport extends AbstractTypeSupport stmt, final Stream> effectiveSubstatements) { final ImmutableList> substatements = buildEffectiveSubstatements(stmt, - statementsToBuild(stmt, effectiveSubstatements - .filter(StmtContext::isSupportedToBuildEffective) - .collect(Collectors.toUnmodifiableList()))); + statementsToBuild(stmt, effectiveSubstatements.filter(StmtContext::isSupportedToBuildEffective))); // First look up the proper base type final TypeEffectiveStatement typeStmt = resolveType(stmt); diff --git a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java index b0ba945f29..c774f38b1f 100644 --- a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java +++ b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java @@ -9,12 +9,7 @@ package org.opendaylight.yangtools.yang.parser.spi.meta; import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableList; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -42,8 +37,8 @@ public abstract class AbstractStatementSupport protected AbstractStatementSupport(final StatementDefinition publicDefinition, final StatementPolicy policy, final YangParserConfiguration config, final @Nullable SubstatementValidator validator) { super(publicDefinition, policy); - this.retainDeclarationReference = config.retainDeclarationReferences(); - this.substatementValidator = validator; + retainDeclarationReference = config.retainDeclarationReferences(); + substatementValidator = validator; } @Override @@ -61,7 +56,7 @@ public abstract class AbstractStatementSupport @NonNull ImmutableList> substatements); private @NonNull D attachDeclarationReference(final @NonNull D stmt, final @NonNull BoundStmtCtx ctx) { - final DeclarationReference ref = ctx.sourceReference().declarationReference(); + final var ref = ctx.sourceReference().declarationReference(); return ref == null ? stmt : attachDeclarationReference(stmt, ref); } @@ -76,13 +71,8 @@ public abstract class AbstractStatementSupport protected abstract @NonNull D attachDeclarationReference(@NonNull D stmt, @NonNull DeclarationReference reference); @Override - public final E createEffective(final Current stmt, - final Stream> declaredSubstatements, - final Stream> inferredSubstatements) { - final ImmutableList> substatements = - buildEffectiveSubstatements(stmt, statementsToBuild(stmt, - declaredSubstatements(declaredSubstatements, inferredSubstatements))); - return createEffective(stmt, substatements); + public final E createEffective(final Current stmt, final Stream> substatements) { + return createEffective(stmt, buildEffectiveSubstatements(stmt, statementsToBuild(stmt, substatements))); } protected abstract @NonNull E createEffective(@NonNull Current stmt, @@ -102,15 +92,15 @@ public abstract class AbstractStatementSupport * @param substatements Substatement contexts which have been determined to be built * @return Substatement context which are to be actually built */ - protected List> statementsToBuild(final Current ctx, - final List> substatements) { + protected Stream> statementsToBuild(final Current ctx, + final Stream> substatements) { return substatements; } // FIXME: add documentation public static final > @Nullable E findFirstStatement( final Collection> statements, final Class type) { - for (EffectiveStatement stmt : statements) { + for (var stmt : statements) { if (type.isInstance(stmt)) { return type.cast(stmt); } @@ -135,72 +125,7 @@ public abstract class AbstractStatementSupport * @return Built effective substatements */ protected @NonNull ImmutableList> buildEffectiveSubstatements( - final Current stmt, final List> substatements) { - return substatements.stream().map(StmtContext::buildEffective).collect(ImmutableList.toImmutableList()); - } - - private static @NonNull List> declaredSubstatements( - final Stream> declaredSubstatements, - final Stream> effectiveSubstatements) { - /* - * This dance is required to ensure that effects of 'uses' nodes are applied in the same order as - * the statements were defined -- i.e. if we have something like this: - * - * container foo { - * uses bar; - * uses baz; - * } - * - * grouping baz { - * leaf baz { - * type string; - * } - * } - * - * grouping bar { - * leaf bar { - * type string; - * } - * } - * - * The reactor would first inline 'uses baz' as that definition is the first one completely resolved and then - * inline 'uses bar'. Here we are iterating in declaration order re-inline the statements. - * - * FIXME: 7.0.0: this really should be handled by UsesStatementSupport such that 'uses baz' would have a - * prerequisite of a resolved 'uses bar'. - */ - final List> declaredInit = declaredSubstatements - .filter(StmtContext::isSupportedByFeatures) - .collect(Collectors.toList()); - - final List> substatementsInit = new ArrayList<>(); - Set> filteredStatements = null; - for (final StmtContext declaredSubstatement : declaredInit) { - substatementsInit.add(declaredSubstatement); - - // FIXME: YANGTOOLS-1161: we need to integrate this functionality into the reactor, so that this - // transformation is something reactor's declared statements already take into - // account. - final Collection> effect = declaredSubstatement.getEffectOfStatement(); - if (!effect.isEmpty()) { - if (filteredStatements == null) { - filteredStatements = new HashSet<>(); - } - filteredStatements.addAll(effect); - // Note: we need to filter here to exclude unsupported statements - effect.stream().filter(StmtContext::isSupportedToBuildEffective).forEach(substatementsInit::add); - } - } - - final Stream> effective; - if (filteredStatements != null) { - final Set> filtered = filteredStatements; - effective = effectiveSubstatements.filter(stmt -> !filtered.contains(stmt)); - } else { - effective = effectiveSubstatements; - } - - substatementsInit.addAll(effective.collect(Collectors.toList())); - return substatementsInit; + final Current stmt, final Stream> substatements) { + return substatements.map(StmtContext::buildEffective).collect(ImmutableList.toImmutableList()); } } diff --git a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java index 40ffb4effd..931df13f36 100644 --- a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java +++ b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java @@ -40,10 +40,8 @@ public abstract class ForwardingStatementSupport stmt, - final Stream> declaredSubstatements, - final Stream> inferredSubstatements) { - return delegate.createEffective(stmt, declaredSubstatements, inferredSubstatements); + public E createEffective(final Current stmt, final Stream> substatements) { + return delegate.createEffective(stmt, substatements); } @Override diff --git a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementFactory.java b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementFactory.java index f35d34c302..93976f5481 100644 --- a/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementFactory.java +++ b/parser/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementFactory.java @@ -37,14 +37,10 @@ public interface StatementFactory, E extends E * Create a {@link EffectiveStatement} for specified context. * * @param stmt Effective capture of this statement's significant state - * @param declaredSubstatements effectively-visible declared substatements - * @param inferredSubstatements effectively-visible inferred substatements + * @param substatements effectively-visible substatements * @return An effective statement instance */ - // FIXME: we really want a single coherent 'effectiveSubstatements' stream - @NonNull E createEffective(@NonNull Current stmt, - Stream> declaredSubstatements, - Stream> inferredSubstatements); + @NonNull E createEffective(@NonNull Current stmt, @NonNull Stream> substatements); /** * Create a {@link EffectiveStatement} copy of provided original for specified context. -- 2.36.6