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 <robert.varga@pantheon.tech>
(cherry picked from commit 6ac608d5c6b02fbcc3ee01af8fc4e3301a8ed52b)
(cherry picked from commit d295b816714240cccc7d7161c77ee00d2f0ef155)

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 7cc906cc5dc46f205cc4142d0640ffd6c7ce9c7a..5e716c95549ef48105e71234016fc72b4780776f 100644 (file)
--- 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<TypeDefinition<?>> 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) {
@@ -1435,6 +1435,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);
@@ -1545,13 +1547,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);
@@ -1930,7 +1935,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 a769eabe96c6b053f2273326ddf6c49ad93c02d8..115007c3be6a903ec75558b6b75c3d788004f50c 100644 (file)
--- 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 (file)
index 0000000..8f2301b
--- /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<Type> 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<Type> types, final String className) {
+        final Optional<Type> 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 (file)
index 0000000..a49e9cd
--- /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;
+        }
+    }
+}
+