Skip to content

Commit 86c7970

Browse files
mp911demp911de
committed
Consider component names for custom implementation and fragment bean registration.
We now consider the actual bean name when registering custom implementations and repository fragments after falling back to component scan mode. Previously we derived the bean name either from the repository bean name or the class name. Closes #2487.
1 parent b8b0c0f commit 86c7970

File tree

9 files changed

+237
-31
lines changed

9 files changed

+237
-31
lines changed

‎src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Collection;
1919
import java.util.Optional;
2020
import java.util.Set;
21+
import java.util.function.Supplier;
2122
import java.util.stream.Collectors;
2223

2324
import org.springframework.beans.factory.config.BeanDefinition;
@@ -107,8 +108,22 @@ public Optional<AbstractBeanDefinition> detectCustomImplementation(Implementatio
107108
.filter(lookup::matches) //
108109
.collect(StreamUtils.toUnmodifiableSet());
109110

111+
return selectImplementationCandidate(lookup, definitions, () -> {
112+
113+
if (definitions.isEmpty()) {
114+
return Optional.empty();
115+
}
116+
117+
return Optional.of(definitions.iterator().next());
118+
});
119+
}
120+
121+
private static Optional<AbstractBeanDefinition> selectImplementationCandidate(
122+
ImplementationLookupConfiguration lookup, Set<BeanDefinition> definitions,
123+
Supplier<Optional<BeanDefinition>> fallback) {
124+
110125
return SelectionSet //
111-
.of(definitions, c -> c.isEmpty() ? Optional.empty() : throwAmbiguousCustomImplementationException(c)) //
126+
.of(definitions, c -> c.isEmpty() ? fallback.get() : throwAmbiguousCustomImplementationException(c)) //
112127
.filterIfNecessary(lookup::hasMatchingBeanName) //
113128
.uniqueResult() //
114129
.map(AbstractBeanDefinition.class::cast);

‎src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionBuilder.java

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import org.apache.commons.logging.Log;
2424
import org.apache.commons.logging.LogFactory;
25+
2526
import org.springframework.beans.factory.support.AbstractBeanDefinition;
2627
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
2728
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
@@ -137,26 +138,42 @@ private Optional<String> registerCustomImplementation(RepositoryConfiguration<?>
137138

138139
ImplementationLookupConfiguration lookup = configuration.toLookupConfiguration(metadataReaderFactory);
139140

140-
String beanName = lookup.getImplementationBeanName();
141+
String configurationBeanName = lookup.getImplementationBeanName();
141142

142143
// Already a bean configured?
143-
if (registry.containsBeanDefinition(beanName)) {
144-
return Optional.of(beanName);
144+
if (registry.containsBeanDefinition(configurationBeanName)) {
145+
146+
if (logger.isDebugEnabled()) {
147+
logger.debug(String.format("Custom repository implementation already registered: %s", configurationBeanName));
148+
}
149+
150+
return Optional.of(configurationBeanName);
145151
}
146152

147153
Optional<AbstractBeanDefinition> beanDefinition = implementationDetector.detectCustomImplementation(lookup);
148154

149155
return beanDefinition.map(it -> {
150156

151-
if (logger.isDebugEnabled()) {
152-
logger.debug("Registering custom repository implementation: " + lookup.getImplementationBeanName() + " "
153-
+ it.getBeanClassName());
154-
}
155-
157+
String scannedBeanName = configuration.getConfigurationSource().generateBeanName(it);
156158
it.setSource(configuration.getSource());
157-
registry.registerBeanDefinition(beanName, it);
158159

159-
return beanName;
160+
if (registry.containsBeanDefinition(scannedBeanName)) {
161+
162+
if (logger.isDebugEnabled()) {
163+
logger.debug(String.format("Custom repository implementation already registered: %s %s", scannedBeanName,
164+
it.getBeanClassName()));
165+
}
166+
} else {
167+
168+
if (logger.isDebugEnabled()) {
169+
logger.debug(String.format("Registering custom repository implementation: %s %s", scannedBeanName,
170+
it.getBeanClassName()));
171+
}
172+
173+
registry.registerBeanDefinition(scannedBeanName, it);
174+
}
175+
176+
return scannedBeanName;
160177
});
161178
}
162179

@@ -167,19 +184,20 @@ private Stream<RepositoryFragmentConfiguration> registerRepositoryFragmentsImple
167184
.toImplementationDetectionConfiguration(metadataReaderFactory);
168185

169186
return fragmentMetadata.getFragmentInterfaces(configuration.getRepositoryInterface()) //
170-
.map(it -> detectRepositoryFragmentConfiguration(it, config)) //
187+
.map(it -> detectRepositoryFragmentConfiguration(it, config, configuration)) //
171188
.flatMap(Optionals::toStream) //
172189
.peek(it -> potentiallyRegisterFragmentImplementation(configuration, it)) //
173190
.peek(it -> potentiallyRegisterRepositoryFragment(configuration, it));
174191
}
175192

176193
private Optional<RepositoryFragmentConfiguration> detectRepositoryFragmentConfiguration(String fragmentInterface,
177-
ImplementationDetectionConfiguration config) {
194+
ImplementationDetectionConfiguration config, RepositoryConfiguration<?> configuration) {
178195

179196
ImplementationLookupConfiguration lookup = config.forFragment(fragmentInterface);
180197
Optional<AbstractBeanDefinition> beanDefinition = implementationDetector.detectCustomImplementation(lookup);
181198

182-
return beanDefinition.map(bd -> new RepositoryFragmentConfiguration(fragmentInterface, bd));
199+
return beanDefinition.map(bd -> new RepositoryFragmentConfiguration(fragmentInterface, bd,
200+
configuration.getConfigurationSource().generateBeanName(bd)));
183201
}
184202

185203
private void potentiallyRegisterFragmentImplementation(RepositoryConfiguration<?> repositoryConfiguration,
@@ -189,16 +207,21 @@ private void potentiallyRegisterFragmentImplementation(RepositoryConfiguration<?
189207

190208
// Already a bean configured?
191209
if (registry.containsBeanDefinition(beanName)) {
192-
return;
193-
}
194210

195-
if (logger.isDebugEnabled()) {
196-
logger.debug(String.format("Registering repository fragment implementation: %s %s", beanName,
197-
fragmentConfiguration.getClassName()));
211+
if (logger.isDebugEnabled()) {
212+
logger.debug(String.format("Repository fragment implementation already registered: %s", beanName));
213+
}
214+
215+
return;
198216
}
199217

200218
fragmentConfiguration.getBeanDefinition().ifPresent(bd -> {
201219

220+
if (logger.isDebugEnabled()) {
221+
logger.debug(String.format("Registering repository fragment implementation: %s %s", beanName,
222+
fragmentConfiguration.getClassName()));
223+
}
224+
202225
bd.setSource(repositoryConfiguration.getSource());
203226
registry.registerBeanDefinition(beanName, bd);
204227
});
@@ -211,11 +234,16 @@ private void potentiallyRegisterRepositoryFragment(RepositoryConfiguration<?> co
211234

212235
// Already a bean configured?
213236
if (registry.containsBeanDefinition(beanName)) {
237+
238+
if (logger.isDebugEnabled()) {
239+
logger.debug(String.format("RepositoryFragment already registered: %s", beanName));
240+
}
241+
214242
return;
215243
}
216244

217245
if (logger.isDebugEnabled()) {
218-
logger.debug("Registering repository fragment: " + beanName);
246+
logger.debug(String.format("Registering RepositoryFragment: %s", beanName));
219247
}
220248

221249
BeanDefinitionBuilder fragmentBuilder = BeanDefinitionBuilder.rootBeanDefinition(RepositoryFragment.class,

‎src/main/java/org/springframework/data/repository/config/RepositoryFragmentConfiguration.java

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public final class RepositoryFragmentConfiguration {
3636
private final String interfaceName;
3737
private final String className;
3838
private final Optional<AbstractBeanDefinition> beanDefinition;
39+
private final String beanName;
3940

4041
/**
4142
* Creates a {@link RepositoryFragmentConfiguration} given {@code interfaceName} and {@code className} of the
@@ -45,13 +46,7 @@ public final class RepositoryFragmentConfiguration {
4546
* @param className must not be {@literal null} or empty.
4647
*/
4748
public RepositoryFragmentConfiguration(String interfaceName, String className) {
48-
49-
Assert.hasText(interfaceName, "Interface name must not be null or empty!");
50-
Assert.hasText(className, "Class name must not be null or empty!");
51-
52-
this.interfaceName = interfaceName;
53-
this.className = className;
54-
this.beanDefinition = Optional.empty();
49+
this(interfaceName, className, Optional.empty(), generateBeanName(className));
5550
}
5651

5752
/**
@@ -69,20 +64,45 @@ public RepositoryFragmentConfiguration(String interfaceName, AbstractBeanDefinit
6964
this.interfaceName = interfaceName;
7065
this.className = ConfigurationUtils.getRequiredBeanClassName(beanDefinition);
7166
this.beanDefinition = Optional.of(beanDefinition);
67+
this.beanName = generateBeanName();
7268
}
7369

7470
public RepositoryFragmentConfiguration(String interfaceName, String className,
7571
Optional<AbstractBeanDefinition> beanDefinition) {
72+
this(interfaceName, className, beanDefinition, generateBeanName(className));
73+
}
74+
75+
RepositoryFragmentConfiguration(String interfaceName, AbstractBeanDefinition beanDefinition, String beanName) {
76+
this(interfaceName, ConfigurationUtils.getRequiredBeanClassName(beanDefinition), Optional.of(beanDefinition),
77+
beanName);
78+
}
79+
80+
private RepositoryFragmentConfiguration(String interfaceName, String className,
81+
Optional<AbstractBeanDefinition> beanDefinition, String beanName) {
82+
83+
Assert.hasText(interfaceName, "Interface name must not be null or empty!");
84+
Assert.notNull(beanDefinition, "Bean definition must not be null!");
85+
Assert.notNull(beanName, "Bean name must not be null!");
86+
7687
this.interfaceName = interfaceName;
7788
this.className = className;
7889
this.beanDefinition = beanDefinition;
90+
this.beanName = beanName;
91+
}
92+
93+
private String generateBeanName() {
94+
return generateBeanName(getClassName());
95+
}
96+
97+
private static String generateBeanName(String className) {
98+
return Introspector.decapitalize(ClassUtils.getShortName(className));
7999
}
80100

81101
/**
82102
* @return name of the implementation bean.
83103
*/
84104
public String getImplementationBeanName() {
85-
return Introspector.decapitalize(ClassUtils.getShortName(getClassName()));
105+
return this.beanName;
86106
}
87107

88108
/**

‎src/test/java/org/springframework/data/repository/config/RepositoryComponentProviderUnitTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void shouldConsiderNestedRepositoryInterfacesIfEnabled() {
7272
provider.setConsiderNestedRepositoryInterfaces(true);
7373

7474
Set<BeanDefinition> components = provider.findCandidateComponents("org.springframework.data.repository.config");
75-
String nestedRepositoryClassName = "org.springframework.data.repository.config.RepositoryComponentProviderUnitTests$MyNestedRepository";
75+
String nestedRepositoryClassName = "org.springframework.data.repository.config.RepositoryComponentProviderUnitTests$MyNestedRepositoryDefinition";
7676

7777
assertThat(components.size()).isGreaterThanOrEqualTo(1);
7878
assertThat(components).extracting(BeanDefinition::getBeanClassName).contains(nestedRepositoryClassName);
@@ -92,5 +92,5 @@ void exposesBeanDefinitionRegistry() {
9292
assertThat(provider.getRegistry()).isEqualTo(registry);
9393
}
9494

95-
interface MyNestedRepository extends Repository<Person, Long> {}
95+
interface MyNestedRepositoryDefinition extends Repository<Person, Long> {}
9696
}

‎src/test/java/org/springframework/data/repository/config/RepositoryConfigurationDelegateUnitTests.java

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
import org.springframework.core.type.AnnotationMetadata;
4141
import org.springframework.core.type.StandardAnnotationMetadata;
4242
import org.springframework.data.repository.config.RepositoryConfigurationDelegate.LazyRepositoryInjectionPointResolver;
43+
import org.springframework.data.repository.config.annotated.MyAnnotatedRepository;
44+
import org.springframework.data.repository.config.annotated.MyAnnotatedRepositoryImpl;
45+
import org.springframework.data.repository.config.annotated.MyFragmentImpl;
4346
import org.springframework.data.repository.config.excluded.MyOtherRepositoryImpl;
4447
import org.springframework.data.repository.config.stereotype.MyStereotypeRepository;
4548
import org.springframework.data.repository.core.support.DummyRepositoryFactoryBean;
@@ -140,7 +143,8 @@ void considersAnnotatedBeanNamesFromRepository() {
140143
GenericApplicationContext context = new GenericApplicationContext();
141144

142145
RepositoryConfigurationSource configSource = new AnnotationRepositoryConfigurationSource(
143-
AnnotationMetadata.introspect(AnnotatedBeanNamesConfig.class), EnableRepositories.class, context, environment,
146+
AnnotationMetadata.introspect(AnnotatedDerivedBeanNamesConfig.class), EnableRepositories.class, context,
147+
environment,
144148
context.getDefaultListableBeanFactory(), new AnnotationBeanNameGenerator());
145149

146150
RepositoryConfigurationDelegate delegate = new RepositoryConfigurationDelegate(configSource, context, environment);
@@ -151,6 +155,49 @@ void considersAnnotatedBeanNamesFromRepository() {
151155
assertThat(context.getBeanFactory().getBeanDefinition("fooRepositoryImpl")).isNotNull();
152156
}
153157

158+
@Test // GH-2487
159+
void considersAnnotatedBeanNamesFromAtComponent() {
160+
161+
StandardEnvironment environment = new StandardEnvironment();
162+
GenericApplicationContext context = new GenericApplicationContext();
163+
164+
RepositoryConfigurationSource configSource = new AnnotationRepositoryConfigurationSource(
165+
AnnotationMetadata.introspect(AnnotatedBeanNamesConfig.class), EnableRepositories.class, context, environment,
166+
context.getDefaultListableBeanFactory(), new AnnotationBeanNameGenerator());
167+
168+
RepositoryConfigurationDelegate delegate = new RepositoryConfigurationDelegate(configSource, context, environment);
169+
170+
delegate.registerRepositoriesIn(context, extension);
171+
172+
assertThat(context.getBeanFactory().getBeanDefinition("myAnnotatedRepository")).isNotNull();
173+
assertThat(context.getBeanFactory().getBeanDefinition("anotherBeanName")).isNotNull();
174+
assertThat(context.getBeanFactory().getBeanDefinition("fragment")).isNotNull();
175+
assertThat(context.getBeanFactory().getBeanDefinition("fragmentFragment")).isNotNull();
176+
}
177+
178+
@Test // GH-2487
179+
void skipsRegistrationOnAlreadyRegisteredBeansUsingAtComponentNames() {
180+
181+
StandardEnvironment environment = new StandardEnvironment();
182+
GenericApplicationContext context = new GenericApplicationContext();
183+
context.setAllowBeanDefinitionOverriding(false);
184+
context.registerBean("fragment", MyFragmentImpl.class);
185+
context.registerBean("anotherBeanName", MyAnnotatedRepositoryImpl.class);
186+
187+
RepositoryConfigurationSource configSource = new AnnotationRepositoryConfigurationSource(
188+
AnnotationMetadata.introspect(AnnotatedBeanNamesConfig.class), EnableRepositories.class, context, environment,
189+
context.getDefaultListableBeanFactory(), new AnnotationBeanNameGenerator());
190+
191+
RepositoryConfigurationDelegate delegate = new RepositoryConfigurationDelegate(configSource, context, environment);
192+
193+
delegate.registerRepositoriesIn(context, extension);
194+
195+
assertThat(context.getBeanFactory().getBeanDefinition("myAnnotatedRepository")).isNotNull();
196+
assertThat(context.getBeanFactory().getBeanDefinition("anotherBeanName")).isNotNull();
197+
assertThat(context.getBeanFactory().getBeanDefinition("fragment")).isNotNull();
198+
assertThat(context.getBeanFactory().getBeanDefinition("fragmentFragment")).isNotNull();
199+
}
200+
154201
private static ListableBeanFactory assertLazyRepositoryBeanSetup(Class<?> configClass) {
155202

156203
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(configClass);
@@ -191,6 +238,9 @@ static class DefaultBeanNamesConfig {}
191238

192239
@EnableRepositories(basePackageClasses = MyStereotypeRepository.class,
193240
includeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, classes = MyStereotypeRepository.class))
241+
static class AnnotatedDerivedBeanNamesConfig {}
242+
243+
@EnableRepositories(basePackageClasses = MyAnnotatedRepository.class)
194244
static class AnnotatedBeanNamesConfig {}
195245

196246
static class DummyConfigurationExtension extends RepositoryConfigurationExtensionSupport {

‎src/test/java/org/springframework/data/repository/config/annotated/MyAnnotatedRepository.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright 2021 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+
package org.springframework.data.repository.config.annotated;
17+
18+
import org.springframework.data.mapping.Person;
19+
import org.springframework.data.repository.CrudRepository;
20+
21+
/**
22+
* @author Mark Paluch
23+
*/
24+
public interface MyAnnotatedRepository extends CrudRepository<Person, String>, MyFragment {}

‎src/test/java/org/springframework/data/repository/config/annotated/MyAnnotatedRepositoryImpl.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright 2021 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+
package org.springframework.data.repository.config.annotated;
17+
18+
import org.springframework.stereotype.Component;
19+
20+
/**
21+
* @author Mark Paluch
22+
*/
23+
@Component("anotherBeanName")
24+
public class MyAnnotatedRepositoryImpl {}

‎src/test/java/org/springframework/data/repository/config/annotated/MyFragment.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Copyright 2021 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+
package org.springframework.data.repository.config.annotated;
17+
18+
/**
19+
* @author Mark Paluch
20+
*/
21+
interface MyFragment {}

0 commit comments

Comments
 (0)