Merge pull request #226 from millefoglie/fix/serializers

Fix transaction serializer usage
hyperledger · Feb 14, 2022 · ff9e936 · ff9e936
2 parents 5388349 + d13d307
commit ff9e936
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 54 deletions.
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/ContractRouter.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/ContractRouter.java
@@ -6,16 +6,10 @@
 
 package org.hyperledger.fabric.contract;
 
-import java.io.IOException;
-import java.util.Properties;
-import java.util.logging.Logger;
-
 import org.hyperledger.fabric.Logging;
-import org.hyperledger.fabric.contract.annotation.Serializer;
 import org.hyperledger.fabric.contract.execution.ExecutionFactory;
 import org.hyperledger.fabric.contract.execution.ExecutionService;
 import org.hyperledger.fabric.contract.execution.InvocationRequest;
-import org.hyperledger.fabric.contract.execution.SerializerInterface;
 import org.hyperledger.fabric.contract.metadata.MetadataBuilder;
 import org.hyperledger.fabric.contract.routing.ContractDefinition;
 import org.hyperledger.fabric.contract.routing.RoutingRegistry;
@@ -31,6 +25,10 @@
 import org.hyperledger.fabric.shim.ResponseUtils;
 import org.hyperledger.fabric.traces.Traces;
 
+import java.io.IOException;
+import java.util.Properties;
+import java.util.logging.Logger;
+
 /**
  * Router class routes Init/Invoke requests to contracts. Implements
  * {@link org.hyperledger.fabric.shim.Chaincode} interface.
@@ -46,6 +44,7 @@ public final class ContractRouter extends ChaincodeBase {
     // Store instances of SerializerInterfaces - identified by the contract
     // annotation (default is JSON)
     private final SerializerRegistryImpl serializers;
+    private final ExecutionService executor;
 
     /**
      * Take the arguments from the cli, and initiate processing of cli options and
@@ -79,6 +78,7 @@ public ContractRouter(final String[] args) {
             throw new RuntimeException(cre);
         }
 
+        executor = ExecutionFactory.getInstance().createExecutionService(serializers);
     }
 
     /**
@@ -112,13 +112,6 @@ private Response processRequest(final ChaincodeStub stub) {
                 logger.info(() -> "Got the invoke request for:" + stub.getFunction() + " " + stub.getParameters());
                 final InvocationRequest request = ExecutionFactory.getInstance().createRequest(stub);
                 final TxFunction txFn = getRouting(request);
-
-                // based on the routing information the serializer can be found
-                // TRANSACTION target as this on the 'inbound' to invoke a tx
-                final SerializerInterface si = serializers.getSerializer(txFn.getRouting().getSerializerName(),
-                        Serializer.TARGET.TRANSACTION);
-                final ExecutionService executor = ExecutionFactory.getInstance().createExecutionService(si);
-
                 logger.info(() -> "Got routing:" + txFn.getRouting());
                 return executor.executeRequest(txFn, request, stub);
             } else {

diff --git a/...incode-shim/src/main/java/org/hyperledger/fabric/contract/execution/ExecutionFactory.java b/...incode-shim/src/main/java/org/hyperledger/fabric/contract/execution/ExecutionFactory.java
@@ -8,11 +8,11 @@
 
 import org.hyperledger.fabric.contract.execution.impl.ContractExecutionService;
 import org.hyperledger.fabric.contract.execution.impl.ContractInvocationRequest;
+import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
 import org.hyperledger.fabric.shim.ChaincodeStub;
 
 public class ExecutionFactory {
     private static ExecutionFactory rf;
-    private static ExecutionService es;
 
     /**
      * @return ExecutionFactory
@@ -36,10 +36,7 @@ public InvocationRequest createRequest(final ChaincodeStub context) {
      * @param serializers Instance of the serializer
      * @return Execution Service
      */
-    public ExecutionService createExecutionService(final SerializerInterface serializers) {
-        if (es == null) {
-            es = new ContractExecutionService(serializers);
-        }
-        return es;
+    public ExecutionService createExecutionService(final SerializerRegistryImpl serializers) {
+        return new ContractExecutionService(serializers);
     }
 }
diff --git a/...rc/main/java/org/hyperledger/fabric/contract/execution/impl/ContractExecutionService.java b/...rc/main/java/org/hyperledger/fabric/contract/execution/impl/ContractExecutionService.java
@@ -6,39 +6,38 @@
 
 package org.hyperledger.fabric.contract.execution.impl;
 
