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>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 402b85932413b91a7dd16efce6d566a580f1e1eb)

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 bd2ef8e83af1cf01349f1c2f8d742db3b2975100..586fbade66f39d4fb1dbbb2802d5c4b9faad5f59 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
@@ -209,15 +209,13 @@ public class DefinitionGenerator {
                         //add module name prefix to property name, when ServiceNow can process colons
                         properties.set(localName, childNodeProperties);
                     }
-                } else {
-                    if (node instanceof LeafSchemaNode) {
-                        /*
-                            Add module name prefix to property name, when ServiceNow can process colons(second parameter
-                            of processLeafNode).
-                         */
-                        processLeafNode((LeafSchemaNode) node, localName, properties, required, stack,
-                                definitions, definitionNames, oaversion);
-                    }
+                } else if (node instanceof LeafSchemaNode) {
+                    /*
+                        Add module name prefix to property name, when ServiceNow can process colons(second parameter
+                        of processLeafNode).
+                     */
+                    processLeafNode((LeafSchemaNode) node, localName, properties, required, stack,
+                            definitions, definitionNames, oaversion);
                 }
             }
             stack.exit();
@@ -482,57 +480,75 @@ 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) {
-                    processLeafNode((LeafSchemaNode) node, propertyName, properties,
-                            required, stack, definitions, definitionNames, oaversion);
-                } else if (node instanceof AnyxmlSchemaNode) {
-                    processAnyXMLNode((AnyxmlSchemaNode) node, propertyName, properties,
-                            required);
-                } else if (node instanceof AnydataSchemaNode) {
-                    processAnydataNode((AnydataSchemaNode) node, 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) {
-                        property = processLeafListNode((LeafListSchemaNode) node, stack, definitions,
-                                definitionNames, oaversion);
-
-                    } else if (node instanceof ChoiceSchemaNode) {
-                        for (final CaseSchemaNode variant : ((ChoiceSchemaNode) node).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) {
+            processLeafNode((LeafSchemaNode) node, name, properties, JsonNodeFactory.instance.arrayNode(), stack,
+                    definitions, definitionNames, oaversion);
+
+        } else if (node instanceof AnyxmlSchemaNode) {
+            processAnyXMLNode((AnyxmlSchemaNode) node, name, properties, JsonNodeFactory.instance.arrayNode());
+
+        } else if (node instanceof AnydataSchemaNode) {
+            processAnydataNode((AnydataSchemaNode) node, 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) {
+                property = processLeafListNode((LeafListSchemaNode) node, stack, definitions, definitionNames,
+                        oaversion);
+
+            } else if (node instanceof ChoiceSchemaNode) {
+                for (final CaseSchemaNode variant : ((ChoiceSchemaNode) node).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) {
@@ -551,60 +567,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) {
-                processLeafNode((LeafSchemaNode) node, name, properties,
-                        JsonNodeFactory.instance.arrayNode(), stack, definitions, definitionNames, oaversion);
-            } else if (node instanceof AnyxmlSchemaNode) {
-                processAnyXMLNode((AnyxmlSchemaNode) node, name, properties,
-                        JsonNodeFactory.instance.arrayNode());
-            } else if (node instanceof AnydataSchemaNode) {
-                processAnydataNode((AnydataSchemaNode) node, 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) {
-                    property = processLeafListNode((LeafListSchemaNode) node, stack, definitions,
-                            definitionNames, oaversion);
-
-                } else if (node instanceof ChoiceSchemaNode) {
-                    for (final CaseSchemaNode variant : ((ChoiceSchemaNode) node).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();
@@ -907,7 +869,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 {