From 7cfd372af2e1319b74519b61f227e074d4f5910a Mon Sep 17 00:00:00 2001 From: He Weiwei Date: Tue, 23 Feb 2021 12:23:36 +0800 Subject: [PATCH] fix: use clone query in loop of artifact.Iterator func (#14283) Closes #14251 Signed-off-by: He Weiwei --- src/controller/artifact/helper.go | 2 +- src/controller/artifact/helper_test.go | 20 +- src/testing/pkg/artifact/fake_manager.go | 97 +++++++++ src/testing/pkg/artifact/manager.go | 256 ++++++++++++++++------- src/testing/pkg/pkg.go | 1 + 5 files changed, 295 insertions(+), 81 deletions(-) create mode 100644 src/testing/pkg/artifact/fake_manager.go diff --git a/src/controller/artifact/helper.go b/src/controller/artifact/helper.go index 7e4dc0788..d0b80fa7f 100644 --- a/src/controller/artifact/helper.go +++ b/src/controller/artifact/helper.go @@ -47,7 +47,7 @@ func Iterator(ctx context.Context, chunkSize int, query *q.Query, option *Option break } - query.PageNumber++ + clone.PageNumber++ } }() diff --git a/src/controller/artifact/helper_test.go b/src/controller/artifact/helper_test.go index 392770d65..646d40440 100644 --- a/src/controller/artifact/helper_test.go +++ b/src/controller/artifact/helper_test.go @@ -18,22 +18,24 @@ import ( "context" "testing" + "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/artifact" artifacttesting "github.com/goharbor/harbor/src/testing/pkg/artifact" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) type IteratorTestSuite struct { suite.Suite - artMgr *artifacttesting.FakeManager + artMgr *artifacttesting.Manager ctl *controller originalCtl Controller } func (suite *IteratorTestSuite) SetupSuite() { - suite.artMgr = &artifacttesting.FakeManager{} + suite.artMgr = &artifacttesting.Manager{} suite.originalCtl = Ctl suite.ctl = &controller{artMgr: suite.artMgr} @@ -45,10 +47,20 @@ func (suite *IteratorTestSuite) TeardownSuite() { } func (suite *IteratorTestSuite) TestIterator() { - suite.artMgr.On("List").Return([]*artifact.Artifact{ + q1 := &q.Query{PageNumber: 1, PageSize: 5, Keywords: map[string]interface{}{}} + suite.artMgr.On("List", mock.Anything, q1).Return([]*artifact.Artifact{ {ID: 1}, {ID: 2}, {ID: 3}, + {ID: 4}, + {ID: 5}, + }, nil) + + q2 := &q.Query{PageNumber: 2, PageSize: 5, Keywords: map[string]interface{}{}} + suite.artMgr.On("List", mock.Anything, q2).Return([]*artifact.Artifact{ + {ID: 6}, + {ID: 7}, + {ID: 8}, }, nil) var artifacts []*Artifact @@ -56,7 +68,7 @@ func (suite *IteratorTestSuite) TestIterator() { artifacts = append(artifacts, art) } - suite.Len(artifacts, 3) + suite.Len(artifacts, 8) } func TestIteratorTestSuite(t *testing.T) { diff --git a/src/testing/pkg/artifact/fake_manager.go b/src/testing/pkg/artifact/fake_manager.go new file mode 100644 index 000000000..e8b007157 --- /dev/null +++ b/src/testing/pkg/artifact/fake_manager.go @@ -0,0 +1,97 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package artifact + +import ( + "context" + "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg/artifact" + "github.com/stretchr/testify/mock" +) + +// FakeManager is a fake artifact manager that implement src/pkg/artifact.Manager interface +type FakeManager struct { + mock.Mock +} + +// Count ... +func (f *FakeManager) Count(ctx context.Context, query *q.Query) (int64, error) { + args := f.Called() + return int64(args.Int(0)), args.Error(1) +} + +// List ... +func (f *FakeManager) List(ctx context.Context, query *q.Query) ([]*artifact.Artifact, error) { + args := f.Called() + var artifacts []*artifact.Artifact + if args.Get(0) != nil { + artifacts = args.Get(0).([]*artifact.Artifact) + } + return artifacts, args.Error(1) +} + +// Get ... +func (f *FakeManager) Get(ctx context.Context, id int64) (*artifact.Artifact, error) { + args := f.Called() + var art *artifact.Artifact + if args.Get(0) != nil { + art = args.Get(0).(*artifact.Artifact) + } + return art, args.Error(1) +} + +// GetByDigest ... +func (f *FakeManager) GetByDigest(ctx context.Context, repository, digest string) (*artifact.Artifact, error) { + args := f.Called() + var art *artifact.Artifact + if args.Get(0) != nil { + art = args.Get(0).(*artifact.Artifact) + } + return art, args.Error(1) +} + +// Create ... +func (f *FakeManager) Create(ctx context.Context, artifact *artifact.Artifact) (int64, error) { + args := f.Called() + return int64(args.Int(0)), args.Error(1) +} + +// Delete ... +func (f *FakeManager) Delete(ctx context.Context, id int64) error { + args := f.Called() + return args.Error(0) +} + +// UpdatePullTime ... +func (f *FakeManager) Update(ctx context.Context, artifact *artifact.Artifact, props ...string) error { + args := f.Called() + return args.Error(0) +} + +// ListReferences ... +func (f *FakeManager) ListReferences(ctx context.Context, query *q.Query) ([]*artifact.Reference, error) { + args := f.Called() + var references []*artifact.Reference + if args.Get(0) != nil { + references = args.Get(0).([]*artifact.Reference) + } + return references, args.Error(1) +} + +// DeleteReference ... +func (f *FakeManager) DeleteReference(ctx context.Context, id int64) error { + args := f.Called() + return args.Error(0) +} diff --git a/src/testing/pkg/artifact/manager.go b/src/testing/pkg/artifact/manager.go index e8b007157..bcc3c3f33 100644 --- a/src/testing/pkg/artifact/manager.go +++ b/src/testing/pkg/artifact/manager.go @@ -1,97 +1,201 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +// Code generated by mockery v2.1.0. DO NOT EDIT. package artifact import ( - "context" - "github.com/goharbor/harbor/src/lib/q" - "github.com/goharbor/harbor/src/pkg/artifact" - "github.com/stretchr/testify/mock" + context "context" + + artifact "github.com/goharbor/harbor/src/pkg/artifact" + + mock "github.com/stretchr/testify/mock" + + q "github.com/goharbor/harbor/src/lib/q" ) -// FakeManager is a fake artifact manager that implement src/pkg/artifact.Manager interface -type FakeManager struct { +// Manager is an autogenerated mock type for the Manager type +type Manager struct { mock.Mock } -// Count ... -func (f *FakeManager) Count(ctx context.Context, query *q.Query) (int64, error) { - args := f.Called() - return int64(args.Int(0)), args.Error(1) -} +// Count provides a mock function with given fields: ctx, query +func (_m *Manager) Count(ctx context.Context, query *q.Query) (int64, error) { + ret := _m.Called(ctx, query) -// List ... -func (f *FakeManager) List(ctx context.Context, query *q.Query) ([]*artifact.Artifact, error) { - args := f.Called() - var artifacts []*artifact.Artifact - if args.Get(0) != nil { - artifacts = args.Get(0).([]*artifact.Artifact) + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *q.Query) int64); ok { + r0 = rf(ctx, query) + } else { + r0 = ret.Get(0).(int64) } - return artifacts, args.Error(1) -} -// Get ... -func (f *FakeManager) Get(ctx context.Context, id int64) (*artifact.Artifact, error) { - args := f.Called() - var art *artifact.Artifact - if args.Get(0) != nil { - art = args.Get(0).(*artifact.Artifact) + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *q.Query) error); ok { + r1 = rf(ctx, query) + } else { + r1 = ret.Error(1) } - return art, args.Error(1) + + return r0, r1 } -// GetByDigest ... -func (f *FakeManager) GetByDigest(ctx context.Context, repository, digest string) (*artifact.Artifact, error) { - args := f.Called() - var art *artifact.Artifact - if args.Get(0) != nil { - art = args.Get(0).(*artifact.Artifact) +// Create provides a mock function with given fields: ctx, _a1 +func (_m *Manager) Create(ctx context.Context, _a1 *artifact.Artifact) (int64, error) { + ret := _m.Called(ctx, _a1) + + var r0 int64 + if rf, ok := ret.Get(0).(func(context.Context, *artifact.Artifact) int64); ok { + r0 = rf(ctx, _a1) + } else { + r0 = ret.Get(0).(int64) } - return art, args.Error(1) -} -// Create ... -func (f *FakeManager) Create(ctx context.Context, artifact *artifact.Artifact) (int64, error) { - args := f.Called() - return int64(args.Int(0)), args.Error(1) -} - -// Delete ... -func (f *FakeManager) Delete(ctx context.Context, id int64) error { - args := f.Called() - return args.Error(0) -} - -// UpdatePullTime ... -func (f *FakeManager) Update(ctx context.Context, artifact *artifact.Artifact, props ...string) error { - args := f.Called() - return args.Error(0) -} - -// ListReferences ... -func (f *FakeManager) ListReferences(ctx context.Context, query *q.Query) ([]*artifact.Reference, error) { - args := f.Called() - var references []*artifact.Reference - if args.Get(0) != nil { - references = args.Get(0).([]*artifact.Reference) + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *artifact.Artifact) error); ok { + r1 = rf(ctx, _a1) + } else { + r1 = ret.Error(1) } - return references, args.Error(1) + + return r0, r1 } -// DeleteReference ... -func (f *FakeManager) DeleteReference(ctx context.Context, id int64) error { - args := f.Called() - return args.Error(0) +// Delete provides a mock function with given fields: ctx, id +func (_m *Manager) Delete(ctx context.Context, id int64) error { + ret := _m.Called(ctx, id) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok { + r0 = rf(ctx, id) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// DeleteReference provides a mock function with given fields: ctx, id +func (_m *Manager) DeleteReference(ctx context.Context, id int64) error { + ret := _m.Called(ctx, id) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, int64) error); ok { + r0 = rf(ctx, id) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Get provides a mock function with given fields: ctx, id +func (_m *Manager) Get(ctx context.Context, id int64) (*artifact.Artifact, error) { + ret := _m.Called(ctx, id) + + var r0 *artifact.Artifact + if rf, ok := ret.Get(0).(func(context.Context, int64) *artifact.Artifact); ok { + r0 = rf(ctx, id) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*artifact.Artifact) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, int64) error); ok { + r1 = rf(ctx, id) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetByDigest provides a mock function with given fields: ctx, repository, digest +func (_m *Manager) GetByDigest(ctx context.Context, repository string, digest string) (*artifact.Artifact, error) { + ret := _m.Called(ctx, repository, digest) + + var r0 *artifact.Artifact + if rf, ok := ret.Get(0).(func(context.Context, string, string) *artifact.Artifact); ok { + r0 = rf(ctx, repository, digest) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*artifact.Artifact) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + r1 = rf(ctx, repository, digest) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// List provides a mock function with given fields: ctx, query +func (_m *Manager) List(ctx context.Context, query *q.Query) ([]*artifact.Artifact, error) { + ret := _m.Called(ctx, query) + + var r0 []*artifact.Artifact + if rf, ok := ret.Get(0).(func(context.Context, *q.Query) []*artifact.Artifact); ok { + r0 = rf(ctx, query) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*artifact.Artifact) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *q.Query) error); ok { + r1 = rf(ctx, query) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ListReferences provides a mock function with given fields: ctx, query +func (_m *Manager) ListReferences(ctx context.Context, query *q.Query) ([]*artifact.Reference, error) { + ret := _m.Called(ctx, query) + + var r0 []*artifact.Reference + if rf, ok := ret.Get(0).(func(context.Context, *q.Query) []*artifact.Reference); ok { + r0 = rf(ctx, query) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*artifact.Reference) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *q.Query) error); ok { + r1 = rf(ctx, query) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Update provides a mock function with given fields: ctx, _a1, props +func (_m *Manager) Update(ctx context.Context, _a1 *artifact.Artifact, props ...string) error { + _va := make([]interface{}, len(props)) + for _i := range props { + _va[_i] = props[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, _a1) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *artifact.Artifact, ...string) error); ok { + r0 = rf(ctx, _a1, props...) + } else { + r0 = ret.Error(0) + } + + return r0 } diff --git a/src/testing/pkg/pkg.go b/src/testing/pkg/pkg.go index c35712e02..6aa043272 100644 --- a/src/testing/pkg/pkg.go +++ b/src/testing/pkg/pkg.go @@ -14,6 +14,7 @@ package pkg +//go:generate mockery --case snake --dir ../../pkg/artifact --name Manager --output ./artifact --outpkg artifact //go:generate mockery --case snake --dir ../../pkg/blob --name Manager --output ./blob --outpkg blob //go:generate mockery --case snake --dir ../../vendor/github.com/docker/distribution --name Manifest --output ./distribution --outpkg distribution //go:generate mockery --case snake --dir ../../pkg/project --name Manager --output ./project --outpkg project