spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.84k stars 40.61k forks source link

Views for different contenttypes cannot be resolved anymore. #13424

Open michael-simons opened 6 years ago

michael-simons commented 6 years ago

Find full demo and failing test attached: demo.zip

Given the following controller

@Controller
public class SomeController {
    @GetMapping(value = "/", produces = {MediaType.TEXT_HTML_VALUE, MediaType.APPLICATION_ATOM_XML_VALUE, "text/csv"})
    public String someView() {
        return "someView";
    }
}

I can have multiple views providing the requested content types. For example an atom and a csv view:

@Component("someView.csv")
class SomeCSVView extends AbstractView {
    public SomeCSVView() {
        super.setContentType("text/csv");
    }

    @Override
    protected void renderMergedOutputModel(Map<String, Object> map, HttpServletRequest request, HttpServletResponse response) throws Exception {
        response.setContentType(UTF_8.name());
        try(Writer out = new OutputStreamWriter(response.getOutputStream(), UTF_8)) {
            out.write("this;is;a;test\n");
        }
        response.flushBuffer();
    }
}

@Component("someView.atom")
class SomeAtomView extends AbstractAtomFeedView {

    @Override
    protected List<Entry> buildFeedEntries(Map<String, Object> map, HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) throws Exception {
        final Entry entry = new Entry();
        entry.setTitle("This is a test");
        return Arrays.asList(entry);
    }
}

In addition to that I have to configure unknown media types as so

spring.mvc.content-negotiation.media-types.atom = application/atom+xml
spring.mvc.content-negotiation.media-types.csv = text/csv

I expect that the views are correctly resolved depending on the media type given through the accept header. However, only the text/html gets resolved (for example as an answer to ` curl -v -H "Accept: application/atom+xml" localhost:8080).

This does not work anymore since 2.0.0.RC1, as the new Path Matching and Content Negotiation break ContentNegotiatingViewResolver.

The official? (or at least usual) way to name the different views was "name.ext". That worked as shown above in 1.x and in 2 until 2.0.0.RC1, where those changes have been introduced.

Those changes are fine, but they break ContentNegotiatingViewResolver. In org.springframework.web.servlet.view.ContentNegotiatingViewResolver#getCandidateViews it uses a ContentNegotiationManager. String viewNameWithExtension = viewName + '.' + extension; pretty much assures me that naming the views as above has been correct.

However, the ContentNegotiationManagercannot resolve any extension in resolveFileExtensions because there is no more MediaTypeFileExtensionResolver as spring.mvc.content-negotiation.favor-path-extension is false since 2.0.0.RC1.

I would understand the the failure if I requested /someView.atom but I'm using the accept header as shown in the test.

Workaround is to set favor to true, but that is not recommend for good reasons.

Another one is to provide a bean post processor like this

@Bean
    public BeanPostProcessor contentNegotiationManagerPostProcessor(final WebMvcProperties webMvcProperties) {
        return new BeanPostProcessor() {
            @Override
            public Object postProcessBeforeInitialization(final Object bean, final String beanName) throws BeansException {
                if(ContentNegotiationManager.class.isInstance(bean)) {
                    ((ContentNegotiationManager)bean)
                        .addFileExtensionResolvers(new MappingMediaTypeFileExtensionResolver(webMvcProperties.getContentnegotiation().getMediaTypes()));
                }
                return bean;
            }
        };
    }

and that would be something I expect from Boot itself.

bclozel commented 6 years ago

Hi @michael-simons

Thanks for the (very detailed) report, it's spot on 😄

Implementation details

Here's what's happening in that very case, for future reference:

  1. During application startup, Spring creates a ContentNegotiationManager according to the user configuration (supporting path extension, query parameters, HTTP Accept headers, etc... for content negotiation)
  2. During the handler mapping process (mapping the incoming request to the appropriate Controller method), we resolve the media types asked by the request, using the strategies in ContentNegotiationManager
  3. Once the handler has been called and we get back the view name, the ContentNegotiatingViewResolver comes into play: it's selecting the appropriate media type (comparing the ones asked by the request and the ones that the handler can produce)
  4. Then it's trying to find a view matching the name given by the handler, here "someView"; there's an existing Thymeleaf template with that name, but the bean views are named "someView.atom" and "someView.csv"
  5. At that point, the MediaTypeFileExtensionResolver instances configured in the ContentNegotiationManager can use the user-configured media type/extension pairs (or all known, depending on the configuration) to resolve additional view names

With just the HeaderContentNegotiationStrategy configured (no path extension strategy), we don't have any MediaTypeFileExtensionResolver configured, as expected: the information is gathered from the Accept header, nothing else. This is why those views are not found by default here.

This is also why the BeanPostProcessor adding that MediaTypeFileExtensionResolver works around this problem.

Possible solutions

Spring Framework could add that MediaTypeFileExtensionResolver even for header-based strategies, but it wouldn't really make sense and could break applications in even more subtle ways.

You're right, setting spring.mvc.contentnegotiation.favor-path-extension back to true is not the best choice, especially if your application doesn't match on path extensions.

Another (simpler) way of solving that would be to enable the other option:

spring.mvc.contentnegotiation.favor-parameter=true

I've added that option to your repro project and the tests all pass.

This adds another content negotiation strategy to the mix, but also a MediaTypeFileExtensionResolver that does the job in this case. The only side-effect of that is, HTTP clients will be able to drive the content negotiation using a query parameter, like GET /?format=atom.

This is the solution described in the reference documentation already.

On the top of my head, I don't see how I can improve that part of the documentation (or the Spring Framework reference one).

Let me know what you think!

spring-projects-issues commented 6 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

michael-simons commented 6 years ago

Thanks @bclozel for your detailed feedback and better explanation.

The proposed solution does indeed work for and I don't mind having the ability around to use a query parameter as well.

I think there could be work done in both Spring MVC and Spring Boot docs.

I wonder how many people new to Spring MVC and Spring WebFlux would figure out that one can name views view.csv etc. I neither find it here nor here (Maybe I'm just to blind).

Having that in place something like

Enabling a content negotiation based on a explicit format either via favor-parameter or favor-path-extension enables the MediaTypeFileExtensionResolver and you can have several views with the same base name and varying extension

(Pretty rough, but I would find something along these lines helpful).