BeanFactory.get*ForAnnotation should take @Bean metadata into account

[mdeinum]

I'm currently writing a small BeanFactoryPostProcessor and for that I need to process annotations on @Bean methods. Or at least detect the bean definition based on that metadata. I was expecting that getBeanNamesForAnnotation (and friends) would also take annotations on @Bean methods into consideration. However it only works for annotated types.

So when doing something like this.

@Component
@CustomAnnotation
public class MyClass { ... }

It works. But doing the same on an @Bean annotated method doesn't work.

@Bean
@CustomAnnotation
public MyClass myClass() { ... }

It would be nice if the methods would return beans (or bean definitions) annotated throuhg @Bean methods as well. Especially if we want to apply this to beans that aren't under our own control (i.e. external libraries).

For the Spring annotations like @Lazy, @Primary etc. this already works but those are handled in specialized cases when parsing the configuration files. @Lazy on a component works the same as @Lazy on a @Bean method.

In this particulair case I wanted to detect beans with a special annotation so that we could register them in JNDI. It works when annotating the component with our specialized annotation not when using the annotation on @Bean methods. This is especially annoying for classes that are outside of our control.

Currently the only way is to extend the class only for adding the annotation.

[sbrannen]

Are you referring explicitly to getBeanNamesForAnnotation(...) and getBeansWithAnnotation(...) in org.springframework.beans.factory.ListableBeanFactory, or did you have other concrete methods in mind?

Also, if we were to introduce such support, would the introduction of a new method specific to your proposed use case be sufficient/acceptable?

[mdeinum]

I'm indeed referring to those methods (I also believe they eventually delegate to the same internal methods).

I would not expect additional methods to be added, but if that is more convenient (or less of a surprise to existing users) then I'm perfectly fine with that.

[sbrannen]

I'm indeed referring to those methods (I also believe they eventually delegate to the same internal methods).

Yes, getBeansWithAnnotation() delegates to getBeanNamesForAnnotation().

I would not expect additional methods to be added, but if that is more convenient (or less of a surprise to existing users) then I'm perfectly fine with that.

Aiming for the element of least surprise is exactly my intention here: changing the behavior of those existing methods would technically be a breaking change since such invocations would start to return things that they previously did not.

[sbrannen]

If you want to implement this on your own, it is technically possible. For example, the following works for @Bean methods that do not accept arguments.

public class MyBeanFactoryPostProcessor implements BeanFactoryPostProcessor {

	@Override
	public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
		Class<? extends Annotation> annotationType = MyAnnotation.class;

		Arrays.stream(beanFactory.getBeanDefinitionNames())//
				.map(beanFactory::getBeanDefinition)//
				.filter(bd -> !bd.isAbstract())//
				.filter(bd -> bd.getFactoryMethodName() != null)//
				.filter(bd -> bd.getFactoryBeanName() != null)//
				.filter(bd -> isBeanMethodAnnotated(beanFactory, bd, annotationType))//
				.forEach(bd -> System.out.println("@Bean method " + bd.getFactoryMethodName()
						+ " is annotated with @" + annotationType.getSimpleName() + "."));
	}

	private boolean isBeanMethodAnnotated(ConfigurableListableBeanFactory beanFactory,
			BeanDefinition beanDefinition, Class<? extends Annotation> annotationType) {

		BeanDefinition factoryBeanDefinition = beanFactory.getBeanDefinition(beanDefinition.getFactoryBeanName());

		if (factoryBeanDefinition instanceof AbstractBeanDefinition) {
			AbstractBeanDefinition abd = (AbstractBeanDefinition) factoryBeanDefinition;
			if (abd.hasBeanClass()) {
				Class<?> factoryClass = ClassUtils.getUserClass(abd.getBeanClass());
				String factoryMethodName = beanDefinition.getFactoryMethodName();
				Method factoryMethod = ReflectionUtils.findMethod(factoryClass, factoryMethodName);
				return AnnotatedElementUtils.isAnnotated(factoryMethod, Bean.class)
						&& AnnotatedElementUtils.isAnnotated(factoryMethod, annotationType);
			}
		}

		return false;
	}

}

If you want to support @Bean methods that accept arguments, that should also be possible by processing the ConstructorArgumentValues in the BeanDefinition and then selecting the correct factory method by supplying the argument types to ReflectionUtils.findMethod(...). However, at that point the code starts to become cumbersome (more so than it already is).

In any case, the above serves as a proof of concept.

[sbrannen]

Tentatively slated for 5.2 M1 for further investigation and discussion

[sbrannen]

@mdeinum, feel free to experiment with the above proof of concept in my branch.

[mdeinum]

Wouldn't it be nicer to detect the annotations while scanning/detecting beans and store them in the BeanDefinition as a cache? That way it doesn't really matter if an @Component class is annotated or if the metadata comes from an @Bean annotated method.

For me as a user (I know I might not be the average user :) ), it doesn't really matter if I specify metadata on the @Bean or the class/interface.

[sbrannen]

@mdeinum, it looks like Spring Boot already has the support you're looking for...

https://github.com/spring-projects/spring-boot/blob/add8c6f2957211b5e6099127bf7993ed3f3fd174/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationBeanFactoryMetadata.java

What do you think about that?

[sbrannen]

Update: due to changes in RootBeanDefinition introduced in #22420, I can now simplify my previous prototype as follows.

public class MyBeanFactoryPostProcessor implements BeanFactoryPostProcessor {

	@Override
	public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
		Class<? extends Annotation> annotationType = MyAnnotation.class;

		findAnnotatedBeanDefinitions(beanFactory, annotationType)//
				.forEach(bd -> System.out.println("@Bean method " + bd.getFactoryMethodName()
						+ " is annotated with @" + annotationType.getSimpleName() + "."));
	}

	private List<BeanDefinition> findAnnotatedBeanDefinitions(ConfigurableListableBeanFactory beanFactory,
			Class<? extends Annotation> annotationType) {

		return Arrays.stream(beanFactory.getBeanDefinitionNames())//
				.map(beanFactory::getBeanDefinition)//
				.filter(bd -> !bd.isAbstract())//
				.filter(RootBeanDefinition.class::isInstance)//
				.map(RootBeanDefinition.class::cast)//
				.filter(bd -> isAnnotatedBeanMethod(bd, annotationType))//
				.collect(toList());
	}

	private boolean isAnnotatedBeanMethod(RootBeanDefinition beanDefinition,
			Class<? extends Annotation> annotationType) {

		Method factoryMethod = beanDefinition.getResolvedFactoryMethod();
		return factoryMethod != null && AnnotatedElementUtils.isAnnotated(factoryMethod, Bean.class)
				&& AnnotatedElementUtils.isAnnotated(factoryMethod, annotationType);
	}

}

[jhoeller]

On review, I'm in favor of redefining getBeanNamesForAnnotation to detect annotations on @Bean methods as well. We can do that sort of change in 5.2 even if technically may break compatibility for some callers; I doubt it'll be the case though since we're just falling back to checking the factory method. It's hard to come up with a scenario where the bean class does not have a particular annotation but the @Bean method has that annotation type, and an existing call to getBeanNamesForAnnotation for that particular annotation does not want to see the latter.

As mentioned above, this fits nicely with the getResolvedFactoryMethod revision that came with #22420. It also fits nicely with our qualifier resolution algorithm which checks the factory method as well.

[sbrannen]

SGTM 👍