Repository: nifi Updated Branches: refs/heads/master a7e6820fd -> 4ce7b679e
NIFI-1595 fixed ReflectionUtils to honor bridge methods Refactored and simplified ReflectionUtils while at it Added ReflectionUtilsTest This closes #260. Signed-off-by: Aldrin Piri <[email protected]> With adjustments to formatting and whitespace. Signed-off-by: Aldrin Piri <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/4ce7b679 Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/4ce7b679 Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/4ce7b679 Branch: refs/heads/master Commit: 4ce7b679e17b14ae88991bf9132a144e068ac0f0 Parents: a7e6820 Author: Oleg Zhurakousky <[email protected]> Authored: Sun Mar 6 14:46:35 2016 -0500 Committer: Aldrin Piri <[email protected]> Committed: Mon Mar 7 15:27:35 2016 -0500 ---------------------------------------------------------------------- .../org/apache/nifi/util/ReflectionUtils.java | 280 ++++++++----------- .../apache/nifi/util/ReflectionUtilsTest.java | 151 ++++++++++ 2 files changed, 272 insertions(+), 159 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/4ce7b679/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ReflectionUtils.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ReflectionUtils.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ReflectionUtils.java index 4588b9e..9f0380b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ReflectionUtils.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ReflectionUtils.java @@ -19,12 +19,12 @@ package org.apache.nifi.util; import java.lang.annotation.Annotation; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.List; +import java.util.Arrays; import org.apache.nifi.logging.ProcessorLog; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.core.annotation.AnnotationUtils; public class ReflectionUtils { @@ -60,69 +60,14 @@ public class ReflectionUtils { * @throws IllegalArgumentException ex * @throws IllegalAccessException ex */ - public static void invokeMethodsWithAnnotations( - final Class<? extends Annotation> preferredAnnotation, final Class<? extends Annotation> alternateAnnotation, final Object instance, final Object... args) - throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { - final List<Class<? extends Annotation>> annotationClasses = new ArrayList<>(alternateAnnotation == null ? 1 : 2); - annotationClasses.add(preferredAnnotation); - if (alternateAnnotation != null) { - annotationClasses.add(alternateAnnotation); - } - - boolean annotationFound = false; - for (final Class<? extends Annotation> annotationClass : annotationClasses) { - if (annotationFound) { - break; - } - - try { - for (final Method method : instance.getClass().getMethods()) { - if (method.isAnnotationPresent(annotationClass)) { - annotationFound = true; - final boolean isAccessible = method.isAccessible(); - method.setAccessible(true); - - try { - final Class<?>[] argumentTypes = method.getParameterTypes(); - if (argumentTypes.length > args.length) { - throw new IllegalArgumentException(String.format("Unable to invoke method %1$s on %2$s because method expects %3$s parameters but only %4$s were given", - method.getName(), instance, argumentTypes.length, args.length)); - } - - for (int i = 0; i < argumentTypes.length; i++) { - final Class<?> argType = argumentTypes[i]; - if (!argType.isAssignableFrom(args[i].getClass())) { - throw new IllegalArgumentException(String.format( - "Unable to invoke method %1$s on %2$s because method parameter %3$s is expected to be of type %4$s but argument passed was of type %5$s", - method.getName(), instance, i, argType, args[i].getClass())); - } - } - - if (argumentTypes.length == args.length) { - method.invoke(instance, args); - } else { - final Object[] argsToPass = new Object[argumentTypes.length]; - for (int i = 0; i < argsToPass.length; i++) { - argsToPass[i] = args[i]; - } - - method.invoke(instance, argsToPass); - } - } finally { - if (!isAccessible) { - method.setAccessible(false); - } - } - } - } - } catch (final InvocationTargetException ite) { - if (ite.getCause() instanceof RuntimeException) { - throw (RuntimeException) ite.getCause(); - } else { - throw ite; - } - } - } + @SuppressWarnings("unchecked") + public static void invokeMethodsWithAnnotations(final Class<? extends Annotation> preferredAnnotation, + final Class<? extends Annotation> alternateAnnotation, final Object instance, final Object... args) + throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { + + Class<? extends Annotation>[] annotationArray = (Class<? extends Annotation>[]) (alternateAnnotation != null + ? new Class<?>[] { preferredAnnotation, alternateAnnotation } : new Class<?>[] { preferredAnnotation }); + invokeMethodsWithAnnotations(false, null, instance, annotationArray, args); } /** @@ -152,7 +97,8 @@ public class ReflectionUtils { * @return <code>true</code> if all appropriate methods were invoked and returned without throwing an Exception, <code>false</code> if one of the methods threw an Exception or could not be * invoked; if <code>false</code> is returned, an error will have been logged. */ - public static boolean quietlyInvokeMethodsWithAnnotation(final Class<? extends Annotation> annotation, final Object instance, final ProcessorLog logger, final Object... args) { + public static boolean quietlyInvokeMethodsWithAnnotation(final Class<? extends Annotation> annotation, + final Object instance, final ProcessorLog logger, final Object... args) { return quietlyInvokeMethodsWithAnnotations(annotation, null, instance, logger, args); } @@ -168,110 +114,126 @@ public class ReflectionUtils { * @return <code>true</code> if all appropriate methods were invoked and returned without throwing an Exception, <code>false</code> if one of the methods threw an Exception or could not be * invoked; if <code>false</code> is returned, an error will have been logged. */ - public static boolean quietlyInvokeMethodsWithAnnotations( - final Class<? extends Annotation> preferredAnnotation, final Class<? extends Annotation> alternateAnnotation, final Object instance, final Object... args) { + public static boolean quietlyInvokeMethodsWithAnnotations(final Class<? extends Annotation> preferredAnnotation, + final Class<? extends Annotation> alternateAnnotation, final Object instance, final Object... args) { return quietlyInvokeMethodsWithAnnotations(preferredAnnotation, alternateAnnotation, instance, null, args); } - /** - * Invokes all methods on the given instance that have been annotated with the given preferredAnnotation and if no such method exists will invoke all methods on the given instance that have been - * annotated with the given alternateAnnotation, if any exists. If the signature of the method that is defined in <code>instance</code> uses 1 or more parameters, those parameters must be - * specified by the <code>args</code> parameter. However, if more arguments are supplied by the <code>args</code> parameter than needed, the extra arguments will be ignored. - * - * @param preferredAnnotation preferred - * @param alternateAnnotation alternate - * @param instance instance - * @param logger the ProcessorLog to use for logging any errors. If null, will use own logger, but that will not generate bulletins or easily tie to the Processor's log messages. - * @param args args - * @return <code>true</code> if all appropriate methods were invoked and returned without throwing an Exception, <code>false</code> if one of the methods threw an Exception or could not be - * invoked; if <code>false</code> is returned, an error will have been logged. - */ - public static boolean quietlyInvokeMethodsWithAnnotations( - final Class<? extends Annotation> preferredAnnotation, final Class<? extends Annotation> alternateAnnotation, final Object instance, final ProcessorLog logger, final Object... args) { - final List<Class<? extends Annotation>> annotationClasses = new ArrayList<>(alternateAnnotation == null ? 1 : 2); - annotationClasses.add(preferredAnnotation); - if (alternateAnnotation != null) { - annotationClasses.add(alternateAnnotation); - } - - boolean annotationFound = false; - for (final Class<? extends Annotation> annotationClass : annotationClasses) { - if (annotationFound) { - break; - } - - for (final Method method : instance.getClass().getMethods()) { - if (method.isAnnotationPresent(annotationClass)) { - annotationFound = true; - - final boolean isAccessible = method.isAccessible(); - method.setAccessible(true); + private static boolean invokeMethodsWithAnnotations(boolean quietly, ProcessorLog logger, Object instance, + Class<? extends Annotation>[] annotations, Object... args) + throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { + return invokeMethodsWithAnnotations(quietly, logger, instance, instance.getClass(), annotations, args); + } + private static boolean invokeMethodsWithAnnotations(boolean quietly, ProcessorLog logger, Object instance, + Class<?> clazz, Class<? extends Annotation>[] annotations, Object... args) + throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { + boolean isSuccess = true; + for (Method method : clazz.getMethods()) { + if (isAnyAnnotationPresent(method, annotations)) { + Object[] modifiedArgs = buildUpdatedArgumentsList(quietly, method, annotations, logger, args); + if (modifiedArgs != null) { try { - final Class<?>[] argumentTypes = method.getParameterTypes(); - if (argumentTypes.length > args.length) { - if (logger == null) { - LOG.error("Unable to invoke method {} on {} because method expects {} parameters but only {} were given", - new Object[]{method.getName(), instance, argumentTypes.length, args.length}); - } else { - logger.error("Unable to invoke method {} on {} because method expects {} parameters but only {} were given", - new Object[]{method.getName(), instance, argumentTypes.length, args.length}); - } - - return false; + method.invoke(instance, modifiedArgs); + } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { + isSuccess = false; + if (quietly) { + logErrorMessage("Failed while invoking annotated method '" + method + "' with arguments '" + + Arrays.asList(modifiedArgs) + "'.", logger, e); + } else { + throw e; } + } + } + } + } + return isSuccess; + } - for (int i = 0; i < argumentTypes.length; i++) { - final Class<?> argType = argumentTypes[i]; - if (!argType.isAssignableFrom(args[i].getClass())) { - if (logger == null) { - LOG.error("Unable to invoke method {} on {} because method parameter {} is expected to be of type {} but argument passed was of type {}", - new Object[]{method.getName(), instance, i, argType, args[i].getClass()}); - } else { - logger.error("Unable to invoke method {} on {} because method parameter {} is expected to be of type {} but argument passed was of type {}", - new Object[]{method.getName(), instance, i, argType, args[i].getClass()}); - } - - return false; - } - } + private static boolean isAnyAnnotationPresent(Method method, Class<? extends Annotation>[] annotations) { + for (Class<? extends Annotation> annotation : annotations) { + if (AnnotationUtils.findAnnotation(method, annotation) != null) { + return true; + } + } + return false; + } - try { - if (argumentTypes.length == args.length) { - method.invoke(instance, args); - } else { - final Object[] argsToPass = new Object[argumentTypes.length]; - for (int i = 0; i < argsToPass.length; i++) { - argsToPass[i] = args[i]; - } + private static Object[] buildUpdatedArgumentsList(boolean quietly, Method method, Class<?>[] annotations, ProcessorLog processLogger, Object... args) { + boolean parametersCompatible = true; + int argsCount = 0; + + Class<?>[] paramTypes = method.getParameterTypes(); + for (int i = 0; parametersCompatible && i < paramTypes.length && i < args.length; i++) { + if (paramTypes[i].isAssignableFrom(args[i].getClass())) { + argsCount++; + } else { + logErrorMessage("Can not invoke method '" + method + "' with provided arguments since argument " + i + " of type '" + paramTypes[i] + + "' is not assignable from provided value of type '" + args[i].getClass() + "'.", processLogger, null); + if (quietly){ + parametersCompatible = false; + } else { + argsCount++; + } + } + } - method.invoke(instance, argsToPass); - } - } catch (final InvocationTargetException ite) { - if (logger == null) { - LOG.error("Unable to invoke method {} on {} due to {}", new Object[]{method.getName(), instance, ite.getCause()}); - LOG.error("", ite.getCause()); - } else { - logger.error("Unable to invoke method {} on {} due to {}", new Object[]{method.getName(), instance, ite.getCause()}); - } - } catch (final IllegalAccessException | IllegalArgumentException t) { - if (logger == null) { - LOG.error("Unable to invoke method {} on {} due to {}", new Object[]{method.getName(), instance, t}); - LOG.error("", t); - } else { - logger.error("Unable to invoke method {} on {} due to {}", new Object[]{method.getName(), instance, t}); - } + Object[] updatedArguments = null; + if (parametersCompatible) { + updatedArguments = Arrays.copyOf(args, argsCount); + } + return updatedArguments; + } - return false; - } - } finally { - if (!isAccessible) { - method.setAccessible(false); - } - } - } + private static void logErrorMessage(String message, ProcessorLog processLogger, Exception e) { + if (processLogger != null) { + if (e != null) { + processLogger.error(message, e); + } else { + processLogger.error(message); + } + } else { + if (e != null) { + LOG.error(message, e); + } else { + LOG.error(message); } } - return true; + } + + /** + * Invokes all methods on the given instance that have been annotated with + * the given preferredAnnotation and if no such method exists will invoke + * all methods on the given instance that have been annotated with the given + * alternateAnnotation, if any exists. If the signature of the method that + * is defined in <code>instance</code> uses 1 or more parameters, those + * parameters must be specified by the <code>args</code> parameter. However, + * if more arguments are supplied by the <code>args</code> parameter than + * needed, the extra arguments will be ignored. + * + * @param preferredAnnotation preferred + * @param alternateAnnotation alternate + * @param instance instance + * @param logger the ProcessorLog to use for logging any errors. If null, will + * use own logger, but that will not generate bulletins or easily + * tie to the Processor's log messages. + * @param args args + * @return <code>true</code> if all appropriate methods were invoked and + * returned without throwing an Exception, <code>false</code> if one + * of the methods threw an Exception or could not be invoked; if + * <code>false</code> is returned, an error will have been logged. + */ + @SuppressWarnings("unchecked") + public static boolean quietlyInvokeMethodsWithAnnotations(final Class<? extends Annotation> preferredAnnotation, + final Class<? extends Annotation> alternateAnnotation, final Object instance, final ProcessorLog logger, + final Object... args) { + Class<? extends Annotation>[] annotationArray = (Class<? extends Annotation>[]) (alternateAnnotation != null + ? new Class<?>[] { preferredAnnotation, alternateAnnotation } : new Class<?>[] { preferredAnnotation }); + try { + return invokeMethodsWithAnnotations(true, logger, instance, annotationArray, args); + } catch (Exception e) { + LOG.error("Failed while attemptiing to invoke methods with '" + Arrays.asList(annotationArray) + "' annotations", e); + return false; + } } } http://git-wip-us.apache.org/repos/asf/nifi/blob/4ce7b679/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/util/ReflectionUtilsTest.java ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/util/ReflectionUtilsTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/util/ReflectionUtilsTest.java new file mode 100644 index 0000000..c3b35a9 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/util/ReflectionUtilsTest.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.util; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; + +import org.apache.nifi.annotation.lifecycle.OnStopped; +import org.apache.nifi.logging.ProcessorLog; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +public class ReflectionUtilsTest { + + private List<String> invocations = new ArrayList<>(); + + @Before + public void reset() { + this.invocations.clear(); + } + + /* + * Some JDKs will generate Bridge method that will be missing annotation + * public void org.apache.nifi.util.ReflectionUtilsTest$B.setFoo(java.lang.Object) + * and will not be invoked. This validates that ReflectionUtils handles it. + */ + @Test + public void invokeWithBridgePresent() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new B(), 2); + assertEquals(2, this.invocations.size()); + assertEquals("B", this.invocations.get(0)); + assertEquals("B", this.invocations.get(1)); + } + + @Test + public void ensureParentMethodIsCalled() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new C(), 4); + assertEquals(1, this.invocations.size()); + assertEquals("A", this.invocations.get(0)); + } + + @Test + public void ensureOnlyOverridenMethodIsCalled() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new D(), "String"); + assertEquals(1, this.invocations.size()); + assertEquals("D", this.invocations.get(0)); + } + + @Test(expected = IllegalArgumentException.class) + public void validateFailureWithWrongArgumentType() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new B(), "foo"); + } + + @Test + public void ensureSuccessWhenArgumentCountDoesntMatch() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new B(), 3, "hjk"); + assertEquals(2, this.invocations.size()); + assertEquals("B", this.invocations.get(0)); + assertEquals("B", this.invocations.get(1)); + } + + @Test + public void ensureSuccessWithMultipleArgumentsOfPropperType() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new E(), 3, "hjk", "hjk".getBytes()); + assertEquals(1, this.invocations.size()); + assertEquals("E", this.invocations.get(0)); + } + + @Test(expected = IllegalArgumentException.class) + public void validateFailureIfOneOfArgumentsWrongType() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new E(), 3, "hjk", "hjk"); + } + + @Test + public void validateNoFailureIfQuiatelyIfOneOfArgumentsWrongTypeAndProcessLog() throws Exception { + ProcessorLog pl = mock(ProcessorLog.class); + ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnStopped.class, new E(), pl, 3, "hjk", "hjk"); + verify(pl, Mockito.atMost(1)).error(Mockito.anyString()); + } + + @Test(expected = InvocationTargetException.class) + public void validateInvocationFailure() throws Exception { + ReflectionUtils.invokeMethodsWithAnnotation(OnStopped.class, new F(), 3); + } + + @Test + public void validateQuiteSuccessWithInvocationFailure() throws Exception { + ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnStopped.class, new F(), 3); + } + + public abstract class A<T> { + @OnStopped + public void setFoo(T a) { + invocations.add("A"); + } + } + + public class B extends A<Integer> { + @Override + @OnStopped + public void setFoo(Integer a) { + invocations.add("B"); + } + } + + public class C extends A<String> { + + } + + public class D extends C { + @Override + public void setFoo(String a) { + invocations.add("D"); + } + } + + public class E { + @OnStopped + public void foo(Integer a, String b, byte[] c) { + invocations.add("E"); + } + } + + public class F { + @OnStopped + public void foo(Integer a) { + throw new RuntimeException("Intentional"); + } + } + +}
