nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ald...@apache.org
Subject nifi git commit: NIFI-1595 fixed ReflectionUtils to honor bridge methods Refactored and simplified ReflectionUtils while at it Added ReflectionUtilsTest
Date Mon, 07 Mar 2016 20:28:00 GMT
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 <aldrin@apache.org>
With adjustments to formatting and whitespace.

Signed-off-by: Aldrin Piri <aldrin@apache.org>


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 <oleg@suitcase.io>
Authored: Sun Mar 6 14:46:35 2016 -0500
Committer: Aldrin Piri <aldrin@apache.org>
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");
+        }
+    }
+
+}


Mime
View raw message