Repository navigation
Conversation
…eycloak#52038) JpaChangesPerformer replayed update tasks against a synthetic notes map that only forwarded get/put (client sessions) or get/put/remove/clear (user sessions) to the persisted model, so AuthenticatedClientSessionModel#removeNote and restartClientSession() never reached the database. Both merges now run the tasks against a snapshot of the persisted notes and write the result back once, which makes every Map operation behave correctly. UserSessionAdapter#setNote(name, null) removes the note from the task's entity instead of registering a nested update task. Fixes keycloak#52038 Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds regression coverage and fixes for persisting removal/clearing of session notes (client + user sessions) when persistent sessions are enabled, ensuring update tasks applied by JpaChangesPerformer correctly reach the database.
Changes:
- Add an integration test suite to verify note removals and restart-driven note clearing are persisted for both client and user sessions.
- Change
JpaChangesPerformerto apply update tasks to a mutable snapshot of persisted notes and write back once, enabling correct behavior for allMapoperations (remove/clear/containsKey/iteration/etc.). - Update
UserSessionAdapter#setNote(name, null)to remove the note directly from the update entity rather than registering nested tasks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/session/SessionNotePersistenceTest.java | Adds regression tests validating that note removals/clears are persisted to DB (bypassing cache). |
| model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/JpaChangesPerformer.java | Applies updates to a notes snapshot and writes back once to ensure correct persistence for all Map operations. |
| model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/UserSessionAdapter.java | Adjusts null note handling to directly remove from the entity notes map during updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
Contributor
Author
|
LGTM |
ahus1
approved these changes
Aug 26, 2026
This was referenced Aug 26, 2026
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.
With persistent user sessions, JpaChangesPerformer replayed the recorded update tasks against a synthetic entity whose getNotes() returned a fresh anonymous HashMap forwarding only get/put (client sessions) or get/put/remove/clear (user sessions) to the persisted model. Every other Map operation ran against the throwaway map, so AuthenticatedClientSessionModel#removeNote and restartClientSession() (which clears all notes) never reached the database.
The synthetic entities now expose the persisted model's notes map directly, so all Map operations of the update tasks are applied to the persisted state.
PersistentAuthenticatedClientSessionAdapter#getNotes() lazily initializes the map instead of returning an immutable empty map, so that the tasks can mutate it.
UserSessionAdapter#setNote(name, null) removes the note from the task's entity instead of registering a nested update task from within the task, which would modify the task list while it is being replayed.
Fixes #52038