Ignore empty augmentations at runtime 56/100156/1
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 20 Mar 2022 11:11:34 +0000 (12:11 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Sun, 20 Mar 2022 11:12:00 +0000 (12:12 +0100)
We may end up generating an augmentation for a semantically-empty
construct, which in turn will not have a valid AugmentationIdentifier.
Ignore sure augmentations in codec.

JIRA: MDSAL-735
Change-Id: Ibb279a6b554c49857d0ee2ae0108d4d424939d02
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataContainerCodecPrototype.java
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/AbstractAugmentGenerator.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/ModuleAugmentGenerator.java
binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/UsesAugmentGenerator.java

index 4ab8c5e161924d9c34c85091d701d50da1786e18..614f4c4250e7bbb075335beaafc2726df3d8ff8b 100644 (file)
@@ -9,7 +9,6 @@ package org.opendaylight.mdsal.binding.dom.codec.impl;
 
 import static com.google.common.base.Verify.verify;
 
-import com.google.common.collect.Iterables;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
 import org.eclipse.jdt.annotation.NonNull;
@@ -87,8 +86,9 @@ final class DataContainerCodecPrototype<T extends RuntimeTypeContainer> implemen
         this.factory = factory;
 
         if (arg instanceof AugmentationIdentifier) {
-            this.namespace = Iterables.getFirst(((AugmentationIdentifier) arg).getPossibleChildNames(), null)
-                    .getModule();
+            final var childNames = ((AugmentationIdentifier) arg).getPossibleChildNames();
+            verify(!childNames.isEmpty(), "Unexpected empty identifier for %s", type);
+            this.namespace = childNames.iterator().next().getModule();
         } else {
             this.namespace = arg.getNodeType().getModule();
         }
index 47575f9321d707a46eeb43a1276d983e76127bb5..2fa41e9585c2599e14dd84413affe9c588828c1b 100644 (file)
@@ -177,14 +177,16 @@ public abstract class DataObjectCodecContext<D extends DataObject, T extends Com
         final Map<PathArgument, DataContainerCodecPrototype<?>> augByYang = new HashMap<>();
         final Map<Class<?>, DataContainerCodecPrototype<?>> augByStream = new HashMap<>();
         for (final AugmentRuntimeType augment : possibleAugmentations) {
-            final DataContainerCodecPrototype<?> augProto = getAugmentationPrototype(augment);
-            final PathArgument augYangArg = augProto.getYangArg();
-            if (augByYang.putIfAbsent(augYangArg, augProto) == null) {
-                LOG.trace("Discovered new YANG mapping {} -> {} in {}", augYangArg, augProto, this);
-            }
-            final Class<?> augBindingClass = augProto.getBindingClass();
-            if (augByStream.putIfAbsent(augBindingClass, augProto) == null) {
-                LOG.trace("Discovered new class mapping {} -> {} in {}", augBindingClass, augProto, this);
+            final DataContainerCodecPrototype<?> augProto = loadAugmentPrototype(augment);
+            if (augProto != null) {
+                final PathArgument augYangArg = augProto.getYangArg();
+                if (augByYang.putIfAbsent(augYangArg, augProto) == null) {
+                    LOG.trace("Discovered new YANG mapping {} -> {} in {}", augYangArg, augProto, this);
+                }
+                final Class<?> augBindingClass = augProto.getBindingClass();
+                if (augByStream.putIfAbsent(augBindingClass, augProto) == null) {
+                    LOG.trace("Discovered new class mapping {} -> {} in {}", augBindingClass, augProto, this);
+                }
             }
         }
         augmentationByYang = ImmutableMap.copyOf(augByYang);
@@ -386,23 +388,29 @@ public abstract class DataObjectCodecContext<D extends DataObject, T extends Com
         return cls.equals(loaded);
     }
 
-    private @NonNull DataContainerCodecPrototype<?> getAugmentationPrototype(final AugmentRuntimeType augment) {
-        final BindingRuntimeContext ctx = factory().getRuntimeContext();
+    private @Nullable DataContainerCodecPrototype<?> loadAugmentPrototype(final AugmentRuntimeType augment) {
+        // FIXME: in face of deviations this code should be looking at declared view, i.e. all possibilities at augment
+        //        declaration site
+        final var possibleChildren = augment.statement()
+            .streamEffectiveSubstatements(SchemaTreeEffectiveStatement.class)
+            .map(SchemaTreeEffectiveStatement::getIdentifier)
+            .collect(ImmutableSet.toImmutableSet());
+        if (possibleChildren.isEmpty()) {
+            return null;
+        }
 
+        final var factory = factory();
         final GeneratedType javaType = augment.javaType();
         final Class<? extends Augmentation<?>> augClass;
         try {
-            augClass = ctx.loadClass(javaType);
+            augClass = factory.getRuntimeContext().loadClass(javaType);
         } catch (final ClassNotFoundException e) {
             throw new IllegalStateException(
                 "RuntimeContext references type " + javaType + " but failed to load its class", e);
         }
 
-        // TODO: at some point we need the effective children
-        return DataContainerCodecPrototype.from(augClass, new AugmentationIdentifier(augment.statement()
-            .streamEffectiveSubstatements(SchemaTreeEffectiveStatement.class)
-            .map(SchemaTreeEffectiveStatement::getIdentifier)
-            .collect(ImmutableSet.toImmutableSet())), augment, factory());
+        return DataContainerCodecPrototype.from(augClass, new AugmentationIdentifier(possibleChildren), augment,
+            factory);
     }
 
     @SuppressWarnings("checkstyle:illegalCatch")
index c25e6f867fb76accf8fc37c296569763271b150b..d2ae1890aeff33390174153c72afe6d5b41fcd12 100644 (file)
@@ -11,9 +11,11 @@ import static com.google.common.base.Verify.verify;
 import static com.google.common.base.Verify.verifyNotNull;
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.collect.ImmutableList;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
+import java.util.function.Function;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.generator.impl.reactor.CollisionDomain.Member;
 import org.opendaylight.mdsal.binding.generator.impl.rt.DefaultAugmentRuntimeType;
@@ -31,6 +33,8 @@ import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ChoiceEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement.SchemaTreeNamespace;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 
 /**
@@ -166,6 +170,23 @@ abstract class AbstractAugmentGenerator
 
     abstract @NonNull TargetAugmentEffectiveStatement effectiveIn(SchemaTreeAwareEffectiveStatement<?, ?> target);
 
+    final @NonNull TargetAugmentEffectiveStatement effectiveIn(final SchemaTreeAwareEffectiveStatement<?, ?> target,
+            final Function<QName, QName> transform) {
+        final var augment = statement();
+        final var stmts = augment.effectiveSubstatements();
+        final var builder = ImmutableList.<EffectiveStatement<?, ?>>builderWithExpectedSize(stmts.size());
+        for (var child : stmts) {
+            if (child instanceof SchemaTreeEffectiveStatement) {
+                final var qname = ((SchemaTreeEffectiveStatement<?>) child).getIdentifier();
+                // Note: a match in target may be missing -- for example if it was 'deviate unsupported'
+                target.get(SchemaTreeNamespace.class, transform.apply(qname)).ifPresent(builder::add);
+            } else {
+                builder.add(child);
+            }
+        }
+        return new TargetAugmentEffectiveStatement(augment, target, builder.build());
+    }
+
     @Override
     final void addAsGetterMethod(final GeneratedTypeBuilderBase<?> builder, final TypeBuilderFactory builderFactory) {
         // Augments are never added as getters, as they are handled via Augmentable mechanics
index bf6964ec249035444bc48ebf3a0483389bf2d48a..cb1f6418004b7a01cfc6fec29ada0cb146b0a2da 100644 (file)
@@ -7,13 +7,10 @@
  */
 package org.opendaylight.mdsal.binding.generator.impl.reactor;
 
-import com.google.common.collect.ImmutableList;
+import java.util.function.Function;
 import org.eclipse.jdt.annotation.NonNull;
-import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement;
-import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement.SchemaTreeNamespace;
-import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeEffectiveStatement;
 
 /**
  * Generator corresponding to a {@code augment} statement used as a child of a {@code module} statement.
@@ -25,19 +22,7 @@ final class ModuleAugmentGenerator extends AbstractAugmentGenerator {
 
     @Override
     TargetAugmentEffectiveStatement effectiveIn(final SchemaTreeAwareEffectiveStatement<?, ?> target) {
-        final var augment = statement();
-        final var stmts = augment.effectiveSubstatements();
-        final var builder = ImmutableList.<EffectiveStatement<?, ?>>builderWithExpectedSize(stmts.size());
-        for (var child : stmts) {
-            if (child instanceof SchemaTreeEffectiveStatement) {
-                final var qname = ((SchemaTreeEffectiveStatement<?>) child).getIdentifier();
-                // FIXME: orElseThrow()?
-                target.get(SchemaTreeNamespace.class, qname).ifPresent(builder::add);
-            } else {
-                builder.add(child);
-            }
-        }
-        return new TargetAugmentEffectiveStatement(augment, target, builder.build());
+        return effectiveIn(target, Function.identity());
     }
 
     @NonNull AugmentRequirement startLinkage(final GeneratorContext context) {
index dbb1e10a0f83165ce1aeae7807a76d47bdf2e1ec..59bc06b301a445f5825a4d3070a91e951f169ef7 100644 (file)
@@ -11,13 +11,10 @@ import static com.google.common.base.Verify.verify;
 import static com.google.common.base.Verify.verifyNotNull;
 import static java.util.Objects.requireNonNull;
 
-import com.google.common.collect.ImmutableList;
 import org.eclipse.jdt.annotation.NonNull;
-import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement;
-import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeAwareEffectiveStatement.SchemaTreeNamespace;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaTreeEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.UsesEffectiveStatement;
 
@@ -67,21 +64,8 @@ final class UsesAugmentGenerator extends AbstractAugmentGenerator {
     TargetAugmentEffectiveStatement effectiveIn(final SchemaTreeAwareEffectiveStatement<?, ?> target) {
         verify(target instanceof SchemaTreeEffectiveStatement, "Unexpected statement %s", target);
         // 'uses'/'augment': our children are binding to target's namespace
-        final var targetNamespace = ((SchemaTreeEffectiveStatement<?>) target).argument().getModule();
-
-        final var augment = statement();
-        final var stmts = augment.effectiveSubstatements();
-        final var builder = ImmutableList.<EffectiveStatement<?, ?>>builderWithExpectedSize(stmts.size());
-        for (var stmt : stmts) {
-            if (stmt instanceof SchemaTreeEffectiveStatement) {
-                final var qname = ((SchemaTreeEffectiveStatement<?>) stmt).getIdentifier().bindTo(targetNamespace);
-                // FIXME: orElseThrow()?
-                target.get(SchemaTreeNamespace.class, qname).ifPresent(builder::add);
-            } else {
-                builder.add(stmt);
-            }
-        }
 
-        return new TargetAugmentEffectiveStatement(augment, target, builder.build());
+        final var targetNamespace = ((SchemaTreeEffectiveStatement<?>) target).argument().getModule();
+        return effectiveIn(target, qname -> qname.bindTo(targetNamespace));
     }
 }