Clarify StatementFactory.createEffective() 20/95020/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Feb 2021 18:41:15 +0000 (19:41 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 3 Feb 2021 18:54:18 +0000 (19:54 +0100)
There is a wide-scale confusion about declared/effective and what
it means where. Document StatementFactory to use different
terminology.

This flushes out the need to document
StatementContextBase.stream{Declared,Effective}, which is updated
with the logical requirement to support buildEffective().

JIRA: YANGTOOLS-1150
Change-Id: I2d455fd9c328f97995d88e37b903145b2e45f793
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java
yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java
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/extension/ExtensionStatementSupport.java
yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java
yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java
yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementFactory.java

index b470a569a8cb09c164d7da940b3a7087d8db0981..025198a35e4c869c467a8ebebeb4039bcf7843ea 100644 (file)
@@ -204,12 +204,12 @@ abstract class AbstractResumedStatement<A, D extends DeclaredStatement<A>, E ext
 
     @Override
     final Stream<? extends StmtContext<?, ?, ?>> streamDeclared() {
-        return declaredSubstatements().stream();
+        return declaredSubstatements().stream().filter(StmtContext::isSupportedToBuildEffective);
     }
 
     @Override
     final Stream<? extends StmtContext<?, ?, ?>> streamEffective() {
-        return effective.stream();
+        return effective.stream().filter(StmtContext::isSupportedToBuildEffective);
     }
 
     @Override
index e3ffc8003cdd68afb6af4e837aed8d87c3a47542..b05c0f636d50a146a3d8a41c0050c58329581fdf 100644 (file)
@@ -235,7 +235,6 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
 
         // We can reuse this statement let's see if all the statements agree
         final List<EffectiveCopy> declCopy = prototype.streamDeclared()
-            .filter(StmtContext::isSupportedByFeatures)
             .map(sub -> effectiveCopy((ReactorStmtCtx<?, ?, ?>) sub))
             .filter(Objects::nonNull)
             .collect(Collectors.toUnmodifiableList());
@@ -288,8 +287,7 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
 
     private List<ReactorStmtCtx<?, ?, ?>> reusePrototypeReplicas() {
         return reusePrototypeReplicas(Streams.concat(
-            prototype.streamDeclared().filter(StmtContext::isSupportedByFeatures),
-            prototype.streamEffective()));
+            prototype.streamDeclared(), prototype.streamEffective()));
     }
 
     private List<ReactorStmtCtx<?, ?, ?>> reusePrototypeReplicas(final Stream<StmtContext<?, ?, ?>> stream) {
@@ -460,7 +458,7 @@ final class InferredStatementContext<A, D extends DeclaredStatement<A>, E extend
     @Override
     Stream<? extends StmtContext<?, ?, ?>> streamEffective() {
         accessSubstatements();
-        return ensureEffectiveSubstatements().stream();
+        return ensureEffectiveSubstatements().stream().filter(StmtContext::isSupportedToBuildEffective);
     }
 
     private void accessSubstatements() {
index d26a8e9adfe06e8877ec38113735d8dba1d0ab3c..fc83c28ed34a989744d3357356fc9f4dc0f73439 100644 (file)
@@ -358,9 +358,23 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
         return factory.createEffective(ctx, ctx.streamDeclared(), ctx.streamEffective());
     }
 
-    abstract Stream<? extends StmtContext<?, ?, ?>> streamDeclared();
+    /**
+     * Return a stream of declared statements which can be built into an {@link EffectiveStatement}, as per
+     * {@link StmtContext#buildEffective()} contract.
+     *
+     * @return Stream of supported declared statements.
+     */
+    // FIXME: we really want to unify this with streamEffective(), under its name
+    abstract Stream<? extends @NonNull StmtContext<?, ?, ?>> streamDeclared();
 
-    abstract Stream<? extends StmtContext<?, ?, ?>> streamEffective();
+    /**
+     * Return a stream of inferred statements which can be built into an {@link EffectiveStatement}, as per
+     * {@link StmtContext#buildEffective()} contract.
+     *
+     * @return Stream of supported effective statements.
+     */
+    // FIXME: this method is currently a misnomer, but unifying with streamDeclared() would make this accurate again
+    abstract Stream<? extends @NonNull StmtContext<?, ?, ?>> streamEffective();
 
     @Override
     final boolean doTryToCompletePhase(final ModelProcessingPhase phase) {
index 6e4c43277917024fed98897d8d04555705a3b173..66c995bf30e57aee3078c7f1b9e0a796af0e09f6 100644 (file)
@@ -99,7 +99,7 @@ public final class ExtensionStatementSupport
     @Override
     public ExtensionEffectiveStatement createEffective(final Current<QName, ExtensionStatement> stmt,
             final Stream<? extends StmtContext<?, ?, ?>> declaredSubstatements,
-            final Stream<? extends StmtContext<?, ?, ?>> effectiveSubstatements) {
+            final Stream<? extends StmtContext<?, ?, ?>> inferredSubstatements) {
         Map<Current<?, ?>, ExtensionEffectiveStatementImpl> tl = TL_BUILDERS.get();
         if (tl == null) {
             tl = new IdentityHashMap<>();
@@ -117,7 +117,7 @@ public final class ExtensionStatementSupport
                 stmt.optionalPath());
             verify(tl.put(stmt, created) == null);
             try {
-                return super.createEffective(stmt, declaredSubstatements, effectiveSubstatements);
+                return super.createEffective(stmt, declaredSubstatements, inferredSubstatements);
             } finally {
                 verify(tl.remove(stmt) == created);
             }
index acc2f2880be740ce69b9463ec0e1db9f05a02d92..694a91860e71fef36358bd3552d5859dfd82bde7 100644 (file)
@@ -54,10 +54,10 @@ public abstract class AbstractStatementSupport<A, D extends DeclaredStatement<A>
     @Override
     public E createEffective(final Current<A, D> stmt,
             final Stream<? extends StmtContext<?, ?, ?>> declaredSubstatements,
-            final Stream<? extends StmtContext<?, ?, ?>> effectiveSubstatements) {
+            final Stream<? extends StmtContext<?, ?, ?>> inferredSubstatements) {
         final ImmutableList<? extends EffectiveStatement<?, ?>> substatements =
                 buildEffectiveSubstatements(stmt, statementsToBuild(stmt,
-                    declaredSubstatements(declaredSubstatements, effectiveSubstatements)));
+                    declaredSubstatements(declaredSubstatements, inferredSubstatements)));
         return createEffective(stmt, substatements);
     }
 
@@ -112,15 +112,7 @@ public abstract class AbstractStatementSupport<A, D extends DeclaredStatement<A>
      */
     protected @NonNull ImmutableList<? extends EffectiveStatement<?, ?>> buildEffectiveSubstatements(
             final Current<A, D> stmt, final List<? extends StmtContext<?, ?, ?>> substatements) {
-        return defaultBuildEffectiveSubstatements(substatements);
-    }
-
-    private static @NonNull ImmutableList<? extends EffectiveStatement<?, ?>> defaultBuildEffectiveSubstatements(
-            final List<? extends StmtContext<?, ?, ?>> substatements) {
-        return substatements.stream()
-                .filter(StmtContext::isSupportedToBuildEffective)
-                .map(StmtContext::buildEffective)
-                .collect(ImmutableList.toImmutableList());
+        return substatements.stream().map(StmtContext::buildEffective).collect(ImmutableList.toImmutableList());
     }
 
     private static @NonNull List<StmtContext<?, ?, ?>> declaredSubstatements(
index 5e8d619bbf083b8dccb07f5a705699cdbc3f6bb8..61f30ed0c8813320280e3d9831cd1c73a84d79a8 100644 (file)
@@ -42,8 +42,8 @@ public abstract class ForwardingStatementSupport<A, D extends DeclaredStatement<
     @Override
     public E createEffective(final Current<A, D> stmt,
             final Stream<? extends StmtContext<?, ?, ?>> declaredSubstatements,
-            final Stream<? extends StmtContext<?, ?, ?>> effectiveSubstatements) {
-        return delegate.createEffective(stmt, declaredSubstatements, effectiveSubstatements);
+            final Stream<? extends StmtContext<?, ?, ?>> inferredSubstatements) {
+        return delegate.createEffective(stmt, declaredSubstatements, inferredSubstatements);
     }
 
     @Override
index 279e9336965ab8d857723cbdae97557666efaacf..777227f45c33939ff0efe579ab4b5bb8005ece0a 100644 (file)
@@ -35,11 +35,14 @@ public interface StatementFactory<A, D extends DeclaredStatement<A>, E extends E
      * Create a {@link EffectiveStatement} for specified context.
      *
      * @param stmt Effective capture of this statement's significant state
+     * @param declaredSubstatements effectively-visible declared substatements
+     * @param inferredSubstatements effectively-visible inferred substatements
      * @return An effective statement instance
      */
+    // FIXME: we really want a single coherent 'effectiveSubstatements' stream
     @NonNull E createEffective(@NonNull Current<A, D> stmt,
         Stream<? extends StmtContext<?, ?, ?>> declaredSubstatements,
-        Stream<? extends StmtContext<?, ?, ?>> effectiveSubstatements);
+        Stream<? extends StmtContext<?, ?, ?>> inferredSubstatements);
 
     /**
      * Create a {@link EffectiveStatement} copy of provided original for specified context.