Fix dataGet() of MapEntries

Expand RestconfDataGetTest to cover more NormalizedNode structures,
verifying we are producing what we need.

This flushes out an alignment problem between XML and JSON: the latter
does need the entrypoint to be MapNode, the other does not care.

Change-Id: Ic11863b6dec5bd1a5bb860dd3d3732c5e3c53384
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/NormalizedFormattableBody.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/NormalizedFormattableBody.java
index 64a96d19fcf80f9930632196a1b1acd2a4a6932f..19063bd6b8706893aa5aa8597a0b426b509df858 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/NormalizedFormattableBody.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/NormalizedFormattableBody.java
@@ -22,15 +22,14 @@ import org.opendaylight.restconf.api.query.PrettyPrintParam;
 import org.opendaylight.restconf.server.api.DatabindContext;
 import org.opendaylight.restconf.server.api.DatabindFormattableBody;
 import org.opendaylight.restconf.server.api.DatabindPath.Data;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
-import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
-import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter;
 import org.opendaylight.yangtools.yang.data.codec.gson.JSONCodecFactory;
 import org.opendaylight.yangtools.yang.data.codec.xml.XmlCodecFactory;
-import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack.Inference;
+import org.opendaylight.yangtools.yang.data.spi.node.ImmutableNodes;
 
 /**
  * A {@link FormattableBody} representing a data resource.
@@ -59,18 +58,30 @@ public abstract sealed class NormalizedFormattableBody<N extends NormalizedNode>
             throw new VerifyException("Unexpected root data contract " + data.contract());
         }
 
-        // Read of a sub-resource. We need to adjust the inference to point to the NormalizedNode parent of the node
-        // being output.
-        final Inference parentInference;
-        if (data instanceof MapEntryNode || data instanceof LeafSetEntryNode || data instanceof UnkeyedListEntryNode) {
-            parentInference = inference;
-        } else {
-            final var stack = inference.toSchemaInferenceStack();
-            stack.exitToDataTree();
-            parentInference = stack.toInference();
-        }
-
-        return new DataFormattableBody<>(path.databind(), parentInference, data, writerFactory);
+        // RESTCONF allows returning one list item. We need to wrap it in map node in order to serialize it properly.
+        // We need to point to a 'a parent inference' and provide an appropriate data entry. Unfortunately it is not
+        // quite defined what that actually means.
+        //
+        // This is a tricky thing, as JSON and XML have different representations of a MapNode. In JSON it is the array
+        // containing individual objects. In XML it is transparent.
+        //
+        // This means that for XML we could just move 'parent inference' to the MapNode and emit the MapEntryNode as
+        // usual. For JSON that does not work, as we also need to wrap the MapEntryNode in a MapNode.
+        //
+        // What we do here is we unconditionally:
+        //   - wrap the node if it is a list entry node
+        //   - move the inference to parent
+        //
+        // For XML that does not seem to matter. For JSON it does matter a lot.
+        final var stack = inference.toSchemaInferenceStack();
+        stack.exit();
+
+        return new DataFormattableBody<>(path.databind(), stack.toInference(), data instanceof MapEntryNode mapEntry
+            ? ImmutableNodes.newSystemMapBuilder()
+                .withNodeIdentifier(new NodeIdentifier(data.name().getNodeType()))
+                .withChild(mapEntry)
+                .build()
+            : data, writerFactory);
     }
 
     /**
@@ -122,6 +133,4 @@ public abstract sealed class NormalizedFormattableBody<N extends NormalizedNode>
     protected ToStringHelper addToStringAttributes(final ToStringHelper helper) {
         return helper.add("body", data.prettyTree());
     }
-
-
 }

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/jaxrs/RestconfDataGetTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/jaxrs/RestconfDataGetTest.java
index 74848baf1ddf9ca5aaed8bccaea12d88af781f21..f8e7b88eaec9243152e01807c439d3b74a187077 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/jaxrs/RestconfDataGetTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/jaxrs/RestconfDataGetTest.java
@@ -35,11 +35,19 @@ import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild;
 import org.opendaylight.yangtools.yang.data.spi.node.ImmutableNodes;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
+// FIXME: this test suite should be refactored as an AbstractDataBrokerTest without mocks:
+//          - AbstractRestconfTest.restconf should be replaced with an instance wired to
+//            AbstractDataBrokerTest.getDomBroker() et al.
+//          - then each test case should initialize the datastores with test data
+//          - then each test case should execute the request
+//        if you are doing this, please structure it so that the infra can be brought down to AbstractRestconfTest and
+//        reused in Netconf822Test and the like
 @ExtendWith(MockitoExtension.class)
 class RestconfDataGetTest extends AbstractRestconfTest {
     private static final NodeIdentifier PLAYLIST_NID = new NodeIdentifier(PLAYLIST_QNAME);
@@ -273,4 +281,36 @@ class RestconfDataGetTest extends AbstractRestconfTest {
               </player>
             </jukebox>""", body::formatToXML, true);
     }
+
+    @Test
+    void readListEntry() {
+        final var parameters = new MultivaluedHashMap<String, String>();
+        parameters.putSingle("content", "nonconfig");
+
+        doReturn(parameters).when(uriInfo).getQueryParameters();
+        doReturn(immediateFluentFuture(Optional.of(ImmutableNodes.newMapEntryBuilder()
+            .withNodeIdentifier(NodeIdentifierWithPredicates.of(ARTIST_QNAME, NAME_QNAME, "IAmN0t"))
+            .withChild(ImmutableNodes.leafNode(NAME_QNAME, "IAmN0t"))
+            .build()))).when(tx).read(LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.builder()
+                .node(JUKEBOX_QNAME)
+                .node(LIBRARY_QNAME)
+                .node(ARTIST_QNAME)
+                .nodeWithKey(ARTIST_QNAME, NAME_QNAME, "IAmN0t")
+                .build());
+
+        final var body = assertNormalizedBody(200,
+            ar -> restconf.dataGET(apiPath("example-jukebox:jukebox/library/artist=IAmN0t"), uriInfo, ar));
+        assertFormat("""
+            {
+              "example-jukebox:artist": [
+                {
+                  "name": "IAmN0t"
+                }
+              ]
+            }""", body::formatToJSON, true);
+        assertFormat("""
+            <artist xmlns="http://example.com/ns/example-jukebox">
+              <name>IAmN0t</name>
+            </artist>""", body::formatToXML, true);
+    }
 }