From ca6e3a9e88fc45ecc66dbeefbf3f9be34e81dd53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Tue, 31 Jan 2023 11:36:33 +0100 Subject: [PATCH] satellite/orders: create mock based unit test Most of our (~integration) tests based on testplanet runner. However running testplanet for each test make the testing process slow. It seems to be better to use real unit tests (without db dependency) when it's possible. This patch makes small modification to make it possible to test orders.Service with real unit test. As the existing unit test of `service.go` is isolated with `_test` package name, it's moved to an `_integration_test.go` file to make place for the unit test. Change-Id: Ia69f26a34e2c48d230d8d36c2040dd02a60455a6 --- go.mod | 2 +- satellite/orders/mock_test.go | 97 ++++++++++++++ satellite/orders/service.go | 15 ++- satellite/orders/service_integration_test.go | 66 ++++++++++ satellite/orders/service_test.go | 126 ++++++++++++------- 5 files changed, 257 insertions(+), 49 deletions(-) create mode 100644 satellite/orders/mock_test.go create mode 100644 satellite/orders/service_integration_test.go diff --git a/go.mod b/go.mod index 537be5828..9d0350f21 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/go-oauth2/oauth2/v4 v4.4.2 github.com/go-redis/redis/v8 v8.11.5 github.com/gogo/protobuf v1.3.2 + github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.5.8 github.com/gorilla/mux v1.8.0 github.com/gorilla/schema v1.2.0 @@ -77,7 +78,6 @@ require ( github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 // indirect github.com/golang-jwt/jwt v3.2.1+incompatible // indirect github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7 // indirect - github.com/golang/mock v1.6.0 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/google/pprof v0.0.0-20211108044417-e9b028704de0 // indirect github.com/google/uuid v1.1.1 // indirect diff --git a/satellite/orders/mock_test.go b/satellite/orders/mock_test.go new file mode 100644 index 000000000..dbed1666d --- /dev/null +++ b/satellite/orders/mock_test.go @@ -0,0 +1,97 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: storj.io/storj/satellite/orders (interfaces: Overlay) + +// Package orders is a generated GoMock package. +package orders + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + storj "storj.io/common/storj" + overlay "storj.io/storj/satellite/overlay" +) + +// MockOverlayForOrders is a mock of Overlay interface. +type MockOverlayForOrders struct { + ctrl *gomock.Controller + recorder *MockOverlayForOrdersMockRecorder +} + +// MockOverlayForOrdersMockRecorder is the mock recorder for MockOverlayForOrders. +type MockOverlayForOrdersMockRecorder struct { + mock *MockOverlayForOrders +} + +// NewMockOverlayForOrders creates a new mock instance. +func NewMockOverlayForOrders(ctrl *gomock.Controller) *MockOverlayForOrders { + mock := &MockOverlayForOrders{ctrl: ctrl} + mock.recorder = &MockOverlayForOrdersMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockOverlayForOrders) EXPECT() *MockOverlayForOrdersMockRecorder { + return m.recorder +} + +// CachedGetOnlineNodesForGet mocks base method. +func (m *MockOverlayForOrders) CachedGetOnlineNodesForGet(arg0 context.Context, arg1 []storj.NodeID) (map[storj.NodeID]*overlay.SelectedNode, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CachedGetOnlineNodesForGet", arg0, arg1) + ret0, _ := ret[0].(map[storj.NodeID]*overlay.SelectedNode) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CachedGetOnlineNodesForGet indicates an expected call of CachedGetOnlineNodesForGet. +func (mr *MockOverlayForOrdersMockRecorder) CachedGetOnlineNodesForGet(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CachedGetOnlineNodesForGet", reflect.TypeOf((*MockOverlayForOrders)(nil).CachedGetOnlineNodesForGet), arg0, arg1) +} + +// Get mocks base method. +func (m *MockOverlayForOrders) Get(arg0 context.Context, arg1 storj.NodeID) (*overlay.NodeDossier, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1) + ret0, _ := ret[0].(*overlay.NodeDossier) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockOverlayForOrdersMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockOverlayForOrders)(nil).Get), arg0, arg1) +} + +// GetOnlineNodesForAuditRepair mocks base method. +func (m *MockOverlayForOrders) GetOnlineNodesForAuditRepair(arg0 context.Context, arg1 []storj.NodeID) (map[storj.NodeID]*overlay.NodeReputation, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetOnlineNodesForAuditRepair", arg0, arg1) + ret0, _ := ret[0].(map[storj.NodeID]*overlay.NodeReputation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetOnlineNodesForAuditRepair indicates an expected call of GetOnlineNodesForAuditRepair. +func (mr *MockOverlayForOrdersMockRecorder) GetOnlineNodesForAuditRepair(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOnlineNodesForAuditRepair", reflect.TypeOf((*MockOverlayForOrders)(nil).GetOnlineNodesForAuditRepair), arg0, arg1) +} + +// IsOnline mocks base method. +func (m *MockOverlayForOrders) IsOnline(arg0 *overlay.NodeDossier) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsOnline", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsOnline indicates an expected call of IsOnline. +func (mr *MockOverlayForOrdersMockRecorder) IsOnline(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsOnline", reflect.TypeOf((*MockOverlayForOrders)(nil).IsOnline), arg0) +} diff --git a/satellite/orders/service.go b/satellite/orders/service.go index 1342089c1..f28c9faed 100644 --- a/satellite/orders/service.go +++ b/satellite/orders/service.go @@ -39,13 +39,24 @@ type Config struct { OrdersSemaphoreSize int `help:"how many concurrent orders to process at once. zero is unlimited" default:"2"` } +// Overlay defines the overlay dependency of orders.Service. +// use `go install github.com/golang/mock/mockgen@v1.6.0` if missing +// +//go:generate mockgen -destination mock_test.go -package orders . OverlayForOrders +type Overlay interface { + CachedGetOnlineNodesForGet(context.Context, []storj.NodeID) (map[storj.NodeID]*overlay.SelectedNode, error) + GetOnlineNodesForAuditRepair(context.Context, []storj.NodeID) (map[storj.NodeID]*overlay.NodeReputation, error) + Get(ctx context.Context, nodeID storj.NodeID) (*overlay.NodeDossier, error) + IsOnline(node *overlay.NodeDossier) bool +} + // Service for creating order limits. // // architecture: Service type Service struct { log *zap.Logger satellite signing.Signer - overlay *overlay.Service + overlay Overlay orders DB encryptionKeys EncryptionKeys @@ -58,7 +69,7 @@ type Service struct { // NewService creates new service for creating order limits. func NewService( - log *zap.Logger, satellite signing.Signer, overlay *overlay.Service, + log *zap.Logger, satellite signing.Signer, overlay Overlay, orders DB, config Config, ) (*Service, error) { if config.EncryptionKeys.Default.IsZero() { diff --git a/satellite/orders/service_integration_test.go b/satellite/orders/service_integration_test.go new file mode 100644 index 000000000..539ec9f37 --- /dev/null +++ b/satellite/orders/service_integration_test.go @@ -0,0 +1,66 @@ +// Copyright (C) 2019 Storj Labs, Inc. +// See LICENSE for copying information. + +package orders_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "storj.io/common/memory" + "storj.io/common/testcontext" + "storj.io/common/testrand" + "storj.io/storj/private/testplanet" + "storj.io/storj/satellite/metabase" +) + +func TestOrderLimitsEncryptedMetadata(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + const ( + bucketName = "testbucket" + filePath = "test/path" + ) + + var ( + satellitePeer = planet.Satellites[0] + uplinkPeer = planet.Uplinks[0] + projectID = uplinkPeer.Projects[0].ID + ) + // Setup: Upload an object and create order limits + require.NoError(t, uplinkPeer.Upload(ctx, satellitePeer, bucketName, filePath, testrand.Bytes(5*memory.KiB))) + + bucket := metabase.BucketLocation{ProjectID: projectID, BucketName: bucketName} + + segments, err := satellitePeer.Metabase.DB.TestingAllSegments(ctx) + require.NoError(t, err) + require.Equal(t, 1, len(segments)) + + limits, _, err := satellitePeer.Orders.Service.CreateGetOrderLimits(ctx, bucket, segments[0], 0) + require.NoError(t, err) + require.Equal(t, 3, len(limits)) + + // Test: get the bucket name and project ID from the encrypted metadata and + // compare with the old method of getting the data from the serial numbers table. + orderLimit1 := limits[0].Limit + // from 3 order limits only one can be nil + if orderLimit1 == nil { + orderLimit1 = limits[1].Limit + } + require.True(t, len(orderLimit1.EncryptedMetadata) > 0) + + _, err = metabase.ParseBucketPrefix(metabase.BucketPrefix("")) + require.Error(t, err) + var x []byte + _, err = metabase.ParseBucketPrefix(metabase.BucketPrefix(x)) + require.Error(t, err) + actualOrderMetadata, err := satellitePeer.Orders.Service.DecryptOrderMetadata(ctx, orderLimit1) + require.NoError(t, err) + actualBucketInfo, err := metabase.ParseCompactBucketPrefix(actualOrderMetadata.GetCompactProjectBucketPrefix()) + require.NoError(t, err) + require.Equal(t, bucketName, actualBucketInfo.BucketName) + require.Equal(t, projectID, actualBucketInfo.ProjectID) + }) +} diff --git a/satellite/orders/service_test.go b/satellite/orders/service_test.go index 539ec9f37..9753b94a3 100644 --- a/satellite/orders/service_test.go +++ b/satellite/orders/service_test.go @@ -1,66 +1,100 @@ -// Copyright (C) 2019 Storj Labs, Inc. +// Copyright (C) 2023 Storj Labs, Inc. // See LICENSE for copying information. package orders_test import ( + "fmt" "testing" + "time" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" - "storj.io/common/memory" + "storj.io/common/identity/testidentity" + "storj.io/common/pb" + "storj.io/common/signing" + "storj.io/common/storj" "storj.io/common/testcontext" "storj.io/common/testrand" - "storj.io/storj/private/testplanet" "storj.io/storj/satellite/metabase" + "storj.io/storj/satellite/orders" + "storj.io/storj/satellite/overlay" ) -func TestOrderLimitsEncryptedMetadata(t *testing.T) { - testplanet.Run(t, testplanet.Config{ - SatelliteCount: 1, StorageNodeCount: 4, UplinkCount: 1, - }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { - const ( - bucketName = "testbucket" - filePath = "test/path" - ) +func TestGetOrderLimits(t *testing.T) { + ctx := testcontext.New(t) + ctrl := gomock.NewController(t) - var ( - satellitePeer = planet.Satellites[0] - uplinkPeer = planet.Uplinks[0] - projectID = uplinkPeer.Projects[0].ID - ) - // Setup: Upload an object and create order limits - require.NoError(t, uplinkPeer.Upload(ctx, satellitePeer, bucketName, filePath, testrand.Bytes(5*memory.KiB))) + bucket := metabase.BucketLocation{ProjectID: testrand.UUID(), BucketName: "bucket1"} - bucket := metabase.BucketLocation{ProjectID: projectID, BucketName: bucketName} - - segments, err := satellitePeer.Metabase.DB.TestingAllSegments(ctx) - require.NoError(t, err) - require.Equal(t, 1, len(segments)) - - limits, _, err := satellitePeer.Orders.Service.CreateGetOrderLimits(ctx, bucket, segments[0], 0) - require.NoError(t, err) - require.Equal(t, 3, len(limits)) - - // Test: get the bucket name and project ID from the encrypted metadata and - // compare with the old method of getting the data from the serial numbers table. - orderLimit1 := limits[0].Limit - // from 3 order limits only one can be nil - if orderLimit1 == nil { - orderLimit1 = limits[1].Limit + pieces := metabase.Pieces{} + nodes := map[storj.NodeID]*overlay.SelectedNode{} + for i := 0; i < 8; i++ { + nodeID := testrand.NodeID() + nodes[nodeID] = &overlay.SelectedNode{ + ID: nodeID, + Address: &pb.NodeAddress{ + Address: fmt.Sprintf("host%d.com", i), + }, } - require.True(t, len(orderLimit1.EncryptedMetadata) > 0) - _, err = metabase.ParseBucketPrefix(metabase.BucketPrefix("")) - require.Error(t, err) - var x []byte - _, err = metabase.ParseBucketPrefix(metabase.BucketPrefix(x)) - require.Error(t, err) - actualOrderMetadata, err := satellitePeer.Orders.Service.DecryptOrderMetadata(ctx, orderLimit1) - require.NoError(t, err) - actualBucketInfo, err := metabase.ParseCompactBucketPrefix(actualOrderMetadata.GetCompactProjectBucketPrefix()) - require.NoError(t, err) - require.Equal(t, bucketName, actualBucketInfo.BucketName) - require.Equal(t, projectID, actualBucketInfo.ProjectID) + pieces = append(pieces, metabase.Piece{ + Number: uint16(i), + StorageNode: nodeID, + }) + } + testIdentity, err := testidentity.PregeneratedIdentity(0, storj.LatestIDVersion()) + require.NoError(t, err) + k := signing.SignerFromFullIdentity(testIdentity) + + overlayService := orders.NewMockOverlayForOrders(ctrl) + overlayService. + EXPECT(). + CachedGetOnlineNodesForGet(gomock.Any(), gomock.Any()). + Return(nodes, nil).AnyTimes() + + service, err := orders.NewService(zaptest.NewLogger(t), k, overlayService, orders.NewNoopDB(), orders.Config{ + EncryptionKeys: orders.EncryptionKeys{ + Default: orders.EncryptionKey{ + ID: orders.EncryptionKeyID{1, 2, 3, 4, 5, 6, 7, 8}, + Key: testrand.Key(), + }, + }, }) + require.NoError(t, err) + + segment := metabase.Segment{ + StreamID: testrand.UUID(), + CreatedAt: time.Now(), + Redundancy: storj.RedundancyScheme{ + Algorithm: storj.ReedSolomon, + ShareSize: 256, + RequiredShares: 4, + RepairShares: 5, + OptimalShares: 6, + TotalShares: 10, + }, + Pieces: pieces, + EncryptedKey: []byte{1, 2, 3, 4}, + RootPieceID: testrand.PieceID(), + } + + checkExpectedLimits := func(received int) { + limits, _, err := service.CreateGetOrderLimits(ctx, bucket, segment, 0) + require.NoError(t, err) + realLimits := 0 + for _, limit := range limits { + if limit.Limit != nil { + realLimits++ + } + } + require.Equal(t, received, realLimits) + } + + t.Run("Do not request any specific number", func(t *testing.T) { + checkExpectedLimits(6) + }) + }