Fix RestconfOperationsService.getOperations(UriInfo) 85/97685/4
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 29 Sep 2021 18:53:16 +0000 (20:53 +0200)
committerRobert Varga <nite@hq.sk>
Tue, 5 Oct 2021 12:21:36 +0000 (12:21 +0000)
ietf-restconf.yang has a rather ugly wart in its definition of the
operations container, which we are using an ugly workaround for.

This workaround is rendered inoperable due to strict binding to
effective model context, which does not find the dynamic leaves required
to encode the content.

Rework the implementation to side-step NormalizedNode documents and
provide ready JSON/XML strings.

JIRA: NETCONF-822
Change-Id: I57760d06240e09940026fee9b195e207853c01b2
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
12 files changed:
restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/OperationsContent.java [new file with mode: 0644]
restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/util/OperationsResourceUtils.java
restconf/restconf-common/src/test/java/org/opendaylight/restconf/common/Netconf822Test.java [new file with mode: 0644]
restconf/restconf-common/src/test/resources/nc822/foo@2021-09-29.yang [new file with mode: 0644]
restconf/restconf-common/src/test/resources/nc822/foo@2021-09-30.yang [new file with mode: 0644]
restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/rest/api/RestconfService.java
restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/rest/impl/RestconfCompositeWrapper.java
restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java
restconf/restconf-nb-bierman02/src/main/java/org/opendaylight/netconf/sal/restconf/impl/StatisticsRestconfServiceWrapper.java
restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/api/RestconfOperationsService.java
restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfOperationsServiceImpl.java
restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfOperationsServiceTest.java

