Fix leafref-to-enum encoding 77/90377/2
authorPeter Valka <Peter.Valka@pantheon.tech>
Wed, 6 May 2020 07:54:18 +0000 (09:54 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 10 Jun 2020 15:02:47 +0000 (17:02 +0200)
When we have a leafref pointing to an enum via an absolute path,
in and RPC, we end up using a no-op codec, whereas we should be
extracting the name (from enum constant, or SchemaContext).

This ends up being a problem not in codec itself, as it is using
reflection to acquire the return type, defaulting to no-op.

The problem lies with AbstractTypeGenerator, which ends up not
being able to look up the generated type in case of RPC. The problem
here is the order in which types are generated, as the leaf being
referenced is generated after the RPC itself -- hence we cannot
find the definition.

While this is again highlighting the problem of single-pass code
generation, we can side-step the problem by moving RPC/notification
generation after the root data generation.

As a further complication, we need to record the enum/schema mapping,
as the codec needs to be able to find them via leafref types.

JIRA: MDSAL-552
Change-Id: Ifd92807029cdb7ba92dcad5655bb34d3ff4cef9d
Signed-off-by: Peter Valka <Peter.Valka@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit f9eceb5081877db3fc3f840c44e943605f3f4335)
(cherry picked from commit 4ad86c646f3e447842a46b89b8697b8276942955)

binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal552Test.java [new file with mode: 0644]
binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java
binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal552Test.java [new file with mode: 0644]
binding/mdsal-binding-generator-impl/src/test/resources/mdsal552.yang [new file with mode: 0644]
binding/mdsal-binding-test-model/src/main/yang/mdsal552.yang [new file with mode: 0644]

diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal552Test.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal552Test.java
new file mode 100644 (file)
index 0000000..0eef8c7
--- /dev/null
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2020 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.dom.codec.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import org.junit.Test;
+import org.opendaylight.mdsal.binding.dom.codec.test.AbstractBindingCodecTest;
+import org.opendaylight.yang.gen.v1.mdsal552.norev.Mdsal552Data.OutputA;
+import org.opendaylight.yang.gen.v1.mdsal552.norev.RefTestOutput;
+import org.opendaylight.yang.gen.v1.mdsal552.norev.RefTestOutputBuilder;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
+import org.opendaylight.yangtools.yang.model.api.SchemaPath;
+
+public class Mdsal552Test extends AbstractBindingCodecTest {
+    private static final SchemaPath OUTPUT_PATH = SchemaPath.create(true, QName.create(RefTestOutput.QNAME, "ref_test"),
+        RefTestOutput.QNAME);
+    private static final QName OUTPUTREF = QName.create(RefTestOutput.QNAME, "outputref");
+
+    @Test
+    public void testLeafrefEnumerationToNormalized() throws IOException {
+        assertEquals(Builders.containerBuilder()
+            .withNodeIdentifier(new NodeIdentifier(RefTestOutput.QNAME))
+            .withChild(ImmutableNodes.leafNode(OUTPUTREF, OutputA.DownTest.getName()))
+            .build(),
+            registry.toNormalizedNodeRpcData(new RefTestOutputBuilder().setOutputref(OutputA.DownTest).build()));
+    }
+
+    @Test
+    public void testLeafrefEnumerationFromNormalized() throws IOException {
+        assertEquals(new RefTestOutputBuilder().setOutputref(OutputA.DownTest).build(),
+            registry.fromNormalizedNodeRpcData(OUTPUT_PATH, Builders.containerBuilder()
+                .withNodeIdentifier(new NodeIdentifier(RefTestOutput.QNAME))
+                .withChild(ImmutableNodes.leafNode(OUTPUTREF, OutputA.DownTest.getName()))
+                .build()));
+    }
+}
index e46741bf08275ae26b1e9a5e2e0bd352b824b19c..c04647d62ee459f382300bac36be88a63133e908 100644 (file)
@@ -65,6 +65,7 @@ import java.util.Set;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.mdsal.binding.model.api.AccessModifier;
 import org.opendaylight.mdsal.binding.model.api.Constant;
