This is an automated email from the ASF dual-hosted git repository.

markap14 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi.git


The following commit(s) were added to refs/heads/master by this push:
     new aa7c5e2  NIFI-7511 In ControllerServiceProxyWrapper extended 
documentation. Minor refactor in StandardControllerServiceInvocationHandler. 
Also removed an unused import from NiFiSystemIT.
aa7c5e2 is described below

commit aa7c5e21782cf4d2f4b10ae63851c53f7c47a80f
Author: Tamas Palfy <[email protected]>
AuthorDate: Fri Jun 5 21:43:49 2020 +0200

    NIFI-7511 In ControllerServiceProxyWrapper extended documentation. Minor 
refactor in StandardControllerServiceInvocationHandler. Also removed an unused 
import from NiFiSystemIT.
    
    This closes #4317.
    
    Signed-off-by: Mark Payne <[email protected]>
---
 .../controller/ControllerServiceProxyWrapper.java  | 117 +++++++++++++++++----
 ...StandardControllerServiceInvocationHandler.java |  78 +++++++-------
 2 files changed, 133 insertions(+), 62 deletions(-)

diff --git 
a/nifi-framework-api/src/main/java/org/apache/nifi/controller/ControllerServiceProxyWrapper.java
 
b/nifi-framework-api/src/main/java/org/apache/nifi/controller/ControllerServiceProxyWrapper.java
index 3a7bd75..b00e218 100644
--- 
a/nifi-framework-api/src/main/java/org/apache/nifi/controller/ControllerServiceProxyWrapper.java
+++ 
b/nifi-framework-api/src/main/java/org/apache/nifi/controller/ControllerServiceProxyWrapper.java
@@ -18,35 +18,110 @@
 package org.apache.nifi.controller;
 
 /**
- * An interface that can be added to a Proxy object for a Controller Service 
in order to get the underlying object that is being proxied.
- * This is done so that any object that is returned by a Controller Service
- * can be unwrapped if it's passed back to the Controller Service. This is 
needed in order to accommodate for the following scenario.
- * Consider a Controller Service that has two methods:
- *
- * <pre><code>
- * public MyObject createObject();
- * public void useObject(MyObject object);
+ * The purpose of this interface is to help handle the following scenario:
+ *
+ * A Controller Service method returns a value which is wrapped in a proxy by 
the framework.
+ * Another method of the Controller Service receives the same proxy as an 
argument. (The Controller Service gets back the object.)
+ *  This method expects the argument to be of the concrete type of the real 
return value.
+ *  Since the proxy only preserves the interface of the real return value but 
not the concrete type, the method fails.
+ *
+ * To fix this, this interface is added to the proxy (along with the original 
interface) and the framework will get the real value via {@link #getWrapped()}
+ *  so the Controller Service will receive the real object.
+ *
+ * E.g.:
+ *
+ * <pre><code>public interface IConnectionProviderService {
+ *     IConnection getConnection();
+ *     void closeConnection(IConnection);
+ * }
+ *
+ * public class ConnectionProviderServiceImpl {
+ *     IConnection getConnection() {
+ *         return new SimpleConnection();
+ *     }
+ *
+ *     void closeConnection(IConnection) {
+ *         if (connection instanceof SimpleConnection) {
+ *             ...
+ *         } else {
+ *             throw new InvalidArgumentException();
+ *         }
+ *     }
+ * }
+ *
+ * public class ConnectionUserProcessor {
+ *     IConnectionProviderService service; #Set to 
ConnectionProviderServiceImpl
+ *
+ *     void onTrigger() {
+ *         IConnection connection = service.getConnection();
+ *
+ *         # 'connection' at this point is a proxy of a 'SimpleConnection' 
object
+ *         # So '(connection instanceof IConnection)' is true, but
+ *         # '(connection instanceof SimpleConnection)' is false
+ *
+ *         ...
+ *
+ *         service.closeConnection(connection); # !! This would have thrown 
InvalidArgumentException
+ *     }
+ * }
  * </code></pre>
  *
- * And further consider that MyObject is an interface with multiple 
implementations.
- * If the {@code useObject} method is implemented using logic such as:
+ * But why wrap the return value in a proxy in the first place? It is needed 
to handle the following scenario:
+ *
+ * A Controller Service method returns an object to a Processor.
+ * A method is called on the returned object int the Processor.
+ * This method tries to load a class that is in the same package as the return 
object.
+ * Since it tries to use the Processor classloader, it fails.
+ *
+ * E.g.:
+ * <pre><code>package root.interface;
+ *
+ * public interface IReportService {
+ *     IReport getReport();
+ * }
+ * public interface IReport {
+ *     void submit();
+ * }
+ *
+ *
+ * package root.service;
+ *
+ * public class ReportServiceImpl {
+ *     IReport getReport() {
+ *         return new Report();
+ *     }
+ * }
+ * public class ReportImpl {
+ *     void submit() {
+ *         Class.forName("roo.service.OtherClass");
+ *         ...
+ *     }
+ * }
+ * public class OtherClass {}
+ *
+ *
+ * package root.processor;
+ *
+ * public class ReportProcessor {
+ *     IReportService service; #Set to ReportServiceImpl
  *
- * <pre><code>
- * public void useObject(final MyObject object) {
- *       if (object instanceof SomeObject) {
- *           // perform some action
- *       }
+ *     void onTrigger() {
+ *         IReport report = service.getReport();
+ *         ...
+ *         report.submit(); # !! This would have thrown ClassNotFoundException
+ *     }
  * }
  * </code></pre>
  *
- * In this case, if the {@code createObject} method does in fact create an 
instance of {@code SomeObject}, the proxied object that is returned will not be 
of type {@code SomeObject}
- * because {@code SomeObject} is a class, not an interface. So the proxy 
implements the {@code MyObject} interface, but it is not an instance of {@code 
SomeObject}.
- * As a result, the instanceof check would return {@code false} but the 
service implementor should reasonably expect it to return {@code true}.
- * In order to accommodate this behavior, this interface can be added to the 
proxy and then the underlying object can be "unwrapped" when being provided to 
the Controller Service.
+ * So in general there is a barrier between the Controller Service and the 
Processor (or another Controller Service) due to the fact that they have
+ * their own classloaders.
+ * When an object crosses the barrier, it needs to be proxied to be able to 
use its original classloader.
+ * Also when it crosses the barrier back again, it needs to be unproxied to 
preserve specific type information.
  *
- * The {@link java.lang.reflect.InvocationHandler InvocationHandler} is then 
able to implement the method in order to unwrap the object.
+ * Note that wrapping may not be necessary if the class is loaded by a 
classloader that is parent to both the Controller Service and the Processor,
+ * as in this case the 'barrier' not really there for instances of that class.
  *
- * @param <T> the type of the wrapped object
+ * @param <T> the type of the wrapped/proxied object
  */
 public interface ControllerServiceProxyWrapper<T> {
     /**
diff --git 
a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceInvocationHandler.java
 
b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceInvocationHandler.java
index ef0cdf1..f6217b8 100644
--- 
a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceInvocationHandler.java
+++ 
b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceInvocationHandler.java
@@ -97,22 +97,12 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
                 + serviceNodeHolder.get().getIdentifier() + " because the 
Controller Service's State is currently " + state);
         }
 
-        final ClassLoader originalClassLoader = 
Thread.currentThread().getContextClassLoader();
+        final ClassLoader callerClassLoader = 
Thread.currentThread().getContextClassLoader();
         try (final NarCloseable narCloseable = 
NarCloseable.withComponentNarLoader(extensionManager, 
originalService.getClass(), originalService.getIdentifier())) {
             // If any objects are proxied, unwrap them so that we provide the 
unproxied object to the Controller Service.
-            final Object[] unwrapped = unwrapProxies(args, 
Thread.currentThread().getContextClassLoader(), method);
+            ClassLoader serviceClassLoader = 
Thread.currentThread().getContextClassLoader();
 
-            // Invoke the method on the underlying implementation
-            final Object returnedFromImpl = method.invoke(originalService, 
unwrapped);
-
-            // If the return object is known to the caller, it can be returned 
directly. Otherwise, proxy the object so that
-            // calls into the proxy are called through the appropriate 
ClassLoader.
-            if (returnedFromImpl == null || 
isInHierarchy(returnedFromImpl.getClass().getClassLoader(), 
originalClassLoader)) {
-                return returnedFromImpl;
-            }
-
-            // Proxy the return object, if necessary, and return the proxy.
-            return proxy(returnedFromImpl, method.getReturnType());
+            return invoke(originalService, method, args, serviceClassLoader, 
callerClassLoader);
         } catch (final InvocationTargetException e) {
             // If the ControllerService throws an Exception, it'll be wrapped 
in an InvocationTargetException. We want
             // to instead re-throw what the ControllerService threw, so we 
pull it out of the InvocationTargetException.
@@ -130,8 +120,8 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
         return isInHierarchy(objectClassLoader, 
classLoaderHierarchy.getParent());
     }
 
-    private Object proxy(final Object toProxy, final Class<?> declaredType) {
-        if (toProxy == null) {
+    private Object proxy(final Object bareObject, final Class<?> declaredType) 
{
+        if (bareObject == null) {
             return null;
         }
 
@@ -139,12 +129,12 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
         // was invoked as being an interface. For example, if a method is 
expected to return a java.lang.String,
         // we do not want to instead return a proxy because the Proxy won't be 
a String.
         if (declaredType == null || !declaredType.isInterface()) {
-            return toProxy;
+            return bareObject;
         }
 
         // If the ClassLoader is null, we have a primitive type, which we 
can't proxy.
-        if (toProxy.getClass().getClassLoader() == null) {
-            return toProxy;
+        if (bareObject.getClass().getClassLoader() == null) {
+            return bareObject;
         }
 
         // The proxy that is to be returned needs to ensure that it implements 
all interfaces that are defined by the
@@ -155,9 +145,9 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
         //
         // final javax.jms.Message myMessage = controllerService.getMessage();
         // final boolean isBytes = myMessage instanceof javax.jms.BytesMessage;
-        final List<Class<?>> interfaces = 
ClassUtils.getAllInterfaces(toProxy.getClass());
+        final List<Class<?>> interfaces = 
ClassUtils.getAllInterfaces(bareObject.getClass());
         if (interfaces == null || interfaces.isEmpty()) {
-            return toProxy;
+            return bareObject;
         }
 
         // Add the ControllerServiceProxyWrapper to the List of interfaces to 
implement. See javadocs for ControllerServiceProxyWrapper
@@ -167,8 +157,8 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
         }
 
         final Class<?>[] interfaceTypes = interfaces.toArray(new Class<?>[0]);
-        final InvocationHandler invocationHandler = new 
ProxiedReturnObjectInvocationHandler(toProxy);
-        return Proxy.newProxyInstance(toProxy.getClass().getClassLoader(), 
interfaceTypes, invocationHandler);
+        final InvocationHandler invocationHandler = new 
ProxiedReturnObjectInvocationHandler(bareObject);
+        return Proxy.newProxyInstance(bareObject.getClass().getClassLoader(), 
interfaceTypes, invocationHandler);
     }
 
     private Object[] unwrapProxies(final Object[] values, final ClassLoader 
expectedClassLoader, final Method method) {
@@ -229,40 +219,46 @@ public class StandardControllerServiceInvocationHandler 
implements ControllerSer
     }
 
     private class ProxiedReturnObjectInvocationHandler implements 
InvocationHandler {
-        private final Object toProxy;
-        private final ClassLoader classLoaderForProxy;
+        private final Object bareObject;
+        private ClassLoader bareObjectClassLoader;
 
-        public ProxiedReturnObjectInvocationHandler(final Object toProxy) {
-            this.toProxy = toProxy;
-            this.classLoaderForProxy = toProxy.getClass().getClassLoader();
+        public ProxiedReturnObjectInvocationHandler(final Object bareObject) {
+            this.bareObject = bareObject;
+            this.bareObjectClassLoader = 
bareObject.getClass().getClassLoader();
         }
 
         @Override
         public Object invoke(final Object proxy, final Method method, final 
Object[] args) throws Throwable {
             if (PROXY_WRAPPER_GET_WRAPPED_METHOD.equals(method)) {
-                return toProxy;
+                return this.bareObject;
             }
 
-            final Object[] unwrapped = unwrapProxies(args, 
classLoaderForProxy, method);
-
-            final ClassLoader originalClassLoader = 
Thread.currentThread().getContextClassLoader();
+            final ClassLoader callerClassLoader = 
Thread.currentThread().getContextClassLoader();
             try {
-                
Thread.currentThread().setContextClassLoader(classLoaderForProxy);
-                final Object returnedFromImpl = method.invoke(toProxy, 
unwrapped);
-
-                // If the return object is known to the caller, it can be 
returned directly. Otherwise, proxy the object so that
-                // calls into the proxy are called through the appropriate 
ClassLoader.
-                if (returnedFromImpl == null || 
isInHierarchy(returnedFromImpl.getClass().getClassLoader(), 
originalClassLoader)) {
-                    return returnedFromImpl;
-                }
+                
Thread.currentThread().setContextClassLoader(this.bareObjectClassLoader);
 
-                return proxy(returnedFromImpl, method.getReturnType());
+                return 
StandardControllerServiceInvocationHandler.this.invoke(this.bareObject, method, 
args, this.bareObjectClassLoader, callerClassLoader);
             } catch (final InvocationTargetException ite) {
                 throw ite.getCause();
             } finally {
-                
Thread.currentThread().setContextClassLoader(originalClassLoader);
+                
Thread.currentThread().setContextClassLoader(callerClassLoader);
             }
         }
+    }
+
+    private Object invoke(Object bareObject, Method method, Object[] args, 
ClassLoader bareObjectClassLoader, ClassLoader callerClassLoader) throws 
IllegalAccessException, InvocationTargetException {
+        // If any objects are proxied, unwrap them so that we provide the 
unproxied object to the Controller Service.
+        final Object[] unwrappedArgs = unwrapProxies(args, 
bareObjectClassLoader, method);
+
+        // Invoke the method on the underlying implementation
+        final Object returnedFromBareObject = method.invoke(bareObject, 
unwrappedArgs);
+
+        // If the return object is known to the caller, it can be returned 
directly. Otherwise, proxy the object so that
+        // calls into the proxy are called through the appropriate ClassLoader.
+        if (returnedFromBareObject == null || 
isInHierarchy(returnedFromBareObject.getClass().getClassLoader(), 
callerClassLoader)) {
+            return returnedFromBareObject;
+        }
 
+        return proxy(returnedFromBareObject, method.getReturnType());
     }
 }

Reply via email to