From 95f069c755174d40a6401f4d932da1b0e11417d4 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 26 May 2020 18:05:26 +0200 Subject: [PATCH] DATACMNS-1734 - Defer initialization of Repositories in DomainClassConverter. DomainClassConverter now delays the initialization of the Repositories instances used to avoid the premature initialization of repository instances as that can cause deadlocks if the infrastructure backing the repositories is initialized on a separate thread. Refactored nested converter classes to allow them to be static ones. Related issues: spring-projects/spring-boot#16230, spring-projects/spring-framework#25131. Original pull request: #445. --- .../support/DomainClassConverter.java | 38 ++++++++++++++----- .../DomainClassConverterUnitTests.java | 2 +- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java index 740c99de47..01080cdfb2 100644 --- a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java +++ b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java @@ -30,6 +30,7 @@ import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.core.RepositoryInformation; +import org.springframework.data.util.Lazy; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -47,7 +48,7 @@ public class DomainClassConverter repositories = Lazy.of(Repositories.NONE); private Optional toEntityConverter = Optional.empty(); private Optional toIdConverter = Optional.empty(); @@ -97,7 +98,7 @@ public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { * @return */ private Optional getConverter(TypeDescriptor targetType) { - return repositories.hasRepositoryFor(targetType.getType()) ? toEntityConverter : toIdConverter; + return repositories.get().hasRepositoryFor(targetType.getType()) ? toEntityConverter : toIdConverter; } /* @@ -106,13 +107,18 @@ private Optional getConverter(TypeDescrip */ public void setApplicationContext(ApplicationContext context) { - this.repositories = new Repositories(context); + this.repositories = Lazy.of(() -> { - this.toEntityConverter = Optional.of(new ToEntityConverter(this.repositories, this.conversionService)); - this.toEntityConverter.ifPresent(it -> this.conversionService.addConverter(it)); + Repositories repositories = new Repositories(context); - this.toIdConverter = Optional.of(new ToIdConverter()); - this.toIdConverter.ifPresent(it -> this.conversionService.addConverter(it)); + this.toEntityConverter = Optional.of(new ToEntityConverter(repositories, conversionService)); + this.toEntityConverter.ifPresent(it -> conversionService.addConverter(it)); + + this.toIdConverter = Optional.of(new ToIdConverter(repositories, conversionService)); + this.toIdConverter.ifPresent(it -> conversionService.addConverter(it)); + + return repositories; + }); } /** @@ -121,9 +127,11 @@ public void setApplicationContext(ApplicationContext context) { * @author Oliver Gierke * @since 1.10 */ - private class ToEntityConverter implements ConditionalGenericConverter { + private static class ToEntityConverter implements ConditionalGenericConverter { private final RepositoryInvokerFactory repositoryInvokerFactory; + private final Repositories repositories; + private final ConversionService conversionService; /** * Creates a new {@link ToEntityConverter} for the given {@link Repositories} and {@link ConversionService}. @@ -132,7 +140,10 @@ private class ToEntityConverter implements ConditionalGenericConverter { * @param conversionService must not be {@literal null}. */ public ToEntityConverter(Repositories repositories, ConversionService conversionService) { + this.repositoryInvokerFactory = new DefaultRepositoryInvokerFactory(repositories, conversionService); + this.repositories = repositories; + this.conversionService = conversionService; } /* @@ -206,7 +217,16 @@ public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { * @author Oliver Gierke * @since 1.10 */ - class ToIdConverter implements ConditionalGenericConverter { + static class ToIdConverter implements ConditionalGenericConverter { + + private final Repositories repositories; + private final ConversionService conversionService; + + public ToIdConverter(Repositories repositories, ConversionService conversionService) { + + this.repositories = repositories; + this.conversionService = conversionService; + } /* * (non-Javadoc) diff --git a/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java b/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java index 65c1f9c941..0c8282d86b 100755 --- a/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/DomainClassConverterUnitTests.java @@ -29,7 +29,6 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; - import org.springframework.aop.framework.Advised; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; @@ -181,6 +180,7 @@ void supportsConversionFromEntityToString() { void toIdConverterDoesNotMatchIfTargetTypeIsAssignableFromSource() throws NoSuchMethodException { converter.setApplicationContext(initContextWithRepo()); + assertMatches(false); @SuppressWarnings("rawtypes") Optional toIdConverter = (Optional) ReflectionTestUtils.getField(converter,