Skip to content

HttpInvokerProxyFactoryBean and co do not reliably expose correct type when declared via @Bean [SPR-11842] #16461

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

Closed
spring-projects-issues opened this issue Jun 2, 2014 · 8 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 2, 2014

Pierre Maréchal opened SPR-11842 and commented

Using @Bean to declare my HttpInvokerProxyFactoryBean and @Autowire to inject it, Spring does not seem to resolve that my bean depends on the factory bean.
I've attached a sample project displaying the behaviour.
Simply run "mvn test"

un-commenting the @DependsOn in MyBean works around the issue though.


Affects: 4.0.5

Attachments:

Issue Links:

Referenced from: commits ae66e45, 85b2c7d

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

The demo sample is available on the spring-framework-issues project

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Jürgen, I am assigning this one to 4.1 since our internal testing shows that there is indeed something we might do on our side.

Pierre, it is recommended not to use such FactoryBean with java config and try as much as possible to give the exact type of the bean. In your example, HttpInvokerProxyFactoryBean has the following declaration implements FactoryBean<Object>. When Spring inspects it to figure out if that bean is an autowired candidate, Object is returned.

Of course, we could invoke the bean definition and actually create the bean to figure out the exact type but this may lead to beans being initialized too early in the application lifecycle.

Either way, keep in mind that it's always best to define the runtime type of your bean as soon as possible. The following code snippet is an example of what I mean:

@Bean
public MyService myService() {
    HttpInvokerProxyFactoryBean factory = new HttpInvokerProxyFactoryBean();
    factory.setServiceUrl("/svc/dummy");
    factory.setServiceInterface(MyService.class);
    return (MyService) factory.getObject();
}

@spring-projects-issues
Copy link
Collaborator Author

Pierre Maréchal commented

Hi Stéphane,

That's exactly what I tried at first. But your code snippet would return a null because the FactoryBean has not yet been initialized fully.

This would work:

@Bean
public MyService myService() {
    HttpInvokerProxyFactoryBean factory = new HttpInvokerProxyFactoryBean();
    factory.setServiceUrl("/svc/dummy");
    factory.setServiceInterface(MyService.class);
    factory.afterPropertiesSet();
    return (MyService) factory.getObject();
}

though I find manually calling afterPropertiesSet() not very elegant.
How does the XML counterpart work internally, I don't remember having to do anything special with FactoryBeans when using XML declaration.

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Oh, right sorry.

Well, the XML declaration, is an XML declaration, right? We can just read the model that you have defined using XML without creating the FactoryBean instance. Javaconfig does not provide a model for the bean but the bean itself. That's why it's harder to figure out what the type can be with javaconfig and the main reason we advise to always have a @Bean return type that is as close as the target type as possible.

@spring-projects-issues
Copy link
Collaborator Author

Pierre Maréchal commented

OK thanks for the clarification.

So I guess that for the moment, the safest is to declare it this way:

@Bean
public HttpInvokerProxyFactoryBean myServiceFactoryBean() {
    HttpInvokerProxyFactoryBean factory = new HttpInvokerProxyFactoryBean();
    factory.setServiceUrl("/svc/dummy");
    factory.setServiceInterface(MyService.class);
    return factory;
}

@Bean
public MyService myService() {
    return (MyService)myServiceFactoryBean().getObject();
}

Or is there a better way?

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

I wouldn't consider this "safe" but I see what you mean. You're still exposing your service twice and MyService is now exposed as "myServiceFactoryBean" as well. Hopefully, the context is smart enough to relalize that this is the same instance so autowiring by type would still work.

But If we improve our support in the future, this would probably cause a problem as we have now two different beans exposing the same type. Juergen Hoeller, thoughts?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Let's reposition this issue towards the specific FactoryBeans at hand here:

We generally can't do much about type resolution for @Bean declarations which return a non-typed FactoryBean - since from a container perspective, we don't know anything about the kind of FactoryBean and its transitive dependencies which we'd trigger if we invoke its factory method just for type matching purposes...

That said, we should at least have a better recommendation for how to set up HTTP invoker proxies (and other remoting proxies) in an @Bean style. Shipping regular factory variants (a la ProxyFactory's getProxy methods and our LocalSessionFactoryBuilder for Hibernate 4) next to the existing XXProxyFactoryBeans might be the most idiomatic solution here.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Taking a step from my previous comment...

There is away to set up remoting proxies programmatically: via a ProxyFactory.getProxy call with an HttpInvokerClientInterceptor argument. We could make that combination more straightforward through a combined HttpInvokerProxyFactory builder class but it won't get significantly more convenient than that, so it might not be worth it individually. A framework-wide approach for a new proxy builder API style would be more compelling but we cannot take it that far at this point.

In fact, there is a minor twist in the core container that significantly eases things: At the moment, for @Bean declared FactoryBeans, we only fall back to a full factory method call if the FactoryBean declaration is non-generic, i.e. declared as a raw FactoryBean implementation. If we extend that rule to consider a FactoryBean<Object> as 'raw' as well, a scenario like yours starts to work out fine without any further changes, since all our proxy factory beans are declared as FactoryBean<Object> already, effectively indicating that they have no idea about the type before getting constructed and populated with properties... whereas all other kinds of FactoryBeans are declared with some specific base type at least.

Note that an @Bean factory method call does not necessarily mean that the bean is fully initialized at that point. In fact, the FactoryBean type resolution algorithm will only construct an early instance through that factory method call but not process the instance any further, i.e. not apply any container callbacks such as afterPropertiesSet yet. This is not that far from the way we're doing early resolution of XML-defined FactoryBeans - for which we don't introspect a generic FactoryBean declaration at all, whether raw or FactoryBean<Object>: We rather always construct an early instance there. The proposed core container refinement effectively makes @Bean FactoryBean processing behave more analogously now.

So in summary, this seems to be an isolated change with good rewards for @Bean-declared FactoryBeans, in particular for a transition from XML-declared FactoryBeans, so I'm applying it for Spring Framework 4.1. Further steps to be taken and/or recommendations to be refined later on.

Hope that helps,

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants