Skip to content

Commit 99ba8ce

Browse files
kkj333GWeale
authored andcommitted
fix(firestore): preserve update timestamps in list_sessions
Merge #5640 ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** Closes: #5632 ### Problem `FirestoreSessionService.list_sessions()` always sets `Session.last_update_time` to `0.0`, even when Firestore documents contain `updateTime`. ### Solution Use the same timestamp conversion logic as `get_session()` in `list_sessions()`: - if `updateTime` is a `datetime`, use `.timestamp()` - otherwise, attempt `float(updateTime)` - fallback to `0.0` when conversion fails This keeps list responses consistent with `get_session()` and preserves accurate update timestamps. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Updated tests: - `test_list_sessions_with_user_id` - `test_list_sessions_without_user_id` Local pytest summary: - `tests/unittests/integrations/firestore/test_firestore_session_service.py` - `17 passed` **Manual End-to-End (E2E) Tests:** Reproduced with a mocked Firestore client where session docs include `updateTime`; before fix `last_update_time` was `0.0`, after fix it matches the Firestore value. ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [ ] Any dependent changes have been merged and published in downstream modules. Co-authored-by: George Weale <gweale@google.com> COPYBARA_INTEGRATE_REVIEW=#5640 from kkj333:fix/firestore-list-sessions-update-time-5632 9586cc6 PiperOrigin-RevId: 941318502
1 parent 5735690 commit 99ba8ce

2 files changed

Lines changed: 90 additions & 14 deletions

File tree

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@
6262
DEFAULT_USER_STATE_COLLECTION = "user_states"
6363

6464

