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

ProxyFactoryBean doesn't warn when it cannot determine if last interceptorNames element is or not an Advice/Advisor [SPR-4640] #9317

Closed
spring-issuemaster opened this issue Mar 28, 2008 · 3 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Mar 28, 2008

nicolas de loof opened SPR-4640 and commented

My configuration uses a ProxyFactoryBean to proxy another bean, that itself is a TransactionProxyFactoryBean on the final target bean.

When the application start, an exception is throwed : AopConfigException("Unknown advisor type ...

My ProxyFactoryBean configuration uses the interceptorNames to set both interceptors and the target bean. ProxyFactoryBean .checkInterceptorNames() is expected to detect the last element to be the target bean. isNamedBeanAnAdvisorOrAdvice() ask the beanFactory for the element type. In my case this doesn't resolve (maybe because the target is itself a ProxyFactoryBean ?) and the default value "true" is returned.

1 - there is no log to inform the ProxyFactoryBean took this fallback decision. Some INFO level message would have been very helpfull to find this issue.

2 - I don't understand why such a type-check is required as my configuration does not declare a target/targetSource/targetName. In such a case, the last interceptorNames element MUST be the target bean.
Why not implement InitializingBean ? When no target* has been set, there is no requirement to check the last bean to be an Advice, as it MUST be the target bean name. The GLOBAL_SUFFIX check (from initializeAdvisorChain) could also be done at this level.

Maybe using interceptorNames to set the target bean is considered confusing and should be deprecated (IMHO)


Affects: 2.5.2

Attachments:

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 28, 2008

nicolas de loof commented

The attached patch change the ProxyFactoryBean initialization to rely on InitializingBean.afterPropertiesSet() :

If no targetSource / targetName has been set, select the last interceptorNames element as a target, with no check for bean type. This required to make a tiny change on the testcase for testGetObjectTypeWithNoTargetOrTargetSource as the exception thrown is AopInvocationException, not UnsupportedOperationException. Can be considered a compatibile change ?

Maybe it could also check for the bean NOT to be an interceptor if the beanFactory can retrieve the bean type ?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 29, 2008

Juergen Hoeller commented

I've opted for deprecating the target-name-as-final-element-in-"interceptorNames" feature altogether, as well as changing the detection algorithm to assume a target object (rather than an Advisor/Advice) if the type cannot be determined - which should be a more reasonable default. I've also added some further debug logging.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 29, 2008

Juergen Hoeller commented

"afterPropertiesSet"-style checking may have subtle side effects here due to ProxyFactoryBean being supported within circular references as well. The general deprecation of this questionable feature along with the change in default assumption when the type cannot be determined should be the better tradeoff.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.