From a02900a4f7802d15e6ac333a72cb025bbc4bb548 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Tue, 31 May 2022 18:03:58 +0300 Subject: [PATCH] [#474] Use appropriate null version during listing Signed-off-by: Denis Kirillov --- api/data/tree.go | 1 + api/handler/object_list.go | 6 +++++- api/handler/object_list_test.go | 38 +++++++++++++++++++++++++++++++++ api/layer/layer.go | 4 ++-- api/layer/object.go | 14 +++++++++++- api/layer/tagging.go | 2 +- api/layer/util.go | 5 +++-- api/layer/versioning.go | 28 +++++++++++++----------- api/layer/versioning_test.go | 5 +++-- 9 files changed, 81 insertions(+), 22 deletions(-) diff --git a/api/data/tree.go b/api/data/tree.go index a11095c3..31635303 100644 --- a/api/data/tree.go +++ b/api/data/tree.go @@ -27,6 +27,7 @@ type DeleteMarkerInfo struct { type ExtendedObjectInfo struct { ObjectInfo *ObjectInfo NodeVersion *NodeVersion + IsLatest bool } // BaseNodeVersion is minimal node info from tree service. diff --git a/api/handler/object_list.go b/api/handler/object_list.go index e59a26d7..947d8db3 100644 --- a/api/handler/object_list.go +++ b/api/handler/object_list.go @@ -272,6 +272,10 @@ func encodeListObjectVersionsToResponse(info *layer.ListObjectVersionsInfo, buck } for _, ver := range info.Version { + versionID := ver.Object.Version() + if ver.IsUnversioned { + versionID = layer.UnversionedObjectVersionID + } res.Version = append(res.Version, ObjectVersionResponse{ IsLatest: ver.IsLatest, Key: ver.Object.Name, @@ -281,7 +285,7 @@ func encodeListObjectVersionsToResponse(info *layer.ListObjectVersionsInfo, buck DisplayName: ver.Object.Owner.String(), }, Size: ver.Object.Size, - VersionID: ver.Object.Version(), // todo return "null" version for unversioned https://github.com/nspcc-dev/neofs-s3-gw/issues/474 + VersionID: versionID, ETag: ver.Object.HashSum, }) } diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 7b6a2f8f..d28175a8 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -1,8 +1,12 @@ package handler import ( + "bytes" + "context" + "net/http" "testing" + "github.com/nspcc-dev/neofs-s3-gw/api/layer" "github.com/stretchr/testify/require" ) @@ -35,3 +39,37 @@ func TestParseContinuationToken(t *testing.T) { require.Equal(t, tokenStr, token) }) } + +func TestListObjectNullVersions(t *testing.T) { + ctx := context.Background() + hc := prepareHandlerContext(t) + + bktName := "bucket-versioning-enabled" + createTestBucket(ctx, t, hc, bktName) + + objName := "object" + + body := bytes.NewReader([]byte("content")) + w, r := prepareTestPayloadRequest(bktName, objName, body) + hc.Handler().PutObjectHandler(w, r) + assertStatus(t, w, http.StatusOK) + + versioning := &VersioningConfiguration{Status: "Enabled"} + w, r = prepareTestRequest(t, bktName, objName, versioning) + hc.Handler().PutBucketVersioningHandler(w, r) + assertStatus(t, w, http.StatusOK) + + body2 := bytes.NewReader([]byte("content2")) + w, r = prepareTestPayloadRequest(bktName, objName, body2) + hc.Handler().PutObjectHandler(w, r) + assertStatus(t, w, http.StatusOK) + + w, r = prepareTestRequest(t, bktName, objName, nil) + hc.Handler().ListBucketObjectVersionsHandler(w, r) + + result := &ListObjectsVersionsResponse{} + parseTestResponse(t, w, result) + + require.Len(t, result.Version, 2) + require.Equal(t, layer.UnversionedObjectVersionID, result.Version[1].VersionID) +} diff --git a/api/layer/layer.go b/api/layer/layer.go index 31f00d94..8722791b 100644 --- a/api/layer/layer.go +++ b/api/layer/layer.go @@ -470,14 +470,14 @@ func getRandomOID() (*oid.ID, error) { // DeleteObject removes all objects with the passed nice name. func (n *layer) deleteObject(ctx context.Context, bkt *data.BucketInfo, settings *data.BucketSettings, obj *VersionedObject) *VersionedObject { - if len(obj.VersionID) == 0 || obj.VersionID == unversionedObjectVersionID { + 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 = unversionedObjectVersionID + obj.DeleteMarkVersion = UnversionedObjectVersionID newVersion := &data.NodeVersion{ BaseNodeVersion: data.BaseNodeVersion{ OID: *randOID, diff --git a/api/layer/object.go b/api/layer/object.go index 032f9c1c..9aab0903 100644 --- a/api/layer/object.go +++ b/api/layer/object.go @@ -277,7 +277,7 @@ func (n *layer) headLastVersionIfNotDeleted(ctx context.Context, bkt *data.Bucke func (n *layer) headVersion(ctx context.Context, bkt *data.BucketInfo, p *HeadObjectParams) (*data.ObjectInfo, error) { var err error var foundVersion *data.NodeVersion - if p.VersionID == unversionedObjectVersionID { + if p.VersionID == UnversionedObjectVersionID { foundVersion, err = n.treeService.GetUnversioned(ctx, &bkt.CID, p.Object) if err != nil { if errors.Is(err, ErrNodeNotFound) { @@ -572,6 +572,18 @@ func triageObjects(allObjects []*data.ObjectInfo) (prefixes []string, objects [] return } +func triageExtendedObjects(allObjects []*data.ExtendedObjectInfo) (prefixes []string, objects []*data.ExtendedObjectInfo) { + for _, ov := range allObjects { + if ov.ObjectInfo.IsDir { + prefixes = append(prefixes, ov.ObjectInfo.Name) + } else { + objects = append(objects, ov) + } + } + + return +} + func (n *layer) listAllObjects(ctx context.Context, p ListObjectsParamsCommon) ([]*data.ObjectInfo, error) { var ( err error diff --git a/api/layer/tagging.go b/api/layer/tagging.go index 393e7a86..acffc5ec 100644 --- a/api/layer/tagging.go +++ b/api/layer/tagging.go @@ -131,7 +131,7 @@ func (n *layer) getNodeVersion(ctx context.Context, objVersion *ObjectVersion) ( var err error var version *data.NodeVersion - if objVersion.VersionID == unversionedObjectVersionID { + if objVersion.VersionID == UnversionedObjectVersionID { version, err = n.treeService.GetUnversioned(ctx, &objVersion.BktInfo.CID, objVersion.ObjectName) } else if len(objVersion.VersionID) == 0 { version, err = n.treeService.GetLatestVersion(ctx, &objVersion.BktInfo.CID, objVersion.ObjectName) diff --git a/api/layer/util.go b/api/layer/util.go index eaca9965..80e13a53 100644 --- a/api/layer/util.go +++ b/api/layer/util.go @@ -37,8 +37,9 @@ type ( // ObjectVersionInfo stores info about objects versions. ObjectVersionInfo struct { - Object *data.ObjectInfo - IsLatest bool + Object *data.ObjectInfo + IsLatest bool + IsUnversioned bool } // ListObjectVersionsInfo stores info and list of objects versions. diff --git a/api/layer/versioning.go b/api/layer/versioning.go index aa016d8c..49503ab3 100644 --- a/api/layer/versioning.go +++ b/api/layer/versioning.go @@ -8,12 +8,12 @@ import ( ) const ( - unversionedObjectVersionID = "null" + UnversionedObjectVersionID = "null" ) func (n *layer) ListObjectVersions(ctx context.Context, p *ListObjectVersionsParams) (*ListObjectVersionsInfo, error) { var ( - allObjects = make([]*data.ObjectInfo, 0, p.MaxKeys) + allObjects = make([]*data.ExtendedObjectInfo, 0, p.MaxKeys) res = &ListObjectVersionsInfo{} ) @@ -34,35 +34,37 @@ func (n *layer) ListObjectVersions(ctx context.Context, p *ListObjectVersionsPar return sortedVersions[j].NodeVersion.Timestamp < sortedVersions[i].NodeVersion.Timestamp // sort in reverse order }) - for _, version := range sortedVersions { - allObjects = append(allObjects, version.ObjectInfo) + for i, version := range sortedVersions { + version.IsLatest = i == 0 + allObjects = append(allObjects, version) } } for i, obj := range allObjects { - if obj.Name >= p.KeyMarker && obj.Version() >= p.VersionIDMarker { + if obj.ObjectInfo.Name >= p.KeyMarker && obj.ObjectInfo.Version() >= p.VersionIDMarker { allObjects = allObjects[i:] break } } - res.CommonPrefixes, allObjects = triageObjects(allObjects) + res.CommonPrefixes, allObjects = triageExtendedObjects(allObjects) if len(allObjects) > p.MaxKeys { res.IsTruncated = true - res.NextKeyMarker = allObjects[p.MaxKeys].Name - res.NextVersionIDMarker = allObjects[p.MaxKeys].Version() + res.NextKeyMarker = allObjects[p.MaxKeys].ObjectInfo.Name + res.NextVersionIDMarker = allObjects[p.MaxKeys].ObjectInfo.Version() allObjects = allObjects[:p.MaxKeys] - res.KeyMarker = allObjects[p.MaxKeys-1].Name - res.VersionIDMarker = allObjects[p.MaxKeys-1].Version() + res.KeyMarker = allObjects[p.MaxKeys-1].ObjectInfo.Name + res.VersionIDMarker = allObjects[p.MaxKeys-1].ObjectInfo.Version() } objects := make([]*ObjectVersionInfo, len(allObjects)) for i, obj := range allObjects { - objects[i] = &ObjectVersionInfo{Object: obj} - if i == 0 || allObjects[i-1].Name != obj.Name { - objects[i].IsLatest = true + objects[i] = &ObjectVersionInfo{ + Object: obj.ObjectInfo, + IsUnversioned: obj.NodeVersion.IsUnversioned, + IsLatest: obj.IsLatest, } } diff --git a/api/layer/versioning_test.go b/api/layer/versioning_test.go index dd510abc..de61a903 100644 --- a/api/layer/versioning_test.go +++ b/api/layer/versioning_test.go @@ -250,7 +250,7 @@ func TestGetUnversioned(t *testing.T) { }) require.NoError(t, err) - resInfo, buffer := tc.getObject(tc.obj, unversionedObjectVersionID, false) + resInfo, buffer := tc.getObject(tc.obj, UnversionedObjectVersionID, false) require.Equal(t, objContent, buffer) require.Equal(t, objInfo.Version(), resInfo.Version()) } @@ -278,7 +278,8 @@ func TestVersioningDeleteSpecificObjectVersion(t *testing.T) { tc.deleteObject(tc.obj, "", settings) tc.getObject(tc.obj, "", true) - for _, ver := range tc.listVersions().DeleteMarker { + versions := tc.listVersions() + for _, ver := range versions.DeleteMarker { if ver.IsLatest { tc.deleteObject(tc.obj, ver.Object.Version(), settings) }