Fix statement prerequisites and materialization

We are firing onStatementAdded() when copying statements, which means
that:
a) a new inference requirement may be created during inferred
   statemenent materialization
b) the inference requirement may be immediately available in effective
   model

In order to deal with this, ModifierImpl has to always go through a
bootstrap, so that it prerequisites only trigger once an action is
registered.

Furthermore InferredStatementContext needs always instantiate the map
for partial instantiations so as to deal with namespace-driven
instantiations happening while full instantiation is going on.

JIRA: YANGTOOLS-1434
Change-Id: I86dae587c1fe5804cd983c194903e1975f257408
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java
index f5a00a14991712bb0e2bf6001c6be3ed561458e4..af2b5bbe566d6dbe830116f35e7a9aca6ca5b4e8 100644 (file)
--- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java
+++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Streams;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -474,7 +475,8 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
     private List<ReactorStmtCtx<?, ?, ?>> ensureEffectiveSubstatements() {
         accessSubstatements();
         return substatements instanceof List ? castEffective(substatements)
-            : initializeSubstatements(castMaterialized(substatements));
+            // We have either not started or have only partially-materialized statements, ensure full materialization
+            : initializeSubstatements();
     }
 
     @Override
@@ -529,22 +531,31 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
         return count;
     }
 
-    private List<ReactorStmtCtx<?, ?, ?>> initializeSubstatements(
-            final Map<StmtContext<?, ?, ?>, ReactorStmtCtx<?, ?, ?>> materializedSchemaTree) {
-        final Collection<? extends StatementContextBase<?, ?, ?>> declared = prototype.mutableDeclaredSubstatements();
-        final Collection<? extends Mutable<?, ?, ?>> effective = prototype.mutableEffectiveSubstatements();
+    private List<ReactorStmtCtx<?, ?, ?>> initializeSubstatements() {
+        final var declared = prototype.mutableDeclaredSubstatements();
+        final var effective = prototype.mutableEffectiveSubstatements();
+
+        // We are about to instantiate some substatements. The simple act of materializing them may end up triggering
+        // namespace lookups, which in turn can materialize copies by themselves, running ahead of our materialization.
+        // We therefore need a meeting place for, which are the partially-materialized substatements. If we do not have
+        // them yet, instantiate them and we need to populate them as well.
+        final int expectedSize = declared.size() + effective.size();
+        var materializedSchemaTree = castMaterialized(substatements);
+        if (materializedSchemaTree == null) {
+            substatements = materializedSchemaTree = Maps.newHashMapWithExpectedSize(expectedSize);
+        }
 
-        final var buffer = new ArrayList<ReactorStmtCtx<?, ?, ?>>(declared.size() + effective.size());
-        for (final Mutable<?, ?, ?> stmtContext : declared) {
+        final var buffer = new ArrayList<ReactorStmtCtx<?, ?, ?>>(expectedSize);
+        for (var stmtContext : declared) {
             if (stmtContext.isSupportedByFeatures()) {
                 copySubstatement(stmtContext, buffer, materializedSchemaTree);
             }
         }
-        for (final Mutable<?, ?, ?> stmtContext : effective) {
+        for (var stmtContext : effective) {
             copySubstatement(stmtContext, buffer, materializedSchemaTree);
         }
 
-        final List<ReactorStmtCtx<?, ?, ?>> ret = beforeAddEffectiveStatementUnsafe(ImmutableList.of(), buffer.size());
+        final var ret = beforeAddEffectiveStatementUnsafe(ImmutableList.of(), buffer.size());
         ret.addAll(buffer);
         substatements = ret;
         setModified();
@@ -570,10 +581,12 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
         //
         // We could also perform a Map.containsKey() and perform a bulk add, but that would mean the statement order
         // against parent would change -- and we certainly do not want that to happen.
-        final ReactorStmtCtx<?, ?, ?> materialized = findMaterialized(materializedSchemaTree, substatement);
+        final var materialized = findMaterialized(materializedSchemaTree, substatement);
         if (materialized == null) {
             copySubstatement(substatement).ifPresent(copy -> {
-                buffer.add(ensureCompletedPhase(copy));
+                final var cast = ensureCompletedPhase(copy);
+                materializedSchemaTree.put(substatement, cast);
+                buffer.add(cast);
             });
         } else {
             buffer.add(materialized);

diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
index 1fbb0f4bb8542a3e3c62ca413af5b3a1f2a305f7..6e31b1adb2f453c8acfddfeebb453b8175415445 100644 (file)
--- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
+++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
@@ -116,7 +116,7 @@ final class ModifierImpl implements ModelActionBuilder {
 
         PhaseFinished<C> phaseFin = new PhaseFinished<>();
         addReq(phaseFin);
-        contextImpl(context).addPhaseCompletedListener(phase, phaseFin);
+        addBootstrap(() -> contextImpl(context).addPhaseCompletedListener(phase, phaseFin));
         return phaseFin;
     }
 

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java
new file mode 100644 (file)
index 0000000..44123b7
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2022 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.stmt;
+
+import org.junit.Test;
+
+public class YT1434Test extends AbstractYangTest {
+    @Test
+    public void testUniqueViaAugment() {
+        assertEffectiveModel("/bugs/YT1434/foo.yang");
+    }
+
+    @Test
+    public void testUniqueViaUses() {
+        assertEffectiveModel("/bugs/YT1434/bar.yang");
+    }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang
new file mode 100644 (file)
index 0000000..370908b
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang
@@ -0,0 +1,27 @@
+module bar {
+  namespace bar;
+  prefix bar;
+
+  container foo {
+    uses bar;
+  }
+
+  grouping bar {
+    list bar {
+      key one;
+      unique "two three";
+
+      leaf one {
+        type string;
+      }
+
+      leaf two {
+        type string;
+      }
+
+      leaf three {
+        type string;
+      }
+    }
+  }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang
new file mode 100644 (file)
index 0000000..ef6765f
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang
@@ -0,0 +1,25 @@
+module foo {
+  namespace foo;
+  prefix foo;
+
+  container foo;
+
+  augment /foo {
+    list bar {
+      key one;
+      unique "two three";
+
+      leaf one {
+        type string;
+      }
+
+      leaf two {
+        type string;
+      }
+
+      leaf three {
+        type string;
+      }
+    }
+  }
+}