Skip to content

Content-Language not set for responses rendered with Message Converters [SPR-14802] #19368

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

Closed
spring-projects-issues opened this issue Oct 12, 2016 · 17 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Oct 12, 2016

Chris DaMour opened SPR-14802 and commented

When Spring MVC works with a controller result that ends up with rendering a view, the Dispatch servlet sets the locale on the response, which in turn sets the contentLanguage and thus the response Content-Language header. at this line

Locale locale = this.localeResolver.resolveLocale(request);

The same thing does not happen when returning something that is handled by a MessageConverter as there is now mv by the time it gets to

but even then the response has been flushed so it wouldn't work anyways.

Seems like this should be set somewhere during the processing of the http message converter.


Affects: 4.2.7

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Indeed nothing sets the locale on the response for response body methods. However unlike View rendering where the various HTML templating engines have built in support for locale-sensitive messages and templates, there is nothing similar when writing directly to the response body. So even if do set the content-language header of the response that may not match the actual response body unless the application ensures that.

Could you describe a little more what you're doing and how you're alternating language content? Note also that in the controller method the application can have the Locale injected and set through a ResponseEntity:

@GetMapping("/foo")
ResponseEntity<Foo> handle(Locale locale) {
    // ...
    return ResponseEntity.ok()
        .header(HttpHeaders.CONTENT_LANGUAGE, locale.toLanguageTag())
        .build(foo);
}

We could also add a contentLanguage(Locale) to the ResponseEntity HeadersBuilder which would be consistent with others there such as eTag and lastModified.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Scheduling for 5.0 RC1 where we can at least add a contentLanguage(Locale) method to the ResponseEntity builder. Sébastien Deleuze assigning this one to you since you have and had other Locale-related tickets for 5.0.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

I use messageconverters tied to a handlebars seralizer which has it's own locale resolving thing. I'm using message converters vs view renderers because they fit my pattern better (1 responsentity, multiple representations -> hal, amp, html) So i have something like this now:

@Override
    protected void writeInternal(HyperResource hyperResource, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException {
        //normally i'd check for outputMessage being null...but it'll throw in the
        //base class' write() way before it gets here so no need for the double check
        Locale locale = Optional.ofNullable(outputMessage.getHeaders().getFirst(HttpHeaders.CONTENT_LANGUAGE))
                                .map(Locale::forLanguageTag)
                                .orElse(((ServletServerHttpResponse) outputMessage).getServletResponse().getLocale());

        serializer.write(hyperResource, outputMessage.getBody(), locale);
    }

but that fallback is always empty

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

You could use a ResponseBodyAdvice which has access to the request and response (so you can use RequestContextUtils.getLocale(request) and set the response headers accordingly). It also applies only when a specific HttpMessageConverter is chosen so that seems like a straight-forward solution and targeted at a converter that does handle locales.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

that's interesting i'll take a look at that. ReqeustcontextUtils is a no go for me in general because i'm all Rx and coming back on different threads, but if ResponseBodyAdvice has the request i can just map it over. I like that more than the interceptor we went with

public class LocaleHandlerInterceptor extends HandlerInterceptorAdapter  {
    @Override
    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
        throws Exception {
        response.addHeader(HttpHeaders.CONTENT_LANGUAGE, request.getLocale().toLanguageTag());
        return true;
    }

} 

cause it's more MVC level. I still think it should just be set automatically though, that's a pretty important response header. thanks for the workaround!

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

ResponseBodyAdvice seems indeed a good fit for that use case, I don't think there is a strong need for contentLanguage(Locale) since the header is likely to be set globally.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

it's not set globally now though...

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

We're not setting it automatically, correct. The point is that ResponseBodyAdvice gives you a single place to set this vs an option on ResponseEntity.

A ResponesBodyAdvice is extremely easy to deploy, simply annotate with @ControllerAdvice and declare as a bean and off you go.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

why not set it automatically? that's what the ticket is requesting the contentLanguge(locale) thing got us off track..why close this as won't fix?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

It is in the comments above:

unlike View rendering where the various HTML templating engines have built in support for locale-sensitive messages and templates, there is nothing similar when writing directly to the response body ... Could you describe a little more what you're doing?

To which you replied:

I use messageconverters tied to a handlebars seralizer which has it's own locale resolving thing. I'm using message converters vs view renderers because they fit my pattern better (1 responsentity, multiple representations -> hal, amp, html)

Nothing wrong with that but certainly a very specific scenario. It doesn't justify setting the Content-Language broadly for all message converter scenarios. It could very well cause regressions.

ResponseBodyAdvice lets you do this for your scenario. Have you tried it? Is there a reason you can't use it?

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

yes i've used ResponseBodyAdvice.

but why shouldn't message converter based responses (like spring hateoas) NOT have content-language response headers? A HAL resource representation has a locale just as much as an HTML resource representation is what i'm saying. It's just as important for clients to know the locale when reading a response. I don't think that is such a special case.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

It seems to me a HAL resource is more close to a regular REST resource than an HTML page rendered to the end user, so I would say it should follow by default the same principle than regular REST resources.

If you think such header should be set with Spring Hateoas, I think the best place to raise and discuss that is https://github.com/spring-projects/spring-hateoas/issues. If Spring Hateoas team agree on your request, be sure we will discuss that with them to find the best outcome possible.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

HTML is the most RESTful resource of them all. what you're saying doesn't make any sense. HTML is the defacto REST representation of a resource..and yes RESTful resources should include hypermedia controls like response headers that help describe themselves to restful clients...like content-language. that's why it's great that the view renderers do it (since they are most commonly used to generate html representations of resources) but equally why it's so important that representations generated by message converters ALSO have these hypermedia controls in place. The way you are making your stance makes me think you don't see that HAL and HTML are just different representations of the same resource...which has a content-language no matter the representation/mime type.

I'm very confused at the reluctance here. It seems so very basic from my perspective...you return a resource...any resource with a string message or a currency has a locale that's pretty much ever resource i can imagine. I get not doing it for binary images...is that the concern? When would i want my service NOT to return content-language? Such a default makes a lot of the web opaque to clients...especially when the representation format (HAL and other JSON structures being the most prolific) don't have defined standards for describing locale such as HTML does with the lang attribute.

Fixing this in spring-hateoas isn't wide enough a solution. The problem spans any project implemented on top of spring web that utilizes message converters...spring data rest for example (not even sure if that's still the name of that project). I don't use spring hateoas so fixing it there wouldn't help me specifically anyways I only mentioned it as an example..nor anyone else building applications that generate responses based on message converters.

