From 7e8b57605a1f0179f58067652175f0df7482a880 Mon Sep 17 00:00:00 2001
From: Denis Kirillov <denis@nspcc.ru>
Date: Thu, 19 May 2022 12:17:56 +0300
Subject: [PATCH] [#413] Fix delete object without version

Signed-off-by: Denis Kirillov <denis@nspcc.ru>
---
 api/layer/layer.go     | 68 ++++++++++++++++++++++--------------------
 internal/neofs/tree.go |  2 +-
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/api/layer/layer.go b/api/layer/layer.go
index 2fe863d1..5577e043 100644
--- a/api/layer/layer.go
+++ b/api/layer/layer.go
@@ -9,8 +9,6 @@ import (
 	"net/url"
 	"strings"
 
-	oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
-
 	"github.com/nats-io/nats.go"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 	"github.com/nspcc-dev/neofs-s3-gw/api"
@@ -23,6 +21,7 @@ import (
 	cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
 	"github.com/nspcc-dev/neofs-sdk-go/eacl"
 	"github.com/nspcc-dev/neofs-sdk-go/netmap"
+	oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
 	"github.com/nspcc-dev/neofs-sdk-go/session"
 	"github.com/nspcc-dev/neofs-sdk-go/user"
 	"go.uber.org/zap"
@@ -252,8 +251,6 @@ type (
 const (
 	tagPrefix    = "S3-Tag-"
 	tagEmptyMark = "\\"
-
-	emptyOID = "11111111111111111111111111111111"
 )
 
 func (t *VersionedObject) String() string {
@@ -556,54 +553,39 @@ func getRandomOID() (*oid.ID, error) {
 }
 
 // DeleteObject removes all objects with the passed nice name.
-func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, _ *data.BucketSettings, obj *VersionedObject) *VersionedObject {
-	if !isRegularVersion(obj.VersionID) { // version null or empty
+func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, settings *data.BucketSettings, obj *VersionedObject) *VersionedObject {
+	if len(obj.VersionID) == 0 || obj.VersionID == unversionedObjectVersionID {
 		randOID, err := getRandomOID()
 		if err != nil {
 			obj.Error = fmt.Errorf("couldn't get random oid: %w", err)
 			return obj
 		}
-		obj.DeleteMarkVersion = randOID.EncodeToString()
+
+		obj.DeleteMarkVersion = unversionedObjectVersionID
 		newVersion := &NodeVersion{
 			BaseNodeVersion: BaseNodeVersion{
 				OID: *randOID,
 			},
 			IsDeleteMarker: true,
-			IsUnversioned:  obj.VersionID == unversionedObjectVersionID,
+			IsUnversioned:  true,
 		}
+
+		if len(obj.VersionID) == 0 && settings.VersioningEnabled {
+			newVersion.IsUnversioned = false
+			obj.DeleteMarkVersion = randOID.EncodeToString()
+		}
+
 		if obj.Error = n.treeService.AddVersion(ctx, &bkt.CID, obj.Name, newVersion); obj.Error != nil {
 			return obj
 		}
 	} else {
-		if obj.VersionID == emptyOID {
-			obj.Error = errors.GetAPIError(errors.ErrInvalidVersion)
-			return obj
-		}
-
 		versions, err := n.treeService.GetVersions(ctx, &bkt.CID, obj.Name)
 		if err != nil {
 			obj.Error = err
 			return obj
 		}
 
-		var found bool
-		for _, version := range versions {
-			if version.OID.EncodeToString() == obj.VersionID {
-				if err = n.treeService.RemoveVersion(ctx, &bkt.CID, version.ID); err == nil {
-					if err = n.objectDelete(ctx, bkt, version.OID); err == nil {
-						if err = n.DeleteObjectTagging(ctx, bkt, &data.ObjectInfo{ID: version.OID, Bucket: bkt.Name, Name: obj.Name}); err == nil {
-							found = true
-							break
-						}
-					}
-				}
-				obj.Error = err
-				return obj
-			}
-		}
-
-		if !found {
-			obj.Error = errors.GetAPIError(errors.ErrNoSuchVersion)
+		if obj.DeleteMarkVersion, obj.Error = n.removeVersionIfFound(ctx, bkt, versions, obj); obj.Error != nil {
 			return obj
 		}
 	}
@@ -613,8 +595,28 @@ func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, _ *data.
 	return obj
 }
 
-func isRegularVersion(version string) bool {
-	return len(version) != 0 && version != unversionedObjectVersionID
+func (n *layer) removeVersionIfFound(ctx context.Context, bkt *data.BucketInfo, versions []*NodeVersion, obj *VersionedObject) (string, error) {
+	for _, version := range versions {
+		if version.OID.EncodeToString() != obj.VersionID {
+			continue
+		}
+
+		var deleteMarkVersion string
+		if version.IsDeleteMarker {
+			deleteMarkVersion = obj.VersionID
+		}
+
+		if err := n.treeService.RemoveVersion(ctx, &bkt.CID, version.ID); err != nil {
+			return deleteMarkVersion, err
+		}
+		if err := n.objectDelete(ctx, bkt, version.OID); err != nil {
+			return deleteMarkVersion, err
+		}
+
+		return deleteMarkVersion, n.DeleteObjectTagging(ctx, bkt, &data.ObjectInfo{ID: version.OID, Bucket: bkt.Name, Name: obj.Name})
+	}
+
+	return "", errors.GetAPIError(errors.ErrNoSuchVersion)
 }
 
 // DeleteObjects from the storage.
diff --git a/internal/neofs/tree.go b/internal/neofs/tree.go
index a7eb2d40..64afedea 100644
--- a/internal/neofs/tree.go
+++ b/internal/neofs/tree.go
@@ -339,7 +339,7 @@ func (c *TreeClient) addVersion(ctx context.Context, cnrID *cid.ID, treeID, attr
 				return err
 			}
 
-			return c.moveNode(ctx, cnrID, treeID, version.ID, parentID, meta)
+			return c.moveNode(ctx, cnrID, treeID, node.ID, parentID, meta)
 		}
 
 		if !errors.Is(err, layer.ErrNodeNotFound) {