Skip to content
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

Revise CookieLocaleResolver to use ResponseCookie #28779

Closed

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Jul 8, 2022

This PR provides two commits that improve CookieLocaleResolver, as a follow-up on #27609:

  1. Simplify creation of CookieLocaleResolver with custom cookie name

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.

  1. Refactor CookieLocaleResolver

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.

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.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 8, 2022
@rstoyanchev rstoyanchev self-assigned this Sep 28, 2022
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 28, 2022
@rstoyanchev rstoyanchev added this to the 6.0.0-RC1 milestone Sep 28, 2022
@rstoyanchev rstoyanchev changed the title Improve CookieLocaleResolver Revise CookieLocaleResolver to use ResponseCookie Sep 28, 2022
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
@rstoyanchev
Copy link
Contributor

I've also added some improvements in ResponseCookie to make it more friendly for use in CookieLocalResolver so that it doesn't have to replicate all of its fields.

@vpavic vpavic deleted the improve-cookie-locale-resolver branch September 28, 2022 15:44
vpavic referenced this pull request Sep 28, 2022
This commit deprecates CookieGenerator in favor of the more modern
alternative, ResponseCookie.

See gh-28870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants