Rework EffectiveStatementBase statement order restoration 67/85967/1
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 26 Nov 2019 10:03:27 +0000 (11:03 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 26 Nov 2019 10:07:13 +0000 (11:07 +0100)
Rather than relying on mutation of effective substatements when we
are building an effective statements (and thus requiring StmtContext
to be StatementContextBase), use simple filtering to implement
statement ordering.

This limits the scope of mutable state, explains what is going
on and allows use of different StmtContext implementations.

JIRA: YANGTOOLS-1042
Change-Id: I84a2517ef0ce666fbc2f2c6faa3b0d0a43eef6eb
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/EffectiveStatementBase.java

index 02443a920d468ae024b0f20e6a64826dc930b9ea..714d457fef4d10bb3205fc4dd768cd6b805b26af 100644 (file)
@@ -321,6 +321,15 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
         return Collections.unmodifiableCollection(effective);
     }
 
+    /**
+     * Remove a set of statements from effective statements.
+     *
+     * @param statements statements to be removed
+     * @deprecated This method was used by EffectiveStatementBase to restore proper order of effects of uses statements.
+     *             It is no longer used in that capacity and slated for removal.
+     */
+    // FIXME: 5.0.0: remove this method
+    @Deprecated(forRemoval = true)
     public void removeStatementsFromEffectiveSubstatements(
             final Collection<? extends StmtContext<?, ?, ?>> statements) {
         if (!effective.isEmpty()) {
index 8adde0ed8db443c8eaee216bb07430aefd9912f5..668c7d6e730e62b5a29aff5855d6f1f59825542c 100644 (file)
@@ -15,18 +15,18 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.model.api.SchemaNode;
-import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
 import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.meta.IdentifierNamespace;
 import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext;
-import org.opendaylight.yangtools.yang.parser.stmt.reactor.StatementContextBase;
 
 public abstract class EffectiveStatementBase<A, D extends DeclaredStatement<A>> implements EffectiveStatement<A, D> {
     private final @NonNull ImmutableList<? extends EffectiveStatement<?, ?>> substatements;
@@ -37,21 +37,60 @@ public abstract class EffectiveStatementBase<A, D extends DeclaredStatement<A>>
      * @param ctx context of statement.
      */
     protected EffectiveStatementBase(final StmtContext<A, D, ?> ctx) {
-        final Collection<? extends StmtContext<?, ?, ?>> effectiveSubstatements = ctx.effectiveSubstatements();
         final Collection<StmtContext<?, ?, ?>> substatementsInit = new ArrayList<>();
 
+        /*
+         * This dance is required to ensure that effects of 'uses' nodes are applied in the same order as
+         * the statements were defined -- i.e. if we have something like this:
+         *
+         * container foo {
+         *   uses bar;
+         *   uses baz;
+         * }
+         *
+         * grouping baz {
+         *   leaf baz {
+         *     type string;
+         *   }
+         * }
+         *
+         * grouping bar {
+         *   leaf bar {
+         *     type string;
+         *   }
+         * }
+         *
+         * The reactor would first inline 'uses baz' as that definition is the first one completely resolved and then
+         * inline 'uses bar'. Here we are iterating in declaration order re-inline the statements.
+         *
+         * TODO: this really should be handled by UsesStatementSupport such that 'uses baz' would have a prerequisite
+         *       of a resolved 'uses bar'.
+         */
+        Set<StmtContext<?, ?, ?>> filteredStatements = null;
         for (final StmtContext<?, ?, ?> declaredSubstatement : ctx.declaredSubstatements()) {
             if (declaredSubstatement.isSupportedByFeatures()) {
                 substatementsInit.add(declaredSubstatement);
-                if (YangStmtMapping.USES == declaredSubstatement.getPublicDefinition()) {
-                    final Collection<? extends StmtContext<?, ?, ?>> effect =
-                            declaredSubstatement.getEffectOfStatement();
+
+                final Collection<? extends StmtContext<?, ?, ?>> effect = declaredSubstatement.getEffectOfStatement();
+                if (!effect.isEmpty()) {
+                    if (filteredStatements == null) {
+                        filteredStatements = new HashSet<>();
+                    }
+                    filteredStatements.addAll(effect);
                     substatementsInit.addAll(effect);
-                    ((StatementContextBase<?, ?, ?>) ctx).removeStatementsFromEffectiveSubstatements(effect);
                 }
             }
         }
-        substatementsInit.addAll(effectiveSubstatements);
+
+        if (filteredStatements != null) {
+            for (StmtContext<?, ?, ?> stmt : ctx.effectiveSubstatements()) {
+                if (!filteredStatements.contains(stmt)) {
+                    substatementsInit.add(stmt);
+                }
+            }
+        } else {
+            substatementsInit.addAll(ctx.effectiveSubstatements());
+        }
 
         this.substatements = ImmutableList.copyOf(initSubstatements(ctx, substatementsInit));
     }