Skip to content

Commit b703c8b

Browse files
author
Jason Yellick
committed
FAB-9804 Register syscc by name, not path
Currently, the inproccontroller maintains two maps to system chaincodes. One map is done by path, while the other is done by name-version pair. Since no other part of the fabric code seems to use path, and in fact, there is a comment in the syscc package claiming it it unused, we should move inproccontroller onto only name-version lookups. Change-Id: I82487f8d2c15191e81ce4b2985e58f92a92b1c0c Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
1 parent ca2360b commit b703c8b

File tree

4 files changed

+56
-32
lines changed

4 files changed

+56
-32
lines changed

‎core/container/inproccontroller/inproccontroller.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,15 @@ func (r *Registry) NewVM() container.VM {
7070
}
7171

7272
//Register registers system chaincode with given path. The deploy should be called to initialize
73-
func (r *Registry) Register(path string, cc shim.Chaincode) error {
74-
tmp := r.typeRegistry[path]
73+
func (r *Registry) Register(ccid *ccintf.CCID, cc shim.Chaincode) error {
74+
name := ccid.GetName()
75+
inprocLogger.Debugf("Registering chaincode instance: %s", name)
76+
tmp := r.typeRegistry[name]
7577
if tmp != nil {
76-
return SysCCRegisteredErr(path)
78+
return SysCCRegisteredErr(name)
7779
}
7880

79-
r.typeRegistry[path] = &inprocContainer{chaincode: cc}
81+
r.typeRegistry[name] = &inprocContainer{chaincode: cc}
8082
return nil
8183
}
8284

@@ -107,7 +109,8 @@ func (vm *InprocVM) getInstance(ctxt context.Context, ipctemplate *inprocContain
107109

108110
//Deploy verifies chaincode is registered and creates an instance for it. Currently only one instance can be created
109111
func (vm *InprocVM) Deploy(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, reader io.Reader) error {
110-
path := ccid.ChaincodeSpec.ChaincodeId.Path
112+
path := ccid.GetName()
113+
inprocLogger.Debugf("Deploying chaincode instance: %s", path)
111114

112115
ipctemplate := vm.registry.typeRegistry[path]
113116
if ipctemplate == nil {
@@ -179,7 +182,7 @@ func (ipc *inprocContainer) launchInProc(ctxt context.Context, id string, args [
179182

180183
//Start starts a previously registered system codechain
181184
func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, filesToUpload map[string][]byte, builder container.Builder) error {
182-
path := ccid.ChaincodeSpec.ChaincodeId.Path
185+
path := ccid.GetName()
183186

184187
ipctemplate := vm.registry.typeRegistry[path]
185188

@@ -222,7 +225,7 @@ func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string,
222225

223226
//Stop stops a system codechain
224227
func (vm *InprocVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error {
225-
path := ccid.ChaincodeSpec.ChaincodeId.Path
228+
path := ccid.GetName()
226229

227230
ipctemplate := vm.registry.typeRegistry[path]
228231
if ipctemplate == nil {

‎core/container/inproccontroller/inproccontroller_test.go

+29-22
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,29 @@ func TestError(t *testing.T) {
3939
func TestRegisterSuccess(t *testing.T) {
4040
r := NewRegistry()
4141
shim := MockShim{}
42-
err := r.Register("path", shim)
42+
err := r.Register(&ccintf.CCID{
43+
ChaincodeSpec: &pb.ChaincodeSpec{
44+
ChaincodeId: &pb.ChaincodeID{
45+
Name: "name",
46+
},
47+
},
48+
}, shim)
4349

4450
assert.Nil(t, err, "err should be nil")
45-
assert.Equal(t, r.typeRegistry["path"].chaincode, shim, "shim should be correct")
51+
assert.Equal(t, r.typeRegistry["name"].chaincode, shim, "shim should be correct")
4652
}
4753

4854
func TestRegisterError(t *testing.T) {
4955
r := NewRegistry()
50-
r.typeRegistry["path"] = &inprocContainer{}
56+
r.typeRegistry["name"] = &inprocContainer{}
5157
shim := MockShim{}
52-
err := r.Register("path", shim)
58+
err := r.Register(&ccintf.CCID{
59+
ChaincodeSpec: &pb.ChaincodeSpec{
60+
ChaincodeId: &pb.ChaincodeID{
61+
Name: "name",
62+
},
63+
},
64+
}, shim)
5365

5466
assert.NotNil(t, err, "err should not be nil")
5567
}
@@ -139,7 +151,7 @@ func TestDeployNotRegistered(t *testing.T) {
139151
ccid := ccintf.CCID{
140152
ChaincodeSpec: &pb.ChaincodeSpec{
141153
ChaincodeId: &pb.ChaincodeID{
142-
Path: "path",
154+
Name: "name",
143155
},
144156
},
145157
}
@@ -157,7 +169,7 @@ func TestDeployNotRegistered(t *testing.T) {
157169
err := vm.Deploy(mockContext, ccid, args, env, mockReader)
158170

159171
assert.NotNil(t, err, "err should not be nil")
160-
assert.Equal(t, err.Error(), "path not registered. Please register the system chaincode in inprocinstances.go", "error message should be correct")
172+
assert.Equal(t, err.Error(), "name not registered. Please register the system chaincode in inprocinstances.go", "error message should be correct")
161173
}
162174

163175
func TestDeployNoChaincodeInstance(t *testing.T) {
@@ -168,7 +180,7 @@ func TestDeployNoChaincodeInstance(t *testing.T) {
168180
ccid := ccintf.CCID{
169181
ChaincodeSpec: &pb.ChaincodeSpec{
170182
ChaincodeId: &pb.ChaincodeID{
171-
Path: "path",
183+
Name: "name",
172184
},
173185
},
174186
}
@@ -181,11 +193,11 @@ func TestDeployNoChaincodeInstance(t *testing.T) {
181193

182194
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: make(chan struct{})}
183195

184-
r.typeRegistry["path"] = ipc
196+
r.typeRegistry["name"] = ipc
185197

186198
err := vm.Deploy(mockContext, ccid, args, env, mockReader)
187199
assert.NotNil(t, err, "err should not be nil")
188-
assert.Equal(t, err.Error(), "path system chaincode does not contain chaincode instance")
200+
assert.Equal(t, err.Error(), "name system chaincode does not contain chaincode instance")
189201
}
190202

191203
func TestDeployChaincode(t *testing.T) {
@@ -196,7 +208,7 @@ func TestDeployChaincode(t *testing.T) {
196208
ccid := ccintf.CCID{
197209
ChaincodeSpec: &pb.ChaincodeSpec{
198210
ChaincodeId: &pb.ChaincodeID{
199-
Path: "path",
211+
Name: "name",
200212
},
201213
},
202214
}
@@ -211,7 +223,7 @@ func TestDeployChaincode(t *testing.T) {
211223

212224
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: make(chan struct{})}
213225

214-
r.typeRegistry["path"] = ipc
226+
r.typeRegistry["name"] = ipc
215227

216228
err := vm.Deploy(mockContext, ccid, args, env, mockReader)
217229
assert.Nil(t, err, "err should be nil")
@@ -399,7 +411,7 @@ func TestStart(t *testing.T) {
399411
ccid := ccintf.CCID{
400412
ChaincodeSpec: &pb.ChaincodeSpec{
401413
ChaincodeId: &pb.ChaincodeID{
402-
Path: "path",
414+
Name: "name",
403415
},
404416
},
405417
}
@@ -414,7 +426,7 @@ func TestStart(t *testing.T) {
414426

415427
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: make(chan struct{})}
416428

417-
r.typeRegistry["path"] = ipc
429+
r.typeRegistry["name"] = ipc
418430

419431
err := vm.Start(mockContext, ccid, args, env, files, nil)
420432
assert.Nil(t, err, "err should be nil")
@@ -428,7 +440,6 @@ func TestStop(t *testing.T) {
428440
ccid := ccintf.CCID{
429441
ChaincodeSpec: &pb.ChaincodeSpec{
430442
ChaincodeId: &pb.ChaincodeID{
431-
Path: "path",
432443
Name: "name",
433444
},
434445
},
@@ -446,7 +457,7 @@ func TestStop(t *testing.T) {
446457
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: stopChan}
447458
ipc.running = true
448459

449-
r.typeRegistry["path"] = ipc
460+
r.typeRegistry["name-1"] = ipc
450461
r.instRegistry["name-1"] = ipc
451462

452463
go func() {
@@ -466,7 +477,6 @@ func TestStopNoIPCTemplate(t *testing.T) {
466477
ccid := ccintf.CCID{
467478
ChaincodeSpec: &pb.ChaincodeSpec{
468479
ChaincodeId: &pb.ChaincodeID{
469-
Path: "path",
470480
Name: "name",
471481
},
472482
},
@@ -475,7 +485,7 @@ func TestStopNoIPCTemplate(t *testing.T) {
475485

476486
err := vm.Stop(mockContext, ccid, 1000, true, true)
477487
assert.NotNil(t, err, "err should not be nil")
478-
assert.Equal(t, err.Error(), "path not registered", "error should be correct")
488+
assert.Equal(t, err.Error(), "name-1 not registered", "error should be correct")
479489
}
480490

481491
func TestStopNoIPC(t *testing.T) {
@@ -486,7 +496,6 @@ func TestStopNoIPC(t *testing.T) {
486496
ccid := ccintf.CCID{
487497
ChaincodeSpec: &pb.ChaincodeSpec{
488498
ChaincodeId: &pb.ChaincodeID{
489-
Path: "path",
490499
Name: "name",
491500
},
492501
},
@@ -503,7 +512,7 @@ func TestStopNoIPC(t *testing.T) {
503512
stopChan := make(chan struct{})
504513
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: stopChan}
505514

506-
r.typeRegistry["path"] = ipc
515+
r.typeRegistry["name-1"] = ipc
507516

508517
err := vm.Stop(mockContext, ccid, 1000, true, true)
509518
assert.NotNil(t, err, "err should not be nil")
@@ -518,7 +527,6 @@ func TestStopIPCNotRunning(t *testing.T) {
518527
ccid := ccintf.CCID{
519528
ChaincodeSpec: &pb.ChaincodeSpec{
520529
ChaincodeId: &pb.ChaincodeID{
521-
Path: "path",
522530
Name: "name",
523531
},
524532
},
@@ -535,7 +543,7 @@ func TestStopIPCNotRunning(t *testing.T) {
535543
stopChan := make(chan struct{})
536544
ipc := &inprocContainer{args: args, env: env, chaincode: mockInprocContainer.chaincode, stopChan: stopChan}
537545

538-
r.typeRegistry["path"] = ipc
546+
r.typeRegistry["name-1"] = ipc
539547
r.instRegistry["name-1"] = ipc
540548

541549
err := vm.Stop(mockContext, ccid, 1000, true, true)
@@ -549,7 +557,6 @@ func TestDestroy(t *testing.T) {
549557
ccid := ccintf.CCID{
550558
ChaincodeSpec: &pb.ChaincodeSpec{
551559
ChaincodeId: &pb.ChaincodeID{
552-
Path: "path",
553560
Name: "name",
554561
},
555562
},

‎core/scc/scc_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,5 +103,5 @@ func TestRegisterSysCC(t *testing.T) {
103103
Chaincode: nil,
104104
})
105105
assert.Error(t, err)
106-
assert.Contains(t, "path already registered", err)
106+
assert.Contains(t, "lscc-latest already registered", err)
107107
}

‎core/scc/sysccapi.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/hyperledger/fabric/common/util"
1717
"github.com/hyperledger/fabric/core/chaincode/shim"
1818
"github.com/hyperledger/fabric/core/common/ccprovider"
19+
"github.com/hyperledger/fabric/core/container/ccintf"
1920
"github.com/hyperledger/fabric/core/container/inproccontroller"
2021
"github.com/hyperledger/fabric/core/peer"
2122

@@ -29,7 +30,7 @@ var sysccLogger = flogging.MustGetLogger("sccapi")
2930
// Registrar provides a way for system chaincodes to be registered
3031
type Registrar interface {
3132
// Register registers a system chaincode
32-
Register(path string, cc shim.Chaincode) error
33+
Register(ccid *ccintf.CCID, cc shim.Chaincode) error
3334
}
3435

3536
// SystemChaincode defines the metadata needed to initialize system chaincode
@@ -71,7 +72,18 @@ func (p *Provider) registerSysCC(syscc *SystemChaincode) (bool, error) {
7172
return false, nil
7273
}
7374

74-
err := p.Registrar.Register(syscc.Path, syscc.Chaincode)
75+
// XXX This is an ugly hack, version should be tied to the chaincode instance, not he peer binary
76+
version := util.GetSysCCVersion()
77+
78+
ccid := &ccintf.CCID{
79+
ChaincodeSpec: &pb.ChaincodeSpec{
80+
ChaincodeId: &pb.ChaincodeID{
81+
Name: syscc.Name,
82+
},
83+
},
84+
Version: version,
85+
}
86+
err := p.Registrar.Register(ccid, syscc.Chaincode)
7587
if err != nil {
7688
//if the type is registered, the instance may not be... keep going
7789
if _, ok := err.(inproccontroller.SysCCRegisteredErr); !ok {
@@ -120,6 +132,7 @@ func (syscc *SystemChaincode) deploySysCC(chainID string) error {
120132

121133
chaincodeDeploymentSpec := &pb.ChaincodeDeploymentSpec{ExecEnv: pb.ChaincodeDeploymentSpec_SYSTEM, ChaincodeSpec: spec}
122134

135+
// XXX This is an ugly hack, version should be tied to the chaincode instance, not he peer binary
123136
version := util.GetSysCCVersion()
124137

125138
cccid := ccprov.GetCCContext(chainID, chaincodeDeploymentSpec.ChaincodeSpec.ChaincodeId.Name, version, txid, true, nil, nil)
@@ -145,6 +158,7 @@ func (syscc *SystemChaincode) deDeploySysCC(chainID string) error {
145158

146159
ccprov := ccprovider.GetChaincodeProvider()
147160

161+
// XXX This is an ugly hack, version should be tied to the chaincode instance, not he peer binary
148162
version := util.GetSysCCVersion()
149163

150164
cccid := ccprov.GetCCContext(chainID, syscc.Name, version, "", true, nil, nil)

0 commit comments

Comments
 (0)