From f0cd1395b9ebb3161e5b18b9d68e2f76ff0a4e07 Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Sep 2021 14:18:48 -0400 Subject: [PATCH] Remove RemoteRegistry reference from smartContractGW Route all calls through ContractStore instead. Signed-off-by: Andrew Richardson --- internal/contractgateway/smartcontractgw.go | 18 ++-- .../contractgateway/smartcontractgw_test.go | 16 +-- internal/contractregistry/contractstore.go | 30 ++++-- .../contractregistry/contractstore_test.go | 102 +++++++++--------- mocks/contractregistrymocks/contract_store.go | 32 +++++- 5 files changed, 118 insertions(+), 80 deletions(-) diff --git a/internal/contractgateway/smartcontractgw.go b/internal/contractgateway/smartcontractgw.go index fac626fe..b9036b87 100644 --- a/internal/contractgateway/smartcontractgw.go +++ b/internal/contractgateway/smartcontractgw.go @@ -143,10 +143,8 @@ func NewSmartContractGateway(conf *SmartContractGatewayConf, txnConf *tx.TxnProc baseURL, _ = url.Parse("http://localhost:8080") } log.Infof("OpenAPI Smart Contract Gateway configured with base URL '%s'", baseURL.String()) - rr := contractregistry.NewRemoteRegistry(&conf.RemoteRegistry) gw := &smartContractGW{ conf: conf, - rr: rr, baseSwaggerConf: &openapi.ABI2SwaggerConf{ ExternalHost: baseURL.Host, ExternalRootPath: baseURL.Path, @@ -156,14 +154,12 @@ func NewSmartContractGateway(conf *SmartContractGatewayConf, txnConf *tx.TxnProc }, ws: ws, } - gw.cs, err = contractregistry.NewContractStore(&contractregistry.ContractStoreConf{ + rr := contractregistry.NewRemoteRegistry(&conf.RemoteRegistry) + gw.cs = contractregistry.NewContractStore(&contractregistry.ContractStoreConf{ BaseURL: conf.BaseURL, StoragePath: conf.StoragePath, }, rr) - if err != nil { - return nil, err - } - if err = gw.rr.Init(); err != nil { + if err = gw.cs.Init(); err != nil { return nil, err } syncDispatcher := newSyncDispatcher(processor) @@ -175,7 +171,6 @@ func NewSmartContractGateway(conf *SmartContractGatewayConf, txnConf *tx.TxnProc } } gw.r2e = newREST2eth(gw, gw.cs, rpc, gw.sm, processor, asyncDispatcher, syncDispatcher) - gw.cs.Init() return gw, nil } @@ -183,7 +178,6 @@ type smartContractGW struct { conf *SmartContractGatewayConf sm events.SubscriptionManager cs contractregistry.ContractStore - rr contractregistry.RemoteRegistry r2e *rest2eth ws ws.WebSocketChannels baseSwaggerConf *openapi.ABI2SwaggerConf @@ -219,7 +213,7 @@ func (g *smartContractGW) PostDeploy(msg *messages.TransactionReceipt) error { var err error if isRemote { if msg.RegisterAs != "" { - err = g.rr.RegisterInstance(msg.RegisterAs, "0x"+addrHexNo0x) + err = g.cs.AddRemoteInstance(msg.RegisterAs, "0x"+addrHexNo0x) } } else { _, err = g.cs.AddContract(addrHexNo0x, requestID, registeredName, msg.RegisterAs) @@ -1163,8 +1157,8 @@ func (g *smartContractGW) Shutdown() { if g.sm != nil { g.sm.Close() } - if g.rr != nil { - g.rr.Close() + if g.cs != nil { + g.cs.Close() } } diff --git a/internal/contractgateway/smartcontractgw_test.go b/internal/contractgateway/smartcontractgw_test.go index e73062f1..a13204eb 100644 --- a/internal/contractgateway/smartcontractgw_test.go +++ b/internal/contractgateway/smartcontractgw_test.go @@ -509,6 +509,7 @@ func TestRemoteRegistrySwaggerOrABI(t *testing.T) { router.ServeHTTP(res, req) assert.Equal(500, res.Code) + mcs.On("Close").Return() scgw.Shutdown() mcs.AssertExpectations(t) } @@ -552,6 +553,7 @@ func TestRemoteRegistryBadABI(t *testing.T) { json.NewDecoder(res.Body).Decode(&msg) assert.Regexp("Invalid ABI", msg["error"]) + mcs.On("Close").Return() scgw.Shutdown() mcs.AssertExpectations(t) } @@ -753,8 +755,8 @@ func TestPostDeployRemoteRegisteredName(t *testing.T) { }, nil, nil, nil, nil, ) - mrr := &contractregistrymocks.RemoteRegistry{} - s.(*smartContractGW).rr = mrr + mcs := &contractregistrymocks.ContractStore{} + s.(*smartContractGW).cs = mcs contractAddr := ethbind.API.HexToAddress("0x0123456789AbcdeF0123456789abCdef01234567") scgw := s.(*smartContractGW) @@ -774,7 +776,7 @@ func TestPostDeployRemoteRegisteredName(t *testing.T) { RegisterAs: "lobster", } - mrr.On("RegisterInstance", "lobster", "0x0123456789abcdef0123456789abcdef01234567").Return(nil) + mcs.On("AddRemoteInstance", "lobster", "0x0123456789abcdef0123456789abcdef01234567").Return(nil) deployFile := path.Join(dir, "abi_message1.deploy.json") deployMsg := &messages.DeployContract{} @@ -785,7 +787,7 @@ func TestPostDeployRemoteRegisteredName(t *testing.T) { assert.Equal("http://localhost/api/v1/instances/lobster?openapi", replyMsg.ContractSwagger) - mrr.AssertExpectations(t) + mcs.AssertExpectations(t) } func TestPostDeployRemoteRegisteredNameNotSuccess(t *testing.T) { @@ -802,8 +804,8 @@ func TestPostDeployRemoteRegisteredNameNotSuccess(t *testing.T) { }, nil, nil, nil, nil, ) - mrr := &contractregistrymocks.RemoteRegistry{} - s.(*smartContractGW).rr = mrr + mcs := &contractregistrymocks.ContractStore{} + s.(*smartContractGW).cs = mcs contractAddr := ethbind.API.HexToAddress("0x0123456789AbcdeF0123456789abCdef01234567") scgw := s.(*smartContractGW) @@ -832,7 +834,7 @@ func TestPostDeployRemoteRegisteredNameNotSuccess(t *testing.T) { assert.Empty(replyMsg.ContractSwagger) - mrr.AssertExpectations(t) + mcs.AssertExpectations(t) } func TestPostDeployMissingContractAddress(t *testing.T) { diff --git a/internal/contractregistry/contractstore.go b/internal/contractregistry/contractstore.go index 112b2b44..195905c0 100644 --- a/internal/contractregistry/contractstore.go +++ b/internal/contractregistry/contractstore.go @@ -49,9 +49,11 @@ type ContractResolver interface { type ContractStore interface { ContractResolver - Init() + Init() error + Close() AddContract(addrHexNo0x, abiID, pathName, registerAs string) (*ContractInfo, error) AddABI(id string, deployMsg *messages.DeployContract, createdTime time.Time) *ABIInfo + AddRemoteInstance(lookupStr, address string) error GetLocalABIInfo(abiID string) (*ABIInfo, error) ListContracts() []messages.TimeSortable ListABIs() []messages.TimeSortable @@ -72,18 +74,14 @@ type contractStore struct { abiCache *lru.Cache } -func NewContractStore(conf *ContractStoreConf, rr RemoteRegistry) (cs ContractStore, err error) { - c := &contractStore{ +func NewContractStore(conf *ContractStoreConf, rr RemoteRegistry) ContractStore { + return &contractStore{ conf: conf, rr: rr, contractIndex: make(map[string]messages.TimeSortable), contractRegistrations: make(map[string]*ContractInfo), abiIndex: make(map[string]messages.TimeSortable), } - if c.abiCache, err = lru.New(DefaultABICacheSize); err != nil { - return nil, ethconnecterrors.Errorf(ethconnecterrors.RESTGatewayResourceErr, err) - } - return c, nil } // ContractInfo is the minimal data structure we keep in memory, indexed by address @@ -255,7 +253,7 @@ func (cs *contractStore) loadDeployMsg(abiID string) (*messages.DeployContract, return msg, nil } -func (cs *contractStore) Init() { +func (cs *contractStore) buildIndex() { log.Infof("Building installed smart contract index") legacyContractMatcher, _ := regexp.Compile(`^contract_([0-9a-z]{40})\.swagger\.json$`) instanceMatcher, _ := regexp.Compile(`^contract_([0-9a-z]{40})\.instance\.json$`) @@ -281,6 +279,18 @@ func (cs *contractStore) Init() { log.Infof("Smart contract index built. %d entries", len(cs.contractIndex)) } +func (cs *contractStore) Init() (err error) { + if cs.abiCache, err = lru.New(DefaultABICacheSize); err != nil { + return ethconnecterrors.Errorf(ethconnecterrors.RESTGatewayResourceErr, err) + } + cs.buildIndex() + return cs.rr.Init() +} + +func (cs *contractStore) Close() { + cs.rr.Close() +} + func (cs *contractStore) migrateLegacyContract(address, fileName string, createdTime time.Time) { swaggerFile, err := os.OpenFile(fileName, os.O_RDONLY, 0) if err != nil { @@ -404,6 +414,10 @@ func (cs *contractStore) AddABI(id string, deployMsg *messages.DeployContract, c return info } +func (cs *contractStore) AddRemoteInstance(lookupStr, address string) error { + return cs.rr.RegisterInstance(lookupStr, address) +} + func (cs *contractStore) ListContracts() []messages.TimeSortable { cs.idxLock.Lock() retval := make([]messages.TimeSortable, 0, len(cs.contractIndex)) diff --git a/internal/contractregistry/contractstore_test.go b/internal/contractregistry/contractstore_test.go index ce4ddf38..ca2571b7 100644 --- a/internal/contractregistry/contractstore_test.go +++ b/internal/contractregistry/contractstore_test.go @@ -73,7 +73,8 @@ func TestLoadDeployMsgOK(t *testing.T) { defer cleanup(dir) deployFile := path.Join(dir, "abi_abi1.deploy.json") - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) goodMsg := &messages.DeployContract{} @@ -100,8 +101,11 @@ func TestLoadDeployMsgMissing(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) + _, err = cs.GetABI(ABILocation{ ABIType: LocalABI, Name: "abi1", @@ -113,8 +117,11 @@ func TestLoadDeployMsgFileMissing(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) + cs.(*contractStore).abiIndex["abi1"] = &ABIInfo{} _, err = cs.GetABI(ABILocation{ ABIType: LocalABI, @@ -127,8 +134,11 @@ func TestLoadDeployMsgFailure(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) + cs.(*contractStore).abiIndex["abi1"] = &ABIInfo{} ioutil.WriteFile(path.Join(dir, "abi_abi1.deploy.json"), []byte(":bad json"), 0644) _, err = cs.GetABI(ABILocation{ @@ -142,10 +152,11 @@ func TestLoadDeployMsgRemoteLookupNotFound(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) - rr := &mockRR{} - cs.(*contractStore).rr = rr + _, err = cs.GetABI(ABILocation{ ABIType: LocalABI, Name: "abi1", @@ -157,7 +168,9 @@ func TestStoreABIWriteFail(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: path.Join(dir, "badpath")}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: path.Join(dir, "badpath")}, &mockRR{}) + err := cs.Init() assert.NoError(err) i := &ContractInfo{ @@ -171,7 +184,9 @@ func TestLoadABIForInstanceUnknown(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: path.Join(dir, "badpath")}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: path.Join(dir, "badpath")}, &mockRR{}) + err := cs.Init() assert.NoError(err) _, err = cs.GetContractByAddress("invalid") @@ -182,7 +197,9 @@ func TestLoadABIBadData(t *testing.T) { assert := assert.New(t) dir := tempdir() defer cleanup(dir) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) ioutil.WriteFile(path.Join(dir, "badness.abi.json"), []byte(":not json"), 0644) @@ -194,67 +211,49 @@ func TestLoadABIBadData(t *testing.T) { } func TestAddFileToContractIndexBadFileSwallowsError(t *testing.T) { - assert := assert.New(t) dir := tempdir() defer cleanup(dir) - - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) - assert.NoError(err) - + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) cs.(*contractStore).addFileToContractIndex("", "badness") } func TestAddFileToContractIndexBadDataSwallowsError(t *testing.T) { - assert := assert.New(t) dir := tempdir() defer cleanup(dir) - - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) - assert.NoError(err) - + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) fileName := path.Join(dir, "badness") ioutil.WriteFile(fileName, []byte("!JSON"), 0644) cs.(*contractStore).addFileToContractIndex("", fileName) } func TestAddFileToABIIndexBadFileSwallowsError(t *testing.T) { - assert := assert.New(t) dir := tempdir() defer cleanup(dir) - - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) - assert.NoError(err) - + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) cs.(*contractStore).addFileToABIIndex("", "badness", time.Now().UTC()) } func TestCheckNameAvailableRRDuplicate(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{BaseURL: "http://localhost/api/v1"}, nil) - assert.NoError(err) - - rr := &mockRR{ + mrr := &mockRR{ deployMsg: newTestDeployMsg(t, "12345"), } - cs.(*contractStore).rr = rr + cs := NewContractStore(&ContractStoreConf{BaseURL: "http://localhost/api/v1"}, mrr) - err = cs.CheckNameAvailable("lobster", true) + err := cs.CheckNameAvailable("lobster", true) assert.EqualError(err, "Contract address 12345 is already registered for name 'lobster'") } func TestCheckNameAvailableRRFail(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{BaseURL: "http://localhost/api/v1"}, nil) - assert.NoError(err) - - rr := &mockRR{ + mrr := &mockRR{ err: fmt.Errorf("pop"), } - cs.(*contractStore).rr = rr + cs := NewContractStore(&ContractStoreConf{BaseURL: "http://localhost/api/v1"}, mrr) - err = cs.CheckNameAvailable("lobster", true) + err := cs.CheckNameAvailable("lobster", true) assert.EqualError(err, "pop") } @@ -332,9 +331,9 @@ func TestBuildIndex(t *testing.T) { ioutil.WriteFile(path.Join(dir, "abi_e27be4cf-6ae2-411e-8088-db2992618938.deploy.json"), deployBytes, 0644) ioutil.WriteFile(path.Join(dir, "abi_519526b2-0879-41f4-93c0-09acaa62e2da.deploy.json"), []byte(":bad json"), 0644) - cs, err := NewContractStore(&ContractStoreConf{StoragePath: dir}, nil) + cs := NewContractStore(&ContractStoreConf{StoragePath: dir}, &mockRR{}) + err := cs.Init() assert.NoError(err) - cs.Init() contracts := cs.ListContracts() assert.Equal(4, len(contracts)) @@ -364,10 +363,7 @@ func TestBuildIndex(t *testing.T) { func TestGetABIRemoteGateway(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{}, nil) - assert.NoError(err) - - cs.(*contractStore).rr = &mockRR{ + mrr := &mockRR{ deployMsg: &DeployContractWithAddress{ Contract: &messages.DeployContract{ Description: "description", @@ -376,6 +372,10 @@ func TestGetABIRemoteGateway(t *testing.T) { }, } + cs := NewContractStore(&ContractStoreConf{}, mrr) + err := cs.Init() + assert.NoError(err) + location := ABILocation{ABIType: RemoteGateway} deployMsg, err := cs.GetABI(location, false) assert.NoError(err) @@ -386,9 +386,6 @@ func TestGetABIRemoteGateway(t *testing.T) { func TestGetABIRemoteInstance(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{}, nil) - assert.NoError(err) - mrr := &mockRR{ deployMsg: &DeployContractWithAddress{ Contract: &messages.DeployContract{ @@ -397,7 +394,10 @@ func TestGetABIRemoteInstance(t *testing.T) { Address: "address", }, } - cs.(*contractStore).rr = mrr + + cs := NewContractStore(&ContractStoreConf{}, mrr) + err := cs.Init() + assert.NoError(err) location := ABILocation{ABIType: RemoteInstance} deployMsg, err := cs.GetABI(location, false) @@ -417,11 +417,10 @@ func TestGetABIRemoteInstance(t *testing.T) { func TestGetABIRemoteInstanceFail(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{}, nil) + cs := NewContractStore(&ContractStoreConf{}, &mockRR{}) + err := cs.Init() assert.NoError(err) - cs.(*contractStore).rr = &mockRR{} - location := ABILocation{ABIType: RemoteInstance} deployMsg, err := cs.GetABI(location, false) assert.NoError(err) @@ -431,7 +430,8 @@ func TestGetABIRemoteInstanceFail(t *testing.T) { func TestGetABILocalFail(t *testing.T) { assert := assert.New(t) - cs, err := NewContractStore(&ContractStoreConf{}, nil) + cs := NewContractStore(&ContractStoreConf{}, &mockRR{}) + err := cs.Init() assert.NoError(err) location := ABILocation{ABIType: LocalABI, Name: "test"} diff --git a/mocks/contractregistrymocks/contract_store.go b/mocks/contractregistrymocks/contract_store.go index f991e254..fd3061a5 100644 --- a/mocks/contractregistrymocks/contract_store.go +++ b/mocks/contractregistrymocks/contract_store.go @@ -55,6 +55,20 @@ func (_m *ContractStore) AddContract(addrHexNo0x string, abiID string, pathName return r0, r1 } +// AddRemoteInstance provides a mock function with given fields: lookupStr, address +func (_m *ContractStore) AddRemoteInstance(lookupStr string, address string) error { + ret := _m.Called(lookupStr, address) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(lookupStr, address) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // CheckNameAvailable provides a mock function with given fields: name, isRemote func (_m *ContractStore) CheckNameAvailable(name string, isRemote bool) error { ret := _m.Called(name, isRemote) @@ -69,6 +83,11 @@ func (_m *ContractStore) CheckNameAvailable(name string, isRemote bool) error { return r0 } +// Close provides a mock function with given fields: +func (_m *ContractStore) Close() { + _m.Called() +} + // GetABI provides a mock function with given fields: location, refresh func (_m *ContractStore) GetABI(location contractregistry.ABILocation, refresh bool) (*contractregistry.DeployContractWithAddress, error) { ret := _m.Called(location, refresh) @@ -139,8 +158,17 @@ func (_m *ContractStore) GetLocalABIInfo(abiID string) (*contractregistry.ABIInf } // Init provides a mock function with given fields: -func (_m *ContractStore) Init() { - _m.Called() +func (_m *ContractStore) Init() error { + ret := _m.Called() + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 } // ListABIs provides a mock function with given fields: