mirror of
https://github.com/google/adk-go.git
synced 2026-10-28 08:34:37 +08:00
fix(artifact): keep session filenames out of GCS shared listings (#1708)
Narrow the shared-artifact query prefix to exclude private files saved by session "user". Move the existing regression test into the shared suite so both storage backends cover session isolation and version deduplication. Co-authored-by: wolo <wolo@google.com>
This commit is contained in:
1 parent
4f2299737f
commit
9026ee8c0d
3 files changed
+65
-57
No files matched your search
@@ -98,7 +98,10 @@ func buildSessionPrefix(appName, userID, sessionID string) string {
|
||||
}
|
||||
|
||||
func buildUserPrefix(appName, userID string) string {
|
||||
return fmt.Sprintf("%s/%s/user/", appName, userID)
|
||||
// Session ID "user" shares this path with user-scoped artifacts, so include
|
||||
// the filename namespace to exclude its private files. Python rejects the
|
||||
// ID instead, but Go already accepts it.
|
||||
return fmt.Sprintf("%s/%s/user/user:", appName, userID)
|
||||
}
|
||||
|
||||
const (
|
||||
|
||||
@@ -16,7 +16,6 @@ package artifact_test
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"slices"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -35,61 +34,6 @@ func TestInMemoryArtifactService(t *testing.T) {
|
||||
tests.TestArtifactService(t, "InMemory", factory)
|
||||
}
|
||||
|
||||
func TestInMemoryListSessionNamedUser(t *testing.T) {
|
||||
srv := artifact.InMemoryService()
|
||||
for _, file := range []struct {
|
||||
sessionID string
|
||||
fileName string
|
||||
}{
|
||||
{"user", "private.txt"},
|
||||
{"user", "private.txt"}, // Multiple versions must not duplicate filenames.
|
||||
{"user", "user:shared.txt"},
|
||||
{"other-session", "user:shared.txt"},
|
||||
{"other-session", "other.txt"},
|
||||
} {
|
||||
if _, err := srv.Save(t.Context(), &artifact.SaveRequest{
|
||||
AppName: "app", UserID: "u", SessionID: file.sessionID,
|
||||
FileName: file.fileName, Part: genai.NewPartFromText("fixture"),
|
||||
}); err != nil {
|
||||
t.Fatalf("Save failed: %T", err)
|
||||
}
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
sessionID string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "owner retains private and shared artifacts",
|
||||
sessionID: "user",
|
||||
want: []string{"private.txt", "user:shared.txt"},
|
||||
},
|
||||
{
|
||||
name: "other session sees only its own and shared artifacts",
|
||||
sessionID: "other-session",
|
||||
want: []string{"other.txt", "user:shared.txt"},
|
||||
},
|
||||
{
|
||||
name: "empty session sees only shared artifacts",
|
||||
sessionID: "empty-session",
|
||||
want: []string{"user:shared.txt"},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
resp, err := srv.List(t.Context(), &artifact.ListRequest{
|
||||
AppName: "app", UserID: "u", SessionID: tc.sessionID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("List failed: %T", err)
|
||||
}
|
||||
if !slices.Equal(resp.FileNames, tc.want) {
|
||||
t.Error("List must return only the session's own and user-scoped filenames, sorted and without duplicates")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestInMemoryArtifactVersionFields(t *testing.T) {
|
||||
firstCreateTime := time.Date(2026, time.August, 31, 12, 34, 56, 0, time.UTC)
|
||||
secondCreateTime := firstCreateTime.Add(time.Minute)
|
||||
|
||||
@@ -56,6 +56,13 @@ func TestArtifactService(t *testing.T, name string, factory func(t *testing.T) (
|
||||
}
|
||||
testArtifactService_UserScoped(ctx, t, srv, name)
|
||||
})
|
||||
t.Run(fmt.Sprintf("Test%sArtifactService_ListSessionNamedUser", name), func(t *testing.T) {
|
||||
srv, err := factory(t)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to set up service: %T", err)
|
||||
}
|
||||
testArtifactService_ListSessionNamedUser(t, srv)
|
||||
})
|
||||
t.Run(fmt.Sprintf("Test%sArtifactService_GetArtifactVersion", name), func(t *testing.T) {
|
||||
ctx := t.Context()
|
||||
// Create the service using the factory for this sub-test.
|
||||
@@ -67,6 +74,60 @@ func TestArtifactService(t *testing.T, name string, factory func(t *testing.T) (
|
||||
})
|
||||
}
|
||||
|
||||
func testArtifactService_ListSessionNamedUser(t *testing.T, srv artifact.Service) {
|
||||
for _, file := range []struct {
|
||||
sessionID string
|
||||
fileName string
|
||||
}{
|
||||
{"user", "private.txt"},
|
||||
{"user", "private.txt"}, // Multiple versions must not duplicate filenames.
|
||||
{"user", "user:shared.txt"},
|
||||
{"other-session", "user:shared.txt"},
|
||||
{"other-session", "other.txt"},
|
||||
} {
|
||||
if _, err := srv.Save(t.Context(), &artifact.SaveRequest{
|
||||
AppName: "app", UserID: "u", SessionID: file.sessionID,
|
||||
FileName: file.fileName, Part: genai.NewPartFromText("fixture"),
|
||||
}); err != nil {
|
||||
t.Fatalf("Save failed: %T", err)
|
||||
}
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
sessionID string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "owner retains private and shared artifacts",
|
||||
sessionID: "user",
|
||||
want: []string{"private.txt", "user:shared.txt"},
|
||||
},
|
||||
{
|
||||
name: "other session sees only its own and shared artifacts",
|
||||
sessionID: "other-session",
|
||||
want: []string{"other.txt", "user:shared.txt"},
|
||||
},
|
||||
{
|
||||
name: "empty session sees only shared artifacts",
|
||||
sessionID: "empty-session",
|
||||
want: []string{"user:shared.txt"},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
resp, err := srv.List(t.Context(), &artifact.ListRequest{
|
||||
AppName: "app", UserID: "u", SessionID: tc.sessionID,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("List failed: %T", err)
|
||||
}
|
||||
if !slices.Equal(resp.FileNames, tc.want) {
|
||||
t.Error("List must return only the session's own and user-scoped filenames, sorted and without duplicates")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func testArtifactService(ctx context.Context, t *testing.T, srv artifact.Service, testSuffix string) {
|
||||
appName := "testapp"
|
||||
userID := "testuser"
|
||||
|
||||
Reference in new issue
Block a user