From 832717defdb3d9f84bebd410555b30bb5e50087b Mon Sep 17 00:00:00 2001 From: Ashfaq <105435085+Ashfaqbs@users.noreply.github.com> Date: Thu, 27 Aug 2026 07:54:57 +0530 Subject: [PATCH] fix: honor the user: namespace convention in InMemoryArtifactService InMemoryArtifactService keyed every artifact by (appName, userId, sessionId, filename) unconditionally, so a "user:"-prefixed filename was silently scoped to the saving session instead of being visible from every session for that user. GcsArtifactService and adk-python's InMemoryArtifactService both already special-case this prefix. Rework the backing map to a flat path-keyed map, mirroring GcsArtifactService's blob-prefix scheme: a "user:"-prefixed filename is stored under an appName/userId/user/filename key independent of sessionId, and listArtifactKeys unions the session-scoped and user-namespaced prefixes the same way GcsArtifactService unions the two GCS prefixes. Fixes #1460. Generated-by: Claude Code 2.0.76 (Claude Sonnet 5) --- .../artifacts/InMemoryArtifactService.java | 64 +++++++++++++------ .../InMemoryArtifactServiceTest.java | 47 ++++++++++++++ 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/com/google/adk/artifacts/InMemoryArtifactService.java b/core/src/main/java/com/google/adk/artifacts/InMemoryArtifactService.java index 510c96c2e..1854f33ce 100644 --- a/core/src/main/java/com/google/adk/artifacts/InMemoryArtifactService.java +++ b/core/src/main/java/com/google/adk/artifacts/InMemoryArtifactService.java @@ -33,12 +33,36 @@ /** An in-memory implementation of the {@link BaseArtifactService}. */ public final class InMemoryArtifactService implements BaseArtifactService { - private final Map>>>> artifacts; + private final Map> artifacts; public InMemoryArtifactService() { this.artifacts = new HashMap<>(); } + /** + * Checks if a filename uses the user namespace. + * + * @param filename Filename to check. + * @return true if prefixed with "user:", false otherwise. + */ + private static boolean fileHasUserNamespace(String filename) { + return filename != null && filename.startsWith("user:"); + } + + /** + * Builds the storage key for an artifact. + * + *