+import org.opendaylight.mdsal.binding.model.api.Enumeration;
 import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject;
 import org.opendaylight.mdsal.binding.model.api.GeneratedType;
 import org.opendaylight.mdsal.binding.model.api.JavaTypeName;
@@ -232,15 +233,17 @@ abstract class AbstractTypeGenerator {
         genCtx.put(module.getQNameModule(), context);
         allTypeDefinitionsToGenTypes(context);
         groupingsToGenTypes(context, module.getGroupings());
-        rpcMethodsToGenType(context);
         allIdentitiesToGenTypes(context);
-        notificationsToGenType(context);
 
         if (!module.getChildNodes().isEmpty()) {
             final GeneratedTypeBuilder moduleType = moduleToDataType(context);
             context.addModuleNode(moduleType);
             resolveDataSchemaNodes(context, moduleType, moduleType, module.getChildNodes(), false);
         }
+
+        // Resolve RPCs and notifications only after we have created instantiated tree
+        rpcMethodsToGenType(context);
+        notificationsToGenType(context);
         return context;
     }
 
@@ -762,16 +765,15 @@ abstract class AbstractTypeGenerator {
      * @param module Module in which type should be generated
      * @return enumeration builder which contains data from <code>enumTypeDef</code>
      */
-    private EnumBuilder resolveInnerEnumFromTypeDefinition(final EnumTypeDefinition enumTypeDef, final QName enumName,
+    private Enumeration resolveInnerEnumFromTypeDefinition(final EnumTypeDefinition enumTypeDef, final QName enumName,
             final GeneratedTypeBuilder typeBuilder, final ModuleContext context) {
-        if (enumTypeDef != null && typeBuilder != null && enumTypeDef.getQName().getLocalName() != null) {
-            final EnumBuilder enumBuilder = typeBuilder.addEnumeration(BindingMapping.getClassName(enumName));
-            typeProvider.addEnumDescription(enumBuilder, enumTypeDef);
-            enumBuilder.updateEnumPairsFromEnumTypeDef(enumTypeDef);
-            context.addInnerTypedefType(enumTypeDef.getPath(), enumBuilder);
-            return enumBuilder;
-        }
-        return null;
+        final EnumBuilder enumBuilder = typeBuilder.addEnumeration(BindingMapping.getClassName(enumName));
+        typeProvider.addEnumDescription(enumBuilder, enumTypeDef);
+        enumBuilder.updateEnumPairsFromEnumTypeDef(enumTypeDef);
+        final Enumeration ret = enumBuilder.toInstance(typeBuilder);
+        context.addTypeToSchema(ret, enumTypeDef);
+        context.addInnerTypedefType(enumTypeDef.getPath(), ret);
+        return ret;
     }
 
     /**
@@ -1402,13 +1404,8 @@ abstract class AbstractTypeGenerator {
         final TypeDefinition<?> typeDef = CompatUtils.compatType(leaf);
         if (isInnerType(leaf, typeDef)) {
             if (typeDef instanceof EnumTypeDefinition) {
-                returnType = typeProvider.javaTypeForSchemaDefinitionType(typeDef, leaf, inGrouping);
                 final EnumTypeDefinition enumTypeDef = (EnumTypeDefinition) typeDef;
-                final EnumBuilder enumBuilder = resolveInnerEnumFromTypeDefinition(enumTypeDef, leaf.getQName(),
-                    typeBuilder, context);
-                if (enumBuilder != null) {
-                    returnType = enumBuilder.toInstance(typeBuilder);
-                }
+                returnType = resolveInnerEnumFromTypeDefinition(enumTypeDef, leaf.getQName(), typeBuilder, context);
                 typeProvider.putReferencedType(leaf.getPath(), returnType);
             } else if (typeDef instanceof UnionTypeDefinition) {
                 final UnionTypeDefinition unionDef = (UnionTypeDefinition)typeDef;
@@ -1598,11 +1595,8 @@ abstract class AbstractTypeGenerator {
         Type returnType = null;
         if (typeDef.getBaseType() == null) {
             if (typeDef instanceof EnumTypeDefinition) {
-                returnType = typeProvider.javaTypeForSchemaDefinitionType(typeDef, node, inGrouping);
                 final EnumTypeDefinition enumTypeDef = (EnumTypeDefinition) typeDef;
-                final EnumBuilder enumBuilder = resolveInnerEnumFromTypeDefinition(enumTypeDef, nodeName,
-                    typeBuilder, context);
-                returnType = new ReferencedTypeImpl(enumBuilder.getIdentifier());
+                returnType = resolveInnerEnumFromTypeDefinition(enumTypeDef, nodeName, typeBuilder, context);
                 typeProvider.putReferencedType(node.getPath(), returnType);
             } else if (typeDef instanceof UnionTypeDefinition) {
                 final UnionTypeDefinition unionDef = (UnionTypeDefinition)typeDef;
diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal552Test.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal552Test.java
new file mode 100644 (file)
index 0000000..0c158eb
--- /dev/null
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2020 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;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.util.List;
+import org.junit.Test;
+import org.opendaylight.mdsal.binding.model.api.GeneratedType;
+import org.opendaylight.mdsal.binding.model.api.JavaTypeName;
+import org.opendaylight.mdsal.binding.model.api.MethodSignature;
+import org.opendaylight.mdsal.binding.model.api.Type;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class Mdsal552Test {
+    private static final JavaTypeName BAR_INPUT =
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal552.norev", "BarInput");
+    private static final JavaTypeName BAZ =
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal552.norev", "Baz");
+    private static final JavaTypeName ENUMERATION =
+            JavaTypeName.create("org.opendaylight.yang.gen.v1.mdsal552.norev", "Mdsal552Data").createEnclosed("Foo");
+
+    @Test
+    public void enumLeafrefTest() {
+        final List<Type> types = new BindingGeneratorImpl().generateTypes(
+                YangParserTestUtils.parseYangResource("/mdsal552.yang"));
+        assertNotNull(types);
+        assertEquals(5, types.size());
+
+        final Type baz = types.stream()
+                .filter(type -> BAZ.equals(type.getIdentifier()))
+                .findFirst().get();
+        assertThat(baz, instanceOf(GeneratedType.class));
+        final MethodSignature bazGetRef = ((GeneratedType) baz).getMethodDefinitions().stream()
+                .filter(method -> method.getName().equals("getRef"))
+                .findFirst().get();
+        assertEquals(ENUMERATION, bazGetRef.getReturnType().getIdentifier());
+
+        final Type input = types.stream()
+                .filter(type -> BAR_INPUT.equals(type.getIdentifier()))
+                .findFirst().get();
+        assertThat(input, instanceOf(GeneratedType.class));
+        final MethodSignature inputGetRef = ((GeneratedType) input).getMethodDefinitions().stream()
+                .filter(method -> method.getName().equals("getRef"))
+                .findFirst().get();
+        assertEquals(ENUMERATION, inputGetRef.getReturnType().getIdentifier());
+    }
+}
diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/mdsal552.yang b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal552.yang
new file mode 100644 (file)
index 0000000..114b32f
--- /dev/null
@@ -0,0 +1,28 @@
+module mdsal552 {
+  prefix mdsal552;
+  namespace mdsal552;
+
+  leaf foo {
+    type enumeration {
+      enum one;
+    }
+  }
+
+  container baz {
+    leaf ref {
+      type leafref {
+        path /foo;
+      }
+    }
+  }
+
+  rpc bar {
+    input {
+      leaf ref {
+        type leafref {
+          path /foo;
+        }
+      }
+    }
+  }
+}
diff --git a/binding/mdsal-binding-test-model/src/main/yang/mdsal552.yang b/binding/mdsal-binding-test-model/src/main/yang/mdsal552.yang
new file mode 100644 (file)
index 0000000..69e153f
--- /dev/null
@@ -0,0 +1,32 @@
+module mdsal552 {
+  yang-version 1.1;
+  namespace "mdsal552";
+  prefix mdsal552;
+
+  leaf output_a {
+    type enumeration {
+       enum uptest {
+         value 0;
+       }
+       enum down_test {
+         value 1;
+       }
+    }
+  }
+
+  rpc ref_test {
+    description "RPC enum validation using leafref";
+    input {
+      leaf input_a {
+        type uint32;
+      }
+    }
+    output {
+      leaf outputref {
+        type leafref {
+          path "/output_a";
+        }
+      }
+    }
+  }
+}