DATACMNS-764 - Disambiguate custom repository implementation if necessary

When multiple repository implementations are found based on the class name, the one with a bean name matching the interfaces bean name + implementation postfix is picked. Includes support for CDI. RepositoryBeanNameGenerator now no longer implements BeanNameGenerator since while it produces names it does not behave like the interface suggests, i.e. it can work without a BeanFactory in the first place. It now uses constructor injection and is package private. Original pull request: #201.
2017-03-07 17:06:58 +01:00
parent 628d71cf1f
commit eec63cb11d
17 changed files with 526 additions and 75 deletions
--- a/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java
@@ -16,6 +16,7 @@
 package org.springframework.data.repository.cdi;

 import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;

 import java.io.Serializable;
 import java.lang.annotation.Annotation;
@@ -23,17 +24,21 @@ import java.lang.reflect.Type;
 import java.util.Collections;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Function;

 import javax.enterprise.context.ApplicationScoped;
 import javax.enterprise.context.spi.CreationalContext;
 import javax.enterprise.inject.spi.Bean;
 import javax.enterprise.inject.spi.BeanManager;
+import javax.inject.Named;

 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.data.repository.Repository;
+import org.springframework.data.repository.config.CustomRepositoryImplementationDetector;

 /**
 * Unit tests for {@link CdiRepositoryBean}.
@@ -69,8 +74,8 @@ public class CdiRepositoryBeanUnitTests {
 	@Test
 	public void returnsBasicMetadata() {

-		DummyCdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS,
-				SampleRepository.class, beanManager);
+		DummyCdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class,
+				beanManager);

 		assertThat(bean.getBeanClass()).isEqualTo(SampleRepository.class);
 		assertThat(bean.getName()).isEqualTo(SampleRepository.class.getName());
@@ -80,8 +85,8 @@ public class CdiRepositoryBeanUnitTests {
 	@Test
 	public void returnsAllImplementedTypes() {

-		DummyCdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS,
-				SampleRepository.class, beanManager);
+		DummyCdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class,
+				beanManager);

 		Set<Type> types = bean.getTypes();
 		assertThat(types).containsExactlyInAnyOrder(SampleRepository.class, Repository.class);
@@ -91,8 +96,8 @@ public class CdiRepositoryBeanUnitTests {
 	@SuppressWarnings("unchecked")
 	public void detectsStereotypes() {

-		DummyCdiRepositoryBean<StereotypedSampleRepository> bean = new DummyCdiRepositoryBean<>(
-				NO_ANNOTATIONS, StereotypedSampleRepository.class, beanManager);
+		DummyCdiRepositoryBean<StereotypedSampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS,
+				StereotypedSampleRepository.class, beanManager);

 		assertThat(bean.getStereotypes()).containsExactly(StereotypeAnnotation.class);
 	}
@@ -101,22 +106,57 @@ public class CdiRepositoryBeanUnitTests {
 	@SuppressWarnings("rawtypes")
 	public void scopeDefaultsToApplicationScoped() {

-		Bean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class,
-				beanManager);
+		Bean<SampleRepository> bean = new DummyCdiRepositoryBean<>(NO_ANNOTATIONS, SampleRepository.class, beanManager);
 		assertThat(bean.getScope()).isEqualTo(ApplicationScoped.class);
 	}

 	@Test // DATACMNS-322
 	public void createsPassivationId() {

-		CdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>(SINGLE_ANNOTATION,
-				SampleRepository.class, beanManager);
+		CdiRepositoryBean<SampleRepository> bean = new DummyCdiRepositoryBean<>( //
+				SINGLE_ANNOTATION, //
+				SampleRepository.class, //
+				beanManager //
+		);
 		assertThat(bean.getId()).isEqualTo(PASSIVATION_ID);
 	}

+	@Test // DATACMNS-764
+	public void passesCorrectBeanNameToTheImplementationDetector() {
+
+		CustomRepositoryImplementationDetector detector = mock(CustomRepositoryImplementationDetector.class);
+
+		CdiRepositoryBean<SampleRepository> bean = new CdiRepositoryBean<SampleRepository>( //
+				SINGLE_ANNOTATION, //
+				SampleRepository.class, //
+				beanManager, //
+				Optional.of(detector) //
+		) {
+
+			@Override
+			protected SampleRepository create( //
+					CreationalContext<SampleRepository> creationalContext, //
+					Class<SampleRepository> repositoryType, //
+					Optional<Object> customImplementation //
+			) {
+				return null;
+			}
+		};
+
+		bean.create(mock(CreationalContext.class), SampleRepository.class);
+
+		verify(detector).detectCustomImplementation( //
+				eq("CdiRepositoryBeanUnitTests.SampleRepositoryImpl"), //
+				eq("namedRepositoryImpl"), //
+				anySet(), //
+				anySet(), //
+				Mockito.any(Function.class) //
+		);
+	}
+
 	static class DummyCdiRepositoryBean<T> extends CdiRepositoryBean<T> {

-		public DummyCdiRepositoryBean(Set<Annotation> qualifiers, Class<T> repositoryType, BeanManager beanManager) {
+		DummyCdiRepositoryBean(Set<Annotation> qualifiers, Class<T> repositoryType, BeanManager beanManager) {
 			super(qualifiers, repositoryType, beanManager);
 		}

@@ -127,6 +167,7 @@ public class CdiRepositoryBeanUnitTests {
 		}
 	}

+	@Named("namedRepository")
 	static interface SampleRepository extends Repository<Object, Serializable> {

 	}
--- a/src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetectorUnitTests.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2017 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.repository.config;
+
+import static java.util.Arrays.*;
+import static java.util.Collections.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.support.AbstractBeanDefinition;
+import org.springframework.core.env.Environment;
+import org.springframework.core.io.DefaultResourceLoader;
+import org.springframework.core.io.ResourceLoader;
+import org.springframework.core.type.classreading.MetadataReaderFactory;
+import org.springframework.core.type.classreading.SimpleMetadataReaderFactory;
+import org.springframework.core.type.filter.TypeFilter;
+import org.springframework.mock.env.MockEnvironment;
+
+/**
+ * tests {@link CustomRepositoryImplementationDetector}
+ *
+ * @author Jens Schauder
+ */
+public class CustomRepositoryImplementationDetectorUnitTests {
+
+	MetadataReaderFactory metadataFactory = new SimpleMetadataReaderFactory();
+	Environment environment = new MockEnvironment();
+	ResourceLoader resourceLoader = new DefaultResourceLoader();
+	Function<BeanDefinition, String> nameGenerator = mock(Function.class);
+
+	CustomRepositoryImplementationDetector detector = spy(
+			new CustomRepositoryImplementationDetector(metadataFactory, environment, resourceLoader));
+
+	{
+		doReturn("notTheBeanYouAreLookingFor").when(nameGenerator).apply(any(BeanDefinition.class));
+	}
+
+	@Test // DATACMNS-764
+	public void returnsNullWhenNoImplementationFound() {
+
+		doReturn(emptySet()).when(detector).findCandidateBeanDefinitions(anyString(), anyListOf(String.class),
+				anyListOf(TypeFilter.class));
+
+		Optional<AbstractBeanDefinition> beanDefinition = detector.detectCustomImplementation("className", "beanName", emptyList(),
+				emptyList(), nameGenerator);
+
+		assertThat(beanDefinition).isEmpty();
+	}
+
+	@Test // DATACMNS-764
+	public void returnsBeanDefinitionWhenOneImplementationIsFound() {
+
+		AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class);
+
+		doReturn(new HashSet<>(singleton(expectedBeanDefinition))).when(detector).findCandidateBeanDefinitions(anyString(),
+				anyListOf(String.class), anyListOf(TypeFilter.class));
+
+		Optional<AbstractBeanDefinition> beanDefinition = detector.detectCustomImplementation("className", "beanName", emptyList(),
+				emptyList(), nameGenerator);
+
+		assertThat(beanDefinition).contains(expectedBeanDefinition);
+	}
+
+	@Test // DATACMNS-764
+	public void returnsBeanDefinitionMatchingByNameWhenMultipleImplementationAreFound() {
+
+		AbstractBeanDefinition wrongBeanDefinition = mock(AbstractBeanDefinition.class);
+		AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class);
+
+		doReturn("expected").when(nameGenerator).apply(expectedBeanDefinition);
+
+		doReturn(new HashSet<>(asList(wrongBeanDefinition, expectedBeanDefinition))).when(detector)
+				.findCandidateBeanDefinitions(anyString(), anyListOf(String.class), anyListOf(TypeFilter.class));
+
+		Optional<AbstractBeanDefinition> beanDefinition = detector.detectCustomImplementation("className", "expected", emptyList(),
+				emptyList(), nameGenerator);
+
+		assertThat( beanDefinition).contains(expectedBeanDefinition);
+	}
+
+	@Test(expected = IllegalStateException.class) // DATACMNS-764
+	public void throwsExceptionWhenMultipleImplementationAreFound() {
+
+		AbstractBeanDefinition wrongBeanDefinition = mock(AbstractBeanDefinition.class);
+		AbstractBeanDefinition expectedBeanDefinition = mock(AbstractBeanDefinition.class);
+
+		doReturn("expected").when(nameGenerator).apply(any(BeanDefinition.class));
+
+		doReturn(new HashSet<>(asList(wrongBeanDefinition, expectedBeanDefinition))).when(detector)
+				.findCandidateBeanDefinitions(anyString(), anyListOf(String.class), anyListOf(TypeFilter.class));
+
+		Optional<AbstractBeanDefinition> beanDefinition = detector.detectCustomImplementation("className", "expected", emptyList(),
+				emptyList(), nameGenerator);
+	}
+}
--- a/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java
@@ -23,11 +23,15 @@ import lombok.Value;

 import java.util.Optional;

