Skip to content

Commit

Permalink
Fix FilterObjects IncludeInfoObject issue
Browse files Browse the repository at this point in the history
  • Loading branch information
DarioCalovic committed Feb 9, 2024
1 parent ec89b61 commit cb5ae65
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 36 deletions.
62 changes: 28 additions & 34 deletions pkg/gcsstore/gcsservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"hash/crc32"
"io"
"math"
"strconv"
"strings"

"cloud.google.com/go/storage"
Expand Down Expand Up @@ -344,9 +343,32 @@ func (service *GCSService) FilterObjects(ctx context.Context, params GCSFilterPa
Versions: false,
}

// If the object name does not split on "_", we have a composed object.
// If the object name splits on "_" in to four pieces we
// know the object name we are working with is in the format
// [uid]_tmp_[recursion_lvl]_[chunk_idx]. The only time we filter
// these temporary objects is on a delete operation so we can just
// append and continue without worrying about index order
isValidObject := func(name string) (bool, error) {
fileNameParts := strings.Split(name, "/")
fileName := fileNameParts[len(fileNameParts)-1]

split := strings.Split(fileName, "_")

switch len(split) {
case 1:
case 2:
case 4:
default:
return false, errors.New("invalid filter format for object name")
}

return true, nil
}

it := bkt.Objects(ctx, &q)
names := make([]string, 0)
loop:
var names []string

for {
objAttrs, err := it.Next()
if err == iterator.Done {
Expand All @@ -360,41 +382,13 @@ loop:
continue
}

fileNameParts := strings.Split(objAttrs.Name, "/")
fileName := fileNameParts[len(fileNameParts)-1]

split := strings.Split(fileName, "_")

// If the object name does not split on "_", we have a composed object.
// If the object name splits on "_" in to four pieces we
// know the object name we are working with is in the format
// [uid]_tmp_[recursion_lvl]_[chunk_idx]. The only time we filter
// these temporary objects is on a delete operation so we can just
// append and continue without worrying about index order

switch len(split) {
case 1:
names = []string{objAttrs.Name}
break loop
case 2:
case 4:
names = append(names, objAttrs.Name)
continue
default:
err := errors.New("invalid filter format for object name")
return nil, err
}

idx, err := strconv.Atoi(split[1])
ok, err := isValidObject(objAttrs.Name)
if err != nil {
return nil, err
}

if len(names) <= idx {
names = append(names, make([]string, idx-len(names)+1)...)
if ok {
names = append(names, objAttrs.Name)
}

names[idx] = objAttrs.Name
}

return names, nil
Expand Down
107 changes: 105 additions & 2 deletions pkg/gcsstore/gcsservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,58 @@ func TestFilterObject(t *testing.T) {
return
}

if !reflect.DeepEqual(objects, []string{"", "test_directory/test-prefix_1", "test_directory/test-prefix_2"}) {
if !reflect.DeepEqual(objects, []string{"test_directory/test-prefix_1", "test_directory/test-prefix_2"}) {
t.Errorf("Didn't get appropriate objects back: got %v from %v", len(objects), objects)
}
}

func TestFilterObject_WithoutChunk(t *testing.T) {
defer gock.Off()

resp := googleBucketResponse{[]googleObjectResponse{
{Name: "test_directory/test-prefix"},
}}

gock.New("https://storage.googleapis.com").
Get("/storage/v1/b/test-bucket/o").
MatchParam("alt", "json").
MatchParam("pageToken", "").
MatchParam("prefix", "test-prefix").
MatchParam("projection", "full").
Reply(200).
JSON(resp)

gock.New("https://accounts.google.com/").
Post("/o/oauth2/token").Reply(200).JSON(map[string]string{
"access_token": "H3l5321N123sdI4HLY/RF39FjrCRF39FjrCRF39FjrCRF39FjrC_RF39FjrCRF39FjrC",
"token_type": "Bearer",
"refresh_token": "1/smWJksmWJksmWJksmWJksmWJk_smWJksmWJksmWJksmWJksmWJk",
"expiry_date": "1425333671141",
})

ctx := context.Background()
client, err := storage.NewClient(ctx, option.WithHTTPClient(http.DefaultClient), option.WithAPIKey("foo"))
if err != nil {
t.Fatal(err)
return
}

service := GCSService{
Client: client,
}

objects, err := service.FilterObjects(ctx, GCSFilterParams{
Bucket: "test-bucket",
Prefix: "test-prefix",
IncludeInfoObject: false,
})

if err != nil {
t.Errorf("Error: %v", err)
return
}

if !reflect.DeepEqual(objects, []string{"test_directory/test-prefix"}) {
t.Errorf("Didn't get appropriate objects back: got %v from %v", len(objects), objects)
}
}
Expand Down Expand Up @@ -576,7 +627,59 @@ func TestFilterObject_IncludeInfoObject(t *testing.T) {
return
}

if !reflect.DeepEqual(objects, []string{"", "test_directory/test-prefix_1", "test_directory/test-prefix.info", "test_directory/test-prefix_2"}) {
if !reflect.DeepEqual(objects, []string{"test_directory/test-prefix_1", "test_directory/test-prefix.info", "test_directory/test-prefix_2"}) {
t.Errorf("Didn't get appropriate objects back: got %v from %v", len(objects), objects)
}
}

func TestFilterObject_IncludeInfoObject_NonChunked(t *testing.T) {
defer gock.Off()

resp := googleBucketResponse{[]googleObjectResponse{
{Name: "test_directory/test-prefix"},
{Name: "test_directory/test-prefix.info"},
}}

gock.New("https://storage.googleapis.com").
Get("/storage/v1/b/test-bucket/o").
MatchParam("alt", "json").
MatchParam("pageToken", "").
MatchParam("prefix", "test-prefix").
MatchParam("projection", "full").
Reply(200).
JSON(resp)

gock.New("https://accounts.google.com/").
Post("/o/oauth2/token").Reply(200).JSON(map[string]string{
"access_token": "H3l5321N123sdI4HLY/RF39FjrCRF39FjrCRF39FjrCRF39FjrC_RF39FjrCRF39FjrC",
"token_type": "Bearer",
"refresh_token": "1/smWJksmWJksmWJksmWJksmWJk_smWJksmWJksmWJksmWJksmWJk",
"expiry_date": "1425333671141",
})

ctx := context.Background()
client, err := storage.NewClient(ctx, option.WithHTTPClient(http.DefaultClient), option.WithAPIKey("foo"))
if err != nil {
t.Fatal(err)
return
}

service := GCSService{
Client: client,
}

objects, err := service.FilterObjects(ctx, GCSFilterParams{
Bucket: "test-bucket",
Prefix: "test-prefix",
IncludeInfoObject: true,
})

if err != nil {
t.Errorf("Error: %v", err)
return
}

if !reflect.DeepEqual(objects, []string{"test_directory/test-prefix", "test_directory/test-prefix.info"}) {
t.Errorf("Didn't get appropriate objects back: got %v from %v", len(objects), objects)
}
}

0 comments on commit cb5ae65

Please sign in to comment.