From: Robert Varga <robert.varga@pantheon.tech>
Date: Thu, 5 Nov 2020 11:53:04 +0000 (+0100)
Subject: Fix PhaseModificationInNamespacePath effects
X-Git-Tag: v4.0.14~4
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F11%2F93611%2F1;p=yangtools.git

Fix PhaseModificationInNamespacePath effects

PhaseModificationInNamespacePath is a set of requirements, working
along the parent->child axis. As we are resolving individual steps
we have to mark each as being impacted by this mutation, not only
the ultimate target.

This ensures that parent statements are not allowed to complete
until the mutation is resolved one way or another -- thus ensuring
proper parent/child phase completion transitions.

JIRA: YANGTOOLS-1160
Change-Id: I87cd682d32cb9ce3397883f2ac1a61a0bc9c8664
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit ac66e993b3b7b00a70feae6fcc7c0c6fbf45cfe6)
---

diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
index 664ad74a54..5926a06346 100644
--- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
+++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java
@@ -376,6 +376,11 @@ final class ModifierImpl implements ModelActionBuilder {
         }
     }
 
+    /**
+     * This similar to {@link PhaseModificationInNamespace}, but allows recursive descent until it finds the real
+     * target. The mechanics is driven as a sequence of prerequisites along a path: first we hook onto namespace to
+     * give us the first step. When it does, we hook onto the first item to provide us the second step and so on.
+     */
     private final class PhaseModificationInNamespacePath<C extends Mutable<?, ?, ?>, K,
             N extends IdentifierNamespace<K, ? extends StmtContext<?, ?, ?>>> extends AbstractPrerequisite<C>
             implements OnNamespaceItemAdded, ContextMutation {
@@ -407,15 +412,27 @@ final class ModifierImpl implements ModelActionBuilder {
                 return;
             }
 
+            // Hook onto target: we either have a modification of the target itself or one of its children.
+            target.addMutation(modPhase, this);
+            // We have completed the context -> target step, hence we are no longer directly blocking context from
+            // making forward progress.
+            context.removeMutation(modPhase, this);
+
             if (!it.hasNext()) {
-                target.addMutation(modPhase, this);
+                // Last step: we are done
                 resolvePrereq((C) value);
                 return;
             }
 
+            // Make sure target's storage notifies us when the next step becomes available.
             hookOnto(target, namespace, it.next());
         }
 
+        @Override
+        ToStringHelper addToStringAttributes(final ToStringHelper toStringHelper) {
+            return super.addToStringAttributes(toStringHelper).add("phase", modPhase).add("keys", keys);
+        }
+
         void hookOnto(final StmtContext<?, ?, ?> context, final Class<?> namespace) {
             checkArgument(it.hasNext(), "Namespace %s keys may not be empty", namespace);
             hookOnto(contextImpl(context), namespace, it.next());
diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
index 994c2a5a07..f307e1cc01 100644
--- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
+++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
@@ -607,13 +607,17 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
 
         if (openMutations.isEmpty()) {
             phaseMutation.removeAll(phase);
-            if (phaseMutation.isEmpty()) {
-                phaseMutation = ImmutableMultimap.of();
-            }
+            cleanupPhaseMutation();
         }
         return finished;
     }
 
+    private void cleanupPhaseMutation() {
+        if (phaseMutation.isEmpty()) {
+            phaseMutation = ImmutableMultimap.of();
+        }
+    }
+
     /**
      * Occurs on end of {@link ModelProcessingPhase} of source parsing.
      *
@@ -790,10 +794,9 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
     /**
      * Adds a {@link ContextMutation} to a {@link ModelProcessingPhase}.
      *
-     * @throws IllegalStateException
-     *             when the mutation was registered after phase was completed
+     * @throws IllegalStateException when the mutation was registered after phase was completed
      */
-    void addMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
+    final void addMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
         ModelProcessingPhase finishedPhase = completedPhase;
         while (finishedPhase != null) {
             checkState(!phase.equals(finishedPhase), "Mutation registered after phase was completed at: %s",
@@ -807,6 +810,13 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
         phaseMutation.put(phase, mutation);
     }
 
+    final void removeMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
+        if (!phaseMutation.isEmpty()) {
+            phaseMutation.remove(phase, mutation);
+            cleanupPhaseMutation();
+        }
+    }
+
     @Override
     public <K, KT extends K, N extends StatementNamespace<K, ?, ?>> void addContext(final Class<N> namespace,
             final KT key,final StmtContext<?, ?, ?> stmt) {