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.
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
This commit is contained in:
Vedran Pavic
2022-07-06 23:50:20 +02:00
committed by rstoyanchev
parent f6d1806e30
commit 7ea49fa9f4
4 changed files with 202 additions and 107 deletions

View File

@@ -16,6 +16,7 @@
package org.springframework.web.servlet.i18n;
import java.time.Duration;
import java.util.Locale;
import java.util.TimeZone;
@@ -27,6 +28,8 @@ import org.springframework.context.i18n.LocaleContext;
import org.springframework.context.i18n.SimpleLocaleContext;
import org.springframework.context.i18n.SimpleTimeZoneAwareLocaleContext;
import org.springframework.context.i18n.TimeZoneAwareLocaleContext;
import org.springframework.http.HttpHeaders;
import org.springframework.web.testfixture.servlet.MockCookie;
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
import org.springframework.web.util.WebUtils;
@@ -57,7 +60,7 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "nl");
request.setCookies(cookie);
resolver.setCookieName("LanguageKoekje");
resolver = new CookieLocaleResolver("LanguageKoekje");
Locale loc = resolver.resolveLocale(request);
assertThat(loc.getLanguage()).isEqualTo("nl");
}
@@ -67,7 +70,7 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "nl");
request.setCookies(cookie);
resolver.setCookieName("LanguageKoekje");
resolver = new CookieLocaleResolver("LanguageKoekje");
LocaleContext loc = resolver.resolveLocaleContext(request);
assertThat(loc.getLocale().getLanguage()).isEqualTo("nl");
assertThat(loc).isInstanceOf(TimeZoneAwareLocaleContext.class);
@@ -79,7 +82,7 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "nl GMT+1");
request.setCookies(cookie);
resolver.setCookieName("LanguageKoekje");
resolver = new CookieLocaleResolver("LanguageKoekje");
LocaleContext loc = resolver.resolveLocaleContext(request);
assertThat(loc.getLocale().getLanguage()).isEqualTo("nl");
assertThat(loc).isInstanceOf(TimeZoneAwareLocaleContext.class);
@@ -91,7 +94,7 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "++ GMT+1");
request.setCookies(cookie);
resolver.setCookieName("LanguageKoekje");
resolver = new CookieLocaleResolver("LanguageKoekje");
assertThatIllegalStateException().isThrownBy(() -> resolver.resolveLocaleContext(request))
.withMessageContaining("LanguageKoekje")
.withMessageContaining("++ GMT+1");
@@ -104,8 +107,8 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "++ GMT+1");
request.setCookies(cookie);
resolver = new CookieLocaleResolver("LanguageKoekje");
resolver.setDefaultTimeZone(TimeZone.getTimeZone("GMT+2"));
resolver.setCookieName("LanguageKoekje");
LocaleContext loc = resolver.resolveLocaleContext(request);
assertThat(loc.getLocale()).isEqualTo(Locale.GERMAN);
assertThat(loc).isInstanceOf(TimeZoneAwareLocaleContext.class);
@@ -117,7 +120,7 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "nl X-MT");
request.setCookies(cookie);
resolver.setCookieName("LanguageKoekje");
resolver = new CookieLocaleResolver("LanguageKoekje");
assertThatIllegalStateException().isThrownBy(() -> resolver.resolveLocaleContext(request))
.withMessageContaining("LanguageKoekje")
.withMessageContaining("nl X-MT");
@@ -129,8 +132,8 @@ class CookieLocaleResolverTests {
Cookie cookie = new Cookie("LanguageKoekje", "nl X-MT");
request.setCookies(cookie);
resolver = new CookieLocaleResolver("LanguageKoekje");
resolver.setDefaultTimeZone(TimeZone.getTimeZone("GMT+2"));
resolver.setCookieName("LanguageKoekje");
LocaleContext loc = resolver.resolveLocaleContext(request);
assertThat(loc.getLocale().getLanguage()).isEqualTo("nl");
assertThat(loc).isInstanceOf(TimeZoneAwareLocaleContext.class);
@@ -141,12 +144,13 @@ class CookieLocaleResolverTests {
void setAndResolveLocale() {
resolver.setLocale(request, response, new Locale("nl", ""));
Cookie cookie = response.getCookie(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
MockCookie cookie = MockCookie.parse(response.getHeader(HttpHeaders.SET_COOKIE));
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
assertThat(cookie.getDomain()).isNull();
assertThat(cookie.getPath()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_PATH);
assertThat(cookie.getSecure()).isFalse();
assertThat(cookie.getSameSite()).isEqualTo("Lax");
request = new MockHttpServletRequest();
request.setCookies(cookie);
@@ -208,12 +212,13 @@ class CookieLocaleResolverTests {
void setAndResolveLocaleWithCountry() {
resolver.setLocale(request, response, new Locale("de", "AT"));
Cookie cookie = response.getCookie(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
MockCookie cookie = MockCookie.parse(response.getHeader(HttpHeaders.SET_COOKIE));
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
assertThat(cookie.getDomain()).isNull();
assertThat(cookie.getPath()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_PATH);
assertThat(cookie.getSecure()).isFalse();
assertThat(cookie.getSameSite()).isEqualTo("Lax");
assertThat(cookie.getValue()).isEqualTo("de-AT");
request = new MockHttpServletRequest();
@@ -230,12 +235,13 @@ class CookieLocaleResolverTests {
resolver.setLanguageTagCompliant(false);
resolver.setLocale(request, response, new Locale("de", "AT"));
Cookie cookie = response.getCookie(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
MockCookie cookie = MockCookie.parse(response.getHeader(HttpHeaders.SET_COOKIE));
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_NAME);
assertThat(cookie.getDomain()).isNull();
assertThat(cookie.getPath()).isEqualTo(CookieLocaleResolver.DEFAULT_COOKIE_PATH);
assertThat(cookie.getSecure()).isFalse();
assertThat(cookie.getSameSite()).isEqualTo("Lax");
assertThat(cookie.getValue()).isEqualTo("de_AT");
request = new MockHttpServletRequest();
@@ -249,26 +255,27 @@ class CookieLocaleResolverTests {
@Test
void customCookie() {
resolver.setCookieName("LanguageKoek");
resolver = new CookieLocaleResolver("LanguageKoek");
resolver.setCookieDomain(".springframework.org");
resolver.setCookiePath("/mypath");
resolver.setCookieMaxAge(10000);
resolver.setCookieMaxAge(Duration.ofSeconds(10000));
resolver.setCookieSecure(true);
resolver.setCookieSameSite("Lax");
resolver.setLocale(request, response, new Locale("nl", ""));
Cookie cookie = response.getCookie("LanguageKoek");
MockCookie cookie = MockCookie.parse(response.getHeader(HttpHeaders.SET_COOKIE));
assertThat(cookie).isNotNull();
assertThat(cookie.getName()).isEqualTo("LanguageKoek");
assertThat(cookie.getDomain()).isEqualTo(".springframework.org");
assertThat(cookie.getPath()).isEqualTo("/mypath");
assertThat(cookie.getMaxAge()).isEqualTo(10000);
assertThat(cookie.getSecure()).isTrue();
assertThat(cookie.getSameSite()).isEqualTo("Lax");
request = new MockHttpServletRequest();
request.setCookies(cookie);
resolver = new CookieLocaleResolver();
resolver.setCookieName("LanguageKoek");
resolver = new CookieLocaleResolver("LanguageKoek");
Locale loc = resolver.resolveLocale(request);
assertThat(loc.getLanguage()).isEqualTo("nl");
}

View File

@@ -99,6 +99,8 @@ class LocaleResolverTests {
if (localeContextResolver instanceof AbstractLocaleContextResolver) {
((AbstractLocaleContextResolver) localeContextResolver).setDefaultTimeZone(TimeZone.getTimeZone("GMT+1"));
request.removeAttribute(CookieLocaleResolver.LOCALE_REQUEST_ATTRIBUTE_NAME);
localeContextResolver.resolveLocaleContext(request);
assertThat(TimeZone.getTimeZone("GMT+1")).isEqualTo(((TimeZoneAwareLocaleContext) localeContext).getTimeZone());
}
@@ -134,6 +136,8 @@ class LocaleResolverTests {
if (localeContextResolver instanceof AbstractLocaleContextResolver) {
((AbstractLocaleContextResolver) localeContextResolver).setDefaultLocale(Locale.GERMANY);
request.removeAttribute(CookieLocaleResolver.LOCALE_REQUEST_ATTRIBUTE_NAME);
localeContextResolver.resolveLocaleContext(request);
assertThat(localeContext.getLocale()).isEqualTo(Locale.GERMANY);
}
}