break: uses uniform SessionNode abstraction - #78
Merged
Conversation
…enew Introduce lib/services/session/ with a unified SessionNode tree: - CpdailySessionNode (egate) as top-level, IdsSessionNode as its child (renews via parent cpdaily session), LeafSessionNode for token-only providers (gradescope/hydro). - CookieProvider: unified read-only cookie view for downstream apps. - SessionTree.withCookie: 401-renew-retry helper with single-flight + stale-epoch skipping so concurrent callers never storm /auth/renew. ThirdPartyAuthService becomes a facade over the tree, preserving its full public API (egateCookies/egateBinding/renewEgateBinding/etc.) so storage format, cloud-sync serialization, and backend routes are unchanged. Migrate schedule/assignment/oa_gym/home_page callers onto withCookie; drop hand-rolled 401-retry in each service. Add test/session_storm_test.dart covering single-flight, stale-epoch skip, concurrent withCookie renew sharing, and ids→parent delegation. LoggingHttpClient gains an optional inner client for test injection. flutter analyze: clean. flutter test: 25/25 pass.
- Collapse CpdailySessionNode/IdsSessionNode/LeafSessionNode into single
SessionNode class with RenewMode enum (cpdailySession|password|parentCookie)
- 5-node topology: cpdaily (top, /auth/renew) → eams + elearning children
(/auth/third-party/<id>, parent tgc); gradescope + hydro (top, password)
- withCookie two-level retry: initial minting → 401 renew-retry → parent
renew + child re-mint (only on credential 401, not server 500)
- markRenewed cascades to children (clears derived cookies)
- enum ThirdPartyPlatform.egate→cpdaily, fromId('egate') alias, apiPath
getter (cpdaily→'egate' backend route unchanged)
- Storage migrates legacy third_party_egate key → third_party_cpdaily
- Facade: egateNode→cpdailyNode, idsNode deleted, eamsNode/elearningNode/
gradescopeNode/hydroNode added; all egate* accessors renamed cpdaily*
- Callers: schedule→eamsNode, blackboard→elearningNode, gradescope/hydro→
dedicated nodes via withCookie; home_page idsNode→cpdailyNode
- DebugLogger.log() debugPrints [HTTP] method url status tag in debug mode
- Tests: session_storm rewrite (eams/elearning downstream + two-level retry
+ 500-no-escalation), all platform refs .egate→.cpdaily, fromId alias
Child node (eams/elearning) markRenewed → notifyListeners was propagating through tree → tpAuth → assignment _onDepsChanged → fetchAssignments, creating a feedback loop that re-minted downstream cookies on every fetch. Only top-level nodes (cpdaily/gradescope/hydro) now propagate to the tree — child notifications still fire for direct listeners (withCookie epoch checks).
Child node (eams/elearning) derived cookies are now persisted to secure storage so cold start skips the SSO bounce. Previously _derivedCookie was ephemeral — every app restart re-minted via /auth/third-party/<id>, adding 3-20s latency (elearning SSO is especially slow). - StorageService: save/load/clear derived cookies keyed by node id - SessionNode: persistDerived callback, setDerivedCookie boot hydration, _renewWithParentCookie persists after mint, onParentRenewed clears on parent tgc rotation - SessionTree: persistDerived param wired to eams/elearning, setDerivedCookie method for boot hydration - ThirdPartyAuthService: _persistDerivedCookie callback, initialize loads derived cookies after accounts, unbind/replaceAll/clearAll clear them when cpdaily binding is removed
pushIfDue has a 30s throttle — if a user unbinds cpdaily within 30s of
the last push (e.g. right after boot pull), the unbind is silently
skipped and the stale binding stays in the cloud blob. Next boot's pull
restores it, making the unbind appear to not have worked.
- SyncService.forcePush(): bypasses throttle, used by unbind/clearAll
- onBindingsChanged now accepts {bool force}; main.dart routes force→forcePush
- unbind/clearAll call _onTreeChanged(force: true) after clearing state
…mbstones
Replace blind last-write-wins pull (replaceAll) with a per-platform LWW
merge driven by (updatedAt, deviceId). Deletions now carry tombstones so
an unbind on device A is not resurrected when device B pulls an older
cloud copy — fixing the root cause of the forcePush workaround.
Schema:
- New SyncEnvelope {v, accounts, tombstones} with migration ladder
(v0 bare array → v1 → v2 current). Migrations are pure functions;
legacy blobs auto-upgrade on read and are written back on next push.
- Schema version is separate from crypto version (blob salt/nonce layout).
LWW merge:
- ThirdPartyAccount gains updatedAt + deviceId. boundAt stays semantic
(bind moment); updatedAt bumps on every mutation (bind/rebind/renew/
raw update). deviceId is a stable per-device id (16-byte CSPRNG → hex,
persisted in SharedPreferences, generated lazily at boot).
- Timestamp tie-break: larger deviceId wins; empty deviceId always loses
so legacy data yields to any real device write.
- Tombstones: {platform, deletedAt, deviceId}. Deletion wins iff
deletedAt > account.updatedAt (same instant → tombstone preferred).
Wiring:
- touchLocal() stamps accounts on every mutation path (bind, bindCpdailySms,
updateRaw, _persistAccount/renew).
- onUnbind hook records tombstones; clearAll records per-platform.
- applySyncMerge() applies merge results without re-triggering the push
hook (sync path writes the merged envelope itself).
- pull()/restoreWithMasterPassword() merge instead of overwrite; push the
merged envelope back if local had winners.
Tests: 17 new (8 envelope pure-logic + 2 multi-device LWW/tombstone
integration + 7 existing compatible via v0 auto-migration). 44 total pass.
Two redundant-notify cascades found by auditing all notifyListeners → refetch paths: 1. SessionNode.renew double-notify: persist()→setAccount fired notify #1, then markRenewed() fired notify #2 on the same node. Each cascaded through SessionTree → tpAuth → AssignmentService.fetchAssignments + SyncService.pushIfDue. Fix: markRenewed no longer notifies the node itself (persist→setAccount already did); child-renew paths (_renewWithParentCookie) now notify explicitly after markRenewed. 2. applySyncMerge N+1 notify: _suppressSyncPush only blocked the push hook, not notifyListeners(). Each setAccount in the merge loop fired _onTreeChanged → notifyListeners → AssignmentService refetch attempt. Fix: _suppressSyncPush now suppresses ALL downstream effects (notify + push hook); applySyncMerge notifies once at the end.
…ssion Switching semesters fired fetchAssignments (exam_table) concurrently with fetchCourseTable. Both hit courseTableForStd.action on EAMS's stateful Spring/Struts session; concurrent access returns a partially initialized page → 'Failed to extract numeric ids' on exam_table, then succeeds on retry (the first course_table request primed the session). On boot this didn't happen because main.dart awaits fetchAll() before fetchAssignments() — sequential, not concurrent. Fix: selectSemester sets _suppressAssignmentRefetch during its own network fetch. AssignmentService._onScheduleChanged checks the flag and skips the refetch. When selectSemester's fetch completes, it clears the flag and fires one final notify — AssignmentService refetches then, with the EAMS session already primed by fetchCourseTable. exam_table succeeds on the first try.
…uctor) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors third-party session handling to use a unified SessionTree/SessionNode abstraction (including child nodes for EAMS/eLearning), renames the legacy egate platform to cpdaily (keeping backend-route compatibility), and hardens cloud sync with a versioned envelope + last-writer-wins (LWW) merge including tombstones to prevent deleted bindings from being resurrected.
Changes:
- Introduce
SessionTree+SessionNodeand migrate Schedule/Assignments/OA Gym/WebView cookie usage towithCookiefor single-flight renew + retry. - Replace raw third-party sync payloads with
SyncEnvelope(schema v2) supporting tombstones and deterministic LWW merge; add tests to guard regressions. - Rename
ThirdPartyPlatform.egate→ThirdPartyPlatform.cpdaily, with storage migration + legacy ID aliasing.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/sync_service_test.dart | Updates sync tests and adds LWW/tombstone regression coverage; fixture now wires sync hooks like main.dart. |
| test/sync_envelope_test.dart | New tests for schema migration (v0/v1→v2) and envelope LWW merge semantics. |
| test/sync_crypto_test.dart | Updates legacy payload platform id and adds alias test for egate→cpdaily. |
| test/session_storm_test.dart | New tests for single-flight renew + epoch-based anti-storm behavior and two-level child→parent fallback. |
| test/oa_gym_service_test.dart | Updates OA Gym test fixtures to seed cpdaily binding. |
| test/assignment_service_test.dart | Updates fixtures and mocks for downstream (eams/elearning) cookie minting and request bodies. |
| lib/services/third_party_auth_service.dart | Replaces internal account map with SessionTree, adds deviceId stamping, derived cookie persistence, tombstone hooks, and sync-merge apply path. |
| lib/services/sync_service.dart | Implements v2 envelope serialization, LWW merge on pull/restore, tombstone tracking, and force-push for removals. |
| lib/services/sync_envelope.dart | New versioned sync plaintext envelope with migration ladder and per-platform LWW merge including tombstones. |
| lib/services/storage_service.dart | Adds stable device id generation, derived-cookie persistence APIs, and secure-storage migration from legacy third_party_egate. |
| lib/services/session/session_tree.dart | New unified session topology and withCookie helper implementing storm-safe renew+retry and child→parent escalation. |
| lib/services/session/session_node.dart | New node abstraction with single-flight renew, epoching, parent cascade invalidation, and multiple renew modes. |
| lib/services/session/cookie_provider.dart | New credential snapshot object used by session nodes and downstream call sites. |
| lib/services/schedule_service.dart | Migrates schedule calls to SessionTree.withCookie using EAMS derived cookies; adds assignment-refetch suppression during semester switch. |
| lib/services/oa_gym_service.dart | Migrates OA Gym requests to SessionTree.withCookie using CpDaily node; removes direct cookie plumbing. |
| lib/services/http_client.dart | Allows injecting an inner http.Client (enables deterministic HTTP tests). |
| lib/services/debug_logger.dart | Logs HTTP requests to console in debug mode and simplifies entry creation. |
| lib/services/assignment_service.dart | Switches deadline fetchers to use withCookie (EAMS/eLearning derived cookies; Gradescope/Hydro tokens) and reduces redundant refetches on schedule changes. |
| lib/pages/third_party_bind_page.dart | Renames UI state and SMS flow wiring from eGate to CpDaily. |
| lib/pages/third_party_accounts_page.dart | Updates platform icon mapping for cpdaily. |
| lib/pages/settings_page.dart | Updates binding tile UI from eGate to CpDaily. |
| lib/pages/schedule_page.dart | Updates gating to require hasCpdailyBinding. |
| lib/pages/oa_gym_page.dart | Updates gating and identity display to use CpDaily binding. |
| lib/pages/home_page.dart | Uses CpDaily node CookieProvider to inject cookies into WebView features. |
| lib/models/third_party_account.dart | Renames platform enum value, adds updatedAt/deviceId for LWW merge, and adds legacy id alias mapping. |
| lib/main.dart | Wires sync push + tombstone hooks and updates boot flow to use new CpDaily binding gate. |
Comments suppressed due to low confidence (2)
lib/services/oa_gym_service.dart:298
- This error message still refers to “eGate”, but the session source is now the CpDaily binding. The wording should be updated to avoid misleading users.
if (response == null) {
throw OaGymException('当前 eGate 登录态已失效，请重新绑定 eGate');
}
lib/services/assignment_service.dart:456
- Same issue as Gradescope: if Hydro binding has
autoRenewdisabled,expired: statusCode == 401makeswithCookieattempt a password renew, which returns false and causeswithCookieto return null (skipping the 401→unbind path). Only treat 401 as “expired” whenacc.autoRenewis enabled.
}),
tag: 'deadlines:hydro:$domain',
);
return CookieAction(r, expired: r.statusCode == 401);
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+250
to
252
| if (!_tpAuth.hasCpdailyBinding) { | ||
| throw OaGymException('场馆预约需要绑定 eGate 账号，请在「第三方账号」中绑定'); | ||
| } |
Comment on lines
+3
to
+9
| /// A read-only snapshot of cookies + identity a downstream consumer (campus | ||
| /// service, webview feature) injects into its requests. | ||
| /// | ||
| /// Implementations: [CpdailyCookieProvider] (CASTGC-bearing CpDaily session), | ||
| /// [IdsCookieProvider] (IDS SSO cookies minted from the CpDaily session), and | ||
| /// a no-op empty view for leaf nodes that expose no cookies (Gradescope/Hydro | ||
| /// work via bearer tokens server-side, not browser cookies). |
Comment on lines
+162
to
+175
| // First level: single-node renew-retry (handles initial minting + 401). | ||
| var result = await _renewRetry(node, action); | ||
| if (result != null) return result; | ||
|
|
||
| // Second level: for child nodes whose first-level retry returned null. | ||
| // ONLY escalate to parent renew if the child's renew failure was a | ||
| // credential error (HTTP 401 = parent tgc stale). A 500 from the | ||
| // downstream endpoint is a server error — re-minting the parent tgc | ||
| // won't fix it, so we skip the escalation and return null immediately. | ||
| // This prevents wasteful /auth/renew calls on transient backend errors. | ||
| if (node.parent == null) return null; | ||
| if (!node.lastRenewWasCredentialError) return null; | ||
| final parentOk = await node.parent!.renewIfNeeded(node.parent!.epoch); | ||
| if (!parentOk) return null; |
Comment on lines
+408
to
+411
| tag: 'deadlines:gradescope', | ||
| ); | ||
| return CookieAction(r, expired: r.statusCode == 401); | ||
| }, |
Comment on lines
+96
to
+99
| } catch (_) { | ||
| // Corrupt legacy entry — leave it; clearThirdPartyAccount can | ||
| // still remove it via the fromId alias path. | ||
| } |
Comment on lines
+393
to
+401
| /// Stamp [acc] with this device's id and the current wall clock, producing | ||
| /// a new [ThirdPartyAccount] that wins LWW against any older copy. Called | ||
| /// by [ThirdPartyAuthService] mutation paths via the public [touchLocal]. | ||
| ThirdPartyAccount touchLocal(ThirdPartyAccount acc) { | ||
| return acc.copyWith( | ||
| updatedAt: DateTime.now(), | ||
| deviceId: _deviceId, | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.