From: Martin Ciglan Date: Fri, 9 Dec 2016 15:15:47 +0000 (+0100) Subject: Bug 6163: fixed number of argument when resolving rpc input X-Git-Tag: release/boron-sr2~1 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=mdsal.git;a=commitdiff_plain;h=1b56fa17938e8afc2a306ce7b61ce15515802e13 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: Martin Ciglan --- 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 a43bcb0629..03afcbb7f1 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 @@ -14,6 +14,7 @@ import org.opendaylight.mdsal.dom.api.DOMRpcException; import org.opendaylight.mdsal.dom.api.DOMRpcResult; import org.opendaylight.mdsal.dom.api.DOMRpcService; 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; @@ -192,7 +193,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 a11c5a4bc0..10835d7c48 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 @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet.Builder; import com.google.common.collect.Sets; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Type; import java.net.URI; @@ -132,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. @@ -141,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; } /** @@ -155,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(); } @@ -173,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) { @@ -274,14 +272,10 @@ public class BindingReflections { checkArgument(cls != null); String packageName = getModelRootPackageName(cls.getPackage()); final String potentialClassName = getModuleInfoClassName(packageName); - return ClassLoaderUtils.withClassLoader(cls.getClassLoader(), new Callable() { - @Override - public YangModuleInfo call() throws ClassNotFoundException, IllegalAccessException, - IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException { - Class moduleInfoClass = Thread.currentThread().getContextClassLoader().loadClass(potentialClassName); - return (YangModuleInfo) moduleInfoClass.getMethod("getInstance").invoke(null); - } - }); + return ClassLoaderUtils.withClassLoader(cls.getClassLoader(), (Callable) () -> { + Class moduleInfoClass = Thread.currentThread().getContextClassLoader().loadClass(potentialClassName); + return (YangModuleInfo) moduleInfoClass.getMethod("getInstance").invoke(null); + }); } public static String getModuleInfoClassName(final String packageName) { @@ -300,7 +294,7 @@ public class BindingReflections { if (DataContainer.class.isAssignableFrom(cls) || Augmentation.class.isAssignableFrom(cls)) { return true; } - return (cls.getName().startsWith(BindingMapping.PACKAGE_PREFIX)); + return cls.getName().startsWith(BindingMapping.PACKAGE_PREFIX); } /** @@ -367,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) { @@ -448,7 +442,7 @@ public class BindingReflections { @SuppressWarnings("unchecked") private static Optional> getYangModeledReturnType(final Method method) { - if (method.getName().equals("getClass") || !method.getName().startsWith("get") + if ("getClass".equals(method.getName()) || !method.getName().startsWith("get") || method.getParameterTypes().length > 0) { return Optional.absent(); } @@ -456,23 +450,17 @@ 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(), - new Callable>>() { - @SuppressWarnings("rawtypes") - @Override - public Optional> call() { - Type listResult = ClassLoaderUtils.getFirstGenericParameter(method - .getGenericReturnType()); - if (listResult instanceof Class - && DataContainer.class.isAssignableFrom((Class) listResult)) { - return Optional.> of((Class) listResult); - } - return Optional.absent(); + (Callable>>) () -> { + Type listResult = ClassLoaderUtils.getFirstGenericParameter(method.getGenericReturnType()); + if (listResult instanceof Class + && DataContainer.class.isAssignableFrom((Class) listResult)) { + return Optional.of((Class) listResult); } - + return Optional.absent(); }); } catch (Exception e) { /* @@ -490,100 +478,96 @@ 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); } - } - /** - * - * Tries to resolve QName for supplied class. - * - * Looks up for static field with name from constant - * {@link BindingMapping#QNAME_STATIC_FIELD_NAME} and returns value if - * present. - * - * If field is not present uses {@link #computeQName(Class)} to compute - * QName for missing types. - * - * @param key - * @return - */ - private static Optional resolveQNameNoCache(final Class key) { - try { - Field field = key.getField(BindingMapping.QNAME_STATIC_FIELD_NAME); - Object obj = field.get(null); - if (obj instanceof QName) { - return Optional.of((QName) obj); - } + /** + * + * Tries to resolve QName for supplied class. + * + * Looks up for static field with name from constant {@link BindingMapping#QNAME_STATIC_FIELD_NAME} and returns + * value if present. + * + * If field is not present uses {@link #computeQName(Class)} to compute QName for missing types. + * + * @param key + * @return + */ + private static Optional resolveQNameNoCache(final Class key) { + try { + Field field = key.getField(BindingMapping.QNAME_STATIC_FIELD_NAME); + Object obj = field.get(null); + if (obj instanceof QName) { + return Optional.of((QName) obj); + } - } catch (NoSuchFieldException e) { - return Optional.of(computeQName(key)); - - } catch (SecurityException | IllegalArgumentException | IllegalAccessException e) { - /* - * - * It is safe to log this this exception on debug, since this method - * should not fail. Only failures are possible if the runtime / - * backing. - */ - LOG.debug("Unexpected exception during extracting QName for {}", key, e); - } - return Optional.absent(); - } + } catch (NoSuchFieldException e) { + return Optional.of(computeQName(key)); - /** - * Computes QName for supplied class - * - * Namespace and revision are same as {@link YangModuleInfo} associated with - * supplied class. - *

- * If class is - *

    - *
  • rpc input: local name is "input". - *
  • rpc output: local name is "output". - *
  • augmentation: local name is "module name". - *
- * - * There is also fallback, if it is not possible to compute QName using - * following algorithm returns module QName. - * - * FIXME: Extend this algorithm to also provide QName for YANG modeled - * simple types. - * - * @throws IllegalStateException - * If YangModuleInfo could not be resolved - * @throws IllegalArgumentException - * If supplied class was not derived from YANG model. - * - */ - @SuppressWarnings({ "rawtypes", "unchecked" }) - private static QName computeQName(final Class key) { - if (isBindingClass(key)) { - YangModuleInfo moduleInfo; - try { - moduleInfo = getModuleInfo(key); - } catch (Exception e) { - throw new IllegalStateException("Unable to get QName for " + key + ". YangModuleInfo was not found.", e); + } catch (SecurityException | IllegalArgumentException | IllegalAccessException e) { + /* + * + * It is safe to log this this exception on debug, since this method + * should not fail. Only failures are possible if the runtime / + * backing. + */ + LOG.debug("Unexpected exception during extracting QName for {}", key, e); } - final QName module = getModuleQName(moduleInfo).intern(); - if (Augmentation.class.isAssignableFrom(key)) { - return module; - } else if (isRpcType(key)) { - final String className = key.getSimpleName(); - if (className.endsWith(BindingMapping.RPC_OUTPUT_SUFFIX)) { - return QName.create(module, "output").intern(); - } else { - return QName.create(module, "input").intern(); + return Optional.absent(); + } + + /** + * Computes QName for supplied class + * + * Namespace and revision are same as {@link YangModuleInfo} associated with supplied class. + *

+ * If class is + *

    + *
  • rpc input: local name is "input". + *
  • rpc output: local name is "output". + *
  • augmentation: local name is "module name". + *
+ * + * There is also fallback, if it is not possible to compute QName using following algorithm returns module + * QName. + * + * FIXME: Extend this algorithm to also provide QName for YANG modeled simple types. + * + * @throws IllegalStateException If YangModuleInfo could not be resolved + * @throws IllegalArgumentException If supplied class was not derived from YANG model. + * + */ + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static QName computeQName(final Class key) { + if (isBindingClass(key)) { + YangModuleInfo moduleInfo; + try { + moduleInfo = getModuleInfo(key); + } catch (Exception e) { + throw new IllegalStateException("Unable to get QName for " + key + + ". YangModuleInfo was not found.", e); + } + final QName module = getModuleQName(moduleInfo).intern(); + if (Augmentation.class.isAssignableFrom(key)) { + return module; + } else if (isRpcType(key)) { + final String className = key.getSimpleName(); + if (className.endsWith(BindingMapping.RPC_OUTPUT_SUFFIX)) { + return QName.create(module, "output").intern(); + } else { + return QName.create(module, "input").intern(); + } } + /* + * Fallback for Binding types which do not have QNAME field + */ + return module; + } else { + throw new IllegalArgumentException("Supplied class " + key + "is not derived from YANG."); } - /* - * Fallback for Binding types which do not have QNAME field - */ - return module; - } else { - throw new IllegalArgumentException("Supplied class " + key + "is not derived from YANG."); } + } /** 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)); + } + } +} +