Add remotable check to avoid perf penality from Holder processing for local calls

git-svn-id: http://svn.us.apache.org/repos/asf/tuscany@763768 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 139 insertions, 169 deletions

diff --git a/branches/sca-java-1.x/modules/core/src/main/java/org/apache/tuscany/sca/core/invocation/JDKInvocationHandler.java b/branches/sca-java-1.x/modules/core/src/main/java/org/apache/tuscany/sca/core/invocation/JDKInvocationHandler.java
index a084a666c2..5bb648ce61 100644
--- a/branches/sca-java-1.x/modules/core/src/main/java/org/apache/tuscany/sca/core/invocation/JDKInvocationHandler.java
+++ b/branches/sca-java-1.x/modules/core/src/main/java/org/apache/tuscany/sca/core/invocation/JDKInvocationHandler.java
@@ -6,15 +6,15 @@
  * 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.    
+ * under the License.
  */
 
 package org.apache.tuscany.sca.core.invocation;
@@ -23,13 +23,13 @@ import java.io.Serializable;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.lang.reflect.Type;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
+import javax.xml.ws.Holder;
+
 import org.apache.tuscany.sca.core.assembly.RuntimeWireImpl;
 import org.apache.tuscany.sca.core.context.CallableReferenceImpl;
 import org.apache.tuscany.sca.core.context.InstanceWrapper;
@@ -60,10 +60,6 @@ import org.osoa.sca.ConversationEndedException;
 import org.osoa.sca.ServiceReference;
 import org.osoa.sca.ServiceRuntimeException;
 
-import java.util.Iterator;
-import javax.xml.ws.Holder;
-
-
 /**
  * @version $Rev$ $Date$
  */
@@ -156,22 +152,21 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
             throw new IllegalArgumentException("No matching operation is found: " + method);
         }
 
-        // Holder pattern. Items stored in a Holder<T> are promoted to T.
-        // After the invoke, the returned data <T> are placed back in Holder<T>.
-        Object [] promotedArgs = promoteHolderArgs( args );
-        
-        Object result = invoke(chain, promotedArgs, wire, source);
-
-        // Returned Holder data <T> are placed back in Holder<T>.
-        Class [] parameters = method.getParameterTypes();
-        if ( parameters != null ) {
-            for ( int i = 0; i < parameters.length; i++ ) {
-               Class parameterType = parameters[ i ];              
-               if ( isHolder( parameterType ) ) {
-                  // Pop results and place in holder (demote).
-                  Holder holder = (Holder) args[ i ]; 
-                  holder.value = result;
-               }            
+        Object result = invoke(chain, args, wire, source);
+
+        Operation operation = chain.getTargetOperation();
+        if (operation != null && operation.getInterface().isRemotable()) {
+            // Returned Holder data <T> are placed back in Holder<T>.
+            Class<?>[] parameters = method.getParameterTypes();
+            if (parameters != null) {
+                for (int i = 0; i < parameters.length; i++) {
+                    Class<?> parameterType = parameters[i];
+                    if (Holder.class == parameterType) {
+                        // Pop results and place in holder (demote).
+                        Holder holder = (Holder)args[i];
+                        holder.value = result;
+                    }
+                }
             }
         }
 
@@ -205,7 +200,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
 
     /**
      * Determines if the given operation matches the given method
-     * 
+     *
      * @return true if the operation matches, false if does not
      */
     // FIXME: Should it be in the InterfaceContractMapper?
@@ -226,7 +221,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
             }
         }
 
-        // For remotable interface, operation is not overloaded. 
+        // For remotable interface, operation is not overloaded.
         if (operation.getInterface().isRemotable()) {
             return true;
         }
@@ -284,7 +279,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
     }
 
     protected Object invoke(InvocationChain chain, Object[] args, RuntimeWire wire, EndpointReference source)
