Skip to content

Commit d13d307

Browse files
millefogliemillefoglie
committed
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]>
1 parent 5388349 commit d13d307

File tree

4 files changed

+98
-54
lines changed

4 files changed

+98
-54
lines changed

‎fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/ContractRouter.java‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,10 @@
66

77
package org.hyperledger.fabric.contract;
88

9-
import java.io.IOException;
10-
import java.util.Properties;
11-
import java.util.logging.Logger;
12-
139
import org.hyperledger.fabric.Logging;
14-
import org.hyperledger.fabric.contract.annotation.Serializer;
1510
import org.hyperledger.fabric.contract.execution.ExecutionFactory;
1611
import org.hyperledger.fabric.contract.execution.ExecutionService;
1712
import org.hyperledger.fabric.contract.execution.InvocationRequest;
18-
import org.hyperledger.fabric.contract.execution.SerializerInterface;
1913
import org.hyperledger.fabric.contract.metadata.MetadataBuilder;
2014
import org.hyperledger.fabric.contract.routing.ContractDefinition;
2115
import org.hyperledger.fabric.contract.routing.RoutingRegistry;
@@ -31,6 +25,10 @@
3125
import org.hyperledger.fabric.shim.ResponseUtils;
3226
import org.hyperledger.fabric.traces.Traces;
3327

28+
import java.io.IOException;
29+
import java.util.Properties;
30+
import java.util.logging.Logger;
31+
3432
/**
3533
* Router class routes Init/Invoke requests to contracts. Implements
3634
* {@link org.hyperledger.fabric.shim.Chaincode} interface.
@@ -46,6 +44,7 @@ public final class ContractRouter extends ChaincodeBase {
4644
// Store instances of SerializerInterfaces - identified by the contract
4745
// annotation (default is JSON)
4846
private final SerializerRegistryImpl serializers;
47+
private final ExecutionService executor;
4948

5049
/**
5150
* Take the arguments from the cli, and initiate processing of cli options and
@@ -79,6 +78,7 @@ public ContractRouter(final String[] args) {
7978
throw new RuntimeException(cre);
8079
}
8180

81+
executor = ExecutionFactory.getInstance().createExecutionService(serializers);
8282
}
8383

8484
/**
@@ -112,13 +112,6 @@ private Response processRequest(final ChaincodeStub stub) {
112112
logger.info(() -> "Got the invoke request for:" + stub.getFunction() + " " + stub.getParameters());
113113
final InvocationRequest request = ExecutionFactory.getInstance().createRequest(stub);
114114
final TxFunction txFn = getRouting(request);
115-
116-
// based on the routing information the serializer can be found
117-
// TRANSACTION target as this on the 'inbound' to invoke a tx
118-
final SerializerInterface si = serializers.getSerializer(txFn.getRouting().getSerializerName(),
119-
Serializer.TARGET.TRANSACTION);
120-
final ExecutionService executor = ExecutionFactory.getInstance().createExecutionService(si);
121-
122115
logger.info(() -> "Got routing:" + txFn.getRouting());
123116
return executor.executeRequest(txFn, request, stub);
124117
} else {

‎fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/ExecutionFactory.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88

99
import org.hyperledger.fabric.contract.execution.impl.ContractExecutionService;
1010
import org.hyperledger.fabric.contract.execution.impl.ContractInvocationRequest;
11+
import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
1112
import org.hyperledger.fabric.shim.ChaincodeStub;
1213

1314
public class ExecutionFactory {
1415
private static ExecutionFactory rf;
15-
private static ExecutionService es;
1616

1717
/**
1818
* @return ExecutionFactory
@@ -36,10 +36,7 @@ public InvocationRequest createRequest(final ChaincodeStub context) {
3636
* @param serializers Instance of the serializer
3737
* @return Execution Service
3838
*/
39-
public ExecutionService createExecutionService(final SerializerInterface serializers) {
40-
if (es == null) {
41-
es = new ContractExecutionService(serializers);
42-
}
43-
return es;
39+
public ExecutionService createExecutionService(final SerializerRegistryImpl serializers) {
40+
return new ContractExecutionService(serializers);
4441
}
4542
}

‎fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/impl/ContractExecutionService.java‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,38 @@
66

77
package org.hyperledger.fabric.contract.execution.impl;
88

9-
import java.lang.reflect.InvocationTargetException;
10-
import java.util.ArrayList;
11-
import java.util.HashMap;
12-
import java.util.List;
13-
import java.util.Map;
14-
import java.util.logging.Logger;
15-
16-
import org.hyperledger.fabric.contract.Context;
9+
import org.hyperledger.fabric.contract.Context;
1710
import org.hyperledger.fabric.contract.ContractInterface;
1811
import org.hyperledger.fabric.contract.ContractRuntimeException;
12+
import org.hyperledger.fabric.contract.annotation.Serializer;
1913
import org.hyperledger.fabric.contract.execution.ExecutionService;
2014
import org.hyperledger.fabric.contract.execution.InvocationRequest;
2115
import org.hyperledger.fabric.contract.execution.SerializerInterface;
2216
import org.hyperledger.fabric.contract.metadata.TypeSchema;
2317
import org.hyperledger.fabric.contract.routing.ParameterDefinition;
2418
import org.hyperledger.fabric.contract.routing.TxFunction;
19+
import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
2520
import org.hyperledger.fabric.shim.Chaincode;
2621
import org.hyperledger.fabric.shim.ChaincodeException;
2722
import org.hyperledger.fabric.shim.ChaincodeStub;
2823
import org.hyperledger.fabric.shim.ResponseUtils;
2924

