OpenApi: Do not use ArrayNode to store parameters 34/106634/14
authorYaroslav Lastivka <yaroslav.lastivka@pantheon.tech>
Thu, 22 Jun 2023 08:37:04 +0000 (11:37 +0300)
committerIvan Hrasko <ivan.hrasko@pantheon.tech>
Thu, 13 Jul 2023 11:37:26 +0000 (11:37 +0000)
We are using ArrayNode to store parameters. We use it only as a
List which stores parameter objects.

Record Parameter has been created. ArrayNode parameters has been
replaced with List<Parameter> inside Operation.
OperationBuilder#getTypeParentNode has been eliminated.

JIRA: NETCONF-1056
Change-Id: I941e89f78e4b9a82d577a2c96f16a82c261b8a4d
Signed-off-by: Yaroslav Lastivka <yaroslav.lastivka@pantheon.tech>
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/impl/BaseYangOpenApiGenerator.java
restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/model/Operation.java
restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/model/Parameter.java [new file with mode: 0644]
restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/model/builder/OperationBuilder.java
restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/OpenApiTestUtils.java

index 63041d5a07ec4c2dddfbb7e407ff985d7224c862..9ec3faa214fe06d3576419c86b6e8ac67dff7519 100644 (file)
@@ -15,10 +15,8 @@ import static org.opendaylight.restconf.openapi.model.builder.OperationBuilder.b
 import static org.opendaylight.restconf.openapi.model.builder.OperationBuilder.buildPost;
 import static org.opendaylight.restconf.openapi.model.builder.OperationBuilder.buildPostOperation;
 import static org.opendaylight.restconf.openapi.model.builder.OperationBuilder.buildPut;
-import static org.opendaylight.restconf.openapi.model.builder.OperationBuilder.getTypeParentNode;
 import static org.opendaylight.restconf.openapi.util.RestDocgenUtil.resolvePathArgumentsName;
 
-import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -26,6 +24,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.Range;
 import java.io.IOException;
 import java.time.format.DateTimeParseException;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -44,6 +43,7 @@ import org.opendaylight.restconf.openapi.model.Components;
 import org.opendaylight.restconf.openapi.model.Info;
 import org.opendaylight.restconf.openapi.model.OpenApiObject;
 import org.opendaylight.restconf.openapi.model.Operation;
+import org.opendaylight.restconf.openapi.model.Parameter;
 import org.opendaylight.restconf.openapi.model.Path;
 import org.opendaylight.restconf.openapi.model.Schema;
 import org.opendaylight.restconf.openapi.model.SecuritySchemes;
