thymeleaf / thymeleaf-spring

Thymeleaf integration module for Spring
http://www.thymeleaf.org
Apache License 2.0
435 stars 157 forks source link

Possible documentation error for viewResolver() #240

Open xtianus opened 4 years ago

xtianus commented 4 years ago

There might be an error in the Thymeleaf-Spring docs that prevents localized text from appearing in templates. I encountered the problem only since upgrading to Spring 5, and I wonder how it could work before.

The problem is that, when following the docs for the ViewResolver bean creation, the MessageSource is not injected in the TemplateEngine so a default is created that ignores my customization.

This is what I think is wrong:

    @Bean
    public ThymeleafViewResolver viewResolver(){
        ThymeleafViewResolver viewResolver = new ThymeleafViewResolver();
        viewResolver.setTemplateEngine(templateEngine());
        return viewResolver;
    }

The templateEngine() call returns a new instance of SpringTemplateEngine that doesn't get a chance of being injected with the MessageSource. What I'm using now:

    @Bean
    public ThymeleafViewResolver viewResolver(SpringTemplateEngine templateEngine){
        ThymeleafViewResolver viewResolver = new ThymeleafViewResolver();
        viewResolver.setTemplateEngine(templateEngine);
        return viewResolver;
    }

This will not create a new instance of SpringTemplateEngine but reuse the @Bean that Spring has injected with the MessageSource.

The possibly wrong code is shown twice in https://www.thymeleaf.org/doc/tutorials/3.0/thymeleafspring.html

ultraq commented 4 years ago

Hey, thanks for reporting this. The docs do have a very long history, going back to when Spring 5 wasn't even around, and for the most part they haven't needed to change thanks to Spring's commitment to backwards compatibility. I'll admit to leaning very much on Spring Boot nowadays so I haven't had to write a view resolver bean in a while!

I'll find some time to verify this and, if it is as you say, maybe add a Spring 5 specific note to the docs you've linked to. Issue will likely move to our docs repo in the process.

danielfernandez commented 3 years ago

That kind of configuration is correct, even if I admit it might seem a bit odd from the standpoint of pure Java code. But we have to take into account that Spring creates a CGLIB-enhanced proxy for every @Configuration class and that makes this work.

In fact, this kind of configuration is documented in the Spring reference doc: https://docs.spring.io/spring-framework/docs/current/reference/html/core.html#beans-java-further-information-java-config

Note the following paragraph:

This is where the magic comes in: All @Configuration classes are subclassed at startup-time with CGLIB. In the subclass, the child method checks the container first for any cached (scoped) beans before it calls the parent method and creates a new instance.

The code in the example comes from the thymeleafexamples-stsm example application, and the MessageSource is correctly set there into the SpringTemplateEngine before this object being set into the ViewResolver. And message resolution through the MessageSource does indeed work. You can have a look at the working application here: https://github.com/thymeleaf/thymeleafexamples-stsm

I admit injecting dependencies as arguments to the @Bean method is in general more elegant, but in the specific case of this application that would not be possible for every method using beans like e.g. addFormatters(), which overrides a method in the superclass and therefore cannot change its signature — and using ctx.getBean(...) here could potentially lead to issues if beans are not created in the right order.

So if you can confirm this does not work for you... maybe you could provide more details on your configuration, and debug in order to make sure which SpringTemplateEngine instance is being set into your ViewResolver as your issue might be somewhere else…

xtianus commented 3 years ago

I'm very sorry but rather than spend hours digging for the reason why the CGLIB magic doesn't work in my case, I'll stick with the more intuitive solution I'm using, because going the magic way doesn't seem to add any value in this case. Thank you for your support.