WebConversionService breaks embedded value resolving

[snicoll]

See this issue and in particular this comment

There is a sample attached with a Spring Boot project that demonstrates the issue.

[philwebb]

I don't understand why the BackgroundPreinitializer breaks things. We just create then throw away a DefaultFormattingConversionService. Does DateTimeFormatAnnotationFormatterFactory.getFormatter get stored in a static somewhere?

[snicoll]

Yep, I don't either. I've debugged the application in Spring Boot and something should call EmbeddedValueResolutionSupport#setEmbeddedValueResolver but it's not clear as why it's not called. I don't think the background thing has any effect.

Perhaps @rstoyanchev knows a bit more about that part before I start digging again?

[rstoyanchev]

I'm not sure either.

[mhalbritter]

Like @snicoll and @philwebb suspected, this has nothing to do with background preinitialization.

In org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration.EnableWebMvcConfiguration#mvcConversionService, we create a WebConversionService. It extends DefaultFormattingConversionService and calls super(false), meaning the embeddedValueResolver is null at that point. It then calls org.springframework.format.support.DefaultFormattingConversionService#addDefaultFormatters, which calls org.springframework.format.datetime.DateFormatterRegistrar#registerFormatters, which calls org.springframework.format.support.FormattingConversionService#addFormatterForFieldAnnotation. In there, we have this code:

if (this.embeddedValueResolver != null && annotationFormatterFactory instanceof EmbeddedValueResolverAware embeddedValueResolverAware) {
  embeddedValueResolverAware.setEmbeddedValueResolver(this.embeddedValueResolver);
}

this.embeddedValueResolver is null at that point, so annotationFormatterFactory.setEmbeddedValueResolver(...) is never called, which explains why the breakpoint there never hits.

I have some changes in https://github.com/mhalbritter/spring-boot/tree/mh/8923, but I'd like to get a second pair of eyes on that change. It does fix the problem in the reproducer project.

[ameyaatmaa]

Hi, I'd like to work on this issue. Based on @mhalbritter's analysis, I'm planning to override addFormatterForFieldAnnotation and setEmbeddedValueResolver in WebConversionService to track factories registered during construction and retroactively propagate the resolver when it becomes available. I'll include tests. Happy to adjust the approach based on team feedback.

[wilkinsona]

Thanks for the offer, @ameyaatmaa, but as you can see from his comment above yours, @mhalbritter already has some changes for this issue that need to be reviewed.