Maybe asking a different way: why do view resolvers have content-language set but message converters do not? Is this so message converters don't have the concept of HTTP and thus their headers? It seems the latest stuff is trying to abstract this with MessageHeaders.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

What I mean is that HTML pages display language specific contents, that 's why we support Content-Language by default since this is the main use case.

For REST APIs, some people create language/locale independent endpoints with message keys returned rather that directly the messages.
Others deal with language at XML or JSON level, sometimes by returning multiple languages within the same response.
Others like you create language specific REST endpoints and that's perfectly fine.

IMO there is not a one-size-fits-all approach at message converter level, that's why I don't think we should do something automatic/by default here + it could cause regressions like pointed our by Rossen. The ResponseBodyAdvice that allows you to implement easily the behavior you want for all your endpoints seems to me a reasonable answer to the need you describe.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

IMO there is not a one-size-fits-all approach at message converter level

That sums it up. We can set it but it doesn't automagically make it so.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

ok, thanks for walking me through your reasoning in detail.

@spring-projects-issues
Copy link
Collaborator Author

Chris DaMour commented

after working with this for a while an issue that's popped up for me with ResponseBodyAdvice is that it's part of the MVC module and not web module, so it brings in a much larger dependency tree vs just working with converters directly.  not a huge deal because few are using message converters for web respones without webmvc, just feels a little heavy in practice.

 

Solution does work though, so thanks!

 

PS: forgot to say..my solution to avoid web-mvc (if anybody ever came to this) was to just do a regular old ControllerAdvice:

@ControllerAdvice
public class ResponseContentTypeAdvice {

    //HACK: using ModelAttribute is a hacky but ensures this gets called for every request...not sure how else to do this
    @ModelAttribute
    public void setContentLanguageOnResponse(
        HttpServletRequest request,
        HttpServletResponse response
    ){
        response.setHeader(
            HttpHeaders.CONTENT_LANGUAGE,
            request.getLocale().toLanguageTag()
        );
    }
}

 

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
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) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants