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 664ad74a546256600329707f0bf833c148892ae3..5926a0634684c8b339fe0eb96e46efbdb6b2d917 100644 (file)
--- 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 c1cab301b80d6cfdf68679d533332c5d58074d35..cdf31fcf3b51b98ae72ff90318ab7febf6a3450a 100644 (file)
--- 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
@@ -619,13 +619,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.
      *
@@ -802,10 +806,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",
@@ -819,6 +822,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) {