Skip to content

Use cached requestedSession info if available #1578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

oxc
Copy link

@oxc oxc commented Feb 12, 2020

Prevent fetching the session multiple times when checking whether the
sessionId has to be sent to the client, by comparing values before
clearing the requestedSessionCache.

Resolves: #1424

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 12, 2020
Prevent fetching the session multiple times when checking whether the
sessionId has to be sent to the client, by comparing values before
clearing the requestedSessionCache.

Resolves: spring-projects#1424
@oxc oxc force-pushed the use_cached_requested_session branch from b4dbf2d to 66c3984 Compare February 12, 2020 12:50
@oxc
Copy link
Author

oxc commented Feb 12, 2020

Apologies for the failed tests. Why is the findById invocation after session commit expected, or rather, what purpose does it serve? Am I missing something here?
Is it legitimate to reduce the number of expected invocations, since this is exactly what this pull request is trying to fix.

@oxc oxc requested a review from vpavic February 14, 2020 14:09
clearRequestedSessionCache();
SessionRepositoryFilter.this.sessionRepository.save(session);
String sessionId = session.getId();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the sessionId allowed to change on save? In that case, the comparison should still be done after save, but requestedSessionId has to be retrieved before the clearRequestedSessionCache()

@quaff
Copy link
Contributor

quaff commented Jul 21, 2023

This PR should be superseded by #2194

@oxc oxc closed this Jul 21, 2023
@bianjp
Copy link

bianjp commented Jul 21, 2023

This PR should be superseded by #2194

But #2194 was not backported to 2.7.x

@oxc
Copy link
Author

oxc commented Jul 21, 2023

This has been open for over 3 years. Feel free to reopen if anybody really thinks this is going to get merged somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SessionRepositoryFilter's getRequestedSession is called unnecessarily.
4 participants