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

Passing lookup-method arguments to created bean constructor [SPR-7431] #12089

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

Comments

@spring-issuemaster
Copy link
Collaborator

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

Tomasz Nurkiewicz opened SPR-7431 and commented

<lookup-method/> should allow specifying any number of parameters. These parameters should be passed directly to the constructor of the newly created bean. For example:

<bean id="flightValidator" class="com.blogspot.nurkiewicz.lookup.FlightValidator" scope="prototype" lazy-init="true"/>

<bean id="someBean" class="com.blogspot.nurkiewicz.lookup.SomeSpringBean">
    <lookup-method name="createValidator" bean="flightValidator"/>
</bean>
public class SomeSpringBean {

  protected abstract FlightValidator createValidator(Flight flight);

}
public class FlightValidator {

  public FlightValidator(Flight flight) {
    //...
  }

}

This configuration should be valid and flight instance given to createValidator() lookup method should be passed automatically to FlighValidator constructor.

Very rough implementation of this feature (only two lines of code modified!) is discussed here. Similar issue #7703 has been reported and rejected, but its author wanted the lookup-method parameters to be ignored.


Affects: 3.0.3

Attachments:

Issue Links:

  • #9865 @LookupMethod annotation for use with component scanning
  • #15860 BeanFactory lacks method for getting bean by type with specified constructor arguments
  • #12084 Context startup should fail when lookup-method has arguments ("supersedes")

Referenced from: commits eb0ab84

44 votes, 38 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 15, 2011

Cristian Vasile Mocanu commented

Spring definitely needs some way to the problem where you want Spring to provide some dependencies and the other dependencies to be provided by the code (because e.g. they are not known until runtime and depend on some computation).

Currently, I have to implement factories by hand, and while not complex, it's not fun at all.

Guice's way to solve this generic dependency injection issue is AsisstedInject: http://code.google.com/p/google-guice/wiki/AssistedInject
The solution is quite elegant.

The solution to this fix should provide something that solves the same problem as AssistedInject does.

@spring-issuemaster
Copy link
Collaborator Author

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

The Alchemist commented

Attaching a patch which adds support for constructs Tomasz talks about.

Patch includes:

  • Unit tests for current lookup method functionality
  • Unit tests for new functionality
  • Updated some documentation

Thank you, Tomasz, for the blog post which guided me in the process of making this patch.

@Juergen: Is there anything else I can do to improve this patch? Write something in the reference documentation?

@spring-issuemaster
Copy link
Collaborator Author

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

Cristian Vasile Mocanu commented

Hi KP,

I don't know if your patch already does this, but we should be able to specify some arguments in the Spring configuration and other arguments as parameters to the lookup method in Java.

Anything less than this is inferior to Guice's AssistedInject and in this case (where you need to inject some dependencies from the Spring configuration and provide others when calling the lookup method), we still need to create factories by hand.

Note that because I'm using DI quite a lot, I'm running very often into this issue. Not all my objects are singletons.

The basic problem is the following: now you have a constructor to call. How do you match some constructor parameters to the Spring configuration and the others to the parameters of the lookup method?
One way to solve it is matching by name. You have to compile with debug information, but I don't mind that.

I get the feeling that fixing this issue, so that we get the something similar to AssistedInject, takes a lot more code than available in the patch.

@spring-issuemaster
Copy link
Collaborator Author

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

The Alchemist commented

@Cristian Vasile Mocanu: That's just it: currently you can't pass any arguments to the method defined in lookup-method. It's good to hear that Guice has AssistedInject which does something similar.

My patch allows one to specify constructor arguments, similar in style to AssistedInject, it seems, but not nearly as powerful or large in scope. It's a very small step.

@spring-issuemaster
Copy link
Collaborator Author

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

The Alchemist commented

Any luck of this going into 3.1M2? ;)

@spring-issuemaster
Copy link
Collaborator Author

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

Tomasz Nurkiewicz commented

Any news on this issue? Plenty of votes and nothing happening here for more than a year. Can we implement the simplest solution and then discuss how can we extend it in future versions?

@spring-issuemaster
Copy link
Collaborator Author

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

Chris Beams commented

Hi Tomasz, sorry for the long wait here. The suggestions so far look pretty reasonable -- would you be willing to take the current patch suggestion through the contributor guidelines and get it into a pull request? We can discuss further there, but this already looks like a pretty good candidate for getting into the framework.

Thanks!

@spring-issuemaster
Copy link
Collaborator Author

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

Tomasz Nurkiewicz commented

Sure, I will do this ASAP. Should the pull request be against master or 3.1.x branch?

@spring-issuemaster
Copy link
Collaborator Author

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

Chris Beams commented

master, thanks.

@spring-issuemaster
Copy link
Collaborator Author

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

Chris Beams commented

BTW, I just added a section to the contributor guidelines regarding branching from / submitting against master. See "Create your branch from master".

@spring-issuemaster
Copy link
Collaborator Author

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

Tomasz Nurkiewicz commented

Submitted pull request: #84

@spring-issuemaster
Copy link
Collaborator Author

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

Chris Beams commented

Waiting for action on review comments on pull request.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 4, 2012

Chris Beams commented

Tomasz - just double-checking, have you seen my comments on the pull request?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 17, 2013

Carlos Rodarte commented

Has anything happened on this issue? It seems like a pretty handy feature. If Tomasz didn't take action on the code review comments, maybe I can address them :)

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 17, 2013

Tomasz Nurkiewicz commented

I am terribly sorry for the delay, I promise to address all the issues by the end of next week.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jan 18, 2013

Chris Beams commented

No problem, thanks Tomasz. Note that you'll want to work against 3.2.x at this point, i.e. rebase your current branch against 3.2.x, as master now represents work toward Spring Framework 4.0. You may need to resubmit your pull request; I'm not sure if you can change the target branch midflight like that.

As for timing, please note that we plan to release 3.2.1 on Thursday the 24th. So if you'd like the functionality to make it in by then, you'll want to get it in at least a day before.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 4, 2013

Tomasz Nurkiewicz commented

Please have a look at my comment on this pull request. Apparently this change introduces minor (?) backward incompatibility. Not sure how important this is.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 30, 2013

Tomasz Nurkiewicz commented

Any updates or thoughts on this issue? Having this feature would stop people from doing hacks like this.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 13, 2014

Juergen Hoeller commented

Done for 4.1 now, as a follow-up to #15860...

Juergen

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 13, 2014

Juergen Hoeller commented

A nice bonus: Lookup methods can retrieve a target bean just by their return type declaration as well now, without a bean name specified (i.e. without a 'bean' argument in the lookup-method configuration element). This is based on the actual work in #15860.

Juergen

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
2 participants
You can’t perform that action at this time.