From 42a49525a3b291a4af093090a54ab4c88a2313ca Mon Sep 17 00:00:00 2001 From: "miroslav.kovac" Date: Mon, 16 Dec 2019 17:28:41 +0100 Subject: [PATCH] Fix RPC method name conflict with JLS When we are constructing the method name for use with the RPC provider interface we need to pay attention to conflicts with JLS reserved words. If such a conflict occurs, append a single '$' to the name, so that we have a valid method name. JIRA: MDSAL-500 Change-Id: Ifb533e2daa71bc9d1780a5d8eb48da6ce3b2d209 Signed-off-by: miroslav.kovac Signed-off-by: Robert Varga (cherry picked from commit 6e6ebc43866204667658f8bbf7cfb6233caffdc7) --- .../adapter/BindingToNormalizedNodeCodec.java | 2 +- .../dom/adapter/test/Mdsal500Test.java | 176 ++++++++++++++++++ .../generator/impl/AbstractTypeGenerator.java | 2 +- .../binding/generator/impl/Mdsal500Test.java | 32 ++++ .../src/test/resources/mdsal-500.yang | 13 ++ .../binding/spec/naming/BindingMapping.java | 12 ++ .../src/main/yang/mdsal-500.yang | 13 ++ 7 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal500Test.java create mode 100644 binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal500Test.java create mode 100644 binding/mdsal-binding-generator-impl/src/test/resources/mdsal-500.yang create mode 100644 binding/mdsal-binding-test-model/src/main/yang/mdsal-500.yang diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java index 8cd510b8bb..55fde2cd65 100644 --- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java +++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java @@ -395,7 +395,7 @@ public class BindingToNormalizedNodeCodec implements BindingCodecTreeFactory, private Method findRpcMethod(final Class key, final RpcDefinition rpcDef) throws NoSuchMethodException { - final String methodName = BindingMapping.getMethodName(rpcDef.getQName()); + final String methodName = BindingMapping.getRpcMethodName(rpcDef.getQName()); final Class inputClz = runtimeContext().getClassForSchema(rpcDef.getInput()); return key.getMethod(methodName, inputClz); } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal500Test.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal500Test.java new file mode 100644 index 0000000000..daa7f154a2 --- /dev/null +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal500Test.java @@ -0,0 +1,176 @@ +/* + * Copyright (c) 2019 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.test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Multimap; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.junit.Before; +import org.junit.Test; +import org.opendaylight.mdsal.binding.api.RpcConsumerRegistry; +import org.opendaylight.mdsal.binding.api.RpcProviderService; +import org.opendaylight.mdsal.binding.dom.adapter.test.util.BindingBrokerTestFactory; +import org.opendaylight.mdsal.binding.dom.adapter.test.util.BindingTestContext; +import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections; +import org.opendaylight.mdsal.dom.api.DOMRpcIdentifier; +import org.opendaylight.mdsal.dom.api.DOMRpcProviderService; +import org.opendaylight.mdsal.dom.api.DOMRpcResult; +import org.opendaylight.mdsal.dom.api.DOMRpcService; +import org.opendaylight.mdsal.dom.spi.DefaultDOMRpcResult; +import org.opendaylight.yang.gen.v1.rpc.norev.Mdsal500Service; +import org.opendaylight.yang.gen.v1.rpc.norev.SwitchInput; +import org.opendaylight.yang.gen.v1.rpc.norev.SwitchInputBuilder; +import org.opendaylight.yang.gen.v1.rpc.norev.SwitchOutput; +import org.opendaylight.yang.gen.v1.rpc.norev.SwitchOutputBuilder; +import org.opendaylight.yangtools.concepts.ObjectRegistration; +import org.opendaylight.yangtools.util.concurrent.FluentFutures; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.common.RpcResult; +import org.opendaylight.yangtools.yang.common.RpcResultBuilder; +import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; +import org.opendaylight.yangtools.yang.model.api.SchemaPath; + +public class Mdsal500Test { + private static final String FOO = "foo"; + + private static final QName SWITCH_QNAME = QName.create(SwitchOutput.QNAME, "switch"); + private static final SchemaPath SWITCH_PATH = SchemaPath.create(true, SWITCH_QNAME); + + private RpcProviderService baRpcProviderService; + private RpcConsumerRegistry baRpcConsumerService; + private DOMRpcProviderService biRpcProviderService; + private BindingTestContext testContext; + private DOMRpcService biRpcService; + private final Mdsal500ServiceImpl switchRpcImpl = new Mdsal500ServiceImpl(); + + @Before + public void setup() throws Exception { + BindingBrokerTestFactory testFactory = new BindingBrokerTestFactory(); + testFactory.setExecutor(MoreExecutors.newDirectExecutorService()); + testContext = testFactory.getTestContext(); + + testContext.setSchemaModuleInfos(ImmutableSet.of( + BindingReflections.getModuleInfo(Mdsal500Service.class))); + testContext.start(); + baRpcProviderService = testContext.getBindingRpcProviderRegistry(); + baRpcConsumerService = testContext.getBindingRpcConsumerRegistry(); + biRpcProviderService = testContext.getDomRpcRegistry(); + biRpcService = testContext.getDomRpcInvoker(); + } + + @Test + public void testBindingRegistrationWithDOMInvocation() throws Exception { + switchRpcImpl.registerTo(baRpcProviderService).setSwitchResult(switchResult(true)); + + final Mdsal500Service baSwitchService = baRpcConsumerService.getRpcService(Mdsal500Service.class); + assertNotSame(switchRpcImpl, baSwitchService); + + SwitchInput baSwitchInput = switchBuilder(FOO).build(); + + ContainerNode biSwitchInput = toDOMSwitchInput(baSwitchInput); + DOMRpcResult domResult = biRpcService.invokeRpc(SWITCH_PATH, biSwitchInput).get(5, TimeUnit.SECONDS); + assertNotNull(domResult); + assertNotNull(domResult.getResult()); + assertTrue("Binding KnockKnock service was not invoked", + switchRpcImpl.getReceivedSwitch().containsKey(FOO)); + assertEquals(baSwitchInput, switchRpcImpl.getReceivedSwitch().get(FOO).iterator().next()); + } + + @Test + public void testDOMRegistrationWithBindingInvocation() + throws InterruptedException, ExecutionException, TimeoutException { + SwitchOutput baSwitchOutput = new SwitchOutputBuilder().build(); + + biRpcProviderService.registerRpcImplementation((rpc, input) -> + FluentFutures.immediateFluentFuture(new DefaultDOMRpcResult(testContext.getCodec() + .getCodecFactory().toNormalizedNodeRpcData(baSwitchOutput))), + DOMRpcIdentifier.create(SWITCH_PATH)); + + final Mdsal500Service baSwitchService = + baRpcConsumerService.getRpcService(Mdsal500Service.class); + Future> baResult = baSwitchService.switch$(switchBuilder(FOO) + .build()); + assertNotNull(baResult); + assertEquals(baSwitchOutput, baResult.get(5, TimeUnit.SECONDS).getResult()); + } + + @Test + public void testBindingRpcShortcut() throws InterruptedException, ExecutionException, TimeoutException { + final ListenableFuture> baSwitchResult = switchResult(true); + switchRpcImpl.registerTo(baRpcProviderService).setSwitchResult(baSwitchResult); + + final Mdsal500Service baSwitchService = baRpcConsumerService.getRpcService(Mdsal500Service.class); + + SwitchInput baSwitchInput = switchBuilder(FOO).build(); + ListenableFuture> future = baSwitchService.switch$(baSwitchInput); + + final RpcResult rpcResult = future.get(5, TimeUnit.SECONDS); + + assertEquals(baSwitchResult.get().getResult().getClass(), rpcResult.getResult().getClass()); + assertSame(baSwitchResult.get().getResult(), rpcResult.getResult()); + assertSame(baSwitchInput, switchRpcImpl.getReceivedSwitch().get(FOO).iterator().next()); + } + + private static ListenableFuture> switchResult(final boolean success) { + SwitchOutput output = new SwitchOutputBuilder().build(); + RpcResult result = RpcResultBuilder.status(success).withResult(output) + .build(); + return Futures.immediateFuture(result); + } + + private static SwitchInputBuilder switchBuilder(final String foo) { + SwitchInputBuilder builder = new SwitchInputBuilder(); + builder.setFoo(foo); + return builder; + } + + private ContainerNode toDOMSwitchInput(final SwitchInput from) { + return testContext.getCodec().getCodecFactory().toNormalizedNodeRpcData(from); + } + + private static class Mdsal500ServiceImpl implements Mdsal500Service { + private ListenableFuture> switchResult; + private final Multimap receivedSwitch = HashMultimap.create(); + + Mdsal500ServiceImpl setSwitchResult(final ListenableFuture> switchOutput) { + this.switchResult = switchOutput; + return this; + } + + Multimap getReceivedSwitch() { + return receivedSwitch; + } + + Mdsal500ServiceImpl registerTo(final RpcProviderService registry) { + final ObjectRegistration registration = + registry.registerRpcImplementation(Mdsal500Service.class, this); + assertNotNull(registration); + return this; + } + + @Override + public ListenableFuture> switch$(SwitchInput switchInput) { + receivedSwitch.put(switchInput.getFoo(), switchInput); + return switchResult; + } + } + +} diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java index 133d5c16d6..58e53a5022 100644 --- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java +++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/AbstractTypeGenerator.java @@ -522,7 +522,7 @@ abstract class AbstractTypeGenerator { for (final RpcDefinition rpc : rpcDefinitions) { if (rpc != null) { final String rpcName = BindingMapping.getClassName(rpc.getQName()); - final String rpcMethodName = BindingMapping.getPropertyName(rpcName); + final String rpcMethodName = BindingMapping.getRpcMethodName(rpc.getQName()); final MethodSignatureBuilder method = interfaceBuilder.addMethod(rpcMethodName); // Do not refer to annotation class, as it may not be available at runtime diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal500Test.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal500Test.java new file mode 100644 index 0000000000..5b2094e552 --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal500Test.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2019 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.generator.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.util.List; +import org.junit.Test; +import org.opendaylight.mdsal.binding.model.api.GeneratedType; +import org.opendaylight.mdsal.binding.model.api.MethodSignature; +import org.opendaylight.mdsal.binding.model.api.Type; +import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; + +public class Mdsal500Test { + + @Test + public void testAugmentedAction() { + final List types = new BindingGeneratorImpl().generateTypes( + YangParserTestUtils.parseYangResource("/mdsal-500.yang")); + assertNotNull(types); + final MethodSignature signature = ((GeneratedType) types.get(2)).getMethodDefinitions().iterator().next(); + assertEquals("switch$", signature.getName()); + assertEquals(3, types.size()); + } + +} diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/mdsal-500.yang b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal-500.yang new file mode 100644 index 0000000000..c03fe98941 --- /dev/null +++ b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal-500.yang @@ -0,0 +1,13 @@ +module mdsal-500 { + namespace "rpc"; + prefix "rpc"; + + rpc switch { + input { + leaf foo { + type string; + } + } + } +} + diff --git a/binding/mdsal-binding-spec-util/src/main/java/org/opendaylight/mdsal/binding/spec/naming/BindingMapping.java b/binding/mdsal-binding-spec-util/src/main/java/org/opendaylight/mdsal/binding/spec/naming/BindingMapping.java index 4513b60c56..2e915694e5 100644 --- a/binding/mdsal-binding-spec-util/src/main/java/org/opendaylight/mdsal/binding/spec/naming/BindingMapping.java +++ b/binding/mdsal-binding-spec-util/src/main/java/org/opendaylight/mdsal/binding/spec/naming/BindingMapping.java @@ -291,6 +291,18 @@ public final class BindingMapping { return str.substring(0, 1).toLowerCase(Locale.ENGLISH) + str.substring(1); } + /** + * Returns the {@link String} {@code s} with a '$' character as suffix. + * + * @param qname RPC QName + * @return The RPC method name as determined by considering the localname against the JLS. + * @throws NullPointerException if {@code qname} is null + */ + public static @NonNull String getRpcMethodName(final @NonNull QName qname) { + final String methodName = getMethodName(qname); + return JAVA_RESERVED_WORDS.contains(methodName) ? methodName + "$" : methodName; + } + /** * Returns Java identifiers, conforming to JLS9 Section 3.8 to use for specified YANG assigned names * (RFC7950 Section 9.6.4). This method considers two distinct encodings: one the pre-Fluorine mapping, which is diff --git a/binding/mdsal-binding-test-model/src/main/yang/mdsal-500.yang b/binding/mdsal-binding-test-model/src/main/yang/mdsal-500.yang new file mode 100644 index 0000000000..c03fe98941 --- /dev/null +++ b/binding/mdsal-binding-test-model/src/main/yang/mdsal-500.yang @@ -0,0 +1,13 @@ +module mdsal-500 { + namespace "rpc"; + prefix "rpc"; + + rpc switch { + input { + leaf foo { + type string; + } + } + } +} + -- 2.36.6