Revise CookieLocaleResolver to use ResponseCookie#28779
Closed
vpavic wants to merge 2 commits into
Closed
Conversation
At present, creating CookieLocaleResolver with custom cookie name requires creating an instance using the default constructor and then using setter to configure the desired cookie name. This commit introduces CookieLocaleResolver constructor that accepts cookie name thus allowing for a simpler creation of an instance with the desired cookie name.
At present, CookieLocaleResolver extends CookieGenerator instead of AbstractLocale(Context)Resolver like other LocaleResolver implementations. This means it duplicates some common aspects of LocaleResolver hierarchy while also exposing some CookieGenerator operations, such as #addCookie and #removeCookie. Additionally, CookieGenerator's support for writing cookies is based on Servlet support which at current baseline doesn't support SameSite directive. This commit refactors CookieLocaleResolver to make it extend AbstractLocaleContextResolver and also replaces CookieGenerator's cookie writing support with newer and more capable ResponseCookie.
vpavic
commented
Jul 8, 2022
CookieLocaleResolverCookieLocaleResolver to use ResponseCookie
rstoyanchev
pushed a commit
that referenced
this pull request
Sep 28, 2022
At present, CookieLocaleResolver extends CookieGenerator instead of AbstractLocale(Context)Resolver like other LocaleResolver implementations. This means it duplicates some common aspects of LocaleResolver hierarchy while also exposing some CookieGenerator operations, such as #addCookie and #removeCookie. Additionally, CookieGenerator's support for writing cookies is based on Servlet support which at current baseline doesn't support SameSite directive. This commit refactors CookieLocaleResolver to make it extend AbstractLocaleContextResolver and also replaces CookieGenerator's cookie writing support with newer and more capable ResponseCookie. Simplify creation of CookieLocaleResolver with custom cookie name This commit introduces CookieLocaleResolver constructor that accepts cookie name thus allowing for a simpler creation of an instance with the desired cookie name. See gh-28779
rstoyanchev
added a commit
that referenced
this pull request
Sep 28, 2022
Contributor
|
I've also added some improvements in |
vpavic
referenced
this pull request
Sep 28, 2022
This commit deprecates CookieGenerator in favor of the more modern alternative, ResponseCookie. See gh-28870
|
It seems that introducing |
Member
|
@sabomichal, this issue was closed almost 2 years ago. If you believe you have encountered a bug or something worth discussing, please create a new issue. |
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.
This PR provides two commits that improve
CookieLocaleResolver, as a follow-up on #27609:CookieLocaleResolverwith custom cookie nameAt present, creating
CookieLocaleResolverwith custom cookie name requires creating an instance using the default constructor and then using setter to configure the desired cookie name.This commit introduces
CookieLocaleResolverconstructor that accepts cookie name thus allowing for a simpler creation of an instance with the desired cookie name.CookieLocaleResolverAt present,
CookieLocaleResolverextendsCookieGeneratorinstead ofAbstractLocale(Context)Resolverlike otherLocaleResolverimplementations. This means it duplicates some common aspects ofLocaleResolverhierarchy while also exposing someCookieGeneratoroperations, such as#addCookieand#removeCookie.Additionally,
CookieGenerator's support for writing cookies is based on Servlet support which at current baseline doesn't support SameSite directive.This commit refactors
CookieLocaleResolverto make it extendAbstractLocaleContextResolverand also replacesCookieGenerator's cookie writing support with newer and more capableResponseCookie.