diff --git a/blob/fileblob/fileblob.go b/blob/fileblob/fileblob.go index 245d30e0bc..c604071163 100644 --- a/blob/fileblob/fileblob.go +++ b/blob/fileblob/fileblob.go @@ -536,11 +536,13 @@ func (b *bucket) ListPaged(ctx context.Context, opts *driver.ListOptions) (*driv return nil } // If we've already got a full page of results, set NextPageToken and stop. - // Unless the current object is a directory, in which case there may - // still be objects coming that are alphabetically before it (since - // we appended the delimiter). In that case, keep going; we'll trim the - // extra entries (if any) before returning. - if len(result.Objects) == pageSize && !obj.IsDir { + // We can only stop if this object is guaranteed to belong after the page, + // i.e. it sorts after the last object in it. That isn't always the case: + // adding the delimiter can make an object sort before one we've already + // added (e.g., the file "a-b" sorts before the "directory" "a/", but is + // visited after it), so keep going in that case; we'll trim the extra + // entries before returning. + if len(result.Objects) == pageSize && !obj.IsDir && obj.Key > result.Objects[pageSize-1].Key { result.NextPageToken = []byte(result.Objects[pageSize-1].Key) return io.EOF } diff --git a/blob/fileblob/fileblob_test.go b/blob/fileblob/fileblob_test.go index 12469e0883..db12590f1e 100644 --- a/blob/fileblob/fileblob_test.go +++ b/blob/fileblob/fileblob_test.go @@ -25,6 +25,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "strings" "testing" @@ -684,3 +685,66 @@ func TestSkipMetadata(t *testing.T) { } } } + +// TestListPagedSortsBeforeDirectory checks that paging doesn't skip keys when a +// file sorts before the "directory" key generated for its sibling directory. +// For example, "dir-file" sorts before "dir/" because '-' < '/', but the walk +// visits "dir/" first, so the file can show up after the page is already full. +func TestListPagedSortsBeforeDirectory(t *testing.T) { + tests := []struct { + name string + keys []string + }{ + {"one sibling", []string{"dir/a", "dir/b", "dir-file", "f"}}, + {"several siblings", []string{"a/b", "a-c", "a-d"}}, + {"nested", []string{"x/y/z", "x-w", "x-v"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + b, err := OpenBucket(t.TempDir(), nil) + if err != nil { + t.Fatal(err) + } + defer closeWithErrorCheck(t, b) + for _, key := range test.keys { + if err := b.WriteAll(ctx, key, []byte("hello"), nil); err != nil { + t.Fatal(err) + } + } + opts := &blob.ListOptions{Delimiter: "/"} + + // An unpaged list gives the keys in the order they should appear. + var want []string + iter := b.List(opts) + for { + obj, err := iter.Next(ctx) + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + want = append(want, obj.Key) + } + + // Paging must not change the set of keys, or their order. + for pageSize := 1; pageSize <= len(test.keys); pageSize++ { + var got []string + for token := blob.FirstPageToken; token != nil; { + objs, next, err := b.ListPage(ctx, token, pageSize, opts) + if err != nil { + t.Fatal(err) + } + for _, obj := range objs { + got = append(got, obj.Key) + } + token = next + } + if !slices.Equal(got, want) { + t.Errorf("pageSize %d: got %v, want %v", pageSize, got, want) + } + } + }) + } +}