Skip to content

Commit e44692c

Browse files
committed
fix(sessions): list only the empty user's sessions in FirestoreSessionService
list_sessions checked `if user_id:`, so user_id="" fell through to the all-users collection_group query and returned every user's sessions. Check `is not None` instead, as the other session services do. Fixes #7662
1 parent d324744 commit e44692c

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

‎src/google/adk/integrations/firestore/firestore_session_service.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ async def list_sessions(
368368
self, *, app_name: str, user_id: Optional[str] = None
369369
) -> ListSessionsResponse:
370370
"""Lists sessions from Firestore."""
371-
if user_id:
371+
if user_id is not None:
372372
query = self._get_sessions_ref(app_name, user_id).where(
373373
filter=FieldFilter("appName", "==", app_name)
374374
)
@@ -392,7 +392,7 @@ def _iter_sessions_data() -> Iterator[dict[str, Any]]:
392392
app_state = app_doc.to_dict() if app_doc.exists else {}
393393

394394
user_states_map = {}
395-
if user_id:
395+
if user_id is not None:
396396
user_ref = (
397397
self.client.collection(self.user_state_collection)
398398
.document(app_name)

‎tests/unittests/integrations/firestore/test_firestore_session_service.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,32 @@ async def mock_get_all(refs):
961961
assert _where_filter(where) == ("appName", "==", app_name)
962962

963963

964+
@pytest.mark.asyncio
965+
async def test_list_sessions_with_empty_user_id_lists_only_that_user(
966+
mock_firestore_client,
967+
):
968+
service = FirestoreSessionService(client=mock_firestore_client)
969+
root_coll = mock_firestore_client.collection.return_value
970+
users_coll = root_coll.document.return_value.collection.return_value
971+
user_doc_ref = users_coll.document.return_value
972+
user_doc_ref.collection.return_value.where.return_value.get = mock.AsyncMock(
973+
return_value=[]
974+
)
975+
user_doc_ref.get = mock.AsyncMock(return_value=_stored_snapshot(None))
976+
root_coll.document.return_value.get = mock.AsyncMock(
977+
return_value=_stored_snapshot(None)
978+
)
979+
mock_firestore_client.collection_group.return_value.where.return_value.get = (
980+
mock.AsyncMock(return_value=[])
981+
)
982+
983+
response = await service.list_sessions(app_name="test_app", user_id="")
984+
985+
assert response.sessions == []
986+
mock_firestore_client.collection_group.assert_not_called()
987+
users_coll.document.assert_any_call("")
988+
989+
964990
@pytest.mark.asyncio
965991
async def test_get_user_state_returns_stored_user_state(mock_firestore_client):
966992
"""get_user_state returns the user_states document for the app and user."""

0 commit comments

Comments
 (0)