From 9c15778ec0f42719fa98f60e84288f12ed85aafa Mon Sep 17 00:00:00 2001 From: "miroslav.kovac" Date: Mon, 10 Oct 2016 18:53:01 +0200 Subject: [PATCH] Bug 6679 - api explorer creates false examples Api explorer created false examples when yang has two data nodes with same name in different containers or lists. It compared name of data nodes rather than whole path to the data node which result to creting examples of same types even if they should be different. Fix examples by comparing the path to data node rather than just name of the node. Change-Id: I6f73fb37985fc78ad67ecc466ebfdfe569d7cd8d Signed-off-by: miroslav.kovac --- .../doc/impl/BaseYangSwaggerGenerator.java | 27 +++++----- .../sal/rest/doc/impl/ModelGenerator.java | 36 +++++++------ .../doc/model/builder/OperationBuilder.java | 15 +++--- .../rest/doc/impl/ApiDocGeneratorTest.java | 53 ++++++++++--------- 4 files changed, 69 insertions(+), 62 deletions(-) diff --git a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/BaseYangSwaggerGenerator.java b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/BaseYangSwaggerGenerator.java index fd750b4540..abf560ea78 100644 --- a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/BaseYangSwaggerGenerator.java +++ b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/BaseYangSwaggerGenerator.java @@ -201,12 +201,12 @@ public class BaseYangSwaggerGenerator { hasAddRootPostLink = true; } - addApis(node, apis, resourcePath, pathParams, schemaContext, true); + addApis(node, apis, resourcePath, pathParams, schemaContext, true, m.getName()); } pathParams = new ArrayList<>(); resourcePath = getDataStorePath("/operational/", context); - addApis(node, apis, resourcePath, pathParams, schemaContext, false); + addApis(node, apis, resourcePath, pathParams, schemaContext, false, m.getName()); } } @@ -243,7 +243,7 @@ public class BaseYangSwaggerGenerator { final Api apiForRootPostUri = new Api(); apiForRootPostUri.setPath(resourcePath); apiForRootPostUri.setOperations(operationPost(module.getName() + MODULE_NAME_SUFFIX, - module.getDescription(), module, pathParams, true)); + module.getDescription(), module, pathParams, true, "")); apis.add(apiForRootPostUri); } } @@ -266,7 +266,7 @@ public class BaseYangSwaggerGenerator { } private void addApis(final DataSchemaNode node, final List apis, final String parentPath, final List parentPathParams, final SchemaContext schemaContext, - final boolean addConfigApi) { + final boolean addConfigApi, final String parentName) { final Api api = new Api(); final List pathParams = new ArrayList<>(parentPathParams); @@ -280,18 +280,18 @@ public class BaseYangSwaggerGenerator { final DataNodeContainer dataNodeContainer = (DataNodeContainer) node; childSchemaNodes = dataNodeContainer.getChildNodes(); } - api.setOperations(operation(node, pathParams, addConfigApi, childSchemaNodes)); + api.setOperations(operation(node, pathParams, addConfigApi, childSchemaNodes, parentName)); apis.add(api); for (final DataSchemaNode childNode : childSchemaNodes) { if (childNode instanceof ListSchemaNode || childNode instanceof ContainerSchemaNode) { // keep config and operation attributes separate. if (childNode.isConfiguration() == addConfigApi) { - addApis(childNode, apis, resourcePath, pathParams, schemaContext, addConfigApi); + final String newParent = parentName + "/" + node.getQName().getLocalName(); + addApis(childNode, apis, resourcePath, pathParams, schemaContext, addConfigApi, newParent); } } } - } private boolean containsListOrContainer(final Iterable nodes) { @@ -303,15 +303,15 @@ public class BaseYangSwaggerGenerator { return false; } - private List operation(final DataSchemaNode node, final List pathParams, final boolean isConfig, final Iterable childSchemaNodes) { + private List operation(final DataSchemaNode node, final List pathParams, final boolean isConfig, + final Iterable childSchemaNodes, final String parentName) { final List operations = new ArrayList<>(); final Get getBuilder = new Get(node, isConfig); operations.add(getBuilder.pathParams(pathParams).build()); if (isConfig) { - final Put putBuilder = new Put(node.getQName().getLocalName(), - node.getDescription()); + final Put putBuilder = new Put(node.getQName().getLocalName(), node.getDescription(), parentName); operations.add(putBuilder.pathParams(pathParams).build()); final Delete deleteBuilder = new Delete(node); @@ -319,16 +319,17 @@ public class BaseYangSwaggerGenerator { if (containsListOrContainer(childSchemaNodes)) { operations.addAll(operationPost(node.getQName().getLocalName(), node.getDescription(), - (DataNodeContainer) node, pathParams, isConfig)); + (DataNodeContainer) node, pathParams, isConfig, parentName + "/")); } } return operations; } - private List operationPost(final String name, final String description, final DataNodeContainer dataNodeContainer, final List pathParams, final boolean isConfig) { + private List operationPost(final String name, final String description, final DataNodeContainer dataNodeContainer, + final List pathParams, final boolean isConfig, final String parentName) { final List operations = new ArrayList<>(); if (isConfig) { - final Post postBuilder = new Post(name, description, dataNodeContainer); + final Post postBuilder = new Post(name, parentName + name, description, dataNodeContainer); operations.add(postBuilder.pathParams(pathParams).build()); } return operations; diff --git a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/ModelGenerator.java b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/ModelGenerator.java index 5813f1e4de..24768e8368 100644 --- a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/ModelGenerator.java +++ b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/ModelGenerator.java @@ -122,7 +122,8 @@ public class ModelGenerator { } private void processModules(final Module module, final JSONObject models) throws JSONException { - createConcreteModelForPost(models, module.getName() + BaseYangSwaggerGenerator.MODULE_NAME_SUFFIX, createPropertiesForPost(module)); + createConcreteModelForPost(models, module.getName() + BaseYangSwaggerGenerator.MODULE_NAME_SUFFIX, + createPropertiesForPost(module, module.getName())); } private void processContainersAndLists(final Module module, final JSONObject models, final SchemaContext schemaContext) @@ -253,15 +254,15 @@ public class ModelGenerator { } } - private JSONObject processDataNodeContainer(final DataNodeContainer dataNode, final String moduleName, final JSONObject models, + private JSONObject processDataNodeContainer(final DataNodeContainer dataNode, final String parentName, final JSONObject models, final boolean isConfig, final SchemaContext schemaContext) throws JSONException, IOException { if (dataNode instanceof ListSchemaNode || dataNode instanceof ContainerSchemaNode) { Preconditions.checkArgument(dataNode instanceof SchemaNode, "Data node should be also schema node"); final Iterable containerChildren = dataNode.getChildNodes(); - final JSONObject properties = processChildren(containerChildren, moduleName, models, isConfig, schemaContext); - - final String nodeName = (isConfig ? OperationBuilder.CONFIG : OperationBuilder.OPERATIONAL) - + ((SchemaNode) dataNode).getQName().getLocalName(); + final String localName = ((SchemaNode) dataNode).getQName().getLocalName(); + final JSONObject properties = processChildren(containerChildren, parentName + "/" + localName, models, isConfig, schemaContext); + final String nodeName = parentName + (isConfig ? OperationBuilder.CONFIG : OperationBuilder.OPERATIONAL) + + localName; final JSONObject childSchema = getSchemaTemplate(); childSchema.put(TYPE_KEY, OBJECT_TYPE); @@ -271,8 +272,8 @@ public class ModelGenerator { models.put(nodeName, childSchema); if (isConfig) { - createConcreteModelForPost(models, ((SchemaNode) dataNode).getQName().getLocalName(), - createPropertiesForPost(dataNode)); + createConcreteModelForPost(models, localName, + createPropertiesForPost(dataNode, parentName + "/" + localName)); } return processTopData(nodeName, models, (SchemaNode) dataNode); @@ -290,12 +291,13 @@ public class ModelGenerator { models.put(nodePostName, postSchema); } - private JSONObject createPropertiesForPost(final DataNodeContainer dataNodeContainer) throws JSONException { + private JSONObject createPropertiesForPost(final DataNodeContainer dataNodeContainer, final String parentName) + throws JSONException { final JSONObject properties = new JSONObject(); for (final DataSchemaNode childNode : dataNodeContainer.getChildNodes()) { if (childNode instanceof ListSchemaNode || childNode instanceof ContainerSchemaNode) { final JSONObject items = new JSONObject(); - items.put(REF_KEY, "(config)" + childNode.getQName().getLocalName()); + items.put(REF_KEY, parentName + "(config)" + childNode.getQName().getLocalName()); final JSONObject property = new JSONObject(); property.put(TYPE_KEY, childNode instanceof ListSchemaNode ? ARRAY_TYPE : OBJECT_TYPE); property.put(ITEMS_KEY, items); @@ -316,7 +318,7 @@ public class ModelGenerator { /** * Processes the nodes. */ - private JSONObject processChildren(final Iterable nodes, final String moduleName, final JSONObject models, + private JSONObject processChildren(final Iterable nodes, final String parentName, final JSONObject models, final boolean isConfig, final SchemaContext schemaContext) throws JSONException, IOException { final JSONObject properties = new JSONObject(); @@ -328,20 +330,20 @@ public class ModelGenerator { property = processLeafNode((LeafSchemaNode) node); } else if (node instanceof ListSchemaNode) { - property = processDataNodeContainer((ListSchemaNode) node, moduleName, models, isConfig, + property = processDataNodeContainer((ListSchemaNode) node, parentName, models, isConfig, schemaContext); } else if (node instanceof LeafListSchemaNode) { property = processLeafListNode((LeafListSchemaNode) node); } else if (node instanceof ChoiceSchemaNode) { - property = processChoiceNode((ChoiceSchemaNode) node, moduleName, models, schemaContext); + property = processChoiceNode((ChoiceSchemaNode) node, parentName, models, schemaContext); } else if (node instanceof AnyXmlSchemaNode) { property = processAnyXMLNode((AnyXmlSchemaNode) node); } else if (node instanceof ContainerSchemaNode) { - property = processDataNodeContainer((ContainerSchemaNode) node, moduleName, models, isConfig, + property = processDataNodeContainer((ContainerSchemaNode) node, parentName, models, isConfig, schemaContext); } else { @@ -368,15 +370,15 @@ public class ModelGenerator { return props; } - private JSONObject processChoiceNode(final ChoiceSchemaNode choiceNode, final String moduleName, final JSONObject models, - final SchemaContext schemaContext) throws JSONException, IOException { + private JSONObject processChoiceNode(final ChoiceSchemaNode choiceNode, final String parentName, final JSONObject models, + final SchemaContext schemaContext) throws JSONException, IOException { final Set cases = choiceNode.getCases(); final JSONArray choiceProps = new JSONArray(); for (final ChoiceCaseNode choiceCase : cases) { final String choiceName = choiceCase.getQName().getLocalName(); - final JSONObject choiceProp = processChildren(choiceCase.getChildNodes(), moduleName, models, schemaContext); + final JSONObject choiceProp = processChildren(choiceCase.getChildNodes(), parentName, models, schemaContext); final JSONObject choiceObj = new JSONObject(); choiceObj.put(choiceName, choiceProp); choiceObj.put(TYPE_KEY, OBJECT_TYPE); diff --git a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/model/builder/OperationBuilder.java b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/model/builder/OperationBuilder.java index 385a08b485..d24636bfee 100644 --- a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/model/builder/OperationBuilder.java +++ b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/model/builder/OperationBuilder.java @@ -58,12 +58,14 @@ public final class OperationBuilder { public static class Put { protected Operation spec; protected String nodeName; + protected String parentName; private static final String METHOD_NAME = "PUT"; - public Put(final String nodeName, final String description) { + public Put(final String nodeName, final String description, final String parentName) { this.nodeName = nodeName; + this.parentName = parentName; spec = new Operation(); - spec.setType(CONFIG + nodeName); + spec.setType(parentName + CONFIG + nodeName + TOP); spec.setNotes(description); spec.setConsumes(CONSUMES_PUT_POST); } @@ -72,7 +74,7 @@ public final class OperationBuilder { final List parameters = new ArrayList<>(params); final Parameter payload = new Parameter(); payload.setParamType("body"); - payload.setType(CONFIG + nodeName + TOP); + payload.setType(parentName + CONFIG + nodeName + TOP); payload.setName(CONFIG + nodeName); parameters.add(payload); spec.setParameters(parameters); @@ -91,8 +93,8 @@ public final class OperationBuilder { public static final String METHOD_NAME = "POST"; private final DataNodeContainer dataNodeContainer; - public Post(final String nodeName, final String description, final DataNodeContainer dataNodeContainer) { - super(nodeName, description); + public Post(final String nodeName, final String parentName, final String description, final DataNodeContainer dataNodeContainer) { + super(nodeName, description, parentName.replace("_module", "")); this.dataNodeContainer = dataNodeContainer; spec.setType(CONFIG + nodeName + METHOD_NAME); spec.setConsumes(CONSUMES_PUT_POST); @@ -112,14 +114,13 @@ public final class OperationBuilder { if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) { final Parameter payload = new Parameter(); payload.setParamType("body"); - payload.setType(CONFIG + node.getQName().getLocalName() + TOP); + payload.setType(parentName + CONFIG + node.getQName().getLocalName() + TOP); payload.setName("**" + CONFIG + node.getQName().getLocalName()); parameters.add(payload); } } spec.setParameters(parameters); return this; - } public Post summary(final String summary) { diff --git a/restconf/sal-rest-docgen/src/test/java/org/opendaylight/controller/sal/rest/doc/impl/ApiDocGeneratorTest.java b/restconf/sal-rest-docgen/src/test/java/org/opendaylight/controller/sal/rest/doc/impl/ApiDocGeneratorTest.java index d0a200bcde..0202905080 100644 --- a/restconf/sal-rest-docgen/src/test/java/org/opendaylight/controller/sal/rest/doc/impl/ApiDocGeneratorTest.java +++ b/restconf/sal-rest-docgen/src/test/java/org/opendaylight/controller/sal/rest/doc/impl/ApiDocGeneratorTest.java @@ -94,12 +94,14 @@ public class ApiDocGeneratorTest { final Api lstApi = findApi("/config/toaster2:lst/", doc); assertNotNull("Api /config/toaster2:lst/ wasn't found", lstApi); assertTrue("POST for cont1 in lst is missing", - findOperation(lstApi.getOperations(), "POST", "(config)lstPOST", "(config)lst1-TOP", "(config)cont1-TOP")); + findOperation(lstApi.getOperations(), "POST", "(config)lstPOST", "toaster2/lst(config)lst1-TOP", + "toaster2/lst(config)cont1-TOP")); final Api cont1Api = findApi("/config/toaster2:lst/cont1/", doc); assertNotNull("Api /config/toaster2:lst/cont1/ wasn't found", cont1Api); assertTrue("POST for cont11 in cont1 is missing", - findOperation(cont1Api.getOperations(), "POST", "(config)cont1POST", "(config)cont11-TOP", "(config)lst11-TOP")); + findOperation(cont1Api.getOperations(), "POST", "(config)cont1POST", "toaster2/lst/cont1(config)cont11-TOP", + "toaster2/lst/cont1(config)lst11-TOP")); // no POST URI final Api cont11Api = findApi("/config/toaster2:lst/cont1/cont11/", doc); @@ -167,47 +169,47 @@ public class ApiDocGeneratorTest { final JSONObject models = doc.getModels(); assertNotNull(models); try { - final JSONObject configLstTop = models.getJSONObject("(config)lst-TOP"); + final JSONObject configLstTop = models.getJSONObject("toaster2(config)lst-TOP"); assertNotNull(configLstTop); - containsReferences(configLstTop, "lst"); + containsReferences(configLstTop, "lst", "toaster2(config)"); - final JSONObject configLst = models.getJSONObject("(config)lst"); + final JSONObject configLst = models.getJSONObject("toaster2(config)lst"); assertNotNull(configLst); - containsReferences(configLst, "lst1"); - containsReferences(configLst, "cont1"); + containsReferences(configLst, "lst1", "toaster2/lst(config)"); + containsReferences(configLst, "cont1", "toaster2/lst(config)"); - final JSONObject configLst1Top = models.getJSONObject("(config)lst1-TOP"); + final JSONObject configLst1Top = models.getJSONObject("toaster2/lst(config)lst1-TOP"); assertNotNull(configLst1Top); - containsReferences(configLst1Top, "lst1"); + containsReferences(configLst1Top, "lst1", "toaster2/lst(config)"); - final JSONObject configLst1 = models.getJSONObject("(config)lst1"); + final JSONObject configLst1 = models.getJSONObject("toaster2/lst(config)lst1"); assertNotNull(configLst1); - final JSONObject configCont1Top = models.getJSONObject("(config)cont1-TOP"); + final JSONObject configCont1Top = models.getJSONObject("toaster2/lst(config)cont1-TOP"); assertNotNull(configCont1Top); - containsReferences(configCont1Top, "cont1"); - final JSONObject configCont1 = models.getJSONObject("(config)cont1"); + containsReferences(configCont1Top, "cont1", "toaster2/lst(config)"); + final JSONObject configCont1 = models.getJSONObject("toaster2/lst(config)cont1"); assertNotNull(configCont1); - containsReferences(configCont1, "cont11"); - containsReferences(configCont1, "lst11"); + containsReferences(configCont1, "cont11", "toaster2/lst/cont1(config)"); + containsReferences(configCont1, "lst11", "toaster2/lst/cont1(config)"); - final JSONObject configCont11Top = models.getJSONObject("(config)cont11-TOP"); + final JSONObject configCont11Top = models.getJSONObject("toaster2/lst/cont1(config)cont11-TOP"); assertNotNull(configCont11Top); - containsReferences(configCont11Top, "cont11"); - final JSONObject configCont11 = models.getJSONObject("(config)cont11"); + containsReferences(configCont11Top, "cont11", "toaster2/lst/cont1(config)"); + final JSONObject configCont11 = models.getJSONObject("toaster2/lst/cont1(config)cont11"); assertNotNull(configCont11); - final JSONObject configlst11Top = models.getJSONObject("(config)lst11-TOP"); + final JSONObject configlst11Top = models.getJSONObject("toaster2/lst/cont1(config)lst11-TOP"); assertNotNull(configlst11Top); - containsReferences(configlst11Top, "lst11"); - final JSONObject configLst11 = models.getJSONObject("(config)lst11"); + containsReferences(configlst11Top, "lst11", "toaster2/lst/cont1(config)"); + final JSONObject configLst11 = models.getJSONObject("toaster2/lst/cont1(config)lst11"); assertNotNull(configLst11); } catch (final JSONException e) { fail("JSONException wasn't expected"); @@ -218,7 +220,8 @@ public class ApiDocGeneratorTest { /** * Checks whether object {@code mainObject} contains in properties/items key $ref with concrete value. */ - private void containsReferences(final JSONObject mainObject, final String childObject) throws JSONException { + private void containsReferences(final JSONObject mainObject, final String childObject, final String prefix) + throws JSONException { final JSONObject properties = mainObject.getJSONObject("properties"); assertNotNull(properties); @@ -229,7 +232,7 @@ public class ApiDocGeneratorTest { assertNotNull(itemsInNodeInProperties); final String itemRef = itemsInNodeInProperties.getString("$ref"); - assertEquals("(config)" + childObject, itemRef); + assertEquals(prefix + childObject, itemRef); } @Test @@ -354,12 +357,12 @@ public class ApiDocGeneratorTest { private void validateTosterDocContainsModulePrefixes(final ApiDeclaration doc) { final JSONObject topLevelJson = doc.getModels(); try { - final JSONObject configToaster = topLevelJson.getJSONObject("(config)toaster"); + final JSONObject configToaster = topLevelJson.getJSONObject("toaster2(config)toaster"); assertNotNull("(config)toaster JSON object missing", configToaster); // without module prefix containsProperties(configToaster, "toasterSlot"); - final JSONObject toasterSlot = topLevelJson.getJSONObject("(config)toasterSlot"); + final JSONObject toasterSlot = topLevelJson.getJSONObject("toaster2/toaster(config)toasterSlot"); assertNotNull("(config)toasterSlot JSON object missing", toasterSlot); // with module prefix containsProperties(toasterSlot, "toaster-augmented:slotInfo"); -- 2.36.6