[#542] Fix object removal

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
This commit is contained in:
Denis Kirillov 2022-06-24 15:39:30 +03:00 committed by Alex Vanin
parent 88c392d024
commit fdf0974679
5 changed files with 268 additions and 181 deletions

View file

@ -80,14 +80,12 @@ func (h *handler) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) {
p := &layer.DeleteObjectParams{
BktInfo: bktInfo,
Objects: versionedObject,
Settings: bktSettings,
}
deletedObjects, err := h.obj.DeleteObjects(r.Context(), p)
deletedObjects := h.obj.DeleteObjects(r.Context(), p)
deletedObject := deletedObjects[0]
if err == nil {
err = deletedObject.Error
}
if err != nil {
if isErrObjectLocked(err) {
if deletedObject.Error != nil {
if isErrObjectLocked(deletedObject.Error) {
h.logAndSendError(w, "object is locked", reqInfo, errors.GetAPIError(errors.ErrAccessDenied))
return
}
@ -95,7 +93,7 @@ func (h *handler) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) {
zap.String("request_id", reqInfo.RequestID),
zap.String("bucket_name", reqInfo.BucketName),
zap.String("object_name", reqInfo.ObjectName),
zap.Error(err))
zap.Error(deletedObject.Error))
}
var m *SendNotificationParams
@ -198,6 +196,12 @@ func (h *handler) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *http.Re
return
}
bktSettings, err := h.obj.GetBucketSettings(r.Context(), bktInfo)
if err != nil {
h.logAndSendError(w, "could not get bucket settings", reqInfo, err)
return
}
marshaler := zapcore.ArrayMarshalerFunc(func(encoder zapcore.ArrayEncoder) error {
for _, obj := range toRemove {
encoder.AppendString(obj.String())
@ -208,12 +212,9 @@ func (h *handler) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *http.Re
p := &layer.DeleteObjectParams{
BktInfo: bktInfo,
Objects: toRemove,
Settings: bktSettings,
}
deletedObjects, err := h.obj.DeleteObjects(r.Context(), p)
if !requested.Quiet && err != nil {
h.logAndSendError(w, "couldn't delete objects", reqInfo, err)
return
}
deletedObjects := h.obj.DeleteObjects(r.Context(), p)
var errs []error
for _, obj := range deletedObjects {

View file

@ -1,142 +1,187 @@
package handler
import (
"context"
"io"
"net/http"
"net/url"
"testing"
"github.com/nspcc-dev/neofs-s3-gw/api"
"github.com/nspcc-dev/neofs-s3-gw/api/layer"
"github.com/nspcc-dev/neofs-s3-gw/api/data"
"github.com/stretchr/testify/require"
)
const (
emptyVersion = ""
)
func TestDeleteObject(t *testing.T) {
ctx := context.Background()
tc := prepareHandlerContext(t)
bktName := "bucket-for-removal"
createTestBucket(ctx, t, tc, bktName)
bktInfo, err := tc.Layer().GetBucketInfo(ctx, bktName)
require.NoError(t, err)
bktName, objName := "bucket-for-removal", "object-to-delete"
bktInfo, objInfo := createBucketAndObject(t, tc, bktName, objName)
objName := "object"
objInfo := createTestObject(ctx, t, tc, bktInfo, objName)
checkFound(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
w, r := prepareTestRequest(t, bktName, objName, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusOK)
w, r = prepareTestRequest(t, bktName, objName, nil)
tc.Handler().DeleteObjectHandler(w, r)
assertStatus(t, w, http.StatusNoContent)
w, r = prepareTestRequest(t, bktName, objName, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusNotFound)
p := &layer.GetObjectParams{
BucketInfo: bktInfo,
ObjectInfo: objInfo,
Writer: io.Discard,
}
err = tc.Layer().GetObject(ctx, p)
require.Error(t, err)
require.False(t, existInMockedNeoFS(tc, bktInfo, objInfo))
}
func TestDeleteObjectVersioned(t *testing.T) {
ctx := context.Background()
tc := prepareHandlerContext(t)
bktName := "bucket-for-removal"
createTestBucket(ctx, t, tc, bktName)
bktInfo, err := tc.Layer().GetBucketInfo(ctx, bktName)
require.NoError(t, err)
bktName, objName := "bucket-for-removal", "object-to-delete"
bktInfo, objInfo := createVersionedBucketAndObject(t, tc, bktName, objName)
cfg := &VersioningConfiguration{Status: "Enabled"}
w, r := prepareTestRequest(t, bktName, "", cfg)
tc.Handler().PutBucketVersioningHandler(w, r)
assertStatus(t, w, http.StatusOK)
checkFound(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
objName := "object"
objInfo := createTestObject(ctx, t, tc, bktInfo, objName)
checkFound(t, tc, bktName, objName, objInfo.Version())
deleteObject(t, tc, bktName, objName, objInfo.Version())
checkNotFound(t, tc, bktName, objName, objInfo.Version())
w, r = prepareTestRequest(t, bktName, objName, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusOK)
w, r = prepareTestRequest(t, bktName, objName, nil)
tc.Handler().DeleteObjectHandler(w, r)
assertStatus(t, w, http.StatusNoContent)
w, r = prepareTestRequest(t, bktName, objName, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusNotFound)
query := make(url.Values)
query.Add(api.QueryVersionID, objInfo.Version())
w, r = prepareTestFullRequest(t, bktName, objName, query, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusOK)
w, r = prepareTestFullRequest(t, bktName, objName, query, nil)
r.URL.RawQuery = query.Encode()
tc.Handler().DeleteObjectHandler(w, r)
assertStatus(t, w, http.StatusNoContent)
p := &layer.GetObjectParams{
BucketInfo: bktInfo,
ObjectInfo: objInfo,
Writer: io.Discard,
require.False(t, existInMockedNeoFS(tc, bktInfo, objInfo), "object exists but shouldn't")
}
err = tc.Layer().GetObject(ctx, p)
require.Error(t, err)
func TestRemoveDeleteMarker(t *testing.T) {
tc := prepareHandlerContext(t)
bktName, objName := "bucket-for-removal", "object-to-delete"
bktInfo, objInfo := createVersionedBucketAndObject(t, tc, bktName, objName)
checkFound(t, tc, bktName, objName, emptyVersion)
deleteMarkerVersion := deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
checkFound(t, tc, bktName, objName, objInfo.Version())
deleteObject(t, tc, bktName, objName, deleteMarkerVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
require.True(t, existInMockedNeoFS(tc, bktInfo, objInfo), "object doesn't exist but should")
}
func TestDeleteObjectCombined(t *testing.T) {
ctx := context.Background()
tc := prepareHandlerContext(t)
bktName := "bucket-for-removal"
createTestBucket(ctx, t, tc, bktName)
bktInfo, err := tc.Layer().GetBucketInfo(ctx, bktName)
bktName, objName := "bucket-for-removal", "object-to-delete"
bktInfo, objInfo := createBucketAndObject(t, tc, bktName, objName)
putBucketVersioning(t, tc, bktName, true)
checkFound(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
checkFound(t, tc, bktName, objName, objInfo.Version())
require.True(t, existInMockedNeoFS(tc, bktInfo, objInfo), "object doesn't exist but should")
}
func TestDeleteObjectSuspended(t *testing.T) {
tc := prepareHandlerContext(t)
bktName, objName := "bucket-for-removal", "object-to-delete"
bktInfo, objInfo := createBucketAndObject(t, tc, bktName, objName)
putBucketVersioning(t, tc, bktName, true)
checkFound(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, emptyVersion)
putBucketVersioning(t, tc, bktName, false)
deleteObject(t, tc, bktName, objName, emptyVersion)
checkNotFound(t, tc, bktName, objName, objInfo.Version())
require.False(t, existInMockedNeoFS(tc, bktInfo, objInfo), "object exists but shouldn't")
}
func TestDeleteMarkers(t *testing.T) {
tc := prepareHandlerContext(t)
bktName, objName := "bucket-for-removal", "object-to-delete"
createTestBucket(tc.Context(), t, tc, bktName)
putBucketVersioning(t, tc, bktName, true)
checkNotFound(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
deleteObject(t, tc, bktName, objName, emptyVersion)
versions := listVersions(t, tc, bktName)
require.Len(t, versions.DeleteMarker, 3, "invalid delete markers length")
require.Len(t, versions.Version, 0, "versions must be empty")
require.Len(t, listOIDsFromMockedNeoFS(t, tc, bktName, objName), 0, "shouldn't be any object in neofs")
}
func createBucketAndObject(t *testing.T, tc *handlerContext, bktName, objName string) (*data.BucketInfo, *data.ObjectInfo) {
createTestBucket(tc.Context(), t, tc, bktName)
bktInfo, err := tc.Layer().GetBucketInfo(tc.Context(), bktName)
require.NoError(t, err)
objName := "object"
objInfo := createTestObject(ctx, t, tc, bktInfo, objName)
objInfo := createTestObject(tc.Context(), t, tc, bktInfo, objName)
w, r := prepareTestRequest(t, bktName, objName, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusOK)
return bktInfo, objInfo
}
cfg := &VersioningConfiguration{Status: "Enabled"}
w, r = prepareTestRequest(t, bktName, objName, cfg)
func createVersionedBucketAndObject(t *testing.T, tc *handlerContext, bktName, objName string) (*data.BucketInfo, *data.ObjectInfo) {
createTestBucket(tc.Context(), t, tc, bktName)
bktInfo, err := tc.Layer().GetBucketInfo(tc.Context(), bktName)
require.NoError(t, err)
putBucketVersioning(t, tc, bktName, true)
objInfo := createTestObject(tc.Context(), t, tc, bktInfo, objName)
return bktInfo, objInfo
}
func putBucketVersioning(t *testing.T, tc *handlerContext, bktName string, enabled bool) {
cfg := &VersioningConfiguration{Status: "Suspended"}
if enabled {
cfg.Status = "Enabled"
}
w, r := prepareTestRequest(t, bktName, "", cfg)
tc.Handler().PutBucketVersioningHandler(w, r)
assertStatus(t, w, http.StatusOK)
}
w, r = prepareTestRequest(t, bktName, objName, nil)
func deleteObject(t *testing.T, tc *handlerContext, bktName, objName, version string) string {
query := make(url.Values)
query.Add(api.QueryVersionID, version)
w, r := prepareTestFullRequest(t, bktName, objName, query, nil)
tc.Handler().DeleteObjectHandler(w, r)
assertStatus(t, w, http.StatusNoContent)
w, r = prepareTestRequest(t, bktName, objName, nil)
return w.Header().Get(api.AmzVersionID)
}
func checkNotFound(t *testing.T, tc *handlerContext, bktName, objName, version string) {
query := make(url.Values)
query.Add(api.QueryVersionID, version)
w, r := prepareTestFullRequest(t, bktName, objName, query, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusNotFound)
}
func checkFound(t *testing.T, tc *handlerContext, bktName, objName, version string) {
query := make(url.Values)
query.Add(api.QueryVersionID, objInfo.Version())
query.Add(api.QueryVersionID, version)
w, r = prepareTestFullRequest(t, bktName, objName, query, nil)
w, r := prepareTestFullRequest(t, bktName, objName, query, nil)
tc.Handler().HeadObjectHandler(w, r)
assertStatus(t, w, http.StatusNotFound) // because we remove null version
assertStatus(t, w, http.StatusOK)
}
p := &layer.GetObjectParams{
BucketInfo: bktInfo,
ObjectInfo: objInfo,
Writer: io.Discard,
}
err = tc.Layer().GetObject(ctx, p)
require.Error(t, err)
func listVersions(t *testing.T, tc *handlerContext, bktName string) *ListObjectsVersionsResponse {
w, r := prepareTestRequest(t, bktName, "", nil)
tc.Handler().ListBucketObjectVersionsHandler(w, r)
assertStatus(t, w, http.StatusOK)
res := &ListObjectsVersionsResponse{}
parseTestResponse(t, w, res)
return res
}

View file

@ -20,6 +20,7 @@ import (
"github.com/nspcc-dev/neofs-s3-gw/api/resolver"
cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
"github.com/nspcc-dev/neofs-sdk-go/object"
oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
"github.com/nspcc-dev/neofs-sdk-go/user"
usertest "github.com/nspcc-dev/neofs-sdk-go/user/test"
"github.com/stretchr/testify/require"
@ -29,6 +30,7 @@ import (
type handlerContext struct {
h *handler
tp *layer.TestNeoFS
context context.Context
}
func (hc *handlerContext) Handler() *handler {
@ -43,6 +45,10 @@ func (hc *handlerContext) Layer() layer.Client {
return hc.h.obj
}
func (hc *handlerContext) Context() context.Context {
return hc.context
}
func prepareHandlerContext(t *testing.T) *handlerContext {
key, err := keys.NewPrivateKey()
require.NoError(t, err)
@ -71,6 +77,7 @@ func prepareHandlerContext(t *testing.T) *handlerContext {
return &handlerContext{
h: h,
tp: tp,
context: context.Background(),
}
}
@ -167,3 +174,29 @@ func parseTestResponse(t *testing.T, response *httptest.ResponseRecorder, body i
err := xml.NewDecoder(response.Result().Body).Decode(body)
require.NoError(t, err)
}
func existInMockedNeoFS(tc *handlerContext, bktInfo *data.BucketInfo, objInfo *data.ObjectInfo) bool {
p := &layer.GetObjectParams{
BucketInfo: bktInfo,
ObjectInfo: objInfo,
Writer: io.Discard,
}
return tc.Layer().GetObject(tc.Context(), p) == nil
}
func listOIDsFromMockedNeoFS(t *testing.T, tc *handlerContext, bktName, objectName string) []oid.ID {
bktInfo, err := tc.Layer().GetBucketInfo(tc.Context(), bktName)
require.NoError(t, err)
p := layer.PrmObjectSelect{
Container: bktInfo.CID,
ExactAttribute: [2]string{
object.AttributeFileName, objectName,
},
}
ids, err := tc.MockedPool().SelectObjects(tc.Context(), p)
require.NoError(t, err)
return ids
}

View file

@ -117,6 +117,7 @@ type (
DeleteObjectParams struct {
BktInfo *data.BucketInfo
Objects []*VersionedObject
Settings *data.BucketSettings
}
// PutSettingsParams stores object copy request parameters.
@ -236,7 +237,7 @@ type (
ListObjectsV2(ctx context.Context, p *ListObjectsParamsV2) (*ListObjectsInfoV2, error)
ListObjectVersions(ctx context.Context, p *ListObjectVersionsParams) (*ListObjectVersionsInfo, error)
DeleteObjects(ctx context.Context, p *DeleteObjectParams) ([]*VersionedObject, error)
DeleteObjects(ctx context.Context, p *DeleteObjectParams) []*VersionedObject
CreateMultipartUpload(ctx context.Context, p *CreateMultipartParams) error
CompleteMultipartUpload(ctx context.Context, p *CompleteMultipartParams) (*UploadData, *data.ObjectInfo, error)
@ -473,22 +474,31 @@ func getRandomOID() (oid.ID, error) {
return objID, nil
}
// DeleteObject removes all objects with the passed nice name.
func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, obj *VersionedObject) *VersionedObject {
if len(obj.VersionID) == 0 {
func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, settings *data.BucketSettings, obj *VersionedObject) *VersionedObject {
if len(obj.VersionID) != 0 {
var nodeVersion *data.NodeVersion
if nodeVersion, obj.Error = n.getNodeVersionToDelete(ctx, bkt, obj); obj.Error != nil {
return obj
}
if obj.DeleteMarkVersion, obj.Error = n.removeOldVersion(ctx, bkt, nodeVersion, obj); obj.Error != nil {
return obj
}
obj.Error = n.treeService.RemoveVersion(ctx, bkt.CID, nodeVersion.ID)
return obj
}
var newVersion *data.NodeVersion
if !settings.VersioningEnabled {
obj.VersionID = UnversionedObjectVersionID
}
objVersion := &ObjectVersion{
BktInfo: bkt,
ObjectName: obj.Name,
VersionID: obj.VersionID,
NoErrorOnDeleteMarker: true,
}
var nodeVersion *data.NodeVersion
nodeVersion, obj.Error = n.getNodeVersion(ctx, objVersion)
if nodeVersion, obj.Error = n.getNodeVersionToDelete(ctx, bkt, obj); obj.Error != nil {
return obj
}
if obj.VersionID == UnversionedObjectVersionID {
if obj.Error == nil {
if obj.DeleteMarkVersion, obj.Error = n.removeOldVersion(ctx, bkt, nodeVersion, obj); obj.Error != nil {
return obj
@ -496,14 +506,14 @@ func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, obj *Ver
} else if !errors.IsS3Error(obj.Error, errors.ErrNoSuchKey) {
return obj
}
}
randOID, err := getRandomOID()
if err != nil {
obj.Error = fmt.Errorf("couldn't get random oid: %w", err)
return obj
}
newVersion := &data.NodeVersion{
newVersion = &data.NodeVersion{
BaseNodeVersion: data.BaseNodeVersion{
OID: randOID,
FilePath: obj.Name,
@ -512,31 +522,26 @@ func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, obj *Ver
Created: time.Now(),
Owner: n.Owner(ctx),
},
IsUnversioned: true,
IsUnversioned: !settings.VersioningEnabled,
}
if obj.Error = n.treeService.AddVersion(ctx, bkt.CID, newVersion); obj.Error != nil {
return obj
}
n.namesCache.Delete(bkt.Name + "/" + obj.Name)
} else {
if obj.Error != nil {
return obj
}
if obj.DeleteMarkVersion, obj.Error = n.removeOldVersion(ctx, bkt, nodeVersion, obj); obj.Error != nil {
return obj
func (n *layer) getNodeVersionToDelete(ctx context.Context, bkt *data.BucketInfo, obj *VersionedObject) (*data.NodeVersion, error) {
objVersion := &ObjectVersion{
BktInfo: bkt,
ObjectName: obj.Name,
VersionID: obj.VersionID,
NoErrorOnDeleteMarker: true,
}
if obj.Error = n.treeService.RemoveVersion(ctx, bkt.CID, nodeVersion.ID); obj.Error != nil {
return obj
}
}
n.listsCache.CleanCacheEntriesContainingObject(obj.Name, bkt.CID)
return obj
return n.getNodeVersion(ctx, objVersion)
}
func (n *layer) removeOldVersion(ctx context.Context, bkt *data.BucketInfo, nodeVersion *data.NodeVersion, obj *VersionedObject) (string, error) {
@ -548,12 +553,12 @@ func (n *layer) removeOldVersion(ctx context.Context, bkt *data.BucketInfo, node
}
// DeleteObjects from the storage.
func (n *layer) DeleteObjects(ctx context.Context, p *DeleteObjectParams) ([]*VersionedObject, error) {
func (n *layer) DeleteObjects(ctx context.Context, p *DeleteObjectParams) []*VersionedObject {
for i, obj := range p.Objects {
p.Objects[i] = n.deleteObject(ctx, p.BktInfo, obj)
p.Objects[i] = n.deleteObject(ctx, p.BktInfo, p.Settings, obj)
}
return p.Objects, nil
return p.Objects
}
func (n *layer) CreateBucket(ctx context.Context, p *CreateBucketParams) (*data.BucketInfo, error) {

View file

@ -53,15 +53,15 @@ func (tc *testContext) getObject(objectName, versionID string, needError bool) (
return extendedInfo.ObjectInfo, content.Bytes()
}
func (tc *testContext) deleteObject(objectName, versionID string) {
func (tc *testContext) deleteObject(objectName, versionID string, settings *data.BucketSettings) {
p := &DeleteObjectParams{
BktInfo: tc.bktInfo,
Settings: settings,
Objects: []*VersionedObject{
{Name: objectName, VersionID: versionID},
},
}
deletedObjects, err := tc.layer.DeleteObjects(tc.ctx, p)
require.NoError(tc.t, err)
deletedObjects := tc.layer.DeleteObjects(tc.ctx, p)
for _, obj := range deletedObjects {
require.NoError(tc.t, obj.Error)
}
@ -230,7 +230,7 @@ func TestVersioningDeleteObject(t *testing.T) {
tc.putObject([]byte("content obj1 v1"))
tc.putObject([]byte("content obj1 v2"))
tc.deleteObject(tc.obj, "")
tc.deleteObject(tc.obj, "", settings)
tc.getObject(tc.obj, "", true)
tc.checkListObjects()
@ -268,19 +268,19 @@ func TestVersioningDeleteSpecificObjectVersion(t *testing.T) {
objV3Content := []byte("content obj1 v3")
objV3Info := tc.putObject(objV3Content)
tc.deleteObject(tc.obj, objV2Info.Version())
tc.deleteObject(tc.obj, objV2Info.Version(), settings)
tc.getObject(tc.obj, objV2Info.Version(), true)
_, buffer3 := tc.getObject(tc.obj, "", false)
require.Equal(t, objV3Content, buffer3)
tc.deleteObject(tc.obj, "")
tc.deleteObject(tc.obj, "", settings)
tc.getObject(tc.obj, "", true)
versions := tc.listVersions()
for _, ver := range versions.DeleteMarker {
if ver.IsLatest {
tc.deleteObject(tc.obj, ver.Object.Version())
tc.deleteObject(tc.obj, ver.Object.Version(), settings)
}
}
@ -295,7 +295,10 @@ func TestNoVersioningDeleteObject(t *testing.T) {
tc.putObject([]byte("content obj1 v1"))
tc.putObject([]byte("content obj1 v2"))
tc.deleteObject(tc.obj, "")
settings, err := tc.layer.GetBucketSettings(tc.ctx, tc.bktInfo)
require.NoError(t, err)
tc.deleteObject(tc.obj, "", settings)
tc.getObject(tc.obj, "", true)
tc.checkListObjects()
}