65+
def _to_last_update_time(update_time: Any) -> float:
66+
"""Converts a Firestore updateTime value to epoch seconds, or 0.0."""
67+
if not update_time:
68+
return 0.0
69+
if isinstance(update_time, datetime):
70+
return update_time.timestamp()
71+
try:
72+
return float(update_time)
73+
except (ValueError, TypeError):
74+
return 0.0
75+
76+
6577
class FirestoreSessionService(BaseSessionService): # type: ignore[misc]
6678
"""Session service that uses Google Cloud Firestore as the backend.
6779
@@ -322,26 +334,14 @@ async def get_session(
322334

323335
merged_state = self._merge_state(app_state, user_state, session_state)
324336

325-
# Convert timestamp
326-
update_time = data.get("updateTime")
327-
last_update_time = 0.0
328-
if update_time:
329-
if isinstance(update_time, datetime):
330-
last_update_time = update_time.timestamp()
331-
else:
332-
try:
333-
last_update_time = float(update_time)
334-
except (ValueError, TypeError):
335-
pass
336-
337337
current_revision = data.get("revision", 0)
338338
session = Session(
339339
id=session_id,
340340
app_name=app_name,
341341
user_id=user_id,
342342
state=merged_state,
343343
events=events,
344-
last_update_time=last_update_time,
344+
last_update_time=_to_last_update_time(data.get("updateTime")),
345345
)
346346
session._storage_update_marker = str(current_revision)
347347
return session
@@ -418,7 +418,7 @@ def _iter_sessions_data() -> Iterator[dict[str, Any]]:
418418
user_id=data["userId"],
419419
state=merged,
420420
events=[],
421-
last_update_time=0.0,
421+
last_update_time=_to_last_update_time(data.get("updateTime")),
422422
)
423423
)
424424

tests/unittests/integrations/firestore/test_firestore_session_service.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
from __future__ import annotations
1616

17+
from datetime import datetime
18+
from datetime import timezone
1719
import json
1820
from unittest import mock
1921

@@ -387,6 +389,7 @@ async def test_list_sessions_with_user_id(mock_firestore_client):
387389
"appName": app_name,
388390
"userId": user_id,
389391
"state": {"session_key": "session_val"},
392+
"updateTime": 1234567890.0,
390393
}
391394

392395
app_state_coll = mock.MagicMock()
@@ -442,6 +445,77 @@ def collection_side_effect(name):
442445
assert session.state["session_key"] == "session_val"
443446
assert session.state["app:app_key"] == "app_val"
444447
assert session.state["user:user_key"] == "user_val"
448+
assert session.last_update_time == 1234567890.0
449+
450+
451+
@pytest.mark.asyncio
452+
async def test_list_sessions_preserves_datetime_update_time(
453+
mock_firestore_client,
454+
):
455+
"""list_sessions converts datetime updateTime to last_update_time."""
456+
service = FirestoreSessionService(client=mock_firestore_client)
457+
app_name = "test_app"
458+
user_id = "test_user"
459+
update_time = datetime(2024, 6, 15, 10, 30, 0, tzinfo=timezone.utc)
460+
461+
session_doc = mock.MagicMock()
462+
session_doc.to_dict.return_value = {
463+
"id": "session1",
464+
"appName": app_name,
465+
"userId": user_id,
466+
"state": {},
467+
"updateTime": update_time,
468+
}
469+
470+
app_state_coll = mock.MagicMock()
471+
user_state_coll = mock.MagicMock()
472+
sessions_coll = mock.MagicMock()
473+
474+
def collection_side_effect(name):
475+
if name == service.app_state_collection:
476+
return app_state_coll
477+
elif name == service.user_state_collection:
478+
return user_state_coll
479+
elif name == service.root_collection:
480+
return sessions_coll
481+
return mock.MagicMock()
482+
483+
mock_firestore_client.collection.side_effect = collection_side_effect
484+
485+
app_doc = mock.MagicMock()
486+
app_doc.exists = False
487+
app_doc.to_dict.return_value = {}
488+
app_doc_ref = mock.MagicMock()
489+
app_state_coll.document.return_value = app_doc_ref
490+
app_doc_ref.get = mock.AsyncMock(return_value=app_doc)
491+
492+
user_doc = mock.MagicMock()
493+
user_doc.exists = False
494+
user_doc.to_dict.return_value = {}
495+
user_app_doc = mock.MagicMock()
496+
user_state_coll.document.return_value = user_app_doc
497+
users_coll = mock.MagicMock()
498+
user_app_doc.collection.return_value = users_coll
499+
user_doc_ref = mock.MagicMock()
500+
users_coll.document.return_value = user_doc_ref
501+
user_doc_ref.get = mock.AsyncMock(return_value=user_doc)
502+
503+
app_doc_in_root = mock.MagicMock()
504+
sessions_coll.document.return_value = app_doc_in_root
505+
users_coll = mock.MagicMock()
506+
app_doc_in_root.collection.return_value = users_coll
507+
user_doc_in_users = mock.MagicMock()
508+
users_coll.document.return_value = user_doc_in_users
509+
sessions_subcoll = mock.MagicMock()
510+
user_doc_in_users.collection.return_value = sessions_subcoll
511+
sessions_query = mock.MagicMock()
512+
sessions_subcoll.where.return_value = sessions_query
513+
sessions_query.get = mock.AsyncMock(return_value=[session_doc])
514+
515+
response = await service.list_sessions(app_name=app_name, user_id=user_id)
516+
517+
assert len(response.sessions) == 1
518+
assert response.sessions[0].last_update_time == update_time.timestamp()
445519

446520

447521
@pytest.mark.asyncio
@@ -455,6 +529,7 @@ async def test_list_sessions_without_user_id(mock_firestore_client):
455529
"appName": app_name,
456530
"userId": "user1",
457531
"state": {"session_key": "session_val"},
532+
"updateTime": 1234567890.0,
458533
}
459534

460535
mock_firestore_client.collection_group.return_value.where.return_value.get = (
@@ -503,6 +578,7 @@ async def mock_get_all(refs):
503578
assert session.id == "session1"
504579
assert session.state["app:app_key"] == "app_val"
505580
assert session.state["user:user_key"] == "user_val"
581+
assert session.last_update_time == 1234567890.0
506582

507583
mock_firestore_client.collection_group.assert_called_once_with("sessions")
508584
mock_firestore_client.collection_group.return_value.where.assert_called_once_with(

0 commit comments

Comments
 (0)