Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ public interface RedisClientAdapter {
*/
void addToSet(String key, String member);

/**
* Remove a member from a set.
*
* @param key the Redis set key
* @param member the member to remove
*/
default void removeFromSet(String key, String member) {
// Default no-op for custom adapters that do not track session keys in a Redis set.
}

/**
* Get all members of a set.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,19 @@ public void delete(SessionKey sessionKey) {
}
}

@Override
public void delete(SessionKey sessionKey, String key) {
String sessionId = sessionKey.toIdentifier();
String keysKey = getKeysKey(sessionId);

try {
client.deleteKeys(getStateKey(sessionId, key));
client.removeFromSet(keysKey, key);
} catch (Exception e) {
throw new RuntimeException("Failed to delete state: " + key, e);
}
}

@Override
public Set<SessionKey> listSessionKeys() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ public void addToSet(String key, String member) {
unifiedJedis.sadd(key, member);
}

@Override
public void removeFromSet(String key, String member) {
unifiedJedis.srem(key, member);
}

@Override
public Set<String> getSetMembers(String key) {
return unifiedJedis.smembers(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,15 @@ public void addToSet(String key, String member) {
}
}

@Override
public void removeFromSet(String key, String member) {
if (commands != null) {
commands.srem(key, member);
} else {
clusterCommands.srem(key, member);
}
}

@Override
public Set<String> getSetMembers(String key) {
if (commands != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ public void addToSet(String key, String member) {
rSet.add(member);
}

@Override
public void removeFromSet(String key, String member) {
RSet<String> rSet = redissonClient.getSet(key, StringCodec.INSTANCE);
rSet.remove(member);
}

@Override
public Set<String> getSetMembers(String key) {
RSet<String> rSet = redissonClient.getSet(key, StringCodec.INSTANCE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,17 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import io.agentscope.core.agent.Agent;
import io.agentscope.core.shutdown.GracefulShutdownManager;
import io.agentscope.core.shutdown.ShutdownInterruptedState;
import io.agentscope.core.state.SessionKey;
import io.agentscope.core.state.SimpleSessionKey;
import io.agentscope.core.state.State;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -221,6 +226,66 @@ void testDeleteSession() {
verify(unifiedJedis).smembers("agentscope:session:session1:_keys");
}

@Test
@DisplayName("Should delete single state key and remove tracking metadata")
void testDeleteSingleStateKey() {
RedisSession session =
RedisSession.builder()
.jedisClient(unifiedJedis)
.keyPrefix("agentscope:session:")
.build();

SessionKey sessionKey = SimpleSessionKey.of("session1");
session.delete(sessionKey, "shutdown_interrupted");

verify(unifiedJedis).del(new String[] {"agentscope:session:session1:shutdown_interrupted"});
verify(unifiedJedis).srem("agentscope:session:session1:_keys", "shutdown_interrupted");
}

@Test
@DisplayName("Should not return single state after deleting it")
void testDeleteSingleStateKeyRemovesPersistedValue() {
RedisSession session =
RedisSession.builder()
.clientAdapter(new StatefulRedisClientAdapter())
.keyPrefix("agentscope:session:")
.build();

SessionKey sessionKey = SimpleSessionKey.of("session1");
session.save(sessionKey, "shutdown_interrupted", new TestState("interrupted", 1));

assertTrue(session.get(sessionKey, "shutdown_interrupted", TestState.class).isPresent());

session.delete(sessionKey, "shutdown_interrupted");

assertFalse(session.get(sessionKey, "shutdown_interrupted", TestState.class).isPresent());
}

@Test
@DisplayName("Should clear shutdown interrupted flag after first manager check")
void testManagerCheckAndClearShutdownInterruptedWithRedisSession() {
GracefulShutdownManager manager = GracefulShutdownManager.getInstance();
manager.resetForTesting();
try {
RedisSession session =
RedisSession.builder()
.clientAdapter(new StatefulRedisClientAdapter())
.keyPrefix("agentscope:session:")
.build();
SessionKey sessionKey = SimpleSessionKey.of("session1");
Agent agent = mock(Agent.class);
when(agent.getAgentId()).thenReturn("agent-1");

session.save(sessionKey, "shutdown_interrupted", new ShutdownInterruptedState(true));
manager.bindSession(agent, session, sessionKey);

assertTrue(manager.checkAndClearShutdownInterrupted(agent));
assertFalse(manager.checkAndClearShutdownInterrupted(agent));
} finally {
manager.resetForTesting();
}
}

@Test
@DisplayName("Should list all session keys")
void testListSessionKeys() {
Expand Down Expand Up @@ -280,4 +345,76 @@ void testCloseShutsDownClient() {

/** Simple test state record for testing. */
public record TestState(String value, int count) implements State {}

private static class StatefulRedisClientAdapter implements RedisClientAdapter {

private final Map<String, String> values = new HashMap<>();
private final Map<String, Set<String>> sets = new HashMap<>();

@Override
public void set(String key, String value) {
values.put(key, value);
}

@Override
public String get(String key) {
return values.get(key);
}

@Override
public void rightPushList(String key, String value) {}

@Override
public List<String> rangeList(String key, long start, long end) {
return List.of();
}

@Override
public long getListLength(String key) {
return 0;
}

@Override
public void deleteKeys(String... keys) {
for (String key : keys) {
values.remove(key);

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StatefulRedisClientAdapter.deleteKeys(...) currently only removes entries from the values map. Since RedisClientAdapter.deleteKeys is used to delete any Redis key (including the session _keys set), this test adapter can become inconsistent if a test ever deletes set keys via deleteKeys. Consider removing deleted keys from both values and sets (and optionally dropping the set entry when it becomes empty to better mimic Redis behavior).

Suggested change
values.remove(key);
values.remove(key);
sets.remove(key);

Copilot uses AI. Check for mistakes.
}
}

@Override
public void addToSet(String key, String member) {
sets.computeIfAbsent(key, unused -> new HashSet<>()).add(member);
}

@Override
public void removeFromSet(String key, String member) {
Set<String> members = sets.get(key);
if (members != null) {
members.remove(member);
}
}

@Override
public Set<String> getSetMembers(String key) {
return sets.getOrDefault(key, Set.of());
}

@Override
public long getSetSize(String key) {
return getSetMembers(key).size();
}

@Override
public boolean keyExists(String key) {
return values.containsKey(key) || sets.containsKey(key);
}

@Override
public Set<String> findKeysByPattern(String pattern) {
return Set.of();
}

@Override
public void close() {}
}
}
Loading