25+
import java.lang.reflect.InvocationTargetException;
26+
import java.util.ArrayList;
27+
import java.util.List;
28+
import java.util.logging.Logger;
29+
3030
public class ContractExecutionService implements ExecutionService {
3131

3232
private static Logger logger = Logger.getLogger(ContractExecutionService.class.getName());
3333

34-
private final SerializerInterface serializer;
35-
private Map<String, Object> proxies = new HashMap<>();
34+
private final SerializerRegistryImpl serializers;
3635

3736
/**
38-
* @param serializer
37+
* @param serializers
3938
*/
40-
public ContractExecutionService(final SerializerInterface serializer) {
41-
this.serializer = serializer;
39+
public ContractExecutionService(final SerializerRegistryImpl serializers) {
40+
this.serializers = serializers;
4241
}
4342

4443
/**
@@ -84,15 +83,15 @@ public Chaincode.Response executeRequest(final TxFunction txFn, final Invocation
8483
}
8584

8685
private byte[] convertReturn(final Object obj, final TxFunction txFn) {
87-
byte[] buffer;
86+
final SerializerInterface serializer = serializers.getSerializer(
87+
txFn.getRouting().getSerializerName(), Serializer.TARGET.TRANSACTION);
8888
final TypeSchema ts = txFn.getReturnSchema();
89-
buffer = serializer.toBuffer(obj, ts);
90-
91-
return buffer;
89+
return serializer.toBuffer(obj, ts);
9290
}
9391

9492
private List<Object> convertArgs(final List<byte[]> stubArgs, final TxFunction txFn) {
95-
93+
final SerializerInterface serializer = serializers.getSerializer(
94+
txFn.getRouting().getSerializerName(), Serializer.TARGET.TRANSACTION);
9695
final List<ParameterDefinition> schemaParams = txFn.getParamsList();
9796
final List<Object> args = new ArrayList<>(stubArgs.size() + 1); // allow for context as the first argument
9897
for (int i = 0; i < schemaParams.size(); i++) {

‎fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/execution/ContractExecutionServiceTest.java‎

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,37 @@
66

77
package org.hyperledger.fabric.contract.execution;
88

9-
import static org.hamcrest.Matchers.equalTo;
10-
import static org.junit.Assert.assertThat;
11-
import static org.mockito.ArgumentMatchers.any;
12-
import static org.mockito.Mockito.mock;
13-
import static org.mockito.Mockito.spy;
14-
import static org.mockito.Mockito.verify;
15-
import static org.mockito.Mockito.when;
16-
17-
import java.lang.reflect.InvocationTargetException;
18-
import java.util.ArrayList;
19-
9+
import contract.SampleContract;
2010
import org.hyperledger.fabric.contract.ChaincodeStubNaiveImpl;
2111
import org.hyperledger.fabric.contract.Context;
2212
import org.hyperledger.fabric.contract.ContractInterface;
2313
import org.hyperledger.fabric.contract.ContractRuntimeException;
14+
import org.hyperledger.fabric.contract.annotation.Serializer;
2415
import org.hyperledger.fabric.contract.execution.impl.ContractExecutionService;
16+
import org.hyperledger.fabric.contract.metadata.TypeSchema;
17+
import org.hyperledger.fabric.contract.routing.ParameterDefinition;
2518
import org.hyperledger.fabric.contract.routing.TxFunction;
19+
import org.hyperledger.fabric.contract.routing.impl.ParameterDefinitionImpl;
20+
import org.hyperledger.fabric.contract.routing.impl.SerializerRegistryImpl;
2621
import org.hyperledger.fabric.shim.Chaincode.Response;
2722
import org.hyperledger.fabric.shim.ChaincodeStub;
2823
import org.junit.Rule;
2924
import org.junit.Test;
3025
import org.junit.rules.ExpectedException;
3126

32-
import contract.SampleContract;
27+
import java.lang.reflect.InvocationTargetException;
28+
import java.lang.reflect.Method;
29+
import java.lang.reflect.Parameter;
30+
import java.util.ArrayList;
31+
import java.util.Collections;
32+
33+
import static org.hamcrest.Matchers.equalTo;
34+
import static org.junit.Assert.assertThat;
35+
import static org.mockito.ArgumentMatchers.any;
36+
import static org.mockito.Mockito.mock;
37+
import static org.mockito.Mockito.spy;
38+
import static org.mockito.Mockito.verify;
39+
import static org.mockito.Mockito.when;
3340

3441
public class ContractExecutionServiceTest {
3542
@Rule
@@ -39,10 +46,9 @@ public class ContractExecutionServiceTest {
3946
@Test
4047
public void noReturnValue()
4148
throws IllegalAccessException, InstantiationException, InvocationTargetException, NoSuchMethodException, SecurityException {
42-
4349
JSONTransactionSerializer jts = new JSONTransactionSerializer();
44-
45-
ContractExecutionService ces = new ContractExecutionService(jts);
50+
SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
51+
ContractExecutionService ces = new ContractExecutionService(serializerRegistry);
4652

4753
ContractInterface contract = spy(new SampleContract());
4854
TxFunction txFn = mock(TxFunction.class);
@@ -55,6 +61,7 @@ public void noReturnValue()
5561
when(req.getArgs()).thenReturn(new ArrayList<byte[]>());
5662
when(routing.getMethod()).thenReturn(SampleContract.class.getMethod("noReturn", new Class<?>[] {Context.class}));
5763
when(routing.getContractInstance()).thenReturn(contract);
64+
when(serializerRegistry.getSerializer(any(), any())).thenReturn(jts);
5865
ces.executeRequest(txFn, req, stub);
5966

6067
verify(contract).beforeTransaction(any());
@@ -65,9 +72,9 @@ public void noReturnValue()
6572
@Test()
6673
public void failureToInvoke()
6774
throws IllegalAccessException, InstantiationException, InvocationTargetException, NoSuchMethodException, SecurityException {
68-
6975
JSONTransactionSerializer jts = new JSONTransactionSerializer();
70-
ContractExecutionService ces = new ContractExecutionService(jts);
76+
SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
77+
ContractExecutionService ces = new ContractExecutionService(serializerRegistry);
7178

7279
spy(new SampleContract());
7380
TxFunction txFn = mock(TxFunction.class);
@@ -83,6 +90,7 @@ public void failureToInvoke()
8390

8491
when(routing.getContractInstance()).thenThrow(IllegalAccessException.class);
8592
when(routing.toString()).thenReturn("MockMethodName:MockClassName");
93+
when(serializerRegistry.getSerializer(any(), any())).thenReturn(jts);
8694

8795
thrown.expect(ContractRuntimeException.class);
8896
thrown.expectMessage("Could not execute contract method: MockMethodName:MockClassName");
@@ -91,4 +99,51 @@ public void failureToInvoke()
9199
assertThat(resp.getStatusCode(), equalTo(500));
92100
}
93101

102+
@SuppressWarnings({ "serial" })
103+
@Test()
104+
public void invokeWithDifferentSerializers()
105+
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException, InstantiationException {
106+
JSONTransactionSerializer defaultSerializer = spy(new JSONTransactionSerializer());
107+
SerializerInterface customSerializer = mock(SerializerInterface.class);
108+
SerializerRegistryImpl serializerRegistry = spy(new SerializerRegistryImpl());
109+
ExecutionService executionService = ExecutionFactory.getInstance().createExecutionService(serializerRegistry);
110+
111+
TxFunction txFn = mock(TxFunction.class);
112+
InvocationRequest req = mock(InvocationRequest.class);
113+
TxFunction.Routing routing = mock(TxFunction.Routing.class);
114+
115+
TypeSchema ts = TypeSchema.typeConvert(String.class);
116+
Method method = SampleContract.class.getMethod("t1", Context.class, String.class);
117+
Parameter[] params = method.getParameters();
118+
ParameterDefinition pd = new ParameterDefinitionImpl("arg1", String.class, ts, params[1]);
119+
120+
byte[] arg = "asdf".getBytes();
121+
ChaincodeStub stub = new ChaincodeStubNaiveImpl();
122+
ContractInterface contract = spy(new SampleContract());
123+
124+
when(req.getArgs()).thenReturn(Collections.singletonList(arg));
125+
when(txFn.getRouting()).thenReturn(routing);
126+
when(txFn.getParamsList()).thenReturn(Collections.singletonList(pd));
127+
when(txFn.getReturnSchema()).thenReturn(ts);
128+
when(routing.getMethod()).thenReturn(method);
129+
when(routing.getContractInstance()).thenReturn(contract);
130+
131+
String defaultSerializerName = defaultSerializer.getClass().getCanonicalName();
132+
String customSerializerName = "customSerializer";
133+
134+
// execute transaction with the default serializer
135+
when(routing.getSerializerName()).thenReturn(defaultSerializerName);
136+
when(serializerRegistry.getSerializer(defaultSerializerName, Serializer.TARGET.TRANSACTION))
137+
.thenReturn(defaultSerializer);
138+
executionService.executeRequest(txFn, req, stub);
139+
140+
// execute transaction with the custom serializer
141+
when(routing.getSerializerName()).thenReturn(customSerializerName);
142+
when(serializerRegistry.getSerializer(customSerializerName, Serializer.TARGET.TRANSACTION))
143+
.thenReturn(customSerializer);
144+
executionService.executeRequest(txFn, req, stub);
145+
146+
verify(defaultSerializer).fromBuffer(arg, ts);
147+
verify(customSerializer).fromBuffer(arg, ts);
148+
}
94149
}

0 commit comments

Comments
 (0)