+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
 import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.ConstructorArgumentValues;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.data.repository.query.QueryLookupStrategy.Key;

@@ -35,6 +39,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key;
 * Unit tests for {@link DefaultRepositoryConfiguration}.
 * 
 * @author Oliver Gierke
+ * @author Jens Schauder
 */
@RunWith(MockitoJUnitRunner.class)
 public class DefaultRepositoryConfigurationUnitTests {
@@ -44,6 +49,14 @@ public class DefaultRepositoryConfigurationUnitTests {
 	BeanDefinition definition = new RootBeanDefinition("com.acme.MyRepository");
 	RepositoryConfigurationExtension extension = new SimplerRepositoryConfigurationExtension("factory", "module");

+	@Before
+	public void before() {
+
+		RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator(getClass().getClassLoader());
+		Answer<Object> answer = invocation -> generator.generateBeanName((BeanDefinition) invocation.getArguments()[0]);
+		when(source.generateBeanName(Mockito.any(BeanDefinition.class))).then(answer);
+	}
+
 	@Test
 	public void supportsBasicConfiguration() {

@@ -72,7 +85,8 @@ public class DefaultRepositoryConfigurationUnitTests {

 	private DefaultRepositoryConfiguration<RepositoryConfigurationSource> getConfiguration(
 			RepositoryConfigurationSource source) {
-		return new DefaultRepositoryConfiguration<>(source, definition, extension);
+		RootBeanDefinition beanDefinition = createBeanDefinition();
+		return new DefaultRepositoryConfiguration<>(source, beanDefinition, extension);
 	}

 	@Value
@@ -80,4 +94,15 @@ public class DefaultRepositoryConfigurationUnitTests {
 	private static class SimplerRepositoryConfigurationExtension extends RepositoryConfigurationExtensionSupport {
 		String repositoryFactoryBeanClassName, modulePrefix;
 	}
+
+	private static RootBeanDefinition createBeanDefinition() {
+
+		RootBeanDefinition beanDefinition = new RootBeanDefinition("com.acme.MyRepository");
+
+		ConstructorArgumentValues constructorArgumentValues = new ConstructorArgumentValues();
+		constructorArgumentValues.addGenericArgumentValue(MyRepository.class);
+		beanDefinition.setConstructorArgumentValues(constructorArgumentValues);
+
+		return beanDefinition;
+	}
 }
--- a/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/config/RepositoryBeanNameGeneratorUnitTests.java
@@ -32,30 +32,27 @@ import org.springframework.data.repository.core.support.RepositoryFactoryBeanSup
 * Unit tests for {@link RepositoryBeanNameGenerator}.
 * 
 * @author Oliver Gierke
+ * @author Jens Schauder
 */
 public class RepositoryBeanNameGeneratorUnitTests {

-	BeanNameGenerator generator;
+	RepositoryBeanNameGenerator generator;
 	BeanDefinitionRegistry registry;

 	@Before
 	public void setUp() {

-		RepositoryBeanNameGenerator generator = new RepositoryBeanNameGenerator();
-		generator.setBeanClassLoader(Thread.currentThread().getContextClassLoader());
-
-		this.generator = generator;
-		this.registry = new DefaultListableBeanFactory();
+		this.generator = new RepositoryBeanNameGenerator(Thread.currentThread().getContextClassLoader());
 	}

 	@Test
 	public void usesPlainClassNameIfNoAnnotationPresent() {
-		assertThat(generator.generateBeanName(getBeanDefinitionFor(MyRepository.class), registry)).isEqualTo("myRepository");
+		assertThat(generator.generateBeanName(getBeanDefinitionFor(MyRepository.class))).isEqualTo("myRepository");
 	}

 	@Test
 	public void usesAnnotationValueIfAnnotationPresent() {
-		assertThat(generator.generateBeanName(getBeanDefinitionFor(AnnotatedInterface.class), registry)).isEqualTo("specialName");
+		assertThat(generator.generateBeanName(getBeanDefinitionFor(AnnotatedInterface.class))).isEqualTo("specialName");
 	}

 	private BeanDefinition getBeanDefinitionFor(Class<?> repositoryInterface) {
--- a/src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java
+++ b/src/test/java/org/springframework/data/repository/config/SelectionSetUnitTests.java
@@ -0,0 +1,62 @@
+package org.springframework.data.repository.config;
+
+import static java.util.Arrays.*;
+import static java.util.Collections.*;
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link SelectionSet}
+ *
+ * @author Jens Schauder
+ */
+public class SelectionSetUnitTests {
+
+	@Test // DATACMNS-764
+	public void returnsUniqueResult() {
+		assertEquals("single value", new SelectionSet<>(singleton("single value")).uniqueResult());
+	}
+
+	@Test // DATACMNS-764
+	public void emptyCollectionReturnsNull() {
+		assertNull(new SelectionSet<Object>(emptySet()).uniqueResult());
+	}
+
+	@Test(expected = IllegalStateException.class) // DATACMNS-764
+	public void multipleElementsThrowException() {
+		new SelectionSet<>(asList("one", "two")).uniqueResult();
+	}
+
+	@Test(expected = NullPointerException.class) // DATACMNS-764
+	public void throwsCustomExceptionWhenConfigured() {
+
+		new SelectionSet<>(asList("one", "two"), c -> {
+			throw new NullPointerException();
+		}).uniqueResult();
+	}
+
+	@Test // DATACMNS-764
+	public void usesFallbackWhenConfigured() {
+
+		String value = new SelectionSet<>(asList("one", "two"), c -> String.valueOf(c.size())).uniqueResult();
+
+		assertEquals("2", value);
+	}
+
+	@Test // DATACMNS-764
+	public void returnsUniqueResultAfterFilter() {
+
+		SelectionSet<String> selection = new SelectionSet<>(asList("one", "two", "three")).filterIfNecessary(s -> s.contains("w"));
+
+		assertEquals("two", selection.uniqueResult());
+	}
+
+	@Test // DATACMNS-764
+	public void ignoresFilterWhenResultIsAlreadyUnique() {
+
+		SelectionSet<String> selection = new SelectionSet<>(asList("one")).filterIfNecessary(s -> s.contains("w"));
+
+		assertEquals("one", selection.uniqueResult());
+	}
+}