diff --git a/core/src/main/java/com/google/adk/sessions/BaseSessionService.java b/core/src/main/java/com/google/adk/sessions/BaseSessionService.java index 8596f8eb6..b0fcdc99b 100644 --- a/core/src/main/java/com/google/adk/sessions/BaseSessionService.java +++ b/core/src/main/java/com/google/adk/sessions/BaseSessionService.java @@ -35,6 +35,10 @@ * methods for creating, retrieving, listing, and deleting sessions, as well as listing and * appending events to a session. Implementations of this interface handle the underlying storage * and retrieval logic. + * + *

Every {@code createSession} overload that accepts a session id shares one contract: an + * implementation that rejects duplicates signals a taken id with {@link + * SessionAlreadyExistsException}, and implementations differ on whether they do. */ public interface BaseSessionService { @@ -68,6 +72,8 @@ Single createSession( * @param sessionId An optional client-provided identifier for the session. If empty or null, the * service should generate a unique ID. * @return The newly created {@link Session} instance. + * @throws SessionAlreadyExistsException if {@code sessionId} is already in use for this app and + * user; only implementations that reject duplicates throw it. * @throws SessionException if creation fails. */ default Single createSession( diff --git a/core/src/main/java/com/google/adk/sessions/InMemorySessionService.java b/core/src/main/java/com/google/adk/sessions/InMemorySessionService.java index e54289b76..39016ee47 100644 --- a/core/src/main/java/com/google/adk/sessions/InMemorySessionService.java +++ b/core/src/main/java/com/google/adk/sessions/InMemorySessionService.java @@ -74,6 +74,14 @@ public Single createSession( return createSession(appName, userId, (Map) state, sessionId); } + /** + * {@inheritDoc} + * + *

This implementation never overwrites an existing session. + * + * @throws SessionAlreadyExistsException if {@code sessionId} is supplied and a session already + * exists under it for this app and user. + */ @Override public Single createSession( String appName, @@ -102,10 +110,15 @@ public Single createSession( .lastUpdateTime(Instant.now()) .build(); - sessions - .computeIfAbsent(appName, unused -> new ConcurrentHashMap<>()) - .computeIfAbsent(userId, unused -> new ConcurrentHashMap<>()) - .put(resolvedSessionId, newSession); + // Atomic: a read-then-write would let two concurrent creators both win. + Session existing = + sessions + .computeIfAbsent(appName, unused -> new ConcurrentHashMap<>()) + .computeIfAbsent(userId, unused -> new ConcurrentHashMap<>()) + .putIfAbsent(resolvedSessionId, newSession); + if (existing != null) { + return Single.error(new SessionAlreadyExistsException()); + } // Create a mutable copy for the return value Session returnCopy = copySession(newSession); diff --git a/core/src/main/java/com/google/adk/sessions/SessionAlreadyExistsException.java b/core/src/main/java/com/google/adk/sessions/SessionAlreadyExistsException.java new file mode 100644 index 000000000..6954f7c9f --- /dev/null +++ b/core/src/main/java/com/google/adk/sessions/SessionAlreadyExistsException.java @@ -0,0 +1,29 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.sessions; + +/** Indicates that a session already exists under the caller-supplied session id. */ +public class SessionAlreadyExistsException extends SessionException { + + public SessionAlreadyExistsException() { + super("Session already exists"); + } + + public SessionAlreadyExistsException(Throwable cause) { + super("Session already exists", cause); + } +} diff --git a/core/src/test/java/com/google/adk/flows/llmflows/ContentsTest.java b/core/src/test/java/com/google/adk/flows/llmflows/ContentsTest.java index 5597ce8f2..0b79e5067 100644 --- a/core/src/test/java/com/google/adk/flows/llmflows/ContentsTest.java +++ b/core/src/test/java/com/google/adk/flows/llmflows/ContentsTest.java @@ -65,7 +65,9 @@ public final class ContentsTest { private static final String OTHER_AGENT = "other_agent"; private static final Contents contentsProcessor = new Contents(); - private static final InMemorySessionService sessionService = new InMemorySessionService(); + + // Per-test: every helper creates the same session id, so a shared service would reject it. + private final InMemorySessionService sessionService = new InMemorySessionService(); @Test public void rearrangeLatest_emptyList_returnsEmptyList() { diff --git a/core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java b/core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java index 58c445641..30ee02565 100644 --- a/core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java +++ b/core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java @@ -16,6 +16,7 @@ package com.google.adk.sessions; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import com.google.adk.events.Event; import com.google.adk.events.EventActions; @@ -114,6 +115,48 @@ public void lifecycle_listSessions() { assertThat(listedSession.state()).containsEntry("temp:tempKey", "tempValue"); } + @Test + public void createSession_duplicateSessionId_throwsAndKeepsExistingSession() { + InMemorySessionService sessionService = new InMemorySessionService(); + HashMap initialState = new HashMap<>(); + initialState.put("sessionKey", "sessionValue"); + Session session = + sessionService + .createSession("app-name", "user-id", initialState, "session-1") + .blockingGet(); + var unused = sessionService.appendEvent(session, Event.builder().build()).blockingGet(); + + Single duplicate = + sessionService.createSession("app-name", "user-id", new HashMap<>(), "session-1"); + + assertThrows(SessionAlreadyExistsException.class, duplicate::blockingGet); + + assertThat(sessionService.listEvents("app-name", "user-id", "session-1").blockingGet().events()) + .hasSize(1); + Session stored = + sessionService + .getSession("app-name", "user-id", "session-1", Optional.empty()) + .blockingGet(); + assertThat(stored.state()).containsEntry("sessionKey", "sessionValue"); + } + + @Test + public void createSession_sameSessionIdDifferentUser_isAllowed() { + InMemorySessionService sessionService = new InMemorySessionService(); + var unused = + sessionService + .createSession("app-name", "user-a", new HashMap<>(), "session-1") + .blockingGet(); + + Session other = + sessionService + .createSession("app-name", "user-b", new HashMap<>(), "session-1") + .blockingGet(); + + assertThat(other.id()).isEqualTo("session-1"); + assertThat(other.userId()).isEqualTo("user-b"); + } + @Test public void lifecycle_deleteSession() { InMemorySessionService sessionService = new InMemorySessionService();