A "user:"-prefixed filename is stored under a session-independent, user-scoped key so it is + * visible from every session for that user, matching {@link GcsArtifactService} and adk-python's + * {@code InMemoryArtifactService}. Any other filename is scoped to the given session, as before. + */ + private static String artifactKey( + String appName, String userId, String sessionId, String filename) { + return fileHasUserNamespace(filename) + ? String.format("%s/%s/user/%s", appName, userId, filename) + : String.format("%s/%s/%s/%s", appName, userId, sessionId, filename); + } + /** * Saves an artifact in memory and assigns a new version. * @@ -48,8 +72,8 @@ public InMemoryArtifactService() { public Single saveArtifact( String appName, String userId, String sessionId, String filename, Part artifact) { List versions = - getArtifactsMap(appName, userId, sessionId) - .computeIfAbsent(filename, unused -> new ArrayList<>()); + artifacts.computeIfAbsent( + artifactKey(appName, userId, sessionId, filename), unused -> new ArrayList<>()); versions.add(artifact); return Single.just(versions.size() - 1); } @@ -63,8 +87,7 @@ public Single saveArtifact( public Maybe loadArtifact( String appName, String userId, String sessionId, String filename, @Nullable Integer version) { List versions = - getArtifactsMap(appName, userId, sessionId) - .computeIfAbsent(filename, unused -> new ArrayList<>()); + artifacts.getOrDefault(artifactKey(appName, userId, sessionId, filename), List.of()); if (versions.isEmpty()) { return Maybe.empty(); @@ -81,17 +104,25 @@ public Maybe loadArtifact( } /** - * Lists filenames of stored artifacts for the session. + * Lists filenames of stored artifacts for the session, including every "user:"-namespaced + * artifact for the user regardless of which session saved it. * * @return Single with list of artifact filenames. */ @Override public Single listArtifactKeys( String appName, String userId, String sessionId) { - return Single.just( - ListArtifactsResponse.builder() - .filenames(ImmutableList.copyOf(getArtifactsMap(appName, userId, sessionId).keySet())) - .build()); + String sessionPrefix = String.format("%s/%s/%s/", appName, userId, sessionId); + String userPrefix = String.format("%s/%s/user/", appName, userId); + List filenames = new ArrayList<>(); + for (String key : artifacts.keySet()) { + if (key.startsWith(sessionPrefix)) { + filenames.add(key.substring(sessionPrefix.length())); + } else if (key.startsWith(userPrefix)) { + filenames.add(key.substring(userPrefix.length())); + } + } + return Single.just(ListArtifactsResponse.builder().filenames(filenames).build()); } /** @@ -102,7 +133,7 @@ public Single listArtifactKeys( @Override public Completable deleteArtifact( String appName, String userId, String sessionId, String filename) { - getArtifactsMap(appName, userId, sessionId).remove(filename); + artifacts.remove(artifactKey(appName, userId, sessionId, filename)); return Completable.complete(); } @@ -115,9 +146,7 @@ public Completable deleteArtifact( public Single> listVersions( String appName, String userId, String sessionId, String filename) { int size = - getArtifactsMap(appName, userId, sessionId) - .computeIfAbsent(filename, unused -> new ArrayList<>()) - .size(); + artifacts.getOrDefault(artifactKey(appName, userId, sessionId, filename), List.of()).size(); if (size == 0) { return Single.just(ImmutableList.of()); } @@ -130,11 +159,4 @@ public Single saveAndReloadArtifact( return saveArtifact(appName, userId, sessionId, filename, artifact) .flatMap(version -> loadArtifact(appName, userId, sessionId, filename, version).toSingle()); } - - private Map> getArtifactsMap(String appName, String userId, String sessionId) { - return artifacts - .computeIfAbsent(appName, unused -> new HashMap<>()) - .computeIfAbsent(userId, unused -> new HashMap<>()) - .computeIfAbsent(sessionId, unused -> new HashMap<>()); - } } diff --git a/core/src/test/java/com/google/adk/artifacts/InMemoryArtifactServiceTest.java b/core/src/test/java/com/google/adk/artifacts/InMemoryArtifactServiceTest.java index 124a5e9d8..890bfeeeb 100644 --- a/core/src/test/java/com/google/adk/artifacts/InMemoryArtifactServiceTest.java +++ b/core/src/test/java/com/google/adk/artifacts/InMemoryArtifactServiceTest.java @@ -72,6 +72,53 @@ public void saveAndReloadArtifact_reloadsArtifact() { assertThat(result).hasValue(artifact); } + @Test + public void loadArtifact_userNamespacedFile_visibleFromDifferentSession() { + String userFilename = "user:profile.txt"; + Part artifact = Part.fromBytes(new byte[] {1, 2, 3}, "text/plain"); + var unused = + service.saveArtifact(APP_NAME, USER_ID, "session-a", userFilename, artifact).blockingGet(); + + Optional result = + asOptional(service.loadArtifact(APP_NAME, USER_ID, "session-b", userFilename)); + + assertThat(result).hasValue(artifact); + } + + @Test + public void listArtifactKeys_includesUserNamespacedFilesFromOtherSessions() { + String userFilename = "user:profile.txt"; + Part userArtifact = Part.fromBytes(new byte[] {1}, "text/plain"); + Part sessionArtifact = Part.fromBytes(new byte[] {2}, "text/plain"); + var unused1 = + service + .saveArtifact(APP_NAME, USER_ID, "session-a", userFilename, userArtifact) + .blockingGet(); + var unused2 = + service + .saveArtifact(APP_NAME, USER_ID, "session-b", FILENAME, sessionArtifact) + .blockingGet(); + + ListArtifactsResponse response = + service.listArtifactKeys(APP_NAME, USER_ID, "session-b").blockingGet(); + + assertThat(response.filenames()).containsExactly(userFilename, FILENAME); + } + + @Test + public void deleteArtifact_userNamespacedFile_removesFromEverySession() { + String userFilename = "user:profile.txt"; + Part artifact = Part.fromBytes(new byte[] {1, 2, 3}, "text/plain"); + var unused = + service.saveArtifact(APP_NAME, USER_ID, "session-a", userFilename, artifact).blockingGet(); + + service.deleteArtifact(APP_NAME, USER_ID, "session-b", userFilename).blockingAwait(); + + Optional result = + asOptional(service.loadArtifact(APP_NAME, USER_ID, "session-a", userFilename)); + assertThat(result).isEmpty(); + } + private static Optional asOptional(Maybe maybe) { return maybe.map(Optional::of).defaultIfEmpty(Optional.empty()).blockingGet(); }