Fix transaction serializer usage

Fixes the issue of having a single transaction serializer in use for all contracts regardless of configuration. The issue was due to having a singleton instance of ExecutionService that had only one associated serializer. Thus, the first used serializer was also used for all other transactions. The fix removes this singleton and replaces it with a ContractRouter field. #225 Signed-off-by: Bogdan Repeta <[email protected]>
hyperledger · Feb 13, 2022 · d13d307 · d13d307
1 parent 5388349
commit d13d307
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);
+    }
 }