Skip to content

Commit 56e6c4e

Browse files
google-genai-botcopybara-github
authored andcommitted
fix: Handle JSON deserialized State.REMOVED sentinel
The State.REMOVED sentinel object is serialized to a specific string in JSON. When deserialized, this becomes a String object, not the original sentinel instance. This change updates the logic to correctly identify removals when the value is either the State.REMOVED object or its string representation. Additionally, when applying state deltas from app/user state, removals are now handled using a new removeWithoutDelta method on the State object to avoid re-recording these removals as new deltas. PiperOrigin-RevId: 881521527
1 parent 444e0f0 commit 56e6c4e

File tree

4 files changed

+110
-8
lines changed

4 files changed

+110
-8
lines changed

core/src/main/java/com/google/adk/sessions/BaseSessionService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ default Single<Event> appendEvent(Session session, Event event) {
236236
stateDelta.forEach(
237237
(key, value) -> {
238238
if (!key.startsWith(State.TEMP_PREFIX)) {
239-
if (value == State.REMOVED) {
239+
if (State.isRemoved(value)) {
240240
sessionState.remove(key);
241241
} else {
242242
sessionState.put(key, value);

core/src/main/java/com/google/adk/sessions/InMemorySessionService.java

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ public Single<Event> appendEvent(Session session, Event event) {
244244
(key, value) -> {
245245
if (key.startsWith(State.APP_PREFIX)) {
246246
String appStateKey = key.substring(State.APP_PREFIX.length());
247-
if (value == State.REMOVED) {
247+
if (State.isRemoved(value)) {
248248
appState
249249
.computeIfAbsent(appName, unused -> new ConcurrentHashMap<>())
250250
.remove(appStateKey);
@@ -255,7 +255,7 @@ public Single<Event> appendEvent(Session session, Event event) {
255255
}
256256
} else if (key.startsWith(State.USER_PREFIX)) {
257257
String userStateKey = key.substring(State.USER_PREFIX.length());
258-
if (value == State.REMOVED) {
258+
if (State.isRemoved(value)) {
259259
userState
260260
.computeIfAbsent(appName, unused -> new ConcurrentHashMap<>())
261261
.computeIfAbsent(userId, unused -> new ConcurrentHashMap<>())
@@ -267,8 +267,13 @@ public Single<Event> appendEvent(Session session, Event event) {
267267
.put(userStateKey, value);
268268
}
269269
} else {
270-
if (value == State.REMOVED) {
271-
session.state().remove(key);
270+
if (State.isRemoved(value)) {
271+
Map<String, Object> s = session.state();
272+
if (s instanceof State state) {
273+
state.removeWithoutDelta(key);
274+
} else {
275+
s.remove(key);
276+
}
272277
} else {
273278
session.state().put(key, value);
274279
}
@@ -333,12 +338,34 @@ private Session mergeWithGlobalState(String appName, String userId, Session sess
333338
// Merge App State directly into the session's state map
334339
appState
335340
.computeIfAbsent(appName, unused -> new ConcurrentHashMap<>())
336-
.forEach((key, value) -> sessionState.put(State.APP_PREFIX + key, value));
341+
.forEach(
342+
(key, value) -> {
343+
if (State.isRemoved(value)) {
344+
if (sessionState instanceof State state) {
345+
state.removeWithoutDelta(State.APP_PREFIX + key);
346+
} else {
347+
sessionState.remove(State.APP_PREFIX + key);
348+
}
349+
} else {
350+
sessionState.put(State.APP_PREFIX + key, value);
351+
}
352+
});
337353

338354
userState
339355
.computeIfAbsent(appName, unused -> new ConcurrentHashMap<>())
340356
.computeIfAbsent(userId, unused -> new ConcurrentHashMap<>())
341-
.forEach((key, value) -> sessionState.put(State.USER_PREFIX + key, value));
357+
.forEach(
358+
(key, value) -> {
359+
if (State.isRemoved(value)) {
360+
if (sessionState instanceof State state) {
361+
state.removeWithoutDelta(State.USER_PREFIX + key);
362+
} else {
363+
sessionState.remove(State.USER_PREFIX + key);
364+
}
365+
} else {
366+
sessionState.put(State.USER_PREFIX + key, value);
367+
}
368+
});
342369

343370
return session;
344371
}

core/src/main/java/com/google/adk/sessions/State.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.adk.sessions;
1818

1919
import com.fasterxml.jackson.annotation.JsonValue;
20+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2021
import java.util.Collection;
2122
import java.util.Map;
2223
import java.util.Map.Entry;
@@ -33,6 +34,8 @@ public final class State implements ConcurrentMap<String, Object> {
3334
public static final String USER_PREFIX = "user:";
3435
public static final String TEMP_PREFIX = "temp:";
3536

37+
public static final String REMOVED_SENTINEL_STRING = "__ADK_SENTINEL_REMOVED__";
38+
3639
/** Sentinel object to mark removed entries in the delta map. */
3740
public static final Object REMOVED = RemovedSentinel.INSTANCE;
3841

@@ -129,6 +132,19 @@ public Object remove(Object key) {
129132
return state.remove(key);
130133
}
131134

135+
/**
136+
* Removes a key from the state map without recording the removal in the delta map. This is
137+
* intended for internal use when rebuilding state from an event stream where the removal is
138+
* already known and doesn't need to be represented as a new change.
139+
*
140+
* @param key The key to remove.
141+
* @return The previous value associated with key, or null if there was no mapping for key.
142+
*/
143+
@CanIgnoreReturnValue
144+
public Object removeWithoutDelta(Object key) {
145+
return state.remove(key);
146+
}
147+
132148
@Override
133149
public boolean remove(Object key, Object value) {
134150
boolean removed = state.remove(key, value);
@@ -170,6 +186,16 @@ public boolean hasDelta() {
170186
return !delta.isEmpty();
171187
}
172188

189+
/**
190+
* Checks if a value represents a removed state entry, accounting for deserialization from JSON.
191+
*
192+
* @param value The value to check.
193+
* @return True if the value indicates removal, false otherwise.
194+
*/
195+
public static boolean isRemoved(Object value) {
196+
return value == REMOVED || Objects.equals(value, REMOVED_SENTINEL_STRING);
197+
}
198+
173199
private static final class RemovedSentinel {
174200
public static final RemovedSentinel INSTANCE = new RemovedSentinel();
175201

@@ -179,7 +205,7 @@ private RemovedSentinel() {
179205

180206
@JsonValue
181207
public String toJson() {
182-
return "__ADK_SENTINEL_REMOVED__";
208+
return REMOVED_SENTINEL_STRING;
183209
}
184210
}
185211
}

core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,55 @@ public void appendEvent_removesState() {
214214
assertThat(retrievedSessionRemove.state()).doesNotContainKey("temp:tempKey");
215215
}
216216

217+
@Test
218+
public void appendEvent_removesStateFromJsonDeserializedSentinel() {
219+
InMemorySessionService sessionService = new InMemorySessionService();
220+
Session session =
221+
sessionService.createSession("app", "user", new HashMap<>(), "session1").blockingGet();
222+
223+
ConcurrentMap<String, Object> stateDeltaAdd = new ConcurrentHashMap<>();
224+
stateDeltaAdd.put("sessionKey", "sessionValue");
225+
stateDeltaAdd.put("_app_appKey", "appValue");
226+
stateDeltaAdd.put("_user_userKey", "userValue");
227+
228+
Event eventAdd =
229+
Event.builder().actions(EventActions.builder().stateDelta(stateDeltaAdd).build()).build();
230+
231+
var unused = sessionService.appendEvent(session, eventAdd).blockingGet();
232+
233+
// Verify state is added
234+
Session retrievedSessionAdd =
235+
sessionService
236+
.getSession(session.appName(), session.userId(), session.id(), Optional.empty())
237+
.blockingGet();
238+
assertThat(retrievedSessionAdd.state()).containsEntry("sessionKey", "sessionValue");
239+
assertThat(retrievedSessionAdd.state()).containsEntry("_app_appKey", "appValue");
240+
assertThat(retrievedSessionAdd.state()).containsEntry("_user_userKey", "userValue");
241+
242+
// Prepare and append event to remove state using the String representation of the sentinel
243+
// to simulate Jackson JSON deserialization.
244+
ConcurrentMap<String, Object> stateDeltaRemove = new ConcurrentHashMap<>();
245+
stateDeltaRemove.put("sessionKey", State.REMOVED_SENTINEL_STRING);
246+
stateDeltaRemove.put("_app_appKey", State.REMOVED_SENTINEL_STRING);
247+
stateDeltaRemove.put("_user_userKey", State.REMOVED_SENTINEL_STRING);
248+
249+
Event eventRemove =
250+
Event.builder()
251+
.actions(EventActions.builder().stateDelta(stateDeltaRemove).build())
252+
.build();
253+
254+
unused = sessionService.appendEvent(session, eventRemove).blockingGet();
255+
256+
// Verify state is removed despite being a String instead of the State.REMOVED object
257+
Session retrievedSessionRemove =
258+
sessionService
259+
.getSession(session.appName(), session.userId(), session.id(), Optional.empty())
260+
.blockingGet();
261+
assertThat(retrievedSessionRemove.state()).doesNotContainKey("sessionKey");
262+
assertThat(retrievedSessionRemove.state()).doesNotContainKey("_app_appKey");
263+
assertThat(retrievedSessionRemove.state()).doesNotContainKey("_user_userKey");
264+
}
265+
217266
@Test
218267
public void sequentialAgents_shareTempState() {
219268
InMemorySessionService sessionService = new InMemorySessionService();

0 commit comments

Comments
 (0)