Fix ReactorStmtCtx.calculateParentRefcount()

If we have an empty grouping, which is referenced via augment-uses to a
different module, then if that module is buildEffective()'d first, we
end up with the grouping being swept.

This turns out to be a bug in parentRef determination -- in this
particular case we observe parent being REFCOUNT_NONE and invoke it
recursively -- and since the parent is a module, it itself does not have
a parentRef. This leads to us thinking the grouping does not have a
parentRef, whereas it does.

Update calculateParentRefcount() to recognize parent.refCount == 0 as
parent holding us down.

The problem manifests itself in the test when the models are processed
in order foo.yang, bar.yang -- but not when bar.yang is processed before
foo.yang.

Unfortunately the order of processing is not predictable in
BuildGlobalContext, hence we repeat the test 4 times.

JIRA: YANGTOOLS-1474
Change-Id: Ib3d55fb92d5a6d34230722ab1cd39e8f8831b6fe
Signed-off-by: Sangwook Ha <sangwook.ha@verizon.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit ad55ca2b2e72f15d7963cfcb011e4f7610889fce)

diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
index 1b751a70fd0550205fed9c5b5c491079d2b8d7a3..ba682603c9046e2c190ca269f933326172798e3d 100644 (file)
--- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
+++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
@@ -889,13 +889,13 @@ abstract class ReactorStmtCtx<A, D extends DeclaredStatement<A>, E extends Effec
         }
 
         // There are three possibilities:
-        // - REFCOUNT_NONE, in which case we need to search next parent
+        // - REFCOUNT_NONE, in which case we need to check if this statement or its parents are holding a reference
         // - negative (< REFCOUNT_NONE), meaning parent is in some stage of sweeping, hence it does not have
         //   a reference to us
         // - positive (> REFCOUNT_NONE), meaning parent has an explicit refcount which is holding us down
         final int refs = refcount;
         if (refs == REFCOUNT_NONE) {
-            return parentRefcount();
+            return noImplictRef() && noParentRef() ? PARENTREF_ABSENT : PARENTREF_PRESENT;
         }
         return refs < REFCOUNT_NONE ? PARENTREF_ABSENT : PARENTREF_PRESENT;
     }

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java
index e2068e0d1533ded2f91f32e745285603de4c2d98..936dec5f7d3054733feabc7a1f63973129e5c010 100644 (file)
--- a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/AbstractYangTest.java
@@ -14,6 +14,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Throwables;
+import java.util.List;
 import java.util.Set;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -31,11 +32,16 @@ import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
  * Abstract base class containing useful utilities and assertions.
  */
 public abstract class AbstractYangTest {
-    @SuppressWarnings("checkstyle:illegalCatch")
     public static @NonNull EffectiveModelContext assertEffectiveModel(final String... yangResourceName) {
+        return assertEffectiveModel(List.of(yangResourceName), null);
+    }
+
+    @SuppressWarnings("checkstyle:illegalCatch")
+    public static @NonNull EffectiveModelContext assertEffectiveModel(final List<String> yangResourceName,
+        final @Nullable Set<QName> supportedFeatures) {
         final EffectiveModelContext ret;
         try {
-            ret = TestUtils.parseYangSource(yangResourceName);
+            ret = TestUtils.parseYangSource(yangResourceName, supportedFeatures);
         } catch (Exception e) {
             Throwables.throwIfUnchecked(e);
             throw new AssertionError("Failed to assemble effective model", e);

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java
index ccd2213953828ffd9969979be43cb9c1fc75b43d..fce55a6bd5252785f6a6889c7b0f0c16bffbd516 100644 (file)
--- a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/TestUtils.java
@@ -79,11 +79,19 @@ public final class TestUtils {
     }
 
     public static EffectiveModelContext parseYangSource(final String... yangSourceFilePath) throws Exception {
+        return parseYangSource(List.of(yangSourceFilePath), null);
+    }
+
+    public static EffectiveModelContext parseYangSource(final List<String> yangSourceFilePath,
+            final @Nullable Set<QName> supportedFeatures) throws Exception {
         final var reactor = RFC7950Reactors.defaultReactor().newBuild();
         for (var resourcePath : yangSourceFilePath) {
             reactor.addSource(YangStatementStreamSource.create(YangTextSchemaSource.forPath(Path.of(
                 TestUtils.class.getResource(resourcePath).toURI()))));
         }
+        if (supportedFeatures != null) {
+            reactor.setSupportedFeatures(supportedFeatures);
+        }
         return reactor.buildEffective();
     }
 

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1474Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1474Test.java
new file mode 100644 (file)
index 0000000..8df602c
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1474Test.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2023 Verizon 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.stmt;
+
+import java.util.List;
+import java.util.Set;
+import org.junit.jupiter.api.RepeatedTest;
+
+class YT1474Test extends AbstractYangTest {
+    // FIXME: YANGTOOLS-1475: do not repeat the test once we have a way to force predictable execution order
+    @RepeatedTest(4)
+    void testEmptyGrouping() {
+        assertEffectiveModel(List.of("/bugs/YT1474/foo.yang", "/bugs/YT1474/bar.yang"), Set.of());
+    }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/bar.yang
new file mode 100644 (file)
index 0000000..4d26653
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/bar.yang
@@ -0,0 +1,15 @@
+module bar {
+  namespace "urn:bar";
+  prefix "bar";
+
+  import foo {
+    prefix foo;
+  }
+
+  grouping bar-group {
+  }
+
+  augment "/foo:foo" {
+    uses bar-group;
+  }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/foo.yang
new file mode 100644 (file)
index 0000000..e8f424b
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1474/foo.yang
@@ -0,0 +1,10 @@
+module foo {
+  namespace "urn:foo";
+  prefix "foo";
+
+  container foo {
+    leaf foo-leaf {
+      type string;
+    }
+  }
+}