Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Passing lookup-method args to bean constructor #84

Open
wants to merge 1 commit into from

3 participants

@nurkiewicz

should allow specifying any number of parameters.
These parameters should be passed directly to the constructor of the
newly created bean.

Previously had to be parameterless. Now if you provide
any parameters to that method, they are passes directly (1 to 1) to the
bean constructor, just like if they were declared using
. The idea was originally suggested here:

http://nurkiewicz.blogspot.com/2010/08/creating-prototype-spring-beans-on.html

This change is based entirely on a patch provided by Karl Pietrzak.

I submitted CLA.

Issue: SPR-7431

@nurkiewicz nurkiewicz Passing lookup-method args to bean constructor
<lookup-method/> should allow specifying any number of parameters.
These parameters should be passed directly to the constructor of the
newly created bean.

Previously <lookup-method/> had to be parameterless. Now if you provide
any parameters to that method, they are passes directly (1 to 1) to the
bean constructor, just like if they were declared using
<constructor-arg/>. The idea was originally suggested here:

http://nurkiewicz.blogspot.com/2010/08/creating-prototype-spring-beans-on.html

This change is based entirely on a patch provided by Karl Pietrzak.

Issue: SPR-7431
b343ca1
@cbeams cbeams was assigned
@cbeams

Thanks, Tomasz. Will take a look!

@cbeams

Should be updated. Double-check other modified sources to make sure the headers are updated there too.

@cbeams

This should be your name, Tomasz, or Karl should sign the CLA. Or just delete it.

This is Karl. Not sure if it's necessary anymore, but I signed the CLA. Confirmation number 27520120525075935.

@cbeams

Same comment as above regarding author name.

@cbeams

Commit comment had grammar errors:

Pass lookup-method args to bean constructor

Previously <lookup-method> had to be parameterless. Now if parameters
are provided to the method, they are passed through directly to the bean
constructor as if they had been declared using <constructor-arg>.

Issue: SPR-7431

I've removed the URL since we don't typically include these in commit comments. It's fine to put that information in JIRA, however.

Tomasz, please add at least a brief explanation of this enhancement to the <lookup-method> reference documentation at src/reference/docbook/beans-dependencies.xml. Build the docs with gradle reference to test out the changes locally.

@The-Alchemist, thanks for signing the CLA.

@cbeams

{@code ...}, not <code>...</code>. And the markup isn't really necessary in JUnit tests anyway as we never render API documentation for this.

@cbeams

No need for a new compilation unit here, since AbstractBean is only used within LookupMethodTests. Just make it a private or default-visibility nested static class within LookupMethodTests.

@cbeams

org.junit.Assert#assertThat is preferred over #assertEquals

@cbeams

final is not necessary here. please remove all unnecessary final modifiers on local variables.

@cbeams

the type-safe variant #getBean(AbstractBean.class) is preferred.

@nurkiewicz

Unfortunately my pull request causes testReplaceMethodOverrideWithSetterInjection() in org.springframework.beans.factory.xml.XmlBeanFactoryTests to fail. It's this assertion:

TestBean notJenny = oom.getPrototypeDependency("someParam");
assertTrue(!"Jenny".equals(notJenny.getName()));

it makes sure that overloaded getPrototypeDependency(String) method is not modified by Spring and only parameterless getPrototypeDependency() is overriden by lookup-method mechanism. This behavior changes after applying this patch. Now all methods, no matter if they have parameters or not, are overriden by lookup-method mechanism.

This means that lookup-method with parameters is not really backward compatible - in case there was more than one overloaded method. I am not sure how big this is. Applying lookup-method only if target bean has matching constructor (not always, what this pull request does) will help a little, but won't get rid of incompatibility completely.

My reviewed code is available at SPR-7431b branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 24, 2012
  1. @nurkiewicz

    Passing lookup-method args to bean constructor

    nurkiewicz authored
    <lookup-method/> should allow specifying any number of parameters.
    These parameters should be passed directly to the constructor of the
    newly created bean.
    
    Previously <lookup-method/> had to be parameterless. Now if you provide
    any parameters to that method, they are passes directly (1 to 1) to the
    bean constructor, just like if they were declared using
    <constructor-arg/>. The idea was originally suggested here:
    
    http://nurkiewicz.blogspot.com/2010/08/creating-prototype-spring-beans-on.html
    
    This change is based entirely on a patch provided by Karl Pietrzak.
    
    Issue: SPR-7431
