in AutowiredAnnotationBeanPostProcessor
,we buildAutowiringMetadata
against method annotated @Autowired
。
private InjectionMetadata buildAutowiringMetadata(final Class<?> clazz) {
// ....
ReflectionUtils.doWithLocalMethods(targetClass, method -> {
Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
if (!BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod)) {
return;
}
AnnotationAttributes ann = findAutowiredAnnotation(bridgedMethod);
if (ann != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) {
if (Modifier.isStatic(method.getModifiers())) {
if (logger.isInfoEnabled()) {
logger.info("Autowired annotation is not supported on static methods: " + method);
}
return;
}
if (method.getParameterCount() == 0) {
if (logger.isInfoEnabled()) {
logger.info("Autowired annotation should only be used on methods with parameters: " +
method);
}
}
boolean required = determineRequiredStatus(ann);
PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
currElements.add(new AutowiredMethodElement(method, required, pd));
}
});
// ...
}
From the above code, I can see that any method with an @Autowired
annotation will be treated as an injection point, which means that parameters of method will be injected, if parameters cannot be injected, an error will be reported, and the method will be invoked when method InjectionMetadata.inject
executes。
We think there are several problems with the above code:
-
Methods with no arguments should be filtered out,no arguments means do not need
autowire
-
BeanUtils.findPropertyForMethod(bridgedMethod, clazz)
will return null whenbridgedMethod
is neitherreadMethod
norwriteMethod
。In this case, do we still need to inject? - if
bridgedMethod
isreadMethod
that means the method look likevoid getXxx()
,do we still need new aAutowiredMethodElement
?
Because I don't know if this code is designed that way,so I make this issue。
Looking forward to your reply. Thank you very much!!!!
Comment From: sbrannen
Methods with no arguments should be filtered out,no arguments means do not need
autowire
That's correct.
@jhoeller, are you aware of any reason why we don't simply return
early there like with the static
check?
* `BeanUtils.findPropertyForMethod(bridgedMethod, clazz)` will return null when `bridgedMethod` is neither `readMethod ` nor `writeMethod`。In this case, do we still need to inject?
The PropertyDescriptor
supplied to AutowiredMethodElement
is @Nullable
. So that's not an issue. Note that autowired methods do not need to be customary "setter" methods -- for example, @Autowired void config(Service1 s1, Service2 s2)
is a valid candidate for autowiring.
Returning early for zero arguments would also avoid that code path entirely.
if
bridgedMethod
isreadMethod
that means the method look likevoid getXxx()
,do we still need new aAutowiredMethodElement
?
Again, if the method accepts zero arguments, I think we should just return
early.
Looking forward to your reply. Thank you very much!!!!
Let's see what @jhoeller says.
Comment From: jhoeller
This is essentially about cases that aren't really proper, and we're just trying to behave reasonably.
In case of an @Autowired
method with no parameters, we're nevertheless invoking it since it may perform some initialization logic... even if such logic is better suited in an @PostConstruct
method or a constructor.
In case the method doesn't match a JavaBeans property, we're nevertheless invoking it since autowired methods fundamentally may have any signature; we're only trying to determine whether an external property override may affect it.
Comment From: sbrannen
Thanks for the enlightenment, @jhoeller.
In case of an
@Autowired
method with no parameters, we're nevertheless invoking it since it may perform some initialization logic... even if such logic is better suited in an@PostConstruct
method or a constructor.
I wasn't aware that we even "supported" no-args @Autowired
methods. Interesting!
Though I wonder if that's documented and/or tested...
Comment From: daimingzhi
If it is designed in this way, is it better for the code to make the following changes?
in AutowiredAnnotationBeanPostProcessor#buildAutowiringMetadata
boolean required = false;
PropertyDescriptor pd = null;
if (method.getParameterCount() == 0) {
//In case of an @Autowired method with no parameters,
// we're nevertheless invoking it since it may perform some initialization logic...
// even if such logic is better suited in an @PostConstruct method or a constructor.
if (logger.isInfoEnabled()) {
logger.info("Autowired annotation should only be used on methods with parameters: " +
method);
}
} else {
required = determineRequiredStatus(ann);
pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
}
// boolean required = determineRequiredStatus(ann);
// PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
currElements.add(new AutowiredMethodElement(method, required, pd));
The main changes are:
- Add a code path for
no-args
method - Add comments to make this code easy for others to understand(It took me a long time to understand it)
in AutowiredMethodElement#inject
if the method which will be invoked is no-args
,we can end early
protected void inject(Object bean, @Nullable String beanName, @Nullable PropertyValues pvs) throws Throwable {
if (checkPropertySkipping(pvs)) {
return;
}
Method method = (Method) this.member;
Object[] arguments;
if (this.cached) {
// Shortcut for avoiding synchronization...
arguments = resolveCachedArguments(beanName);
} else if (method.getParameterCount() == 0) {
arguments = new Object[0];
} else {
Class<?>[] paramTypes = method.getParameterTypes();
// .....
}
// .....
}
If my suggestion is acceptable,can I make a PR for this?
Thanks!
Comment From: daimingzhi
@sbrannen @jhoeller I'm sorry to disturb you, but I've been waiting for a long time