Fix GeneratorReactor.mapToGenerator() 94/97394/16
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 5 Sep 2021 22:48:00 +0000 (00:48 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 14 Oct 2021 18:11:41 +0000 (20:11 +0200)
GeneratorReactor.mapToGenerator() relies on EffectiveStatements having
an identity, as it looks up only based on statement.

Rework the logic to operate on SchemaInferenceStack's state, which
provides hierarchical path which should match Generator layout. This
turns out to be a simple delegator job, but we need to switch matching
strategies when we go along the grouping or augment axis.

JIRA: MDSAL-694
Change-Id: Id87c54fdfc2ff37cdf44503e0603074a74c1c02c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractCompositeGenerator.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/Generator.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/GeneratorReactor.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/MatchStrategy.java [new file with mode: 0644]

index 13907efec188891e2556772b8adf84625ecf622c..14889c53dd0642a6a34f7988564375a506ce72a6 100644 (file)
@@ -42,6 +42,7 @@ import org.opendaylight.yangtools.yang.model.api.stmt.ListEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.NotificationEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.OutputEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.RpcEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.TypedefEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.UsesEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.ri.type.TypeBuilder;
@@ -82,12 +83,51 @@ abstract class AbstractCompositeGenerator<T extends EffectiveStatement<?, ?>> ex
         return children.isEmpty();
     }
 