-                         throws Throwable {
+        throws Throwable {
         Message msg = messageFactory.createMessage();
         msg.setFrom(source);
         if (target != null) {
@@ -294,9 +289,15 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
         }
         Invoker headInvoker = chain.getHeadInvoker();
         Operation operation = chain.getTargetOperation();
-        
-        msg.setOperation(operation);       
-        msg.setBody( args );
+
+        msg.setOperation(operation);
+
+        // Holder pattern. Items stored in a Holder<T> are promoted to T.
+        // After the invoke, the returned data <T> are placed back in Holder<T>.
+        if (operation != null && operation.getInterface().isRemotable()) {
+            args = promoteHolderArgs(args);
+        }
+        msg.setBody(args);
 
         Message msgContext = ThreadMessageContext.getMessageContext();
         Object currentConversationID = msgContext.getFrom().getReferenceParameters().getConversationID();
@@ -311,23 +312,23 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
             Object body = resp.getBody();
             if (resp.isFault()) {
                 // mark the conversation as ended if the exception is not a business exception
-                if (currentConversationID != null ){
+                if (currentConversationID != null) {
                     try {
                         boolean businessException = false;
-                        
-                        for (DataType dataType : operation.getFaultTypes()){
-                            if (dataType.getPhysical() == ((Throwable)body).getClass()){
+
+                        for (DataType dataType : operation.getFaultTypes()) {
+                            if (dataType.getPhysical() == ((Throwable)body).getClass()) {
                                 businessException = true;
                                 break;
                             }
                         }
-                        
-                        if (businessException == false){
+
+                        if (businessException == false) {
                             abnormalEndConversation = true;
                         }
-                    } catch (Exception ex){
+                    } catch (Exception ex) {
                         // TODO - sure what the best course of action is here. We have
-                        //        a system exception in the middle of a business exception 
+                        //        a system exception in the middle of a business exception
                     }
                 }
                 throw (Throwable)body;
@@ -356,7 +357,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
         parameters.setCallbackReference(msg.getFrom().getCallbackEndpoint());
 
         // If we are passing out a callback target
-        // register the calling component instance against this 
+        // register the calling component instance against this
         // new conversation id so that stateful callbacks will be
         // able to find it
         Object callbackObject = getCallbackObject();
@@ -383,7 +384,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
                     } else {
                         if (!(callbackObject instanceof Serializable)) {
                             throw new IllegalArgumentException(
-                                          "Callback object for stateful callback is not Serializable");
+                                                               "Callback object for stateful callback is not Serializable");
                         }
                         ScopeContainer scopeContainer = getConversationalScopeContainer(wire);
                         if (scopeContainer != null) {
@@ -413,25 +414,25 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
         if (conversation == null || conversation.getState() == ConversationState.ENDED) {
 
             conversation = conversationManager.startConversation(getConversationID());
-            
-            // if this is a local wire then set up the conversation timeouts here based on the 
+
+            // if this is a local wire then set up the conversation timeouts here based on the
             // parameters from the component
-            if (wire.getTarget().getComponent() != null){
+            if (wire.getTarget().getComponent() != null) {
                 conversation.initializeConversationAttributes(wire.getTarget().getComponent());
             }
-            
+
             // connect the conversation to the CallableReference so it can be retrieve in the future
             if (callableReference != null) {
                 ((CallableReferenceImpl)callableReference).attachConversation(conversation);
             }
         } else if (conversation.isExpired()) {
-            throw new ConversationEndedException("Conversation " +  conversation.getConversationID() + " has expired.");
+            throw new ConversationEndedException("Conversation " + conversation.getConversationID() + " has expired.");
         }
 
         // if this is a local wire then schedule conversation timeouts based on the timeout
         // parameters from the service implementation. If this isn't a local wire then
         // the RuntimeWireInvoker will take care of this
-        if (wire.getTarget().getComponent() != null){
+        if (wire.getTarget().getComponent() != null) {
             conversation.updateLastReferencedTime();
         }
 
@@ -447,14 +448,13 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
      */
     @SuppressWarnings("unchecked")
     private void conversationPostInvoke(Message msg, RuntimeWire wire, boolean abnormalEndConversation)
-                     throws TargetDestructionException {
+        throws TargetDestructionException {
         Operation operation = msg.getOperation();
         ConversationSequence sequence = operation.getConversationSequence();
         // We check that conversation has not already ended as there is only one
-        // conversation manager in the runtime and so, in the case of remote bindings, 
+        // conversation manager in the runtime and so, in the case of remote bindings,
         // the conversation will already have been stopped when we get back to the client
-        if ((sequence == ConversationSequence.CONVERSATION_END || abnormalEndConversation) &&
-            (conversation.getState() != ConversationState.ENDED)) {
+        if ((sequence == ConversationSequence.CONVERSATION_END || abnormalEndConversation) && (conversation.getState() != ConversationState.ENDED)) {
 
             // remove conversation id from scope container
             ScopeContainer scopeContainer = getConversationalScopeContainer(wire);
@@ -486,7 +486,7 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
 
     /**
      * Creates a new conversation id
-     * 
+     *
      * @return the conversation id
      */
     private Object createConversationID() {
@@ -536,58 +536,40 @@ public class JDKInvocationHandler implements InvocationHandler, Serializable {
 
     }
 
-    
     /**
      * Creates a copy of arguments. Holder<T> values are promoted to T.
      * Note. It is essential that arg Holders not be destroyed here.
      * PromotedArgs should not destroy holders. They are used on response return.
      * @param args containing Holders and other objects.
-     * @return Object [] 
+     * @return Object []
      */
-    protected static Object [] promoteHolderArgs( Object [] args ) {
-       if ( args == null )
-           return args;
-       Object [] promotedArgs = new Object[ args.length ];
-       
-       for ( int i = 0; i < args.length; i++ ) {
-          Object argument = args[ i ];
-          if ( argument != null ) {
-              if ( isHolder( argument ) ) {
-                 promotedArgs[ i ] = ((Holder)argument).value;      
-              } else {
-                 promotedArgs[ i ] = args[ i ];
-              }
-              
-          }
-       }
-       return promotedArgs;
-    }
+    protected static Object[] promoteHolderArgs(Object[] args) {
+        if (args == null)
+            return args;
+
+        Object[] promotedArgs = new Object[args.length];
+
+        for (int i = 0; i < args.length; i++) {
+            Object argument = args[i];
+            if (argument != null) {
+                if (isHolder(argument)) {
+                    promotedArgs[i] = ((Holder)argument).value;
+                } else {
+                    promotedArgs[i] = args[i];
+                }
 
-    /**
-     * Given a Class, tells if it is a Holder by comparing to "javax.xml.ws.Holder"
-     * @param testClass
-     * @return boolean whether class is Holder type.
-     */
-    protected static boolean isHolder( Class testClass ) {
-        if ( testClass.getName().startsWith( "javax.xml.ws.Holder" )) {
-            return true;
+            }
         }
-        return false;        
+        return promotedArgs;
     }
 
-     
     /**
      * Given an Object, tells if it is a Holder by comparing to "javax.xml.ws.Holder"
      * @param testClass
      * @return boolean stating whether Object is a Holder type.
      */
-    protected static boolean isHolder( Object object ) {
-        String objectName = object.getClass().getName();
-        if ( object instanceof javax.xml.ws.Holder ) {
-            return true;
-        }
-        return false;        
+    protected static boolean isHolder(Object object) {
+        return Holder.class.isInstance(object);
     }
 
-
 }
diff --git a/branches/sca-java-1.x/modules/implementation-java-runtime/src/main/java/org/apache/tuscany/sca/implementation/java/invocation/JavaImplementationInvoker.java b/branches/sca-java-1.x/modules/implementation-java-runtime/src/main/java/org/apache/tuscany/sca/implementation/java/invocation/JavaImplementationInvoker.java
index fa40560f7e..f86d6c750c 100644
--- a/branches/sca-java-1.x/modules/implementation-java-runtime/src/main/java/org/apache/tuscany/sca/implementation/java/invocation/JavaImplementationInvoker.java
+++ b/branches/sca-java-1.x/modules/implementation-java-runtime/src/main/java/org/apache/tuscany/sca/implementation/java/invocation/JavaImplementationInvoker.java
@@ -6,22 +6,22 @@
  * 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.    
+ * under the License.
  */
 package org.apache.tuscany.sca.implementation.java.invocation;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Vector;
 
 import javax.xml.ws.Holder;
 
@@ -45,7 +45,7 @@ import org.osoa.sca.ServiceRuntimeException;
 /**
  * Responsible for synchronously dispatching an invocation to a Java component
  * implementation instance
- * 
+ *
  * @version $Rev$ $Date$
  */
 public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics {
@@ -65,7 +65,7 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
     }
 
     public JavaImplementationInvoker(Operation operation, RuntimeComponent component) {
-        // used if the method can't be computed statically in advance 
+        // used if the method can't be computed statically in advance
         this.operation = operation;
         this.scopeContainer = ((ScopedRuntimeComponent)component).getScopeContainer();
     }
@@ -98,7 +98,7 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
         }
 
         try {
-            // The following call might create a new conversation, as a result, the msg.getConversationID() might 
+            // The following call might create a new conversation, as a result, the msg.getConversationID() might
             // return a new value
             InstanceWrapper wrapper = scopeContainer.getWrapper(contextId);
 
@@ -106,8 +106,8 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
             // happen in the case that the component has conversational scope but only the
             // callback interface is conversational. Or in the callback case if the service interface
             // is conversational and the callback interface isn't. If we are in this situation we need
-            // to get the contextId of this component and remove it after we have invoked the method on 
-            // it. It is possible that the component instance will not go away when it is removed below 
+            // to get the contextId of this component and remove it after we have invoked the method on
+            // it. It is possible that the component instance will not go away when it is removed below
             // because a callback conversation will still be holding a reference to it
             boolean removeTemporaryConversationalComponentAfterCall = false;
             if (parameters != null && (contextId == null) && (parameters.getConversationID() != null)) {
@@ -129,32 +129,33 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
                     throw new IllegalArgumentException("Callback object does not provide method " + e.getMessage());
                 }
             }
-            
+
             // Holder pattern. Any payload parameters <T> which are should be in holders are placed in Holder<T>.
-            if ( imethod != null) {
-                Class<?> [] params = imethod.getParameterTypes();
-                if ( params != null ) {
-                    for ( int i = 0; i < params.length; i++ ) {
-                        Class<?> parameter = params[ i ];
-                        if ( isHolder( parameter )) {
+            // Only check Holder for remotable interfaces
+            if (imethod != null && op.getInterface().isRemotable()) {
+                Class<?>[] params = imethod.getParameterTypes();
+                if (params != null) {
+                    for (int i = 0; i < params.length; i++) {
+                        Class<?> parameter = params[i];
+                        if (Holder.class == parameter) {
                             // System.out.println( "JavaImplementationInvoker.invoke parameter " + i + " is Holder. Payload isArray=" + (payload != null ? payload.getClass().isArray() : "null" ));
                             if (payload != null && !payload.getClass().isArray()) {
                                 // Promote single param from <T> to Holder<T>
-                                payload = new Holder( payload );                               
+                                payload = new Holder(payload);
                             } else {
                                 // Promote array params from [<T>] to [Holder<T>]
-                                Object [] payloadArray = (Object[]) payload;
-                                for ( int j = 0; j < payloadArray.length; j++ ) {
-                                    Object item = payloadArray[ j ];
-                                    payloadArray[ j ] = new Holder( item );
+                                Object[] payloadArray = (Object[])payload;
+                                for (int j = 0; payloadArray != null && j < payloadArray.length; j++) {
+                                    Object item = payloadArray[j];
+                                    payloadArray[j] = new Holder(item);
                                 }
                             }
                             argumentHolderCount++;
                         }
                     }
-                }                
+                }
             }
-            
+
             Object ret;
             if (payload != null && !payload.getClass().isArray()) {
                 ret = imethod.invoke(instance, payload);
@@ -170,49 +171,49 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
                 scopeContainer.remove(contextId);
                 parameters.setConversationID(null);
             }
-            
+
             if (argumentHolderCount > 0) {
                 // Holder pattern. Any payload Holder<T> types are returned as the message body.
-                List returnArgs = new Vector<Object>();
+                List returnArgs = new ArrayList<Object>();
                 int foundHolders = 0;
-                if ( imethod != null) {
-                    Class<?> [] params = imethod.getParameterTypes();
-                    if ( params != null ) {
-                        for ( int i = 0; i < params.length; i++ ) {
-                            Class<?> parameter = params[ i ];
+                if (imethod != null) {
+                    Class<?>[] params = imethod.getParameterTypes();
+                    if (params != null) {
+                        for (int i = 0; i < params.length; i++) {
+                            Class<?> parameter = params[i];
                             // System.out.println( "JavaImplementationInvoker.invoke return parameter " + i + " type=" + parameter.getClass().getName() );
-                            if ( isHolder( parameter )) {
+                            if (Holder.class == parameter) {
                                 if (payload != null && !payload.getClass().isArray()) {
                                     // Demote params from Holder<T> to <T>.
-                                    Holder<Object> holder = (Holder<Object>) payload;
-                                    returnArgs.add( holder.value );
+                                    Holder<Object> holder = (Holder<Object>)payload;
+                                    returnArgs.add(holder.value);
                                     foundHolders++;
                                 } else {
                                     // Demote array params from Holder<T> to <T>.
-                                    Object [] payloadArray = (Object[]) payload;
-                                    for ( int j = 0; j < payloadArray.length; j++ ) {
-                                        Holder<Object> item = (Holder<Object>) payloadArray[ j ];
-                                        payloadArray[ j ] = item.value;
-                                        returnArgs.add( payloadArray[ j ] );
+                                    Object[] payloadArray = (Object[])payload;
+                                    for (int j = 0; j < payloadArray.length; j++) {
+                                        Holder<Object> item = (Holder<Object>)payloadArray[j];
+                                        payloadArray[j] = item.value;
+                                        returnArgs.add(payloadArray[j]);
                                     }
                                     foundHolders++;
                                 }
                             }
                         }
-                    }                
+                    }
                 }
                 // Although payload items are returned in a list, currently only support 1 return type.
-                if ( returnArgs.size() == 1 ) {
-                    Object value = returnArgs.get( 0 );
-                    if (( value != null ) && ( value.getClass().isArray() )) {
-                       Object [] values = (Object []) value;
-                       if (( values != null ) && ( values.length > 0 )) {
-                          msg.setBody( values[ 0 ] );
-                       }
-                    } else 
-                        msg.setBody(value);                
-                } else 
-                   msg.setBody(returnArgs.toArray());                
+                if (returnArgs.size() == 1) {
+                    Object value = returnArgs.get(0);
+                    if ((value != null) && (value.getClass().isArray())) {
+                        Object[] values = (Object[])value;
+                        if ((values != null) && (values.length > 0)) {
+                            msg.setBody(values[0]);
+                        }
+                    } else
+                        msg.setBody(value);
+                } else
+                    msg.setBody(returnArgs.toArray());
             } else {
                 msg.setBody(ret);
             }
@@ -227,27 +228,26 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
                 }
             }
 
-            
-            if (sequence != ConversationSequence.CONVERSATION_NONE ){
+            if (sequence != ConversationSequence.CONVERSATION_NONE) {
                 try {
-//                    // If the exception is not a business exception then end the conversation
-//                    boolean businessException = false;
-//                    
-//                    for (DataType dataType : operation.getFaultTypes()){
-//                        if ((dataType.getPhysical() == e.getCause().getClass()) &&
-//                            (contextId != null) ){
-//                            businessException = true;
-//                            break;
-//                        }
-//                    }
-                    
+                    //                    // If the exception is not a business exception then end the conversation
+                    //                    boolean businessException = false;
+                    //
+                    //                    for (DataType dataType : operation.getFaultTypes()){
+                    //                        if ((dataType.getPhysical() == e.getCause().getClass()) &&
+                    //                            (contextId != null) ){
+                    //                            businessException = true;
+                    //                            break;
+                    //                        }
+                    //                    }
+
                     if (!isChecked && contextId != null) {
                         scopeContainer.remove(contextId);
                         parameters.setConversationID(null);
                     }
-                } catch (Exception ex){
+                } catch (Exception ex) {
                     // TODO - sure what the best course of action is here. We have
-                    //        a system exception in the middle of a business exception 
+                    //        a system exception in the middle of a business exception
                 }
             }
             if (!isChecked) {
@@ -259,10 +259,10 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
                 } else {
                     throw new ServiceRuntimeException(cause.getMessage(), cause);
                 }
-            }            
-                
+            }
+
         } catch (Exception e) {
-            msg.setFaultBody(e);           
+            msg.setFaultBody(e);
         }
         return msg;
     }
@@ -270,16 +270,4 @@ public class JavaImplementationInvoker implements Invoker, DataExchangeSemantics
     public boolean allowsPassByReference() {
         return allowsPBR;
     }
-
-    /**
-     * Given a Class, tells if it is a Holder by comparing to "javax.xml.ws.Holder"
-     * @param testClass
-     * @return
-     */
-    public static boolean isHolder( Class testClass ) {
-        if ( testClass.getName().equals( "javax.xml.ws.Holder" )) {
-            return true;
-        }
-        return false;        
-    }
 }