DATAJDBC-293 - Polishing.

Removed most usage of the Stream API.
Improved error messages.
Added tests.
Code formatting.

Original pull request: #102.

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java

@@ -15,20 +15,16 @@
  */
 package org.springframework.data.jdbc.repository.config;
 
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
-import java.util.function.Function;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 
+import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
-import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
@@ -37,6 +33,8 @@ import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactoryBea
 import org.springframework.data.repository.config.RepositoryConfigurationExtensionSupport;
 import org.springframework.data.repository.config.RepositoryConfigurationSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
@@ -49,7 +47,8 @@ import org.springframework.util.StringUtils;
  */
 public class JdbcRepositoryConfigExtension extends RepositoryConfigurationExtensionSupport {
 
-	private ConfigurableListableBeanFactory listableBeanFactory;
+	private ListableBeanFactory beanFactory;
+
 	/*
 	* (non-Javadoc)
 	* @see org.springframework.data.repository.config.RepositoryConfigurationExtension#getModuleName()
@@ -58,7 +57,6 @@ public class JdbcRepositoryConfigExtension extends RepositoryConfigurationExtens
 	public String getModuleName() {
 		return "JDBC";
 	}
-	
 
 	/*
 	* (non-Javadoc)
@@ -82,83 +80,110 @@ public class JdbcRepositoryConfigExtension extends RepositoryConfigurationExtens
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.config.RepositoryConfigurationExtension#registerBeansForRoot(org.springframework.beans.factory.support.BeanDefinitionRegistry, org.springframework.data.repository.config.RepositoryConfigurationSource)
 	 */
-	public void registerBeansForRoot(BeanDefinitionRegistry registry,
-			RepositoryConfigurationSource configurationSource) {
-		if (registry instanceof ConfigurableListableBeanFactory) {
-			this.listableBeanFactory =   (ConfigurableListableBeanFactory) registry;
+	public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConfigurationSource configurationSource) {
+
+		if (registry instanceof ListableBeanFactory) {
+			this.beanFactory = (ListableBeanFactory) registry;
 		}
 	}
 
-
 	/* 
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.config.RepositoryConfigurationExtensionSupport#postProcess(org.springframework.beans.factory.support.BeanDefinitionBuilder, org.springframework.data.repository.config.RepositoryConfigurationSource)
 	 */
 	@Override
 	public void postProcess(BeanDefinitionBuilder builder, RepositoryConfigurationSource source) {
-		resolveReference(builder, source, "jdbcOperationsRef", "jdbcOperations", NamedParameterJdbcOperations.class,
-				true);
-		resolveReference(builder, source, "dataAccessStrategyRef", "dataAccessStrategy", DataAccessStrategy.class,
-				false);
+
+		resolveReference(builder, source, "jdbcOperationsRef", "jdbcOperations", NamedParameterJdbcOperations.class, true);
+		resolveReference(builder, source, "dataAccessStrategyRef", "dataAccessStrategy", DataAccessStrategy.class, false);
 	}
 
 	private void resolveReference(BeanDefinitionBuilder builder, RepositoryConfigurationSource source,
 			String attributeName, String propertyName, Class<?> classRef, boolean required) {
+
 		Optional<String> beanNameRef = source.getAttribute(attributeName).filter(StringUtils::hasText);
 
-		String beanName = beanNameRef.orElseGet(() -> {
-			if (this.listableBeanFactory != null) {
-				List<String> beanNames = Arrays.asList(listableBeanFactory.getBeanNamesForType(classRef));
-				Map<String, BeanDefinition> bdMap = beanNames.stream()
-						.collect(Collectors.toMap(Function.identity(), listableBeanFactory::getBeanDefinition));
+		String beanName = beanNameRef.orElseGet(() -> determineMatchingBeanName(propertyName, classRef, required));
+
+		if (beanName != null) {
+			builder.addPropertyReference(propertyName, beanName);
+		} else {
+			Assert.isTrue(!required,
+					"The beanName must not be null when requested as 'required'. Please report this as a bug.");
+		}
+
+	}
+
+	@Nullable
+	private String determineMatchingBeanName(String propertyName, Class<?> classRef, boolean required) {
+
+		if (this.beanFactory == null) {
+			return nullOrThrowException(required,
+					() -> new NoSuchBeanDefinitionException(classRef, "No BeanFactory available."));
+		}
+
+		List<String> beanNames = Arrays.asList(beanFactory.getBeanNamesForType(classRef));
 
-				if (beanNames.size() > 1) {
-					// determine primary
+		if (beanNames.isEmpty()) {
+			return nullOrThrowException(required,
+					() -> new NoSuchBeanDefinitionException(classRef, String.format("No bean of type %s available", classRef)));
+		}
 
-					Map<String, BeanDefinition> primaryBdMap = bdMap.entrySet().stream()
-							.filter(e -> e.getValue().isPrimary())
-							.collect(Collectors.toMap(Entry::getKey, Entry::getValue));
+		if (beanNames.size() == 1) {
+			return beanNames.get(0);
+		}
 
-					Optional<String> primaryBeanName = getSingleBeanName(primaryBdMap.keySet(), classRef,
-							() -> "more than one 'primary' bean found among candidates: " + primaryBdMap.keySet());
+		if (!(beanFactory instanceof ConfigurableListableBeanFactory)) {
 
-					// In Java 11 should use Optional.or()
-					if (primaryBeanName.isPresent()) {
-						return primaryBeanName.get();
-					}
+			return nullOrThrowException(required,
+					() -> new NoSuchBeanDefinitionException(String.format(
+							"BeanFactory does not implement ConfigurableListableBeanFactory when trying to find bean of type %s.",
+							classRef)));
+		}
 
-					// determine matchesBeanName
+		List<String> primaryBeanNames = getPrimaryBeanDefinitions(beanNames, (ConfigurableListableBeanFactory) beanFactory);
 
-					Optional<String> matchesBeanName = beanNames.stream()
-							.filter(name -> propertyName.equals(name)
-									|| ObjectUtils.containsElement(listableBeanFactory.getAliases(name), propertyName))
-							.findFirst();
+		if (primaryBeanNames.size() == 1) {
+			return primaryBeanNames.get(0);
+		}
 
-					if (matchesBeanName.isPresent()) {
-						return matchesBeanName.get();
-					}
+		if (primaryBeanNames.size() > 1) {
+			throw new NoUniqueBeanDefinitionException(classRef, primaryBeanNames.size(),
+					"more than one 'primary' bean found among candidates: " + primaryBeanNames);
+		}
 
-				}
+		for (String beanName : beanNames) {
 
-				if (beanNames.size() == 1) {
-					return beanNames.get(0);
-				}
+			if (propertyName.equals(beanName)
+					|| ObjectUtils.containsElement(beanFactory.getAliases(beanName), propertyName)) {
+				return beanName;
 			}
-			return null;
-		});
-		if (beanName != null) {
-			builder.addPropertyReference(propertyName, beanName);
-		} else if (required) {
-			throw new NoSuchBeanDefinitionException(classRef);
 		}
+
+		return nullOrThrowException(required,
+				() -> new NoSuchBeanDefinitionException(String.format("No bean of name %s found.", propertyName)));
 	}
 
-	private Optional<String> getSingleBeanName(Collection<String> beanNames, Class<?> classRef,
-			Supplier<String> errorMessage) {
-		if (beanNames.size() > 1) {
-			throw new NoUniqueBeanDefinitionException(classRef, beanNames.size(), errorMessage.get());
+	private static List<String> getPrimaryBeanDefinitions(List<String> beanNames,
+			ConfigurableListableBeanFactory beanFactory) {
+
+		ArrayList<String> primaryBeanNames = new ArrayList<>();
+		for (String name : beanNames) {
+
+			if (beanFactory.getBeanDefinition(name).isPrimary()) {
+				primaryBeanNames.add(name);
+			}
 		}
+		return primaryBeanNames;
+	}
 
-		return beanNames.stream().findFirst();
+	@Nullable
+	private static String nullOrThrowException(boolean required, Supplier<RuntimeException> exception) {
+
+		if (required) {
+			throw exception.get();
+		}
+		return null;
 	}
+
 }

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtensionUnitTests.java

@@ -0,0 +1,148 @@
+/*
+ * Copyright 2018 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.data.jdbc.repository.config;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.Test;
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import org.springframework.beans.factory.config.RuntimeBeanReference;
+import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
+import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.data.repository.config.RepositoryConfigurationSource;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
+
+/**
+ * @author Jens Schauder
+ */
+public class JdbcRepositoryConfigExtensionUnitTests {
+
+	BeanDefinitionBuilder definitionBuilder = BeanDefinitionBuilder.genericBeanDefinition();
+	RepositoryConfigurationSource configSource = mock(RepositoryConfigurationSource.class);
+	DefaultListableBeanFactory listableBeanFactory = new DefaultListableBeanFactory();
+
+	JdbcRepositoryConfigExtension extension = new JdbcRepositoryConfigExtension();
+
+	@Test // DATAJDBC-293
+	public void exceptionIsThrownOnPostProcessIfNoBeanFactoryIsAvailable() {
+
+		assertThatThrownBy( //
+				() -> extension.postProcess(definitionBuilder, configSource)) //
+						.isInstanceOf(NoSuchBeanDefinitionException.class) //
+						.hasMessageContaining("No BeanFactory");
+
+	}
+
+	@Test // DATAJDBC-293
+	public void exceptionIsThrownOnPostProcessIfNoJdbcOperationsBeanIsAvailable() {
+
+		extension.registerBeansForRoot(listableBeanFactory, null);
+
+		assertThatThrownBy( //
+				() -> extension.postProcess(definitionBuilder, configSource)) //
+						.isInstanceOf(NoSuchBeanDefinitionException.class) //
+						.hasMessageContaining("NamedParameterJdbcOperations"); //
+
+	}
+
+	@Test // DATAJDBC-293
+	public void exceptionIsThrownOnPostProcessIfMultipleJdbcOperationsBeansAreAvailableAndNoConfigurableBeanFactoryAvailable() {
+
+		GenericApplicationContext applicationContext = new GenericApplicationContext();
+
+		applicationContext.registerBean( //
+				"one", //
+				NamedParameterJdbcOperations.class, //
+				() -> mock(NamedParameterJdbcOperations.class));
+		applicationContext.registerBean( //
+				"two", //
+				NamedParameterJdbcOperations.class, //
+				() -> mock(NamedParameterJdbcOperations.class));
+
+		applicationContext.refresh();
+
+		extension.registerBeansForRoot(applicationContext, null);
+
+		assertThatThrownBy( //
+				() -> extension.postProcess(definitionBuilder, configSource)) //
+						.isInstanceOf(NoSuchBeanDefinitionException.class) //
+						.hasMessageContaining("NamedParameterJdbcOperations"); //
+
+	}
+
+	@Test // DATAJDBC-293
+	public void exceptionIsThrownOnPostProcessIfMultiplePrimaryNoJdbcOperationsBeansAreAvailable() {
+
+		registerJdbcOperations("one", true);
+		registerJdbcOperations("two", true);
+
+		extension.registerBeansForRoot(listableBeanFactory, null);
+
+		assertThatThrownBy( //
+				() -> extension.postProcess(definitionBuilder, configSource)) //
+						.isInstanceOf(NoSuchBeanDefinitionException.class) //
+						.hasMessageContaining("NamedParameterJdbcOperations"); //
+
+	}
+
+	@Test // DATAJDBC-293
+	public void uniquePrimaryBeanIsUsedOfNamedParameterJdbcOperations() {
+
+		registerJdbcOperations("one", false);
+		registerJdbcOperations("two", true);
+
+		extension.registerBeansForRoot(listableBeanFactory, null);
+
+		extension.postProcess(definitionBuilder, configSource);
+
+		Object jdbcOperations = definitionBuilder.getBeanDefinition().getPropertyValues().get("jdbcOperations");
+
+		assertThat(jdbcOperations) //
+				.isInstanceOf(RuntimeBeanReference.class) //
+				.extracting(rbr -> ((RuntimeBeanReference) rbr).getBeanName()).contains("two");
+
+		System.out.println(jdbcOperations);
+	}
+
+	@Test // DATAJDBC-293
+	public void matchesByNameAsLastResort() {
+
+		registerJdbcOperations("jdbcOperations", false);
+		registerJdbcOperations("two", false);
+
+		extension.registerBeansForRoot(listableBeanFactory, null);
+
+		extension.postProcess(definitionBuilder, configSource);
+
+		Object jdbcOperations = definitionBuilder.getBeanDefinition().getPropertyValues().get("jdbcOperations");
+
+		assertThat(jdbcOperations) //
+				.isInstanceOf(RuntimeBeanReference.class) //
+				.extracting(rbr -> ((RuntimeBeanReference) rbr).getBeanName()).contains("jdbcOperations");
+
+	}
+
+	private void registerJdbcOperations(String name, boolean primary) {
+
+		listableBeanFactory.registerBeanDefinition(name, BeanDefinitionBuilder.genericBeanDefinition( //
+				NamedParameterJdbcOperations.class, //
+				() -> mock(NamedParameterJdbcOperations.class)) //
+				.applyCustomizers(bd -> bd.setPrimary(primary)) //
+				.getBeanDefinition());
+	}
+}

spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DbActionExecutionExceptionUnitTests.java

@@ -20,9 +20,11 @@ import static org.mockito.Mockito.*;
 import org.junit.Test;
 
 /**
+ * Unit test for {@link DbActionExecutionException}.
+ * 
  * @author Jens Schauder
  */
-public class DbActionExecutionExceptionTest {
+public class DbActionExecutionExceptionUnitTests {
 
 	@Test // DATAJDBC-162
 	public void constructorWorksWithNullPropertyPath() {