Refactor DefaultWebSessionManager #1507
Closed
Conversation
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
- Use constant for default header name - make getHeaderName private Issue: SPR-15917
Switch to unit tests rather than testing with DefaultWebSessionManager Issue: SPR-15917
Previously session.save() was invoked, but we did not ensure it was completed. This commit blocks on session.save()
This test is actually broken and is testing a new session is created because the session id returned by the idResolver does not match the existing WebSession. This commit ensures that the id of the WebSession found by idResolver matches the existing WebSession.
To ensure we test with independence from InMemoryWebSessionStore we use Mockito for the DefaultWebessionManager collaborators.
We want to ensure that when the response is completed, it saves the WebSession and writes it to the response using idResolver
This allows updating the WebSession lastAccessTime without exposing a method on WebSession in an implementation independent way.
DefaultWebSessionManager no longer requires the WebSessionStore to use DefaultWebSession.
This method is not necessary since the WebSession that is returned allows saving the WebSession. Additionally, it is error prone since the wrong type might be passed into it.
This seems to be implementation specific to change the state of DefaultWebSession. Instead handle it copy constructor
Removing ExchangeWebSession based on Feedback from Rossen. There is no need to invoke setSessionId immediately when WebSession.save is invoked because nothing can be communicated to the client until the response is committed.
Fix constructor Javadoc based on feedback from Rossen.
rwinch
force-pushed
the
websessionmanager
branch
from
5eed19c to
f883b44
Compare
rstoyanchev
added a commit
that referenced
this pull request
Sep 6, 2017
This was referenced Jan 11, 2019
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.