From: Dominik Vrbovsky Date: Wed, 30 Mar 2022 17:53:53 +0000 (+0200) Subject: Inject RPC input QName into LazySerializedContainerNode X-Git-Tag: v9.0.2~31 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=mdsal.git;a=commitdiff_plain;h=7c1493fd2f93985c4b9a45e214cf7a6df9e53cf9 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 Signed-off-by: Robert Varga --- 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 fad68245c2..2526e920ea 100644 --- 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 { - 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 8c9dd9c5e3..12c604dcb3 100644 --- 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 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> 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> invoke0(final ContainerNode input) { + private ListenableFuture> invoke0(final ContainerNode input) { final ListenableFuture 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 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 2bdbaf8855..0605d527d5 100644 --- 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 index 0000000000..f48d19b2a7 --- /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()); + } +}