From: Robert Varga Date: Thu, 1 Feb 2018 16:37:20 +0000 (+0100) Subject: Lookup leaf key methods in parents X-Git-Tag: release/nitrogen-sr2~6 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=2c0f88fc510269fb5bc4b1f1ca8ca33062002f99;p=mdsal.git Lookup leaf key methods in parents Union is a valid key type and it can actually come from a grouping, in which case it gets generated as an inner type. If that happens the typedef is not populated into the usual places, leading to a failure to find method return type -- leading to a missing key member. This is a problem similar to YANGTOOLS-424, so let's use the same ModuleContext approach taken then. JIRA: MDSAL-161 Change-Id: Id342701d603bdd2b9b78ba3295c8eae60f253429 Signed-off-by: Robert Varga (cherry picked from commit 6ac608d5c6b02fbcc3ee01af8fc4e3301a8ed52b) --- diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/BindingGeneratorImpl.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/BindingGeneratorImpl.java index 7c7c0ac414..3c70087d56 100644 --- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/BindingGeneratorImpl.java +++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/BindingGeneratorImpl.java @@ -310,7 +310,7 @@ public class BindingGeneratorImpl implements BindingGenerator { checkArgument(module.getName() != null, "Module name cannot be NULL."); final DataNodeIterator it = new DataNodeIterator(module); final List> typeDefinitions = it.allTypedefs(); - checkState(typeDefinitions != null, "Type Definitions for module «module.name» cannot be NULL."); + checkState(typeDefinitions != null, "Type Definitions for module %s cannot be NULL.", module.getName()); for (final TypeDefinition typedef : typeDefinitions) { if (typedef != null) { @@ -1428,6 +1428,8 @@ public class BindingGeneratorImpl implements BindingGenerator { GeneratedTOBuilder genTOBuilder = addTOToTypeBuilder(typeDef, typeBuilder, leaf, parentModule); if (genTOBuilder != null) { returnType = createReturnTypeForUnion(genTOBuilder, typeDef, typeBuilder, parentModule); + // Store the inner type within the union so that we can find the reference for it + genCtx.get(module).addInnerTypedefType(typeDef.getPath(), returnType); } } else if (typeDef instanceof BitsTypeDefinition) { GeneratedTOBuilder genTOBuilder = addTOToTypeBuilder(typeDef, typeBuilder, leaf, parentModule); @@ -1538,13 +1540,16 @@ public class BindingGeneratorImpl implements BindingGenerator { Type returnType; final TypeDefinition typeDef = CompatUtils.compatLeafType(leaf); if (typeDef instanceof UnionTypeDefinition) { - // GeneratedType for this type definition should be already - // created + // GeneratedType for this type definition should have be already created final QName qname = typeDef.getQName(); final Module unionModule = schemaContext.findModuleByNamespaceAndRevision(qname.getNamespace(), qname.getRevision()); final ModuleContext mc = genCtx.get(unionModule); returnType = mc.getTypedefs().get(typeDef.getPath()); + if (returnType == null) { + // This may still be an inner type, try to find it + returnType = mc.getInnerType(typeDef.getPath()); + } } else if (typeDef instanceof EnumTypeDefinition && typeDef.getBaseType() == null) { // Annonymous enumeration (already generated, since it is inherited via uses). LeafSchemaNode originalLeaf = (LeafSchemaNode) SchemaNodeUtils.getRootOriginalIfPossible(leaf); @@ -1923,7 +1928,7 @@ public class BindingGeneratorImpl implements BindingGenerator { if (schemaNode instanceof LeafSchemaNode) { final LeafSchemaNode leaf = (LeafSchemaNode) schemaNode; final String leafName = leaf.getQName().getLocalName(); - final Type type = resolveLeafSchemaNodeAsMethod(typeBuilder, leaf,module); + Type type = resolveLeafSchemaNodeAsMethod(typeBuilder, leaf, module); if (listKeys.contains(leafName)) { if (type == null) { resolveLeafSchemaNodeAsProperty(genTOBuilder, leaf, true, module); diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/ModuleContext.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/ModuleContext.java index a769eabe96..115007c3be 100644 --- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/ModuleContext.java +++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/ModuleContext.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import org.opendaylight.mdsal.binding.model.api.Type; -import org.opendaylight.mdsal.binding.model.api.type.builder.EnumBuilder; import org.opendaylight.mdsal.binding.model.api.type.builder.GeneratedTOBuilder; import org.opendaylight.mdsal.binding.model.api.type.builder.GeneratedTypeBuilder; import org.opendaylight.yangtools.yang.common.QName; @@ -209,13 +208,13 @@ public final class ModuleContext { } /** - * Adds mapping between schema path and inner enum. + * Adds mapping between schema path and an inner type. * * @param path - * @param enumBuilder + * @param type */ - void addInnerTypedefType(SchemaPath path, EnumBuilder enumBuilder) { - innerTypes.put(path, enumBuilder); + void addInnerTypedefType(final SchemaPath path, final Type type) { + innerTypes.put(path, type); } public Type getInnerType(SchemaPath path) { diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal161Test.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal161Test.java new file mode 100644 index 0000000000..8f2301b950 --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal161Test.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2018 Pantheon Technologies, 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.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.Collection; +import java.util.Optional; +import org.junit.Test; +import org.opendaylight.mdsal.binding.model.api.GeneratedTransferObject; +import org.opendaylight.mdsal.binding.model.api.Type; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; +import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; + +public class Mdsal161Test { + + /** + * Test if leaves with inner union type defined in groupings can be used as list keys at the place of instantiation. + */ + @Test + public void mdsal269Test() throws Exception { + final SchemaContext context = YangParserTestUtils.parseYangSource("/mdsal161.yang"); + final Collection types = new BindingGeneratorImpl(false).generateTypes(context); + assertNotNull(types); + assertEquals(24, types.size()); + + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithGrpKey"); + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithGrpExtKey"); + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithGrpTypedefKey"); + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithoutGrpKey"); + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithoutGrpExtKey"); + assertKeyStructure(types, "org.opendaylight.yang.gen.v1.mdsal161.rev700101.WithoutGrpTypedefKey"); + } + + private static void assertKeyStructure(final Collection types, final String className) { + final Optional optType = types.stream().filter(t -> t.getFullyQualifiedName().equals(className)) + .findFirst(); + assertTrue("Type for " + className + " not found", optType.isPresent()); + + final Type type = optType.get(); + assertTrue(type instanceof GeneratedTransferObject); + final GeneratedTransferObject gto = (GeneratedTransferObject) type; + assertEquals(2, gto.getEqualsIdentifiers().size()); + } +} diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/mdsal161.yang b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal161.yang new file mode 100644 index 0000000000..a49e9cd663 --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal161.yang @@ -0,0 +1,120 @@ +module mdsal161 { + namespace "mdsal161"; + prefix "mdsal161"; + + // Reported case: + typedef revision-identifier { + type string { + pattern '\d{4}-\d{2}-\d{2}'; + } + } + + grouping grp { + leaf name { + type string; + } + leaf revision { + type union { + type revision-identifier; + type string { length 0; } + } + } + } + + list with-grp { + key "name revision"; + + uses grp; + } + + // Simpler case: same thing without the grouping + list without-grp { + key "name revision"; + + leaf name { + type string; + } + leaf revision { + type union { + type revision-identifier; + type string { length 0; } + } + } + } + + // Two different cases: the type is defined in a grouping typedef + grouping grp-typedef { + typedef revision-type { + type union { + type revision-identifier; + type string { length 0; } + } + } + + leaf name { + type string; + } + leaf revision { + type revision-type; + } + } + + list with-grp-typedef { + key "name revision"; + + uses grp-typedef; + } + + list without-grp-typedef { + typedef revision-type { + type union { + type revision-identifier; + type string { length 0; } + } + } + + key "name revision"; + + leaf name { + type string; + } + leaf revision { + type revision-type; + } + } + + // Another set of cases: the type is externally typedef'd + typedef ext-typedef { + type union { + type revision-identifier; + type string { length 0; } + } + } + + grouping grp-ext { + leaf name { + type string; + } + leaf revision { + type ext-typedef; + } + } + + list with-grp-ext { + key "name revision"; + + uses grp-ext; + } + + list without-grp-ext { + key "name revision"; + + leaf name { + type string; + } + leaf revision { + type ext-typedef; + } + } +} +