diff --git a/satellite/metabase/copy_object.go b/satellite/metabase/copy_object.go index 0543b6780..5a7d2cb83 100644 --- a/satellite/metabase/copy_object.go +++ b/satellite/metabase/copy_object.go @@ -130,7 +130,6 @@ func (db *DB) FinishCopyObject(ctx context.Context, opts FinishCopyObject) (obje return err } if sourceObject.StreamID != opts.StreamID { - // TODO(versioning): should we report it as "not found" instead? return ErrObjectNotFound.New("object was changed during copy") } if sourceObject.Status.IsDeleteMarker() { diff --git a/satellite/metabase/delete.go b/satellite/metabase/delete.go index fdf14950f..004a730f8 100644 --- a/satellite/metabase/delete.go +++ b/satellite/metabase/delete.go @@ -231,13 +231,11 @@ func (db *DB) DeleteObjectsAllVersions(ctx context.Context, opts DeleteObjectsAl objectKeys[i] = []byte(opts.Locations[i].ObjectKey) } - // TODO(ver): should this insert delete markers? - // Sorting the object keys just in case. - // TODO: Check if this is really necessary for the SQL query. sort.Slice(objectKeys, func(i, j int) bool { return bytes.Compare(objectKeys[i], objectKeys[j]) < 0 }) + err = withRows(db.db.QueryContext(ctx, ` WITH deleted_objects AS ( DELETE FROM objects @@ -338,7 +336,6 @@ func (db *DB) scanMultipleObjectsDeletion(ctx context.Context, rows tagsql.Rows) type DeleteObjectLastCommitted struct { ObjectLocation - // TODO(ver): maybe replace these with an enumeration? Versioned bool Suspended bool } diff --git a/satellite/metabase/delete_test.go b/satellite/metabase/delete_test.go index 056a159c2..6de98d637 100644 --- a/satellite/metabase/delete_test.go +++ b/satellite/metabase/delete_test.go @@ -616,6 +616,94 @@ func TestDeleteObjectVersioning(t *testing.T) { }, }.Check(ctx, t, db) }) + + t.Run("delete last committed unversioned with suspended", func(t *testing.T) { + defer metabasetest.DeleteAll{}.Check(ctx, t, db) + + now := time.Now() + + obj := metabasetest.RandObjectStream() + object := metabasetest.CreateObject(ctx, t, db, obj, 0) + + marker := metabase.Object{ + ObjectStream: metabase.ObjectStream{ + ProjectID: obj.ProjectID, + BucketName: obj.BucketName, + ObjectKey: obj.ObjectKey, + Version: obj.Version + 1, + }, + Status: metabase.DeleteMarkerUnversioned, + CreatedAt: now, + } + + metabasetest.DeleteObjectLastCommitted{ + Opts: metabase.DeleteObjectLastCommitted{ + ObjectLocation: metabase.ObjectLocation{ + ProjectID: obj.ProjectID, + BucketName: obj.BucketName, + ObjectKey: obj.ObjectKey, + }, + Versioned: false, + Suspended: true, + }, + Result: metabase.DeleteObjectResult{ + Markers: []metabase.Object{marker}, + Removed: []metabase.Object{ + object, + }, + }, + OutputMarkerStreamID: &marker.StreamID, + }.Check(ctx, t, db) + + metabasetest.Verify{ + Objects: []metabase.RawObject{ + metabase.RawObject(marker), + }, + }.Check(ctx, t, db) + }) + + t.Run("delete last committed versioned with suspended", func(t *testing.T) { + defer metabasetest.DeleteAll{}.Check(ctx, t, db) + + now := time.Now() + + obj := metabasetest.RandObjectStream() + initial := metabasetest.CreateObjectVersioned(ctx, t, db, obj, 0) + + marker := metabase.Object{ + ObjectStream: metabase.ObjectStream{ + ProjectID: obj.ProjectID, + BucketName: obj.BucketName, + ObjectKey: obj.ObjectKey, + Version: obj.Version + 1, + }, + Status: metabase.DeleteMarkerUnversioned, + CreatedAt: now, + } + + metabasetest.DeleteObjectLastCommitted{ + Opts: metabase.DeleteObjectLastCommitted{ + ObjectLocation: metabase.ObjectLocation{ + ProjectID: obj.ProjectID, + BucketName: obj.BucketName, + ObjectKey: obj.ObjectKey, + }, + Versioned: false, + Suspended: true, + }, + Result: metabase.DeleteObjectResult{ + Markers: []metabase.Object{marker}, + }, + OutputMarkerStreamID: &marker.StreamID, + }.Check(ctx, t, db) + + metabasetest.Verify{ + Objects: []metabase.RawObject{ + metabase.RawObject(initial), + metabase.RawObject(marker), + }, + }.Check(ctx, t, db) + }) }) } @@ -892,95 +980,6 @@ func TestDeleteObjectsAllVersions(t *testing.T) { metabasetest.Verify{}.Check(ctx, t, db) }) - - // TODO(ver): these tests look like they are in the wrong location - t.Run("delete last committed unversioned with suspended", func(t *testing.T) { - defer metabasetest.DeleteAll{}.Check(ctx, t, db) - - now := time.Now() - - obj := metabasetest.RandObjectStream() - object := metabasetest.CreateObject(ctx, t, db, obj, 0) - - marker := metabase.Object{ - ObjectStream: metabase.ObjectStream{ - ProjectID: obj.ProjectID, - BucketName: obj.BucketName, - ObjectKey: obj.ObjectKey, - Version: obj.Version + 1, - }, - Status: metabase.DeleteMarkerUnversioned, - CreatedAt: now, - } - - metabasetest.DeleteObjectLastCommitted{ - Opts: metabase.DeleteObjectLastCommitted{ - ObjectLocation: metabase.ObjectLocation{ - ProjectID: obj.ProjectID, - BucketName: obj.BucketName, - ObjectKey: obj.ObjectKey, - }, - Versioned: false, - Suspended: true, - }, - Result: metabase.DeleteObjectResult{ - Markers: []metabase.Object{marker}, - Removed: []metabase.Object{ - object, - }, - }, - OutputMarkerStreamID: &marker.StreamID, - }.Check(ctx, t, db) - - metabasetest.Verify{ - Objects: []metabase.RawObject{ - metabase.RawObject(marker), - }, - }.Check(ctx, t, db) - }) - - t.Run("delete last committed versioned with suspended", func(t *testing.T) { - defer metabasetest.DeleteAll{}.Check(ctx, t, db) - - now := time.Now() - - obj := metabasetest.RandObjectStream() - initial := metabasetest.CreateObjectVersioned(ctx, t, db, obj, 0) - - marker := metabase.Object{ - ObjectStream: metabase.ObjectStream{ - ProjectID: obj.ProjectID, - BucketName: obj.BucketName, - ObjectKey: obj.ObjectKey, - Version: obj.Version + 1, - }, - Status: metabase.DeleteMarkerUnversioned, - CreatedAt: now, - } - - metabasetest.DeleteObjectLastCommitted{ - Opts: metabase.DeleteObjectLastCommitted{ - ObjectLocation: metabase.ObjectLocation{ - ProjectID: obj.ProjectID, - BucketName: obj.BucketName, - ObjectKey: obj.ObjectKey, - }, - Versioned: false, - Suspended: true, - }, - Result: metabase.DeleteObjectResult{ - Markers: []metabase.Object{marker}, - }, - OutputMarkerStreamID: &marker.StreamID, - }.Check(ctx, t, db) - - metabasetest.Verify{ - Objects: []metabase.RawObject{ - metabase.RawObject(initial), - metabase.RawObject(marker), - }, - }.Check(ctx, t, db) - }) }) }