From a36fd793d86133286a46bf277f90c9a52f74b992 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 14 Jan 2021 13:04:57 +0100 Subject: [PATCH] Move list/key checks to onStatementAdded() We do not want to use the rabbit hole to StmtContext during effective build, but rather perform the check as soon as the statement is added. Depending on how the model is structured action/notification declaration might happen before the key's presence is established and therefore we perform an eager check first, but pay attention to ancestor's state. If we encounter an ancestor which has not completed FULL_DECLARATION, we hook an inference check to run just before it does. JIRA: YANGTOOLS-1186 Change-Id: I8d8871a0eae860ba2327d05c43355a7ee3ffd382 Signed-off-by: Robert Varga --- .../stmt/action/ActionStatementSupport.java | 9 +-- .../AbstractNotificationStatementSupport.java | 5 -- .../NotificationStatementRFC6020Support.java | 8 --- .../NotificationStatementRFC7950Support.java | 10 +-- .../parser/spi/meta/StmtContextUtils.java | 61 ++++++++++++++----- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/action/ActionStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/action/ActionStatementSupport.java index f25b5452da..9dd646c885 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/action/ActionStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/action/ActionStatementSupport.java @@ -7,7 +7,6 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.action; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; import com.google.common.collect.ImmutableList; @@ -70,6 +69,8 @@ public final class ActionStatementSupport extends "Action %s is defined within a case statement", argument); SourceException.throwIf(StmtContextUtils.hasParentOfType(stmt, YangStmtMapping.MODULE), stmt, "Action %s is defined at the top level of a module", stmt.getArgument()); + StmtContextUtils.validateNoKeylessListAncestorOf(stmt, "Action"); + super.onStatementAdded(stmt); } @@ -108,11 +109,7 @@ public final class ActionStatementSupport extends protected ActionEffectiveStatement createEffective(final Current stmt, final ImmutableList> substatements) { final StatementSourceReference ref = stmt.sourceReference(); - checkState(!substatements.isEmpty(), "Missing implicit input/output statements at %s", ref); - final QName argument = stmt.getArgument(); - SourceException.throwIf( - !StmtContextUtils.hasAncestorOfTypeWithChildOfType(stmt, YangStmtMapping.LIST, YangStmtMapping.KEY), stmt, - "Action %s is defined within a list that has no key statement", argument); + verify(!substatements.isEmpty(), "Missing implicit input/output statements at %s", ref); try { return new ActionEffectiveStatementImpl(stmt.declared(), stmt.wrapSchemaPath(), diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/AbstractNotificationStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/AbstractNotificationStatementSupport.java index 3b7b532b83..1acdf04e6e 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/AbstractNotificationStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/AbstractNotificationStatementSupport.java @@ -40,8 +40,6 @@ abstract class AbstractNotificationStatementSupport @Override protected final NotificationEffectiveStatement createEffective(final Current stmt, final ImmutableList> substatements) { - checkEffective(stmt); - try { return new NotificationEffectiveStatementImpl(stmt.declared(), substatements, historyAndStatusFlags(stmt.history(), substatements), stmt.wrapSchemaPath()); @@ -49,7 +47,4 @@ abstract class AbstractNotificationStatementSupport throw new SourceException(e.getMessage(), stmt, e); } } - - // FIXME: remove this method - abstract void checkEffective(Current stmt); } \ No newline at end of file diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC6020Support.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC6020Support.java index 69e96e2934..f85965fb93 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC6020Support.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC6020Support.java @@ -7,10 +7,7 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.notification; -import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; -import org.opendaylight.yangtools.yang.model.api.stmt.NotificationStatement; -import org.opendaylight.yangtools.yang.parser.spi.meta.EffectiveStmtCtx.Current; import org.opendaylight.yangtools.yang.parser.spi.meta.SubstatementValidator; public final class NotificationStatementRFC6020Support extends AbstractNotificationStatementSupport { @@ -44,9 +41,4 @@ public final class NotificationStatementRFC6020Support extends AbstractNotificat protected SubstatementValidator getSubstatementValidator() { return SUBSTATEMENT_VALIDATOR; } - - @Override - void checkEffective(final Current stmt) { - // No-op - } } \ No newline at end of file diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC7950Support.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC7950Support.java index 70b5e9a8cf..173d1d6c1c 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC7950Support.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/notification/NotificationStatementRFC7950Support.java @@ -14,7 +14,6 @@ import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; import org.opendaylight.yangtools.yang.model.api.stmt.NotificationEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.NotificationStatement; -import org.opendaylight.yangtools.yang.parser.spi.meta.EffectiveStmtCtx.Current; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; import org.opendaylight.yangtools.yang.parser.spi.meta.SubstatementValidator; @@ -64,6 +63,7 @@ public final class NotificationStatementRFC7950Support extends AbstractNotificat "Notification %s is defined within an rpc, action, or another notification", argument); SourceException.throwIf(StmtContextUtils.hasParentOfType(stmt, YangStmtMapping.CASE), stmt, "Notification %s is defined within a case statement", argument); + StmtContextUtils.validateNoKeylessListAncestorOf(stmt, "Notification"); super.onStatementAdded(stmt); } @@ -72,12 +72,4 @@ public final class NotificationStatementRFC7950Support extends AbstractNotificat protected SubstatementValidator getSubstatementValidator() { return SUBSTATEMENT_VALIDATOR; } - - @Override - void checkEffective(final Current stmt) { - final QName argument = stmt.argument(); - SourceException.throwIf( - !StmtContextUtils.hasAncestorOfTypeWithChildOfType(stmt, YangStmtMapping.LIST, YangStmtMapping.KEY), stmt, - "Notification %s is defined within a list that has no key statement", argument); - } } diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StmtContextUtils.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StmtContextUtils.java index fbc38cfac7..951275b30c 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StmtContextUtils.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StmtContextUtils.java @@ -11,8 +11,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import com.google.common.base.Strings; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -22,9 +25,9 @@ import org.opendaylight.yangtools.yang.common.Revision; import org.opendaylight.yangtools.yang.common.YangVersion; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; 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.model.api.stmt.BelongsToStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.KeyEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.KeyStatement; import org.opendaylight.yangtools.yang.model.api.stmt.LeafStatement; import org.opendaylight.yangtools.yang.model.api.stmt.MandatoryStatement; @@ -35,6 +38,10 @@ import org.opendaylight.yangtools.yang.model.api.stmt.RevisionStatement; import org.opendaylight.yangtools.yang.model.api.stmt.SubmoduleStatement; import org.opendaylight.yangtools.yang.model.api.stmt.UnknownStatement; import org.opendaylight.yangtools.yang.model.api.stmt.UnrecognizedStatement; +import org.opendaylight.yangtools.yang.parser.spi.meta.ModelActionBuilder.InferenceAction; +import org.opendaylight.yangtools.yang.parser.spi.meta.ModelActionBuilder.InferenceContext; +import org.opendaylight.yangtools.yang.parser.spi.meta.ModelActionBuilder.Prerequisite; +import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; import org.opendaylight.yangtools.yang.parser.spi.source.BelongsToPrefixToModuleName; import org.opendaylight.yangtools.yang.parser.spi.source.ImportPrefixToModuleCtx; import org.opendaylight.yangtools.yang.parser.spi.source.ModuleCtxToModuleQName; @@ -354,32 +361,56 @@ public final class StmtContextUtils { } /** - * Checks whether all of StmtContext's ancestors of specified type have a child of specified type. + * Check whether all of StmtContext's {@code list} ancestors have a {@code key}. * * @param stmt EffectiveStmtCtx to be checked - * @param ancestorType type of ancestor to search for - * @param ancestorChildType type of child to search for in the specified ancestor type - * @return true if all of StmtContext's ancestors of specified type have a child of specified type, otherwise false + * @param name Human-friendly statement name + * @throws SourceException if there is any keyless list ancestor */ - public static > boolean hasAncestorOfTypeWithChildOfType( - final EffectiveStmtCtx.Current stmt, final StatementDefinition ancestorType, - final StatementDefinition ancestorChildType) { + public static void validateNoKeylessListAncestorOf(final Mutable stmt, final String name) { requireNonNull(stmt); - requireNonNull(ancestorType); - final Class> repr = ancestorChildType.getEffectiveRepresentationClass(); - StmtContext current = stmt.caerbannog().getParentContext(); - StmtContext parent = current.getParentContext(); + // We do not expect this to by typically populated + final List> incomplete = new ArrayList<>(0); + + Mutable current = stmt.coerceParentContext(); + Mutable parent = current.getParentContext(); while (parent != null) { - if (ancestorType.equals(current.publicDefinition()) && !current.hasSubstatement(repr)) { - return false; + if (YangStmtMapping.LIST == current.publicDefinition() + && !current.hasSubstatement(KeyEffectiveStatement.class)) { + if (ModelProcessingPhase.FULL_DECLARATION.isCompletedBy(current.getCompletedPhase())) { + throw new SourceException(stmt, "%s %s is defined within a list that has no key statement", name, + stmt.argument()); + } + + // Ancestor has not completed full declaration yet missing 'key' statement may materialize later + incomplete.add(current); } current = parent; parent = current.getParentContext(); } - return true; + // Deal with whatever incomplete ancestors we encountered + for (Mutable ancestor : incomplete) { + // This check must complete during the ancestor's FULL_DECLARATION phase, i.e. the ancestor must not reach + // EFFECTIVE_MODEL until it is done. + final ModelActionBuilder action = ancestor.newInferenceAction(ModelProcessingPhase.FULL_DECLARATION); + action.apply(new InferenceAction() { + @Override + public void apply(final InferenceContext ctx) { + if (!ancestor.hasSubstatement(KeyEffectiveStatement.class)) { + throw new SourceException(stmt, "%s %s is defined within a list that has no key statement", + name, stmt.argument()); + } + } + + @Override + public void prerequisiteFailed(final Collection> failed) { + throw new VerifyException("Should never happen"); + } + }); + } } /** -- 2.36.6