This page is out of date. Refresh to see the latest.
View
2  spring-beans/src/main/java/org/springframework/beans/factory/BeanFactory.java
@@ -170,7 +170,7 @@
* overriding the specified default arguments (if any) in the bean definition.
* @param name the name of the bean to retrieve
* @param args arguments to use if creating a prototype using explicit arguments to a
- * static factory method. It is invalid to use a non-null args value in any other case.
+ * static factory method.
* @return an instance of the bean
* @throws NoSuchBeanDefinitionException if there's no such bean definition
* @throws BeanDefinitionStoreException if arguments have been given but
View
2  ...beans/src/main/java/org/springframework/beans/factory/support/CglibSubclassingInstantiationStrategy.java
@@ -157,7 +157,7 @@ public int hashCode() {
public Object intercept(Object obj, Method method, Object[] args, MethodProxy mp) throws Throwable {
// Cast is safe, as CallbackFilter filters are used selectively.
LookupOverride lo = (LookupOverride) beanDefinition.getMethodOverrides().getOverride(method);
- return owner.getBean(lo.getBeanName());
+ return owner.getBean(lo.getBeanName(), args);
}
}
View
2  spring-beans/src/main/java/org/springframework/beans/factory/support/LookupOverride.java
@@ -61,7 +61,7 @@ public String getBeanName() {
*/
@Override
public boolean matches(Method method) {
- return (method.getName().equals(getMethodName()) && method.getParameterTypes().length == 0);
+ return (method.getName().equals(getMethodName()));
}
View
89 spring-beans/src/test/java/org/springframework/beans/factory/support/LookupMethodTests.java
@@ -0,0 +1,89 @@
+/*
+ * Copyright 2002-2009 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.beans.factory.support;
+
+import junit.framework.TestCase;
+
+import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
+import org.springframework.core.io.ClassPathResource;
+
+import test.beans.AbstractBean;
+import test.beans.TestBean;
+
+/**
+ * Tests the use of lookup-method
+ * @author Karl Pietrzak
+ */
+public class LookupMethodTests extends TestCase {
+
+
+ private DefaultListableBeanFactory beanFactory;
+
+ protected void setUp() throws Exception {
+ final ClassPathResource resource = new ClassPathResource("/org/springframework/beans/factory/xml/lookupMethodBeanTests.xml", getClass());
+ this.beanFactory = new DefaultListableBeanFactory();
+ final XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(this.beanFactory);
+ reader.loadBeanDefinitions(resource);
+ }
+
+ /**
+ * lookup method's bean has no constructor arguments
+ */
+ public void testWithoutConstructorArg() {
+ AbstractBean bean = (AbstractBean)this.beanFactory.getBean("abstractBean");
+ assertNotNull(bean);
+ final Object expected = bean.get();
+ assertEquals(TestBean.class, expected.getClass());
+ }
+
+ /**
+ * Creates a new instance of {@link TestBean} using the constructor which takes a single <code>String</code>
+ */
+ public void testWithOneConstructorArg() {
+ AbstractBean bean = (AbstractBean)this.beanFactory.getBean("abstractBean");
+ assertNotNull(bean);
+ final TestBean expected = bean.getOneArgument("haha");
+ assertEquals(TestBean.class, expected.getClass());
+ assertEquals("haha", expected.getName());
+ }
+
+ /**
+ * Creates a new instance of {@link TestBean} using the constructor which takes a <code>String</code> and an <code>int</code>
+ */
+ public void testWithTwoConstructorArg() {
+ AbstractBean bean = (AbstractBean)this.beanFactory.getBean("abstractBean");
+ assertNotNull(bean);
+ final TestBean expected = bean.getTwoArguments("haha", 72);
+ assertEquals(TestBean.class, expected.getClass());
+ assertEquals("haha", expected.getName());
+ assertEquals(72, expected.getAge());
+ }
+
+ /**
+ * {@link TestBean} doesn't have a constructor that takes a <code>String</code> and two <code>int</code>'s
+ */
+ public void testWithThreeArgsShouldFail() {
+ AbstractBean bean = (AbstractBean)this.beanFactory.getBean("abstractBean");
+ assertNotNull(bean);
+ try {
+ bean.getThreeArguments("name", 1, 2);
+ fail("TestBean does not have a three arg constructor so this should not have worked");
+ } catch (AbstractMethodError e) {
+
+ }
+ }
+}
View
19 spring-beans/src/test/java/test/beans/AbstractBean.java
@@ -0,0 +1,19 @@
+package test.beans;
+
+import org.springframework.beans.factory.support.LookupMethodTests;
+
+/**
+ * A simple bean used for testing <code>lookup-method</code> constructors.
+ *
+ * The actual test class which uses this bean is {@link LookupMethodTests}
+ * @author kpietrzak
+ *
+ */
+public abstract class AbstractBean {
+
+ public abstract TestBean get();
+ public abstract TestBean getOneArgument(String name);
+ public abstract TestBean getTwoArguments(String name, int age);
+ public abstract TestBean getThreeArguments(String name, int age, int anotherArg);
+
+}
View
15 spring-beans/src/test/resources/org/springframework/beans/factory/xml/lookupMethodBeanTests.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.1.xsd">
+
+
+ <bean id="abstractBean" class="test.beans.AbstractBean" >
+ <lookup-method name="get" bean="test"/>
+ <lookup-method name="getOneArgument" bean="test"/>
+ <lookup-method name="getTwoArguments" bean="test"/>
+ </bean>
+
+ <bean id="test" class="test.beans.TestBean" scope="prototype">
+ </bean>
+</beans>
Something went wrong with that request. Please try again.