From f04f0b4634e4c26f396771bb870e8d7c919b78a3 Mon Sep 17 00:00:00 2001 From: Peter Valka Date: Wed, 6 May 2020 09:54:18 +0200 Subject: [PATCH] Fix leafref-to-enum encoding 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 Signed-off-by: Robert Varga (cherry picked from commit f9eceb5081877db3fc3f840c44e943605f3f4335) (cherry picked from commit 4ad86c646f3e447842a46b89b8697b8276942955) --- .../binding/dom/codec/impl/Mdsal552Test.java | 46 +++++++++++++++ .../generator/impl/AbstractTypeGenerator.java | 36 +++++------- .../binding/generator/impl/Mdsal552Test.java | 56 +++++++++++++++++++ .../src/test/resources/mdsal552.yang | 28 ++++++++++ .../src/main/yang/mdsal552.yang | 32 +++++++++++ 5 files changed, 177 insertions(+), 21 deletions(-) create mode 100644 binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal552Test.java create mode 100644 binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal552Test.java create mode 100644 binding/mdsal-binding-generator-impl/src/test/resources/mdsal552.yang create mode 100644 binding/mdsal-binding-test-model/src/main/yang/mdsal552.yang 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 index 0000000000..0eef8c7205 --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal552Test.java @@ -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())); + } +} diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java index e46741bf08..c04647d62e 100644 --- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java +++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java @@ -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 enumTypeDef */ - 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 index 0000000000..0c158eb94b --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal552Test.java @@ -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 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 index 0000000000..114b32f341 --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal552.yang @@ -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 index 0000000000..69e153fb27 --- /dev/null +++ b/binding/mdsal-binding-test-model/src/main/yang/mdsal552.yang @@ -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"; + } + } + } + } +} -- 2.36.6