mirror of
https://github.com/goharbor/harbor.git
synced 2024-09-28 21:37:31 +02:00
Add foreign key to avoid the concurrent issue
Add foreign key to avoid the concurrent issue when operating the artifacts, tags and references Signed-off-by: Wenkai Yin <yinw@vmware.com>
This commit is contained in:
parent
603cc0f5f3
commit
7dc28bcff9
@ -5,8 +5,7 @@ CREATE TABLE artifact_2
|
|||||||
/* image, chart, etc */
|
/* image, chart, etc */
|
||||||
type varchar(255) NOT NULL,
|
type varchar(255) NOT NULL,
|
||||||
media_type varchar(255) NOT NULL,
|
media_type varchar(255) NOT NULL,
|
||||||
/* the media type of some classical image manifest can be null, so don't add the "NOT NULL" constraint*/
|
manifest_media_type varchar(255) NOT NULL,
|
||||||
manifest_media_type varchar(255),
|
|
||||||
project_id int NOT NULL,
|
project_id int NOT NULL,
|
||||||
repository_id int NOT NULL,
|
repository_id int NOT NULL,
|
||||||
digest varchar(255) NOT NULL,
|
digest varchar(255) NOT NULL,
|
||||||
@ -26,6 +25,8 @@ CREATE TABLE tag
|
|||||||
name varchar(255) NOT NULL,
|
name varchar(255) NOT NULL,
|
||||||
push_time timestamp default CURRENT_TIMESTAMP,
|
push_time timestamp default CURRENT_TIMESTAMP,
|
||||||
pull_time timestamp,
|
pull_time timestamp,
|
||||||
|
/* TODO replace artifact_2 after finishing the upgrade work */
|
||||||
|
FOREIGN KEY (artifact_id) REFERENCES artifact_2(id),
|
||||||
CONSTRAINT unique_tag UNIQUE (repository_id, name)
|
CONSTRAINT unique_tag UNIQUE (repository_id, name)
|
||||||
);
|
);
|
||||||
|
|
||||||
@ -36,5 +37,8 @@ CREATE TABLE artifact_reference
|
|||||||
parent_id int NOT NULL,
|
parent_id int NOT NULL,
|
||||||
child_id int NOT NULL,
|
child_id int NOT NULL,
|
||||||
platform varchar(255),
|
platform varchar(255),
|
||||||
|
/* TODO replace artifact_2 after finishing the upgrade work */
|
||||||
|
FOREIGN KEY (parent_id) REFERENCES artifact_2(id),
|
||||||
|
FOREIGN KEY (child_id) REFERENCES artifact_2(id),
|
||||||
CONSTRAINT unique_reference UNIQUE (parent_id, child_id)
|
CONSTRAINT unique_reference UNIQUE (parent_id, child_id)
|
||||||
);
|
);
|
@ -66,7 +66,6 @@ func (a *abstractor) Abstract(ctx context.Context, artifact *artifact.Artifact)
|
|||||||
switch artifact.ManifestMediaType {
|
switch artifact.ManifestMediaType {
|
||||||
// docker manifest v1
|
// docker manifest v1
|
||||||
case "", "application/json", schema1.MediaTypeSignedManifest:
|
case "", "application/json", schema1.MediaTypeSignedManifest:
|
||||||
// TODO as the manifestmediatype isn't null, so add not null constraint in database
|
|
||||||
// unify the media type of v1 manifest to "schema1.MediaTypeSignedManifest"
|
// unify the media type of v1 manifest to "schema1.MediaTypeSignedManifest"
|
||||||
artifact.ManifestMediaType = schema1.MediaTypeSignedManifest
|
artifact.ManifestMediaType = schema1.MediaTypeSignedManifest
|
||||||
// as no config layer in the docker v1 manifest, use the "schema1.MediaTypeSignedManifest"
|
// as no config layer in the docker v1 manifest, use the "schema1.MediaTypeSignedManifest"
|
||||||
|
@ -75,9 +75,7 @@ func NewController() Controller {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO handle concurrency, the redis lock doesn't cover all cases
|
// TODO concurrency summary
|
||||||
// TODO As a redis lock is applied during the artifact pushing, we do not to handle the concurrent issues
|
|
||||||
// for artifacts and tags??
|
|
||||||
|
|
||||||
type controller struct {
|
type controller struct {
|
||||||
repoMgr repository.Manager
|
repoMgr repository.Manager
|
||||||
|
@ -20,7 +20,6 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
// Artifact is the overall view of artifact
|
// Artifact is the overall view of artifact
|
||||||
// TODO reuse the model generated by swagger
|
|
||||||
type Artifact struct {
|
type Artifact struct {
|
||||||
artifact.Artifact
|
artifact.Artifact
|
||||||
Tags []*Tag // the list of tags that attached to the artifact
|
Tags []*Tag // the list of tags that attached to the artifact
|
||||||
|
@ -90,9 +90,10 @@ const (
|
|||||||
PreconditionCode = "PRECONDITION"
|
PreconditionCode = "PRECONDITION"
|
||||||
// GeneralCode ...
|
// GeneralCode ...
|
||||||
GeneralCode = "UNKNOWN"
|
GeneralCode = "UNKNOWN"
|
||||||
|
|
||||||
// DENIED it's used by middleware(readonly, vul and content trust) and returned to docker client to index the request is denied.
|
// DENIED it's used by middleware(readonly, vul and content trust) and returned to docker client to index the request is denied.
|
||||||
DENIED = "DENIED"
|
DENIED = "DENIED"
|
||||||
|
// ViolateForeignKeyConstraintCode is the error code for violating foreign key constraint error
|
||||||
|
ViolateForeignKeyConstraintCode = "VIOLATE_FOREIGN_KEY_CONSTRAINT"
|
||||||
)
|
)
|
||||||
|
|
||||||
// New ...
|
// New ...
|
||||||
|
@ -18,34 +18,44 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"github.com/astaxie/beego/orm"
|
"github.com/astaxie/beego/orm"
|
||||||
ierror "github.com/goharbor/harbor/src/internal/error"
|
ierror "github.com/goharbor/harbor/src/internal/error"
|
||||||
"strings"
|
"github.com/lib/pq"
|
||||||
)
|
)
|
||||||
|
|
||||||
// IsNotFoundError checks whether the err is orm.ErrNoRows. If it it, wrap it
|
// AsNotFoundError checks whether the err is orm.ErrNoRows. If it it, wrap it
|
||||||
// as a src/internal/error.Error with not found error code
|
// as a src/internal/error.Error with not found error code, else return nil
|
||||||
func IsNotFoundError(err error, messageFormat string, args ...interface{}) (*ierror.Error, bool) {
|
func AsNotFoundError(err error, messageFormat string, args ...interface{}) *ierror.Error {
|
||||||
if errors.Is(err, orm.ErrNoRows) {
|
if errors.Is(err, orm.ErrNoRows) {
|
||||||
e := ierror.NotFoundError(err)
|
e := ierror.NotFoundError(err)
|
||||||
if len(messageFormat) > 0 {
|
if len(messageFormat) > 0 {
|
||||||
e.WithMessage(messageFormat, args...)
|
e.WithMessage(messageFormat, args...)
|
||||||
}
|
}
|
||||||
return e, true
|
return e
|
||||||
}
|
}
|
||||||
return nil, false
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// IsConflictError checks whether the err is duplicate key error. If it it, wrap it
|
// AsConflictError checks whether the err is duplicate key error. If it it, wrap it
|
||||||
// as a src/internal/error.Error with conflict error code
|
// as a src/internal/error.Error with conflict error code, else return nil
|
||||||
func IsConflictError(err error, messageFormat string, args ...interface{}) (*ierror.Error, bool) {
|
func AsConflictError(err error, messageFormat string, args ...interface{}) *ierror.Error {
|
||||||
if err == nil {
|
var pqErr *pq.Error
|
||||||
return nil, false
|
if errors.As(err, &pqErr) && pqErr.Code == "23505" {
|
||||||
|
e := ierror.New(err).
|
||||||
|
WithCode(ierror.ConflictCode).
|
||||||
|
WithMessage(messageFormat, args...)
|
||||||
|
return e
|
||||||
}
|
}
|
||||||
if strings.Contains(err.Error(), "duplicate key value violates unique constraint") {
|
return nil
|
||||||
e := ierror.ConflictError(err)
|
}
|
||||||
if len(messageFormat) > 0 {
|
|
||||||
e.WithMessage(messageFormat, args...)
|
// AsForeignKeyError checks whether the err is violating foreign key constraint error. If it it, wrap it
|
||||||
}
|
// as a src/internal/error.Error with violating foreign key constraint error code, else return nil
|
||||||
return e, true
|
func AsForeignKeyError(err error, messageFormat string, args ...interface{}) *ierror.Error {
|
||||||
}
|
var pqErr *pq.Error
|
||||||
return nil, false
|
if errors.As(err, &pqErr) && pqErr.Code == "23503" {
|
||||||
|
e := ierror.New(err).
|
||||||
|
WithCode(ierror.ViolateForeignKeyConstraintCode).
|
||||||
|
WithMessage(messageFormat, args...)
|
||||||
|
return e
|
||||||
|
}
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
@ -18,40 +18,63 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"github.com/astaxie/beego/orm"
|
"github.com/astaxie/beego/orm"
|
||||||
"github.com/goharbor/harbor/src/internal/error"
|
"github.com/goharbor/harbor/src/internal/error"
|
||||||
|
"github.com/lib/pq"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestIsNotFoundError(t *testing.T) {
|
func TestIsNotFoundError(t *testing.T) {
|
||||||
// nil error
|
// nil error
|
||||||
_, ok := IsNotFoundError(nil, "")
|
err := AsNotFoundError(nil, "")
|
||||||
assert.False(t, ok)
|
assert.Nil(t, err)
|
||||||
|
|
||||||
// common error
|
// common error
|
||||||
_, ok = IsNotFoundError(errors.New("common error"), "")
|
err = AsNotFoundError(errors.New("common error"), "")
|
||||||
assert.False(t, ok)
|
assert.Nil(t, err)
|
||||||
|
|
||||||
// pass
|
// pass
|
||||||
message := "message"
|
message := "message"
|
||||||
e, ok := IsNotFoundError(orm.ErrNoRows, message)
|
err = AsNotFoundError(orm.ErrNoRows, message)
|
||||||
assert.True(t, ok)
|
require.NotNil(t, err)
|
||||||
assert.Equal(t, error.NotFoundCode, e.Code)
|
assert.Equal(t, error.NotFoundCode, err.Code)
|
||||||
assert.Equal(t, message, e.Message)
|
assert.Equal(t, message, err.Message)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestIsConflictError(t *testing.T) {
|
func TestIsConflictError(t *testing.T) {
|
||||||
// nil error
|
// nil error
|
||||||
_, ok := IsConflictError(nil, "")
|
err := AsConflictError(nil, "")
|
||||||
assert.False(t, ok)
|
assert.Nil(t, err)
|
||||||
|
|
||||||
// common error
|
// common error
|
||||||
_, ok = IsConflictError(errors.New("common error"), "")
|
err = AsConflictError(errors.New("common error"), "")
|
||||||
assert.False(t, ok)
|
assert.Nil(t, err)
|
||||||
|
|
||||||
// pass
|
// pass
|
||||||
message := "message"
|
message := "message"
|
||||||
e, ok := IsConflictError(errors.New("duplicate key value violates unique constraint"), message)
|
err = AsConflictError(&pq.Error{
|
||||||
assert.True(t, ok)
|
Code: "23505",
|
||||||
assert.Equal(t, error.ConflictCode, e.Code)
|
}, message)
|
||||||
assert.Equal(t, message, e.Message)
|
require.NotNil(t, err)
|
||||||
|
assert.Equal(t, error.ConflictCode, err.Code)
|
||||||
|
assert.Equal(t, message, err.Message)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestIsForeignKeyError(t *testing.T) {
|
||||||
|
// nil error
|
||||||
|
err := AsForeignKeyError(nil, "")
|
||||||
|
assert.Nil(t, err)
|
||||||
|
|
||||||
|
// common error
|
||||||
|
err = AsForeignKeyError(errors.New("common error"), "")
|
||||||
|
assert.Nil(t, err)
|
||||||
|
|
||||||
|
// pass
|
||||||
|
message := "message"
|
||||||
|
err = AsForeignKeyError(&pq.Error{
|
||||||
|
Code: "23503",
|
||||||
|
}, message)
|
||||||
|
require.NotNil(t, err)
|
||||||
|
assert.Equal(t, error.ViolateForeignKeyConstraintCode, err.Code)
|
||||||
|
assert.Equal(t, message, err.Message)
|
||||||
}
|
}
|
||||||
|
@ -83,7 +83,7 @@ func (d *dao) Get(ctx context.Context, id int64) (*Artifact, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err = ormer.Read(artifact); err != nil {
|
if err = ormer.Read(artifact); err != nil {
|
||||||
if e, ok := orm.IsNotFoundError(err, "artifact %d not found", id); ok {
|
if e := orm.AsNotFoundError(err, "artifact %d not found", id); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return nil, err
|
return nil, err
|
||||||
@ -97,8 +97,8 @@ func (d *dao) Create(ctx context.Context, artifact *Artifact) (int64, error) {
|
|||||||
}
|
}
|
||||||
id, err := ormer.Insert(artifact)
|
id, err := ormer.Insert(artifact)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if e, ok := orm.IsConflictError(err, "artifact %s already exists under the repository %d",
|
if e := orm.AsConflictError(err, "artifact %s already exists under the repository %d",
|
||||||
artifact.Digest, artifact.RepositoryID); ok {
|
artifact.Digest, artifact.RepositoryID); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -113,6 +113,10 @@ func (d *dao) Delete(ctx context.Context, id int64) error {
|
|||||||
ID: id,
|
ID: id,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if e := orm.AsForeignKeyError(err,
|
||||||
|
"the artifact %d is referenced by other resources", id); e != nil {
|
||||||
|
err = e
|
||||||
|
}
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if n == 0 {
|
if n == 0 {
|
||||||
@ -140,8 +144,11 @@ func (d *dao) CreateReference(ctx context.Context, reference *ArtifactReference)
|
|||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
id, err := ormer.Insert(reference)
|
id, err := ormer.Insert(reference)
|
||||||
if e, ok := orm.IsConflictError(err, "reference already exists, parent artifact ID: %d, child artifact ID: %d",
|
if e := orm.AsConflictError(err, "reference already exists, parent artifact ID: %d, child artifact ID: %d",
|
||||||
reference.ParentID, reference.ChildID); ok {
|
reference.ParentID, reference.ChildID); e != nil {
|
||||||
|
err = e
|
||||||
|
} else if e := orm.AsForeignKeyError(err, "the reference tries to reference a non existing artifact, parent artifact ID: %d, child artifact ID: %d",
|
||||||
|
reference.ParentID, reference.ChildID); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return id, err
|
return id, err
|
||||||
|
@ -16,7 +16,6 @@ package dao
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
|
||||||
beegoorm "github.com/astaxie/beego/orm"
|
beegoorm "github.com/astaxie/beego/orm"
|
||||||
common_dao "github.com/goharbor/harbor/src/common/dao"
|
common_dao "github.com/goharbor/harbor/src/common/dao"
|
||||||
ierror "github.com/goharbor/harbor/src/internal/error"
|
ierror "github.com/goharbor/harbor/src/internal/error"
|
||||||
@ -38,9 +37,10 @@ var (
|
|||||||
|
|
||||||
type daoTestSuite struct {
|
type daoTestSuite struct {
|
||||||
suite.Suite
|
suite.Suite
|
||||||
dao DAO
|
dao DAO
|
||||||
artifactID int64
|
artifactID int64
|
||||||
ctx context.Context
|
referenceID int64
|
||||||
|
ctx context.Context
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) SetupSuite() {
|
func (d *daoTestSuite) SetupSuite() {
|
||||||
@ -66,10 +66,19 @@ func (d *daoTestSuite) SetupTest() {
|
|||||||
id, err := d.dao.Create(d.ctx, artifact)
|
id, err := d.dao.Create(d.ctx, artifact)
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
d.artifactID = id
|
d.artifactID = id
|
||||||
|
|
||||||
|
id, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
|
||||||
|
ParentID: d.artifactID,
|
||||||
|
ChildID: d.artifactID,
|
||||||
|
})
|
||||||
|
d.Require().Nil(err)
|
||||||
|
d.referenceID = id
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) TearDownTest() {
|
func (d *daoTestSuite) TearDownTest() {
|
||||||
err := d.dao.Delete(d.ctx, d.artifactID)
|
err := d.dao.DeleteReferences(d.ctx, d.artifactID)
|
||||||
|
d.Require().Nil(err)
|
||||||
|
err = d.dao.Delete(d.ctx, d.artifactID)
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -188,9 +197,12 @@ func (d *daoTestSuite) TestDelete() {
|
|||||||
// not exist
|
// not exist
|
||||||
err := d.dao.Delete(d.ctx, 100021)
|
err := d.dao.Delete(d.ctx, 100021)
|
||||||
d.Require().NotNil(err)
|
d.Require().NotNil(err)
|
||||||
var e *ierror.Error
|
d.True(ierror.IsErr(err, ierror.NotFoundCode))
|
||||||
d.Require().True(errors.As(err, &e))
|
|
||||||
d.Equal(ierror.NotFoundCode, e.Code)
|
// foreign key constraint
|
||||||
|
err = d.dao.Delete(d.ctx, d.artifactID)
|
||||||
|
d.Require().NotNil(err)
|
||||||
|
d.True(ierror.IsErr(err, ierror.ViolateForeignKeyConstraintCode))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) TestUpdate() {
|
func (d *daoTestSuite) TestUpdate() {
|
||||||
@ -212,46 +224,47 @@ func (d *daoTestSuite) TestUpdate() {
|
|||||||
ID: 10000,
|
ID: 10000,
|
||||||
})
|
})
|
||||||
d.Require().NotNil(err)
|
d.Require().NotNil(err)
|
||||||
var e *ierror.Error
|
d.True(ierror.IsErr(err, ierror.NotFoundCode))
|
||||||
d.Require().True(errors.As(err, &e))
|
|
||||||
d.Equal(ierror.NotFoundCode, e.Code)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) TestReference() {
|
func (d *daoTestSuite) TestCreateReference() {
|
||||||
// create reference
|
// happy pass is covered in SetupTest
|
||||||
id, err := d.dao.CreateReference(d.ctx, &ArtifactReference{
|
|
||||||
ParentID: d.artifactID,
|
|
||||||
ChildID: 10000,
|
|
||||||
})
|
|
||||||
d.Require().Nil(err)
|
|
||||||
|
|
||||||
// conflict
|
// conflict
|
||||||
_, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
|
_, err := d.dao.CreateReference(d.ctx, &ArtifactReference{
|
||||||
ParentID: d.artifactID,
|
ParentID: d.artifactID,
|
||||||
ChildID: 10000,
|
ChildID: d.artifactID,
|
||||||
})
|
})
|
||||||
d.Require().NotNil(err)
|
d.Require().NotNil(err)
|
||||||
d.True(ierror.IsErr(err, ierror.ConflictCode))
|
d.True(ierror.IsErr(err, ierror.ConflictCode))
|
||||||
|
|
||||||
// list reference
|
// foreign key constraint
|
||||||
|
_, err = d.dao.CreateReference(d.ctx, &ArtifactReference{
|
||||||
|
ParentID: d.artifactID,
|
||||||
|
ChildID: 1000,
|
||||||
|
})
|
||||||
|
d.Require().NotNil(err)
|
||||||
|
d.True(ierror.IsErr(err, ierror.ViolateForeignKeyConstraintCode))
|
||||||
|
}
|
||||||
|
|
||||||
|
func (d *daoTestSuite) TestListReferences() {
|
||||||
references, err := d.dao.ListReferences(d.ctx, &q.Query{
|
references, err := d.dao.ListReferences(d.ctx, &q.Query{
|
||||||
Keywords: map[string]interface{}{
|
Keywords: map[string]interface{}{
|
||||||
"parent_id": d.artifactID,
|
"parent_id": d.artifactID,
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
d.Require().Equal(1, len(references))
|
|
||||||
d.Equal(id, references[0].ID)
|
|
||||||
|
|
||||||
// delete reference
|
|
||||||
err = d.dao.DeleteReferences(d.ctx, d.artifactID)
|
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
|
d.Require().Equal(1, len(references))
|
||||||
|
d.Equal(d.referenceID, references[0].ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (d *daoTestSuite) TestDeleteReferences() {
|
||||||
|
// happy pass is covered in TearDownTest
|
||||||
|
|
||||||
// parent artifact not exist
|
// parent artifact not exist
|
||||||
err = d.dao.DeleteReferences(d.ctx, 10000)
|
err := d.dao.DeleteReferences(d.ctx, 10000)
|
||||||
d.Require().NotNil(err)
|
d.Require().NotNil(err)
|
||||||
var e *ierror.Error
|
d.True(ierror.IsErr(err, ierror.NotFoundCode))
|
||||||
d.Require().True(errors.As(err, &e))
|
|
||||||
d.Equal(ierror.NotFoundCode, e.Code)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestDaoTestSuite(t *testing.T) {
|
func TestDaoTestSuite(t *testing.T) {
|
||||||
|
@ -79,7 +79,7 @@ func (d *dao) Get(ctx context.Context, id int64) (*models.RepoRecord, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := ormer.Read(repository); err != nil {
|
if err := ormer.Read(repository); err != nil {
|
||||||
if e, ok := orm.IsNotFoundError(err, "repository %d not found", id); ok {
|
if e := orm.AsNotFoundError(err, "repository %d not found", id); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return nil, err
|
return nil, err
|
||||||
@ -93,7 +93,7 @@ func (d *dao) Create(ctx context.Context, repository *models.RepoRecord) (int64,
|
|||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
id, err := ormer.Insert(repository)
|
id, err := ormer.Insert(repository)
|
||||||
if e, ok := orm.IsConflictError(err, "repository %s already exists", repository.Name); ok {
|
if e := orm.AsConflictError(err, "repository %s already exists", repository.Name); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return id, err
|
return id, err
|
||||||
|
@ -83,7 +83,7 @@ func (d *dao) Get(ctx context.Context, id int64) (*tag.Tag, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if err := ormer.Read(tag); err != nil {
|
if err := ormer.Read(tag); err != nil {
|
||||||
if e, ok := orm.IsNotFoundError(err, "tag %d not found", id); ok {
|
if e := orm.AsNotFoundError(err, "tag %d not found", id); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return nil, err
|
return nil, err
|
||||||
@ -96,8 +96,11 @@ func (d *dao) Create(ctx context.Context, tag *tag.Tag) (int64, error) {
|
|||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
id, err := ormer.Insert(tag)
|
id, err := ormer.Insert(tag)
|
||||||
if e, ok := orm.IsConflictError(err, "tag %s already exists under the repository %d",
|
if e := orm.AsConflictError(err, "tag %s already exists under the repository %d",
|
||||||
tag.Name, tag.RepositoryID); ok {
|
tag.Name, tag.RepositoryID); e != nil {
|
||||||
|
err = e
|
||||||
|
} else if e := orm.AsForeignKeyError(err, "the tag %s tries to attach to a non existing artifact %d",
|
||||||
|
tag.Name, tag.ArtifactID); e != nil {
|
||||||
err = e
|
err = e
|
||||||
}
|
}
|
||||||
return id, err
|
return id, err
|
||||||
@ -109,6 +112,10 @@ func (d *dao) Update(ctx context.Context, tag *tag.Tag, props ...string) error {
|
|||||||
}
|
}
|
||||||
n, err := ormer.Update(tag, props...)
|
n, err := ormer.Update(tag, props...)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if e := orm.AsForeignKeyError(err, "the tag %d tries to attach to a non existing artifact %d",
|
||||||
|
tag.ID, tag.ArtifactID); e != nil {
|
||||||
|
err = e
|
||||||
|
}
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if n == 0 {
|
if n == 0 {
|
||||||
|
@ -21,6 +21,7 @@ import (
|
|||||||
common_dao "github.com/goharbor/harbor/src/common/dao"
|
common_dao "github.com/goharbor/harbor/src/common/dao"
|
||||||
ierror "github.com/goharbor/harbor/src/internal/error"
|
ierror "github.com/goharbor/harbor/src/internal/error"
|
||||||
"github.com/goharbor/harbor/src/internal/orm"
|
"github.com/goharbor/harbor/src/internal/orm"
|
||||||
|
artdao "github.com/goharbor/harbor/src/pkg/artifact/dao"
|
||||||
"github.com/goharbor/harbor/src/pkg/q"
|
"github.com/goharbor/harbor/src/pkg/q"
|
||||||
"github.com/goharbor/harbor/src/pkg/tag/model/tag"
|
"github.com/goharbor/harbor/src/pkg/tag/model/tag"
|
||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
@ -28,30 +29,43 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
var (
|
|
||||||
repositoryID int64 = 1000
|
|
||||||
artifactID int64 = 1000
|
|
||||||
name = "latest"
|
|
||||||
)
|
|
||||||
|
|
||||||
type daoTestSuite struct {
|
type daoTestSuite struct {
|
||||||
suite.Suite
|
suite.Suite
|
||||||
dao DAO
|
dao DAO
|
||||||
tagID int64
|
artDAO artdao.DAO
|
||||||
ctx context.Context
|
tagID int64
|
||||||
|
artifactID int64
|
||||||
|
ctx context.Context
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) SetupSuite() {
|
func (d *daoTestSuite) SetupSuite() {
|
||||||
d.dao = New()
|
d.dao = New()
|
||||||
common_dao.PrepareTestForPostgresSQL()
|
common_dao.PrepareTestForPostgresSQL()
|
||||||
d.ctx = orm.NewContext(nil, beegoorm.NewOrm())
|
d.ctx = orm.NewContext(nil, beegoorm.NewOrm())
|
||||||
|
d.artDAO = artdao.New()
|
||||||
|
artifactID, err := d.artDAO.Create(d.ctx, &artdao.Artifact{
|
||||||
|
Type: "IMAGE",
|
||||||
|
MediaType: "application/vnd.oci.image.config.v1+json",
|
||||||
|
ManifestMediaType: "application/vnd.oci.image.manifest.v1+json",
|
||||||
|
ProjectID: 1,
|
||||||
|
RepositoryID: 1000,
|
||||||
|
Digest: "sha256:digest",
|
||||||
|
})
|
||||||
|
d.Require().Nil(err)
|
||||||
|
d.artifactID = artifactID
|
||||||
|
}
|
||||||
|
|
||||||
|
func (d *daoTestSuite) TearDownSuite() {
|
||||||
|
err := d.artDAO.Delete(d.ctx, d.artifactID)
|
||||||
|
d.Require().Nil(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) SetupTest() {
|
func (d *daoTestSuite) SetupTest() {
|
||||||
|
|
||||||
tag := &tag.Tag{
|
tag := &tag.Tag{
|
||||||
RepositoryID: repositoryID,
|
RepositoryID: 1000,
|
||||||
ArtifactID: artifactID,
|
ArtifactID: d.artifactID,
|
||||||
Name: name,
|
Name: "latest",
|
||||||
PushTime: time.Time{},
|
PushTime: time.Time{},
|
||||||
PullTime: time.Time{},
|
PullTime: time.Time{},
|
||||||
}
|
}
|
||||||
@ -73,8 +87,8 @@ func (d *daoTestSuite) TestCount() {
|
|||||||
// query by repository ID and name
|
// query by repository ID and name
|
||||||
total, err = d.dao.Count(d.ctx, &q.Query{
|
total, err = d.dao.Count(d.ctx, &q.Query{
|
||||||
Keywords: map[string]interface{}{
|
Keywords: map[string]interface{}{
|
||||||
"repository_id": repositoryID,
|
"repository_id": 1000,
|
||||||
"name": name,
|
"name": "latest",
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
@ -97,8 +111,8 @@ func (d *daoTestSuite) TestList() {
|
|||||||
// query by repository ID and name
|
// query by repository ID and name
|
||||||
tags, err = d.dao.List(d.ctx, &q.Query{
|
tags, err = d.dao.List(d.ctx, &q.Query{
|
||||||
Keywords: map[string]interface{}{
|
Keywords: map[string]interface{}{
|
||||||
"repository_id": repositoryID,
|
"repository_id": 1000,
|
||||||
"name": name,
|
"name": "latest",
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
@ -123,16 +137,28 @@ func (d *daoTestSuite) TestCreate() {
|
|||||||
// the happy pass case is covered in Setup
|
// the happy pass case is covered in Setup
|
||||||
|
|
||||||
// conflict
|
// conflict
|
||||||
tag := &tag.Tag{
|
tg := &tag.Tag{
|
||||||
RepositoryID: repositoryID,
|
RepositoryID: 1000,
|
||||||
ArtifactID: artifactID,
|
ArtifactID: d.artifactID,
|
||||||
Name: name,
|
Name: "latest",
|
||||||
PushTime: time.Time{},
|
PushTime: time.Time{},
|
||||||
PullTime: time.Time{},
|
PullTime: time.Time{},
|
||||||
}
|
}
|
||||||
_, err := d.dao.Create(d.ctx, tag)
|
_, err := d.dao.Create(d.ctx, tg)
|
||||||
d.Require().NotNil(err)
|
d.Require().NotNil(err)
|
||||||
d.True(ierror.IsErr(err, ierror.ConflictCode))
|
d.True(ierror.IsErr(err, ierror.ConflictCode))
|
||||||
|
|
||||||
|
// violating foreign key constraint: the artifact that the tag tries to attach doesn't exist
|
||||||
|
tg = &tag.Tag{
|
||||||
|
RepositoryID: 1000,
|
||||||
|
ArtifactID: 1000,
|
||||||
|
Name: "latest2",
|
||||||
|
PushTime: time.Time{},
|
||||||
|
PullTime: time.Time{},
|
||||||
|
}
|
||||||
|
_, err = d.dao.Create(d.ctx, tg)
|
||||||
|
d.Require().NotNil(err)
|
||||||
|
d.True(ierror.IsErr(err, ierror.ViolateForeignKeyConstraintCode))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d *daoTestSuite) TestDelete() {
|
func (d *daoTestSuite) TestDelete() {
|
||||||
@ -148,16 +174,44 @@ func (d *daoTestSuite) TestDelete() {
|
|||||||
|
|
||||||
func (d *daoTestSuite) TestUpdate() {
|
func (d *daoTestSuite) TestUpdate() {
|
||||||
// pass
|
// pass
|
||||||
err := d.dao.Update(d.ctx, &tag.Tag{
|
artifactID, err := d.artDAO.Create(d.ctx, &artdao.Artifact{
|
||||||
|
Type: "IMAGE",
|
||||||
|
MediaType: "application/vnd.oci.image.config.v1+json",
|
||||||
|
ManifestMediaType: "application/vnd.oci.image.manifest.v1+json",
|
||||||
|
ProjectID: 1,
|
||||||
|
RepositoryID: 1000,
|
||||||
|
Digest: "sha256:digest2",
|
||||||
|
})
|
||||||
|
d.Require().Nil(err)
|
||||||
|
defer func() {
|
||||||
|
err := d.artDAO.Delete(d.ctx, artifactID)
|
||||||
|
d.Require().Nil(err)
|
||||||
|
}()
|
||||||
|
|
||||||
|
err = d.dao.Update(d.ctx, &tag.Tag{
|
||||||
ID: d.tagID,
|
ID: d.tagID,
|
||||||
ArtifactID: 2,
|
ArtifactID: artifactID,
|
||||||
}, "ArtifactID")
|
}, "ArtifactID")
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
|
|
||||||
tg, err := d.dao.Get(d.ctx, d.tagID)
|
tg, err := d.dao.Get(d.ctx, d.tagID)
|
||||||
d.Require().Nil(err)
|
d.Require().Nil(err)
|
||||||
d.Require().NotNil(tg)
|
d.Require().NotNil(tg)
|
||||||
d.Equal(int64(2), tg.ArtifactID)
|
d.Equal(artifactID, tg.ArtifactID)
|
||||||
|
|
||||||
|
err = d.dao.Update(d.ctx, &tag.Tag{
|
||||||
|
ID: d.tagID,
|
||||||
|
ArtifactID: d.artifactID,
|
||||||
|
}, "ArtifactID")
|
||||||
|
d.Require().Nil(err)
|
||||||
|
|
||||||
|
// violating foreign key constraint: the artifact that the tag tries to attach doesn't exist
|
||||||
|
err = d.dao.Update(d.ctx, &tag.Tag{
|
||||||
|
ID: d.tagID,
|
||||||
|
ArtifactID: 2,
|
||||||
|
}, "ArtifactID")
|
||||||
|
d.Require().NotNil(err)
|
||||||
|
d.True(ierror.IsErr(err, ierror.ViolateForeignKeyConstraintCode))
|
||||||
|
|
||||||
// not exist
|
// not exist
|
||||||
err = d.dao.Update(d.ctx, &tag.Tag{
|
err = d.dao.Update(d.ctx, &tag.Tag{
|
||||||
|
@ -25,13 +25,14 @@ import (
|
|||||||
|
|
||||||
var (
|
var (
|
||||||
codeMap = map[string]int{
|
codeMap = map[string]int{
|
||||||
ierror.BadRequestCode: http.StatusBadRequest,
|
ierror.BadRequestCode: http.StatusBadRequest,
|
||||||
ierror.UnAuthorizedCode: http.StatusUnauthorized,
|
ierror.UnAuthorizedCode: http.StatusUnauthorized,
|
||||||
ierror.ForbiddenCode: http.StatusForbidden,
|
ierror.ForbiddenCode: http.StatusForbidden,
|
||||||
ierror.NotFoundCode: http.StatusNotFound,
|
ierror.NotFoundCode: http.StatusNotFound,
|
||||||
ierror.ConflictCode: http.StatusConflict,
|
ierror.ConflictCode: http.StatusConflict,
|
||||||
ierror.PreconditionCode: http.StatusPreconditionFailed,
|
ierror.PreconditionCode: http.StatusPreconditionFailed,
|
||||||
ierror.GeneralCode: http.StatusInternalServerError,
|
ierror.ViolateForeignKeyConstraintCode: http.StatusPreconditionFailed,
|
||||||
|
ierror.GeneralCode: http.StatusInternalServerError,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user