Fix docgen failure on multi-level choice definition

+ minor refactoring: duplicate code replaced with recursive function

JIRA: NETCONF-883
Change-Id: Ie577f930fe7fc104bc92a93286a2788b93c04ac6
Signed-off-by: Ruslan Kashapov <ruslan.kashapov@pantheon.tech>

diff --git a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/DefinitionGenerator.java b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/DefinitionGenerator.java
index a313776ae4aeb1f4bc9cab429d4d129da9b2f86b..e49c7cf1f2d57f1e536c70ead3e8fa75fa8ae440 100644 (file)
--- a/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/DefinitionGenerator.java
+++ b/restconf/sal-rest-docgen/src/main/java/org/opendaylight/netconf/sal/rest/doc/impl/DefinitionGenerator.java
@@ -480,55 +480,74 @@ public class DefinitionGenerator {
         final ObjectNode properties = JsonNodeFactory.instance.objectNode();
         final ArrayNode required = JsonNodeFactory.instance.arrayNode();
         for (final DataSchemaNode node : nodes) {
-            stack.enterSchemaTree(node.getQName());
             if (!isConfig || node.isConfiguration()) {
-                /*
-                    Add module name prefix to property name, when needed, when ServiceNow can process colons,
-                    use RestDocGenUtil#resolveNodesName for creating property name
-                 */
-                final String propertyName = node.getQName().getLocalName();
-                final ObjectNode property;
-                if (node instanceof LeafSchemaNode leaf) {
-                    processLeafNode(leaf, propertyName, properties, required, stack, definitions, definitionNames,
-                            oaversion);
-                } else if (node instanceof AnyxmlSchemaNode anyxml) {
-                    processAnyXMLNode(anyxml, propertyName, properties, required);
-                } else if (node instanceof AnydataSchemaNode anydata) {
-                    processAnydataNode(anydata, propertyName, properties, required);
-                } else {
-                    if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
-                        property = processDataNodeContainer((DataNodeContainer) node, parentName, definitions,
-                                definitionNames, isConfig, stack, oaversion);
-                        if (!isConfig) {
-                            processActionNodeContainer(node, parentName, definitions, definitionNames, stack,
-                                    oaversion);
-                        }
-                    } else if (node instanceof LeafListSchemaNode leafList) {
-                        property = processLeafListNode(leafList, stack, definitions, definitionNames, oaversion);
-
-                    } else if (node instanceof ChoiceSchemaNode choice) {
-                        for (final CaseSchemaNode variant : choice.getCases()) {
-                            stack.enterSchemaTree(variant.getQName());
-                            processChoiceNode(variant.getChildNodes(), parentName, definitions, definitionNames,
-                                    isConfig, stack, properties, oaversion);
-                            stack.exit();
-                        }
-                        stack.exit();
-                        // FIXME dangerous statement here! Try to rework without continue.
-                        continue;
-                    } else {
-                        throw new IllegalArgumentException("Unknown DataSchemaNode type: " + node.getClass());
-                    }
-                    properties.set(propertyName, property);
-                }
+                processChildNode(node, parentName, definitions, definitionNames, isConfig, stack, properties,
+                        oaversion);
             }
-            stack.exit();
         }
         parentNode.set(PROPERTIES_KEY, properties);
         setRequiredIfNotEmpty(parentNode, required);
         return properties;
     }
 
