Do not issue duplicate warnings for lists missing keys

We currently are issuing the config list warning for each instantiated
site, which ends up flooding our logs, as BGPCEP takes advantage of our
lenience -- leading to 3400+ warnings.

Make sure we flag each original list only once, suppressing other
instances of the violation. This presumably will provide enough guidance
while keeping the noise to a sane amount (~200 warnings).

JIRA: YANGTOOLS-1090
Change-Id: I2488ad7144c1827f7689d496669e4c97b62b0570
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/reactor/RFC7950Reactors.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/RFC7950Reactors.java
index 7c7cea8b7c693ee9ec5171a4895a36c91a8743b5..40c3f60ea89196aef913fcb864cc5e917d65f999 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/RFC7950Reactors.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/RFC7950Reactors.java
@@ -67,6 +67,7 @@ import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.leaf.LeafStatementSup
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.leaf_list.LeafListStatementRFC6020Support;
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.leaf_list.LeafListStatementRFC7950Support;
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.length.LengthStatementSupport;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.list.ConfigListWarningNamespace;
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.list.ListStatementRFC6020Support;
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.list.ListStatementRFC7950Support;
 import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.mandatory.MandatoryStatementSupport;
@@ -246,6 +247,7 @@ public final class RFC7950Reactors {
             .addVersionSpecificSupport(VERSION_1_1, GroupingStatementRFC7950Support.getInstance())
             .addVersionSpecificSupport(VERSION_1, ListStatementRFC6020Support.getInstance())
             .addVersionSpecificSupport(VERSION_1_1, ListStatementRFC7950Support.getInstance())
+            .addSupport(ConfigListWarningNamespace.BEHAVIOUR)
             .addSupport(UniqueStatementSupport.getInstance())
             .addVersionSpecificSupport(VERSION_1_1, ActionStatementSupport.getInstance())
             .addVersionSpecificSupport(VERSION_1, RpcStatementRFC6020Support.getInstance())

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/AbstractListStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/AbstractListStatementSupport.java
index c44a92bbc6fab05cde9c5e0279894dde843a6216..8d3d913f7efaeb4ed67cd0a28db43e554bf8d0c5 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/AbstractListStatementSupport.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/AbstractListStatementSupport.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.list;
 
+import static com.google.common.base.Verify.verify;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import java.util.ArrayList;
@@ -14,6 +16,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.ElementCountConstraint;
 import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
@@ -37,7 +40,6 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.InferenceException;
 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;
-import org.opendaylight.yangtools.yang.parser.spi.source.StatementSourceReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -75,7 +77,6 @@ abstract class AbstractListStatementSupport extends BaseQNameStatementSupport<Li
     protected final ListEffectiveStatement createEffective(
             final StmtContext<QName, ListStatement, ListEffectiveStatement> ctx,
             final ListStatement declared, final ImmutableList<? extends EffectiveStatement<?, ?>> substatements) {
-        final StatementSourceReference ref = ctx.getStatementSourceReference();
         final SchemaPath path = ctx.getSchemaPath().get();
         final ListSchemaNode original = (ListSchemaNode) ctx.getOriginalCtx().map(StmtContext::buildEffective)
                 .orElse(null);
@@ -92,8 +93,9 @@ abstract class AbstractListStatementSupport extends BaseQNameStatementSupport<Li
             }
             for (final QName keyQName : keyStmt.argument()) {
                 if (!possibleLeafQNamesForKey.contains(keyQName)) {
-                    throw new InferenceException(ref, "Key '%s' misses node '%s' in list '%s'",
-                        keyStmt.getDeclared().rawArgument(), keyQName.getLocalName(), ctx.getStatementArgument());
+                    throw new InferenceException(ctx.getStatementSourceReference(),
+                        "Key '%s' misses node '%s' in list '%s'", keyStmt.getDeclared().rawArgument(),
+                        keyQName.getLocalName(), ctx.getStatementArgument());
                 }
                 keyDefinitionInit.add(keyQName);
             }
@@ -112,9 +114,7 @@ abstract class AbstractListStatementSupport extends BaseQNameStatementSupport<Li
                     .equals(Ordering.USER))
                 .toFlags();
         if (configuration && keyDefinition.isEmpty() && isInstantied(ctx)) {
-            LOG.info("Configuration list {} does not define any keys in violation of RFC7950 section 7.8.2. While "
-                    + " this is fine with OpenDaylight, it can cause interoperability issues with other systems "
-                    + "[at {}]", ctx.getStatementArgument(), ref);
+            warnConfigList(ctx);
         }
 
         final Optional<ElementCountConstraint> elementCountConstraint =
@@ -125,6 +125,19 @@ abstract class AbstractListStatementSupport extends BaseQNameStatementSupport<Li
                             elementCountConstraint.orElse(null), original);
     }
 
+    private static void warnConfigList(final @NonNull StmtContext<QName, ListStatement, ListEffectiveStatement> ctx) {
+        final StmtContext<QName, ListStatement, ListEffectiveStatement> warnCtx = ctx.getOriginalCtx().orElse(ctx);
+        final Boolean warned = warnCtx.getFromNamespace(ConfigListWarningNamespace.class, Boolean.TRUE);
+        // Hacky check if we have issued a warning for the original statement
+        if (warned == null) {
+            verify(warnCtx instanceof Mutable, "Unexpected context %s", warnCtx);
+            ((Mutable<?, ?, ?>) warnCtx).addToNs(ConfigListWarningNamespace.class, Boolean.TRUE, Boolean.TRUE);
+            LOG.info("Configuration list {} does not define any keys in violation of RFC7950 section 7.8.2. While "
+                    + "this is fine with OpenDaylight, it can cause interoperability issues with other systems "
+                    + "[defined at {}]", ctx.getStatementArgument(), warnCtx.getStatementSourceReference());
+        }
+    }
+
     private static boolean isInstantied(final StmtContext<?, ?, ?> ctx) {
         for (StmtContext<?, ?, ?> parent = ctx.getParentContext(); parent != null; parent = parent.getParentContext()) {
             if (UNINSTANTIATED_DATATREE_STATEMENTS.contains(parent.getPublicDefinition())) {

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ConfigListWarningNamespace.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ConfigListWarningNamespace.java
new file mode 100644 (file)
index 0000000..ed816e1
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ConfigListWarningNamespace.java
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2020 PANTHEON.tech, s.r.o. 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.parser.rfc7950.stmt.list;
+
+import com.google.common.annotations.Beta;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.yangtools.yang.model.api.meta.IdentifierNamespace;
+import org.opendaylight.yangtools.yang.parser.spi.meta.NamespaceBehaviour;
+
+@Beta
+public interface ConfigListWarningNamespace extends IdentifierNamespace<Boolean, Boolean> {
+    NamespaceBehaviour<Boolean, Boolean, @NonNull ConfigListWarningNamespace> BEHAVIOUR =
+            NamespaceBehaviour.statementLocal(ConfigListWarningNamespace.class);
+}