From 2a44d2394ce64155c53a82e61776a6994cfb3eb9 Mon Sep 17 00:00:00 2001 From: Kyrylo Merzlikin Date: Sat, 18 Jul 2020 18:49:36 +0300 Subject: [PATCH] DATACMNS-1754 - Support implementation lookup for nested repositories and fragments. Problem: while repository and fragment interfaces are discovered correctly (provided that considerNestedRepositories=true), their implementations are not found/registered despite proper class naming. Cause: DefaultImplementationLookupConfiguration works in such way that for nested interface (i.e. one whose class name includes name of the enclosing class) expected implementation class name is built including that enclosing class name. In the same time actual implementation class names are always "localized" (i.e. stripped from enclosing class name, if any) before matching. This makes matching implementation classes against nested interface impossible. Solution: when building expected implementation class name, use "local" interface class name, so that it can match any implementation class that follows naming convention "SimpleInterfaceName" + "ImplemenetationPostfix". Original pull request: #460. --- ...aultImplementationLookupConfiguration.java | 15 ++++--- .../cdi/CdiRepositoryBeanUnitTests.java | 6 +-- .../repository/cdi/ComposedRepository.java | 5 ++- .../cdi/NestedFragmentInterfaceImpl.java | 27 ++++++++++++ .../repository/cdi/RepositoryFragments.java | 27 ++++++++++++ .../RepositoryFragmentsIntegrationTests.java | 43 ++++++++++++++++++- ...mentationLookupConfigurationUnitTests.java | 13 ++++++ 7 files changed, 125 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/springframework/data/repository/cdi/NestedFragmentInterfaceImpl.java create mode 100644 src/test/java/org/springframework/data/repository/cdi/RepositoryFragments.java diff --git a/src/main/java/org/springframework/data/repository/config/DefaultImplementationLookupConfiguration.java b/src/main/java/org/springframework/data/repository/config/DefaultImplementationLookupConfiguration.java index 549a1532b..dfd1dac05 100644 --- a/src/main/java/org/springframework/data/repository/config/DefaultImplementationLookupConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/DefaultImplementationLookupConfiguration.java @@ -33,6 +33,7 @@ import org.springframework.util.ClassUtils; * * @author Oliver Gierke * @author Mark Paluch + * @author Kyrylo Merzlikin * @since 2.1 */ class DefaultImplementationLookupConfiguration implements ImplementationLookupConfiguration { @@ -55,8 +56,7 @@ class DefaultImplementationLookupConfiguration implements ImplementationLookupCo this.config = config; this.interfaceName = interfaceName; - this.beanName = Introspector - .decapitalize(ClassUtils.getShortName(interfaceName).concat(config.getImplementationPostfix())); + this.beanName = Introspector.decapitalize(getLocalName(interfaceName).concat(config.getImplementationPostfix())); } /* @@ -110,7 +110,7 @@ class DefaultImplementationLookupConfiguration implements ImplementationLookupCo */ @Override public String getImplementationClassName() { - return ClassUtils.getShortName(interfaceName).concat(getImplementationPostfix()); + return getLocalName(interfaceName).concat(getImplementationPostfix()); } /* @@ -141,13 +141,18 @@ class DefaultImplementationLookupConfiguration implements ImplementationLookupCo } String beanPackage = ClassUtils.getPackageName(beanClassName); - String shortName = ClassUtils.getShortName(beanClassName); - String localName = shortName.substring(shortName.lastIndexOf('.') + 1); + String localName = getLocalName(beanClassName); return localName.equals(getImplementationClassName()) // && getBasePackages().stream().anyMatch(it -> beanPackage.startsWith(it)); } + private String getLocalName(String className) { + + String shortName = ClassUtils.getShortName(className); + return shortName.substring(shortName.lastIndexOf('.') + 1); + } + private boolean isExcluded(String beanClassName, Streamable filters) { try { diff --git a/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java b/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java index ea967d0f0..96733586d 100755 --- a/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java +++ b/src/test/java/org/springframework/data/repository/cdi/CdiRepositoryBeanUnitTests.java @@ -37,7 +37,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.config.CustomRepositoryImplementationDetector; @@ -58,6 +57,7 @@ import org.springframework.data.repository.query.RepositoryQuery; * @author Oliver Gierke * @author Mark Paluch * @author Ariel Carrera + * @author Kyrylo Merzlikin */ @ExtendWith(MockitoExtension.class) class CdiRepositoryBeanUnitTests { @@ -171,8 +171,8 @@ class CdiRepositoryBeanUnitTests { ImplementationLookupConfiguration configuration = captor.getValue(); - assertThat(configuration.getImplementationBeanName()).isEqualTo("cdiRepositoryBeanUnitTests.SampleRepositoryImpl"); - assertThat(configuration.getImplementationClassName()).isEqualTo("CdiRepositoryBeanUnitTests.SampleRepositoryImpl"); + assertThat(configuration.getImplementationBeanName()).isEqualTo("sampleRepositoryImpl"); + assertThat(configuration.getImplementationClassName()).isEqualTo("SampleRepositoryImpl"); } @Test // DATACMNS-1233 diff --git a/src/test/java/org/springframework/data/repository/cdi/ComposedRepository.java b/src/test/java/org/springframework/data/repository/cdi/ComposedRepository.java index 55a4ad1cf..312407291 100644 --- a/src/test/java/org/springframework/data/repository/cdi/ComposedRepository.java +++ b/src/test/java/org/springframework/data/repository/cdi/ComposedRepository.java @@ -21,9 +21,10 @@ import org.springframework.data.repository.Repository; /** * @author Mark Paluch + * @author Kyrylo Merzlikin */ -interface ComposedRepository - extends Repository, FragmentInterface, AnotherFragmentInterface { +interface ComposedRepository extends Repository, FragmentInterface, AnotherFragmentInterface, + RepositoryFragments.NestedFragmentInterface { // duplicate method shadowed by AnotherFragmentInterfaceImpl. The legacy custom implementation comes last, after all // other fragments. diff --git a/src/test/java/org/springframework/data/repository/cdi/NestedFragmentInterfaceImpl.java b/src/test/java/org/springframework/data/repository/cdi/NestedFragmentInterfaceImpl.java new file mode 100644 index 000000000..be5b02215 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/cdi/NestedFragmentInterfaceImpl.java @@ -0,0 +1,27 @@ +/* + * Copyright 2018-2020 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.cdi; + +/** + * @author Kyrylo Merzlikin + */ +public class NestedFragmentInterfaceImpl implements RepositoryFragments.NestedFragmentInterface { + + @Override + public String getKey() { + return "NestedFragmentImpl"; + } +} diff --git a/src/test/java/org/springframework/data/repository/cdi/RepositoryFragments.java b/src/test/java/org/springframework/data/repository/cdi/RepositoryFragments.java new file mode 100644 index 000000000..9b520e1d2 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/cdi/RepositoryFragments.java @@ -0,0 +1,27 @@ +/* + * Copyright 2018-2020 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.cdi; + +/** + * @author Kyrylo Merzlikin + */ +public interface RepositoryFragments { + + interface NestedFragmentInterface { + + String getKey(); + } +} diff --git a/src/test/java/org/springframework/data/repository/cdi/RepositoryFragmentsIntegrationTests.java b/src/test/java/org/springframework/data/repository/cdi/RepositoryFragmentsIntegrationTests.java index 69582515f..1962921b1 100644 --- a/src/test/java/org/springframework/data/repository/cdi/RepositoryFragmentsIntegrationTests.java +++ b/src/test/java/org/springframework/data/repository/cdi/RepositoryFragmentsIntegrationTests.java @@ -17,17 +17,21 @@ package org.springframework.data.repository.cdi; import static org.assertj.core.api.Assertions.*; +import java.io.Serializable; + import javax.enterprise.inject.se.SeContainer; import javax.enterprise.inject.se.SeContainerInitializer; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.springframework.data.repository.Repository; /** * CDI integration tests for composed repositories. - * + * * @author Mark Paluch + * @author Kyrylo Merzlikin */ class RepositoryFragmentsIntegrationTests { @@ -66,6 +70,24 @@ class RepositoryFragmentsIntegrationTests { assertThat(shadowed.getPriority()).isEqualTo(2); } + @Test // DATACMNS-1754 + void shouldFindCustomImplementationForNestedRepository() { + + NestedRepository repository = getBean(NestedRepository.class); + + assertThat(repository.getCustom()).isEqualTo("CustomImpl"); + } + + @Test // DATACMNS-1754 + void shouldFindImplementationForNestedRepositoryFragment() { + + ComposedRepository repository = getBean(ComposedRepository.class); + NestedFragmentInterfaceImpl fragment = getBean(NestedFragmentInterfaceImpl.class); + + assertThat(repository.getKey()).isEqualTo("NestedFragmentImpl"); + assertThat(fragment.getKey()).isEqualTo("NestedFragmentImpl"); + } + protected T getBean(Class type) { return container.select(type).get(); } @@ -74,4 +96,23 @@ class RepositoryFragmentsIntegrationTests { static void tearDown() { container.close(); } + + public interface NestedRepository extends Repository { + + String getCustom(); + } + + public interface NestedRepositoryCustom { + + // duplicate method allowing to provide custom implementation (without modifying original repository) + String getCustom(); + } + + public static class NestedRepositoryImpl implements NestedRepositoryCustom { + + @Override + public String getCustom() { + return "CustomImpl"; + } + } } diff --git a/src/test/java/org/springframework/data/repository/config/DefaultImplementationLookupConfigurationUnitTests.java b/src/test/java/org/springframework/data/repository/config/DefaultImplementationLookupConfigurationUnitTests.java index 23d0b8a29..5da3df4e2 100644 --- a/src/test/java/org/springframework/data/repository/config/DefaultImplementationLookupConfigurationUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/DefaultImplementationLookupConfigurationUnitTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; * Unit tests for {@link DefaultImplementationLookupConfigurationUnitTests}. * * @author Mark Paluch + * @author Kyrylo Merzlikin */ class DefaultImplementationLookupConfigurationUnitTests { @@ -37,6 +38,18 @@ class DefaultImplementationLookupConfigurationUnitTests { assertThat(getImplementationBeanName(idcMock, "com.acme.UdpRepository")).isEqualTo("udpRepositoryImpl"); } + @Test // DATACMNS-1754 + void shouldUseSimpleClassNameWhenDefiningImplementationNames() { + + ImplementationDetectionConfiguration idcMock = mock(ImplementationDetectionConfiguration.class); + when(idcMock.getImplementationPostfix()).thenReturn("Impl"); + + DefaultImplementationLookupConfiguration lookupConfiguration = new DefaultImplementationLookupConfiguration(idcMock, + "com.acme.Repositories$NestedRepository"); + assertThat(lookupConfiguration.getImplementationBeanName()).isEqualTo("nestedRepositoryImpl"); + assertThat(lookupConfiguration.getImplementationClassName()).isEqualTo("NestedRepositoryImpl"); + } + private static String getImplementationBeanName(ImplementationDetectionConfiguration idcMock, String interfaceName) { DefaultImplementationLookupConfiguration configuration = new DefaultImplementationLookupConfiguration(idcMock,