Do not use SchemaNode.getPath() in RestDocgenUtil

Instead of investigating full schema path we can just use
the parent node QName to determine if a child is in the same module
as its parent.

To compare name-space and revision we work with QNames -
we have adapted method resolvePathArgumentsName(..) parameters
to require QNames.

We can go further and mark all parameters as NonNull because
the use-case when parent is null has no sense - we are at direct
child of module - so full path should be used.

Align parameter usage across the methods -
first is node's QName the second is node's parent's QName.

In addition, mark not-used resolveNodesName(SchemaNode, Module) method
as deprecated for removal.

To simplify situation in callers we can resolve resource path before
addPaths or addOperations methods are called. This way we do not need
to increase the number of parameters.

JIRA: NETCONF-819
Change-Id: Ib7a7614a70c9573521c47716c62ec74f887e6132
Signed-off-by: Manoj Chokka <cmanoj8@gmail.com>
Signed-off-by: OleksandrZharov <Oleksandr.Zharov@pantheon.tech>
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>

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 074e043049bab073f57c34b5ab929a8b61517eab..2d5593c6e04c9a099f67e6b32a2cc73184f34795 100644 (file)
--- 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
@@ -329,6 +329,7 @@ public abstract class BaseYangSwaggerGenerator {
             if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
                 LOG.debug("Is Configuration node [{}] [{}]", node.isConfiguration(), node.getQName().getLocalName());
 
+                final String localName = module.getName() + ":" + node.getQName().getLocalName();
                 ArrayNode pathParams = JsonNodeFactory.instance.arrayNode();
                 String resourcePath;
 
@@ -348,24 +349,27 @@ public abstract class BaseYangSwaggerGenerator {
                         hasAddRootPostLink = true;
                     }
 
-                    addPaths(node, deviceName, moduleName, paths, resourcePath, pathParams, schemaContext, true,
-                            module.getName(), definitionNames, uriType, oaversion);
+                    final String resolvedPath = resourcePath + "/" + createPath(node, pathParams, oaversion, localName);
+                    addPaths(node, deviceName, moduleName, paths, pathParams, schemaContext, true,
+                            module.getName(), definitionNames, uriType, oaversion, resolvedPath);
                 }
                 pathParams = JsonNodeFactory.instance.arrayNode();
                 resourcePath = getResourcePath("operational", context);
 
                 if (uriType.equals(URIType.DRAFT02)
                         || uriType.equals(URIType.RFC8040) && !node.isConfiguration()) {
-                    addPaths(node, deviceName, moduleName, paths, resourcePath, pathParams, schemaContext, false,
-                            moduleName, definitionNames, uriType, oaversion);
+                    final String resolvedPath = resourcePath + "/" + createPath(node, pathParams, oaversion, localName);
+                    addPaths(node, deviceName, moduleName, paths, pathParams, schemaContext, false,
+                            moduleName, definitionNames, uriType, oaversion, resolvedPath);
                 }
             }
         }
 
         for (final RpcDefinition rpcDefinition : module.getRpcs()) {
-            final String resourcePath = getResourcePath("operations", context);
-            addOperations(rpcDefinition, moduleName, deviceName, paths, resourcePath, module.getName(), definitionNames,
-                    schemaContext, oaversion);
+            final String resolvedPath = getResourcePath("operations", context) + "/" + moduleName + ":"
+                    + rpcDefinition.getQName().getLocalName();
+            addOperations(rpcDefinition, moduleName, deviceName, paths, module.getName(), definitionNames, oaversion,
+                    resolvedPath);
         }
 
         LOG.debug("Number of Paths found [{}]", paths.size());
@@ -440,13 +444,13 @@ public abstract class BaseYangSwaggerGenerator {
     }
 
     private void addPaths(final DataSchemaNode node, final Optional<String> deviceName, final String moduleName,
-                          final ObjectNode paths, final String parentPath, final ArrayNode parentPathParams,
+                          final ObjectNode paths, final ArrayNode parentPathParams,
                           final EffectiveModelContext schemaContext, final boolean isConfig, final String parentName,
-                          final DefinitionNames definitionNames, final URIType uriType, final OAversion oaversion) {
-        final ArrayNode pathParams = JsonUtil.copy(parentPathParams);
-        final String resourcePath = parentPath + "/" + createPath(node, pathParams, schemaContext, oaversion);
+                          final DefinitionNames definitionNames, final URIType uriType, final OAversion oaversion,
+                          final String resourcePath) {
         LOG.debug("Adding path: [{}]", resourcePath);
 
+        final ArrayNode pathParams = JsonUtil.copy(parentPathParams);
         Iterable<? extends DataSchemaNode> childSchemaNodes = Collections.emptySet();
         if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
             final DataNodeContainer dataNodeContainer = (DataNodeContainer) node;
@@ -458,26 +462,29 @@ public abstract class BaseYangSwaggerGenerator {
                 uriType, oaversion));
         paths.set(resourcePath, path);
 
