TUSCANY-2586: Fix pass-by-value copies as described in the JIRA with the SCA binding doing the necessary copies. Also update the copy code so that the new copy is in the correct class loader so that invocations across Nodes in the same JVM work

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

4 files changed, 75 insertions, 7 deletions

diff --git a/sca-java-2.x/trunk/modules/binding-sca-runtime/META-INF/MANIFEST.MF b/sca-java-2.x/trunk/modules/binding-sca-runtime/META-INF/MANIFEST.MF
index 6fa81c6834..90adf537f3 100644
--- a/sca-java-2.x/trunk/modules/binding-sca-runtime/META-INF/MANIFEST.MF
+++ b/sca-java-2.x/trunk/modules/binding-sca-runtime/META-INF/MANIFEST.MF
@@ -16,6 +16,7 @@ Import-Package: javax.xml.namespace,
  org.apache.tuscany.sca.contribution.processor;version="2.0.0",

  org.apache.tuscany.sca.contribution.resolver;version="2.0.0";resolution:=optional,

  org.apache.tuscany.sca.core;version="2.0.0",

+ org.apache.tuscany.sca.databinding;version="2.0.0",

  org.apache.tuscany.sca.definitions;version="2.0.0",

  org.apache.tuscany.sca.interfacedef;version="2.0.0",

  org.apache.tuscany.sca.invocation;version="2.0.0",

diff --git a/sca-java-2.x/trunk/modules/binding-sca-runtime/pom.xml b/sca-java-2.x/trunk/modules/binding-sca-runtime/pom.xml
index 40ca972851..3d260a30e6 100644
--- a/sca-java-2.x/trunk/modules/binding-sca-runtime/pom.xml
+++ b/sca-java-2.x/trunk/modules/binding-sca-runtime/pom.xml
@@ -58,7 +58,7 @@
 
          <dependency>
             <groupId>org.apache.tuscany.sca</groupId>
-            <artifactId>tuscany-core-spi</artifactId>
+            <artifactId>tuscany-databinding</artifactId>
             <version>2.0-SNAPSHOT</version>
         </dependency>       
         
diff --git a/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/RuntimeSCAReferenceBindingProvider.java b/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/RuntimeSCAReferenceBindingProvider.java
index 22b1e5bd99..ec4e893b5f 100644
--- a/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/RuntimeSCAReferenceBindingProvider.java
+++ b/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/RuntimeSCAReferenceBindingProvider.java
@@ -27,6 +27,8 @@ import org.apache.tuscany.sca.assembly.SCABinding;
 import org.apache.tuscany.sca.assembly.SCABindingFactory;
 import org.apache.tuscany.sca.core.ExtensionPointRegistry;
 import org.apache.tuscany.sca.core.FactoryExtensionPoint;
+import org.apache.tuscany.sca.core.UtilityExtensionPoint;
+import org.apache.tuscany.sca.databinding.Mediator;
 import org.apache.tuscany.sca.interfacedef.InterfaceContract;
 import org.apache.tuscany.sca.interfacedef.Operation;
 import org.apache.tuscany.sca.invocation.InvocationChain;
@@ -63,6 +65,7 @@ public class RuntimeSCAReferenceBindingProvider implements ReferenceBindingProvi
     private BindingProviderFactory<DistributedSCABinding> distributedProviderFactory = null;
     private ReferenceBindingProvider distributedProvider = null;
     private SCABindingFactory scaBindingFactory;
+    private Mediator mediator;
 
     public RuntimeSCAReferenceBindingProvider(ExtensionPointRegistry extensionPoints,
                                               RuntimeEndpointReference endpointReference) {
@@ -82,6 +85,7 @@ public class RuntimeSCAReferenceBindingProvider implements ReferenceBindingProvi
             (BindingProviderFactory<DistributedSCABinding>)factoryExtensionPoint
                 .getProviderFactory(DistributedSCABinding.class);
 
+        this.mediator = extensionPoints.getExtensionPoint(UtilityExtensionPoint.class).getUtility(Mediator.class);
     }
 
     public boolean isTargetRemote() {
@@ -185,7 +189,11 @@ public class RuntimeSCAReferenceBindingProvider implements ReferenceBindingProvi
             RuntimeComponentService service = (RuntimeComponentService)target.getService();
             if (service != null) { // not a callback wire
                 InvocationChain chain = ((RuntimeEndpoint) target).getInvocationChain(operation);
-                return chain == null ? null : new SCABindingInvoker(chain);
+                
+                // it turns out that the chain source and target operations are the same, and are the operation 
+                // from the target, not sure if thats by design or a bug. The SCA binding invoker needs to know 
+                // the source and target class loaders so pass in the real source operation in the constructor 
+                return chain == null ? null : new SCABindingInvoker(chain, operation, mediator);
             }
         }
         return null;
