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 <robert.varga@pantheon.tech>

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 f25b5452da210550d8719c609a69b9d83774502a..9dd646c8853a7e6e8965bb79bec41b4b27db0425 100644 (file)
--- 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<QName, ActionStatement> stmt,
             final ImmutableList<? extends EffectiveStatement<?, ?>> 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 3b7b532b836054af2ac65f404be0b72a62872c87..1acdf04e6ed954ab8d5ff92cf6368a941c2382c5 100644 (file)
--- 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<QName, NotificationStatement> stmt,
             final ImmutableList<? extends EffectiveStatement<?, ?>> 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<QName, NotificationStatement> 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 69e96e2934c87a62cf5d4ec3e21240ce29b386a0..f85965fb9373ef0c9edf56dc1e363ce86f91b369 100644 (file)
--- 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<QName, NotificationStatement> 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 70b5e9a8cf79cdf99719cdf237ae82dc97aacc5d..173d1d6c1ca94b5110d475f55c1796d2dc31ce1c 100644 (file)
--- 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<QName, NotificationStatement> 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 fbc38cfac7d043ba96bb757dd42c710d5a842abe..951275b30c3a7d95406a258757ecc5db7ec9d8fd 100644 (file)
--- 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 <A, D extends DeclaredStatement<A>> 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<? extends EffectiveStatement<?, ?>> repr = ancestorChildType.getEffectiveRepresentationClass();
-        StmtContext<?, ?, ?> current = stmt.caerbannog().getParentContext();
-        StmtContext<?, ?, ?> parent = current.getParentContext();
+        // We do not expect this to by typically populated
+        final List<Mutable<?, ?, ?>> 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<? extends Prerequisite<?>> failed) {
+                    throw new VerifyException("Should never happen");
+                }
+            });
+        }
     }
 
     /**