Skip to content
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

@ConfigurationProperties does not work on @Bean methods in auto-configuration classes when metadata caching is disabled on the bean factory #18440

Closed
arnesacnussem opened this issue Sep 30, 2019 · 9 comments
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@arnesacnussem
Copy link

Spring Boot version 2.2.0.M6

change the value of spring.messages.basename in application.properties does not make any difference

demo.zip
In this demo,the test com.example.demo.DemoApplicationTests#contextLoads will always fail.

Make a breakpoint at resolveCodeWithoutArguments:151, org.springframework.context.support.ResourceBundleMessageSource#resolveCodeWithoutArguments

The getBasenameSet() will always return a set which only contain one element: "messages"

But if eval the getResourceBundle("msg", locale) at that point,it will return the correct thing.

Also
org.springframework.boot.autoconfigure.context.MessageSourceAutoConfiguration.ResourceBundleCondition#getResources returns the correct thing.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 30, 2019
@snicoll
Copy link
Member

snicoll commented Sep 30, 2019

@arnesacnussem Thanks for the sample. I ran the test in demo.zip and it worked for me. both on the command line and in my IDE. How do I reproduce the issue?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 30, 2019
@arnesacnussem
Copy link
Author

found out it happens when using certain jvm
https://github.com/TravaOpenJDK/trava-jdk-11-dcevm/releases/tag/dcevm-11.0.1%2B8

i run into this bug when updating another project from 2.2.0.M1 to 2.2.0.M6,the old version of Spring boot work fine with dcevm.
btw,that project was originally on 2.1.6.RELEASE.

I also tried AdoptOpenJDK 11.0.4 and OracleJDK 1.8.0_201,they all work fine.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 30, 2019
@wilkinsona
Copy link
Member

wilkinsona commented Sep 30, 2019

Thanks for the additional information.

The failure's occurring because the @ConfigurationProperties annotation isn't found on the messageSourceProperties @Bean method. This appears to be because metadata caching is being disabled on the bean factory. I don't fully understand why this is happening, but I would guess that it's due to some of the hot-swapping support in the custom JVM.

The failure can be reproduced on vanilla OpenJDK using an ApplicationContextInitializer to disable caching:

package com.example.demo;

import java.util.Locale;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.SpringBootConfiguration;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.context.MessageSourceAutoConfiguration;
import org.springframework.context.ApplicationContextInitializer;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.MessageSource;

@SpringBootConfiguration
@ImportAutoConfiguration(MessageSourceAutoConfiguration.class)
public class DemoApplication {

	public static void main(String[] args) {
		SpringApplication application = new SpringApplication(DemoApplication.class);
		application.addInitializers(new ApplicationContextInitializer() {

			@Override
			public void initialize(ConfigurableApplicationContext applicationContext) {
				applicationContext.getBeanFactory().setCacheBeanMetadata(false);
				
			}
			
		});
		MessageSource messageSource = application.run(args).getBean(MessageSource.class);
		messageSource.getMessage("content", null, Locale.getDefault());
	}

}

Further investigation is required to determine if this is a Spring Boot problem or a Spring Framework problem.

@wilkinsona wilkinsona changed the title Unable to autoconfig message source @ConfigurationProperties does not work on @Bean methods when metadata caching is disabled on the bean factory Sep 30, 2019
@wilkinsona
Copy link
Member

Here are a couple of Spring Framework tests that illustrate the difference in behaviour:

package example;

import static org.assertj.core.api.Assertions.assertThat;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.context.annotation.ImportSelector;
import org.springframework.core.type.AnnotationMetadata;

class FactoryMethodResolutionTests {
	
	@Test
	void factoryMethodCanBeResolvedWithBeanMetadataCachingEnabled() {
		assertThatFactoryMethodCanBeResolved(true);
	}
	
	@Test
	void factoryMethodCanBeResolvedWithBeanMetadataCachingDisabled() {
		assertThatFactoryMethodCanBeResolved(false);		
	}
	
