From a74d498df2814b081f950331ed6546434ecd84d2 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Fri, 26 Jan 2024 16:57:34 +0300 Subject: [PATCH] [#165] Return sort after HEAD in listing We have to sort object after HEAD because we make request in different goroutines, so the order is not deterministic. Signed-off-by: Denis Kirillov --- api/handler/object_list_test.go | 83 ++++++++++++++++++++++----------- api/layer/listing.go | 4 ++ 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 829220c..7b813e7 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -15,7 +15,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/layer/encryption" - "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/pkg/service/tree" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -74,37 +73,68 @@ func TestListObjectsWithOldTreeNodes(t *testing.T) { srcEnc, err := encryption.NewParams([]byte("1234567890qwertyuiopasdfghjklzxc")) require.NoError(t, err) - objInfo := createTestObject(hc, bktInfo, objName, *srcEnc) - _ = objInfo - prm := &tree.GetNodesParams{ - BktInfo: bktInfo, - TreeID: "version", - Path: []string{objName}, - LatestOnly: true, - AllAttrs: true, + n := 10 + objInfos := make([]*data.ObjectInfo, n) + for i := 0; i < n; i++ { + objInfos[i] = createTestObject(hc, bktInfo, objName+strconv.Itoa(i), *srcEnc) } - nodes, err := hc.treeMock.GetNodes(hc.Context(), prm) - require.NoError(t, err) - require.Len(t, nodes, 1) - node := nodes[0] - meta := make(map[string]string, len(node.GetMeta())) - for _, m := range node.GetMeta() { - if m.GetKey() != "Created" && m.GetKey() != "Owner" { - meta[m.GetKey()] = string(m.GetValue()) + sort.Slice(objInfos, func(i, j int) bool { return objInfos[i].Name < objInfos[j].Name }) + + makeAllTreeObjectsOld(hc, bktInfo) + + listV1 := listObjectsV1(hc, bktName, "", "", "", -1) + checkListOldNodes(hc, listV1.Contents, objInfos) + + listV2 := listObjectsV2(hc, bktName, "", "", "", "", -1) + checkListOldNodes(hc, listV2.Contents, objInfos) + + listVers := listObjectsVersions(hc, bktName, "", "", "", "", -1) + checkListVersionsOldNodes(hc, listVers.Version, objInfos) +} + +func makeAllTreeObjectsOld(hc *handlerContext, bktInfo *data.BucketInfo) { + nodes, err := hc.treeMock.GetSubTree(hc.Context(), bktInfo, "version", 0, 0) + require.NoError(hc.t, err) + + for _, node := range nodes { + if node.GetNodeID() == 0 { + continue } + meta := make(map[string]string, len(node.GetMeta())) + for _, m := range node.GetMeta() { + if m.GetKey() != "Created" && m.GetKey() != "Owner" { + meta[m.GetKey()] = string(m.GetValue()) + } + } + + err = hc.treeMock.MoveNode(hc.Context(), bktInfo, "version", node.GetNodeID(), node.GetParentID(), meta) + require.NoError(hc.t, err) } +} - err = hc.treeMock.MoveNode(hc.Context(), bktInfo, "version", node.GetNodeID(), node.GetParentID(), meta) - require.NoError(t, err) +func checkListOldNodes(hc *handlerContext, list []Object, objInfos []*data.ObjectInfo) { + require.Len(hc.t, list, len(objInfos)) + for i := range list { + require.Equal(hc.t, objInfos[i].Name, list[i].Key) + realSize, err := layer.GetObjectSize(objInfos[i]) + require.NoError(hc.t, err) + require.Equal(hc.t, objInfos[i].Owner.EncodeToString(), list[i].Owner.ID) + require.Equal(hc.t, objInfos[i].Created.UTC().Format(time.RFC3339), list[i].LastModified) + require.Equal(hc.t, realSize, list[i].Size) + } +} - list := listObjectsV1(hc, bktName, "", "", "", -1) - require.Len(t, list.Contents, 1) - realSize, err := layer.GetObjectSize(objInfo) - require.NoError(t, err) - require.Equal(t, objInfo.Owner.EncodeToString(), list.Contents[0].Owner.ID) - require.Equal(t, objInfo.Created.UTC().Format(time.RFC3339), list.Contents[0].LastModified) - require.Equal(t, realSize, list.Contents[0].Size) +func checkListVersionsOldNodes(hc *handlerContext, list []ObjectVersionResponse, objInfos []*data.ObjectInfo) { + require.Len(hc.t, list, len(objInfos)) + for i := range list { + require.Equal(hc.t, objInfos[i].Name, list[i].Key) + realSize, err := layer.GetObjectSize(objInfos[i]) + require.NoError(hc.t, err) + require.Equal(hc.t, objInfos[i].Owner.EncodeToString(), list[i].Owner.ID) + require.Equal(hc.t, objInfos[i].Created.UTC().Format(time.RFC3339), list[i].LastModified) + require.Equal(hc.t, realSize, list[i].Size) + } } func TestListObjectsContextCanceled(t *testing.T) { @@ -657,6 +687,7 @@ func listObjectsV2(hc *handlerContext, bktName, prefix, delimiter, startAfter, c func listObjectsV2Ext(hc *handlerContext, bktName, prefix, delimiter, startAfter, continuationToken, encodingType string, maxKeys int) *ListObjectsV2Response { query := prepareCommonListObjectsQuery(prefix, delimiter, maxKeys) + query.Add("fetch-owner", "true") if len(startAfter) != 0 { query.Add("start-after", startAfter) } diff --git a/api/layer/listing.go b/api/layer/listing.go index f02f344..2de4a21 100644 --- a/api/layer/listing.go +++ b/api/layer/listing.go @@ -214,6 +214,8 @@ func (n *layer) getLatestObjectsVersions(ctx context.Context, p commonLatestVers return nil, nil, fmt.Errorf("failed to get next object from stream: %w", err) } + sort.Slice(objects, func(i, j int) bool { return objects[i].NodeVersion.FilePath < objects[j].NodeVersion.FilePath }) + if len(objects) > p.MaxKeys { next = objects[p.MaxKeys] n.putListLatestVersionsSession(ctx, p, session, objects) @@ -241,6 +243,8 @@ func (n *layer) getAllObjectsVersions(ctx context.Context, p commonVersionsListi allObjects := handleGeneratedVersions(objOutCh, p, session) + sort.SliceStable(allObjects, func(i, j int) bool { return allObjects[i].NodeVersion.FilePath < allObjects[j].NodeVersion.FilePath }) + if err = <-errorCh; err != nil { return nil, false, fmt.Errorf("failed to get next object from stream: %w", err) }