From 65e53613bd23b145425a6e696122125104bfae38 Mon Sep 17 00:00:00 2001 From: Martin Vitez Date: Thu, 6 Mar 2014 17:09:25 +0100 Subject: [PATCH] Fix for Bug 489. Current code generator version was unable to generate sources for augmentation defined under uses statement if augment points to nested node (not top level node added by uses). Failing example: container c { uses route-subobjects { augment "links/link" { leaf id { type string; } } } } AugmentationSchemaBuilderImpl: fixed bug in parsing augment target path for augmentations defined under uses. BindingGeneratorImpl: fixed resolving of augment package name. BindingGeneratorUtil: added new method to generate package name. Added tests. Change-Id: Ie48b2fffc0e6449bf5c13b5a6b636cbd837ffb54 Signed-off-by: Martin Vitez --- .../generator/impl/BindingGeneratorImpl.xtend | 126 ++++++++++++------ .../generator/util/BindingGeneratorUtil.java | 14 +- .../api/generator/test/CompilationTest.java | 18 ++- .../compilation/augment-under-uses/bar.yang | 7 + .../compilation/augment-under-uses/foo.yang | 10 ++ 5 files changed, 127 insertions(+), 48 deletions(-) diff --git a/code-generator/binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/generator/impl/BindingGeneratorImpl.xtend b/code-generator/binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/generator/impl/BindingGeneratorImpl.xtend index 47fb2728de..81ed99e6c1 100644 --- a/code-generator/binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/generator/impl/BindingGeneratorImpl.xtend +++ b/code-generator/binding-generator-impl/src/main/java/org/opendaylight/yangtools/sal/binding/generator/impl/BindingGeneratorImpl.xtend @@ -307,7 +307,7 @@ public class BindingGeneratorImpl implements BindingGenerator { val basePackageName = moduleNamespaceToPackageName(module); for (usesNode : node.uses) { for (augment : usesNode.augmentations) { - augmentationToGenTypes(basePackageName, augment, module, usesNode); + usesAugmentationToGenTypes(basePackageName, augment, module, usesNode, node); processUsesAugments(augment, module); } } @@ -339,7 +339,7 @@ public class BindingGeneratorImpl implements BindingGenerator { val basePackageName = moduleNamespaceToPackageName(module); val List augmentations = resolveAugmentations(module); for (augment : augmentations) { - augmentationToGenTypes(basePackageName, augment, module, null); + augmentationToGenTypes(basePackageName, augment, module); } } @@ -731,56 +731,86 @@ public class BindingGeneratorImpl implements BindingGenerator { *
  • if target path of augSchema equals null
    • * */ - private def void augmentationToGenTypes(String augmentPackageName, AugmentationSchema augSchema, Module module, - UsesNode parentUsesNode) { + private def void augmentationToGenTypes(String augmentPackageName, AugmentationSchema augSchema, Module module) { checkArgument(augmentPackageName !== null, "Package Name cannot be NULL."); checkArgument(augSchema !== null, "Augmentation Schema cannot be NULL."); checkState(augSchema.targetPath !== null, "Augmentation Schema does not contain Target Path (Target Path is NULL)."); - processUsesAugments(augSchema, module); + processUsesAugments(augSchema, module); val targetPath = augSchema.targetPath; - var targetSchemaNode = findDataSchemaNode(schemaContext, targetPath); + var SchemaNode targetSchemaNode = null + + targetSchemaNode = findDataSchemaNode(schemaContext, targetPath); if (targetSchemaNode instanceof DataSchemaNode && (targetSchemaNode as DataSchemaNode).isAddedByUses()) { - if (parentUsesNode == null) { - targetSchemaNode = findOriginal(targetSchemaNode as DataSchemaNode); - } else { - targetSchemaNode = findOriginalTargetFromGrouping(targetSchemaNode.QName.localName, parentUsesNode); - } + targetSchemaNode = findOriginal(targetSchemaNode as DataSchemaNode); if (targetSchemaNode == null) { throw new NullPointerException( - "Failed to find target node from grouping for augmentation " + augSchema + " in module " + + "Failed to find target node from grouping in augmentation " + augSchema + " in module " + module.name); } + } + if (targetSchemaNode == null) { + throw new IllegalArgumentException("augment target not found: " + targetPath) + } + + var targetTypeBuilder = findChildNodeByPath(targetSchemaNode.path) + if (targetTypeBuilder === null) { + targetTypeBuilder = findCaseByPath(targetSchemaNode.path) + } + if (targetTypeBuilder === null) { + throw new NullPointerException("Target type not yet generated: " + targetSchemaNode); } + + if (!(targetSchemaNode instanceof ChoiceNode)) { + var packageName = augmentPackageName; + val augTypeBuilder = addRawAugmentGenTypeDefinition(module, packageName, augmentPackageName, + targetTypeBuilder.toInstance, augSchema); + genCtx.get(module).addAugmentType(augTypeBuilder) + genCtx.get(module).addTypeToAugmentation(augTypeBuilder, augSchema); + } else { + generateTypesFromAugmentedChoiceCases(module, augmentPackageName, targetTypeBuilder.toInstance, + targetSchemaNode as ChoiceNode, augSchema.childNodes); + } + } - if (targetSchemaNode == null) { - throw new IllegalArgumentException("augment target not found: " + targetPath) + private def void usesAugmentationToGenTypes(String augmentPackageName, AugmentationSchema augSchema, Module module, + UsesNode usesNode, DataNodeContainer usesNodeParent) { + checkArgument(augmentPackageName !== null, "Package Name cannot be NULL."); + checkArgument(augSchema !== null, "Augmentation Schema cannot be NULL."); + checkState(augSchema.targetPath !== null, + "Augmentation Schema does not contain Target Path (Target Path is NULL)."); + + processUsesAugments(augSchema, module); + val targetPath = augSchema.targetPath; + var SchemaNode targetSchemaNode = null + targetSchemaNode = findOriginalTargetFromGrouping(targetPath, usesNode); + if (targetSchemaNode == null) { + throw new IllegalArgumentException("augment target not found: " + targetPath) + } + + var targetTypeBuilder = findChildNodeByPath(targetSchemaNode.path) + if (targetTypeBuilder === null) { + targetTypeBuilder = findCaseByPath(targetSchemaNode.path) + } + if (targetTypeBuilder === null) { + throw new NullPointerException("Target type not yet generated: " + targetSchemaNode); } - if (targetSchemaNode !== null) { - var targetTypeBuilder = findChildNodeByPath(targetSchemaNode.path) - if (targetTypeBuilder === null) { - targetTypeBuilder = findCaseByPath(targetSchemaNode.path) - } - if (targetTypeBuilder === null) { - throw new NullPointerException("Target type not yet generated: " + targetSchemaNode); - } - if (!(targetSchemaNode instanceof ChoiceNode)) { - var packageName = augmentPackageName; - if (parentUsesNode != null) { - packageName = packageNameForGeneratedType(augmentPackageName, augSchema.targetPath); - } - val augTypeBuilder = addRawAugmentGenTypeDefinition(module, packageName, augmentPackageName, - targetTypeBuilder.toInstance, augSchema); - genCtx.get(module).addAugmentType(augTypeBuilder) - genCtx.get(module).addTypeToAugmentation(augTypeBuilder,augSchema); - } else { - generateTypesFromAugmentedChoiceCases(module, augmentPackageName, targetTypeBuilder.toInstance, - targetSchemaNode as ChoiceNode, augSchema.childNodes); + if (!(targetSchemaNode instanceof ChoiceNode)) { + var packageName = augmentPackageName; + if (usesNodeParent instanceof SchemaNode) { + packageName = packageNameForGeneratedType(augmentPackageName, (usesNodeParent as SchemaNode).path, true) } + val augTypeBuilder = addRawAugmentGenTypeDefinition(module, packageName, augmentPackageName, + targetTypeBuilder.toInstance, augSchema); + genCtx.get(module).addAugmentType(augTypeBuilder) + genCtx.get(module).addTypeToAugmentation(augTypeBuilder, augSchema); + } else { + generateTypesFromAugmentedChoiceCases(module, augmentPackageName, targetTypeBuilder.toInstance, + targetSchemaNode as ChoiceNode, augSchema.childNodes); } - } + } /** * Utility method which search for original node defined in grouping. @@ -1007,31 +1037,41 @@ public class BindingGeneratorImpl implements BindingGenerator { /** * Convenient method to find node added by uses statement. */ - private def DataSchemaNode findOriginalTargetFromGrouping(String targetSchemaNodeName, UsesNode parentUsesNode) { + private def DataSchemaNode findOriginalTargetFromGrouping(SchemaPath targetPath, UsesNode parentUsesNode) { var SchemaNode targetGrouping = findNodeInSchemaContext(schemaContext, parentUsesNode.groupingPath.path); if (!(targetGrouping instanceof GroupingDefinition)) { throw new IllegalArgumentException("Failed to generate code for augment in " + parentUsesNode); } - var grouping = targetGrouping as GroupingDefinition; - var result = grouping.getDataChildByName(targetSchemaNodeName); + var grouping = targetGrouping as GroupingDefinition; + var SchemaNode result = grouping; + val List path = targetPath.path + for (node : path) { + // finding by local name is valid, grouping cannot contain nodes with same name and different namespace + if (result instanceof DataNodeContainer) { + result = (result as DataNodeContainer).getDataChildByName(node.localName) + } else if (result instanceof ChoiceNode) { + result = (result as ChoiceNode).getCaseNodeByName(node.localName) + } + } if (result == null) { return null; - } - var boolean fromUses = result.addedByUses; - + } + + val String targetSchemaNodeName = result.QName.localName; + var boolean fromUses = (result as DataSchemaNode).addedByUses var Iterator groupingUses = grouping.uses.iterator; while (fromUses) { if (groupingUses.hasNext()) { grouping = findNodeInSchemaContext(schemaContext, groupingUses.next().groupingPath.path) as GroupingDefinition; result = grouping.getDataChildByName(targetSchemaNodeName); - fromUses = result.addedByUses; + fromUses = (result as DataSchemaNode).addedByUses; } else { throw new NullPointerException("Failed to generate code for augment in " + parentUsesNode); } } - return result; + return result as DataSchemaNode } /** diff --git a/code-generator/binding-generator-util/src/main/java/org/opendaylight/yangtools/binding/generator/util/BindingGeneratorUtil.java b/code-generator/binding-generator-util/src/main/java/org/opendaylight/yangtools/binding/generator/util/BindingGeneratorUtil.java index 0bb1979417..34d1760d8b 100644 --- a/code-generator/binding-generator-util/src/main/java/org/opendaylight/yangtools/binding/generator/util/BindingGeneratorUtil.java +++ b/code-generator/binding-generator-util/src/main/java/org/opendaylight/yangtools/binding/generator/util/BindingGeneratorUtil.java @@ -187,6 +187,10 @@ public final class BindingGeneratorUtil { return validateJavaPackage(packageNameBuilder.toString()); } + public static String packageNameForGeneratedType(final String basePackageName, final SchemaPath schemaPath) { + return packageNameForGeneratedType(basePackageName, schemaPath, false); + } + /** * Creates package name from specified basePackageName (package * name for module) and schemaPath. @@ -202,7 +206,8 @@ public final class BindingGeneratorUtil { * name of this node * @return string with valid JAVA package name */ - public static String packageNameForGeneratedType(final String basePackageName, final SchemaPath schemaPath) { + public static String packageNameForGeneratedType(final String basePackageName, final SchemaPath schemaPath, + boolean isUsesAugment) { if (basePackageName == null) { throw new IllegalArgumentException("Base Package Name cannot be NULL!"); } @@ -213,7 +218,12 @@ public final class BindingGeneratorUtil { final StringBuilder builder = new StringBuilder(); builder.append(basePackageName); final List pathToNode = schemaPath.getPath(); - final int traversalSteps = (pathToNode.size() - 1); + final int traversalSteps; + if (isUsesAugment) { + traversalSteps = (pathToNode.size()); + } else { + traversalSteps = (pathToNode.size() - 1); + } for (int i = 0; i < traversalSteps; ++i) { builder.append("."); String nodeLocalName = pathToNode.get(i).getLocalName(); diff --git a/code-generator/binding-java-api-generator/src/test/java/org/opendaylight/yangtools/sal/java/api/generator/test/CompilationTest.java b/code-generator/binding-java-api-generator/src/test/java/org/opendaylight/yangtools/sal/java/api/generator/test/CompilationTest.java index 579b0978a3..3469b71625 100644 --- a/code-generator/binding-java-api-generator/src/test/java/org/opendaylight/yangtools/sal/java/api/generator/test/CompilationTest.java +++ b/code-generator/binding-java-api-generator/src/test/java/org/opendaylight/yangtools/sal/java/api/generator/test/CompilationTest.java @@ -133,16 +133,25 @@ public class CompilationTest extends BaseCompilationTest { // Test if all sources were generated from 'module foo' File parent = new File(sourcesOutputDir, NS_FOO); assertTrue(new File(parent, "Object.java").exists()); + assertTrue(new File(parent, "ClosedObject.java").exists()); assertTrue(new File(parent, "OpenObject.java").exists()); assertTrue(new File(parent, "ExplicitRouteObject.java").exists()); assertTrue(new File(parent, "PathKeySubobject.java").exists()); - assertFilesCount(parent, 7); + assertFilesCount(parent, 9); parent = new File(parent, "object"); assertTrue(new File(parent, "Nodes.java").exists()); assertTrue(new File(parent, "NodesBuilder.java").exists()); assertFilesCount(parent, 2); + parent = new File(sourcesOutputDir, NS_FOO + FS + "closed"); + assertFilesCount(parent, 1); + + parent = new File(parent, "object"); + assertTrue(new File(parent, "Link1.java").exists()); + assertTrue(new File(parent, "Link1Builder.java").exists()); + assertFilesCount(parent, 2); + parent = new File(sourcesOutputDir, NS_FOO + FS + "open"); assertFilesCount(parent, 1); @@ -185,9 +194,12 @@ public class CompilationTest extends BaseCompilationTest { parent = new File(sourcesOutputDir, NS_BAR); assertTrue(new File(parent, "BasicExplicitRouteSubobjects.java").exists()); assertTrue(new File(parent, "ExplicitRouteSubobjects.java").exists()); - assertFilesCount(parent, 3); + assertTrue(new File(parent, "RouteSubobjects.java").exists()); + assertFilesCount(parent, 5); - parent = new File(parent, "basic"); + parent = new File(parent, "route"); + assertFilesCount(parent, 1); + parent = new File(new File(sourcesOutputDir, NS_BAR), "basic"); assertFilesCount(parent, 1); parent = new File(parent, "explicit"); assertFilesCount(parent, 1); diff --git a/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/bar.yang b/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/bar.yang index 940b491c8d..7dee386efe 100644 --- a/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/bar.yang +++ b/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/bar.yang @@ -26,4 +26,11 @@ module bar { uses basic-explicit-route-subobjects; } + grouping route-subobjects { + container links { + container link { + } + } + } + } diff --git a/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/foo.yang b/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/foo.yang index 58df3ca67d..f1cb5f8c1d 100644 --- a/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/foo.yang +++ b/code-generator/binding-java-api-generator/src/test/resources/compilation/augment-under-uses/foo.yang @@ -32,6 +32,16 @@ module foo { } } + grouping closed-object { + uses b:route-subobjects { + augment "links/link" { + leaf id { + type string; + } + } + } + } + grouping explicit-route-object { list subobjects { leaf loose { -- 2.36.6