fix: close_reading_session_v2 takes timestamp, sets closed_at_ms

Review finding #3: close now accepts timestamp_ms and sets closed_at_ms
automatically, removing the need for a separate set_session_closed_at_ms call.
set_session_closed_at_ms retained for override use cases.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
wangdl 2026-06-11 21:33:42 +08:00
parent d21c93db1e
commit 5d6bf41cab
4 changed files with 36 additions and 30 deletions

View File

@ -283,7 +283,7 @@ mod tests {
}
fn teardown_session(id: &str) {
let _ = session_v2::close_reading_session_v2(id);
let _ = session_v2::close_reading_session_v2(id, 0);
let _ = session_v2::remove_session_v2(id);
}
@ -398,7 +398,7 @@ mod tests {
fn test_closed_session_rejects() {
let _guard = lock_test();
let sid = setup_session("mat_reject");
session_v2::close_reading_session_v2(&sid).unwrap();
session_v2::close_reading_session_v2(&sid, 3000).unwrap();
assert!(push_heartbeat_v2(&sid, "mat_reject", 15, None, 3000).is_err());
teardown_session(&sid);
}

View File

@ -106,13 +106,14 @@ pub fn resume_reading_session_v2(session_id: &str) -> Result<(), SessionError> {
Ok(())
}
/// Close the session. No more events allowed.
pub fn close_reading_session_v2(session_id: &str) -> Result<(), SessionError> {
/// Close the session and record its closed_at_ms. No more events allowed.
pub fn close_reading_session_v2(session_id: &str, timestamp_ms: i64) -> Result<(), SessionError> {
with_session_mut(session_id, |s| {
if s.status == ReadingSessionStatus::Closed {
return Err(SessionError::AlreadyClosed);
}
s.status = ReadingSessionStatus::Closed;
s.closed_at_ms = Some(timestamp_ms);
Ok(())
})
}
@ -311,7 +312,7 @@ mod tests {
}
fn teardown(id: &str) {
let _ = close_reading_session_v2(id);
let _ = close_reading_session_v2(id, 0);
let _ = remove_session_v2(id);
}
@ -333,7 +334,7 @@ mod tests {
assert_eq!(get_session_v2(&id).unwrap().status, ReadingSessionStatus::Paused);
resume_reading_session_v2(&id).unwrap();
assert_eq!(get_session_v2(&id).unwrap().status, ReadingSessionStatus::Active);
close_reading_session_v2(&id).unwrap();
close_reading_session_v2(&id, 0).unwrap();
assert_eq!(get_session_v2(&id).unwrap().status, ReadingSessionStatus::Closed);
remove_session_v2(&id).unwrap();
}
@ -358,9 +359,9 @@ mod tests {
#[test]
fn test_closed_rejects_events() {
let id = start_reading_session_v2(test_material(), 1000).unwrap();
close_reading_session_v2(&id).unwrap();
close_reading_session_v2(&id, 0).unwrap();
assert!(record_session_event_v2(&id, 2000, 10, None).is_err());
assert!(close_reading_session_v2(&id).is_err());
assert!(close_reading_session_v2(&id, 0).is_err());
remove_session_v2(&id).unwrap();
}
@ -379,7 +380,7 @@ mod tests {
#[test]
fn test_not_found() {
assert!(get_session_v2("nonexistent").is_err());
assert!(close_reading_session_v2("nonexistent").is_err());
assert!(close_reading_session_v2("nonexistent", 0).is_err());
}
#[test]
@ -506,22 +507,31 @@ mod tests {
#[test]
fn test_closed_rejects_interrupted() {
let id = start_reading_session_v2(test_material(), 1000).unwrap();
close_reading_session_v2(&id).unwrap();
close_reading_session_v2(&id, 0).unwrap();
assert!(mark_session_interrupted_v2(&id).is_err());
assert!(mark_session_failed_v2(&id).is_err());
remove_session_v2(&id).unwrap();
}
#[test]
fn test_set_closed_at_ms() {
fn test_close_sets_closed_at_ms() {
let id = start_reading_session_v2(test_material(), 1000).unwrap();
assert!(get_session_v2(&id).unwrap().closed_at_ms.is_none());
close_reading_session_v2(&id).unwrap();
set_session_closed_at_ms(&id, 5000).unwrap();
close_reading_session_v2(&id, 5000).unwrap();
assert_eq!(get_session_v2(&id).unwrap().closed_at_ms, Some(5000));
remove_session_v2(&id).unwrap();
}
#[test]
fn test_set_closed_at_ms_override() {
let id = start_reading_session_v2(test_material(), 1000).unwrap();
close_reading_session_v2(&id, 5000).unwrap();
// set_session_closed_at_ms can override if needed
set_session_closed_at_ms(&id, 9999).unwrap();
assert_eq!(get_session_v2(&id).unwrap().closed_at_ms, Some(9999));
remove_session_v2(&id).unwrap();
}
#[test]
fn test_get_active_session_by_material() {
let mat_a = ReadingMaterialRefV2::new("mat_a", "/tmp/a.md", "a.md", "markdown");
@ -535,7 +545,7 @@ mod tests {
assert_eq!(found.unwrap().client_session_id, id_a);
// Close session a, should not be found as active
close_reading_session_v2(&id_a).unwrap();
close_reading_session_v2(&id_a, 0).unwrap();
assert!(get_active_session_v2("mat_a").is_none());
teardown(&id_a);

View File

@ -41,8 +41,8 @@ fn resume_reading_session_v2(session_id: String) -> Result<(), String> {
}
#[uniffi::export]
fn close_reading_session_v2(session_id: String) -> Result<(), String> {
zx_document_core::session_v2::close_reading_session_v2(&session_id).map_err(|e| e.to_string())
fn close_reading_session_v2(session_id: String, timestamp_ms: i64) -> Result<(), String> {
zx_document_core::session_v2::close_reading_session_v2(&session_id, timestamp_ms).map_err(|e| e.to_string())
}
#[uniffi::export]
@ -719,7 +719,7 @@ mod ffi_tests {
assert_eq!(e4.event_type, ReadingEventTypeV2::MarkedAsRead);
push_material_closed_v2(sid.clone(), "mat_ffi_test".to_string(), 0, 12000).unwrap();
close_reading_session_v2(sid.clone()).unwrap();
close_reading_session_v2(sid.clone(), 0).unwrap();
let exported = export_pending_events_v2(100, 13000);
assert!(exported.len() >= 4, "expected >=4, got {}", exported.len());
@ -745,7 +745,7 @@ mod ffi_tests {
let sid = start_reading_session_v2(mat, 0).unwrap();
pause_reading_session_v2(sid.clone()).unwrap();
resume_reading_session_v2(sid.clone()).unwrap();
close_reading_session_v2(sid.clone()).unwrap();
close_reading_session_v2(sid.clone(), 0).unwrap();
let _ = zx_document_core::session_v2::remove_session_v2(&sid);
}
@ -770,7 +770,7 @@ mod ffi_tests {
assert!(retry.iter().any(|ev| ev.event_id == e.event_id));
ack_events_v2(vec![e.event_id.clone()]);
close_reading_session_v2(sid.clone()).unwrap();
close_reading_session_v2(sid.clone(), 0).unwrap();
let _ = zx_document_core::session_v2::remove_session_v2(&sid);
}
@ -871,6 +871,6 @@ mod ffi_tests {
#[test]
fn test_session_not_found() {
assert!(close_reading_session_v2("nonexistent".to_string()).is_err());
assert!(close_reading_session_v2("nonexistent".to_string(), 0).is_err());
}
}

View File

@ -34,8 +34,7 @@ App 启动
退出阅读页
→ push_material_closed_v2(sessionId, materialId, residualDelta, timestampMs)
→ close_reading_session_v2(sessionId)
→ set_session_closed_at_ms_v2(sessionId, timestampMs)
→ close_reading_session_v2(sessionId, timestampMs)
→ export_pending_events_v2(limit, timestampMs)
→ 每条 event 转为 ReadingEventUploadItem → 写入本地队列 → ack_events_v2(eventIds)
```
@ -118,10 +117,8 @@ class V2ReadingRuntimeAdapter: ReadingRuntimeAdapter {
let ts = Int64(timestamp.timeIntervalSince1970 * 1000)
// 1. Flush 事件
try await flushPendingEvents(timestamp: timestamp)
// 2. Close session
try close_reading_session_v2(sessionId: sessionId)
// 3. Set closed time
try set_session_closed_at_ms_v2(sessionId: sessionId, timestampMs: ts)
// 2. Close session (sets closed_at_ms automatically)
try close_reading_session_v2(sessionId: sessionId, timestampMs: ts)
}
// ── Export + Queue + Ack ──
@ -198,8 +195,10 @@ func safeCloseSession(sessionId: String?) {
os_log("Flush failed: %@", error.localizedDescription)
}
let ts = Int64(Date().timeIntervalSince1970 * 1000)
do {
try close_reading_session_v2(sessionId: sid)
try close_reading_session_v2(sessionId: sid, timestampMs: ts)
} catch let error as SessionError {
if error == .alreadyClosed {
os_log("Session already closed, ignoring")
@ -207,9 +206,6 @@ func safeCloseSession(sessionId: String?) {
}
os_log("Close failed: %@", error.localizedDescription)
}
let ts = Int64(Date().timeIntervalSince1970 * 1000)
try? set_session_closed_at_ms_v2(sessionId: sid, timestampMs: ts)
}
```