From 0eab8e1fe3bb630c38801df0cd59e88637c3ecd9 Mon Sep 17 00:00:00 2001 From: Martin Ciglan Date: Thu, 10 Nov 2016 11:36:26 +0100 Subject: [PATCH] Bug 6163: fixed number of argument when resolving rpc input - changed functionality of resolving rpc input. Input class is resolved by it's assignability from DataContainer class - quite a bit of cleanup, comments added https://bugs.opendaylight.org/show_bug.cgi?id=6163 Change-Id: I4a7d05283951db280c3e92301590407da4f40ffa Signed-off-by: Peter Nosal Signed-off-by: Martin Ciglan --- .../dom/adapter/RpcServiceAdapter.java | 6 +- .../util/AbstractMappedRpcInvoker.java | 2 +- .../yang/binding/util/BindingReflections.java | 35 +++--- .../yang/binding/util/RpcMethodInvoker.java | 3 +- .../util/RpcMethodInvokerWithInput.java | 3 +- .../util/AbstractMappedRpcInvokerTest.java | 117 ++++++++++++++++++ 6 files changed, 143 insertions(+), 23 deletions(-) create mode 100644 binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvokerTest.java 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 c5171e1228..eab94df199 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 @@ -9,6 +9,7 @@ package org.opendaylight.mdsal.binding.dom.adapter; import com.google.common.base.Function; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.CheckedFuture; @@ -188,7 +189,10 @@ class RpcServiceAdapter implements InvocationHandler { protected RoutedStrategy(final SchemaPath path, final Method rpcMethod, final QName leafName) { super(path); - final Class inputType = BindingReflections.resolveRpcInputClass(rpcMethod).get(); + final Optional> maybeInputType = + BindingReflections.resolveRpcInputClass(rpcMethod); + Preconditions.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); } diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvoker.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvoker.java index 8ee514e53c..b483923327 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvoker.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvoker.java @@ -45,7 +45,7 @@ abstract class AbstractMappedRpcInvoker extends RpcServiceInvoker { @Override public final Future> invokeRpc(@Nonnull final RpcService impl, @Nonnull final QName rpcName, @Nullable final DataObject input) { - Preconditions.checkNotNull(impl, "implemetation must be supplied"); + Preconditions.checkNotNull(impl, "Implementation must be supplied"); RpcMethodInvoker invoker = map.get(qnameToKey(rpcName)); Preconditions.checkArgument(invoker != null, "Supplied RPC is not valid for implementation %s", impl); diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/BindingReflections.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/BindingReflections.java index 789f9542ee..b38e753536 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/BindingReflections.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/BindingReflections.java @@ -131,8 +131,6 @@ public class BindingReflections { /** * Checks if method is RPC invocation * - * - * * @param possibleMethod * Method to check * @return true if method is RPC invocation, false otherwise. @@ -140,7 +138,11 @@ public class BindingReflections { public static boolean isRpcMethod(final Method possibleMethod) { return possibleMethod != null && RpcService.class.isAssignableFrom(possibleMethod.getDeclaringClass()) && Future.class.isAssignableFrom(possibleMethod.getReturnType()) - && possibleMethod.getParameterTypes().length <= 1; + // length <= 2: it seemed to be impossible to get correct RpcMethodInvoker because of + // resolveRpcInputClass() check.While RpcMethodInvoker counts with one argument for + // non input type and two arguments for input type, resolveRpcInputClass() counting + // with zero for non input and one for input type + && possibleMethod.getParameterTypes().length <= 2; } /** @@ -154,12 +156,12 @@ public class BindingReflections { */ @SuppressWarnings("rawtypes") public static Optional> resolveRpcOutputClass(final Method targetMethod) { - checkState(isRpcMethod(targetMethod), "Supplied method is not Rpc invocation method"); + checkState(isRpcMethod(targetMethod), "Supplied method is not a RPC invocation method"); Type futureType = targetMethod.getGenericReturnType(); Type rpcResultType = ClassLoaderUtils.getFirstGenericParameter(futureType); Type rpcResultArgument = ClassLoaderUtils.getFirstGenericParameter(rpcResultType); if (rpcResultArgument instanceof Class && !Void.class.equals(rpcResultArgument)) { - return Optional.> of((Class) rpcResultArgument); + return Optional.of((Class) rpcResultArgument); } return Optional.absent(); } @@ -172,17 +174,14 @@ public class BindingReflections { * method to scan * @return Optional.absent() if rpc has no input, Rpc input type otherwise. */ - @SuppressWarnings("unchecked") + @SuppressWarnings("rawtypes") public static Optional> resolveRpcInputClass(final Method targetMethod) { - @SuppressWarnings("rawtypes") - Class[] types = targetMethod.getParameterTypes(); - if (types.length == 0) { - return Optional.absent(); - } - if (types.length == 1) { - return Optional.> of(types[0]); + for (Class clazz : targetMethod.getParameterTypes()) { + if (DataContainer.class.isAssignableFrom(clazz)) { + return Optional.of(clazz); + } } - throw new IllegalArgumentException("Method has 2 or more arguments."); + return Optional.absent(); } public static QName getQName(final Class context) { @@ -362,7 +361,7 @@ public class BindingReflections { * @return Set of {@link YangModuleInfo} available for supplied classloader. */ public static ImmutableSet loadModuleInfos(final ClassLoader loader) { - Builder moduleInfoSet = ImmutableSet. builder(); + Builder moduleInfoSet = ImmutableSet.builder(); ServiceLoader serviceLoader = ServiceLoader.load(YangModelBindingProvider.class, loader); for (YangModelBindingProvider bindingProvider : serviceLoader) { @@ -451,7 +450,7 @@ public class BindingReflections { @SuppressWarnings("rawtypes") Class returnType = method.getReturnType(); if (DataContainer.class.isAssignableFrom(returnType)) { - return Optional.> of(returnType); + return Optional.of(returnType); } else if (List.class.isAssignableFrom(returnType)) { try { return ClassLoaderUtils.withClassLoader(method.getDeclaringClass().getClassLoader(), @@ -459,7 +458,7 @@ public class BindingReflections { Type listResult = ClassLoaderUtils.getFirstGenericParameter(method.getGenericReturnType()); if (listResult instanceof Class && DataContainer.class.isAssignableFrom((Class) listResult)) { - return Optional.> of((Class) listResult); + return Optional.of((Class) listResult); } return Optional.absent(); }); @@ -479,7 +478,7 @@ public class BindingReflections { private static class ClassToQNameLoader extends CacheLoader, Optional> { @Override - public Optional load(final Class key) throws Exception { + public Optional load(@SuppressWarnings("NullableProblems") final Class key) throws Exception { return resolveQNameNoCache(key); } diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvoker.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvoker.java index 466dfaf9b7..69f4e6f4ae 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvoker.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvoker.java @@ -35,6 +35,5 @@ abstract class RpcMethodInvoker { } catch (IllegalAccessException e) { throw new IllegalStateException("Lookup on public method failed.",e); } - } -} +} \ No newline at end of file diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvokerWithInput.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvokerWithInput.java index a77dd35b9e..31d749aa85 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvokerWithInput.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/util/RpcMethodInvokerWithInput.java @@ -17,7 +17,8 @@ import org.opendaylight.yangtools.yang.common.RpcResult; class RpcMethodInvokerWithInput extends RpcMethodInvoker { - private static final MethodType INVOCATION_SIGNATURE = MethodType.methodType(Future.class, RpcService.class,DataObject.class); + private static final MethodType INVOCATION_SIGNATURE = + MethodType.methodType(Future.class, RpcService.class, DataObject.class); private final MethodHandle handle; RpcMethodInvokerWithInput(MethodHandle methodHandle) { diff --git a/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvokerTest.java b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvokerTest.java new file mode 100644 index 0000000000..bffba5f59f --- /dev/null +++ b/binding/yang-binding/src/test/java/org/opendaylight/yangtools/yang/binding/util/AbstractMappedRpcInvokerTest.java @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2016 Cisco Systems, Inc. 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.yangtools.yang.binding.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.Futures; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.Future; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.junit.Test; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.RpcService; +import org.opendaylight.yangtools.yang.common.QName; + +public class AbstractMappedRpcInvokerTest { + + @Test + public void invokeRpcTest() throws Exception { + final Method methodWithoutInput = + TestRpcService.class.getDeclaredMethod("methodWithoutInput", RpcService.class); + final Method methodWithInput = + TestRpcService.class.getDeclaredMethod("methodWithInput", RpcService.class, DataObject.class); + + methodWithInput.setAccessible(true); + methodWithoutInput.setAccessible(true); + + final RpcService rpcService = new TestRpcService(); + + final RpcServiceInvoker testRpcInvoker = + new TestRpcInvokerImpl(ImmutableMap.of( + "tstWithoutInput", methodWithoutInput, + "tstWithInput", methodWithInput)); + + final Field testInvokerMapField = testRpcInvoker.getClass().getSuperclass().getDeclaredField("map"); + testInvokerMapField.setAccessible(true); + final Map testInvokerMap = + (Map) testInvokerMapField.get(testRpcInvoker); + + assertTrue(testInvokerMap.get("tstWithInput") instanceof RpcMethodInvokerWithInput); + assertTrue(testInvokerMap.get("tstWithoutInput") instanceof RpcMethodInvokerWithoutInput); + + final Crate crateWithoutInput = + (Crate) testRpcInvoker.invokeRpc(rpcService, QName.create("tstWithoutInput"), null).get(); + assertEquals(TestRpcService.methodWithoutInput(rpcService).get().getRpcService(), + crateWithoutInput.getRpcService()); + assertFalse(crateWithoutInput.getDataObject().isPresent()); + + final DataObject dataObject = mock(DataObject.class); + final Crate crateWithInput = + (Crate) testRpcInvoker.invokeRpc(rpcService, QName.create("tstWithInput"), dataObject).get(); + assertEquals(TestRpcService.methodWithInput(rpcService, dataObject).get().getRpcService(), + crateWithInput.getRpcService()); + assertTrue(crateWithInput.getDataObject().isPresent()); + assertEquals(dataObject, crateWithInput.getDataObject().get()); + } + + private class TestRpcInvokerImpl extends AbstractMappedRpcInvoker { + + TestRpcInvokerImpl(Map map) { + super(map); + } + + @Override + protected String qnameToKey(QName qname) { + return qname.toString(); + } + } + + static class Crate { + private final RpcService rpcService; + private final ThreadLocal> dataObject; + + Crate(@Nonnull RpcService rpcService, @Nullable DataObject dataObject) { + this.rpcService = rpcService; + this.dataObject = new ThreadLocal>() { + @Override + protected Optional initialValue() { + return dataObject == null ? Optional.empty() : Optional.of(dataObject); + } + }; + } + + RpcService getRpcService() { + return this.rpcService; + } + + Optional getDataObject() { + return this.dataObject.get(); + } + } + + static class TestRpcService implements RpcService { + static Future methodWithoutInput(RpcService testArgument) { + return Futures.immediateFuture(new Crate(testArgument, null)); + } + + static Future methodWithInput(RpcService testArgument, DataObject testArgument2) { + return Futures.immediateFuture(new Crate(testArgument, testArgument2)); + } + } +} + -- 2.36.6