-    @Override
-    final @Nullable AbstractExplicitGenerator<?> findGenerator(final EffectiveStatement<?, ?> stmt) {
-        for (Generator gen : children) {
-            if (gen instanceof AbstractExplicitGenerator) {
-                final AbstractExplicitGenerator<?> ret = (AbstractExplicitGenerator<?>) gen;
-                if (ret.statement() == stmt) {
+    final @Nullable AbstractExplicitGenerator<?> findGenerator(final List<EffectiveStatement<?, ?>> stmtPath) {
+        return findGenerator(MatchStrategy.identity(), stmtPath, 0);
+    }
+
+    final @Nullable AbstractExplicitGenerator<?> findGenerator(final MatchStrategy childStrategy,
+            // TODO: Wouldn't this method be nicer with Deque<EffectiveStatement<?, ?>> ?
+            final List<EffectiveStatement<?, ?>> stmtPath, final int offset) {
+        final EffectiveStatement<?, ?> stmt = stmtPath.get(offset);
+
+        // Try direct children first, which is simple
+        AbstractExplicitGenerator<?> ret = childStrategy.findGenerator(stmt, children);
+        if (ret != null) {
+            final int next = offset + 1;
+            if (stmtPath.size() == next) {
+                // Final step, return child
+                return ret;
+            }
+            if (ret instanceof AbstractCompositeGenerator) {
+                // We know how to descend down
+                return ((AbstractCompositeGenerator<?>) ret).findGenerator(childStrategy, stmtPath, next);
+            }
+            // Yeah, don't know how to continue here
+            return null;
+        }
+
+        // At this point we are about to fork for augments or groupings. In either case only schema tree statements can
+        // be found this way. The fun part is that if we find a match and need to continue, we will use the same
+        // strategy for children as well. We now know that this (and subsequent) statements need to have a QName
+        // argument.
+        if (stmt instanceof SchemaTreeEffectiveStatement) {
+            // grouping -> uses instantiation changes the namespace to the local namespace of the uses site. We are
+            // going the opposite direction, hence we are changing namespace from local to the grouping's namespace.
+            for (GroupingGenerator gen : groupings) {
+                final MatchStrategy strat = MatchStrategy.grouping(gen);
+                ret = gen.findGenerator(strat, stmtPath, offset);
+                if (ret != null) {
+                    return ret;
+                }
+            }
+
+            // All augments are dead simple: they need to match on argument (which we expect to be a QName)
+            final MatchStrategy strat = MatchStrategy.augment();
+            for (AbstractAugmentGenerator gen : augments) {
+                ret = gen.findGenerator(strat, stmtPath, offset);
+                if (ret != null) {
                     return ret;
                 }
             }
index b096c3df70c578a2c61c034d59b6ee1f7cded10e..7f639216a2bdec24210932ffa86c2c8ab4c065e8 100644 (file)
@@ -65,7 +65,7 @@ public abstract class Generator implements Iterable<Generator> {
     private String javaPackage;
 
     Generator() {
-        this.parent = null;
+        parent = null;
     }
 
     Generator(final AbstractCompositeGenerator<?> parent) {
@@ -99,14 +99,6 @@ public abstract class Generator implements Iterable<Generator> {
         return true;
     }
 
-    @Nullable AbstractExplicitGenerator<?> findGenerator(final EffectiveStatement<?, ?> stmt) {
-        return null;
-    }
-
-    final @NonNull AbstractExplicitGenerator<?> getGenerator(final EffectiveStatement<?, ?> stmt) {
-        return verifyNotNull(findGenerator(stmt), "Cannot match statement %s in %s", stmt, this);
-    }
-
     /**
      * Return the namespace of this statement.
      *
index 5519d1438aa44e5cb4aa9245d6db4464190adaa9..b93cd50bd6cfa6b00659d593569c867ccf8a255e 100644 (file)
@@ -11,11 +11,11 @@ import static com.google.common.base.Verify.verify;
 import static com.google.common.base.Verify.verifyNotNull;
 
 import com.google.common.base.Stopwatch;
+import com.google.common.base.VerifyException;
 import com.google.common.collect.Maps;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
-import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -30,10 +30,8 @@ import org.opendaylight.yangtools.yang.binding.ChildOf;
 import org.opendaylight.yangtools.yang.binding.ChoiceIn;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.QNameModule;
-import org.opendaylight.yangtools.yang.model.api.DerivableSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
 import org.opendaylight.yangtools.yang.model.api.PathExpression;
-import org.opendaylight.yangtools.yang.model.api.SchemaNode;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ModuleEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier;
@@ -301,56 +299,38 @@ public final class GeneratorReactor extends GeneratorContext implements Mutable
     }
 
     private @NonNull AbstractTypeAwareGenerator<?> strictResolvePath(final @NonNull PathExpression path) {
-        final EffectiveStatement<?, ?> stmt;
         try {
-            stmt = inferenceStack.resolvePathExpression(path);
+            inferenceStack.resolvePathExpression(path);
         } catch (IllegalArgumentException e) {
             throw new IllegalArgumentException("Failed to find leafref target " + path.getOriginalString(), e);
         }
-        return mapToGenerator(stmt);
+        return mapToGenerator();
     }
 
     private @Nullable AbstractTypeAwareGenerator<?> lenientResolveLeafref(final @NonNull PathExpression path) {
-        final EffectiveStatement<?, ?> stmt;
         try {
-            stmt = inferenceStack.resolvePathExpression(path);
+            inferenceStack.resolvePathExpression(path);
         } catch (IllegalArgumentException e) {
             LOG.debug("Ignoring unresolved path {}", path, e);
             return null;
         }
-        return mapToGenerator(stmt);
+        return mapToGenerator();
     }
 
     // Map a statement to the corresponding generator
-    private @NonNull AbstractTypeAwareGenerator<?> mapToGenerator(final EffectiveStatement<?, ?> stmt) {
-        if (leafGenerators == null) {
-            final Map<EffectiveStatement<?, ?>, AbstractTypeAwareGenerator<?>> map = new IdentityHashMap<>();
-            indexLeafGenerators(map, children);
-            leafGenerators = map;
-        }
-
-        AbstractTypeAwareGenerator<?> match = leafGenerators.get(stmt);
-        if (match == null && stmt instanceof DerivableSchemaNode) {
-            final SchemaNode orig = ((DerivableSchemaNode) stmt).getOriginal().orElse(null);
-            if (orig instanceof EffectiveStatement) {
-                match = leafGenerators.get(orig);
-            }
-        }
-
-        return verifyNotNull(match, "Cannot resolve generator for %s", stmt);
-    }
-
-    private static void indexLeafGenerators(final Map<EffectiveStatement<?, ?>, AbstractTypeAwareGenerator<?>> map,
-            final Iterable<? extends Generator> parent) {
-        for (Generator child : parent) {
-            if (child instanceof AbstractTypeAwareGenerator) {
-                final AbstractTypeAwareGenerator<?> value = (AbstractTypeAwareGenerator<?>) child;
-                final EffectiveStatement<?, ?> key = value.statement();
-                final AbstractTypeAwareGenerator<?> prev = map.putIfAbsent(key, value);
-                verify(prev == null, "Conflict on %s between %s and %s", key, prev, value);
-            }
-            indexLeafGenerators(map, child);
+    private @NonNull AbstractTypeAwareGenerator<?> mapToGenerator() {
+        // Some preliminaries first: we need to be in the correct module to walk the path
+        final ModuleEffectiveStatement module = inferenceStack.currentModule();
+        final ModuleGenerator gen = verifyNotNull(generators.get(module.localQNameModule()),
+            "Cannot find generator for %s", module);
+
+        // Now kick of the search
+        final List<EffectiveStatement<?, ?>> stmtPath = inferenceStack.toInference().statementPath();
+        final AbstractExplicitGenerator<?> found = gen.findGenerator(stmtPath);
+        if (found instanceof AbstractTypeAwareGenerator) {
+            return (AbstractTypeAwareGenerator<?>) found;
         }
+        throw new VerifyException("Statements " + stmtPath + " resulted in unexpected " + found);
     }
 
     // Note: unlike other methods, this method pushes matching child to the stack
diff --git a/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/MatchStrategy.java b/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/MatchStrategy.java
new file mode 100644 (file)
index 0000000..3b15fed
--- /dev/null
@@ -0,0 +1,121 @@
+/*
+ * Copyright (c) 2021 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.mdsal.binding.generator.impl.reactor;
+
+import static com.google.common.base.Verify.verify;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.MoreObjects.ToStringHelper;
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
+import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
+
+/**
+ * A strategy for matching an {@link EffectiveStatement} to an {@link AbstractExplicitGenerator}.
+ */
+abstract class MatchStrategy {
+    /**
+     * Strategy matching on exact statement identity. We use this initially as it works for non-weird cases.
+     */
+    private static final class Identity extends MatchStrategy {
+        static final @NonNull Identity INSTANCE = new Identity();
+
+        @Override
+        AbstractExplicitGenerator<?> findGenerator(final EffectiveStatement<?, ?> needle,
+                final Iterable<? extends Generator> haystack) {
+            for (Generator gen : haystack) {
+                if (gen instanceof AbstractExplicitGenerator) {
+                    final AbstractExplicitGenerator<?> ret = (AbstractExplicitGenerator<?>) gen;
+                    if (needle == ret.statement()) {
+                        return ret;
+                    }
+                }
+            }
+            return null;
+        }
+    }
+
+    /**
+     * Strategy matching on exact QName argument. Used when we are switching along the 'augments' axis.
+     */
+    private static class OnQName extends MatchStrategy {
+        static final @NonNull OnQName INSTANCE = new OnQName();
+
+        @Override
+        final AbstractExplicitGenerator<?> findGenerator(final EffectiveStatement<?, ?> needle,
+                final Iterable<? extends Generator> haystack) {
+            final Object arg = needle.argument();
+            verify(arg instanceof QName, "Unexpected argument %s in %s", arg, needle);
+            return findGenerator((QName) arg, haystack);
+        }
+
+        AbstractExplicitGenerator<?> findGenerator(final QName needle, final Iterable<? extends Generator> haystack) {
+            for (Generator gen : haystack) {
+                if (gen instanceof AbstractExplicitGenerator) {
+                    final AbstractExplicitGenerator<?> ret = (AbstractExplicitGenerator<?>) gen;
+                    if (needle.equals(ret.statement().argument())) {
+                        return ret;
+                    }
+                }
+            }
+            return null;
+        }
+    }
+
+    /**
+     * Strategy matching on exact QName argument rehosted to a particular grouping. This is how we can locate a node
+     * inlined via a 'uses' of that grouping.
+     */
+    private static final class Grouping extends OnQName {
+        private final @NonNull QNameModule module;
+
+        Grouping(final GroupingGenerator grouping) {
+            module = grouping.statement().argument().getModule();
+        }
+
+        @Override
+        AbstractExplicitGenerator<?> findGenerator(final QName needle, final Iterable<? extends Generator> haystack) {
+            return super.findGenerator(needle.bindTo(module), haystack);
+        }
+
+        @Override
+        ToStringHelper addToStringAttributes(final ToStringHelper helper) {
+            return super.addToStringAttributes(helper).add("module", module);
+        }
+    }
+
+    private MatchStrategy() {
+        // Hidden on purpose
+    }
+
+    static @NonNull MatchStrategy augment() {
+        return OnQName.INSTANCE;
+    }
+
+    static @NonNull MatchStrategy grouping(final GroupingGenerator grouping) {
+        return new Grouping(grouping);
+    }
+
+    static @NonNull MatchStrategy identity() {
+        return Identity.INSTANCE;
+    }
+
+    abstract @Nullable AbstractExplicitGenerator<?> findGenerator(EffectiveStatement<?, ?> needle,
+            Iterable<? extends Generator> haystack);
+
+    @Override
+    public final String toString() {
+        return addToStringAttributes(MoreObjects.toStringHelper(this)).toString();
+    }
+
+    ToStringHelper addToStringAttributes(final ToStringHelper helper) {
+        return helper;
+    }
+}