continue updating code per review comments

Signed-off-by: wang yan <wangyan@vmware.com>
This commit is contained in:
wang yan 2020-06-23 13:00:29 +08:00
parent 446739f967
commit 0e175017aa
7 changed files with 61 additions and 42 deletions

View File

@ -24,6 +24,7 @@ import (
"github.com/goharbor/harbor/src/lib/log"
"github.com/goharbor/harbor/src/lib/orm"
"github.com/goharbor/harbor/src/pkg/blob"
blob_models "github.com/goharbor/harbor/src/pkg/blob/models"
)
var (
@ -74,8 +75,8 @@ type Controller interface {
// GetAcceptedBlobSize returns the accepted size of stream upload blob.
GetAcceptedBlobSize(sessionID string) (int64, error)
// Touch updates the blob status and increase version every time.
Touch(ctx context.Context, blob *blob.Blob) (int64, error)
// Touch updates the blob status to StatusNone and increase version every time.
Touch(ctx context.Context, blob *blob.Blob) error
// Update updates the blob, it cannot handle blob status transitions.
Update(ctx context.Context, blob *blob.Blob) error
@ -322,8 +323,16 @@ func (c *controller) GetAcceptedBlobSize(sessionID string) (int64, error) {
return size, nil
}
func (c *controller) Touch(ctx context.Context, blob *blob.Blob) (int64, error) {
return c.blobMgr.UpdateBlobStatus(ctx, blob)
func (c *controller) Touch(ctx context.Context, blob *blob.Blob) error {
blob.Status = blob_models.StatusNone
count, err := c.blobMgr.UpdateBlobStatus(ctx, blob)
if err != nil {
return err
}
if count == 0 {
return errors.New(nil).WithMessage(fmt.Sprintf("no blob item is updated to StatusNone, id:%d, digest:%s", blob.ID, blob.Digest)).WithCode(errors.NotFoundCode)
}
return nil
}
func (c *controller) Update(ctx context.Context, blob *blob.Blob) error {

View File

@ -17,6 +17,7 @@ package blob
import (
"context"
"fmt"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/pkg/blob/models"
"testing"
@ -28,6 +29,8 @@ import (
blobtesting "github.com/goharbor/harbor/src/testing/pkg/blob"
"github.com/google/uuid"
"github.com/stretchr/testify/suite"
pkg_blob "github.com/goharbor/harbor/src/pkg/blob"
)
type ControllerTestSuite struct {
@ -268,19 +271,25 @@ func (suite *ControllerTestSuite) TestGetSetAcceptedBlobSize() {
suite.Equal(int64(100), size)
}
func (suite *ControllerTestSuite) TestUpdateStatus() {
func (suite *ControllerTestSuite) TestTouch() {
ctx := suite.Context()
err := Ctl.Touch(ctx, &blob.Blob{
Status: models.StatusNone,
})
suite.NotNil(err)
suite.True(errors.IsNotFoundErr(err))
digest := suite.prepareBlob()
blob, err := Ctl.Get(ctx, digest)
suite.Nil(err)
suite.Equal(blob.Status, models.StatusNone)
blob.Status = models.StatusDelete
count, err := Ctl.Touch(ctx, blob)
_, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), blob)
suite.Nil(err)
suite.Equal(blob.Status, models.StatusDelete)
suite.Equal(int64(1), count)
err = Ctl.Touch(ctx, blob)
suite.Nil(err)
suite.Equal(blob.Status, models.StatusNone)
}
func (suite *ControllerTestSuite) TestDelete() {

View File

@ -18,6 +18,7 @@ import (
"context"
"fmt"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/log"
"strings"
"time"
@ -200,20 +201,28 @@ func (d *dao) UpdateBlobStatus(ctx context.Context, blob *models.Blob) (int64, e
WHERE "id" IN ( SELECT T0."id" FROM "blob" T0 WHERE T0."version" >= $4 AND T0."id" = $5 AND T0."status" IN ('delete', 'deleting') )
*/
return qt.SetCond(c).Filter("id", blob.ID).
count, err := qt.SetCond(c).Filter("id", blob.ID).
Filter("status__in", models.StatusMap[blob.Status]).
Update(data)
if err != nil {
return count, err
}
if count == 0 {
log.Warningf("no blob is updated according to query condition, id: %d, status_in, %v", blob.ID, models.StatusMap[blob.Status])
return 0, nil
}
return count, nil
}
// UpdateBlob cannot handle the status change.
// UpdateBlob cannot handle the status change and version increase, for handling blob status change, please call
// for the UpdateBlobStatus.
func (d *dao) UpdateBlob(ctx context.Context, blob *models.Blob) error {
o, err := orm.FromContext(ctx)
if err != nil {
return err
}
blob.Version = blob.Version + 1
blob.UpdateTime = time.Now()
_, err = o.Update(blob, "size", "content_type", "version", "update_time")
_, err = o.Update(blob, "size", "content_type", "update_time")
return err
}

View File

@ -139,7 +139,7 @@ func (suite *DaoTestSuite) TestUpdateBlob() {
blob, err := suite.dao.GetBlobByDigest(ctx, digest)
if suite.Nil(err) {
suite.Equal(int64(100), blob.Size)
suite.Equal(int64(1), blob.Version)
suite.Equal(int64(0), blob.Version)
}
}
@ -147,7 +147,7 @@ func (suite *DaoTestSuite) TestUpdateBlob() {
suite.Nil(suite.dao.UpdateBlob(ctx, blob), "cannot be updated.")
blob, err = suite.dao.GetBlobByDigest(ctx, digest)
if suite.Nil(err) {
suite.Equal(int64(2), blob.Version)
suite.Equal(int64(0), blob.Version)
suite.Equal(models.StatusNone, blob.Status)
}
}

View File

@ -9,6 +9,7 @@ import (
"github.com/goharbor/harbor/src/lib/log"
blob_models "github.com/goharbor/harbor/src/pkg/blob/models"
"github.com/goharbor/harbor/src/server/middleware"
"github.com/goharbor/harbor/src/server/middleware/requestid"
"net/http"
)
@ -26,29 +27,26 @@ func HeadBlobMiddleware() func(http.Handler) http.Handler {
// handleHead ...
func handleHead(req *http.Request) error {
none := lib.ArtifactInfo{}
art := lib.GetArtifactInfo(req.Context())
if art == none {
return errors.New("cannot get the artifact information from request context").WithCode(errors.NotFoundCode)
// for head blob, the GetArtifactInfo is actually get the information of blob.
blobInfo := lib.GetArtifactInfo(req.Context())
if blobInfo == none {
return errors.New("cannot get the blob information from request context").WithCode(errors.NotFoundCode)
}
bb, err := blob.Ctl.Get(req.Context(), art.Digest)
bb, err := blob.Ctl.Get(req.Context(), blobInfo.Digest)
if err != nil {
return err
}
switch bb.Status {
case blob_models.StatusNone, blob_models.StatusDelete:
bb.Status = blob_models.StatusNone
count, err := blob.Ctl.Touch(req.Context(), bb)
err := blob.Ctl.Touch(req.Context(), bb)
if err != nil {
log.Errorf("failed to update blob: %s status to None, error:%v", art.Digest, err)
return err
}
if count == 0 {
return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode)
log.Errorf("failed to update blob: %s status to StatusNone, error:%v", blobInfo.Digest, err)
return errors.Wrapf(err, fmt.Sprintf("the request id is: %s", req.Header.Get(requestid.HeaderXRequestID)))
}
case blob_models.StatusDeleting, blob_models.StatusDeleteFailed:
return errors.New("the asking blob is in GC, mark it as non existing").WithCode(errors.NotFoundCode)
return errors.New(nil).WithMessage(fmt.Sprintf("the asking blob is in GC, mark it as non existing, request id: %s", req.Header.Get(requestid.HeaderXRequestID))).WithCode(errors.NotFoundCode)
default:
return errors.New(nil).WithMessage(fmt.Sprintf("wrong blob status, %s", bb.Status))
}

View File

@ -6,6 +6,7 @@ import (
"github.com/goharbor/harbor/src/controller/blob"
"github.com/goharbor/harbor/src/lib"
"github.com/goharbor/harbor/src/lib/orm"
pkg_blob "github.com/goharbor/harbor/src/pkg/blob"
blob_models "github.com/goharbor/harbor/src/pkg/blob/models"
htesting "github.com/goharbor/harbor/src/testing"
"github.com/stretchr/testify/suite"
@ -64,9 +65,9 @@ func (suite *HeadBlobUploadMiddlewareTestSuite) TestHeadBlobStatusDeleting() {
suite.Nil(err)
// status-none -> status-delete -> status-deleting
_, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDelete})
_, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDelete})
suite.Nil(err)
_, err = blob.Ctl.Touch(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDeleting, Version: 1})
_, err = pkg_blob.Mgr.UpdateBlobStatus(suite.Context(), &blob_models.Blob{ID: id, Status: blob_models.StatusDeleting, Version: 1})
suite.Nil(err)
req := suite.NewRequest(http.MethodHead, fmt.Sprintf("/v2/%s/blobs/%s", projectName, digest), nil)

View File

@ -271,24 +271,17 @@ func (_m *Controller) Sync(ctx context.Context, references []distribution.Descri
}
// Touch provides a mock function with given fields: ctx, _a1
func (_m *Controller) Touch(ctx context.Context, _a1 *models.Blob) (int64, error) {
func (_m *Controller) Touch(ctx context.Context, _a1 *models.Blob) error {
ret := _m.Called(ctx, _a1)
var r0 int64
if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) int64); ok {
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, *models.Blob) error); ok {
r0 = rf(ctx, _a1)
} else {
r0 = ret.Get(0).(int64)
r0 = ret.Error(0)
}
var r1 error
if rf, ok := ret.Get(1).(func(context.Context, *models.Blob) error); ok {
r1 = rf(ctx, _a1)
} else {
r1 = ret.Error(1)
}
return r0, r1
return r0
}
// Update provides a mock function with given fields: ctx, _a1