Support Accept-Language header in MockHttpServletRequest's getLocale() and getLocales() methods [SPR-11701]

[spring-projects-issues]

Rob Winch opened SPR-11701 and commented

Overview

Currently the MockHttpServletRequest defaults to Locale.ENGLISH. However, the ServletRequest javadoc states:

If the client request doesn't provide an Accept-Language header, this method returns the default locale for the server.

We should probably also have some sort of way to ensure that if the Accept-Language header is specified that it is synched with the Locale (similar to Content-Type).

---

Deliverables

1. Update class-level Javadoc for MockHttpServletRequest with information regarding the default locale for the mocked server.
2. (?) Ensure that MockHttpServletRequest.getLocale() complies with the Javadoc from the Servlet specification with regard to the Accept-Language header.
3. (?) Ensure that MockHttpServletRequest.getLocales() complies with the Javadoc from the Servlet specification with regard to the Accept-Language header.

---

Affects: 4.0.3

Reference URL: https://github.com/spring-projects/spring-test-mvc-htmlunit/issues/21

Issue Links:

• Document that MockHttpServletRequest does not comply with Servlet spec regarding Accept-Language header [SPR-12043] #16659 Document that MockHttpServletRequest does not comply with Servlet spec regarding Accept-Language header

Referenced from: commits 9bf73f3

[spring-projects-issues]

Juergen Hoeller commented

Well, that mock server of ours simply always runs in the US locale :-)

Seriously, for mocking purposes, it may make sense to keep defining a fixed default locale for consistent test results... I'm not sure we should change that.

Juergen

[spring-projects-issues]

Rob Winch commented

I understand the concern. However, not following the Javadoc seems to cause issues with other components that do use Locale.getDefault() as demonstrated by the Reference URL.

[spring-projects-issues]

Juergen Hoeller commented

Well, the javadoc doesn't indicate the JVM's default Locale, it suggests the server's default Locale.That doesn't necessarily mean equivalence to the JVM's Locale.getDefault(). There may be custom configuration in the server that sets that default Locale for requests.

If a test happens to rely on Locale parsing there, it should rather make sure that it compares to the actual ServetRequest.getLocale() result, passing that through to any parsing code if necessary, or to set the ServletRequest's preferred locales specifically. From my perspective, it's a flawed assumption to rely on equivalence to Locale.getDefault() there.

The main problem on our side is that changing MockHttpServletRequest's getLocale() default may break a ton of user tests out there which do rely on the fixed nature for consistent test results between development machines and CI servers etc.

Juergen

[spring-projects-issues]

Sam Brannen commented

In general, I agree with Juergen:

1. The default locale for the test server is Locale.ENGLISH (not US) and should remain that way for backwards compatibility.
   • changing that would break existing test suites.
2. If you want to change the default locale in the mock for a specific testing scenario, you can do that via addPreferredLocale() or setPreferredLocales().

On the other hand, you have a valid point regarding the Accept-Language header: MockHttpServletRequest.getLocale() currently ignores that completely.

So if we make any changes related to this JIRA issue, I think it should be limited to adding support for the Accept-Language header in MockHttpServletRequest.getLocale().

Thoughts?

Sam

[spring-projects-issues]

Rob Winch commented

I think the proposal sounds good. It might be nice to supplement with a bit of Javadoc on what the default Locale is.

PS: Thanks for pointing out the typo on the default Locale.

[spring-projects-issues]

Sam Brannen commented

I think the proposal sounds good.

OK. I'll slate it for 4.1 then.

It might be nice to supplement with a bit of Javadoc on what the default Locale is.

It's actually already documented in the primary constructor, and all other constructors reference that with an @see tag, but... I agree that we could make it a little more obvious. ;)

Regards,

Sam

[spring-projects-issues]

Sam Brannen commented

Updated documentation as described in the comments for GitHub commit 9bf73f3:

Document default locale in MockHttpServletRequest

This commit updates the class-level Javadoc for MockHttpServletRequest
with information regarding the default locale for the mocked server.

[spring-projects-issues]

Sam Brannen commented

Hi guys,

After further research into this issue, I determined that providing real support for the Accept-Language header is not as trivial as I first assumed. For example, Tomcat, Jetty, and Undertow each have rather involved custom implementations for determining which languages are accepted with proper sorting based on the q=... quality values, etc.

Thus, for the sake of our mocks, I'm not convinced that it makes sense to fully implement this quality value parsing and sorting algorithm, especially since one can easily invoke the addPreferredLocale() or setPreferredLocales() methods on MockHttpServletRequest to achieve similar behavior.

Consequently, I created #16659 to document the fact that Spring's MockHttpServletRequest explicitly does not take the Accept-Language header into consideration in its implementations of getLocale() and getLocales().

I am therefore assigning this issue to the General Backlog and leaving it open in case it is later decided that it is in fact worth the effort implementing the quality value parsing and sorting algorithm in the mock request.

Regards,

Sam