From 92667ea86e13a4d29a54d733e39772a1eb3d126c Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Fri, 18 Mar 2022 17:12:15 -0400 Subject: [PATCH] Change node/org lookups to support name or ID See #583 Signed-off-by: Andrew Richardson --- docs/swagger/swagger.yaml | 8 +-- internal/apiserver/route_get_net_node.go | 6 +- internal/apiserver/route_get_net_node_test.go | 2 +- internal/apiserver/route_get_net_org.go | 6 +- internal/apiserver/route_get_net_org_test.go | 2 +- internal/networkmap/data_query.go | 46 +++++++------- internal/networkmap/data_query_test.go | 60 ++++++++++++------- internal/networkmap/manager.go | 4 +- mocks/networkmapmocks/manager.go | 20 +++---- 9 files changed, 88 insertions(+), 66 deletions(-) diff --git a/docs/swagger/swagger.yaml b/docs/swagger/swagger.yaml index cdb705564c..1076c8176c 100644 --- a/docs/swagger/swagger.yaml +++ b/docs/swagger/swagger.yaml @@ -9191,14 +9191,14 @@ paths: description: Success default: description: "" - /network/nodes/{nid}: + /network/nodes/{nameOrId}: get: description: 'TODO: Description' operationId: getNetworkNode parameters: - description: 'TODO: Description' in: path - name: nid + name: nameOrId required: true schema: type: string @@ -9592,14 +9592,14 @@ paths: description: Success default: description: "" - /network/organizations/{oid}: + /network/organizations/{nameOrId}: get: description: 'TODO: Description' operationId: getNetworkOrg parameters: - description: 'TODO: Description' in: path - name: oid + name: nameOrId required: true schema: type: string diff --git a/internal/apiserver/route_get_net_node.go b/internal/apiserver/route_get_net_node.go index 3554f35a5a..8a4edc72af 100644 --- a/internal/apiserver/route_get_net_node.go +++ b/internal/apiserver/route_get_net_node.go @@ -26,10 +26,10 @@ import ( var getNetworkNode = &oapispec.Route{ Name: "getNetworkNode", - Path: "network/nodes/{nid}", + Path: "network/nodes/{nameOrId}", Method: http.MethodGet, PathParams: []*oapispec.PathParam{ - {Name: "nid", Description: i18n.MsgTBD}, + {Name: "nameOrId", Description: i18n.MsgTBD}, }, QueryParams: nil, FilterFactory: nil, @@ -38,7 +38,7 @@ var getNetworkNode = &oapispec.Route{ JSONOutputValue: func() interface{} { return &fftypes.Identity{} }, JSONOutputCodes: []int{http.StatusOK}, JSONHandler: func(r *oapispec.APIRequest) (output interface{}, err error) { - output, err = getOr(r.Ctx).NetworkMap().GetNodeByID(r.Ctx, r.PP["nid"]) + output, err = getOr(r.Ctx).NetworkMap().GetNodeByNameOrID(r.Ctx, r.PP["nameOrId"]) return output, err }, } diff --git a/internal/apiserver/route_get_net_node_test.go b/internal/apiserver/route_get_net_node_test.go index 194e8cabc5..b988d9f5a4 100644 --- a/internal/apiserver/route_get_net_node_test.go +++ b/internal/apiserver/route_get_net_node_test.go @@ -34,7 +34,7 @@ func TestGetNode(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - nmn.On("GetNodeByID", mock.Anything, "node12345"). + nmn.On("GetNodeByNameOrID", mock.Anything, "node12345"). Return(&fftypes.Identity{}, nil) r.ServeHTTP(res, req) diff --git a/internal/apiserver/route_get_net_org.go b/internal/apiserver/route_get_net_org.go index 2d8d1b68d3..77cea51356 100644 --- a/internal/apiserver/route_get_net_org.go +++ b/internal/apiserver/route_get_net_org.go @@ -26,10 +26,10 @@ import ( var getNetworkOrg = &oapispec.Route{ Name: "getNetworkOrg", - Path: "network/organizations/{oid}", + Path: "network/organizations/{nameOrId}", Method: http.MethodGet, PathParams: []*oapispec.PathParam{ - {Name: "oid", Description: i18n.MsgTBD}, + {Name: "nameOrId", Description: i18n.MsgTBD}, }, QueryParams: nil, FilterFactory: nil, @@ -38,7 +38,7 @@ var getNetworkOrg = &oapispec.Route{ JSONOutputValue: func() interface{} { return &fftypes.Identity{} }, JSONOutputCodes: []int{http.StatusOK}, JSONHandler: func(r *oapispec.APIRequest) (output interface{}, err error) { - output, err = getOr(r.Ctx).NetworkMap().GetOrganizationByID(r.Ctx, r.PP["oid"]) + output, err = getOr(r.Ctx).NetworkMap().GetOrganizationByNameOrID(r.Ctx, r.PP["nameOrId"]) return output, err }, } diff --git a/internal/apiserver/route_get_net_org_test.go b/internal/apiserver/route_get_net_org_test.go index 9dcbebd9ad..6e71be06a6 100644 --- a/internal/apiserver/route_get_net_org_test.go +++ b/internal/apiserver/route_get_net_org_test.go @@ -34,7 +34,7 @@ func TestGetOrg(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - nmn.On("GetOrganizationByID", mock.Anything, "org12345"). + nmn.On("GetOrganizationByNameOrID", mock.Anything, "org12345"). Return(&fftypes.Identity{}, nil) r.ServeHTTP(res, req) diff --git a/internal/networkmap/data_query.go b/internal/networkmap/data_query.go index 4acf915895..f2572318d5 100644 --- a/internal/networkmap/data_query.go +++ b/internal/networkmap/data_query.go @@ -25,23 +25,26 @@ import ( "github.com/hyperledger/firefly/pkg/fftypes" ) -func (nm *networkMap) GetOrganizationByID(ctx context.Context, id string) (*fftypes.Identity, error) { - u, err := fftypes.ParseUUID(ctx, id) - if err != nil { - return nil, err - } - o, err := nm.database.GetIdentityByID(ctx, u) +func (nm *networkMap) GetOrganizationByNameOrID(ctx context.Context, nameOrID string) (org *fftypes.Identity, err error) { + u, err := fftypes.ParseUUID(ctx, nameOrID) if err != nil { + if err := fftypes.ValidateFFNameField(ctx, nameOrID, "name"); err != nil { + return nil, err + } + if org, err = nm.database.GetIdentityByName(ctx, fftypes.IdentityTypeOrg, fftypes.SystemNamespace, nameOrID); err != nil { + return nil, err + } + } else if org, err = nm.database.GetIdentityByID(ctx, u); err != nil { return nil, err } - if o == nil { + if org == nil { return nil, i18n.NewError(ctx, i18n.Msg404NotFound) } - if o.Type != fftypes.IdentityTypeOrg { - log.L(ctx).Warnf("Identity '%s' (%s) is not an org identity", o.DID, o.ID) + if org.Type != fftypes.IdentityTypeOrg { + log.L(ctx).Warnf("Identity '%s' (%s) is not an org identity", org.DID, org.ID) return nil, nil } - return o, nil + return org, nil } func (nm *networkMap) GetOrganizations(ctx context.Context, filter database.AndFilter) ([]*fftypes.Identity, *database.FilterResult, error) { @@ -50,23 +53,26 @@ func (nm *networkMap) GetOrganizations(ctx context.Context, filter database.AndF return nm.database.GetIdentities(ctx, filter) } -func (nm *networkMap) GetNodeByID(ctx context.Context, id string) (*fftypes.Identity, error) { - u, err := fftypes.ParseUUID(ctx, id) - if err != nil { - return nil, err - } - n, err := nm.database.GetIdentityByID(ctx, u) +func (nm *networkMap) GetNodeByNameOrID(ctx context.Context, nameOrID string) (node *fftypes.Identity, err error) { + u, err := fftypes.ParseUUID(ctx, nameOrID) if err != nil { + if err := fftypes.ValidateFFNameField(ctx, nameOrID, "name"); err != nil { + return nil, err + } + if node, err = nm.database.GetIdentityByName(ctx, fftypes.IdentityTypeNode, fftypes.SystemNamespace, nameOrID); err != nil { + return nil, err + } + } else if node, err = nm.database.GetIdentityByID(ctx, u); err != nil { return nil, err } - if n == nil { + if node == nil { return nil, i18n.NewError(ctx, i18n.Msg404NotFound) } - if n.Type != fftypes.IdentityTypeNode { - log.L(ctx).Warnf("Identity '%s' (%s) is not a node identity", n.DID, n.ID) + if node.Type != fftypes.IdentityTypeNode { + log.L(ctx).Warnf("Identity '%s' (%s) is not a node identity", node.DID, node.ID) return nil, nil } - return n, nil + return node, nil } func (nm *networkMap) GetNodes(ctx context.Context, filter database.AndFilter) ([]*fftypes.Identity, *database.FilterResult, error) { diff --git a/internal/networkmap/data_query_test.go b/internal/networkmap/data_query_test.go index 27cdb08b35..6644766a69 100644 --- a/internal/networkmap/data_query_test.go +++ b/internal/networkmap/data_query_test.go @@ -27,98 +27,114 @@ import ( "github.com/stretchr/testify/mock" ) -func TestGetOrganizationByIDOk(t *testing.T) { +func TestGetOrganizationByNameOrIDOk(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id). Return(&fftypes.Identity{IdentityBase: fftypes.IdentityBase{ID: id, Type: fftypes.IdentityTypeOrg}}, nil) - res, err := nm.GetOrganizationByID(nm.ctx, id.String()) + res, err := nm.GetOrganizationByNameOrID(nm.ctx, id.String()) assert.NoError(t, err) assert.Equal(t, *id, *res.ID) } -func TestGetOrganizationByIDNotOrg(t *testing.T) { +func TestGetOrganizationByNameOrIDNotOrg(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id). Return(&fftypes.Identity{IdentityBase: fftypes.IdentityBase{ID: id, Type: fftypes.IdentityTypeNode}}, nil) - res, err := nm.GetOrganizationByID(nm.ctx, id.String()) + res, err := nm.GetOrganizationByNameOrID(nm.ctx, id.String()) assert.NoError(t, err) assert.Nil(t, res) } -func TestGetOrganizationByIDNotFound(t *testing.T) { +func TestGetOrganizationByNameOrIDNotFound(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id).Return(nil, nil) - _, err := nm.GetOrganizationByID(nm.ctx, id.String()) + _, err := nm.GetOrganizationByNameOrID(nm.ctx, id.String()) assert.Regexp(t, "FF10109", err) } -func TestGetOrganizationByIDError(t *testing.T) { +func TestGetOrganizationByNameOrIDError(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id).Return(nil, fmt.Errorf("pop")) - _, err := nm.GetOrganizationByID(nm.ctx, id.String()) + _, err := nm.GetOrganizationByNameOrID(nm.ctx, id.String()) assert.Regexp(t, "pop", err) } -func TestGetOrganizationByIDBadUUID(t *testing.T) { +func TestGetOrganizationByNameBadName(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() - _, err := nm.GetOrganizationByID(nm.ctx, "bad") - assert.Regexp(t, "FF10142", err) + _, err := nm.GetOrganizationByNameOrID(nm.ctx, "!bad") + assert.Regexp(t, "FF10131", err) +} + +func TestGetOrganizationByNameError(t *testing.T) { + nm, cancel := newTestNetworkmap(t) + defer cancel() + nm.database.(*databasemocks.Plugin).On("GetIdentityByName", nm.ctx, fftypes.IdentityTypeOrg, fftypes.SystemNamespace, "bad").Return(nil, fmt.Errorf("pop")) + _, err := nm.GetOrganizationByNameOrID(nm.ctx, "bad") + assert.Regexp(t, "pop", err) } -func TestGetNodeByIDOk(t *testing.T) { +func TestGetNodeByNameOrIDOk(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id). Return(&fftypes.Identity{IdentityBase: fftypes.IdentityBase{ID: id, Type: fftypes.IdentityTypeNode}}, nil) - res, err := nm.GetNodeByID(nm.ctx, id.String()) + res, err := nm.GetNodeByNameOrID(nm.ctx, id.String()) assert.NoError(t, err) assert.Equal(t, *id, *res.ID) } -func TestGetNodeByIDWrongType(t *testing.T) { +func TestGetNodeByNameOrIDWrongType(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id). Return(&fftypes.Identity{IdentityBase: fftypes.IdentityBase{ID: id, Type: fftypes.IdentityTypeOrg}}, nil) - res, err := nm.GetNodeByID(nm.ctx, id.String()) + res, err := nm.GetNodeByNameOrID(nm.ctx, id.String()) assert.NoError(t, err) assert.Nil(t, res) } -func TestGetNodeByIDNotFound(t *testing.T) { +func TestGetNodeByNameOrIDNotFound(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id).Return(nil, nil) - _, err := nm.GetNodeByID(nm.ctx, id.String()) + _, err := nm.GetNodeByNameOrID(nm.ctx, id.String()) assert.Regexp(t, "FF10109", err) } -func TestGetNodeByIDError(t *testing.T) { +func TestGetNodeByNameOrIDError(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() id := fftypes.NewUUID() nm.database.(*databasemocks.Plugin).On("GetIdentityByID", nm.ctx, id).Return(nil, fmt.Errorf("pop")) - _, err := nm.GetNodeByID(nm.ctx, id.String()) + _, err := nm.GetNodeByNameOrID(nm.ctx, id.String()) assert.Regexp(t, "pop", err) } -func TestGetNodeByIDBadUUID(t *testing.T) { +func TestGetNodeByNameBadName(t *testing.T) { nm, cancel := newTestNetworkmap(t) defer cancel() - _, err := nm.GetNodeByID(nm.ctx, "bad") - assert.Regexp(t, "FF10142", err) + _, err := nm.GetNodeByNameOrID(nm.ctx, "!bad") + assert.Regexp(t, "FF10131", err) +} + +func TestGetNodeByNameError(t *testing.T) { + nm, cancel := newTestNetworkmap(t) + defer cancel() + nm.database.(*databasemocks.Plugin).On("GetIdentityByName", nm.ctx, fftypes.IdentityTypeNode, fftypes.SystemNamespace, "bad").Return(nil, fmt.Errorf("pop")) + _, err := nm.GetNodeByNameOrID(nm.ctx, "bad") + assert.Regexp(t, "pop", err) } func TestGetOrganizations(t *testing.T) { diff --git a/internal/networkmap/manager.go b/internal/networkmap/manager.go index 6570fd7736..875652eb94 100644 --- a/internal/networkmap/manager.go +++ b/internal/networkmap/manager.go @@ -35,9 +35,9 @@ type Manager interface { RegisterIdentity(ctx context.Context, ns string, dto *fftypes.IdentityCreateDTO, waitConfirm bool) (identity *fftypes.Identity, err error) UpdateIdentity(ctx context.Context, ns string, id string, dto *fftypes.IdentityUpdateDTO, waitConfirm bool) (identity *fftypes.Identity, err error) - GetOrganizationByID(ctx context.Context, id string) (*fftypes.Identity, error) + GetOrganizationByNameOrID(ctx context.Context, nameOrID string) (*fftypes.Identity, error) GetOrganizations(ctx context.Context, filter database.AndFilter) ([]*fftypes.Identity, *database.FilterResult, error) - GetNodeByID(ctx context.Context, id string) (*fftypes.Identity, error) + GetNodeByNameOrID(ctx context.Context, nameOrID string) (*fftypes.Identity, error) GetNodes(ctx context.Context, filter database.AndFilter) ([]*fftypes.Identity, *database.FilterResult, error) GetIdentityByID(ctx context.Context, ns string, id string) (*fftypes.Identity, error) GetIdentities(ctx context.Context, ns string, filter database.AndFilter) ([]*fftypes.Identity, *database.FilterResult, error) diff --git a/mocks/networkmapmocks/manager.go b/mocks/networkmapmocks/manager.go index 7ec1c297d3..24618e70cb 100644 --- a/mocks/networkmapmocks/manager.go +++ b/mocks/networkmapmocks/manager.go @@ -128,13 +128,13 @@ func (_m *Manager) GetIdentityVerifiers(ctx context.Context, ns string, id strin return r0, r1, r2 } -// GetNodeByID provides a mock function with given fields: ctx, id -func (_m *Manager) GetNodeByID(ctx context.Context, id string) (*fftypes.Identity, error) { - ret := _m.Called(ctx, id) +// GetNodeByNameOrID provides a mock function with given fields: ctx, nameOrID +func (_m *Manager) GetNodeByNameOrID(ctx context.Context, nameOrID string) (*fftypes.Identity, error) { + ret := _m.Called(ctx, nameOrID) var r0 *fftypes.Identity if rf, ok := ret.Get(0).(func(context.Context, string) *fftypes.Identity); ok { - r0 = rf(ctx, id) + r0 = rf(ctx, nameOrID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*fftypes.Identity) @@ -143,7 +143,7 @@ func (_m *Manager) GetNodeByID(ctx context.Context, id string) (*fftypes.Identit var r1 error if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, id) + r1 = rf(ctx, nameOrID) } else { r1 = ret.Error(1) } @@ -183,13 +183,13 @@ func (_m *Manager) GetNodes(ctx context.Context, filter database.AndFilter) ([]* return r0, r1, r2 } -// GetOrganizationByID provides a mock function with given fields: ctx, id -func (_m *Manager) GetOrganizationByID(ctx context.Context, id string) (*fftypes.Identity, error) { - ret := _m.Called(ctx, id) +// GetOrganizationByNameOrID provides a mock function with given fields: ctx, nameOrID +func (_m *Manager) GetOrganizationByNameOrID(ctx context.Context, nameOrID string) (*fftypes.Identity, error) { + ret := _m.Called(ctx, nameOrID) var r0 *fftypes.Identity if rf, ok := ret.Get(0).(func(context.Context, string) *fftypes.Identity); ok { - r0 = rf(ctx, id) + r0 = rf(ctx, nameOrID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*fftypes.Identity) @@ -198,7 +198,7 @@ func (_m *Manager) GetOrganizationByID(ctx context.Context, id string) (*fftypes var r1 error if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { - r1 = rf(ctx, id) + r1 = rf(ctx, nameOrID) } else { r1 = ret.Error(1) }