Add query string to listing artifact API to support specify only show the tagged artifacts

Specify the query string "tags=NOT_NULL" to the listing artifact API to only return the tagged artifacts

Signed-off-by: Wenkai Yin <yinw@vmware.com>

api/v2.0/swagger.yaml

@@ -34,6 +34,11 @@ paths:
           description: Query the artifacts by type. Valid values can be "IMAGE", "CHART", etc.
           type: string
           required: false
+        - name: tags
+          in: query
+          description: Without the "tags" query string, both tagged and untagged artifacts will be returned. Specify it as "*" to return only tagged artifacts; specify it as "nil" to return only untagged artifacts.
+          type: string
+          required: false
         - name: with_tag
           in: query
           description: Specify whether the tags are inclued inside the returning artifacts
@@ -102,6 +107,8 @@ paths:
         - $ref: '#/parameters/projectName'
         - $ref: '#/parameters/repositoryName'
         - $ref: '#/parameters/reference'
+        - $ref: '#/parameters/page'
+        - $ref: '#/parameters/pageSize'
         - name: with_tag
           in: query
           description: Specify whether the tags are inclued inside the returning artifacts

src/pkg/artifact/dao/dao.go

@@ -16,6 +16,7 @@ package dao
 
 import (
 	"context"
+	beegoorm "github.com/astaxie/beego/orm"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/internal/orm"
 	"github.com/goharbor/harbor/src/pkg/q"
@@ -43,6 +44,27 @@ type DAO interface {
 	DeleteReferences(ctx context.Context, parentID int64) (err error)
 }
 
+const (
+	// TODO replace the table name "artifact_2" after upgrade
+	// both tagged and untagged artifacts
+	all = `IN (
+		SELECT DISTINCT art.id FROM artifact_2 art
+		LEFT JOIN tag ON art.id=tag.artifact_id 
+		LEFT JOIN artifact_reference ref ON art.id=ref.child_id
+		WHERE tag.id IS NOT NULL OR ref.id IS NULL)`
+	// only untagged artifacts
+	untagged = `IN (
+		SELECT DISTINCT art.id FROM artifact_2 art
+		LEFT JOIN tag ON art.id=tag.artifact_id 
+		LEFT JOIN artifact_reference ref ON art.id=ref.child_id
+		WHERE tag.id IS NULL AND ref.id IS NULL)`
+	// only tagged artifacts
+	tagged = `IN (
+		SELECT DISTINCT art.id FROM artifact_2 art
+		LEFT JOIN tag ON art.id=tag.artifact_id
+		WHERE tag.id IS NOT NULL)`
+)
+
 // New returns an instance of the default DAO
 func New() DAO {
 	return &dao{}
@@ -57,7 +79,7 @@ func (d *dao) Count(ctx context.Context, query *q.Query) (int64, error) {
 			Keywords: query.Keywords,
 		}
 	}
-	qs, err := orm.QuerySetter(ctx, &Artifact{}, query)
+	qs, err := querySetter(ctx, query)
 	if err != nil {
 		return 0, err
 	}
@@ -65,7 +87,7 @@ func (d *dao) Count(ctx context.Context, query *q.Query) (int64, error) {
 }
 func (d *dao) List(ctx context.Context, query *q.Query) ([]*Artifact, error) {
 	artifacts := []*Artifact{}
-	qs, err := orm.QuerySetter(ctx, &Artifact{}, query)
+	qs, err := querySetter(ctx, query)
 	if err != nil {
 		return nil, err
 	}
@@ -181,3 +203,33 @@ func (d *dao) DeleteReferences(ctx context.Context, parentID int64) error {
 	_, err = qs.Delete()
 	return err
 }
+
+func querySetter(ctx context.Context, query *q.Query) (beegoorm.QuerySeter, error) {
+	// as we modify the query in this method, copy it to avoid the impact for the original one
+	query = q.Copy(query)
+	// show both tagged and untagged artifacts by default
+	rawFilter := all
+	if query != nil && len(query.Keywords) > 0 {
+		value, ok := query.Keywords["Tags"]
+		if ok {
+			switch value.(string) {
+			case "nil":
+				// only show untagged artifacts
+				rawFilter = untagged
+			case "*":
+				// only show tagged artifacts
+				rawFilter = tagged
+			default:
+				return nil, ierror.New(nil).WithCode(ierror.BadRequestCode).
+					WithMessage("invalid value of tags: %s", value.(string))
+			}
+			// as the "Tags" isn't a table column, remove the "Tags" from the query to avoid orm error
+			delete(query.Keywords, "Tags")
+		}
+	}
+	qs, err := orm.QuerySetter(ctx, &Artifact{}, query)
+	if err != nil {
+		return nil, err
+	}
+	return qs.FilterRaw("id", rawFilter), nil
+}

src/pkg/artifact/dao/dao_test.go

@@ -21,109 +21,176 @@ import (
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/internal/orm"
 	"github.com/goharbor/harbor/src/pkg/q"
+	tagdao "github.com/goharbor/harbor/src/pkg/tag/dao"
+	"github.com/goharbor/harbor/src/pkg/tag/model/tag"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/suite"
 	"testing"
 	"time"
 )
 
-var (
-	typee                   = "IMAGE"
-	mediaType               = "application/vnd.oci.image.config.v1+json"
-	manifestMediaType       = "application/vnd.oci.image.manifest.v1+json"
-	projectID         int64 = 1
-	repositoryID      int64 = 1
-	digest                  = "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180"
-)
-
 type daoTestSuite struct {
 	suite.Suite
-	dao         DAO
+	dao           DAO
-	artifactID  int64
+	tagDAO        tagdao.DAO
-	referenceID int64
+	parentArtID   int64
-	ctx         context.Context
+	childArt01ID  int64
+	childArt02ID  int64
+	reference01ID int64
+	reference02ID int64
+	tagID         int64
+	ctx           context.Context
 }
 
 func (d *daoTestSuite) SetupSuite() {
 	d.dao = New()
+	d.tagDAO = tagdao.New()
 	common_dao.PrepareTestForPostgresSQL()
 	d.ctx = orm.NewContext(nil, beegoorm.NewOrm())
 }
 
 func (d *daoTestSuite) SetupTest() {
-	artifact := &Artifact{
+	now := time.Now()
-		Type:              typee,
+	parentArt := &Artifact{
-		MediaType:         mediaType,
+		Type:              "IMAGE",
-		ManifestMediaType: manifestMediaType,
+		MediaType:         v1.MediaTypeImageConfig,
-		ProjectID:         projectID,
+		ManifestMediaType: v1.MediaTypeImageIndex,
-		RepositoryID:      repositoryID,
+		ProjectID:         1,
-		Digest:            digest,
+		RepositoryID:      1,
-		Size:              1024,
+		Digest:            "parent_digest",
-		PushTime:          time.Now(),
+		PushTime:          now,
-		PullTime:          time.Now(),
+		PullTime:          now,
-		ExtraAttrs:        `{"attr1":"value1"}`,
 		Annotations:       `{"anno1":"value1"}`,
 	}
-	id, err := d.dao.Create(d.ctx, artifact)
+	id, err := d.dao.Create(d.ctx, parentArt)
 	d.Require().Nil(err)
-	d.artifactID = id
+	d.parentArtID = id
+
+	childArt01 := &Artifact{
+		Type:              "IMAGE",
+		MediaType:         v1.MediaTypeImageConfig,
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		ProjectID:         1,
+		RepositoryID:      1,
+		Digest:            "child_digest_01",
+		Size:              1024,
+		PushTime:          now,
+		PullTime:          now,
+		ExtraAttrs:        `{"attr1":"value1"}`,
+	}
+	id, err = d.dao.Create(d.ctx, childArt01)
+	d.Require().Nil(err)
+	d.childArt01ID = id
+
+	childArt02 := &Artifact{
+		Type:              "IMAGE",
+		MediaType:         v1.MediaTypeImageConfig,
+		ManifestMediaType: v1.MediaTypeImageManifest,
+		ProjectID:         1,
+		RepositoryID:      1,
+		Digest:            "child_digest_02",
+		Size:              1024,
+		PushTime:          now,
+		PullTime:          now,
+		ExtraAttrs:        `{"attr1":"value1"}`,
+	}
+	id, err = d.dao.Create(d.ctx, childArt02)
+	d.Require().Nil(err)
+	d.childArt02ID = id
 
 	id, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
-		ParentID: d.artifactID,
+		ParentID: d.parentArtID,
-		ChildID:  d.artifactID,
+		ChildID:  d.childArt01ID,
 	})
 	d.Require().Nil(err)
-	d.referenceID = id
+	d.reference01ID = id
+
+	id, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
+		ParentID: d.parentArtID,
+		ChildID:  d.childArt02ID,
+	})
+	d.Require().Nil(err)
+	d.reference02ID = id
+
+	id, err = d.tagDAO.Create(d.ctx, &tag.Tag{
+		RepositoryID: 1,
+		ArtifactID:   d.childArt01ID,
+		Name:         "latest",
+		PushTime:     now,
+		PullTime:     now,
+	})
+	d.Require().Nil(err)
+	d.tagID = id
 }
 
 func (d *daoTestSuite) TearDownTest() {
-	err := d.dao.DeleteReferences(d.ctx, d.artifactID)
+	err := d.dao.DeleteReferences(d.ctx, d.parentArtID)
 	d.Require().Nil(err)
-	err = d.dao.Delete(d.ctx, d.artifactID)
+	d.tagDAO.Delete(d.ctx, d.tagID)
+	d.Require().Nil(err)
+	err = d.dao.Delete(d.ctx, d.childArt01ID)
+	d.Require().Nil(err)
+	err = d.dao.Delete(d.ctx, d.childArt02ID)
+	d.Require().Nil(err)
+	err = d.dao.Delete(d.ctx, d.parentArtID)
 	d.Require().Nil(err)
 }
 
 func (d *daoTestSuite) TestCount() {
-	// nil query
+	// query by repository ID: both tagged and untagged artifacts
-	total, err := d.dao.Count(d.ctx, nil)
+	totalOfAll, err := d.dao.Count(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+		},
+	})
 	d.Require().Nil(err)