diff --git a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/OperationsContent.java b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/OperationsContent.java
new file mode 100644 (file)
index 0000000..199728f
--- /dev/null
@@ -0,0 +1,156 @@
+/*
+ * Copyright (c) 2021 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.common;
+
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.collect.HashBasedTable;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+import org.opendaylight.yangtools.yang.common.Revision;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
+import org.opendaylight.yangtools.yang.model.api.stmt.ModuleEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.RpcEffectiveStatement;
+
+/**
+ * RESTCONF {@code /operations} content for a {@code GET} operation as per
+ * <a href="https://datatracker.ietf.org/doc/html/rfc8040#section-3.3.2">RFC8040</a>.
+ */
+// FIXME: when bierman02 is gone, this should be folded to nb-rfc8040, as it is a server-side thing.
+public enum OperationsContent {
+    JSON("{ \"ietf-restconf:operations\" : { } }") {
+        @Override
+        String createBody(final List<Entry<String, List<String>>> rpcsByPrefix) {
+            final var sb = new StringBuilder("{\n"
+                + "  \"ietf-restconf:operations\" : {\n");
+            var entryIt = rpcsByPrefix.iterator();
+            var entry = entryIt.next();
+            var nameIt = entry.getValue().iterator();
+            while (true) {
+                sb.append("    \"").append(entry.getKey()).append(':').append(nameIt.next()).append("\": [null]");
+                if (nameIt.hasNext()) {
+                    sb.append(",\n");
+                    continue;
+                }
+
+                if (entryIt.hasNext()) {
+                    sb.append(",\n");
+                    entry = entryIt.next();
+                    nameIt = entry.getValue().iterator();
+                    continue;
+                }
+
+                break;
+            }
+
+            return sb.append("\n  }\n}").toString();
+        }
+
+        @Override
+        String prefix(final ModuleEffectiveStatement module) {
+            return module.argument().getLocalName();
+        }
+    },
+
+    XML("<operations xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"/>") {
+        @Override
+        String createBody(final List<Entry<String, List<String>>> rpcsByPrefix) {
+            // Header with namespace declarations for each module
+            final var sb = new StringBuilder("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<operations xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"");
+            for (int i = 0; i < rpcsByPrefix.size(); ++i) {
+                final var prefix = "ns" + i;
+                sb.append("\n            xmlns:").append(prefix).append("=\"").append(rpcsByPrefix.get(i).getKey())
+                    .append("\"");
+            }
+            sb.append(" >");
+
+            // Second pass: emit all leaves
+            for (int i = 0; i < rpcsByPrefix.size(); ++i) {
+                final var prefix = "ns" + i;
+                for (var localName : rpcsByPrefix.get(i).getValue()) {
+                    sb.append("\n  <").append(prefix).append(':').append(localName).append("/>");
+                }
+            }
+
+            return sb.append("\n</operations>").toString();
+        }
+
+        @Override
+        String prefix(final ModuleEffectiveStatement module) {
+            return module.localQNameModule().getNamespace().toString();
+        }
+    };
+
+    private final @NonNull String emptyBody;
+
+    OperationsContent(final String emptyBody) {
+        this.emptyBody = requireNonNull(emptyBody);
+    }
+
+    /**
+     * Return the content for a particular {@link EffectiveModelContext}.
+     *
+     * @param context Context to use
+     * @return Content of HTTP GET operation as a String
+     */
+    public final @NonNull String bodyFor(final @Nullable EffectiveModelContext context) {
+        if (context == null) {
+            return emptyBody;
+        }
+        final var modules = context.getModuleStatements();
+        if (modules.isEmpty()) {
+            return emptyBody;
+        }
+
+        // Index into prefix -> revision -> module table
+        final var prefixRevModule = HashBasedTable.<String, Optional<Revision>, ModuleEffectiveStatement>create();
+        for (var module : modules.values()) {
+            prefixRevModule.put(prefix(module), module.localQNameModule().getRevision(), module);
+        }
+
+        // Now extract RPC names for each module with highest revision. This needed so we expose the right set of RPCs,
+        // as we always pick the latest revision to resolve prefix (or module name)
+        // TODO: Simplify this once we have yangtools-7.0.9+
+        final var moduleRpcs = new ArrayList<Entry<String, List<String>>>();
+        for (var moduleEntry : prefixRevModule.rowMap().entrySet()) {
+            final var revisions = new ArrayList<>(moduleEntry.getValue().keySet());
+            revisions.sort(Revision::compare);
+            final var selectedRevision = revisions.get(revisions.size() - 1);
+
+            final var rpcNames = moduleEntry.getValue().get(selectedRevision)
+                .streamEffectiveSubstatements(RpcEffectiveStatement.class)
+                .map(rpc -> rpc.argument().getLocalName())
+                .collect(Collectors.toUnmodifiableList());
+            if (!rpcNames.isEmpty()) {
+                moduleRpcs.add(Map.entry(moduleEntry.getKey(), rpcNames));
+            }
+        }
+
+        if (moduleRpcs.isEmpty()) {
+            // No RPCs, return empty content
+            return emptyBody;
+        }
+
+        // Ensure stability: sort by prefix
+        moduleRpcs.sort(Comparator.comparing(Entry::getKey));
+
+        return modules.isEmpty() ? emptyBody : createBody(moduleRpcs);
+    }
+
+    abstract @NonNull String createBody(List<Entry<String, List<String>>> rpcsByPrefix);
+
+    abstract @NonNull String prefix(ModuleEffectiveStatement module);
+}
index c1fbab30b1d0950190233e8b7e9e9862c6002ab6..362f4c29f5df8fb7445f2044a02628826a7802de 100644 (file)
@@ -26,6 +26,7 @@ import org.opendaylight.yangtools.yang.model.api.RpcDefinition;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
 
+// FIXME: remove this class
 public final class OperationsResourceUtils {
     private OperationsResourceUtils() {
         // Hidden on purpose
diff --git a/restconf/restconf-common/src/test/java/org/opendaylight/restconf/common/Netconf822Test.java b/restconf/restconf-common/src/test/java/org/opendaylight/restconf/common/Netconf822Test.java
new file mode 100644 (file)
index 0000000..70f56bf
--- /dev/null
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2021 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.common;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class Netconf822Test {
+    private static EffectiveModelContext CONTEXT;
+
+    @BeforeClass
+    public static void beforeClass() {
+        CONTEXT = YangParserTestUtils.parseYangResourceDirectory("/nc822");
+    }
+
+    @Test
+    public void testOperationsContentJSON() {
+        assertEquals("{\n"
+            + "  \"ietf-restconf:operations\" : {\n"
+            + "    \"foo:new\": [null]\n"
+            + "  }\n"
+            + "}", OperationsContent.JSON.bodyFor(CONTEXT));
+    }
+
+    @Test
+    public void testOperationsContentXML() {
+        assertEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<operations xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"\n"
+            + "            xmlns:ns0=\"foo\" >\n"
+            + "  <ns0:new/>\n"
+            + "</operations>", OperationsContent.XML.bodyFor(CONTEXT));
+    }
+}
diff --git a/restconf/restconf-common/src/test/resources/nc822/foo@2021-09-29.yang b/restconf/restconf-common/src/test/resources/nc822/foo@2021-09-29.yang
new file mode 100644 (file)
index 0000000..797b0e4
--- /dev/null
@@ -0,0 +1,7 @@
+module foo {
+  prefix foo;
+  namespace foo;
+  revision 2021-09-29;
+
+  rpc old;
+}
diff --git a/restconf/restconf-common/src/test/resources/nc822/foo@2021-09-30.yang b/restconf/restconf-common/src/test/resources/nc822/foo@2021-09-30.yang
new file mode 100644 (file)
index 0000000..386c802
--- /dev/null
@@ -0,0 +1,7 @@
+module foo {
+  prefix foo;
+  namespace foo;
+  revision 2021-09-30;
+
+  rpc new;
+}
index f29fa26fe146b782a54ca3eaa4ff51f0e9bde8ff..142e3cd2be2726ad1c61a26ff5dadd192f2c87ff 100644 (file)
@@ -127,23 +127,28 @@ public interface RestconfService {
     /**
      * List of rpc or action operations supported by the server.
      *
-     * @param uriInfo
-     *            URI information
-     * @return {@link NormalizedNodeContext}
+     * @return A JSON document string
      * @deprecated do not use this method. It will be replaced by
      *             RestconfOperationsService#getOperations(UriInfo)
      */
     @Deprecated
     @GET
     @Path("/operations")
-    @Produces({
-        Draft02.MediaTypes.API + JSON,
-        Draft02.MediaTypes.API + XML,
-        MediaType.APPLICATION_JSON,
-        MediaType.APPLICATION_XML,
-        MediaType.TEXT_XML
-    })
-    NormalizedNodeContext getOperations(@Context UriInfo uriInfo);
+    @Produces({ Draft02.MediaTypes.API + JSON, MediaType.APPLICATION_JSON })
+    String getOperationsJSON();
+
+    /**
+     * List of rpc or action operations supported by the server.
+     *
+     * @return A XML document string
+     * @deprecated do not use this method. It will be replaced by
+     *             RestconfOperationsService#getOperations(UriInfo)
+     */
+    @Deprecated
+    @GET
+    @Path("/operations")
+    @Produces({ Draft02.MediaTypes.API + XML, MediaType.APPLICATION_XML, MediaType.TEXT_XML })
+    String getOperationsXML();
 
     /**
      * Valid for mount points. List of operations supported by the server.
index 8198fd9fc2dbee8978b3ff15c401ba53ad758a04..6a528dbce52fc14f23d38a9e671baba63571e67e 100644 (file)
@@ -49,8 +49,13 @@ public class RestconfCompositeWrapper implements RestconfService, SchemaRetrieva
     }
 
     @Override
-    public NormalizedNodeContext getOperations(final UriInfo uriInfo) {
-        return this.restconf.getOperations(uriInfo);
+    public String getOperationsJSON() {
+        return this.restconf.getOperationsJSON();
+    }
+
+    @Override
+    public String getOperationsXML() {
+        return this.restconf.getOperationsXML();
     }
 
     @Override
index e81d0316d208cf08c039cba03481a7e7979726db..d1d1add8c81dc4c6762dca83795e8ce0bc519df3 100644 (file)
@@ -71,6 +71,7 @@ import org.opendaylight.netconf.sal.streams.listeners.ListenerAdapter;
 import org.opendaylight.netconf.sal.streams.listeners.NotificationListenerAdapter;
 import org.opendaylight.netconf.sal.streams.listeners.Notificator;
 import org.opendaylight.netconf.sal.streams.websockets.WebSocketServer;
+import org.opendaylight.restconf.common.OperationsContent;
 import org.opendaylight.restconf.common.context.InstanceIdentifierContext;
 import org.opendaylight.restconf.common.context.NormalizedNodeContext;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
@@ -327,8 +328,14 @@ public final class RestconfImpl implements RestconfService {
 
     @Override
     @Deprecated
-    public NormalizedNodeContext getOperations(final UriInfo uriInfo) {
-        return OperationsResourceUtils.contextForModelContext(controllerContext.getGlobalSchema(), null);
+    public String getOperationsJSON() {
+        return OperationsContent.JSON.bodyFor(controllerContext.getGlobalSchema());
+    }
+
+    @Override
+    @Deprecated
+    public String getOperationsXML() {
+        return OperationsContent.XML.bodyFor(controllerContext.getGlobalSchema());
     }
 
     @Override
index 33830d150c70db2d3f30182872f94953f11119aa..cfd326fc8832158bedf35b5db8e363d181153208 100644 (file)
@@ -77,8 +77,13 @@ public final class StatisticsRestconfServiceWrapper implements RestconfService {
     }
 
     @Override
-    public NormalizedNodeContext getOperations(final UriInfo uriInfo) {
-        return this.delegate.getOperations(uriInfo);
+    public String getOperationsJSON() {
+        return this.delegate.getOperationsJSON();
+    }
+
+    @Override
+    public String getOperationsXML() {
+        return this.delegate.getOperationsXML();
     }
 
     @Override
index de59c2877265cb8b37883d35ec24610297e2e4db..2a1a1a5bba472d2c9c93c0c1682402165f7e4f1b 100644 (file)
@@ -22,21 +22,24 @@ import org.opendaylight.restconf.nb.rfc8040.MediaTypes;
  */
 public interface RestconfOperationsService {
     /**
-     * List of rpc or action operations supported by the server.
+     * List RPC and action operations in RFC7951 format.
      *
-     * @param uriInfo URI information
-     * @return {@link NormalizedNodeContext}
+     * @return A string containing a JSON document conforming to both RFC8040 and RFC7951.
      */
     @GET
     @Path("/operations")
-    @Produces({
-        MediaTypes.APPLICATION_YANG_DATA_JSON,
-        MediaTypes.APPLICATION_YANG_DATA_XML,
-        MediaType.APPLICATION_JSON,
-        MediaType.APPLICATION_XML,
-        MediaType.TEXT_XML
-    })
-    NormalizedNodeContext getOperations(@Context UriInfo uriInfo);
+    @Produces({ MediaTypes.APPLICATION_YANG_DATA_JSON, MediaType.APPLICATION_JSON })
+    String getOperationsJSON();
+
+    /**
+     * List RPC and action operations in RFC8040 XML format.
+     *
+     * @return A string containing a JSON document conforming to both RFC8040 section 11.3.1 and page 84.
+     */
+    @GET
+    @Path("/operations")
+    @Produces({ MediaTypes.APPLICATION_YANG_DATA_XML, MediaType.APPLICATION_XML, MediaType.TEXT_XML })
+    String getOperationsXML();
 
     /**
      * Valid for mount points. List of operations supported by the server.
index 4e12ba40a640cd494d4e0f2af025b0e325eefae3..28c0cd14cd66fcdc75d1b47418c5d8559fbabaae 100644 (file)
@@ -15,6 +15,7 @@ import javax.ws.rs.core.UriInfo;
 import org.opendaylight.mdsal.dom.api.DOMMountPoint;
 import org.opendaylight.mdsal.dom.api.DOMMountPointService;
 import org.opendaylight.mdsal.dom.api.DOMSchemaService;
+import org.opendaylight.restconf.common.OperationsContent;
 import org.opendaylight.restconf.common.context.InstanceIdentifierContext;
 import org.opendaylight.restconf.common.context.NormalizedNodeContext;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
@@ -53,8 +54,13 @@ public class RestconfOperationsServiceImpl implements RestconfOperationsService
     }
 
     @Override
-    public NormalizedNodeContext getOperations(final UriInfo uriInfo) {
-        return OperationsResourceUtils.contextForModelContext(schemaContextHandler.get(), null);
+    public String getOperationsJSON() {
+        return OperationsContent.JSON.bodyFor(schemaContextHandler.get());
+    }
+
+    @Override
+    public String getOperationsXML() {
+        return OperationsContent.XML.bodyFor(schemaContextHandler.get());
     }
 
     @Override
index 31f1638d9c7f87c290923aaacbc61f99cc1a424b..e65a8cdbba9550c9864a078bc6f7bf8efcf1fd49 100644 (file)
@@ -8,74 +8,42 @@
 package org.opendaylight.restconf.nb.rfc8040.rests.services.impl;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
 
-import com.google.common.collect.ImmutableSet;
-import java.util.Set;
-import javax.ws.rs.core.UriInfo;
-import org.junit.Before;
+import java.io.IOException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.mdsal.dom.api.DOMMountPointService;
-import org.opendaylight.restconf.common.context.NormalizedNodeContext;
 import org.opendaylight.restconf.nb.rfc8040.TestRestconfUtils;
 import org.opendaylight.restconf.nb.rfc8040.TestUtils;
-import org.opendaylight.restconf.nb.rfc8040.handlers.SchemaContextHandler;
-import org.opendaylight.yangtools.yang.common.Empty;
-import org.opendaylight.yangtools.yang.common.QName;
-import org.opendaylight.yangtools.yang.common.QNameModule;
-import org.opendaylight.yangtools.yang.common.XMLNamespace;
-import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
-import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
-import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext;
 import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
 
 @RunWith(MockitoJUnitRunner.StrictStubs.class)
 public class RestconfOperationsServiceTest {
-
-    @Mock
-    private DOMMountPointService domMountPointService;
-
-    @Mock
-    private UriInfo uriInfo;
-
-    private EffectiveModelContext schemaContext;
-    private SchemaContextHandler schemaContextHandler;
-
-    private Set<QName> listOfRpcsNames;
-
-    @Before
-    public void init() throws Exception {
-        this.schemaContext = YangParserTestUtils.parseYangFiles(TestRestconfUtils.loadFiles("/modules"));
-        this.schemaContextHandler = TestUtils.newSchemaContextHandler(schemaContext);
-
-        final QNameModule module1 = QNameModule.create(XMLNamespace.of("module:1"));
-        final QNameModule module2 = QNameModule.create(XMLNamespace.of("module:2"));
-
-        this.listOfRpcsNames = ImmutableSet.of(QName.create(module1, "dummy-rpc1-module1"),
-                QName.create(module1, "dummy-rpc2-module1"), QName.create(module2, "dummy-rpc1-module2"),
-                QName.create(module2, "dummy-rpc2-module2"));
-    }
-
     @Test
-    public void getOperationsTest() {
-        final RestconfOperationsServiceImpl oper =
-                new RestconfOperationsServiceImpl(this.schemaContextHandler, this.domMountPointService);
-        final NormalizedNodeContext operations = oper.getOperations(this.uriInfo);
-        final ContainerNode data = (ContainerNode) operations.getData();
-        assertEquals("urn:ietf:params:xml:ns:yang:ietf-restconf",
-            data.getIdentifier().getNodeType().getNamespace().toString());
-        assertEquals("operations", data.getIdentifier().getNodeType().getLocalName());
-
-        assertEquals(4, data.body().size());
-
-        for (final DataContainerChild child : data.body()) {
-            assertEquals(Empty.getInstance(), child.body());
-
-            final QName qname = child.getIdentifier().getNodeType().withoutRevision();
-            assertTrue(this.listOfRpcsNames.contains(qname));
-        }
+    public void getOperationsTest() throws IOException {
+        final var oper = new RestconfOperationsServiceImpl(
+            TestUtils.newSchemaContextHandler(
+                YangParserTestUtils.parseYangFiles(TestRestconfUtils.loadFiles("/modules"))),
+            mock(DOMMountPointService.class));
+
+        assertEquals("{\n"
+            + "  \"ietf-restconf:operations\" : {\n"
+            + "    \"module1:dummy-rpc1-module1\": [null],\n"
+            + "    \"module1:dummy-rpc2-module1\": [null],\n"
+            + "    \"module2:dummy-rpc1-module2\": [null],\n"
+            + "    \"module2:dummy-rpc2-module2\": [null]\n"
+            + "  }\n"
+            + "}", oper.getOperationsJSON());
+        assertEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<operations xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"\n"
+            + "            xmlns:ns0=\"module:1\"\n"
+            + "            xmlns:ns1=\"module:2\" >\n"
+            + "  <ns0:dummy-rpc1-module1/>\n"
+            + "  <ns0:dummy-rpc2-module1/>\n"
+            + "  <ns1:dummy-rpc1-module2/>\n"
+            + "  <ns1:dummy-rpc2-module2/>\n"
+            + "</operations>", oper.getOperationsXML());
     }
 }