move out the tags methods of artifact ctl

1, move the tag methods out of artifact ctl, let api to call tag ctr
2, update the ensure sequence for existing tag

Signed-off-by: wang yan <wangyan@vmware.com>

src/api/artifact/controller.go

@@ -73,12 +73,6 @@ type Controller interface {
 	Delete(ctx context.Context, id int64) (err error)
 	// Copy the artifact specified by "srcRepo" and "reference" into the repository specified by "dstRepo"
 	Copy(ctx context.Context, srcRepo, reference, dstRepo string) (id int64, err error)
-	// ListTags lists the tags according to the query, specify the properties returned with option
-	ListTags(ctx context.Context, query *q.Query, option *tag.Option) (tags []*tag.Tag, err error)
-	// CreateTag creates a tag
-	CreateTag(ctx context.Context, tag *tag.Tag) (id int64, err error)
-	// DeleteTag deletes the tag specified by tagID
-	DeleteTag(ctx context.Context, tagID int64) (err error)
 	// UpdatePullTime updates the pull time for the artifact. If the tagID is provides, update the pull
 	// time of the tag as well
 	UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) (err error)
@@ -300,7 +294,11 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot bool) er
 
 	// delete all tags that attached to the root artifact
 	if isRoot {
-		if err = c.tagCtl.DeleteTags(ctx, art.Tags); err != nil {
+		var ids []int64
+		for _, tag := range art.Tags {
+			ids = append(ids, tag.ID)
+		}
+		if err = c.tagCtl.DeleteTags(ctx, ids); err != nil {
 			return err
 		}
 	}
@@ -407,23 +405,6 @@ func (c *controller) copyDeeply(ctx context.Context, srcRepo, reference, dstRepo
 	return id, nil
 }
 
-func (c *controller) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) {
-	// TODO fire event
-	return c.tagCtl.Create(ctx, tag)
-}
-func (c *controller) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) {
-	tags, err := c.tagCtl.List(ctx, query, option)
-	if err != nil {
-		return nil, err
-	}
-	return tags, nil
-}
-
-func (c *controller) DeleteTag(ctx context.Context, tagID int64) error {
-	// TODO fire delete tag event
-	return c.tagCtl.Delete(ctx, tagID)
-}
-
 func (c *controller) UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) error {
 	tag, err := c.tagCtl.Get(ctx, tagID, nil)
 	if err != nil {

src/api/artifact/controller_test.go

@@ -478,40 +478,6 @@ func (c *controllerTestSuite) TestCopy() {
 	c.Require().Nil(err)
 }
 
-func (c *controllerTestSuite) TestListTags() {
-	c.tagCtl.On("List").Return([]*tag.Tag{
-		{
-			Tag: model_tag.Tag{
-				ID:           1,
-				RepositoryID: 1,
-				Name:         "latest",
-				ArtifactID:   1,
-			},
-		},
-	}, nil)
-	c.artMgr.On("Get").Return(&artifact.Artifact{}, nil)
-	tags, err := c.ctl.ListTags(nil, nil, nil)
-	c.Require().Nil(err)
-	c.Len(tags, 1)
-	c.tagCtl.AssertExpectations(c.T())
-	c.Equal(tags[0].Immutable, false)
-	// TODO check other properties: label, etc
-}
-
-func (c *controllerTestSuite) TestCreateTag() {
-	c.tagCtl.On("Create").Return(1, nil)
-	id, err := c.ctl.CreateTag(nil, &tag.Tag{})
-	c.Require().Nil(err)
-	c.Equal(int64(1), id)
-}
-
-func (c *controllerTestSuite) TestDeleteTag() {
-	c.tagCtl.On("Delete").Return(nil)
-	err := c.ctl.DeleteTag(nil, 1)
-	c.Require().Nil(err)
-	c.tagCtl.AssertExpectations(c.T())
-}
-
 func (c *controllerTestSuite) TestUpdatePullTime() {
 	// artifact ID and tag ID matches
 	c.tagCtl.On("Get").Return(&tag.Tag{

src/api/tag/controller.go

@@ -38,7 +38,7 @@ type Controller interface {
 	// Delete the tag specified by ID with limitation check
 	Delete(ctx context.Context, id int64) (err error)
 	// DeleteTags deletes all tags
-	DeleteTags(ctx context.Context, tags []*Tag) (err error)
+	DeleteTags(ctx context.Context, ids []int64) (err error)
 }
 
 // NewController creates an instance of the default repository controller
@@ -66,7 +66,6 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64,
 	}
 	tags, err := c.List(ctx, query, &Option{
 		WithImmutableStatus: true,
-		WithSignature:       true,
 	})
 	if err != nil {
 		return err
@@ -74,19 +73,15 @@ func (c *controller) Ensure(ctx context.Context, repositoryID, artifactID int64,
 	// the tag already exists under the repository
 	if len(tags) > 0 {
 		tag := tags[0]
+		// the tag already exists under the repository and is attached to the artifact, return directly
+		if tag.ArtifactID == artifactID {
+			return nil
+		}
 		// existing tag must check the immutable status and signature
 		if tag.Immutable {
 			return ierror.New(nil).WithCode(ierror.PreconditionCode).
 				WithMessage("the tag %s configured as immutable, cannot be updated", tag.Name)
 		}
-		if tag.Signed {
-			return ierror.New(nil).WithCode(ierror.PreconditionCode).
-				WithMessage("the tag %s with signature cannot be updated", tag.Name)
-		}
-		// the tag already exists under the repository and is attached to the artifact, return directly
-		if tag.ArtifactID == artifactID {
-			return nil
-		}
 		// the tag exists under the repository, but it is attached to other artifact
 		// update it to point to the provided artifact
 		tag.ArtifactID = artifactID
@@ -138,15 +133,7 @@ func (c *controller) Get(ctx context.Context, id int64, option *Option) (tag *Ta
 		return tag, nil
 	}
 
-	if option.WithImmutableStatus {
+	return c.assembleTag(ctx, &tag.Tag, option), nil
-		c.populateImmutableStatus(ctx, tag)
-	}
-
-	if option.WithSignature {
-		c.populateTagSignature(ctx, tag, option)
-	}
-
-	return tag, nil
 }
 
 // Create ...
@@ -181,10 +168,10 @@ func (c *controller) Delete(ctx context.Context, id int64) (err error) {
 }
 
 // DeleteTags ...
-func (c *controller) DeleteTags(ctx context.Context, tags []*Tag) (err error) {
+func (c *controller) DeleteTags(ctx context.Context, ids []int64) (err error) {
 	// in order to leverage the signature and immutable status check
-	for _, tag := range tags {
+	for _, id := range ids {
-		if err := c.Delete(ctx, tag.ID); err != nil {
+		if err := c.Delete(ctx, id); err != nil {
 			return err
 		}
 	}

src/api/tag/controller_test.go

@@ -179,16 +179,8 @@ func (c *controllerTestSuite) TestDeleteTags() {
 	}, nil)
 	c.immutableMtr.On("Match").Return(false, nil)
 	c.tagMgr.On("Delete").Return(nil)
-	tags := []*Tag{
+	ids := []int64{1, 2, 3, 4}
-		{
+	err := c.ctl.DeleteTags(nil, ids)
-			Tag: tag.Tag{
-				RepositoryID: 10,
-				Name:         "test2",
-			},
-			Signed: true,
-		},
-	}
-	err := c.ctl.DeleteTags(nil, tags)
 	c.Require().Nil(err)
 }
 

src/server/registry/tag.go

@@ -17,8 +17,8 @@ package registry
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/goharbor/harbor/src/api/artifact"
 	"github.com/goharbor/harbor/src/api/repository"
+	"github.com/goharbor/harbor/src/api/tag"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/pkg/q"
 	serror "github.com/goharbor/harbor/src/server/error"
@@ -32,13 +32,13 @@ import (
 func newTagHandler() http.Handler {
 	return &tagHandler{
 		repoCtl: repository.Ctl,
-		artCtl:  artifact.Ctl,
+		tagCtl:  tag.Ctl,
 	}
 }
 
 type tagHandler struct {
 	repoCtl        repository.Controller
-	artCtl         artifact.Controller
+	tagCtl         tag.Controller
 	repositoryName string
 }
 
@@ -80,7 +80,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	}
 
 	// get tags ...
-	tags, err := t.artCtl.ListTags(req.Context(), &q.Query{
+	tags, err := t.tagCtl.List(req.Context(), &q.Query{
 		Keywords: map[string]interface{}{
 			"RepositoryID": repository.RepositoryID,
 		}}, nil)

src/server/registry/tag_test.go

@@ -16,13 +16,12 @@ package registry
 
 import (
 	"encoding/json"
-	"github.com/goharbor/harbor/src/api/artifact"
 	"github.com/goharbor/harbor/src/api/repository"
 	"github.com/goharbor/harbor/src/api/tag"
 	"github.com/goharbor/harbor/src/common/models"
 	model_tag "github.com/goharbor/harbor/src/pkg/tag/model/tag"
-	arttesting "github.com/goharbor/harbor/src/testing/api/artifact"
 	repotesting "github.com/goharbor/harbor/src/testing/api/repository"
+	tagtesting "github.com/goharbor/harbor/src/testing/api/tag"
 	"github.com/stretchr/testify/suite"
 	"net/http"
 	"net/http/httptest"
@@ -33,20 +32,20 @@ type tagTestSuite struct {
 	suite.Suite
 	originalRepoCtl repository.Controller
 	repoCtl         *repotesting.FakeController
-	originalArtCtl  artifact.Controller
+	originalTagCtl  tag.Controller
-	artCtl          *arttesting.FakeController
+	tagCtl          *tagtesting.FakeController
 }
 
 func (c *tagTestSuite) SetupSuite() {
-	c.originalArtCtl = artifact.Ctl
 	c.originalRepoCtl = repository.Ctl
+	c.originalTagCtl = tag.Ctl
 }
 
 func (c *tagTestSuite) SetupTest() {
-	c.artCtl = &arttesting.FakeController{}
-	artifact.Ctl = c.artCtl
 	c.repoCtl = &repotesting.FakeController{}
 	repository.Ctl = c.repoCtl
+	c.tagCtl = &tagtesting.FakeController{}
+	tag.Ctl = c.tagCtl
 }
 
 func (c *tagTestSuite) TearDownTest() {
@@ -54,7 +53,7 @@ func (c *tagTestSuite) TearDownTest() {
 
 func (c *tagTestSuite) TearDownSuite() {
 	repository.Ctl = c.originalRepoCtl
-	artifact.Ctl = c.originalArtCtl
+	tag.Ctl = c.originalTagCtl
 }
 
 func (c *tagTestSuite) TestListTag() {
@@ -65,7 +64,7 @@ func (c *tagTestSuite) TestListTag() {
 		RepositoryID: 1,
 		Name:         "library/hello-world",
 	}, nil)
-	c.artCtl.On("ListTags").Return([]*tag.Tag{
+	c.tagCtl.On("List").Return([]*tag.Tag{
 		{
 			Tag: model_tag.Tag{
 				RepositoryID: 1,
@@ -100,7 +99,7 @@ func (c *tagTestSuite) TestListTagPagination1() {
 		RepositoryID: 1,
 		Name:         "hello-world",
 	}, nil)
-	c.artCtl.On("ListTags").Return([]*tag.Tag{
+	c.tagCtl.On("List").Return([]*tag.Tag{
 		{
 			Tag: model_tag.Tag{
 				RepositoryID: 1,
@@ -136,7 +135,7 @@ func (c *tagTestSuite) TestListTagPagination2() {
 		RepositoryID: 1,
 		Name:         "hello-world",
 	}, nil)
-	c.artCtl.On("ListTags").Return([]*tag.Tag{
+	c.tagCtl.On("List").Return([]*tag.Tag{
 		{
 			Tag: model_tag.Tag{
 				RepositoryID: 1,
@@ -172,7 +171,7 @@ func (c *tagTestSuite) TestListTagPagination3() {
 		RepositoryID: 1,
 		Name:         "hello-world",
 	}, nil)
-	c.artCtl.On("ListTags").Return([]*tag.Tag{
+	c.tagCtl.On("List").Return([]*tag.Tag{
 		{
 			Tag: model_tag.Tag{
 				RepositoryID: 1,

src/server/v2.0/handler/artifact.go

@@ -51,6 +51,7 @@ func newArtifactAPI() *artifactAPI {
 		proMgr:  project.Mgr,
 		repoCtl: repository.Ctl,
 		scanCtl: scan.DefaultController,
+		tagCtl:  tag.Ctl,
 	}
 }
 
@@ -60,6 +61,7 @@ type artifactAPI struct {
 	proMgr  project.Manager
 	repoCtl repository.Controller
 	scanCtl scan.Controller
+	tagCtl  tag.Controller
 }
 
 func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListArtifactsParams) middleware.Responder {
@@ -238,7 +240,7 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP
 	tag.ArtifactID = art.ID
 	tag.Name = params.Tag.Name
 	tag.PushTime = time.Now()
-	if _, err = a.artCtl.CreateTag(ctx, tag); err != nil {
+	if _, err = a.tagCtl.Create(ctx, tag); err != nil {
 		return a.SendError(ctx, err)
 	}
 	// TODO set location header?
@@ -269,7 +271,7 @@ func (a *artifactAPI) DeleteTag(ctx context.Context, params operation.DeleteTagP
 			"tag %s attached to artifact %d not found", params.TagName, artifact.ID)
 		return a.SendError(ctx, err)
 	}
-	if err = a.artCtl.DeleteTag(ctx, id); err != nil {
+	if err = a.tagCtl.Delete(ctx, id); err != nil {
 		return a.SendError(ctx, err)
 	}
 	return operation.NewDeleteTagOK()

src/testing/api/artifact/controller.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"github.com/goharbor/harbor/src/api/artifact"
 	"github.com/goharbor/harbor/src/api/artifact/abstractor/resolver"
-	"github.com/goharbor/harbor/src/api/tag"
 	"github.com/goharbor/harbor/src/pkg/q"
 	"github.com/stretchr/testify/mock"
 	"time"
@@ -83,28 +82,6 @@ func (f *FakeController) Copy(ctx context.Context, srcRepo, ref, dstRepo string)
 	return int64(args.Int(0)), args.Error(1)
 }
 
-// ListTags ...
-func (f *FakeController) ListTags(ctx context.Context, query *q.Query, option *tag.Option) ([]*tag.Tag, error) {
-	args := f.Called()
-	var tags []*tag.Tag
-	if args.Get(0) != nil {
-		tags = args.Get(0).([]*tag.Tag)
-	}
-	return tags, args.Error(1)
-}
-
-// CreateTag ...
-func (f *FakeController) CreateTag(ctx context.Context, tag *tag.Tag) (int64, error) {
-	args := f.Called()
-	return int64(args.Int(0)), args.Error(1)
-}
-
-// DeleteTag ...
-func (f *FakeController) DeleteTag(ctx context.Context, tagID int64) (err error) {
-	args := f.Called()
-	return args.Error(0)
-}
-
 // UpdatePullTime ...
 func (f *FakeController) UpdatePullTime(ctx context.Context, artifactID int64, tagID int64, time time.Time) error {
 	args := f.Called()

src/testing/api/tag/controller.go

@@ -63,7 +63,7 @@ func (f *FakeController) Delete(ctx context.Context, id int64) (err error) {
 }
 
 // DeleteTags ...
-func (f *FakeController) DeleteTags(ctx context.Context, tags []*tag.Tag) (err error) {
+func (f *FakeController) DeleteTags(ctx context.Context, ids []int64) (err error) {
 	args := f.Called()
 	return args.Error(0)
 }