-	d.True(total > 0)
+	d.True(totalOfAll >= 2)
+
+	// only query tagged artifacts
+	totalOfTagged, err := d.dao.Count(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+			"Tags":         "*",
+		},
+	})
+	d.Require().Nil(err)
+	d.Equal(totalOfAll-1, totalOfTagged)
+
+	// only query untagged artifacts
+	totalOfUnTagged, err := d.dao.Count(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+			"Tags":         "nil",
+		},
+	})
+	d.Require().Nil(err)
+	d.Equal(totalOfAll-1, totalOfUnTagged)
+
+	// invalid tags value
+	_, err = d.dao.Count(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+			"Tags":         "invalid_value",
+		},
+	})
+	d.Require().NotNil(err)
+	d.True(ierror.IsErr(err, ierror.BadRequestCode))
 
 	// query by repository ID and digest
-	total, err = d.dao.Count(d.ctx, &q.Query{
+	total, err := d.dao.Count(d.ctx, &q.Query{
 		Keywords: map[string]interface{}{
-			"repository_id": repositoryID,
+			"RepositoryID": 1,
-			"digest":        digest,
+			"Digest":       "parent_digest",
 		},
 	})
 	d.Require().Nil(err)
 	d.Equal(int64(1), total)
 
-	// query by repository ID and digest
-	total, err = d.dao.Count(d.ctx, &q.Query{
-		Keywords: map[string]interface{}{
-			"repository_id": repositoryID,
-			"digest":        digest,
-		},
-	})
-	d.Require().Nil(err)
-	d.Equal(int64(1), total)
-
-	// populate more data
-	id, err := d.dao.Create(d.ctx, &Artifact{
-		Type:              typee,
-		MediaType:         mediaType,
-		ManifestMediaType: manifestMediaType,
-		ProjectID:         projectID,
-		RepositoryID:      repositoryID,
-		Digest:            "sha256:digest",
-	})
-	d.Require().Nil(err)
-	defer func() {
-		err = d.dao.Delete(d.ctx, id)
-		d.Require().Nil(err)
-	}()
 	// set pagination in query
 	total, err = d.dao.Count(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+		},
 		PageNumber: 1,
 		PageSize:   1,
 	})