@@ -239,7 +239,7 @@ public abstract class BaseYangOpenApiGenerator {
                 final String localName = moduleName + ":" + node.getQName().getLocalName();
                 final String resourcePath  = getResourcePath("data", context);
 
-                final ArrayNode pathParams = JsonNodeFactory.instance.arrayNode();
+                final List<Parameter> pathParams = new ArrayList<>();
                 /*
                  * When there are two or more top container or list nodes
                  * whose config statement is true in module, make sure that
@@ -257,12 +257,11 @@ public abstract class BaseYangOpenApiGenerator {
             }
         }
 
-        final ArrayNode pathParams = JsonNodeFactory.instance.arrayNode();
         for (final RpcDefinition rpcDefinition : module.getRpcs()) {
             final String resolvedPath = getResourcePath("operations", context) + "/" + moduleName + ":"
                     + rpcDefinition.getQName().getLocalName();
             addOperations(rpcDefinition, moduleName, deviceName, paths, moduleName, definitionNames,
-                resolvedPath, pathParams);
+                resolvedPath, new ArrayList<>());
         }
 
         LOG.debug("Number of Paths found [{}]", paths.size());
@@ -277,7 +276,7 @@ public abstract class BaseYangOpenApiGenerator {
     }
 
     private static void addRootPostLink(final Module module, final String deviceName,
-            final ArrayNode pathParams, final String resourcePath, final Map<String, Path> paths) {
+            final List<Parameter> pathParams, final String resourcePath, final Map<String, Path> paths) {
         if (containsListOrContainer(module.getChildNodes())) {
             final String moduleName = module.getName();
             final String name = moduleName + MODULE_NAME_SUFFIX;
@@ -302,12 +301,12 @@ public abstract class BaseYangOpenApiGenerator {
     public abstract String getResourcePath(String resourceType, String context);
 
     private void addPaths(final DataSchemaNode node, final String deviceName, final String moduleName,
-            final Map<String, Path> paths, final ArrayNode parentPathParams, final EffectiveModelContext schemaContext,
-            final boolean isConfig, final String parentName, final DefinitionNames definitionNames,
-            final String resourcePathPart, final String context) {
+            final Map<String, Path> paths, final List<Parameter> parentPathParams,
+            final EffectiveModelContext schemaContext, final boolean isConfig, final String parentName,
+            final DefinitionNames definitionNames, final String resourcePathPart, final String context) {
         final String dataPath = getResourcePath("data", context) + "/" + resourcePathPart;
         LOG.debug("Adding path: [{}]", dataPath);
-        final ArrayNode pathParams = JsonNodeFactory.instance.arrayNode().addAll(parentPathParams);
+        final List<Parameter> pathParams = new ArrayList<>(parentPathParams);
         Iterable<? extends DataSchemaNode> childSchemaNodes = Collections.emptySet();
         if (node instanceof ListSchemaNode || node instanceof ContainerSchemaNode) {
             final DataNodeContainer dataNodeContainer = (DataNodeContainer) node;
@@ -334,7 +333,7 @@ public abstract class BaseYangOpenApiGenerator {
                 final boolean newIsConfig = isConfig && childNode.isConfiguration();
                 addPaths(childNode, deviceName, moduleName, paths, pathParams, schemaContext,
                     newIsConfig, newParent, definitionNames, newPathPart, context);
-                pathParams.removeAll();
+                pathParams.clear();
                 pathParams.addAll(parentPathParams);
             }
         }
@@ -350,7 +349,7 @@ public abstract class BaseYangOpenApiGenerator {
     }
 
     private static Path operations(final DataSchemaNode node, final String moduleName,
-            final String deviceName, final ArrayNode pathParams, final boolean isConfig,
+            final String deviceName, final List<Parameter> pathParams, final boolean isConfig,
             final String parentName, final DefinitionNames definitionNames) {
         final Path.Builder operationsBuilder = new Path.Builder();
 
@@ -381,7 +380,8 @@ public abstract class BaseYangOpenApiGenerator {
         return operationsBuilder.build();
     }
 
-    private String createPath(final DataSchemaNode schemaNode, final ArrayNode pathParams, final String localName) {
+    private static String createPath(final DataSchemaNode schemaNode, final List<Parameter> pathParams,
+            final String localName) {
         final StringBuilder path = new StringBuilder();
         path.append(localName);
 
@@ -391,12 +391,12 @@ public abstract class BaseYangOpenApiGenerator {
             for (final QName listKey : ((ListSchemaNode) schemaNode).getKeyDefinition()) {
                 final String keyName = listKey.getLocalName();
                 String paramName = keyName;
-                for (final JsonNode pathParam : pathParams) {
-                    if (paramName.equals(pathParam.get("name").asText())) {
+                for (final Parameter pathParam : pathParams) {
+                    if (paramName.equals(pathParam.name())) {
                         paramName = keyName + discriminator;
                         discriminator++;
-                        for (final JsonNode pathParameter : pathParams) {
-                            if (paramName.equals(pathParameter.get("name").asText())) {
+                        for (final Parameter pathParameter : pathParams) {
+                            if (paramName.equals(pathParameter.name())) {
                                 paramName = keyName + discriminator;
                                 discriminator++;
                             }
@@ -406,22 +406,17 @@ public abstract class BaseYangOpenApiGenerator {
 
                 final String pathParamIdentifier = prefix + "{" + paramName + "}";
                 prefix = ",";
-
                 path.append(pathParamIdentifier);
 
-                final ObjectNode pathParam = JsonNodeFactory.instance.objectNode();
-                pathParam.put("name", paramName);
-
-                ((DataNodeContainer) schemaNode).findDataChildByName(listKey).flatMap(DataSchemaNode::getDescription)
-                        .ifPresent(desc -> pathParam.put("description", desc));
-
-                final ObjectNode typeParent = getTypeParentNode(pathParam);
-
-                typeParent.put("type", "string");
-                pathParam.put("in", "path");
-                pathParam.put("required", true);
-
-                pathParams.add(pathParam);
+                final String description = ((DataNodeContainer) schemaNode).findDataChildByName(listKey)
+                    .flatMap(DataSchemaNode::getDescription).orElse(null);
+                final Parameter.Builder pathParamBuilder = new Parameter.Builder()
+                    .name(paramName)
+                    .schema(new Schema.Builder().type("string").build())
+                    .in("path")
+                    .required(true)
+                    .description(description);
+                pathParams.add(pathParamBuilder.build());
             }
         }
         return path.toString();
@@ -452,7 +447,7 @@ public abstract class BaseYangOpenApiGenerator {
 
     private static void addOperations(final OperationDefinition operDef, final String moduleName,
             final String deviceName, final Map<String, Path> paths, final String parentName,
-            final DefinitionNames definitionNames, final String resourcePath, final ArrayNode parentPathParams) {
+            final DefinitionNames definitionNames, final String resourcePath, final List<Parameter> parentPathParams) {
         final var pathBuilder = new Path.Builder();
         pathBuilder.post(buildPostOperation(operDef, moduleName, deviceName, parentName, definitionNames,
             parentPathParams));
index 7ded13abae5782119c93401438efe0a774857b9b..158495220ba7d3f4899409605e6ca5da5d636463 100644 (file)
@@ -10,11 +10,12 @@ package org.opendaylight.restconf.openapi.model;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.List;
 
 @JsonInclude(JsonInclude.Include.NON_NULL)
-public record Operation(boolean deprecated, ArrayNode tags, ArrayNode parameters, ArrayNode security, ArrayNode servers,
-        ObjectNode callbacks, ObjectNode externalDocs, ObjectNode requestBody, ObjectNode responses,
-        String description, String operationId, String summary) {
+public record Operation(boolean deprecated, ArrayNode tags, List<Parameter> parameters, ArrayNode security,
+        ArrayNode servers, ObjectNode callbacks, ObjectNode externalDocs, ObjectNode requestBody,
+        ObjectNode responses, String description, String operationId, String summary) {
 
     private Operation(final Builder builder) {
         this(builder.deprecated, builder.tags, builder.parameters, builder.security, builder.servers, builder.callbacks,
@@ -26,7 +27,7 @@ public record Operation(boolean deprecated, ArrayNode tags, ArrayNode parameters
     public static class Builder {
         private boolean deprecated;
         private ArrayNode tags;
-        private ArrayNode parameters;
+        private List<Parameter> parameters;
         private ArrayNode security;
         private ArrayNode servers;
         private ObjectNode callbacks;
@@ -47,7 +48,7 @@ public record Operation(boolean deprecated, ArrayNode tags, ArrayNode parameters
             return this;
         }
 
-        public Builder parameters(final ArrayNode parameters) {
+        public Builder parameters(final List<Parameter> parameters) {
             this.parameters = parameters;
             return this;
         }
diff --git a/restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/model/Parameter.java b/restconf/restconf-openapi/src/main/java/org/opendaylight/restconf/openapi/model/Parameter.java
new file mode 100644 (file)
index 0000000..e838577
--- /dev/null
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2023 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.restconf.openapi.model;
+
+import static java.util.Objects.requireNonNull;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public record Parameter(
+        @NonNull String name,
+        @NonNull String in,
+        boolean required,
+        @Nullable Schema schema,
+        @Nullable String description) {
+
+    public Parameter {
+        requireNonNull(name);
+        requireNonNull(in);
+    }
+
+    private Parameter(final Builder builder) {
+        this(builder.name, builder.in, builder.required, builder.schema, builder.description);
+    }
+
+    @SuppressWarnings("checkstyle:hiddenField")
+    public static class Builder {
+        private String name;
+        private String in;
+        private String description;
+        private boolean required;
+        private Schema schema;
+
+        public Builder name(final String name) {
+            this.name = name;
+            return this;
+        }
+
+        public Builder in(final String in) {
+            this.in = in;
+            return this;
+        }
+
+        public Builder description(final String description) {
+            this.description = description;
+            return this;
+        }
+
+        public Builder required(final boolean required) {
+            this.required = required;
+            return this;
+        }
+
+        public Builder schema(final Schema schema) {
+            this.schema = schema;
+            return this;
+        }
+
+        public Parameter build() {
+            return new Parameter(this);
+        }
+    }
+}
index 3739523dd52ff5c81b01cbc2d64a5042239aca29..e669b9193dc37c329d16130b164960b1aa95379d 100644 (file)
@@ -15,6 +15,7 @@ import static org.opendaylight.restconf.openapi.impl.DefinitionGenerator.OUTPUT_
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.ArrayList;
 import java.util.List;
 import javax.ws.rs.HttpMethod;
 import javax.ws.rs.core.MediaType;
@@ -22,6 +23,8 @@ import javax.ws.rs.core.Response;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.restconf.openapi.impl.DefinitionNames;
 import org.opendaylight.restconf.openapi.model.Operation;
+import org.opendaylight.restconf.openapi.model.Parameter;
+import org.opendaylight.restconf.openapi.model.Schema;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.InputSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.OperationDefinition;
@@ -33,7 +36,6 @@ public final class OperationBuilder {
     public static final String CONTENT_KEY = "content";
     public static final String COMPONENTS_PREFIX = "#/components/schemas/";
     public static final String DESCRIPTION_KEY = "description";
-    public static final String IN_KEY = "in";
     public static final String INPUT_KEY = "input";
     public static final String NAME_KEY = "name";
     public static final String NONCONFIG_QUERY_PARAM = "nonconfig";
@@ -45,10 +47,8 @@ public final class OperationBuilder {
     public static final String XML_KEY = "xml";
     private static final String CONTENT = "content";
     private static final ArrayNode CONSUMES_PUT_POST;
-    private static final String ENUM_KEY = "enum";
     private static final List<String> MIME_TYPES = List.of(MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON);
     private static final String OBJECT = "object";
-    private static final String REQUIRED_KEY = "required";
     private static final String STRING = "string";
     private static final String TYPE_KEY = "type";
     private static final String QUERY = "query";
@@ -66,10 +66,10 @@ public final class OperationBuilder {
 
     public static Operation buildPost(final String parentName, final String nodeName, final String discriminator,
             final String moduleName, final @Nullable String deviceName, final String description,
-            final ArrayNode pathParams) {
+            final List<Parameter> pathParams) {
         final var summary = buildSummaryValue(HttpMethod.POST, moduleName, deviceName, nodeName);
         final ArrayNode tags = buildTagsValue(deviceName, moduleName);
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(pathParams);
+        final List<Parameter> parameters = new ArrayList<>(pathParams);
         final ObjectNode ref = JsonNodeFactory.instance.objectNode();
         final String cleanDefName = parentName + CONFIG + "_" + nodeName;
         final String defName = cleanDefName + discriminator;
@@ -91,13 +91,13 @@ public final class OperationBuilder {
     }
 
     public static Operation buildGet(final DataSchemaNode node, final String moduleName,
-            final @Nullable String deviceName, final ArrayNode pathParams, final String defName,
+            final @Nullable String deviceName, final List<Parameter> pathParams, final String defName,
             final String defNameTop, final boolean isConfig) {
         final String description = node.getDescription().orElse("");
         final String summary = buildSummaryValue(HttpMethod.GET, moduleName, deviceName,
                 node.getQName().getLocalName());
         final ArrayNode tags = buildTagsValue(deviceName, moduleName);
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(pathParams);
+        final List<Parameter> parameters = new ArrayList<>(pathParams);
         addQueryParameters(parameters, isConfig);
         final ObjectNode responses = JsonNodeFactory.instance.objectNode();
         final ObjectNode schema = JsonNodeFactory.instance.objectNode();
@@ -117,31 +117,27 @@ public final class OperationBuilder {
             .build();
     }
 
-    private static void addQueryParameters(final ArrayNode parameters, final boolean isConfig) {
-        final ObjectNode contentParam = JsonNodeFactory.instance.objectNode();
+    private static void addQueryParameters(final List<Parameter> parameters, final boolean isConfig) {
         final ArrayNode cases = JsonNodeFactory.instance.arrayNode();
         cases.add(NONCONFIG_QUERY_PARAM);
         if (isConfig) {
             cases.add(CONFIG_QUERY_PARAM);
-        } else {
-            contentParam.put(REQUIRED_KEY, true);
         }
-        contentParam.put(IN_KEY, QUERY);
-        contentParam.put(NAME_KEY, CONTENT);
-
-        final ObjectNode typeParent = getTypeParentNode(contentParam);
-        typeParent.put(TYPE_KEY, STRING);
-        typeParent.set(ENUM_KEY, cases);
 
-        parameters.add(contentParam);
+        final Parameter.Builder contentParamBuilder = new Parameter.Builder()
+            .in(QUERY)
+            .name(CONTENT)
+            .schema(new Schema.Builder().type(STRING).schemaEnum(cases).build())
+            .required(!isConfig);
+        parameters.add(contentParamBuilder.build());
     }
 
     public static Operation buildPut(final String parentName, final String nodeName, final String discriminator,
             final String moduleName, final @Nullable String deviceName, final String description,
-            final ArrayNode pathParams) {
+            final List<Parameter> pathParams) {
         final String summary = buildSummaryValue(HttpMethod.PUT, moduleName, deviceName, nodeName);
         final ArrayNode tags = buildTagsValue(deviceName, moduleName);
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(pathParams);
+        final List<Parameter> parameters = new ArrayList<>(pathParams);
         final String defName = parentName + CONFIG + "_" + nodeName + TOP;
         final String xmlDefName = parentName + CONFIG + "_" + nodeName;
         final ObjectNode requestBody = createRequestBodyParameter(defName, xmlDefName, nodeName + CONFIG, summary);
@@ -162,10 +158,10 @@ public final class OperationBuilder {
     }
 
     public static Operation buildPatch(final String parentName, final String nodeName, final String moduleName,
-            final @Nullable String deviceName, final String description, final ArrayNode pathParams) {
+            final @Nullable String deviceName, final String description, final List<Parameter> pathParams) {
         final String summary = buildSummaryValue(HttpMethod.PATCH, moduleName, deviceName, nodeName);
         final ArrayNode tags = buildTagsValue(deviceName, moduleName);
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(pathParams);
+        final List<Parameter> parameters = new ArrayList<>(pathParams);
         final String defName = parentName + CONFIG + "_" + nodeName + TOP;
         final String xmlDefName = parentName + CONFIG + "_" + nodeName;
         final ObjectNode requestBody = createRequestBodyParameter(defName, xmlDefName, nodeName + CONFIG, summary);
@@ -186,12 +182,12 @@ public final class OperationBuilder {
     }
 
     public static Operation buildDelete(final DataSchemaNode node, final String moduleName,
-            final @Nullable String deviceName, final ArrayNode pathParams) {
+            final @Nullable String deviceName, final List<Parameter> pathParams) {
         final String summary = buildSummaryValue(HttpMethod.DELETE, moduleName, deviceName,
                 node.getQName().getLocalName());
         final ArrayNode tags = buildTagsValue(deviceName, moduleName);
         final String description = node.getDescription().orElse("");
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(pathParams);
+        final List<Parameter> parameters = new ArrayList<>(pathParams);
 
         final ObjectNode responses = JsonNodeFactory.instance.objectNode();
         responses.set(String.valueOf(Response.Status.NO_CONTENT.getStatusCode()), buildResponse("Deleted"));
@@ -207,8 +203,8 @@ public final class OperationBuilder {
 
     public static Operation buildPostOperation(final OperationDefinition operDef, final String moduleName,
             final @Nullable String deviceName, final String parentName, final DefinitionNames definitionNames,
-            final ArrayNode parentPathParameters) {
-        final ArrayNode parameters = JsonNodeFactory.instance.arrayNode().addAll(parentPathParameters);
+            final List<Parameter> parentPathParameters) {
+        final List<Parameter> parameters = new ArrayList<>(parentPathParameters);
         final String operationName = operDef.getQName().getLocalName();
         final String inputName = operationName + INPUT_SUFFIX;
         final String summary = buildSummaryValue(HttpMethod.POST, moduleName, deviceName, operationName);
@@ -356,10 +352,4 @@ public final class OperationBuilder {
         }
         return JsonNodeFactory.instance.arrayNode().add("mounted " + deviceName + " " + moduleName);
     }
-
-    public static ObjectNode getTypeParentNode(final ObjectNode parameter) {
-        final ObjectNode schema = JsonNodeFactory.instance.objectNode();
-        parameter.set(SCHEMA_KEY, schema);
-        return schema;
-    }
 }
index 846c1085701a570709259f26184d9e6a0e67903d..3091f9f0110918a1c02540c03f5a6ac147a07b85 100644 (file)
@@ -7,9 +7,9 @@
  */
 package org.opendaylight.restconf.openapi;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import org.opendaylight.restconf.openapi.model.Parameter;
 import org.opendaylight.restconf.openapi.model.Path;
 
 public final class OpenApiTestUtils {
@@ -24,9 +24,9 @@ public final class OpenApiTestUtils {
      * @return {@link List} of parameters
      */
     public static List<String> getPathParameters(final Map<String, Path> paths, final String path) {
-        final var params = new ArrayList<String>();
-        paths.get(path).post().parameters().elements()
-            .forEachRemaining(p -> params.add(p.get("name").asText()));
-        return params;
+        return paths.get(path).post().parameters()
+            .stream()
+            .map(Parameter::name)
+            .toList();
     }
 }