diff --git a/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/SCABindingInvoker.java b/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/SCABindingInvoker.java
index 4dee1b8601..0f46e9390f 100644
--- a/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/SCABindingInvoker.java
+++ b/sca-java-2.x/trunk/modules/binding-sca-runtime/src/main/java/org/apache/tuscany/sca/binding/sca/provider/SCABindingInvoker.java
@@ -19,25 +19,34 @@
 
 package org.apache.tuscany.sca.binding.sca.provider;
 
+import org.apache.tuscany.sca.databinding.Mediator;
+import org.apache.tuscany.sca.interfacedef.Operation;
+import org.apache.tuscany.sca.invocation.DataExchangeSemantics;
 import org.apache.tuscany.sca.invocation.Interceptor;
 import org.apache.tuscany.sca.invocation.InvocationChain;
 import org.apache.tuscany.sca.invocation.Invoker;
 import org.apache.tuscany.sca.invocation.Message;
-import org.apache.tuscany.sca.invocation.DataExchangeSemantics;
 
 /**
  * @version $Rev$ $Date$
  */
 public class SCABindingInvoker implements Interceptor, DataExchangeSemantics {
     private InvocationChain chain;
+    private Mediator mediator;
+    private Operation sourceOperation;
+    private Operation targetOperation;
+    private boolean copyArgs;
     
     /**
      * Construct a SCABindingInvoker that delegates to the service invocaiton chain
-     * @param chain
      */
-    public SCABindingInvoker(InvocationChain chain) {
+    public SCABindingInvoker(InvocationChain chain, Operation sourceOperation, Mediator mediator) {
         super();
         this.chain = chain;
+        this.mediator = mediator;
+        this.sourceOperation = sourceOperation;
+        this.targetOperation = chain.getTargetOperation();
+        initCopyArgs();
     }
 
     /**
@@ -58,15 +67,65 @@ public class SCABindingInvoker implements Interceptor, DataExchangeSemantics {
      * @see org.apache.tuscany.sca.invocation.Invoker#invoke(org.apache.tuscany.sca.invocation.Message)
      */
     public Message invoke(Message msg) {
-        return getNext().invoke(msg);
+
+        if (copyArgs) {
+            msg.setBody(mediator.copyInput(msg.getBody(), sourceOperation, targetOperation));
+        }
+        
+        Message resultMsg = getNext().invoke(msg);
+        
+        if (copyArgs) {
+            // Note source and target operation swapped so result is in source class loader
+            if (resultMsg.isFault()) {
+                resultMsg.setFaultBody(mediator.copyFault(resultMsg.getBody(), targetOperation, sourceOperation));
+            } else {
+                if (sourceOperation.getOutputType() != null) {
+                    resultMsg.setBody(mediator.copyOutput(resultMsg.getBody(), targetOperation, sourceOperation));
+                }
+            }
+        }
+
+        return resultMsg;
+    }
+
+    /**
+     * Work out if pass-by-value copies or cross classloader copies need to be done
+     * - if source and target are in different classloaders
+     * - if the interfaces are remotable unless @AllowsPassByReference or 
+     *   a data transformation has been done in the chain
+     * - what else?
+     *    - have a flag to optionally disable copies for individual composite/service/operation
+     *      to improve the performance of specific local invocations?
+     */
+    private void initCopyArgs() {
+        this.copyArgs = crossClassLoaders() || isRemotable();
     }
 
+    private boolean crossClassLoaders() {
+        // TODO: for now if the operation is remotable the cross classloader copying will 
+        // happen automatically but this needs also to check the non-remotable operation classloaders 
+        return false;
+    }
+
+    /**
+     * Pass-by-value copies are required if the interfaces are remotable unless the
+     * implementation uses the @AllowsPassByReference annotation.
+     */
+    protected boolean isRemotable() {
+        if (!sourceOperation.getInterface().isRemotable()) {
+            return false;
+        }
+        if (!chain.getTargetOperation().getInterface().isRemotable()) {
+            return false;
+        }
+        return true;
+    }
+    
     /**
      * @see org.apache.tuscany.sca.invocation.DataExchangeSemantics#allowsPassByReference()
      */
     public boolean allowsPassByReference() {
         return false;
-//        return chain.allowsPassByReference();
     }
 
 }