@@ -132,28 +199,114 @@ func (d *daoTestSuite) TestCount() {
 }
 
 func (d *daoTestSuite) TestList() {
-	// nil query
+	// query by repository ID: both tagged and untagged artifacts
-	artifacts, err := d.dao.List(d.ctx, nil)
+	artifacts, err := d.dao.List(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+		},
+	})
 	d.Require().Nil(err)
-	found := false
+
+	parentArtFound := false
+	childArt01Found := false
+	childArt02Found := false
 	for _, artifact := range artifacts {
-		if artifact.ID == d.artifactID {
+		if artifact.ID == d.parentArtID {
-			found = true
+			parentArtFound = true
-			break
+			continue
+		}
+		if artifact.ID == d.childArt01ID {
+			childArt01Found = true
+			continue
+		}
+		if artifact.ID == d.childArt02ID {
+			childArt02Found = true
+			continue
 		}
 	}
-	d.True(found)
+	d.True(parentArtFound)
+	d.True(childArt01Found)
+	d.False(childArt02Found)
+
+	// only query tagged artifacts
+	artifacts, err = d.dao.List(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+			"Tags":         "*",
+		},
+	})
+	d.Require().Nil(err)
+	parentArtFound = false
+	childArt01Found = false
+	childArt02Found = false
+	for _, artifact := range artifacts {
+		if artifact.ID == d.parentArtID {
+			parentArtFound = true
+			continue
+		}
+		if artifact.ID == d.childArt01ID {
+			childArt01Found = true
+			continue
+		}
+		if artifact.ID == d.childArt02ID {
+			childArt02Found = true
+			continue
+		}
+	}
+	d.False(parentArtFound)
+	d.True(childArt01Found)
+	d.False(childArt02Found)
+
+	// only query untagged artifacts
+	artifacts, err = d.dao.List(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+			"Tags":         "nil",
+		},
+	})
+	d.Require().Nil(err)
+	parentArtFound = false
+	childArt01Found = false
+	childArt02Found = false
+	for _, artifact := range artifacts {
+		if artifact.ID == d.parentArtID {
+			parentArtFound = true
+			continue
+		}
+		if artifact.ID == d.childArt01ID {
+			childArt01Found = true
+			continue
+		}
+		if artifact.ID == d.childArt02ID {
+			childArt02Found = true
+			continue
+		}
+	}
+	d.True(parentArtFound)
+	d.False(childArt01Found)
+	d.False(childArt02Found)
 
 	// query by repository ID and digest
 	artifacts, err = d.dao.List(d.ctx, &q.Query{
 		Keywords: map[string]interface{}{
-			"repository_id": repositoryID,
+			"RepositoryID": 1,
-			"digest":        digest,
+			"Digest":       "parent_digest",
 		},
 	})
 	d.Require().Nil(err)
