fix(session): merge a save into the stored session instead of overwriting it - #189
Merged
tina4stack merged 3 commits intoOct 1, 2026
Conversation
…ting it A save wrote back the whole session as this request loaded it. When requests on one session overlapped, the one that finished later put its stale copy back over the other's changes, and re-created the session if the other request had destroyed it. regenerate() moved the same stale copy to the new id. A save now re-reads the stored session and applies only the keys this request set, changed or removed since it loaded; after clear() it replaces the session. If the stored session is gone, it stays gone: nothing is written and no cookie is sent. If the store cannot be read, nothing is written and the change is kept for a retry. regenerate() re-reads the same way before moving the session, and mints no id for a session that has ended. Signed-off-by: Michael <[email protected]>
This was referenced Oct 1, 2026
Contributor
Author
|
I have read the Tina4 Contributor Licence Agreement and I agree to it. |
…session merge The merge on save adds complexity to this one file. Only its entry in the baseline changes. Signed-off-by: Michael <[email protected]>
…rrent-request tests save() now re-writes a stored session even when this request changed nothing, re-stamping the backend deadline to now + TINA4_SESSION_TTL so a session expires after inactivity, not a fixed span after its last change. The re-write is the same re-read, merged record the concurrent-save contract computes, so sliding stays concurrent-safe and never re-creates an ended record. All existing guards keep their precedence: no session id writes nothing; an ended (gone) record is forgotten, not re-created; a failed backend read keeps the dirty flag for a retry; a cleared session replaces rather than merges; TINA4_SESSION_TTL=0 still means never-expires. PHP is the reference (ADR-0087). tests/test_session_concurrent_requests.py: the two monkeypatch tests are replaced with real-dependency tests (no mock/monkeypatch anywhere in the file): - a read-only request now MOVES THE EXPIRY FORWARD - a real FileSessionHandler on a real tmp dir, the stored _expires read off disk before/after, asserting it advanced while the stored data is unchanged. - the unreachable-store test points a REAL RedisSessionHandler at a closed TCP port (bind to get a free port, close it) so the backend genuinely refuses; save() returns False and the dirty change is retained for a later retry. Signed-off-by: Andre van Zuydam <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]> Co-Authored-By: Tina4 <[email protected]>
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
3 participants
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.
A session save wrote back the whole session as the request loaded it. When requests on one session overlap, the one that finishes later puts its stale copy back over the other's changes, and re-creates the session if the other request destroyed it.
regenerate()moved the same stale copy to the new id.Change. A save re-reads the stored session and applies only the keys this request set, changed or removed since it loaded; after
clear()it replaces the session. If the stored session is gone, it stays gone: nothing is written and no cookie is sent. If the store cannot be read, nothing is written and the change is kept for a retry.regenerate()re-reads the same way before moving the session, and returnsNoneinstead of minting an id for a session that has ended.Test.
tests/test_session_concurrent_requests.py: two sessions on one store stand in for overlapping requests (27 cases, including the documented login flow and an SSO callback). 16 fail onv3without the change. Each part of the change, reverted on its own, turns a case red. The full suite shows no new failures againstv3on the same machine. Also run end to end over HTTP on the built-in server, with the file and the database (SQLite) session backends.Parity. The same change in all four frameworks: #189, tina4stack/tina4-php#260, tina4stack/tina4-ruby#94, tina4stack/tina4-nodejs#108.