Bug 6679 - api explorer creates false examples 51/46751/3
authormiroslav.kovac <miroslav.kovac@pantheon.tech>
Mon, 10 Oct 2016 16:53:01 +0000 (18:53 +0200)
committermiroslav.kovac <miroslav.kovac@pantheon.tech>
Wed, 12 Oct 2016 07:43:10 +0000 (09:43 +0200)
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 <miroslav.kovac@pantheon.tech>
restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/BaseYangSwaggerGenerator.java
restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/ModelGenerator.java
restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/model/builder/OperationBuilder.java
restconf/sal-rest-docgen/src/test/java/org/opendaylight/controller/sal/rest/doc/impl/ApiDocGeneratorTest.java

index fd750b45407860841024ad5ba193f24720e1804d..abf560ea78fe9d44cffa322a12b62f456e295f09 100644 (file)
@@ -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<Api> apis, final String parentPath, final List<Parameter> parentPathParams, final SchemaContext schemaContext,
-                         final boolean addConfigApi) {
+                         final boolean addConfigApi, final String parentName) {
 
         final Api api = new Api();
         final List<Parameter> 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<DataSchemaNode> nodes) {
@@ -303,15 +303,15 @@ public class BaseYangSwaggerGenerator {
         return false;
     }
 
-    private List<Operation> operation(final DataSchemaNode node, final List<Parameter> pathParams, final boolean isConfig, final Iterable<DataSchemaNode> childSchemaNodes) {
+    private List<Operation> operation(final DataSchemaNode node, final List<Parameter> pathParams, final boolean isConfig,
+                                      final Iterable<DataSchemaNode> childSchemaNodes, final String parentName) {
         final List<Operation> 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<Operation> operationPost(final String name, final String description, final DataNodeContainer dataNodeContainer, final List<Parameter> pathParams, final boolean isConfig) {
+    private List<Operation> operationPost(final String name, final String description, final DataNodeContainer dataNodeContainer,
+                                          final List<Parameter> pathParams, final boolean isConfig, final String parentName) {
         final List<Operation> 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;
index 5813f1e4def9ccb5eb7cee0452cc3071ac3f962b..24768e836864ff76261c9dff5a102153304cdd86 100644 (file)
@@ -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<DataSchemaNode> 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<DataSchemaNode> nodes, final String moduleName, final JSONObject models,
+    private JSONObject processChildren(final Iterable<DataSchemaNode> 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<ChoiceCaseNode> 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);
index 385a08b4852d3d286b0a8b7cd65d570fb5827498..d24636bfee060451de3cf7cda594036fbfc027e1 100644 (file)
@@ -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<Parameter> 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) {
index d0a200bcde0f2e5e17d110f87e486fea07083997..020290508097db36cf21a3fb0f65fabd94d42b7b 100644 (file)
@@ -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");