-	d.Require().Equal(1, len(artifacts))
+	d.Require().Len(artifacts, 1)
-	d.Equal(d.artifactID, artifacts[0].ID)
+	d.Equal(d.parentArtID, artifacts[0].ID)
+
+	// set pagination in query
+	artifacts, err = d.dao.List(d.ctx, &q.Query{
+		Keywords: map[string]interface{}{
+			"RepositoryID": 1,
+		},
+		PageNumber: 1,
+		PageSize:   1,
+	})
+	d.Require().Nil(err)
+	d.Require().Len(artifacts, 1)
 }
 
 func (d *daoTestSuite) TestGet() {
@@ -163,10 +316,10 @@ func (d *daoTestSuite) TestGet() {
 	d.True(ierror.IsErr(err, ierror.NotFoundCode))
 
 	// get the exist artifact
-	artifact, err := d.dao.Get(d.ctx, d.artifactID)
+	artifact, err := d.dao.Get(d.ctx, d.parentArtID)
 	d.Require().Nil(err)
 	d.Require().NotNil(artifact)
-	d.Equal(d.artifactID, artifact.ID)
+	d.Equal(d.parentArtID, artifact.ID)
 }
 
 func (d *daoTestSuite) TestCreate() {
@@ -174,12 +327,12 @@ func (d *daoTestSuite) TestCreate() {
 
 	// conflict
 	artifact := &Artifact{
-		Type:              typee,
+		Type:              "IMAGE",
-		MediaType:         mediaType,
+		MediaType:         v1.MediaTypeImageConfig,
-		ManifestMediaType: manifestMediaType,
+		ManifestMediaType: v1.MediaTypeImageManifest,
-		ProjectID:         projectID,
+		ProjectID:         1,
-		RepositoryID:      repositoryID,
+		RepositoryID:      1,
-		Digest:            digest,
+		Digest:            "child_digest_01",
 		Size:              1024,
 		PushTime:          time.Now(),
 		PullTime:          time.Now(),
@@ -200,7 +353,7 @@ func (d *daoTestSuite) TestDelete() {
 	d.True(ierror.IsErr(err, ierror.NotFoundCode))
 
 	// foreign key constraint
-	err = d.dao.Delete(d.ctx, d.artifactID)
+	err = d.dao.Delete(d.ctx, d.childArt01ID)
 	d.Require().NotNil(err)
 	d.True(ierror.IsErr(err, ierror.ViolateForeignKeyConstraintCode))
 }
@@ -209,12 +362,12 @@ func (d *daoTestSuite) TestUpdate() {
 	// pass
 	now := time.Now()
 	err := d.dao.Update(d.ctx, &Artifact{
-		ID:       d.artifactID,
+		ID:       d.parentArtID,
 		PushTime: now,
 	}, "PushTime")
 	d.Require().Nil(err)
 
-	artifact, err := d.dao.Get(d.ctx, d.artifactID)
+	artifact, err := d.dao.Get(d.ctx, d.parentArtID)
 	d.Require().Nil(err)
 	d.Require().NotNil(artifact)
 	d.Equal(now.Unix(), artifact.PullTime.Unix())
@@ -232,15 +385,15 @@ func (d *daoTestSuite) TestCreateReference() {
 
 	// conflict
 	_, err := d.dao.CreateReference(d.ctx, &ArtifactReference{
-		ParentID: d.artifactID,
+		ParentID: d.parentArtID,
-		ChildID:  d.artifactID,
+		ChildID:  d.childArt01ID,
 	})
 	d.Require().NotNil(err)
 	d.True(ierror.IsErr(err, ierror.ConflictCode))
 
 	// foreign key constraint
 	_, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
-		ParentID: d.artifactID,
+		ParentID: d.parentArtID,
 		ChildID:  1000,
 	})
 	d.Require().NotNil(err)
@@ -250,12 +403,13 @@ func (d *daoTestSuite) TestCreateReference() {
 func (d *daoTestSuite) TestListReferences() {
 	references, err := d.dao.ListReferences(d.ctx, &q.Query{
 		Keywords: map[string]interface{}{
-			"parent_id": d.artifactID,
+			"ParentID": d.parentArtID,
+			"ChildID":  d.childArt01ID,
 		},
 	})
 	d.Require().Nil(err)
 	d.Require().Equal(1, len(references))
-	d.Equal(d.referenceID, references[0].ID)
+	d.Equal(d.reference01ID, references[0].ID)
 }
 
 func (d *daoTestSuite) TestDeleteReferences() {

src/pkg/q/query.go

@@ -23,3 +23,19 @@ type Query struct {
 	// List of key words
 	Keywords map[string]interface{}
 }
+
+// Copy the specified query object
+func Copy(query *Query) *Query {
+	if query == nil {
+		return nil
+	}
+	q := &Query{
+		PageNumber: query.PageNumber,
+		PageSize:   query.PageSize,
+		Keywords:   map[string]interface{}{},
+	}
+	for key, value := range query.Keywords {
+		q.Keywords[key] = value
+	}
+	return q
+}

src/pkg/q/query_test.go

@@ -0,0 +1,46 @@
+// 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 q
+
+import (
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestCopy(t *testing.T) {
+	// nil
+	q := Copy(nil)
+	assert.Nil(t, q)
+
+	// not nil
+	query := &Query{
+		PageNumber: 1,
+		PageSize:   10,
+		Keywords: map[string]interface{}{
+			"key": "value",
+		},
+	}
+	q = Copy(query)
+	require.NotNil(t, q)
+	assert.Equal(t, int64(1), q.PageNumber)
+	assert.Equal(t, int64(10), q.PageSize)
+	assert.Equal(t, "value", q.Keywords["key"].(string))
+	// changes for the copy doesn't effect the original one
+	q.PageSize = 20
+	q.Keywords["key"] = "value2"
+	assert.Equal(t, int64(10), query.PageSize)
+	assert.Equal(t, "value", query.Keywords["key"].(string))
+}

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

@@ -58,6 +58,9 @@ func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListAr
 	if params.Type != nil {
 		query.Keywords["Type"] = *(params.Type)
 	}
+	if params.Tags != nil {
+		query.Keywords["Tags"] = *(params.Tags)
+	}
 	if params.Page != nil {
 		query.PageNumber = *(params.Page)
 	}