Inject RPC input QName into LazySerializedContainerNode

When ExecuteRpc message is sent with LazySerializedContainerNode
as 'input' parameter, during serialization is invoked its
getIdentifier() method that returns NodeIdentfier of rpc,
not input. So OpsInvoker receives the message with incorrect 'input'
parameter and  we get error downstream because of it.

We need to inject rpc input Qname into LazySerializedContainerNode
to get NodeIdentifier of rpc input from getIdentifier() method. While we
are at it, let's pass a NodeIdentifier, which saves us the cost of
NodeIdentifier.create() in the fast path.

JIRA: MDSAL-739
Change-Id: I77c870fe91a758037bd27e54607e778255d611fc
Signed-off-by: Dominik Vrbovsky <dominik.vrbovsky@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNode.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNode.java
index fad68245c2cebd41c14a672e47272f92f6bd583b..2526e920ea0189ee22f0d69122c8eb0f4d2b0bbc 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNode.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNode.java
@@ -9,10 +9,10 @@ package org.opendaylight.mdsal.binding.dom.adapter;
 
 import static java.util.Objects.requireNonNull;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer;
 import org.opendaylight.mdsal.binding.dom.codec.spi.AbstractBindingLazyContainerNode;
 import org.opendaylight.yangtools.yang.binding.DataObject;
