From dff671423a8eae1c185640c734439fa97580d167 Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Jun 2022 17:11:34 -0400 Subject: [PATCH 1/6] Remove "VerifyNamespaceExists" method Since the data manager is now instantiated inside a namespace, it's redundant to check if that namespace exists. Signed-off-by: Andrew Richardson --- docs/reference/config.md | 7 --- internal/broadcast/datatype.go | 3 - internal/broadcast/datatype_test.go | 18 ------ internal/broadcast/message.go | 4 -- internal/broadcast/message_test.go | 37 ------------ internal/broadcast/tokenpool.go | 3 - internal/broadcast/tokenpool_test.go | 26 --------- internal/coreconfig/coreconfig.go | 4 -- internal/data/data_manager.go | 35 ------------ internal/data/data_manager_test.go | 37 ------------ internal/networkmap/register_identity.go | 4 -- internal/networkmap/register_identity_test.go | 56 ------------------- internal/networkmap/register_node_test.go | 5 -- internal/networkmap/register_org_test.go | 5 -- internal/orchestrator/subscriptions.go | 3 - internal/orchestrator/subscriptions_test.go | 17 ------ internal/privatemessaging/message.go | 3 - internal/privatemessaging/message_test.go | 54 ------------------ mocks/datamocks/manager.go | 14 ----- 19 files changed, 335 deletions(-) diff --git a/docs/reference/config.md b/docs/reference/config.md index c3a11a5346..b182b10b4d 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -581,13 +581,6 @@ nav_order: 2 |default|The default namespace - must be in the predefined list|`string`|`` |predefined|A list of namespaces to ensure exists, without requiring a broadcast from the network|List `string`|`` -## namespaces.cache - -|Key|Description|Type|Default Value| -|---|-----------|----|-------------| -|size|The size of the cache|[`BytesSize`](https://pkg.go.dev/github.com/docker/go-units#BytesSize)|`` -|ttl|The time to live (TTL) for the cache|[`time.Duration`](https://pkg.go.dev/time#Duration)|`` - ## namespaces.predefined[] |Key|Description|Type|Default Value| diff --git a/internal/broadcast/datatype.go b/internal/broadcast/datatype.go index 2255bfa1a3..159731bdfb 100644 --- a/internal/broadcast/datatype.go +++ b/internal/broadcast/datatype.go @@ -35,9 +35,6 @@ func (bm *broadcastManager) BroadcastDatatype(ctx context.Context, datatype *cor if err := datatype.Validate(ctx, false); err != nil { return nil, err } - if err := bm.data.VerifyNamespaceExists(ctx, datatype.Namespace); err != nil { - return nil, err - } datatype.Hash = datatype.Value.Hash() // Verify the data type is now all valid, before we broadcast it diff --git a/internal/broadcast/datatype_test.go b/internal/broadcast/datatype_test.go index 9b28e42d52..7d98afba61 100644 --- a/internal/broadcast/datatype_test.go +++ b/internal/broadcast/datatype_test.go @@ -40,25 +40,10 @@ func TestBroadcastDatatypeBadType(t *testing.T) { assert.Regexp(t, "FF00111.*validator", err) } -func TestBroadcastDatatypeNSGetFail(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - mdm := bm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(fmt.Errorf("pop")) - _, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{ - Name: "name1", - Namespace: "ns1", - Version: "0.0.1", - Value: fftypes.JSONAnyPtr(`{}`), - }, false) - assert.EqualError(t, err, "pop") -} - func TestBroadcastDatatypeBadValue(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() mdm := bm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil) mim := bm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) @@ -79,7 +64,6 @@ func TestBroadcastUpsertFail(t *testing.T) { mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil) _, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{ @@ -103,7 +87,6 @@ func TestBroadcastDatatypeInvalid(t *testing.T) { mim.On("ResolveInputIdentity", mock.Anything, mock.Anything).Return(nil) mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) _, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{ @@ -122,7 +105,6 @@ func TestBroadcastOk(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil) mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil) diff --git a/internal/broadcast/message.go b/internal/broadcast/message.go index 236c24e8fe..9287c016c5 100644 --- a/internal/broadcast/message.go +++ b/internal/broadcast/message.go @@ -114,10 +114,6 @@ func (s *broadcastSender) resolveAndSend(ctx context.Context, method sendMethod) func (s *broadcastSender) resolve(ctx context.Context) error { msg := s.msg.Message - if err := s.mgr.data.VerifyNamespaceExists(ctx, msg.Header.Namespace); err != nil { - return err - } - // Resolve the sending identity if msg.Header.Type != core.MessageTypeDefinition || msg.Header.Tag != core.SystemTagIdentityClaim { if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, &msg.Header.SignerRef); err != nil { diff --git a/internal/broadcast/message_test.go b/internal/broadcast/message_test.go index d1e3009d95..6ede2a3a52 100644 --- a/internal/broadcast/message_test.go +++ b/internal/broadcast/message_test.go @@ -39,7 +39,6 @@ func TestBroadcastMessageOk(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil) mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil) @@ -72,7 +71,6 @@ func TestBroadcastMessageWaitConfirmOk(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil) @@ -119,7 +117,6 @@ func TestBroadcastMessageTooLarge(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", ctx, mock.Anything).Run( func(args mock.Arguments) { newMsg := args[1].(*data.NewMessage) @@ -155,7 +152,6 @@ func TestBroadcastMessageBadInput(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", ctx, mock.Anything).Return(fmt.Errorf("pop")) mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil) @@ -175,8 +171,6 @@ func TestBroadcastMessageBadIdentity(t *testing.T) { ctx := context.Background() mim := bm.identity.(*identitymanagermocks.Manager) - mdm := bm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(fmt.Errorf("pop")) _, err := bm.BroadcastMessage(ctx, &core.MessageInOut{ @@ -186,7 +180,6 @@ func TestBroadcastMessageBadIdentity(t *testing.T) { }, false) assert.Regexp(t, "FF10206", err) - mdm.AssertExpectations(t) mim.AssertExpectations(t) } @@ -197,7 +190,6 @@ func TestBroadcastPrepare(t *testing.T) { mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil) @@ -222,32 +214,3 @@ func TestBroadcastPrepare(t *testing.T) { mdm.AssertExpectations(t) } - -func TestBroadcastPrepareBadNamespace(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - mdm := bm.data.(*datamocks.Manager) - - ctx := context.Background() - mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(fmt.Errorf("pop")) - - msg := &core.MessageInOut{ - Message: core.Message{ - Header: core.MessageHeader{ - SignerRef: core.SignerRef{ - Author: "did:firefly:org/abcd", - Key: "0x12345", - }, - }, - }, - InlineData: core.InlineData{ - {Value: fftypes.JSONAnyPtr(`{"hello": "world"}`)}, - }, - } - sender := bm.NewBroadcast(msg) - err := sender.Prepare(ctx) - - assert.EqualError(t, err, "pop") - - mdm.AssertExpectations(t) -} diff --git a/internal/broadcast/tokenpool.go b/internal/broadcast/tokenpool.go index 5a6dc400a0..7b62e9d901 100644 --- a/internal/broadcast/tokenpool.go +++ b/internal/broadcast/tokenpool.go @@ -26,9 +26,6 @@ func (bm *broadcastManager) BroadcastTokenPool(ctx context.Context, pool *core.T if err := pool.Pool.Validate(ctx); err != nil { return nil, err } - if err := bm.data.VerifyNamespaceExists(ctx, pool.Pool.Namespace); err != nil { - return nil, err - } msg, err = bm.BroadcastDefinitionAsNode(ctx, pool, core.SystemTagDefinePool, waitConfirm) if msg != nil { diff --git a/internal/broadcast/tokenpool_test.go b/internal/broadcast/tokenpool_test.go index aaeaa19046..aea276d411 100644 --- a/internal/broadcast/tokenpool_test.go +++ b/internal/broadcast/tokenpool_test.go @@ -30,30 +30,6 @@ import ( "github.com/stretchr/testify/mock" ) -func TestBroadcastTokenPoolNSGetFail(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - mdm := bm.data.(*datamocks.Manager) - - pool := &core.TokenPoolAnnouncement{ - Pool: &core.TokenPool{ - ID: fftypes.NewUUID(), - Namespace: "ns1", - Name: "mypool", - Type: core.TokenTypeNonFungible, - Locator: "N1", - Symbol: "COIN", - }, - } - - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(fmt.Errorf("pop")) - - _, err := bm.BroadcastTokenPool(context.Background(), pool, false) - assert.EqualError(t, err, "pop") - - mdm.AssertExpectations(t) -} - func TestBroadcastTokenPoolInvalid(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() @@ -96,7 +72,6 @@ func TestBroadcastTokenPoolBroadcastFail(t *testing.T) { } mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("WriteNewMessage", mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) _, err := bm.BroadcastTokenPool(context.Background(), pool, false) @@ -124,7 +99,6 @@ func TestBroadcastTokenPoolOk(t *testing.T) { } mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("WriteNewMessage", mock.Anything, mock.Anything).Return(nil) _, err := bm.BroadcastTokenPool(context.Background(), pool, false) diff --git a/internal/coreconfig/coreconfig.go b/internal/coreconfig/coreconfig.go index 84d771024e..2d78ac2d9a 100644 --- a/internal/coreconfig/coreconfig.go +++ b/internal/coreconfig/coreconfig.go @@ -234,10 +234,6 @@ var ( NamespacesDefault = ffc("namespaces.default") // NamespacesPredefined is a list of namespaces to ensure exists, without requiring a broadcast from the network NamespacesPredefined = ffc("namespaces.predefined") - // NamespaceCacheSize cache size for namespaces - NamespaceCacheSize = ffc("namespaces.cache.size") - // NamespaceCacheTTL cache time-to-live for namespaces - NamespaceCacheTTL = ffc("namespaces.cache.ttl") // NodeName is the short name for the node NodeName = ffc("node.name") // NodeDescription is a description for the node diff --git a/internal/data/data_manager.go b/internal/data/data_manager.go index d7b3b7276e..f432a85571 100644 --- a/internal/data/data_manager.go +++ b/internal/data/data_manager.go @@ -47,7 +47,6 @@ type Manager interface { UpdateMessageStateIfCached(ctx context.Context, id *fftypes.UUID, state core.MessageState, confirmed *fftypes.FFTime) ResolveInlineData(ctx context.Context, msg *NewMessage) error WriteNewMessage(ctx context.Context, newMsg *NewMessage) error - VerifyNamespaceExists(ctx context.Context, ns string) error UploadJSON(ctx context.Context, inData *core.DataRefOrValue) (*core.Data, error) UploadBlob(ctx context.Context, inData *core.DataRefOrValue, blob *ffapi.Multipart, autoMeta bool) (*core.Data, error) @@ -63,8 +62,6 @@ type dataManager struct { exchange dataexchange.Plugin validatorCache *ccache.Cache validatorCacheTTL time.Duration - namespaceCache *ccache.Cache - namespaceCacheTTL time.Duration messageCache *ccache.Cache messageCacheTTL time.Duration messageWriter *messageWriter @@ -120,11 +117,6 @@ func NewDataManager(ctx context.Context, ns string, di database.Plugin, pi share ccache.Configure(). MaxSize(config.GetByteSize(coreconfig.ValidatorCacheSize)), ) - dm.namespaceCache = ccache.New( - // We use a LRU cache with a size-aware max - ccache.Configure(). - MaxSize(config.GetByteSize(coreconfig.NamespaceCacheSize)), - ) dm.messageCache = ccache.New( // We use a LRU cache with a size-aware max ccache.Configure(). @@ -144,33 +136,6 @@ func (dm *dataManager) CheckDatatype(ctx context.Context, datatype *core.Datatyp return err } -func (dm *dataManager) namespaceExistsCached(ctx context.Context, ns string) (bool, error) { - cached := dm.namespaceCache.Get(ns) - if cached != nil { - cached.Extend(dm.namespaceCacheTTL) - return true, nil - } - log.L(context.Background()).Debugf("Cache miss for namespace %s", ns) - namespace, err := dm.database.GetNamespace(ctx, ns) - if err != nil || namespace == nil { - return false, err - } - dm.namespaceCache.Set(ns, namespace, dm.namespaceCacheTTL) - return true, nil -} - -func (dm *dataManager) VerifyNamespaceExists(ctx context.Context, ns string) error { - if err := core.ValidateFFNameField(ctx, ns, "namespace"); err != nil { - return err - } - if exists, err := dm.namespaceExistsCached(ctx, ns); err != nil { - return err - } else if !exists { - return i18n.NewError(ctx, coremsgs.MsgNamespaceNotExist) - } - return nil -} - // getValidatorForDatatype only returns database errors - not found (of all kinds) is a nil func (dm *dataManager) getValidatorForDatatype(ctx context.Context, validator core.ValidatorType, datatypeRef *core.DatatypeRef) (Validator, error) { if validator == "" { diff --git a/internal/data/data_manager_test.go b/internal/data/data_manager_test.go index d2376a2fa0..78a6ca2c3e 100644 --- a/internal/data/data_manager_test.go +++ b/internal/data/data_manager_test.go @@ -769,43 +769,6 @@ func TestValidateAllStoredValidatorInvalid(t *testing.T) { mdi.AssertExpectations(t) } -func TestVerifyNamespaceExistsInvalidFFName(t *testing.T) { - dm, ctx, cancel := newTestDataManager(t) - defer cancel() - err := dm.VerifyNamespaceExists(ctx, "!wrong") - assert.Regexp(t, "FF00140", err) -} - -func TestVerifyNamespaceExistsLookupErr(t *testing.T) { - dm, ctx, cancel := newTestDataManager(t) - defer cancel() - mdi := dm.database.(*databasemocks.Plugin) - mdi.On("GetNamespace", mock.Anything, "ns1").Return(nil, fmt.Errorf("pop")) - err := dm.VerifyNamespaceExists(ctx, "ns1") - assert.Regexp(t, "pop", err) -} - -func TestVerifyNamespaceExistsNotFound(t *testing.T) { - dm, ctx, cancel := newTestDataManager(t) - defer cancel() - mdi := dm.database.(*databasemocks.Plugin) - mdi.On("GetNamespace", mock.Anything, "ns1").Return(nil, nil) - err := dm.VerifyNamespaceExists(ctx, "ns1") - assert.Regexp(t, "FF10187", err) -} - -func TestVerifyNamespaceExistsOk(t *testing.T) { - dm, ctx, cancel := newTestDataManager(t) - defer cancel() - mdi := dm.database.(*databasemocks.Plugin) - mdi.On("GetNamespace", mock.Anything, "ns1").Return(&core.Namespace{}, nil).Once() - err := dm.VerifyNamespaceExists(ctx, "ns1") - assert.NoError(t, err) - // second lookup is from cache - err = dm.VerifyNamespaceExists(ctx, "ns1") - assert.NoError(t, err) -} - func TestHydrateBatchOK(t *testing.T) { dm, ctx, cancel := newTestDataManager(t) defer cancel() diff --git a/internal/networkmap/register_identity.go b/internal/networkmap/register_identity.go index 3423ae7c73..b7af679758 100644 --- a/internal/networkmap/register_identity.go +++ b/internal/networkmap/register_identity.go @@ -56,10 +56,6 @@ func (nm *networkMap) RegisterIdentity(ctx context.Context, dto *core.IdentityCr }, } - if err := nm.data.VerifyNamespaceExists(ctx, nm.namespace); err != nil { - return nil, err - } - // Set defaults if identity.Type == "" { identity.Type = core.IdentityTypeCustom diff --git a/internal/networkmap/register_identity_test.go b/internal/networkmap/register_identity_test.go index 28dadc722a..e9f51569c8 100644 --- a/internal/networkmap/register_identity_test.go +++ b/internal/networkmap/register_identity_test.go @@ -24,7 +24,6 @@ import ( "github.com/hyperledger/firefly-common/pkg/fftypes" "github.com/hyperledger/firefly/internal/syncasync" "github.com/hyperledger/firefly/mocks/broadcastmocks" - "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" "github.com/hyperledger/firefly/pkg/core" @@ -45,9 +44,6 @@ func TestRegisterIdentityOrgWithParentOk(t *testing.T) { Key: "0x23456", }, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mockMsg1 := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mockMsg2 := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mbm := nm.broadcast.(*broadcastmocks.Manager) @@ -77,7 +73,6 @@ func TestRegisterIdentityOrgWithParentOk(t *testing.T) { mim.AssertExpectations(t) mbm.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityOrgWithParentWaitConfirmOk(t *testing.T) { @@ -103,9 +98,6 @@ func TestRegisterIdentityOrgWithParentWaitConfirmOk(t *testing.T) { assert.NoError(t, err) }).Return(nil, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mockMsg1 := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mockMsg2 := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mbm := nm.broadcast.(*broadcastmocks.Manager) @@ -134,34 +126,6 @@ func TestRegisterIdentityOrgWithParentWaitConfirmOk(t *testing.T) { mim.AssertExpectations(t) mbm.AssertExpectations(t) msa.AssertExpectations(t) - mdm.AssertExpectations(t) -} - -func TestRegisterIdentityCustomBadNS(t *testing.T) { - - nm, cancel := newTestNetworkmap(t) - defer cancel() - - mim := nm.identity.(*identitymanagermocks.Manager) - mim.On("CachedIdentityLookupMustExist", nm.ctx, "did:firefly:org/parent1").Return(&core.Identity{ - IdentityBase: core.IdentityBase{ - ID: fftypes.NewUUID(), - DID: "did:firefly:org/parent1", - }, - }, false, nil) - - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(fmt.Errorf("pop")) - - _, err := nm.RegisterIdentity(nm.ctx, &core.IdentityCreateDTO{ - Name: "custom1", - Key: "0x12345", - Parent: "did:firefly:org/parent1", - }, false) - assert.Regexp(t, "pop", err) - - mim.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityCustomWithParentFail(t *testing.T) { @@ -183,9 +147,6 @@ func TestRegisterIdentityCustomWithParentFail(t *testing.T) { Key: "0x23456", }, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mockMsg := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mbm := nm.broadcast.(*broadcastmocks.Manager) @@ -212,7 +173,6 @@ func TestRegisterIdentityCustomWithParentFail(t *testing.T) { mim.AssertExpectations(t) mbm.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityGetParentMsgFail(t *testing.T) { @@ -226,9 +186,6 @@ func TestRegisterIdentityGetParentMsgFail(t *testing.T) { mim.On("VerifyIdentityChain", nm.ctx, mock.AnythingOfType("*core.Identity")).Return(parentIdentity, false, nil) mim.On("ResolveIdentitySigner", nm.ctx, parentIdentity).Return(nil, fmt.Errorf("pop")) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - _, err := nm.RegisterIdentity(nm.ctx, &core.IdentityCreateDTO{ Name: "custom1", Key: "0x12345", @@ -237,7 +194,6 @@ func TestRegisterIdentityGetParentMsgFail(t *testing.T) { assert.Regexp(t, "pop", err) mim.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityRootBroadcastFail(t *testing.T) { @@ -248,9 +204,6 @@ func TestRegisterIdentityRootBroadcastFail(t *testing.T) { mim := nm.identity.(*identitymanagermocks.Manager) mim.On("VerifyIdentityChain", nm.ctx, mock.AnythingOfType("*core.Identity")).Return(nil, false, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mbm := nm.broadcast.(*broadcastmocks.Manager) mbm.On("BroadcastIdentityClaim", nm.ctx, mock.AnythingOfType("*core.IdentityClaim"), @@ -268,7 +221,6 @@ func TestRegisterIdentityRootBroadcastFail(t *testing.T) { mim.AssertExpectations(t) mbm.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityMissingKey(t *testing.T) { @@ -279,9 +231,6 @@ func TestRegisterIdentityMissingKey(t *testing.T) { mim := nm.identity.(*identitymanagermocks.Manager) mim.On("VerifyIdentityChain", nm.ctx, mock.AnythingOfType("*core.Identity")).Return(nil, false, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - _, err := nm.RegisterIdentity(nm.ctx, &core.IdentityCreateDTO{ Name: "custom1", Parent: fftypes.NewUUID().String(), @@ -289,7 +238,6 @@ func TestRegisterIdentityMissingKey(t *testing.T) { assert.Regexp(t, "FF10352", err) mim.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityVerifyFail(t *testing.T) { @@ -300,9 +248,6 @@ func TestRegisterIdentityVerifyFail(t *testing.T) { mim := nm.identity.(*identitymanagermocks.Manager) mim.On("VerifyIdentityChain", nm.ctx, mock.AnythingOfType("*core.Identity")).Return(nil, false, fmt.Errorf("pop")) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - _, err := nm.RegisterIdentity(nm.ctx, &core.IdentityCreateDTO{ Name: "custom1", Parent: fftypes.NewUUID().String(), @@ -310,7 +255,6 @@ func TestRegisterIdentityVerifyFail(t *testing.T) { assert.Regexp(t, "pop", err) mim.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterIdentityBadParent(t *testing.T) { diff --git a/internal/networkmap/register_node_test.go b/internal/networkmap/register_node_test.go index 484375ae79..4ae4c28a02 100644 --- a/internal/networkmap/register_node_test.go +++ b/internal/networkmap/register_node_test.go @@ -25,7 +25,6 @@ import ( "github.com/hyperledger/firefly/internal/coreconfig" "github.com/hyperledger/firefly/mocks/broadcastmocks" "github.com/hyperledger/firefly/mocks/dataexchangemocks" - "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/pkg/core" "github.com/stretchr/testify/assert" @@ -49,9 +48,6 @@ func TestRegisterNodeOk(t *testing.T) { signerRef := &core.SignerRef{Key: "0x23456"} mim.On("ResolveIdentitySigner", nm.ctx, parentOrg).Return(signerRef, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mdx := nm.exchange.(*dataexchangemocks.Plugin) mdx.On("GetEndpointInfo", nm.ctx).Return(fftypes.JSONObject{ "id": "peer1", @@ -72,7 +68,6 @@ func TestRegisterNodeOk(t *testing.T) { mim.AssertExpectations(t) mdx.AssertExpectations(t) mbm.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterNodePeerInfoFail(t *testing.T) { diff --git a/internal/networkmap/register_org_test.go b/internal/networkmap/register_org_test.go index f120e09c53..1a9d8a05c2 100644 --- a/internal/networkmap/register_org_test.go +++ b/internal/networkmap/register_org_test.go @@ -23,7 +23,6 @@ import ( "github.com/hyperledger/firefly-common/pkg/fftypes" "github.com/hyperledger/firefly/mocks/broadcastmocks" - "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/pkg/core" "github.com/stretchr/testify/assert" @@ -63,9 +62,6 @@ func TestRegisterNodeOrgOk(t *testing.T) { }, nil) mim.On("VerifyIdentityChain", nm.ctx, mock.AnythingOfType("*core.Identity")).Return(nil, false, nil) - mdm := nm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", nm.ctx, "ns1").Return(nil) - mockMsg := &core.Message{Header: core.MessageHeader{ID: fftypes.NewUUID()}} mbm := nm.broadcast.(*broadcastmocks.Manager) mbm.On("BroadcastIdentityClaim", nm.ctx, @@ -81,7 +77,6 @@ func TestRegisterNodeOrgOk(t *testing.T) { mim.AssertExpectations(t) mbm.AssertExpectations(t) - mdm.AssertExpectations(t) } func TestRegisterNodeOrgNoName(t *testing.T) { diff --git a/internal/orchestrator/subscriptions.go b/internal/orchestrator/subscriptions.go index a459da622b..2d390fc816 100644 --- a/internal/orchestrator/subscriptions.go +++ b/internal/orchestrator/subscriptions.go @@ -40,9 +40,6 @@ func (or *orchestrator) createUpdateSubscription(ctx context.Context, subDef *co subDef.Created = fftypes.Now() subDef.Namespace = or.namespace subDef.Ephemeral = false - if err := or.data.VerifyNamespaceExists(ctx, subDef.Namespace); err != nil { - return nil, err - } if err := core.ValidateFFNameFieldNoUUID(ctx, subDef.Name, "name"); err != nil { return nil, err } diff --git a/internal/orchestrator/subscriptions_test.go b/internal/orchestrator/subscriptions_test.go index 2086201567..fd6afb4228 100644 --- a/internal/orchestrator/subscriptions_test.go +++ b/internal/orchestrator/subscriptions_test.go @@ -29,24 +29,10 @@ import ( "github.com/stretchr/testify/mock" ) -func TestCreateSubscriptionBadNamespace(t *testing.T) { - or := newTestOrchestrator() - defer or.cleanup(t) - - or.mdm.On("VerifyNamespaceExists", mock.Anything, "ns").Return(fmt.Errorf("pop")) - _, err := or.CreateSubscription(or.ctx, &core.Subscription{ - SubscriptionRef: core.SubscriptionRef{ - Name: "!sub1", - }, - }) - assert.EqualError(t, err, "pop") -} - func TestCreateSubscriptionBadName(t *testing.T) { or := newTestOrchestrator() defer or.cleanup(t) - or.mdm.On("VerifyNamespaceExists", mock.Anything, "ns").Return(nil) _, err := or.CreateSubscription(or.ctx, &core.Subscription{ SubscriptionRef: core.SubscriptionRef{ Name: "!sub1", @@ -59,7 +45,6 @@ func TestCreateSubscriptionSystemTransport(t *testing.T) { or := newTestOrchestrator() defer or.cleanup(t) - or.mdm.On("VerifyNamespaceExists", mock.Anything, "ns").Return(nil) _, err := or.CreateSubscription(or.ctx, &core.Subscription{ Transport: system.SystemEventsTransport, SubscriptionRef: core.SubscriptionRef{ @@ -78,7 +63,6 @@ func TestCreateSubscriptionOk(t *testing.T) { Name: "sub1", }, } - or.mdm.On("VerifyNamespaceExists", mock.Anything, "ns").Return(nil) or.mem.On("CreateUpdateDurableSubscription", mock.Anything, mock.Anything, true).Return(nil) s1, err := or.CreateSubscription(or.ctx, sub) assert.NoError(t, err) @@ -95,7 +79,6 @@ func TestCreateUpdateSubscriptionOk(t *testing.T) { Name: "sub1", }, } - or.mdm.On("VerifyNamespaceExists", mock.Anything, "ns").Return(nil) or.mem.On("CreateUpdateDurableSubscription", mock.Anything, mock.Anything, false).Return(nil) s1, err := or.CreateUpdateSubscription(or.ctx, sub) assert.NoError(t, err) diff --git a/internal/privatemessaging/message.go b/internal/privatemessaging/message.go index 4ebc2d970c..cb33e76e3e 100644 --- a/internal/privatemessaging/message.go +++ b/internal/privatemessaging/message.go @@ -131,9 +131,6 @@ func (s *messageSender) resolveAndSend(ctx context.Context, method sendMethod) e func (s *messageSender) resolve(ctx context.Context) error { msg := s.msg.Message - if err := s.mgr.data.VerifyNamespaceExists(ctx, msg.Header.Namespace); err != nil { - return err - } // Resolve the sending identity if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, &msg.Header.SignerRef); err != nil { diff --git a/internal/privatemessaging/message_test.go b/internal/privatemessaging/message_test.go index 761da9be6b..6120441859 100644 --- a/internal/privatemessaging/message_test.go +++ b/internal/privatemessaging/message_test.go @@ -86,7 +86,6 @@ func TestSendConfirmMessageE2EOk(t *testing.T) { mim.On("CachedIdentityLookupByID", pm.ctx, rootOrg.ID).Return(rootOrg, nil) mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(nil) mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() @@ -140,7 +139,6 @@ func TestSendUnpinnedMessageE2EOk(t *testing.T) { groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(nil) mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() @@ -177,9 +175,6 @@ func TestSendMessageBadGroup(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) defer cancel() - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) - mim := pm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", pm.ctx, mock.Anything).Return(nil) @@ -200,9 +195,6 @@ func TestSendMessageBadIdentity(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) defer cancel() - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) - mim := pm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")) @@ -218,7 +210,6 @@ func TestSendMessageBadIdentity(t *testing.T) { }, false) assert.Regexp(t, "FF10206.*pop", err) - mdm.AssertExpectations(t) mim.AssertExpectations(t) } @@ -245,7 +236,6 @@ func TestResolveAndSendBadInlineData(t *testing.T) { mdi.On("GetGroupByHash", pm.ctx, "ns1", mock.Anything, mock.Anything).Return(&core.Group{Hash: fftypes.NewRandB32()}, nil, nil).Once() mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")) message := &messageSender{ @@ -287,7 +277,6 @@ func TestSendUnpinnedMessageTooLarge(t *testing.T) { dataID := fftypes.NewUUID() groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Run(func(args mock.Arguments) { newMsg := args[1].(*data.NewMessage) newMsg.Message.Data = core.DataRefs{ @@ -363,7 +352,6 @@ func TestMessagePrepare(t *testing.T) { mdi.On("GetGroupByHash", pm.ctx, "ns1", mock.Anything, mock.Anything).Return(&core.Group{Hash: fftypes.NewRandB32()}, nil, nil).Once() mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(nil) message := pm.NewMessage(&core.MessageInOut{ @@ -388,34 +376,6 @@ func TestMessagePrepare(t *testing.T) { } -func TestMessagePrepareBadNamespace(t *testing.T) { - - pm, cancel := newTestPrivateMessaging(t) - defer cancel() - - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(fmt.Errorf("pop")) - - message := pm.NewMessage(&core.MessageInOut{ - Message: core.Message{ - Data: core.DataRefs{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, - }, - Group: &core.InputGroup{ - Members: []core.MemberInput{ - {Identity: "localorg"}, - }, - }, - }) - - err := message.Prepare(pm.ctx) - assert.EqualError(t, err, "pop") - - mdm.AssertExpectations(t) - -} - func TestSendUnpinnedMessageGroupLookupFail(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) @@ -463,7 +423,6 @@ func TestSendUnpinnedMessageInsertFail(t *testing.T) { groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(nil) mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")).Once() @@ -499,9 +458,6 @@ func TestSendUnpinnedMessageConfirmFail(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) defer cancel() - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) - mim := pm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")) @@ -520,7 +476,6 @@ func TestSendUnpinnedMessageConfirmFail(t *testing.T) { }, true) assert.Regexp(t, "pop", err) - mdm.AssertExpectations(t) mim.AssertExpectations(t) } @@ -530,9 +485,6 @@ func TestSendUnpinnedMessageResolveGroupFail(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) defer cancel() - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) - mim := pm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", pm.ctx, mock.Anything).Return(nil) @@ -562,7 +514,6 @@ func TestSendUnpinnedMessageResolveGroupFail(t *testing.T) { }, false) assert.EqualError(t, err, "pop") - mdm.AssertExpectations(t) mdi.AssertExpectations(t) mim.AssertExpectations(t) @@ -573,9 +524,6 @@ func TestSendUnpinnedMessageResolveGroupNotFound(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) defer cancel() - mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) - mim := pm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", pm.ctx, mock.Anything).Return(nil) @@ -605,7 +553,6 @@ func TestSendUnpinnedMessageResolveGroupNotFound(t *testing.T) { }, false) assert.Regexp(t, "FF10226", err) - mdm.AssertExpectations(t) mdi.AssertExpectations(t) mim.AssertExpectations(t) @@ -657,7 +604,6 @@ func TestRequestReplySuccess(t *testing.T) { Return(nil, nil) mdm := pm.data.(*datamocks.Manager) - mdm.On("VerifyNamespaceExists", pm.ctx, "ns1").Return(nil) mdm.On("ResolveInlineData", pm.ctx, mock.Anything).Return(nil) mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() diff --git a/mocks/datamocks/manager.go b/mocks/datamocks/manager.go index d74159d3e9..c857acfef0 100644 --- a/mocks/datamocks/manager.go +++ b/mocks/datamocks/manager.go @@ -302,20 +302,6 @@ func (_m *Manager) ValidateAll(ctx context.Context, _a1 core.DataArray) (bool, e return r0, r1 } -// VerifyNamespaceExists provides a mock function with given fields: ctx, ns -func (_m *Manager) VerifyNamespaceExists(ctx context.Context, ns string) error { - ret := _m.Called(ctx, ns) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, ns) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // WaitStop provides a mock function with given fields: func (_m *Manager) WaitStop() { _m.Called() From 1c7c1e4bb35d50b319d4672c075a82dd36d73398 Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Jun 2022 17:12:13 -0400 Subject: [PATCH 2/6] Handle non-existent namespaces at the route level Since most routes now query an orchestrator from namespace manager, they should return 404 if an orchestrator can't be found (ie if namespace does not exist). Signed-off-by: Andrew Richardson --- internal/apiserver/route_spi_get_ops.go | 4 +- internal/apiserver/server.go | 39 ++++++++++++---- internal/apiserver/server_test.go | 62 ++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/internal/apiserver/route_spi_get_ops.go b/internal/apiserver/route_spi_get_ops.go index b96f3b29ac..2c7ba066e6 100644 --- a/internal/apiserver/route_spi_get_ops.go +++ b/internal/apiserver/route_spi_get_ops.go @@ -34,11 +34,11 @@ var spiGetOps = &ffapi.Route{ JSONInputValue: nil, JSONOutputValue: func() interface{} { return []*core.Operation{} }, JSONOutputCodes: []int{http.StatusOK}, + Tag: routeTagNonDefaultNamespace, Extensions: &coreExtensions{ FilterFactory: database.OperationQueryFactory, CoreJSONHandler: func(r *ffapi.APIRequest, cr *coreRequest) (output interface{}, err error) { - or := cr.mgr.Orchestrator(r.PP["ns"]) - return filterResult(or.GetOperations(cr.ctx, cr.filter)) + return filterResult(cr.or.GetOperations(cr.ctx, cr.filter)) }, }, } diff --git a/internal/apiserver/server.go b/internal/apiserver/server.go index 32dcdb65e8..ec39c9c6a2 100644 --- a/internal/apiserver/server.go +++ b/internal/apiserver/server.go @@ -200,7 +200,11 @@ func (as *apiServer) swaggerGenerator(routes []*ffapi.Route, apiBaseURL string) func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL string) func(req *http.Request) (*openapi3.T, error) { return func(req *http.Request) (*openapi3.T, error) { vars := mux.Vars(req) - cm := mgr.Orchestrator(vars["ns"]).Contracts() + or := mgr.Orchestrator(vars["ns"]) + if or == nil { + return nil, i18n.NewError(req.Context(), coremsgs.Msg404NotFound) + } + cm := or.Contracts() api, err := cm.GetContractAPI(req.Context(), apiBaseURL, vars["apiName"]) if err != nil { return nil, err @@ -218,17 +222,22 @@ func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL } } -func getOrchestrator(mgr namespace.Manager, tag string, r *ffapi.APIRequest) orchestrator.Orchestrator { - if tag == routeTagDefaultNamespace { - return mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault)) - } - if tag == routeTagNonDefaultNamespace { +func getOrchestrator(ctx context.Context, mgr namespace.Manager, tag string, r *ffapi.APIRequest) (or orchestrator.Orchestrator, err error) { + switch { + case tag == routeTagDefaultNamespace: + or = mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault)) + case tag == routeTagNonDefaultNamespace: vars := mux.Vars(r.Req) if ns, ok := vars["ns"]; ok { - return mgr.Orchestrator(ns) + or = mgr.Orchestrator(ns) } + default: + return nil, nil + } + if or == nil { + return nil, i18n.NewError(ctx, coremsgs.Msg404NotFound) } - return nil + return or, nil } func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manager, apiBaseURL string, route *ffapi.Route) http.HandlerFunc { @@ -244,9 +253,14 @@ func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manage } } + or, err := getOrchestrator(r.Req.Context(), mgr, route.Tag, r) + if err != nil { + return nil, err + } + cr := &coreRequest{ mgr: mgr, - or: getOrchestrator(mgr, route.Tag, r), + or: or, ctx: r.Req.Context(), filter: filter, apiBaseURL: apiBaseURL, @@ -255,9 +269,14 @@ func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manage } if ce.CoreFormUploadHandler != nil { route.FormUploadHandler = func(r *ffapi.APIRequest) (output interface{}, err error) { + or, err := getOrchestrator(r.Req.Context(), mgr, route.Tag, r) + if err != nil { + return nil, err + } + cr := &coreRequest{ mgr: mgr, - or: getOrchestrator(mgr, route.Tag, r), + or: or, ctx: r.Req.Context(), apiBaseURL: apiBaseURL, } diff --git a/internal/apiserver/server_test.go b/internal/apiserver/server_test.go index 8600db82a3..38da684aa6 100644 --- a/internal/apiserver/server_test.go +++ b/internal/apiserver/server_test.go @@ -17,10 +17,12 @@ package apiserver import ( + "bytes" "context" "encoding/json" "fmt" "io/ioutil" + "mime/multipart" "net/http" "net/http/httptest" "testing" @@ -50,7 +52,9 @@ func newTestServer() (*namespacemocks.Manager, *orchestratormocks.Orchestrator, InitConfig() mgr := &namespacemocks.Manager{} o := &orchestratormocks.Orchestrator{} - mgr.On("Orchestrator", mock.Anything).Return(o) + mgr.On("Orchestrator", "default").Return(o).Maybe() + mgr.On("Orchestrator", "mynamespace").Return(o).Maybe() + mgr.On("Orchestrator", "ns1").Return(o).Maybe() as := &apiServer{ apiTimeout: 5 * time.Second, maxFilterLimit: 100, @@ -322,6 +326,21 @@ func TestContractAPISwaggerJSONGetFFIFail(t *testing.T) { assert.Equal(t, 500, res.StatusCode) } +func TestContractAPISwaggerJSONBadNamespace(t *testing.T) { + mgr, o, as := newTestServer() + r := as.createMuxRouter(context.Background(), mgr) + mcm := &contractmocks.Manager{} + o.On("Contracts").Return(mcm) + s := httptest.NewServer(r) + defer s.Close() + + mgr.On("Orchestrator", "BAD").Return(nil) + + res, err := http.Get(fmt.Sprintf("http://%s/api/v1/namespaces/BAD/apis/my-api/api/swagger.json", s.Listener.Addr())) + assert.NoError(t, err) + assert.Equal(t, 404, res.StatusCode) +} + func TestContractAPISwaggerUI(t *testing.T) { _, r := newTestAPIServer() s := httptest.NewServer(r) @@ -333,3 +352,44 @@ func TestContractAPISwaggerUI(t *testing.T) { b, _ := ioutil.ReadAll(res.Body) assert.Regexp(t, "html", string(b)) } + +func TestJSONBadNamespace(t *testing.T) { + mgr, _, as := newTestServer() + r := as.createMuxRouter(context.Background(), mgr) + s := httptest.NewServer(r) + defer s.Close() + + mgr.On("Orchestrator", "BAD").Return(nil) + + var b bytes.Buffer + req := httptest.NewRequest("GET", "/api/v1/namespaces/BAD/apis", &b) + req.Header.Set("Content-Type", "application/json; charset=utf-8") + res := httptest.NewRecorder() + + r.ServeHTTP(res, req) + + assert.Equal(t, 404, res.Result().StatusCode) +} + +func TestFormDataBadNamespace(t *testing.T) { + mgr, _, as := newTestServer() + r := as.createMuxRouter(context.Background(), mgr) + s := httptest.NewServer(r) + defer s.Close() + + mgr.On("Orchestrator", "BAD").Return(nil) + + var b bytes.Buffer + w := multipart.NewWriter(&b) + writer, err := w.CreateFormFile("file", "filename.ext") + assert.NoError(t, err) + writer.Write([]byte(`some data`)) + w.Close() + req := httptest.NewRequest("POST", "/api/v1/namespaces/BAD/data", &b) + req.Header.Set("Content-Type", w.FormDataContentType()) + res := httptest.NewRecorder() + + r.ServeHTTP(res, req) + + assert.Equal(t, 404, res.Result().StatusCode) +} From 9c232ab9b6b8f8772e55205b34c4a54aa93898fa Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Jun 2022 20:27:42 -0400 Subject: [PATCH 3/6] Use a better error for non-existent namespace Signed-off-by: Andrew Richardson --- internal/apiserver/server.go | 4 ++-- internal/coremsgs/en_error_messages.go | 2 +- internal/events/websockets/websockets.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/apiserver/server.go b/internal/apiserver/server.go index ec39c9c6a2..1d117078b4 100644 --- a/internal/apiserver/server.go +++ b/internal/apiserver/server.go @@ -202,7 +202,7 @@ func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL vars := mux.Vars(req) or := mgr.Orchestrator(vars["ns"]) if or == nil { - return nil, i18n.NewError(req.Context(), coremsgs.Msg404NotFound) + return nil, i18n.NewError(req.Context(), coremsgs.MsgNamespaceDoesNotExist) } cm := or.Contracts() api, err := cm.GetContractAPI(req.Context(), apiBaseURL, vars["apiName"]) @@ -235,7 +235,7 @@ func getOrchestrator(ctx context.Context, mgr namespace.Manager, tag string, r * return nil, nil } if or == nil { - return nil, i18n.NewError(ctx, coremsgs.Msg404NotFound) + return nil, i18n.NewError(ctx, coremsgs.MsgNamespaceDoesNotExist) } return or, nil } diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index 9de4529580..64dfd5840a 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -86,7 +86,7 @@ var ( MsgMaxFilterLimit = ffe("FF10184", "Your query exceeds the maximum filter limit (%d)", 400) MsgAPIServerStaticFail = ffe("FF10185", "An error occurred loading static content", 500) MsgEventListenerClosing = ffe("FF10186", "Event listener closing") - MsgNamespaceNotExist = ffe("FF10187", "Namespace does not exist") + MsgNamespaceDoesNotExist = ffe("FF10187", "Namespace does not exist", 404) MsgInvalidSubscription = ffe("FF10189", "Invalid subscription", 400) MsgMismatchedTransport = ffe("FF10190", "Connection ID '%s' appears not to be unique between transport '%s' and '%s'", 400) MsgInvalidFirstEvent = ffe("FF10191", "Invalid firstEvent definition - must be 'newest','oldest' or a sequence number", 400) diff --git a/internal/events/websockets/websockets.go b/internal/events/websockets/websockets.go index 8b3d0ecc47..25f8256453 100644 --- a/internal/events/websockets/websockets.go +++ b/internal/events/websockets/websockets.go @@ -122,7 +122,7 @@ func (ws *WebSockets) start(wc *websocketConnection, start *core.WSStart) error return wc.durableSubMatcher(sr) }) } - return i18n.NewError(ws.ctx, coremsgs.MsgNamespaceNotExist) + return i18n.NewError(ws.ctx, coremsgs.MsgNamespaceDoesNotExist) } func (ws *WebSockets) connClosed(connID string) { From a56a5634fe03d92e62c19e1bf7924f1341081fbb Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Jun 2022 20:40:04 -0400 Subject: [PATCH 4/6] Slightly reorder some init in orchestrator Signed-off-by: Andrew Richardson --- internal/identity/identitymanager.go | 7 ++--- internal/identity/identitymanager_test.go | 6 ++--- internal/networkmap/manager.go | 7 ++--- internal/networkmap/manager_test.go | 6 ++--- internal/orchestrator/orchestrator.go | 30 +++++++++++----------- internal/orchestrator/orchestrator_test.go | 2 +- 6 files changed, 24 insertions(+), 34 deletions(-) diff --git a/internal/identity/identitymanager.go b/internal/identity/identitymanager.go index 26844067ed..70f37ad2ad 100644 --- a/internal/identity/identitymanager.go +++ b/internal/identity/identitymanager.go @@ -28,7 +28,6 @@ import ( "github.com/hyperledger/firefly-common/pkg/log" "github.com/hyperledger/firefly/internal/coreconfig" "github.com/hyperledger/firefly/internal/coremsgs" - "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/multiparty" "github.com/hyperledger/firefly/pkg/blockchain" "github.com/hyperledger/firefly/pkg/core" @@ -58,7 +57,6 @@ type Manager interface { type identityManager struct { database database.Plugin blockchain blockchain.Plugin - data data.Manager multiparty multiparty.Manager namespace string defaultKey string @@ -72,14 +70,13 @@ type identityManager struct { signingKeyCache *ccache.Cache } -func NewIdentityManager(ctx context.Context, ns, defaultKey, orgName, orgKey string, di database.Plugin, bi blockchain.Plugin, dm data.Manager, mp multiparty.Manager) (Manager, error) { - if di == nil || bi == nil || dm == nil { +func NewIdentityManager(ctx context.Context, ns, defaultKey, orgName, orgKey string, di database.Plugin, bi blockchain.Plugin, mp multiparty.Manager) (Manager, error) { + if di == nil || bi == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "IdentityManager") } im := &identityManager{ database: di, blockchain: bi, - data: dm, namespace: ns, multiparty: mp, defaultKey: defaultKey, diff --git a/internal/identity/identitymanager_test.go b/internal/identity/identitymanager_test.go index b65fcf5100..3ed3bc8989 100644 --- a/internal/identity/identitymanager_test.go +++ b/internal/identity/identitymanager_test.go @@ -25,7 +25,6 @@ import ( "github.com/hyperledger/firefly/internal/coreconfig" "github.com/hyperledger/firefly/mocks/blockchainmocks" "github.com/hyperledger/firefly/mocks/databasemocks" - "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/multipartymocks" "github.com/hyperledger/firefly/pkg/core" "github.com/stretchr/testify/assert" @@ -37,19 +36,18 @@ func newTestIdentityManager(t *testing.T) (context.Context, *identityManager) { mdi := &databasemocks.Plugin{} mbi := &blockchainmocks.Plugin{} - mdm := &datamocks.Manager{} mmp := &multipartymocks.Manager{} mbi.On("VerifierType").Return(core.VerifierTypeEthAddress).Maybe() ctx := context.Background() - im, err := NewIdentityManager(ctx, "ns1", "", "", "", mdi, mbi, mdm, mmp) + im, err := NewIdentityManager(ctx, "ns1", "", "", "", mdi, mbi, mmp) assert.NoError(t, err) return ctx, im.(*identityManager) } func TestNewIdentityManagerMissingDeps(t *testing.T) { - _, err := NewIdentityManager(context.Background(), "", "", "", "", nil, nil, nil, nil) + _, err := NewIdentityManager(context.Background(), "", "", "", "", nil, nil, nil) assert.Regexp(t, "FF10128", err) } diff --git a/internal/networkmap/manager.go b/internal/networkmap/manager.go index 7156043a87..d3734319bd 100644 --- a/internal/networkmap/manager.go +++ b/internal/networkmap/manager.go @@ -22,7 +22,6 @@ import ( "github.com/hyperledger/firefly-common/pkg/i18n" "github.com/hyperledger/firefly/internal/broadcast" "github.com/hyperledger/firefly/internal/coremsgs" - "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/identity" "github.com/hyperledger/firefly/internal/syncasync" "github.com/hyperledger/firefly/pkg/core" @@ -61,15 +60,14 @@ type networkMap struct { orgName string orgDesc string database database.Plugin - data data.Manager broadcast broadcast.Manager exchange dataexchange.Plugin identity identity.Manager syncasync syncasync.Bridge } -func NewNetworkMap(ctx context.Context, ns, orgName, orgDesc string, di database.Plugin, dm data.Manager, bm broadcast.Manager, dx dataexchange.Plugin, im identity.Manager, sa syncasync.Bridge) (Manager, error) { - if di == nil || dm == nil || bm == nil || dx == nil || im == nil { +func NewNetworkMap(ctx context.Context, ns, orgName, orgDesc string, di database.Plugin, bm broadcast.Manager, dx dataexchange.Plugin, im identity.Manager, sa syncasync.Bridge) (Manager, error) { + if di == nil || bm == nil || dx == nil || im == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "NetworkMap") } @@ -79,7 +77,6 @@ func NewNetworkMap(ctx context.Context, ns, orgName, orgDesc string, di database orgName: orgName, orgDesc: orgDesc, database: di, - data: dm, broadcast: bm, exchange: dx, identity: im, diff --git a/internal/networkmap/manager_test.go b/internal/networkmap/manager_test.go index b6e9bb67d8..ba986bb6aa 100644 --- a/internal/networkmap/manager_test.go +++ b/internal/networkmap/manager_test.go @@ -24,7 +24,6 @@ import ( "github.com/hyperledger/firefly/mocks/broadcastmocks" "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/dataexchangemocks" - "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" "github.com/stretchr/testify/assert" @@ -34,18 +33,17 @@ func newTestNetworkmap(t *testing.T) (*networkMap, func()) { coreconfig.Reset() ctx, cancel := context.WithCancel(context.Background()) mdi := &databasemocks.Plugin{} - mdm := &datamocks.Manager{} mbm := &broadcastmocks.Manager{} mdx := &dataexchangemocks.Plugin{} mim := &identitymanagermocks.Manager{} msa := &syncasyncmocks.Bridge{} - nm, err := NewNetworkMap(ctx, "ns1", "org0", "org0", mdi, mdm, mbm, mdx, mim, msa) + nm, err := NewNetworkMap(ctx, "ns1", "org0", "org0", mdi, mbm, mdx, mim, msa) assert.NoError(t, err) return nm.(*networkMap), cancel } func TestNewNetworkMapMissingDep(t *testing.T) { - _, err := NewNetworkMap(context.Background(), "", "", "", nil, nil, nil, nil, nil, nil) + _, err := NewNetworkMap(context.Background(), "", "", "", nil, nil, nil, nil, nil) assert.Regexp(t, "FF10128", err) } diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 220830f7cf..dc1459be89 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -395,6 +395,13 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { or.multiparty = multiparty.NewMultipartyManager(or.ctx, or.namespace, or.config.Multiparty.Contracts, or.blockchain()) } + if or.identity == nil { + or.identity, err = identity.NewIdentityManager(ctx, or.namespace, or.config.DefaultKey, or.config.Multiparty.OrgName, or.config.Multiparty.OrgKey, or.database(), or.blockchain(), or.multiparty) + if err != nil { + return err + } + } + if or.data == nil { or.data, err = data.NewDataManager(ctx, or.namespace, or.database(), or.sharedstorage(), or.dataexchange()) if err != nil { @@ -406,13 +413,14 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { or.txHelper = txcommon.NewTransactionHelper(or.namespace, or.database(), or.data) } - if or.identity == nil { - or.identity, err = identity.NewIdentityManager(ctx, or.namespace, or.config.DefaultKey, or.config.Multiparty.OrgName, or.config.Multiparty.OrgKey, or.database(), or.blockchain(), or.data, or.multiparty) - if err != nil { + if or.operations == nil { + if or.operations, err = operations.NewOperationsManager(ctx, or.namespace, or.database(), or.txHelper); err != nil { return err } } + or.syncasync = syncasync.NewSyncAsyncBridge(ctx, or.namespace, or.database(), or.data) + if or.batch == nil { or.batch, err = batch.NewBatchManager(ctx, or.namespace, or, or.database(), or.data, or.txHelper) if err != nil { @@ -420,14 +428,6 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { } } - if or.operations == nil { - if or.operations, err = operations.NewOperationsManager(ctx, or.namespace, or.database(), or.txHelper); err != nil { - return err - } - } - - or.syncasync = syncasync.NewSyncAsyncBridge(ctx, or.namespace, or.database(), or.data) - if or.batchpin == nil { if or.batchpin, err = batchpin.NewBatchPinSubmitter(ctx, or.namespace, or.database(), or.identity, or.multiparty, or.metrics, or.operations); err != nil { return err @@ -446,6 +446,10 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { } } + if or.networkmap == nil { + or.networkmap, err = networkmap.NewNetworkMap(ctx, or.namespace, or.config.Multiparty.OrgName, or.config.Multiparty.OrgDesc, or.database(), or.broadcast, or.dataexchange(), or.identity, or.syncasync) + } + if or.assets == nil { or.assets, err = assets.NewAssetManager(ctx, or.namespace, or.database(), or.identity, or.syncasync, or.broadcast, or.messaging, or.tokens(), or.metrics, or.operations, or.txHelper) if err != nil { @@ -482,10 +486,6 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { } or.syncasync.Init(or.events) - - if or.networkmap == nil { - or.networkmap, err = networkmap.NewNetworkMap(ctx, or.namespace, or.config.Multiparty.OrgName, or.config.Multiparty.OrgDesc, or.database(), or.data, or.broadcast, or.dataexchange(), or.identity, or.syncasync) - } return err } diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 3c3d61bd64..4cb9b99621 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -315,7 +315,6 @@ func TestInitIdentityComponentFail(t *testing.T) { defer or.cleanup(t) or.plugins.Database.Plugin = nil or.identity = nil - or.txHelper = nil or.multiparty = nil err := or.initComponents(context.Background()) assert.Regexp(t, "FF10128", err) @@ -362,6 +361,7 @@ func TestInitOperationsComponentFail(t *testing.T) { defer or.cleanup(t) or.plugins.Database.Plugin = nil or.operations = nil + or.txHelper = nil err := or.initComponents(context.Background()) assert.Regexp(t, "FF10128", err) } From 1ff243a84429f2808c9f33f3e433894c5fc5831f Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Wed, 22 Jun 2022 20:46:20 -0400 Subject: [PATCH 5/6] Reorder manager params to put plugins first Signed-off-by: Andrew Richardson --- internal/assets/manager.go | 2 +- internal/assets/manager_test.go | 2 +- internal/broadcast/manager.go | 2 +- internal/broadcast/manager_test.go | 2 +- internal/contracts/manager.go | 2 +- internal/contracts/manager_test.go | 6 +++--- internal/networkmap/manager.go | 2 +- internal/networkmap/manager_test.go | 2 +- internal/orchestrator/orchestrator.go | 10 +++++----- internal/privatemessaging/privatemessaging.go | 2 +- internal/privatemessaging/privatemessaging_test.go | 2 +- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/assets/manager.go b/internal/assets/manager.go index f6e07ff436..6c028c1f8d 100644 --- a/internal/assets/manager.go +++ b/internal/assets/manager.go @@ -84,7 +84,7 @@ type assetManager struct { keyNormalization int } -func NewAssetManager(ctx context.Context, ns string, di database.Plugin, im identity.Manager, sa syncasync.Bridge, bm broadcast.Manager, pm privatemessaging.Manager, ti map[string]tokens.Plugin, mm metrics.Manager, om operations.Manager, txHelper txcommon.Helper) (Manager, error) { +func NewAssetManager(ctx context.Context, ns string, di database.Plugin, ti map[string]tokens.Plugin, im identity.Manager, sa syncasync.Bridge, bm broadcast.Manager, pm privatemessaging.Manager, mm metrics.Manager, om operations.Manager, txHelper txcommon.Helper) (Manager, error) { if di == nil || im == nil || sa == nil || bm == nil || pm == nil || ti == nil || mm == nil || om == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "AssetManager") } diff --git a/internal/assets/manager_test.go b/internal/assets/manager_test.go index 9f2ae349ea..6cae98b09d 100644 --- a/internal/assets/manager_test.go +++ b/internal/assets/manager_test.go @@ -63,7 +63,7 @@ func newTestAssetsCommon(t *testing.T, metrics bool) (*assetManager, func()) { mom.On("RegisterHandler", mock.Anything, mock.Anything, mock.Anything) mti.On("Name").Return("ut").Maybe() ctx, cancel := context.WithCancel(context.Background()) - a, err := NewAssetManager(ctx, "ns1", mdi, mim, msa, mbm, mpm, map[string]tokens.Plugin{"magic-tokens": mti}, mm, mom, txHelper) + a, err := NewAssetManager(ctx, "ns1", mdi, map[string]tokens.Plugin{"magic-tokens": mti}, mim, msa, mbm, mpm, mm, mom, txHelper) rag := mdi.On("RunAsGroup", mock.Anything, mock.Anything).Maybe() rag.RunFn = func(a mock.Arguments) { rag.ReturnArguments = mock.Arguments{a[1].(func(context.Context) error)(a[0].(context.Context))} diff --git a/internal/broadcast/manager.go b/internal/broadcast/manager.go index 580e549154..13dc0c7de4 100644 --- a/internal/broadcast/manager.go +++ b/internal/broadcast/manager.go @@ -76,7 +76,7 @@ type broadcastManager struct { operations operations.Manager } -func NewBroadcastManager(ctx context.Context, ns string, di database.Plugin, im identity.Manager, dm data.Manager, bi blockchain.Plugin, dx dataexchange.Plugin, si sharedstorage.Plugin, ba batch.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) { +func NewBroadcastManager(ctx context.Context, ns string, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, si sharedstorage.Plugin, im identity.Manager, dm data.Manager, ba batch.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) { if di == nil || im == nil || dm == nil || bi == nil || dx == nil || si == nil || ba == nil || mm == nil || om == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "BroadcastManager") } diff --git a/internal/broadcast/manager_test.go b/internal/broadcast/manager_test.go index 19c2107440..a8eff6734a 100644 --- a/internal/broadcast/manager_test.go +++ b/internal/broadcast/manager_test.go @@ -76,7 +76,7 @@ func newTestBroadcastCommon(t *testing.T, metricsEnabled bool) (*broadcastManage } ctx, cancel := context.WithCancel(context.Background()) - b, err := NewBroadcastManager(ctx, "ns1", mdi, mim, mdm, mbi, mdx, mpi, mba, msa, mbp, mmi, mom) + b, err := NewBroadcastManager(ctx, "ns1", mdi, mbi, mdx, mpi, mim, mdm, mba, msa, mbp, mmi, mom) assert.NoError(t, err) return b.(*broadcastManager), cancel } diff --git a/internal/contracts/manager.go b/internal/contracts/manager.go index 6cc9c3e2ed..dd8ba33c6c 100644 --- a/internal/contracts/manager.go +++ b/internal/contracts/manager.go @@ -79,7 +79,7 @@ type contractManager struct { syncasync syncasync.Bridge } -func NewContractManager(ctx context.Context, ns string, di database.Plugin, bm broadcast.Manager, im identity.Manager, bi blockchain.Plugin, om operations.Manager, txHelper txcommon.Helper, sa syncasync.Bridge) (Manager, error) { +func NewContractManager(ctx context.Context, ns string, di database.Plugin, bi blockchain.Plugin, bm broadcast.Manager, im identity.Manager, om operations.Manager, txHelper txcommon.Helper, sa syncasync.Bridge) (Manager, error) { if di == nil || bm == nil || im == nil || bi == nil || om == nil || txHelper == nil || sa == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "ContractManager") } diff --git a/internal/contracts/manager_test.go b/internal/contracts/manager_test.go index 743d95268d..41b62c2108 100644 --- a/internal/contracts/manager_test.go +++ b/internal/contracts/manager_test.go @@ -61,7 +61,7 @@ func newTestContractManager() *contractManager { a[1].(func(context.Context) error)(a[0].(context.Context)), } } - cm, _ := NewContractManager(context.Background(), "ns1", mdi, mbm, mim, mbi, mom, txHelper, msa) + cm, _ := NewContractManager(context.Background(), "ns1", mdi, mbi, mbm, mim, mom, txHelper, msa) cm.(*contractManager).txHelper = &txcommonmocks.Helper{} return cm.(*contractManager) } @@ -86,7 +86,7 @@ func TestNewContractManagerFFISchemaLoaderFail(t *testing.T) { txHelper := txcommon.NewTransactionHelper("ns1", mdi, mdm) msa := &syncasyncmocks.Bridge{} mbi.On("GetFFIParamValidator", mock.Anything).Return(nil, fmt.Errorf("pop")) - _, err := NewContractManager(context.Background(), "ns1", mdi, mbm, mim, mbi, mom, txHelper, msa) + _, err := NewContractManager(context.Background(), "ns1", mdi, mbi, mbm, mim, mom, txHelper, msa) assert.Regexp(t, "pop", err) } @@ -101,7 +101,7 @@ func TestNewContractManagerFFISchemaLoader(t *testing.T) { msa := &syncasyncmocks.Bridge{} mbi.On("GetFFIParamValidator", mock.Anything).Return(ðereum.FFIParamValidator{}, nil) mom.On("RegisterHandler", mock.Anything, mock.Anything, mock.Anything) - _, err := NewContractManager(context.Background(), "ns1", mdi, mbm, mim, mbi, mom, txHelper, msa) + _, err := NewContractManager(context.Background(), "ns1", mdi, mbi, mbm, mim, mom, txHelper, msa) assert.NoError(t, err) } diff --git a/internal/networkmap/manager.go b/internal/networkmap/manager.go index d3734319bd..e72da55581 100644 --- a/internal/networkmap/manager.go +++ b/internal/networkmap/manager.go @@ -66,7 +66,7 @@ type networkMap struct { syncasync syncasync.Bridge } -func NewNetworkMap(ctx context.Context, ns, orgName, orgDesc string, di database.Plugin, bm broadcast.Manager, dx dataexchange.Plugin, im identity.Manager, sa syncasync.Bridge) (Manager, error) { +func NewNetworkMap(ctx context.Context, ns, orgName, orgDesc string, di database.Plugin, dx dataexchange.Plugin, bm broadcast.Manager, im identity.Manager, sa syncasync.Bridge) (Manager, error) { if di == nil || bm == nil || dx == nil || im == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "NetworkMap") } diff --git a/internal/networkmap/manager_test.go b/internal/networkmap/manager_test.go index ba986bb6aa..e033628087 100644 --- a/internal/networkmap/manager_test.go +++ b/internal/networkmap/manager_test.go @@ -37,7 +37,7 @@ func newTestNetworkmap(t *testing.T) (*networkMap, func()) { mdx := &dataexchangemocks.Plugin{} mim := &identitymanagermocks.Manager{} msa := &syncasyncmocks.Bridge{} - nm, err := NewNetworkMap(ctx, "ns1", "org0", "org0", mdi, mbm, mdx, mim, msa) + nm, err := NewNetworkMap(ctx, "ns1", "org0", "org0", mdi, mdx, mbm, mim, msa) assert.NoError(t, err) return nm.(*networkMap), cancel diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index dc1459be89..468540e96e 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -435,30 +435,30 @@ func (or *orchestrator) initComponents(ctx context.Context) (err error) { } if or.messaging == nil { - if or.messaging, err = privatemessaging.NewPrivateMessaging(ctx, or.namespace, or.database(), or.identity, or.dataexchange(), or.blockchain(), or.batch, or.data, or.syncasync, or.batchpin, or.metrics, or.operations); err != nil { + if or.messaging, err = privatemessaging.NewPrivateMessaging(ctx, or.namespace, or.database(), or.dataexchange(), or.blockchain(), or.identity, or.batch, or.data, or.syncasync, or.batchpin, or.metrics, or.operations); err != nil { return err } } if or.broadcast == nil { - if or.broadcast, err = broadcast.NewBroadcastManager(ctx, or.namespace, or.database(), or.identity, or.data, or.blockchain(), or.dataexchange(), or.sharedstorage(), or.batch, or.syncasync, or.batchpin, or.metrics, or.operations); err != nil { + if or.broadcast, err = broadcast.NewBroadcastManager(ctx, or.namespace, or.database(), or.blockchain(), or.dataexchange(), or.sharedstorage(), or.identity, or.data, or.batch, or.syncasync, or.batchpin, or.metrics, or.operations); err != nil { return err } } if or.networkmap == nil { - or.networkmap, err = networkmap.NewNetworkMap(ctx, or.namespace, or.config.Multiparty.OrgName, or.config.Multiparty.OrgDesc, or.database(), or.broadcast, or.dataexchange(), or.identity, or.syncasync) + or.networkmap, err = networkmap.NewNetworkMap(ctx, or.namespace, or.config.Multiparty.OrgName, or.config.Multiparty.OrgDesc, or.database(), or.dataexchange(), or.broadcast, or.identity, or.syncasync) } if or.assets == nil { - or.assets, err = assets.NewAssetManager(ctx, or.namespace, or.database(), or.identity, or.syncasync, or.broadcast, or.messaging, or.tokens(), or.metrics, or.operations, or.txHelper) + or.assets, err = assets.NewAssetManager(ctx, or.namespace, or.database(), or.tokens(), or.identity, or.syncasync, or.broadcast, or.messaging, or.metrics, or.operations, or.txHelper) if err != nil { return err } } if or.contracts == nil { - or.contracts, err = contracts.NewContractManager(ctx, or.namespace, or.database(), or.broadcast, or.identity, or.blockchain(), or.operations, or.txHelper, or.syncasync) + or.contracts, err = contracts.NewContractManager(ctx, or.namespace, or.database(), or.blockchain(), or.broadcast, or.identity, or.operations, or.txHelper, or.syncasync) if err != nil { return err } diff --git a/internal/privatemessaging/privatemessaging.go b/internal/privatemessaging/privatemessaging.go index d855de4f39..33be7d7e92 100644 --- a/internal/privatemessaging/privatemessaging.go +++ b/internal/privatemessaging/privatemessaging.go @@ -86,7 +86,7 @@ type blobTransferTracker struct { op *core.PreparedOperation } -func NewPrivateMessaging(ctx context.Context, ns string, di database.Plugin, im identity.Manager, dx dataexchange.Plugin, bi blockchain.Plugin, ba batch.Manager, dm data.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) { +func NewPrivateMessaging(ctx context.Context, ns string, di database.Plugin, dx dataexchange.Plugin, bi blockchain.Plugin, im identity.Manager, ba batch.Manager, dm data.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) { if di == nil || im == nil || dx == nil || bi == nil || ba == nil || dm == nil || mm == nil || om == nil { return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "PrivateMessaging") } diff --git a/internal/privatemessaging/privatemessaging_test.go b/internal/privatemessaging/privatemessaging_test.go index 825751d103..118bb379b6 100644 --- a/internal/privatemessaging/privatemessaging_test.go +++ b/internal/privatemessaging/privatemessaging_test.go @@ -77,7 +77,7 @@ func newTestPrivateMessagingCommon(t *testing.T, metricsEnabled bool) (*privateM mom.On("RegisterHandler", mock.Anything, mock.Anything, mock.Anything) ctx, cancel := context.WithCancel(context.Background()) - pm, err := NewPrivateMessaging(ctx, "ns1", mdi, mim, mdx, mbi, mba, mdm, msa, mbp, mmi, mom) + pm, err := NewPrivateMessaging(ctx, "ns1", mdi, mdx, mbi, mim, mba, mdm, msa, mbp, mmi, mom) assert.NoError(t, err) // Default mocks to save boilerplate in the tests From fc5bb3c48f7c0d199b309d90763bc98cd413c10f Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Fri, 24 Jun 2022 09:19:42 -0400 Subject: [PATCH 6/6] Clean up switch statement Signed-off-by: Andrew Richardson --- internal/apiserver/server.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/apiserver/server.go b/internal/apiserver/server.go index 1d117078b4..f9bc9a50ef 100644 --- a/internal/apiserver/server.go +++ b/internal/apiserver/server.go @@ -223,10 +223,10 @@ func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL } func getOrchestrator(ctx context.Context, mgr namespace.Manager, tag string, r *ffapi.APIRequest) (or orchestrator.Orchestrator, err error) { - switch { - case tag == routeTagDefaultNamespace: + switch tag { + case routeTagDefaultNamespace: or = mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault)) - case tag == routeTagNonDefaultNamespace: + case routeTagNonDefaultNamespace: vars := mux.Vars(r.Req) if ns, ok := vars["ns"]; ok { or = mgr.Orchestrator(ns)