Skip to content

Commit eea281b

Browse files
Jason Yellicksykesm
Jason Yellick
authored and
sykesm
committed
[FAB-9582] Remove the sysccprovider singleton
The assorted singletons in the peer code are a constant obstacle to writing testable code. This CR removes the sysccprovider singleton and instead takes the interface implementation as context when necessary. Change-Id: Iffc690c6649ff80cc9e43970cb47a2b179564734 Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
1 parent 80e4797 commit eea281b

28 files changed

+147
-239
lines changed

‎core/chaincode/chaincode_support.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/golang/protobuf/proto"
1414
"github.com/hyperledger/fabric/core/chaincode/accesscontrol"
1515
"github.com/hyperledger/fabric/core/common/ccprovider"
16+
"github.com/hyperledger/fabric/core/common/sysccprovider"
1617
"github.com/hyperledger/fabric/core/container"
1718
"github.com/hyperledger/fabric/core/container/ccintf"
1819
pb "github.com/hyperledger/fabric/protos/peer"
@@ -123,6 +124,15 @@ type ChaincodeSupport struct {
123124
executetimeout time.Duration
124125
userRunsCC bool
125126
ContainerRuntime Runtime
127+
sccp sysccprovider.SystemChaincodeProvider
128+
}
129+
130+
// SetSysCCProvider is a bit of a hack to make a latent dependency of ChaincodeSupport
131+
// be an explicit dependency. Because the chaincode support must be registered before
132+
// the sysccprovider implementation can be created, we cannot make the sccp part of the
133+
// constructor for ChaincodeSupport
134+
func (cs *ChaincodeSupport) SetSysCCProvider(sccp sysccprovider.SystemChaincodeProvider) {
135+
cs.sccp = sccp
126136
}
127137

128138
// DuplicateChaincodeHandlerError returned if attempt to register same chaincodeID while a stream already exists.

‎core/chaincode/chaincode_support_test.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
"github.com/hyperledger/fabric/core/chaincode/mock"
3434
"github.com/hyperledger/fabric/core/chaincode/shim"
3535
"github.com/hyperledger/fabric/core/common/ccprovider"
36-
"github.com/hyperledger/fabric/core/common/sysccprovider"
3736
"github.com/hyperledger/fabric/core/config"
3837
"github.com/hyperledger/fabric/core/container"
3938
"github.com/hyperledger/fabric/core/ledger"
@@ -152,9 +151,8 @@ func initMockPeer(chainIDs ...string) error {
152151
GetApplicationConfigRv: &mc.MockApplication{CapabilitiesRv: &mc.MockApplicationCapabilities{}},
153152
GetApplicationConfigBoolRv: true,
154153
}
155-
sysccprovider.RegisterSystemChaincodeProviderFactory(
156-
&scc.ProviderFactory{Peer: peer.Default, PeerSupport: msi},
157-
)
154+
155+
sccp := &scc.ProviderImpl{Peer: peer.Default, PeerSupport: msi}
158156

