[build] internal/gcsfs: handle explicit GCS folders

[Nikita Kruglov (Gerrit)]

Nikita Kruglov has uploaded the change for review

Commit message

internal/gcsfs: handle explicit GCS folders

GCS can represent an explicit folder with a zero-byte object whose name
ends in a slash. When listing that folder, GCS returns the marker itself
alongside its children. ReadDir treats it as a regular file with the
folder's base name, introducing a nonexistent child.

Skip directory marker objects when constructing ReadDir results. Keep
them visible to Stat's iterator so explicit empty folders can still be
recognized as directories.

Fixes golang/go#81359

Change-Id: I1143546363ef27c6e098215dcd3946e6c091850b

Change diff

diff --git a/internal/gcsfs/gcsfs.go b/internal/gcsfs/gcsfs.go
index 5a428eb..b426ab2 100644
--- a/internal/gcsfs/gcsfs.go
+++ b/internal/gcsfs/gcsfs.go
@@ -217,6 +217,10 @@
 		if err != nil {
 			break
 		}
+		if strings.HasSuffix(info.Name, "/") {
+			// This object marks the directory itself, not one of its children.
+			continue
+		}
 		result = append(result, &gcsFileInfo{info})
 		if len(result) == n {
 			break
diff --git a/internal/gcsfs/gcsfs_test.go b/internal/gcsfs/gcsfs_test.go
index de5f1c0..ff6c532 100644
--- a/internal/gcsfs/gcsfs_test.go
+++ b/internal/gcsfs/gcsfs_test.go
@@ -7,9 +7,14 @@
 import (
 	"context"
 	"flag"
+	"fmt"
+	"io"
 	"io/fs"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"slices"
 	"testing"
 	"testing/fstest"
 	"time"
@@ -20,6 +25,117 @@
 
 var slowTest = flag.Bool("slow", false, "run slow tests that access GCS")
 
+func TestGCSReadDir(t *testing.T) {
+	// GCS lists child directories as prefixes, but returns a directory's own
+	// marker object in items when its name equals the query prefix.
+	for _, test := range []struct {
+		name, dir, response string
+		want                []string
+	}{
+		{
+			name:     "root",
+			dir:      ".",
+			response: `{"prefixes":["dir/","empty/","implicit/"],"items":[{"name":"zero.txt","size":"0"}]}`,
+			want:     []string{"dir/", "empty/", "implicit/", "zero.txt"},
+		},
+		{
+			name:     "implicit",
+			dir:      "implicit",
+			response: `{"items":[{"name":"implicit/file.txt","size":"4"}]}`,
+			want:     []string{"file.txt"},
+		},
+		{
+			name:     "explicit",
+			dir:      "dir",
+			response: `{"items":[{"name":"dir/","size":"0"},{"name":"dir/file.txt","size":"4"}]}`,
+			want:     []string{"file.txt"},
+		},
+		{
+			name:     "empty",
+			dir:      "empty",
+			response: `{"items":[{"name":"empty/","size":"0"}]}`,
+		},
+		{
+			name:     "nested",
+			dir:      "dir/nested",
+			response: `{"prefixes":["dir/nested/empty/"],"items":[{"name":"dir/nested/","size":"0"},{"name":"dir/nested/file.txt","size":"4"}]}`,
+			want:     []string{"empty/", "file.txt"},
+		},
+		{
+			name:     "nested_empty",
+			dir:      "dir/nested/empty",
+			response: `{"items":[{"name":"dir/nested/empty/","size":"0"}]}`,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				prefix := ""
+				if test.dir != "." {
+					prefix = test.dir + "/"
+				}
+				if r.Method != "GET" || r.URL.Path != "/b/test/o" || r.URL.Query().Get("prefix") != prefix || r.URL.Query().Get("delimiter") != "/" {
+					t.Errorf("unexpected request: %s %s", r.Method, r.URL)
+					http.Error(w, "unexpected request", http.StatusBadRequest)
+					return
+				}
+				w.Header().Set("Content-Type", "application/json")
+				io.WriteString(w, test.response)
+			}))
+			defer server.Close()
+			client, err := storage.NewClient(t.Context(), option.WithEndpoint(server.URL), option.WithoutAuthentication())
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer client.Close()
+			fsys := NewFS(t.Context(), client, "test")
+			for _, sub := range []bool{false, true} {
+				for _, n := range []int{-1, 0, 1, 2} {
+					t.Run(fmt.Sprintf("sub=%v/n=%d", sub, n), func(t *testing.T) {
+						fsys, dir := fsys, test.dir
+						if sub {
+							var err error
+							fsys, err = fs.Sub(fsys, dir)
+							if err != nil {
+								t.Fatal(err)
+							}
+							dir = "."
+						}
+						f, err := fsys.Open(dir)
+						if err != nil {
+							t.Fatal(err)
+						}
+						defer f.Close()
+						var got []string
+						for {
+							entries, err := f.(fs.ReadDirFile).ReadDir(n)
+							if err != nil && (n <= 0 || err != io.EOF) {
+								t.Fatal(err)
+							}
+							if n > 0 && (len(entries) > n || len(entries) == 0 && err == nil) {
+								t.Fatalf("ReadDir(%d) returned %d entries, %v", n, len(entries), err)
+							}
+							for _, entry := range entries {
+								name := entry.Name()
+								if entry.IsDir() {
+									name += "/"
+								}
+								got = append(got, name)
+							}
+							if n <= 0 || err == io.EOF {
+								break
+							}
+						}
+						slices.Sort(got)
+						if !slices.Equal(got, test.want) {
+							t.Errorf("ReadDir: got %q, want %q", got, test.want)
+						}
+					})
+				}
+			}
+		})
+	}
+}
+
 func TestGCSFS(t *testing.T) {
 	if !*slowTest {
 		t.Skip("reads a largeish GCS bucket")

Change information

Files:

• M internal/gcsfs/gcsfs.go
• M internal/gcsfs/gcsfs_test.go

Change size: M

Delta: 2 files changed, 120 insertions(+), 0 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newchange

Gerrit-Project: build

Gerrit-Branch: master

Gerrit-Change-Id: I1143546363ef27c6e098215dcd3946e6c091850b

Gerrit-Change-Number: 828164

Gerrit-PatchSet: 1

Gerrit-Owner: Nikita Kruglov <nikkru...@gmail.com>