OpenApi: Remove incorrect list POST requests 86/107086/20
authorYaroslav Lastivka <yaroslav.lastivka@pantheon.tech>
Wed, 26 Jul 2023 15:18:03 +0000 (18:18 +0300)
committerIvan Hrasko <ivan.hrasko@pantheon.tech>
Wed, 13 Sep 2023 06:46:26 +0000 (06:46 +0000)
Our logic creates POST requests for lists with keys, containing
multiple resources in payload. This does not align with the expected
structure for creating child resources in the YANG model.

Added condition that prevents the creation of POST requests
that has list as a last element.

JIRA: NETCONF-1101
Change-Id: I46820f222c9c5ef8078ac8675d2adc12b06f1253
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/test/java/org/opendaylight/restconf/openapi/impl/ListPostRequestsTest.java [new file with mode: 0644]
restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/impl/OpenApiGeneratorRFC8040Test.java
restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/impl/PostPayloadTest.java
restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/mountpoints/MountPointOpenApiTest.java

index e004cb5ff855fca83a7a5047e722dbef0c672810..0a2397765f87c155f15fc821cb9db7b6e9e55ade 100644 (file)
@@ -319,9 +319,11 @@ public abstract class BaseYangOpenApiGenerator {
             final Operation delete = buildDelete(node, moduleName, deviceName, pathParams);
             operationsBuilder.delete(delete);
 
-            final Operation post = buildPost(node, parentName, nodeName, discriminator, moduleName, deviceName,
+            if (!(node instanceof ListSchemaNode)) {
+                final Operation post = buildPost(node, parentName, nodeName, discriminator, moduleName, deviceName,
                     node.getDescription().orElse(""), pathParams);
-            operationsBuilder.post(post);
+                operationsBuilder.post(post);
+            }
         }
         return operationsBuilder.build();
     }
diff --git a/restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/impl/ListPostRequestsTest.java b/restconf/restconf-openapi/src/test/java/org/opendaylight/restconf/openapi/impl/ListPostRequestsTest.java
new file mode 100644 (file)
index 0000000..75dfe90
--- /dev/null
@@ -0,0 +1,76 @@
+/*
+ * 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.impl;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.opendaylight.mdsal.dom.api.DOMSchemaService;
+import org.opendaylight.restconf.openapi.DocGenTestHelper;
+import org.opendaylight.restconf.openapi.model.OpenApiObject;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class ListPostRequestsTest {
+    private static OpenApiObject doc;
+
+    @BeforeClass
+    public static void startUp() throws Exception {
+        final var context = YangParserTestUtils.parseYang("""
+            module list-post {
+              namespace "list-post";
+              prefix lp;
+                container container {
+                  list list {
+                    key "name address";
+                    leaf name {
+                      type string;
+                    }
+                    leaf address {
+                      type string;
+                    }
+                  }
+                }
+              }""");
+        final var schemaService = mock(DOMSchemaService.class);
+        when(schemaService.getGlobalContext()).thenReturn(context);
+        final var generator = new OpenApiGeneratorRFC8040(schemaService);
+        final var uriInfo = DocGenTestHelper.createMockUriInfo("http://localhost/path");
+        doc = generator.getApiDeclaration("list-post", null, uriInfo);
+        assertNotNull(doc);
+    }
+
+    /**
+     * Test to verify that we do NOT generate OpenApi example POST request for path ending with list element.
+     *
+     * <p>
+     * Assert that for paths ending with container we have examples for all types of requests.
+     * Assert that for paths ending with list we do NOT have POST example.
+     */
+    @Test
+    public void testListPostRequest() {
+        // for container, we have both post (with child as payload) and put (with itself as payload)
+        final var pathToContainer = "/rests/data/list-post:container";
+        assertNotNull(doc.paths().get(pathToContainer).get());
+        assertNotNull(doc.paths().get(pathToContainer).post());
+        assertNotNull(doc.paths().get(pathToContainer).put());
+        assertNotNull(doc.paths().get(pathToContainer).patch());
+        assertNotNull(doc.paths().get(pathToContainer).delete());
+
+        // for list, we cannot make a post request
+        final var pathToList = "/rests/data/list-post:container/list={name},{address}";
+        assertNotNull(doc.paths().get(pathToList).get());
+        assertNull(doc.paths().get(pathToList).post());
+        assertNotNull(doc.paths().get(pathToList).put());
+        assertNotNull(doc.paths().get(pathToList).patch());
+        assertNotNull(doc.paths().get(pathToList).delete());
+    }
+}
index 1359a1e194866fb5f766b4d6e8be68dbde891529..da26b1254bbaf652c35818f5ee26f86233eaef73 100644 (file)
@@ -94,6 +94,8 @@ public final class OpenApiGeneratorRFC8040Test {
                 "/rests/data/toaster2:lst={lf1}/cont1/cont11",
                 "/rests/data/toaster2:lst={lf1}/cont1/lst11={lf111}",
                 "/rests/data/toaster2:lst={lf1}/lst1={key1},{key2}");
