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

Context startup should fail when lookup-method has arguments [SPR-7426] #12084

Closed
spring-issuemaster opened this issue Aug 5, 2010 · 5 comments
Closed

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Aug 5, 2010

Tomasz Nurkiewicz opened SPR-7426 and commented

When <lookup-method> tag references method with some arguments, Spring context starts up successfully ignoring the lookup method and not implementing it using CGLIB. But when the lookup method is called at runtime, AbstractMethodError is thrown. Spring should validate lookup method parameters existence at startup and fail if no no-arg method exist (or at least issue a warning).


Affects: 3.0.3

Issue Links:

  • #12089 Passing lookup-method arguments to created bean constructor ("is superseded by")
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 6, 2010

Tomasz Nurkiewicz commented

If #12089 will be implemented, this issue would become irrelevant.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Apr 28, 2011

The Alchemist commented

I was in the process of writing a patch to support #12089 and I was thinking about implementing this as well.

Unfortunately, it seems this can't really be done easily with the current API because when you hit the lookup-method elements, the other bean may not even be defined yet. e.g.,

<bean id="abstractBean" class="test.beans.AbstractBean" >
	<lookup-method name="get" bean="test"/>
</bean>
	
<bean id="test" class="test.beans.TestBean" scope="prototype">
</bean>

Above, the bean test isn't even defined when parsing the definition of abstractBean. So how can you verify if a no-arg constructor exists if we haven't gotten to test yet (at least in the same phase of initialization).

It would require some post-processing, and I'm not sure what the way to do that is. If someone could point me in the right direction, I would be grateful.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 19, 2011

buko.obele commented

I ran into some similar problems and it occurred to me that perhaps the lookup-method/abstract-method isn't always the best way to do this. This can also be achieved through Spring AOP. See http://therobotmenace.blogspot.com/2011/05/proton-changes.html for full details.

What would be cool is if we could annotate prototype-factory methods like:

@Prototype
public Foo createFoo(Bar a, Bar b) {
return new Foo(a, b);
}

And at that point the Spring-generated CGlib-subclass would intercept the method-call and return the Spring-created bean with all di-injection and constructor-injection goodness. Logically from a design perspective I see this as very consistent too: when I annotate a method with the @Prototype annotation I'm being very explicit that I want a new instance each time.

This also, by leveraging the current Spring AOP infrastructure, bypasses KP's problems with timing. Prototype-resolution doesn't actually have to happen until run-time.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 19, 2011

buko.obele commented

Also I would add that this feature/functionality isn't really 'trivial'. Some of us live in secure or cloud-based environments or where full-on bytecode weaving isn't possible but we still have a real need to be able to do prototype-creation at runtime and continue to enjoy all the benefits of Spring. I feel like this is actually a big hole in Spring3 and we should do our best to patch it up ASAP. (Yes you can, implement ApplicationContextAware but one of the benefits of Spring is the lack of intrusiveness so I don't consider ApplicationContextAware a solution by any means).

Thanks

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 15, 2012

Chris Beams commented

Won't fix, in favor of implementing #12089, which makes this request obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.