159157
mockAclProvider = &mocks.MockACLProvider{}
160158
mockAclProvider.Reset()
@@ -174,6 +172,7 @@ func initMockPeer(chainIDs ...string) error {
174172
certGenerator := accesscontrol.NewAuthenticator(ca)
175173
theChaincodeSupport = NewChaincodeSupport(GlobalConfig(), "0.0.0.0:7052", false, ccStartupTimeout, ca.CertBytes(), certGenerator)
176174
SideEffectInitialize(theChaincodeSupport)
175+
theChaincodeSupport.SetSysCCProvider(sccp)
177176
theChaincodeSupport.executetimeout = time.Duration(1) * time.Second
178177

179178
// Mock policy checker
@@ -288,10 +287,10 @@ func endTx(t *testing.T, cccid *ccprovider.CCContext, txsim ledger.TxSimulator,
288287
}
289288

290289
func execCC(t *testing.T, ctxt context.Context, ccSide *mockpeer.MockCCComm, cccid *ccprovider.CCContext, waitForERROR bool, expectExecErr bool, done chan error, cis *pb.ChaincodeInvocationSpec, respSet *mockpeer.MockResponseSet) error {
290+
291291
ccSide.SetResponses(respSet)
292292

293293
_, _, err := theChaincodeSupport.ExecuteWithErrorFilter(ctxt, cccid, cis)
294-
295294
if err == nil && expectExecErr {
296295
t.Fatalf("expected error but succeeded")
297296
} else if err != nil && !expectExecErr {
@@ -1008,7 +1007,7 @@ func TestLaunchAndWaitLaunchError(t *testing.T) {
10081007
}
10091008

10101009
func TestGetTxContextFromHandler(t *testing.T) {
1011-
h := Handler{txCtxs: NewTransactionContexts()}
1010+
h := Handler{txCtxs: NewTransactionContexts(), sccp: &scc.ProviderImpl{Peer: peer.Default, PeerSupport: peer.DefaultSupport}}
10121011

10131012
chnl := "test"
10141013
txid := "1"
@@ -1194,10 +1193,6 @@ func cc2SameCC(t *testing.T, chainID, chainID2, ccname string, ccSide *mockpeer.
11941193
}
11951194

11961195
func TestCCFramework(t *testing.T) {
1197-
// XXX temporarily skipping to make a CR series easier to review
1198-
// restored in https://gerrit.hyperledger.org/r/c/20749/
1199-
t.Skip()
1200-
12011196
//register 2 channels
12021197
chainID := "mockchainid"
12031198
chainID2 := "secondchain"
@@ -1219,7 +1214,7 @@ func TestCCFramework(t *testing.T) {
12191214
initializeCC(t, chainID, ccname, ccSide)
12201215

12211216
//chaincode support should not allow dups
1222-
if err := theChaincodeSupport.registerHandler(&Handler{ChaincodeID: &pb.ChaincodeID{Name: ccname + ":0"}}); err == nil {
1217+
if err := theChaincodeSupport.registerHandler(&Handler{ChaincodeID: &pb.ChaincodeID{Name: ccname + ":0"}, sccp: theChaincodeSupport.sccp}); err == nil {
12231218
t.Fatalf("expected re-register to fail")
12241219
} else if err, _ := err.(*DuplicateChaincodeHandlerError); err == nil {
12251220
t.Fatalf("expected DuplicateChaincodeHandlerError")

‎core/chaincode/exectransaction_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ import (
5757
var runTests bool
5858

5959
func testForSkip(t *testing.T) {
60-
// XXX temporarily skipping to make a CR series easier to review
61-
// restored in https://gerrit.hyperledger.org/r/c/20749/
62-
runTests = false
63-
6460
//run tests
6561
if !runTests {
6662
t.SkipNow()
@@ -988,10 +984,6 @@ func TestChaincodeInvokeChaincodeErrorCase(t *testing.T) {
988984

989985
// Test the invocation of a transaction.
990986
func TestQueries(t *testing.T) {
991-
// XXX temporarily skipping to make a CR series easier to review
992-
// restored in https://gerrit.hyperledger.org/r/c/20749/
993-
t.Skip()
994-
995987
// Allow queries test alone so that end to end test can be performed. It takes less than 5 seconds.
996988
//testForSkip(t)
997989

‎core/chaincode/handler.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ type Handler struct {
8686
registered bool
8787
readyNotify chan bool
8888

89+
sccp sysccprovider.SystemChaincodeProvider
90+
8991
//chan to pass error in sync and nonsync mode
9092
errChan chan error
9193

@@ -226,7 +228,7 @@ func (h *Handler) cleanupQueryContext(txContext *TransactionContext, queryID str
226228
func (h *Handler) checkACL(signedProp *pb.SignedProposal, proposal *pb.Proposal, ccIns *sysccprovider.ChaincodeInstance) error {
227229
// ensure that we don't invoke a system chaincode
228230
// that is not invokable through a cc2cc invocation
229-
if sysccprovider.GetSystemChaincodeProvider().IsSysCCAndNotInvokableCC2CC(ccIns.ChaincodeName) {
231+
if h.sccp.IsSysCCAndNotInvokableCC2CC(ccIns.ChaincodeName) {
230232
return errors.Errorf("system chaincode %s cannot be invoked with a cc2cc invocation", ccIns.ChaincodeName)
231233
}
232234

@@ -237,7 +239,7 @@ func (h *Handler) checkACL(signedProp *pb.SignedProposal, proposal *pb.Proposal,
237239
// - an application chaincode (and we still need to determine
238240
// whether the invoker can invoke it)
239241

240-
if sysccprovider.GetSystemChaincodeProvider().IsSysCC(ccIns.ChaincodeName) {
242+
if h.sccp.IsSysCC(ccIns.ChaincodeName) {
241243
// Allow this call
242244
return nil
243245
}
@@ -354,11 +356,11 @@ func (h *Handler) processStream() error {
354356
func HandleChaincodeStream(chaincodeSupport *ChaincodeSupport, ctxt context.Context, stream ccintf.ChaincodeStream) error {
355357
deadline, ok := ctxt.Deadline()
356358
chaincodeLogger.Debugf("Current context deadline = %s, ok = %v", deadline, ok)
357-
h := newChaincodeSupportHandler(chaincodeSupport, stream)
359+
h := newChaincodeSupportHandler(chaincodeSupport, stream, chaincodeSupport.sccp)
358360
return h.processStream()
359361
}
360362

361-
func newChaincodeSupportHandler(chaincodeSupport *ChaincodeSupport, peerChatStream ccintf.ChaincodeStream) *Handler {
363+
func newChaincodeSupportHandler(chaincodeSupport *ChaincodeSupport, peerChatStream ccintf.ChaincodeStream, sccp sysccprovider.SystemChaincodeProvider) *Handler {
362364
v := &Handler{
363365
ChatStream: peerChatStream,
364366
handlerSupport: chaincodeSupport,
@@ -368,6 +370,7 @@ func newChaincodeSupportHandler(chaincodeSupport *ChaincodeSupport, peerChatStre
368370
activeTransactions: NewActiveTransactions(),
369371
keepalive: chaincodeSupport.keepalive,
370372
userRunsCC: chaincodeSupport.userRunsCC,
373+
sccp: sccp,
371374
}
372375

373376
v.readyStateHandlers = stateHandlers{
@@ -1027,7 +1030,7 @@ func (h *Handler) getTxContextForMessage(channelID string, txid string, msgType
10271030
// If calledCcIns is not an SCC, isValidTxSim should be called which will return an err.
10281031
// We do not want to propagate calls to user CCs when the original call was to a SCC
10291032
// without a channel context (ie, no ledger context).
1030-
if isscc := sysccprovider.GetSystemChaincodeProvider().IsSysCC(calledCcIns.ChaincodeName); !isscc {
1033+
if isscc := h.sccp.IsSysCC(calledCcIns.ChaincodeName); !isscc {
10311034
// normal path - UCC invocation with an empty ("") channel: isValidTxSim will return an error
10321035
return h.isValidTxSim("", txid, fmtStr, args)
10331036
}
@@ -1163,7 +1166,7 @@ func (h *Handler) handleModState(msg *pb.ChaincodeMessage) {
11631166
shorttxid(msg.Txid), calledCcIns.ChaincodeName, calledCcIns.ChainID)
11641167

11651168
//is the chaincode a system chaincode ?
1166-
isscc := sysccprovider.GetSystemChaincodeProvider().IsSysCC(calledCcIns.ChaincodeName)
1169+
isscc := h.sccp.IsSysCC(calledCcIns.ChaincodeName)
11671170

11681171
var version string
11691172
if !isscc {

‎core/committer/txvalidator/validator.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,11 @@ type blockValidationResult struct {
9090
}
9191

9292
// NewTxValidator creates new transactions validator
93-
func NewTxValidator(support Support) Validator {
93+
func NewTxValidator(support Support, sccp sysccprovider.SystemChaincodeProvider) Validator {
9494
// Encapsulates interface implementation
9595
return &txValidator{
9696
support: support,
97-
vscc: newVSCCValidator(support)}
97+
vscc: newVSCCValidator(support, sccp)}
9898
}
9999

100100
func (v *txValidator) chainExists(chain string) bool {

‎core/committer/txvalidator/validator_test.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/hyperledger/fabric/common/util"
3333
"github.com/hyperledger/fabric/core/chaincode/shim"
3434
ccp "github.com/hyperledger/fabric/core/common/ccprovider"
35-
"github.com/hyperledger/fabric/core/common/sysccprovider"
3635
"github.com/hyperledger/fabric/core/ledger"
3736
"github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/rwsetutil"
3837
"github.com/hyperledger/fabric/core/ledger/ledgermgmt"
@@ -71,7 +70,8 @@ func setupLedgerAndValidatorExplicit(t *testing.T, cpb *mockconfig.MockApplicati
7170
*mocktxvalidator.Support
7271
*semaphore.Weighted
7372
}{&mocktxvalidator.Support{LedgerVal: theLedger, ACVal: cpb}, semaphore.NewWeighted(10)}
74-
theValidator := NewTxValidator(vcs)
73+
mp := (&scc.MocksccProviderFactory{}).NewSystemChaincodeProvider()
74+
theValidator := NewTxValidator(vcs, mp)
7575

7676
return theLedger, theValidator
7777
}
@@ -634,7 +634,8 @@ func TestLedgerIsNoAvailable(t *testing.T) {
634634
*mocktxvalidator.Support
635635
*semaphore.Weighted
636636
}{&mocktxvalidator.Support{LedgerVal: theLedger, ACVal: &mockconfig.MockApplicationCapabilities{}}, semaphore.NewWeighted(10)}
637-
validator := NewTxValidator(vcs)
637+
mp := (&scc.MocksccProviderFactory{}).NewSystemChaincodeProvider()
638+
validator := NewTxValidator(vcs, mp)
638639

639640
ccID := "mycc"
640641
tx := getEnv(ccID, createRWset(t, ccID), t)
@@ -662,7 +663,8 @@ func TestValidationInvalidEndorsing(t *testing.T) {
662663
*mocktxvalidator.Support
663664
*semaphore.Weighted
664665
}{&mocktxvalidator.Support{LedgerVal: theLedger, ACVal: &mockconfig.MockApplicationCapabilities{}}, semaphore.NewWeighted(10)}
665-
validator := NewTxValidator(vcs)
666+
mp := (&scc.MocksccProviderFactory{}).NewSystemChaincodeProvider()
667+
validator := NewTxValidator(vcs, mp)
666668

667669
ccID := "mycc"
668670
tx := getEnv(ccID, createRWset(t, ccID), t)
@@ -699,11 +701,13 @@ func TestValidationInvalidEndorsing(t *testing.T) {
699701
func TestValidationResourceUpdate(t *testing.T) {
700702
theLedger := new(mockLedger)
701703
sup := &mocktxvalidator.Support{LedgerVal: theLedger, ACVal: &mockconfig.MockApplicationCapabilities{}}
704+
702705
vcs := struct {
703706
*mocktxvalidator.Support
704707
*semaphore.Weighted
705708
}{sup, semaphore.NewWeighted(10)}
706-
validator := NewTxValidator(vcs)
709+
mp := (&scc.MocksccProviderFactory{}).NewSystemChaincodeProvider()
710+
validator := NewTxValidator(vcs, mp)
707711

708712
ccID := "mycc"
709713
tx := getEnvWithType(ccID, createRWset(t, ccID), common.HeaderType_PEER_RESOURCE_UPDATE, t)
@@ -771,7 +775,6 @@ var executeChaincodeProvider = &ccExecuteChaincode{
771775
}
772776

773777
func TestMain(m *testing.M) {
774-
sysccprovider.RegisterSystemChaincodeProviderFactory(&scc.MocksccProviderFactory{})
775778
ccp.RegisterChaincodeProviderFactory(&ccprovider.MockCcProviderFactory{executeChaincodeProvider})
776779

777780
msptesttools.LoadMSPSetupForTesting()

‎core/committer/txvalidator/vscc_validator.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ type vsccValidatorImpl struct {
4343
}
4444

4545
// newVSCCValidator creates new vscc validator
46-
func newVSCCValidator(support Support) *vsccValidatorImpl {
46+
func newVSCCValidator(support Support, sccp sysccprovider.SystemChaincodeProvider) *vsccValidatorImpl {
4747
return &vsccValidatorImpl{
4848
support: support,
4949
ccprovider: ccprovider.GetChaincodeProvider(),
50-
sccprovider: sysccprovider.GetSystemChaincodeProvider(),
50+
sccprovider: sccp,
5151
}
5252
}
5353

@@ -324,7 +324,7 @@ func (v *vsccValidatorImpl) GetInfoForValidate(chdr *common.ChannelHeader, ccID
324324
}
325325
var policy []byte
326326
var err error
327-
if !sysccprovider.GetSystemChaincodeProvider().IsSysCC(ccID) {
327+
if !v.sccprovider.IsSysCC(ccID) {
328328
// when we are validating a chaincode that is not a
329329
// system CC, we need to ask the CC to give us the name
330330
// of VSCC and of the policy that should be used

‎core/common/ccprovider/cc_info_provider.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ import (
1515
)
1616

1717
// IsChaincodeDeployed returns true if the chaincode with given name and version is deployed
18-
func IsChaincodeDeployed(chainid, ccName, ccVersion string, ccHash []byte) (bool, error) {
19-
sccprovider := sysccprovider.GetSystemChaincodeProvider()
20-
qe, err := sccprovider.GetQueryExecutorForLedger(chainid)
18+
func IsChaincodeDeployed(chainid, ccName, ccVersion string, ccHash []byte, sccp sysccprovider.SystemChaincodeProvider) (bool, error) {
19+
qe, err := sccp.GetQueryExecutorForLedger(chainid)
2120
if err != nil {
2221
return false, fmt.Errorf("Could not retrieve QueryExecutor for channel %s, error %s", chainid, err)
2322
}
2423
defer qe.Done()
2524

25+
// XXX We are leaking details of the LSCC table structure to other parts of the code, and this is terrible
2626
chaincodeDataBytes, err := qe.GetState("lscc", ccName)
2727
if err != nil {
2828
return false, fmt.Errorf("Could not retrieve state for chaincode %s on channel %s, error %s", ccName, chainid, err)

‎core/common/sysccprovider/sysccprovider.go

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,30 +53,6 @@ type SystemChaincodeProvider interface {
5353
PolicyManager(channelID string) (policies.Manager, bool)
5454
}
5555

56-
var sccFactory SystemChaincodeProviderFactory
57-
58-
// SystemChaincodeProviderFactory defines a factory interface so
59-
// that the actual implementation can be injected
60-
type SystemChaincodeProviderFactory interface {
61-
NewSystemChaincodeProvider() SystemChaincodeProvider
62-
}
63-
64-
// RegisterSystemChaincodeProviderFactory is to be called once to set
65-
// the factory that will be used to obtain instances of ChaincodeProvider
66-
func RegisterSystemChaincodeProviderFactory(sccfact SystemChaincodeProviderFactory) {
67-
sccFactory = sccfact
68-
}
69-
70-
// GetSystemChaincodeProvider returns instances of SystemChaincodeProvider;
71-
// the actual implementation is controlled by the factory that
72-
// is registered via RegisterSystemChaincodeProviderFactory
73-
func GetSystemChaincodeProvider() SystemChaincodeProvider {
74-
if sccFactory == nil {
75-
panic("The factory must be set first via RegisterSystemChaincodeProviderFactory")
76-
}
77-
return sccFactory.NewSystemChaincodeProvider()
78-
}
79-
8056
// ChaincodeInstance is unique identifier of chaincode instance
8157
type ChaincodeInstance struct {
8258
ChainID string

0 commit comments

Comments
 (0)