+        final List<String> configPathsForPost = List.of("/rests/data/toaster2:lst={lf1}/cont1",
+                "/rests/data/toaster2:lst={lf1}/cont1/cont11");
 
         final OpenApiObject doc = generator.getApiDeclaration(TOASTER_2, REVISION_DATE, uriInfo);
 
@@ -102,9 +104,13 @@ public final class OpenApiGeneratorRFC8040Test {
             assertNotNull(node.get());
             assertNotNull(node.put());
             assertNotNull(node.delete());
-            assertNotNull(node.post());
             assertNotNull(node.patch());
         }
+
+        for (final String path : configPathsForPost) {
+            final Path node = doc.paths().get(path);
+            assertNotNull(node.post());
+        }
     }
 
     /**
@@ -228,14 +234,16 @@ public final class OpenApiGeneratorRFC8040Test {
             assertNotNull(delete);
             assertEquals(expectedSize, delete.parameters().size());
 
-            final var post = path.post();
-            assertNotNull(post);
-            assertEquals(expectedSize, post.parameters().size());
-
             final var patch = path.patch();
             assertNotNull(patch);
             assertEquals(expectedSize, patch.parameters().size());
         }
+
+        // we do not generate POST for lists
+        final var path = paths.get("/rests/data/recursive:container-root");
+        final var post = path.post();
+        final int expectedSize = configPaths.get("/rests/data/recursive:container-root");
+        assertEquals(expectedSize, post.parameters().size());
     }
 
     /**
@@ -313,8 +321,6 @@ public final class OpenApiGeneratorRFC8040Test {
         verifyRequestRef(jsonNodeToaster.get(), "#/components/schemas/toaster2_toaster", CONTAINER);
 
         final var jsonNodeToasterSlot = doc.paths().get("/rests/data/toaster2:toaster/toasterSlot={slotId}");
-        verifyRequestRef(jsonNodeToasterSlot.post(), "#/components/schemas/toaster2_toaster_toasterSlot_slotInfo",
-            CONTAINER);
         verifyRequestRef(jsonNodeToasterSlot.put(), "#/components/schemas/toaster2_toaster_toasterSlot", LIST);
         verifyRequestRef(jsonNodeToasterSlot.get(), "#/components/schemas/toaster2_toaster_toasterSlot", LIST);
 
@@ -328,13 +334,10 @@ public final class OpenApiGeneratorRFC8040Test {
             CONTAINER);
 
         final var jsonNodeLst = doc.paths().get("/rests/data/toaster2:lst={lf1}");
-        verifyRequestRef(jsonNodeLst.post(), "#/components/schemas/toaster2_lst_cont1", CONTAINER);
         verifyRequestRef(jsonNodeLst.put(), "#/components/schemas/toaster2_lst", LIST);
         verifyRequestRef(jsonNodeLst.get(), "#/components/schemas/toaster2_lst", LIST);
 
         final var jsonNodeLst1 = doc.paths().get("/rests/data/toaster2:lst={lf1}/lst1={key1},{key2}");
-        verifyPostDataRequestRef(jsonNodeLst1.post(), "#/components/schemas/toaster2_lst_lst1",
-            "#/components/schemas/toaster2_lst_lst1");
         verifyRequestRef(jsonNodeLst1.put(), "#/components/schemas/toaster2_lst_lst1", LIST);
         verifyRequestRef(jsonNodeLst1.get(), "#/components/schemas/toaster2_lst_lst1", LIST);
 
index 63198a8aab220eb7b9a1f5e15eb1fcb229817d18..feced2b90a52bf058fb735b591849e551d04b61d 100644 (file)
@@ -140,14 +140,6 @@ public class PostPayloadTest {
         final var xmlRef2 = getXmlRef(containerDoc, path2);
         assertEquals("#/components/schemas/container-test_cont_cont1_list4", xmlRef2);
 
-        final var path3 = "/rests/data/container-test:cont/cont1/list4={key4}";
-        assertTrue(containerDoc.paths().containsKey(path3));
-        final var jsonRef3 = getJsonRef(containerDoc, path3);
-        assertEquals("{\"cont2\":{\"$ref\":\"#/components/schemas/container-test_cont_cont1_list4_cont2\"}}",
-            jsonRef3);
-        final var xmlRef3 = getXmlRef(containerDoc, path3);
-        assertEquals("#/components/schemas/container-test_cont_cont1_list4_cont2", xmlRef3);
-
         final var path4 = "/rests/data/container-test:cont/cont1/list4={key4}/cont2";
         assertTrue(containerDoc.paths().containsKey(path4));
         final var jsonRef4 = getJsonRef(containerDoc, path4);
@@ -166,13 +158,6 @@ public class PostPayloadTest {
             + "#/components/schemas/list-test_cont_list1\"}}}", jsonRef1);
         final var xmlRef1 = getXmlRef(listDoc, path1);
         assertEquals("#/components/schemas/list-test_cont_list1", xmlRef1);
-
-        final var path2 = "/rests/data/list-test:cont/list2={key2}";
-        final var jsonRef2 = getJsonRef(listDoc, path2);
-        assertEquals("{\"list3\":{\"type\":\"array\",\"items\":{\"$ref\":\""
-            + "#/components/schemas/list-test_cont_list2_list3\"}}}", jsonRef2);
-        final var xmlRef2 = getXmlRef(listDoc, path2);
-        assertEquals("#/components/schemas/list-test_cont_list2_list3", xmlRef2);
     }
 
     private static String getJsonRef(final OpenApiObject openApiObject, final String path) {
index 7b7d106e7a33865b5c280edd6809b75aafe98e95..2e8eb56aa25bf00afe906c0b6a883722d1f91d06 100644 (file)
@@ -161,7 +161,7 @@ public final class MountPointOpenApiTest {
         }
 
         assertEquals("Unexpected GET paths size", 37, getOperations.size());
-        assertEquals("Unexpected POST paths size", 43, postOperations.size());
+        assertEquals("Unexpected POST paths size", 27, postOperations.size());
         assertEquals("Unexpected PUT paths size", 35, putOperations.size());
         assertEquals("Unexpected PATCH paths size", 35, patchOperations.size());
         assertEquals("Unexpected DELETE paths size", 35, deleteOperations.size());
@@ -205,14 +205,16 @@ public final class MountPointOpenApiTest {
             assertNotNull(delete);
             assertEquals(expectedSize, delete.parameters().size());
 
-            final var post = path.post();
-            assertNotNull(post);
-            assertEquals(expectedSize, post.parameters().size());
-
             final var patch = path.patch();
             assertNotNull(patch);
             assertEquals(expectedSize, patch.parameters().size());
         }
+
+        // POST request exists only for containers
+        final var post = paths.get("/rests/data/nodes/node=123/yang-ext:mount/recursive:container-root").post();
+        assertNotNull(post);
+        final int expectedSize = configPaths.get("/rests/data/nodes/node=123/yang-ext:mount/recursive:container-root");
+        assertEquals(expectedSize, post.parameters().size());
     }
 
     /**