Skip to content

Commit 0b716c4

Browse files
philwebbmbhavewilkinsona
authored andcommitted
Allow flexible constructor arguments in factory implementations
Update `SpringFactoriesLoader` so that factory implementation classes can have a constructor with arguments that are resolved dynamically. Arguments are resolved using a `ArgumentResolver` interface that is passed to the `loadFactories` method. This strategy interface is intentionally simple and only allows resolution based on the argument type. A number of convenience methods are provided to allow resolvers to be built. For example: ArgumentResolver.of(String.class, "tests") .and(Integer.class, 123); Factory implementation classes must have a non-ambiguous constructor in order to be instantiated. The `SpringFactoriesLoader` uses the same algorithm as `BeanUtils.getResolvableConstructor`. See gh-28057 Co-authored-by: Madhura Bhave <[email protected]> Co-authored-by: Andy Wilkinson <[email protected]>
1 parent ae1956c commit 0b716c4

File tree

7 files changed

+760
-37
lines changed

7 files changed

+760
-37
lines changed

spring-core/src/main/java/org/springframework/core/io/support/SpringFactoriesLoader.java

Lines changed: 316 additions & 35 deletions
Large diffs are not rendered by default.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2002-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.core.io.support;
18+
19+
/**
20+
* Used by {@link SpringFactoriesLoaderTests}.
21+
*
22+
* @author Andy Wilkinson
23+
*/
24+
class ConstructorArgsDummyFactory implements DummyFactory {
25+
26+
private final String string;
27+
28+
public ConstructorArgsDummyFactory(String string) {
29+
this(string, 0);
30+
}
31+
32+
private ConstructorArgsDummyFactory(String string, int reasonCode) {
33+
this.string = string;
34+
}
35+
36+
@Override
37+
public String getString() {
38+
return this.string;
39+
}
40+
41+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright 2002-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.core.io.support;
18+
19+
/**
20+
* Used by {@link SpringFactoriesLoaderTests}.
21+
*
22+
* @author Madhura Bhave
23+
*/
24+
class MultipleConstructorArgsDummyFactory implements DummyFactory {
25+
26+
private final String string;
27+
28+
private final Integer age;
29+
30+
MultipleConstructorArgsDummyFactory(String string) {
31+
this(string, null);
32+
}
33+
34+
MultipleConstructorArgsDummyFactory(String string, Integer age) {
35+
this.string = string;
36+
this.age = age;
37+
}
38+
39+
40+
@Override
41+
public String getString() {
42+
return this.string + this.age;
43+
}
44+
45+
}

spring-core/src/test/java/org/springframework/core/io/support/SpringFactoriesLoaderTests.java

Lines changed: 241 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,22 +16,33 @@
1616

1717
package org.springframework.core.io.support;
1818

19+
import java.io.File;
1920
import java.lang.reflect.Modifier;
21+
import java.net.MalformedURLException;
22+
import java.net.URL;
23+
import java.net.URLClassLoader;
2024
import java.util.List;
2125

2226
import org.junit.jupiter.api.AfterAll;
2327
import org.junit.jupiter.api.BeforeAll;
28+
import org.junit.jupiter.api.Nested;
2429
import org.junit.jupiter.api.Test;
2530

31+
import org.springframework.core.io.support.SpringFactoriesLoader.ArgumentResolver;
32+
import org.springframework.core.io.support.SpringFactoriesLoader.FactoryInstantiator;
33+
2634
import static org.assertj.core.api.Assertions.assertThat;
2735
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
36+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2837

2938
/**
3039
* Tests for {@link SpringFactoriesLoader}.
3140
*
3241
* @author Arjen Poutsma
3342
* @author Phillip Webb
3443
* @author Sam Brannen
44+
* @author Andy Wilkinson
45+
* @author Madhura Bhave
3546
*/
3647
class SpringFactoriesLoaderTests {
3748

@@ -43,9 +54,11 @@ static void clearCache() {
4354

4455
@AfterAll
4556
static void checkCache() {
46-
assertThat(SpringFactoriesLoader.cache).hasSize(1);
57+
assertThat(SpringFactoriesLoader.cache).hasSize(3);
58+
SpringFactoriesLoader.cache.clear();
4759
}
4860

61+
4962
@Test
5063
void loadFactoryNames() {
5164
List<String> factoryNames = SpringFactoriesLoader.loadFactoryNames(DummyFactory.class, null);
@@ -82,4 +95,230 @@ void attemptToLoadFactoryOfIncompatibleType() {
8295
+ "[org.springframework.core.io.support.MyDummyFactory1] for factory type [java.lang.String]");
8396
}
8497

98+
@Test
99+
void loadFactoryWithNonDefaultConstructor() {
100+
ArgumentResolver resolver = ArgumentResolver.of(String.class, "injected");
101+
List<DummyFactory> factories = SpringFactoriesLoader.loadFactories(DummyFactory.class, LimitedClassLoader.constructorArgumentFactories, resolver);
102+
assertThat(factories).hasSize(3);
103+
assertThat(factories.get(0)).isInstanceOf(MyDummyFactory1.class);
104+
assertThat(factories.get(1)).isInstanceOf(MyDummyFactory2.class);
105+
assertThat(factories.get(2)).isInstanceOf(ConstructorArgsDummyFactory.class);
106+
assertThat(factories).extracting(DummyFactory::getString).containsExactly("Foo", "Bar", "injected");
107+
}
108+
109+
@Test
110+
void loadFactoryWithMultipleConstructors() {
111+
ArgumentResolver resolver = ArgumentResolver.of(String.class, "injected");
112+
assertThatIllegalArgumentException()
113+
.isThrownBy(() -> SpringFactoriesLoader.loadFactories(DummyFactory.class, LimitedClassLoader.multipleArgumentFactories, resolver))
114+
.withMessageContaining("Unable to instantiate factory class "
115+
+ "[org.springframework.core.io.support.MultipleConstructorArgsDummyFactory] for factory type [org.springframework.core.io.support.DummyFactory]")
116+
.havingRootCause().withMessageContaining("Class [org.springframework.core.io.support.MultipleConstructorArgsDummyFactory] has no suitable constructor");
117+
}
118+
119+
120+
@Nested
121+
class ArgumentResolverTests {
122+
123+
@Test
124+
void ofValueResolvesValue() {
125+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test");
126+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
127+
assertThat(resolver.resolve(String.class)).isNull();
128+
assertThat(resolver.resolve(Integer.class)).isNull();
129+
}
130+
131+
@Test
132+
void ofValueSupplierResolvesValue() {
133+
ArgumentResolver resolver = ArgumentResolver.ofSupplied(CharSequence.class, () -> "test");
134+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
135+
assertThat(resolver.resolve(String.class)).isNull();
136+
assertThat(resolver.resolve(Integer.class)).isNull();
137+
}
138+
139+
@Test
140+
void fromAdaptsFunction() {
141+
ArgumentResolver resolver = ArgumentResolver.from(
142+
type -> CharSequence.class.equals(type) ? "test" : null);
143+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
144+
assertThat(resolver.resolve(String.class)).isNull();
145+
assertThat(resolver.resolve(Integer.class)).isNull();
146+
}
147+
148+
@Test
149+
void andValueReturnsComposite() {
150+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test").and(Integer.class, 123);
151+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
152+
assertThat(resolver.resolve(String.class)).isNull();
153+
assertThat(resolver.resolve(Integer.class)).isEqualTo(123);
154+
}
155+
156+
@Test
157+
void andValueWhenSameTypeReturnsCompositeResolvingFirst() {
158+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test").and(CharSequence.class, "ignore");
159+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
160+
}
161+
162+
@Test
163+
void andValueSupplierReturnsComposite() {
164+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test").andSupplied(Integer.class, () -> 123);
165+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
166+
assertThat(resolver.resolve(String.class)).isNull();
167+
assertThat(resolver.resolve(Integer.class)).isEqualTo(123);
168+
}
169+
170+
@Test
171+
void andValueSupplierWhenSameTypeReturnsCompositeResolvingFirst() {
172+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test").andSupplied(CharSequence.class, () -> "ignore");
173+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
174+
}
175+
176+
@Test
177+
void andResolverReturnsComposite() {
178+
ArgumentResolver resolver = ArgumentResolver.of(CharSequence.class, "test").and(Integer.class, 123);
179+
resolver = resolver.and(ArgumentResolver.of(CharSequence.class, "ignore").and(Long.class, 234L));
180+
assertThat(resolver.resolve(CharSequence.class)).isEqualTo("test");
181+
assertThat(resolver.resolve(String.class)).isNull();
182+
assertThat(resolver.resolve(Integer.class)).isEqualTo(123);
183+
assertThat(resolver.resolve(Long.class)).isEqualTo(234L);
184+
}
185+
186+
}
187+
188+
@Nested
189+
class FactoryInstantiatorTests {
190+
191+
private final ArgumentResolver resolver = ArgumentResolver.of(String.class, "test");
192+
193+
@Test
194+
void defaultConstructorCreatesInstance() throws Exception {
195+
Object instance = FactoryInstantiator.forClass(
196+
DefaultConstructor.class).instantiate(this.resolver);
197+
assertThat(instance).isNotNull();
198+
}
199+
200+
@Test
201+
void singleConstructorWithArgumentsCreatesInstance() throws Exception {
202+
Object instance = FactoryInstantiator.forClass(
203+
SingleConstructor.class).instantiate(this.resolver);
204+
assertThat(instance).isNotNull();
205+
}
206+
207+
@Test
208+
void multiplePrivateAndSinglePublicConstructorCreatesInstance() throws Exception {
209+
Object instance = FactoryInstantiator.forClass(
210+
MultiplePrivateAndSinglePublicConstructor.class).instantiate(this.resolver);
211+
assertThat(instance).isNotNull();
212+
}
213+
214+
@Test
215+
void multiplePackagePrivateAndSinglePublicConstructorCreatesInstance() throws Exception {
216+
Object instance = FactoryInstantiator.forClass(
217+
MultiplePackagePrivateAndSinglePublicConstructor.class).instantiate(this.resolver);
218+
assertThat(instance).isNotNull();
219+
}
220+
221+
@Test
222+
void singlePackagePrivateConstructorCreatesInstance() throws Exception {
223+
Object instance = FactoryInstantiator.forClass(
224+
SinglePackagePrivateConstructor.class).instantiate(this.resolver);
225+
assertThat(instance).isNotNull();
226+
}
227+
228+
@Test
229+
void singlePrivateConstructorCreatesInstance() throws Exception {
230+
Object instance = FactoryInstantiator.forClass(
231+
SinglePrivateConstructor.class).instantiate(this.resolver);
232+
assertThat(instance).isNotNull();
233+
}
234+
235+
@Test
236+
void multiplePackagePrivateConstructorsThrowsException() throws Exception {
237+
assertThatIllegalStateException().isThrownBy(
238+
() -> FactoryInstantiator.forClass(MultiplePackagePrivateConstructors.class))
239+
.withMessageContaining("has no suitable constructor");
240+
}
241+
242+
static class DefaultConstructor {
243+
244+
}
245+
246+
static class SingleConstructor {
247+
248+
SingleConstructor(String arg) {
249+
}
250+
251+
}
252+
253+
static class MultiplePrivateAndSinglePublicConstructor {
254+
255+
public MultiplePrivateAndSinglePublicConstructor(String arg) {
256+
this(arg, false);
257+
}
258+
259+
private MultiplePrivateAndSinglePublicConstructor(String arg, boolean extra) {
260+
}
261+
262+
}
263+
264+
static class MultiplePackagePrivateAndSinglePublicConstructor {
265+
266+
public MultiplePackagePrivateAndSinglePublicConstructor(String arg) {
267+
this(arg, false);
268+
}
269+
270+
MultiplePackagePrivateAndSinglePublicConstructor(String arg, boolean extra) {
271+
}
272+
273+
}
274+
275+
276+
static class SinglePackagePrivateConstructor {
277+
278+
SinglePackagePrivateConstructor(String arg) {
279+
}
280+
281+
}
282+
283+
static class SinglePrivateConstructor {
284+
285+
private SinglePrivateConstructor(String arg) {
286+
}
287+
288+
}
289+
290+
static class MultiplePackagePrivateConstructors {
291+
292+
MultiplePackagePrivateConstructors(String arg) {
293+
this(arg, false);
294+
}
295+
296+
MultiplePackagePrivateConstructors(String arg, boolean extra) {
297+
}
298+
299+
}
300+
301+
}
302+
303+
private static class LimitedClassLoader extends URLClassLoader {
304+
305+
private static final ClassLoader constructorArgumentFactories = new LimitedClassLoader("constructor-argument-factories");
306+
307+
private static final ClassLoader multipleArgumentFactories = new LimitedClassLoader("multiple-arguments-factories");
308+
309+
LimitedClassLoader(String location) {
310+
super(new URL[] { toUrl(location) });
311+
}
312+
313+
private static URL toUrl(String location) {
314+
try {
315+
return new File("src/test/resources/org/springframework/core/io/support/" + location + "/").toURI().toURL();
316+
}
317+
catch (MalformedURLException ex) {
318+
throw new IllegalStateException(ex);
319+
}
320+
}
321+
322+
}
323+
85324
}

0 commit comments

Comments
 (0)