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 <miroslav.kovac@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 6e6ebc43866204667658f8bbf7cfb6233caffdc7)

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 8cd510b8bb1f6a5112f8a496c54f8e4b8d9de8cc..55fde2cd655b19b5e47517d3ca0ec7895f84cf50 100644 (file)
--- 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<? extends RpcService> 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 (file)
index 0000000..daa7f15
--- /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<RpcResult<SwitchOutput>> 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<RpcResult<SwitchOutput>> baSwitchResult = switchResult(true);
+        switchRpcImpl.registerTo(baRpcProviderService).setSwitchResult(baSwitchResult);
+
+        final Mdsal500Service baSwitchService = baRpcConsumerService.getRpcService(Mdsal500Service.class);
+
+        SwitchInput baSwitchInput = switchBuilder(FOO).build();
+        ListenableFuture<RpcResult<SwitchOutput>> future = baSwitchService.switch$(baSwitchInput);
+
+        final RpcResult<SwitchOutput> 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<RpcResult<SwitchOutput>> switchResult(final boolean success) {
+        SwitchOutput output = new SwitchOutputBuilder().build();
+        RpcResult<SwitchOutput> result = RpcResultBuilder.<SwitchOutput>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<RpcResult<SwitchOutput>> switchResult;
+        private final Multimap<String, SwitchInput> receivedSwitch = HashMultimap.create();
+
+        Mdsal500ServiceImpl setSwitchResult(final ListenableFuture<RpcResult<SwitchOutput>> switchOutput) {
+            this.switchResult = switchOutput;
+            return this;
+        }
+
+        Multimap<String, SwitchInput> getReceivedSwitch() {
+            return receivedSwitch;
+        }
+
+        Mdsal500ServiceImpl registerTo(final RpcProviderService registry) {
+            final ObjectRegistration<Mdsal500ServiceImpl> registration =
+                    registry.registerRpcImplementation(Mdsal500Service.class, this);
+            assertNotNull(registration);
+            return this;
+        }
+
+        @Override
+        public ListenableFuture<RpcResult<SwitchOutput>> 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 133d5c16d6dcf4695dca8ae597d156cc0c0a5d55..58e53a5022e9d5084088e3c6652c929542b8fa87 100644 (file)
--- 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 (file)
index 0000000..5b2094e
--- /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<Type> 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 (file)
index 0000000..c03fe98
--- /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 4513b60c567a31696ba9e092bf9e202c07a0ef47..2e915694e511b4536ed8e013f3b056daa9c56d3f 100644 (file)
--- 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 (file)
index 0000000..c03fe98
--- /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;
+            }
+        }
+    }
+}
+