-import java.lang.reflect.InvocationTargetException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.logging.Logger;
-
-import org.hyperledger.fabric.contract.Context;
+ import org.hyperledger.fabric.contract.Context;
 import org.hyperledger.fabric.contract.ContractInterface;
 import org.hyperledger.fabric.contract.ContractRuntimeException;
+import org.hyperledger.fabric.contract.annotation.Serializer;
 import org.hyperledger.fabric.contract.execution.ExecutionService;
 import org.hyperledger.fabric.contract.execution.InvocationRequest;
 import org.hyperledger.fabric.contract.execution.SerializerInterface;
 import org.hyperledger.fabric.contract.metadata.TypeSchema;
 import org.hyperledger.fabric.contract.routing.ParameterDefinition;
 import org.hyperledger.fabric.contract.routing.TxFunction;
+import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
 import org.hyperledger.fabric.shim.Chaincode;
 import org.hyperledger.fabric.shim.ChaincodeException;
 import org.hyperledger.fabric.shim.ChaincodeStub;
 import org.hyperledger.fabric.shim.ResponseUtils;
 
+import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Logger;
+
 public class ContractExecutionService implements ExecutionService {
 
     private static Logger logger = Logger.getLogger(ContractExecutionService.class.getName());
 
-    private final SerializerInterface serializer;
-    private Map<String, Object> proxies = new HashMap<>();
+    private final SerializerRegistryImpl serializers;
 
     /**
-     * @param serializer
+     * @param serializers
      */
-    public ContractExecutionService(final SerializerInterface serializer) {
-        this.serializer = serializer;
+    public ContractExecutionService(final SerializerRegistryImpl serializers) {
+        this.serializers = serializers;
     }
 
     /**
@@ -84,15 +83,15 @@ public Chaincode.Response executeRequest(final TxFunction txFn, final Invocation
     }
 
     private byte[] convertReturn(final Object obj, final TxFunction txFn) {
-        byte[] buffer;
+        final SerializerInterface serializer = serializers.getSerializer(
+                txFn.getRouting().getSerializerName(), Serializer.TARGET.TRANSACTION);
         final TypeSchema ts = txFn.getReturnSchema();
-        buffer = serializer.toBuffer(obj, ts);
-
-        return buffer;
+        return serializer.toBuffer(obj, ts);
     }
 
     private List<Object> convertArgs(final List<byte[]> stubArgs, final TxFunction txFn) {
-
+        final SerializerInterface serializer = serializers.getSerializer(
+                txFn.getRouting().getSerializerName(), Serializer.TARGET.TRANSACTION);
         final List<ParameterDefinition> schemaParams = txFn.getParamsList();
         final List<Object> args = new ArrayList<>(stubArgs.size() + 1); // allow for context as the first argument
         for (int i = 0; i < schemaParams.size(); i++) {

diff --git a/...src/test/java/org/hyperledger/fabric/contract/execution/ContractExecutionServiceTest.java b/...src/test/java/org/hyperledger/fabric/contract/execution/ContractExecutionServiceTest.java
@@ -6,30 +6,37 @@
 
 package org.hyperledger.fabric.contract.execution;
 
-import static org.hamcrest.Matchers.equalTo;
-import static org.junit.Assert.assertThat;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import java.lang.reflect.InvocationTargetException;
-import java.util.ArrayList;
-
+import contract.SampleContract;
 import org.hyperledger.fabric.contract.ChaincodeStubNaiveImpl;
 import org.hyperledger.fabric.contract.Context;
 import org.hyperledger.fabric.contract.ContractInterface;
 import org.hyperledger.fabric.contract.ContractRuntimeException;
+import org.hyperledger.fabric.contract.annotation.Serializer;
 import org.hyperledger.fabric.contract.execution.impl.ContractExecutionService;
+import org.hyperledger.fabric.contract.metadata.TypeSchema;
+import org.hyperledger.fabric.contract.routing.ParameterDefinition;
 import org.hyperledger.fabric.contract.routing.TxFunction;
+import org.hyperledger.fabric.contract.routing.impl.ParameterDefinitionImpl;
+import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
 import org.hyperledger.fabric.shim.Chaincode.Response;
 import org.hyperledger.fabric.shim.ChaincodeStub;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import contract.SampleContract;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class ContractExecutionServiceTest {
     @Rule
@@ -39,10 +46,9 @@ public class ContractExecutionServiceTest {
     @Test
     public void noReturnValue()
             throws IllegalAccessException, InstantiationException, InvocationTargetException, NoSuchMethodException, SecurityException {
-
         JSONTransactionSerializer jts = new JSONTransactionSerializer();
-
-        ContractExecutionService ces = new ContractExecutionService(jts);
+        SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
+        ContractExecutionService ces = new ContractExecutionService(serializerRegistry);
 
         ContractInterface contract = spy(new SampleContract());
         TxFunction txFn = mock(TxFunction.class);
@@ -55,6 +61,7 @@ public void noReturnValue()
         when(req.getArgs()).thenReturn(new ArrayList<byte[]>());
         when(routing.getMethod()).thenReturn(SampleContract.class.getMethod("noReturn", new Class<?>[] {Context.class}));
         when(routing.getContractInstance()).thenReturn(contract);
+        when(serializerRegistry.getSerializer(any(), any())).thenReturn(jts);
         ces.executeRequest(txFn, req, stub);
 
         verify(contract).beforeTransaction(any());
@@ -65,9 +72,9 @@ public void noReturnValue()
     @Test()
     public void failureToInvoke()
             throws IllegalAccessException, InstantiationException, InvocationTargetException, NoSuchMethodException, SecurityException {
-
         JSONTransactionSerializer jts = new JSONTransactionSerializer();
-        ContractExecutionService ces = new ContractExecutionService(jts);
+        SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
+        ContractExecutionService ces = new ContractExecutionService(serializerRegistry);
 
         spy(new SampleContract());
         TxFunction txFn = mock(TxFunction.class);
@@ -83,6 +90,7 @@ public void failureToInvoke()
 
         when(routing.getContractInstance()).thenThrow(IllegalAccessException.class);
         when(routing.toString()).thenReturn("MockMethodName:MockClassName");
+        when(serializerRegistry.getSerializer(any(), any())).thenReturn(jts);
 
         thrown.expect(ContractRuntimeException.class);
         thrown.expectMessage("Could not execute contract method: MockMethodName:MockClassName");
@@ -91,4 +99,51 @@ public void failureToInvoke()
         assertThat(resp.getStatusCode(), equalTo(500));
     }
 
+    @SuppressWarnings({ "serial" })
+    @Test()
+    public void invokeWithDifferentSerializers()
+            throws NoSuchMethodException, InvocationTargetException, IllegalAccessException, InstantiationException {
+        JSONTransactionSerializer defaultSerializer = spy(new JSONTransactionSerializer());
+        SerializerInterface customSerializer = mock(SerializerInterface.class);
+        SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
+        ExecutionService executionService = ExecutionFactory.getInstance().createExecutionService(serializerRegistry);
+
+        TxFunction txFn = mock(TxFunction.class);
+        InvocationRequest req = mock(InvocationRequest.class);
+        TxFunction.Routing routing = mock(TxFunction.Routing.class);
+
+        TypeSchema ts = TypeSchema.typeConvert(String.class);
+        Method method = SampleContract.class.getMethod("t1", Context.class, String.class);
+        Parameter[] params = method.getParameters();
+        ParameterDefinition pd = new ParameterDefinitionImpl("arg1", String.class, ts, params[1]);
+
+        byte[] arg = "asdf".getBytes();
+        ChaincodeStub stub = new ChaincodeStubNaiveImpl();
+        ContractInterface contract = spy(new SampleContract());
+
+        when(req.getArgs()).thenReturn(Collections.singletonList(arg));
+        when(txFn.getRouting()).thenReturn(routing);
+        when(txFn.getParamsList()).thenReturn(Collections.singletonList(pd));
+        when(txFn.getReturnSchema()).thenReturn(ts);
+        when(routing.getMethod()).thenReturn(method);
+        when(routing.getContractInstance()).thenReturn(contract);
+
+        String defaultSerializerName = defaultSerializer.getClass().getCanonicalName();
+        String customSerializerName = "customSerializer";
+
+        // execute transaction with the default serializer
+        when(routing.getSerializerName()).thenReturn(defaultSerializerName);
+        when(serializerRegistry.getSerializer(defaultSerializerName, Serializer.TARGET.TRANSACTION))
+                .thenReturn(defaultSerializer);
+        executionService.executeRequest(txFn, req, stub);
+
+        // execute transaction with the custom serializer
+        when(routing.getSerializerName()).thenReturn(customSerializerName);
+        when(serializerRegistry.getSerializer(customSerializerName, Serializer.TARGET.TRANSACTION))
+                .thenReturn(customSerializer);
+        executionService.executeRequest(txFn, req, stub);
+
+        verify(defaultSerializer).fromBuffer(arg, ts);
+        verify(customSerializer).fromBuffer(arg, ts);
+    }
 }