-import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
@@ -30,19 +30,19 @@ import org.opendaylight.yangtools.yang.data.api.schema.LeafNode;
 class LazySerializedContainerNode
         extends AbstractBindingLazyContainerNode<DataObject, BindingNormalizedNodeSerializer> {
 
-    private LazySerializedContainerNode(final QName identifier, final DataObject binding,
+    private LazySerializedContainerNode(final @NonNull NodeIdentifier identifier, final DataObject binding,
             final BindingNormalizedNodeSerializer codec) {
-        super(NodeIdentifier.create(identifier), binding, requireNonNull(codec));
+        super(identifier, binding, requireNonNull(codec));
     }
 
-    static ContainerNode create(final QName rpcName, final DataObject data,
+    static ContainerNode create(final @NonNull NodeIdentifier identifier, final DataObject data,
             final BindingNormalizedNodeSerializer codec) {
-        return data == null ? null : new LazySerializedContainerNode(rpcName, data, codec);
+        return data == null ? null : new LazySerializedContainerNode(identifier, data, codec);
     }
 
-    static ContainerNode withContextRef(final QName rpcName, final DataObject data,
+    static ContainerNode withContextRef(final @NonNull NodeIdentifier identifier, final DataObject data,
             final LeafNode<?> contextRef, final BindingNormalizedNodeSerializer serializer) {
-        return new WithContextRef(rpcName, data, contextRef, serializer);
+        return new WithContextRef(identifier, data, contextRef, serializer);
     }
 
     @Override
@@ -56,8 +56,8 @@ class LazySerializedContainerNode
     private static final class WithContextRef extends LazySerializedContainerNode {
         private final LeafNode<?> contextRef;
 
-        protected WithContextRef(final QName identifier, final DataObject binding, final LeafNode<?> contextRef,
-                final BindingNormalizedNodeSerializer codec) {
+        protected WithContextRef(final @NonNull NodeIdentifier identifier, final DataObject binding,
+                final LeafNode<?> contextRef, final BindingNormalizedNodeSerializer codec) {
             super(identifier, binding, codec);
             this.contextRef = requireNonNull(contextRef);
         }

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/RpcServiceAdapter.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/RpcServiceAdapter.java
index 8c9dd9c5e3e62ca8d440f222b474c2c80b117475..12c604dcb3497a24446bcd94dc0cf72e4fc29240 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/RpcServiceAdapter.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/RpcServiceAdapter.java
@@ -55,7 +55,7 @@ class RpcServiceAdapter implements InvocationHandler {
             final DOMRpcService domService) {
         this.type = requireNonNull(type);
         this.adapterContext = requireNonNull(adapterContext);
-        this.delegate = requireNonNull(domService);
+        delegate = requireNonNull(domService);
 
         final ImmutableBiMap<Method, RpcDefinition> methods = adapterContext.currentSerializer()
                 .getRpcMethodToSchema(type);
@@ -114,11 +114,13 @@ class RpcServiceAdapter implements InvocationHandler {
     }
 
     private abstract class RpcInvocationStrategy {
+        private final @NonNull NodeIdentifier inputIdentifier;
         private final @NonNull Absolute outputPath;
 
         RpcInvocationStrategy(final QName rpcName) {
-            this.outputPath = Absolute.of(rpcName, YangConstants.operationOutputQName(rpcName.getModule()).intern())
-                    .intern();
+            final var namespace = rpcName.getModule();
+            outputPath = Absolute.of(rpcName, YangConstants.operationOutputQName(namespace).intern()).intern();
+            inputIdentifier = NodeIdentifier.create(YangConstants.operationInputQName(namespace.intern()));
         }
 
         final ListenableFuture<RpcResult<?>> invoke(final DataObject input) {
@@ -127,11 +129,11 @@ class RpcServiceAdapter implements InvocationHandler {
 
         abstract ContainerNode serialize(DataObject input);
 
-        final QName getRpcName() {
-            return outputPath.firstNodeIdentifier();
+        final @NonNull NodeIdentifier inputIdentifier() {
+            return inputIdentifier;
         }
 
-        ListenableFuture<RpcResult<?>> invoke0(final ContainerNode input) {
+        private ListenableFuture<RpcResult<?>> invoke0(final ContainerNode input) {
             final ListenableFuture<? extends DOMRpcResult> result =
                     delegate.invokeRpc(outputPath.firstNodeIdentifier(), input);
             if (ENABLE_CODEC_SHORTCUT && result instanceof BindingRpcFutureAware) {
@@ -164,7 +166,7 @@ class RpcServiceAdapter implements InvocationHandler {
 
         @Override
         ContainerNode serialize(final DataObject input) {
-            return LazySerializedContainerNode.create(getRpcName(), input, adapterContext.currentSerializer());
+            return LazySerializedContainerNode.create(inputIdentifier(), input, adapterContext.currentSerializer());
         }
     }
 
@@ -179,7 +181,7 @@ class RpcServiceAdapter implements InvocationHandler {
             checkState(maybeInputType.isPresent(), "RPC method %s has no input", rpcMethod.getName());
             final Class<? extends DataContainer> inputType = maybeInputType.get();
             refExtractor = ContextReferenceExtractor.from(inputType);
-            this.contextName = new NodeIdentifier(leafName);
+            contextName = new NodeIdentifier(leafName);
         }
 
         @Override
@@ -190,9 +192,9 @@ class RpcServiceAdapter implements InvocationHandler {
             if (bindingII != null) {
                 final YangInstanceIdentifier yangII = serializer.toCachedYangInstanceIdentifier(bindingII);
                 final LeafNode<?> contextRef = ImmutableNodes.leafNode(contextName, yangII);
-                return LazySerializedContainerNode.withContextRef(getRpcName(), input, contextRef, serializer);
+                return LazySerializedContainerNode.withContextRef(inputIdentifier(), input, contextRef, serializer);
             }
-            return LazySerializedContainerNode.create(getRpcName(), input, serializer);
+            return LazySerializedContainerNode.create(inputIdentifier(), input, serializer);
         }
 
     }

diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNodeTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNodeTest.java
index 2bdbaf8855bb9c5cd4344cbf55551ffe7849ee2c..0605d527d562204fff1dffaf338e0447d3c16868 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNodeTest.java
+++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/LazySerializedContainerNodeTest.java
@@ -10,6 +10,7 @@ package org.opendaylight.mdsal.binding.dom.adapter;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
@@ -47,20 +48,21 @@ public class LazySerializedContainerNodeTest {
 
         final ImmutableBiMap<?, ?> biMap = bindingTestContext.getCodec().currentSerializer()
                 .getRpcMethodToSchema(OpendaylightTestRpcServiceService.class);
-        final QName rpcName = ((RpcDefinition) biMap.values().iterator().next()).getQName();
+        final NodeIdentifier name = new NodeIdentifier(((RpcDefinition) biMap.values().iterator().next()).getInput()
+            .getQName());
         final LeafNode<?> leafNode = ImmutableLeafNodeBuilder.create().withNodeIdentifier(NodeIdentifier
                 .create(QName.create("", "test"))).withValue("").build();
-        final ContainerNode normalizedNode = LazySerializedContainerNode.create(rpcName, dataObject, codec);
+        final ContainerNode normalizedNode = LazySerializedContainerNode.create(name, dataObject, codec);
         assertNotNull(normalizedNode);
         final LazySerializedContainerNode lazySerializedContainerNode =
-                (LazySerializedContainerNode) LazySerializedContainerNode.withContextRef(rpcName, dataObject, leafNode,
+                (LazySerializedContainerNode) LazySerializedContainerNode.withContextRef(name, dataObject, leafNode,
                         codec);
         assertNotNull(lazySerializedContainerNode);
         assertEquals(leafNode, lazySerializedContainerNode.childByArg(leafNode.getIdentifier()));
         assertNull(lazySerializedContainerNode.childByArg(mock(PathArgument.class)));
 
         assertTrue(lazySerializedContainerNode.body().isEmpty());
-        assertEquals(rpcName, lazySerializedContainerNode.getIdentifier().getNodeType());
+        assertSame(name, lazySerializedContainerNode.getIdentifier());
         assertEquals(dataObject, lazySerializedContainerNode.getDataObject());
     }
 }
\ No newline at end of file

diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal739Test.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal739Test.java
new file mode 100644 (file)
index 0000000..f48d19b
--- /dev/null
+++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Mdsal739Test.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2022 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.mdsal.binding.dom.adapter;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
+import java.util.concurrent.ExecutionException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.opendaylight.mdsal.binding.dom.adapter.test.util.BindingBrokerTestFactory;
+import org.opendaylight.mdsal.binding.dom.adapter.test.util.BindingTestContext;
+import org.opendaylight.mdsal.dom.api.DOMRpcService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.bi.ba.rpcservice.rev140701.OpendaylightTestRpcServiceService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.bi.ba.rpcservice.rev140701.RockTheHouseInput;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.bi.ba.rpcservice.rev140701.RockTheHouseInputBuilder;
+import org.opendaylight.yangtools.yang.common.QName;
+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.NormalizedNode;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
+
+@ExtendWith(MockitoExtension.class)
+public class Mdsal739Test {
+    private ListeningExecutorService executorService;
+    private AdapterContext adapterContext;
+
+    @BeforeEach
+    public void before() {
+        executorService = MoreExecutors.newDirectExecutorService();
+
+        final BindingBrokerTestFactory bindingBrokerTestFactory = new BindingBrokerTestFactory();
+        bindingBrokerTestFactory.setExecutor(executorService);
+        final BindingTestContext bindingTestContext = bindingBrokerTestFactory.getTestContext();
+        bindingTestContext.start();
+
+        adapterContext = bindingTestContext.getCodec();
+    }
+
+    @AfterEach
+    public void after() {
+        executorService.shutdownNow();
+    }
+
+    @Test
+    public void testRpcInputName() {
+        final var rpcService = mock(DOMRpcService.class);
+
+        final var captor = ArgumentCaptor.forClass(NormalizedNode.class);
+        doReturn(Futures.immediateFailedFuture(new Throwable())).when(rpcService).invokeRpc(any(), captor.capture());
+        final var adapter = (OpendaylightTestRpcServiceService) new RpcServiceAdapter(
+            OpendaylightTestRpcServiceService.class, adapterContext, rpcService).getProxy();
+
+        final var result = adapter.rockTheHouse(new RockTheHouseInputBuilder().setZipCode("12345").build());
+        assertThrows(ExecutionException.class, () -> Futures.getDone(result));
+        final var input = captor.getValue();
+        assertThat(input, instanceOf(ContainerNode.class));
+        assertSame(NodeIdentifier.create(RockTheHouseInput.QNAME), input.getIdentifier());
+        final var body = ((ContainerNode) input).body();
+        assertEquals(1, body.size());
+        assertEquals(ImmutableNodes.leafNode(QName.create(RockTheHouseInput.QNAME, "zip-code"), "12345"),
+            body.iterator().next());
+    }
+}