	private void assertThatFactoryMethodCanBeResolved(boolean cache) {
		try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
			context.getBeanFactory().setCacheBeanMetadata(cache);
			context.register(ImportSelectorConfiguration.class);
			context.refresh();
			BeanDefinition definition = context.getBeanFactory().getMergedBeanDefinition("exampleBean");
			assertThat(((RootBeanDefinition)definition).getResolvedFactoryMethod()).isNotNull();
		}
	}
	
	@Configuration
	@Import(ExampleImportSelector.class)
	static class ImportSelectorConfiguration {
		
	}
	
	static class ExampleImportSelector implements ImportSelector {

		@Override
		public String[] selectImports(AnnotationMetadata importingClassMetadata) {
			return new String[] { TestConfiguration.class.getName() };
		}
		
	}
		
	@Configuration
	static class TestConfiguration {
		
		@Bean
		@ExampleAnnotation
		public ExampleBean exampleBean() {
			return new ExampleBean();
		}
		
	}
	
	static class ExampleBean {
		
	}
	
	@Target(ElementType.METHOD)
	@Retention(RetentionPolicy.RUNTIME)
	@interface ExampleAnnotation {
		
	}

}

@wilkinsona
Copy link
Member

wilkinsona commented Oct 1, 2019

The Framework tests behave the same way with 5.1 and 5.2, but from a Boot perspective this appears to be a regression. The test above fails with Spring Boot 2.2.0 snapshots but passes with Spring Boot 2.1.8. I think the root cause of that is the fix for #16220 where we moved from our own reflective search for the factory method to retrieving it from the bean definition. This relied on some changes in Framework that were tracked by spring-projects/spring-framework#22420. I'm not sure if we need a change in Framework or if Boot needs a fallback when the factory method isn't available.

@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review type: regression A regression from a previous release and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Oct 1, 2019
@wilkinsona
Copy link
Member

@jhoeller could you guide us here please?

@jhoeller
Copy link

jhoeller commented Oct 2, 2019

@philwebb since the initial refactoring came from your side, are you aware of any gaps there? I'm not quite aware what kind of factory methods Boot would have found that we're still not exposing there...

@philwebb philwebb self-assigned this Oct 2, 2019
@philwebb philwebb added this to the 2.2.x milestone Oct 2, 2019
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Oct 2, 2019
@wilkinsona
Copy link
Member

@jhoeller I don't think this is a result of the initial refactoring as Framework behaves the same way in 5.1 and 5.2. It's a regression in Boot as, in Boot 2.2, we're leaning more heavily on Framework and are now being affected by the difference in behaviour when caching is and is not enabled.

factoryMethodToIntrospect is set on the ConfigurationClassBeanDefinition due to a call that's made to beanFactory.getBeanNamesForType(BeanDefinitionRegistryPostProcessor.class, true, false) when invoking bean factory post-processors. With caching enabled, this merged bean definition is held in the bean factory's mergedBeanDefinitions map. It's then available when making a subsequent call to getResolvedFactoryMethod(). When caching is disabled, the merged bean definition on which factoryMethodToIntrospect is set is not held in the mergedBeanDefinitions map so the information is lost. When a new merged bean definition is subsequently retrieved via a call to getMergedBeanDefinition, there's no logic to set factoryMethodToIntrospect so it's null.

Having investigated further and written up the above, I'm now of the opinion that this is a bug in Framework. getResolvedFactoryMethod() only works reliably due to a side-effect of getBeanNamesForType(). When caching is disabled, that side-effect is too short-lived to be useful. We could work around this in Boot by falling back to the old logic when getResolvedFactoryMethod() is null, but that would be a point-solution to what feels like a general problem.

@wilkinsona wilkinsona changed the title @ConfigurationProperties does not work on @Bean methods when metadata caching is disabled on the bean factory @ConfigurationProperties does not work on @Bean methods in auto-configuration classes when metadata caching is disabled on the bean factory Oct 15, 2019
@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.0 Oct 15, 2019
@wilkinsona
Copy link
Member

I've opened spring-projects/spring-framework#23795 so that we can hopefully revert the main code changes in b240c81 in the future.

snicoll added a commit that referenced this issue Oct 15, 2019
wilkinsona added a commit that referenced this issue Sep 12, 2023
This reverts the main code changes from commit
b240c81. The tests are kept to
verify that the workaround is no longer required.

Closes gh-18591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

6 participants