+    private void processChildNode(
+            final DataSchemaNode node, final String parentName, final ObjectNode definitions,
+            final DefinitionNames definitionNames, final boolean isConfig,
+            final SchemaInferenceStack stack, final ObjectNode properties, final OAversion oaversion)
+            throws IOException {
+
+        stack.enterSchemaTree(node.getQName());
+
+        /*
+            Add module name prefix to property name, when needed, when ServiceNow can process colons,
+            use RestDocGenUtil#resolveNodesName for creating property name
+         */
+        final String name = node.getQName().getLocalName();
+
+        if (node instanceof LeafSchemaNode leaf) {
+            processLeafNode(leaf, name, properties, JsonNodeFactory.instance.arrayNode(), stack, definitions,
+                    definitionNames, oaversion);
+
+        } else if (node instanceof AnyxmlSchemaNode anyxml) {
+            processAnyXMLNode(anyxml, name, properties, JsonNodeFactory.instance.arrayNode());
+
+        } else if (node instanceof AnydataSchemaNode anydata) {
+            processAnydataNode(anydata, name, properties, JsonNodeFactory.instance.arrayNode());
+
+        } else {
+
+            final ObjectNode property;
+            if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
+                property = processDataNodeContainer((DataNodeContainer) node, parentName, definitions,
+                        definitionNames, isConfig, stack, oaversion);
+                if (!isConfig) {
+                    processActionNodeContainer(node, parentName, definitions, definitionNames, stack, oaversion);
+                }
+            } else if (node instanceof LeafListSchemaNode leafList) {
+                property = processLeafListNode(leafList, stack, definitions, definitionNames, oaversion);
+
+            } else if (node instanceof ChoiceSchemaNode choice) {
+                for (final CaseSchemaNode variant : choice.getCases()) {
+                    stack.enterSchemaTree(variant.getQName());
+                    for (final DataSchemaNode childNode : variant.getChildNodes()) {
+                        processChildNode(childNode, parentName, definitions, definitionNames, isConfig, stack,
+                                properties, oaversion);
+                    }
+                    stack.exit();
+                }
+                property = null;
+
+            } else {
+                throw new IllegalArgumentException("Unknown DataSchemaNode type: " + node.getClass());
+            }
+            if (property != null) {
+                properties.set(name, property);
+            }
+        }
+
+        stack.exit();
+    }
+
     private ObjectNode processLeafListNode(final LeafListSchemaNode listNode, final SchemaInferenceStack stack,
                                            final ObjectNode definitions, final DefinitionNames definitionNames,
                                            final OAversion oaversion) {
@@ -547,57 +566,6 @@ public class DefinitionGenerator {
         return props;
     }
 
-    private void processChoiceNode(
-            final Iterable<? extends DataSchemaNode> nodes, final String parentName, final ObjectNode definitions,
-            final DefinitionNames definitionNames, final boolean isConfig,
-            final SchemaInferenceStack stack, final ObjectNode properties, final OAversion oaversion)
-            throws IOException {
-        for (final DataSchemaNode node : nodes) {
-            stack.enterSchemaTree(node.getQName());
-            /*
-                Add module name prefix to property name, when needed, when ServiceNow can process colons,
-                use RestDocGenUtil#resolveNodesName for creating property name
-             */
-            final String name = node.getQName().getLocalName();
-            final ObjectNode property;
-
-            /*
-                Ignore mandatoriness(passing unreferenced arrayNode to process...Node), because choice produces multiple
-                properties
-             */
-            if (node instanceof LeafSchemaNode leaf) {
-                processLeafNode(leaf, name, properties, JsonNodeFactory.instance.arrayNode(), stack, definitions,
-                        definitionNames, oaversion);
-            } else if (node instanceof AnyxmlSchemaNode anyxml) {
-                processAnyXMLNode(anyxml, name, properties, JsonNodeFactory.instance.arrayNode());
-            } else if (node instanceof AnydataSchemaNode anydata) {
-                processAnydataNode(anydata, name, properties, JsonNodeFactory.instance.arrayNode());
-            } else {
-                if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
-                    property = processDataNodeContainer((DataNodeContainer) node, parentName, definitions,
-                            definitionNames, isConfig, stack, oaversion);
-                    if (!isConfig) {
-                        processActionNodeContainer(node, parentName, definitions, definitionNames, stack,
-                                oaversion);
-                    }
-                } else if (node instanceof LeafListSchemaNode leafList) {
-                    property = processLeafListNode(leafList, stack, definitions, definitionNames, oaversion);
-
-                } else if (node instanceof ChoiceSchemaNode choice) {
-                    for (final CaseSchemaNode variant : choice.getCases()) {
-                        processChoiceNode(variant.getChildNodes(), parentName, definitions, definitionNames, isConfig,
-                                stack, properties, oaversion);
-                    }
-                    continue;
-                } else {
-                    throw new IllegalArgumentException("Unknown DataSchemaNode type: " + node.getClass());
-                }
-                properties.set(name, property);
-            }
-            stack.exit();
-        }
-    }
-
     private static void processElementCount(final Optional<ElementCountConstraint> constraint, final ObjectNode props) {
         if (constraint.isPresent()) {
             final ElementCountConstraint constr = constraint.get();
@@ -899,7 +867,7 @@ public class DefinitionGenerator {
     private static boolean isHexadecimalOrOctal(final RangeRestrictedTypeDefinition<?, ?> typeDef) {
         final Optional<?> optDefaultValue = typeDef.getDefaultValue();
         if (optDefaultValue.isPresent()) {
-            final String defaultValue = (String)optDefaultValue.get();
+            final String defaultValue = (String) optDefaultValue.get();
             return defaultValue.startsWith("0") || defaultValue.startsWith("-0");
         }
         return false;

diff --git a/restconf/sal-rest-docgen/src/test/resources/yang/toaster.yang b/restconf/sal-rest-docgen/src/test/resources/yang/toaster.yang
index ffddc8c3daa0f36f8ea51a2e4e39000b44438fe6..dab5c0299f5487353c484198f2f5c9aa1a540136 100644 (file)
--- a/restconf/sal-rest-docgen/src/test/resources/yang/toaster.yang
+++ b/restconf/sal-rest-docgen/src/test/resources/yang/toaster.yang
@@ -131,6 +131,28 @@ module toaster {
                      type string;
                  }
              }
+
+            case other {
+              description "2nd level choice";
+
+              choice scheduled {
+                case weekly {
+                  leaf weekly {
+                    type string;
+                  }
+                }
+                case monthly {
+                  leaf monthly {
+                    type string;
+                  }
+                }
+                case yearly {
+                  leaf yearly {
+                    type string;
+                  }
+                }
+              }
+            }
          }
 
       leaf toasterManufacturer {