-
         if (uriType.equals(URIType.RFC8040)) {
-            final String operationPath = "rests/operations" + resourcePath.substring(11);
-            ((ActionNodeContainer) node).getActions().forEach(actionDef ->
-                    addOperations(actionDef, moduleName, deviceName, paths, operationPath, parentName, definitionNames,
-                            schemaContext, oaversion));
+            ((ActionNodeContainer) node).getActions().forEach(actionDef -> {
+                final String resolvedPath = "rests/operations" + resourcePath.substring(11)
+                        + "/" + resolvePathArgumentsName(actionDef.getQName(), node.getQName(), schemaContext);
+                addOperations(actionDef, moduleName, deviceName, paths, parentName, definitionNames, oaversion,
+                        resolvedPath);
+            });
         }
 
-
         for (final DataSchemaNode childNode : childSchemaNodes) {
             if (childNode instanceof ListSchemaNode || childNode instanceof ContainerSchemaNode) {
                 final String newParent = parentName + "_" + node.getQName().getLocalName();
+                final String localName = resolvePathArgumentsName(childNode.getQName(), node.getQName(), schemaContext);
+                final String newResourcePath = resourcePath + "/" + createPath(childNode, pathParams, oaversion,
+                        localName);
                 if (uriType.equals(URIType.RFC8040)) {
                     final boolean newIsConfig = isConfig && childNode.isConfiguration();
-                    addPaths(childNode, deviceName, moduleName, paths, resourcePath, pathParams, schemaContext,
-                            newIsConfig, newParent, definitionNames, uriType, oaversion);
+                    addPaths(childNode, deviceName, moduleName, paths, pathParams, schemaContext,
+                            newIsConfig, newParent, definitionNames, uriType, oaversion, newResourcePath);
                 } else {
                     if (!isConfig || childNode.isConfiguration()) {
-                        addPaths(childNode, deviceName, moduleName, paths, resourcePath, pathParams, schemaContext,
-                                isConfig, newParent, definitionNames, uriType, oaversion);
+                        addPaths(childNode, deviceName, moduleName, paths, pathParams, schemaContext,
+                                isConfig, newParent, definitionNames, uriType, oaversion, newResourcePath);
                     }
                 }
             }
@@ -531,9 +538,8 @@ public abstract class BaseYangSwaggerGenerator {
     protected abstract ListPathBuilder newListPathBuilder();
 
     private String createPath(final DataSchemaNode schemaNode, final ArrayNode pathParams,
-                              final EffectiveModelContext schemaContext, final OAversion oaversion) {
+                              final OAversion oaversion, final String localName) {
         final StringBuilder path = new StringBuilder();
-        final String localName = resolvePathArgumentsName(schemaNode, schemaContext);
         path.append(localName);
 
         if (schemaNode instanceof ListSchemaNode) {
@@ -610,11 +616,9 @@ public abstract class BaseYangSwaggerGenerator {
     }
 
     private static void addOperations(final OperationDefinition operDef, final String moduleName,
-            final Optional<String> deviceName, final ObjectNode paths, final String parentPath, final String parentName,
-            final DefinitionNames definitionNames, final EffectiveModelContext schemaContext,
-            final OAversion oaversion) {
+            final Optional<String> deviceName, final ObjectNode paths, final String parentName,
+            final DefinitionNames definitionNames, final OAversion oaversion, final String resourcePath) {
         final ObjectNode operations = JsonNodeFactory.instance.objectNode();
-        final String resourcePath = parentPath + "/" + resolvePathArgumentsName(operDef, schemaContext);
         operations.set("post", buildPostOperation(operDef, moduleName, deviceName, parentName, definitionNames,
                 oaversion));
         paths.set(resourcePath, operations);

diff --git a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/util/RestDocgenUtil.java b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/util/RestDocgenUtil.java
index bafecbc070b63d36ca6b485ee60ccc73ae07b526..8105fb2352a7e7206c9cec8fb3d8c488284e872a 100644 (file)
--- a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/util/RestDocgenUtil.java
+++ b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/util/RestDocgenUtil.java
@@ -8,12 +8,13 @@
 package org.opendaylight.netconf.sal.rest.doc.util;
 
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Optional;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.Revision;
 import org.opendaylight.yangtools.yang.common.XMLNamespace;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
 import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.model.api.SchemaNode;
@@ -35,53 +36,48 @@ public final class RestDocgenUtil {
      *
      * @return name of {@code node}
      */
-    public static String resolvePathArgumentsName(final SchemaNode node, final SchemaContext schemaContext) {
-        final Iterable<QName> schemaPath = node.getPath().getPathTowardsRoot();
-        final Iterator<QName> it = schemaPath.iterator();
-        final QName nodeQName = it.next();
-
-        QName parentQName = null;
-        if (it.hasNext()) {
-            parentQName = it.next();
-        }
-        if (isEqualNamespaceAndRevision(parentQName, nodeQName)) {
-            return node.getQName().getLocalName();
+    public static String resolvePathArgumentsName(@NonNull final QName node, @NonNull final QName parent,
+                                                  @NonNull final EffectiveModelContext schemaContext) {
+        if (isEqualNamespaceAndRevision(node, parent)) {
+            return node.getLocalName();
         } else {
             return resolveFullNameFromNode(node, schemaContext);
         }
     }
 
-    private static synchronized String resolveFullNameFromNode(final SchemaNode node,
-            final SchemaContext schemaContext) {
-        final XMLNamespace namespace = node.getQName().getNamespace();
-        final Optional<Revision> revision = node.getQName().getRevision();
-
-        Map<Optional<Revision>, Module> revisionToModule =
-            NAMESPACE_AND_REVISION_TO_MODULE.computeIfAbsent(namespace, k -> new HashMap<>());
-        Module module =
-            revisionToModule.computeIfAbsent(revision, k -> schemaContext.findModule(namespace, k).orElse(null));
-        if (module != null) {
-            return module.getName() + ":" + node.getQName().getLocalName();
-        }
-        return node.getQName().getLocalName();
-    }
-
+    /*
+     * Resolve full name according to module and node namespace and revision equality.
+     *
+     * @deprecated Most likely this method is useless because when we are going from module to its direct children
+     * there is no need for reasoning if we should use full name.
+     */
+    @Deprecated(forRemoval = true)
     public static String resolveNodesName(final SchemaNode node, final Module module,
             final SchemaContext schemaContext) {
         if (node.getQName().getNamespace().equals(module.getQNameModule().getNamespace())
                 && node.getQName().getRevision().equals(module.getQNameModule().getRevision())) {
             return node.getQName().getLocalName();
         } else {
-            return resolveFullNameFromNode(node, schemaContext);
+            return resolveFullNameFromNode(node.getQName(), schemaContext);
         }
     }
 
-    private static boolean isEqualNamespaceAndRevision(final QName parentQName, final QName nodeQName) {
-        if (parentQName == null) {
-            return nodeQName == null;
-        }
-        return parentQName.getNamespace().equals(nodeQName.getNamespace())
-                && parentQName.getRevision().equals(nodeQName.getRevision());
+    private static boolean isEqualNamespaceAndRevision(final QName node, final QName parent) {
+        return parent.getNamespace().equals(node.getNamespace())
+                && parent.getRevision().equals(node.getRevision());
     }
 
+    private static String resolveFullNameFromNode(final QName node, final SchemaContext schemaContext) {
+        final XMLNamespace namespace = node.getNamespace();
+        final Optional<Revision> revision = node.getRevision();
+
+        final Map<Optional<Revision>, Module> revisionToModule =
+            NAMESPACE_AND_REVISION_TO_MODULE.computeIfAbsent(namespace, k -> new HashMap<>());
+        final Module module = revisionToModule.computeIfAbsent(revision,
+                k -> schemaContext.findModule(namespace, k).orElse(null));
+        if (module != null) {
+            return module.getName() + ":" + node.getLocalName();
+        }
+        return node.getLocalName();
+    }
 }