Introduce PropertiesBasedNamedQueriesFactoryBean to reduce bean indirections

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-commons</artifactId>
-	<version>3.0.0-SNAPSHOT</version>
+	<version>3.0.0-GH-2584-SNAPSHOT</version>
 
 	<name>Spring Data Core</name>
 	<description>Core Spring concepts underpinning every Spring Data module.</description>

src/main/java/org/springframework/data/repository/config/NamedQueriesBeanDefinitionBuilder.java

@@ -16,19 +16,18 @@
 package org.springframework.data.repository.config;
 
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.beans.factory.config.PropertiesFactoryBean;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.data.repository.core.NamedQueries;
-import org.springframework.data.repository.core.support.PropertiesBasedNamedQueries;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
- * Builder to create a {@link BeanDefinition} for a {@link NamedQueries} instance.
+ * Builder to create a {@link BeanDefinition} for a {@link NamedQueries} instance using properties.
  *
  * @author Oliver Gierke
+ * @author Mark Paluch
  */
 public class NamedQueriesBeanDefinitionBuilder {
 
@@ -67,21 +66,15 @@ public void setLocations(String locations) {
 	 */
 	public BeanDefinition build(@Nullable Object source) {
 
-		BeanDefinitionBuilder properties = BeanDefinitionBuilder.rootBeanDefinition(PropertiesFactoryBean.class);
-
+		BeanDefinitionBuilder namedQueries = BeanDefinitionBuilder
+				.rootBeanDefinition(PropertiesBasedNamedQueriesFactoryBean.class);
 		String locationsToUse = StringUtils.hasText(locations) ? locations : defaultLocation;
-		properties.addPropertyValue("locations", locationsToUse);
+		namedQueries.addPropertyValue("locations", locationsToUse);
 
 		if (!StringUtils.hasText(locations)) {
-			properties.addPropertyValue("ignoreResourceNotFound", true);
+			namedQueries.addPropertyValue("ignoreResourceNotFound", true);
 		}
 
-		AbstractBeanDefinition propertiesDefinition = properties.getBeanDefinition();
-		propertiesDefinition.setSource(source);
-
-		BeanDefinitionBuilder namedQueries = BeanDefinitionBuilder.rootBeanDefinition(PropertiesBasedNamedQueries.class);
-		namedQueries.addConstructorArgValue(propertiesDefinition);
-
 		AbstractBeanDefinition namedQueriesDefinition = namedQueries.getBeanDefinition();
 		namedQueriesDefinition.setSource(source);
 

src/main/java/org/springframework/data/repository/config/NamedQueriesBeanDefinitionParser.java

@@ -18,13 +18,11 @@
 import java.util.Properties;
 
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.beans.factory.config.PropertiesFactoryBean;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.xml.BeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.data.repository.core.NamedQueries;
-import org.springframework.data.repository.core.support.PropertiesBasedNamedQueries;
 import org.springframework.lang.NonNull;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -36,6 +34,7 @@
  * {@link Properties} file fom the given location.
  *
  * @author Oliver Gierke
+ * @author Mark Paluch
  */
 public class NamedQueriesBeanDefinitionParser implements BeanDefinitionParser {
 
@@ -55,19 +54,14 @@ public NamedQueriesBeanDefinitionParser(String defaultLocation) {
 	@NonNull
 	public BeanDefinition parse(Element element, ParserContext parserContext) {
 
-		BeanDefinitionBuilder properties = BeanDefinitionBuilder.rootBeanDefinition(PropertiesFactoryBean.class);
-		properties.addPropertyValue("locations", getDefaultedLocation(element));
+		BeanDefinitionBuilder namedQueries = BeanDefinitionBuilder
+				.rootBeanDefinition(PropertiesBasedNamedQueriesFactoryBean.class);
+		namedQueries.addPropertyValue("locations", getDefaultedLocation(element));
 
 		if (isDefaultLocation(element)) {
-			properties.addPropertyValue("ignoreResourceNotFound", true);
+			namedQueries.addPropertyValue("ignoreResourceNotFound", true);
 		}
 
-		AbstractBeanDefinition propertiesDefinition = properties.getBeanDefinition();
-		propertiesDefinition.setSource(parserContext.extractSource(element));
-
-		BeanDefinitionBuilder namedQueries = BeanDefinitionBuilder.rootBeanDefinition(PropertiesBasedNamedQueries.class);
-		namedQueries.addConstructorArgValue(propertiesDefinition);
-
 		AbstractBeanDefinition namedQueriesDefinition = namedQueries.getBeanDefinition();
 		namedQueriesDefinition.setSource(parserContext.extractSource(element));
 

src/main/java/org/springframework/data/repository/config/PropertiesBasedNamedQueriesFactoryBean.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2022 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
+ *
+ *      https://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.data.repository.config;
+
+import java.io.IOException;
+import java.util.Properties;
+
+import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.core.io.support.PropertiesLoaderSupport;
+import org.springframework.data.repository.core.support.PropertiesBasedNamedQueries;
+import org.springframework.lang.Nullable;
+
+/**
+ * Factory bean to create {@link PropertiesBasedNamedQueries}.
+ * <p>
+ * Supports loading from a properties file and/or setting local properties on this FactoryBean. The created Properties
+ * instance will be merged from loaded and local values. If neither a location nor local properties are set, an
+ * exception will be thrown on initialization.
+ * <p>
+ * Can create a singleton or a new object on each request. Default is a singleton.
+ *
+ * @author Mark Paluch
+ * @since 3.0
+ */
+public class PropertiesBasedNamedQueriesFactoryBean extends PropertiesLoaderSupport
+		implements FactoryBean<PropertiesBasedNamedQueries>, InitializingBean {
+
+	private boolean singleton = true;
+
+	private @Nullable PropertiesBasedNamedQueries singletonInstance;
+
+	/**
+	 * Set whether a shared singleton {@code PropertiesBasedNamedQueries} instance should be created, or rather a new
+	 * {@code PropertiesBasedNamedQueries} instance on each request.
+	 * <p>
+	 * Default is {@code true} (a shared singleton).
+	 */
+	public void setSingleton(boolean singleton) {
+		this.singleton = singleton;
+	}
+
+	@Override
+	public boolean isSingleton() {
+		return this.singleton;
+	}
+
+	@Override
+	public void afterPropertiesSet() throws IOException {
+		if (this.singleton) {
+			this.singletonInstance = new PropertiesBasedNamedQueries(createProperties());
+		}
+	}
+
+	@Override
+	@Nullable
+	public PropertiesBasedNamedQueries getObject() throws IOException {
+		if (this.singleton) {
+			return this.singletonInstance;
+		} else {
+			return new PropertiesBasedNamedQueries(createProperties());
+		}
+	}
+
+	@Override
+	public Class<PropertiesBasedNamedQueries> getObjectType() {
+		return PropertiesBasedNamedQueries.class;
+	}
+
+	protected Properties createProperties() throws IOException {
+		return mergeProperties();
+	}
+}

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

@@ -15,16 +15,20 @@
  */
 package org.springframework.data.repository.config;
 
+import static org.springframework.beans.factory.config.BeanDefinition.*;
+
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
+import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.BeanDefinitionReaderUtils;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.core.env.Environment;
 import org.springframework.core.io.ResourceLoader;
@@ -35,6 +39,7 @@
 import org.springframework.data.repository.core.support.RepositoryFragmentsFactoryBean;
 import org.springframework.data.util.Optionals;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 
 /**
  * Builder to create {@link BeanDefinitionBuilder} instance to eventually create Spring Data repository instances.
@@ -119,17 +124,8 @@ public BeanDefinitionBuilder build(RepositoryConfiguration<?> configuration) {
 			builder.addDependsOn(it);
 		});
 
-		BeanDefinitionBuilder fragmentsBuilder = BeanDefinitionBuilder
-				.rootBeanDefinition(RepositoryFragmentsFactoryBean.class);
-
-		List<String> fragmentBeanNames = registerRepositoryFragmentsImplementation(configuration) //
-				.map(RepositoryFragmentConfiguration::getFragmentBeanName) //
-				.collect(Collectors.toList());
-
-		fragmentsBuilder.addConstructorArgValue(fragmentBeanNames);
-
-		builder.addPropertyValue("repositoryFragments",
-				ParsingUtils.getSourceBeanDefinition(fragmentsBuilder, configuration.getSource()));
+		String fragmentsBeanName = registerRepositoryFragments(configuration);
+		builder.addPropertyValue("repositoryFragments", new RuntimeBeanReference(fragmentsBeanName));
 
 		return builder;
 	}
@@ -161,6 +157,26 @@ private Optional<String> registerCustomImplementation(RepositoryConfiguration<?>
 		});
 	}
 
+	private String registerRepositoryFragments(RepositoryConfiguration<?> configuration) {
+
+		BeanDefinitionBuilder fragmentsBuilder = BeanDefinitionBuilder
+				.rootBeanDefinition(RepositoryFragmentsFactoryBean.class) //
+				.setRole(ROLE_INFRASTRUCTURE);
+
+		List<String> fragmentBeanNames = registerRepositoryFragmentsImplementation(configuration) //
+				.map(RepositoryFragmentConfiguration::getFragmentBeanName) //
+				.collect(Collectors.toList());
+
+		fragmentsBuilder.addConstructorArgValue(fragmentBeanNames);
+
+		String fragmentsBeanName = BeanDefinitionReaderUtils
+				.uniqueBeanName(String.format("%s.%s.fragments", extension.getModuleName().toLowerCase(Locale.ROOT),
+						ClassUtils.getShortName(configuration.getRepositoryInterface())), registry);
+		registry.registerBeanDefinition(fragmentsBeanName, fragmentsBuilder.getBeanDefinition());
+		return fragmentsBeanName;
+	}
+
+
 	private Stream<RepositoryFragmentConfiguration> registerRepositoryFragmentsImplementation(
 			RepositoryConfiguration<?> configuration) {
 

src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java

@@ -78,6 +78,16 @@ void registersBeanDefinitionForFoundBean() {
 		assertNoBeanDefinitionRegisteredFor("profileRepository");
 	}
 
+
+	@Test // GH-2584
+	void shouldExposeFragmentsAsBean() {
+
+		AnnotationMetadata metadata = new StandardAnnotationMetadata(SampleConfiguration.class, true);
+
+		registrar.registerBeanDefinitions(metadata, registry);
+		verify(registry, atLeast(1)).registerBeanDefinition(eq("commons.MyRepository.fragments#0"), any(BeanDefinition.class));
+	}
+
 	@Test // DATACMNS-1754
 	void registersBeanDefinitionForNestedRepositories() {