From fee153f724c870c40f19e053cdaaa99774121856 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 21 Mar 2017 11:50:50 +0100 Subject: [PATCH] =?UTF-8?q?DATACMNS-867=20-=20RepositoryInvoker=20now=20re?= =?UTF-8?q?turns=20an=20Optional=20for=20invokeFindOne(=E2=80=A6).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapted ReflectionRepositoryInvoker to transparently unwrap other potentially used wrapper types to then convert the result into an Optional. --- .../QuerydslRepositoryInvokerAdapter.java | 3 ++- .../support/CrudRepositoryInvoker.java | 4 ++-- .../support/DomainClassConverter.java | 6 ++---- .../support/ReflectionRepositoryInvoker.java | 18 ++++++++++++++--- .../repository/support/RepositoryInvoker.java | 3 ++- ...ositoryInvokerFactoryIntegrationTests.java | 6 ++++-- .../ReflectionRepositoryInvokerUnitTests.java | 20 +++++++++++++++++++ 7 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java b/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java index 28aa255f4..9ecc76a57 100644 --- a/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java +++ b/src/main/java/org/springframework/data/querydsl/QuerydslRepositoryInvokerAdapter.java @@ -17,6 +17,7 @@ package org.springframework.data.querydsl; import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Optional; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; @@ -125,7 +126,7 @@ public class QuerydslRepositoryInvokerAdapter implements RepositoryInvoker { * @see org.springframework.data.repository.support.RepositoryInvoker#invokeFindOne(java.io.Serializable) */ @Override - public T invokeFindOne(Serializable id) { + public Optional invokeFindOne(Serializable id) { return delegate.invokeFindOne(id); } diff --git a/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java index 3aec49509..4b8767b7e 100644 --- a/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java @@ -89,8 +89,8 @@ class CrudRepositoryInvoker extends ReflectionRepositoryInvoker { */ @Override @SuppressWarnings("unchecked") - public T invokeFindOne(Serializable id) { - return customFindOneMethod ? super.invokeFindOne(id) : (T) repository.findOne(convertId(id)); + public Optional invokeFindOne(Serializable id) { + return customFindOneMethod ? super.invokeFindOne(id) : (Optional) repository.findOne(convertId(id)); } /* 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 63351f18f..5321b88b7 100644 --- a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java +++ b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java @@ -152,11 +152,9 @@ public class DomainClassConverter domainType = targetType.getType(); RepositoryInvoker invoker = repositoryInvokerFactory.getInvokerFor(domainType); + RepositoryInformation information = repositories.getRequiredRepositoryInformation(domainType); - return repositories.getRepositoryInformationFor(domainType)// - .map(it -> invoker.invokeFindOne(conversionService.convert(source, it.getIdType())))// - .orElseThrow(() -> new IllegalStateException( - String.format("Couldn't find RepositoryInformation for %s!", domainType))); + return invoker.invokeFindOne(conversionService.convert(source, information.getIdType())).orElse(null); } /* diff --git a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java index f366b685f..02fdf74f3 100644 --- a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java @@ -30,6 +30,7 @@ import org.springframework.data.domain.Sort; import org.springframework.data.repository.core.CrudMethods; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.Param; +import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.MultiValueMap; @@ -135,12 +136,23 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { * @see org.springframework.data.rest.core.invoke.RepositoryInvoker#invokeFindOne(java.io.Serializable) */ @Override - public T invokeFindOne(Serializable id) { + @SuppressWarnings("unchecked") + public Optional invokeFindOne(Serializable id) { Method method = methods.getFindOneMethod()// .orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-one-method declared!")); - return invoke(method, convertId(id)); + Object invoke = invoke(method, convertId(id)); + + if (Optional.class.isInstance(invoke)) { + return (Optional) invoke; + } + + if (invoke == null) { + return Optional.empty(); + } + + return conversionService.convert(QueryExecutionConverters.unwrap(invoke), Optional.class); } /* @@ -169,7 +181,7 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { if (idTypes.contains(parameterType)) { invoke(method, convertId(id)); } else { - invoke(method, this.invokeFindOne(id)); + invoke(method, this. invokeFindOne(id).orElse(null)); } } diff --git a/src/main/java/org/springframework/data/repository/support/RepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/RepositoryInvoker.java index facc1d232..18514209e 100644 --- a/src/main/java/org/springframework/data/repository/support/RepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/RepositoryInvoker.java @@ -17,6 +17,7 @@ package org.springframework.data.repository.support; import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Optional; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; @@ -49,7 +50,7 @@ public interface RepositoryInvoker extends RepositoryInvocationInformation { * @return the entity with the given id. * @throws IllegalStateException if the repository does not expose a find-one-method. */ - T invokeFindOne(Serializable id); + Optional invokeFindOne(Serializable id); /** * Invokes the find-all method of the underlying repository using the method taking a {@link Pageable} as parameter if diff --git a/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactoryIntegrationTests.java b/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactoryIntegrationTests.java index a72ba4cb8..9c5a5883c 100755 --- a/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/repository/support/DefaultRepositoryInvokerFactoryIntegrationTests.java @@ -18,6 +18,8 @@ package org.springframework.data.repository.support; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import java.util.Optional; + import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -59,9 +61,9 @@ public class DefaultRepositoryInvokerFactoryIntegrationTests { Product product = new Product(); when(productRepository.findOne(4711L)).thenReturn(product); - Object invokeFindOne = factory.getInvokerFor(Product.class).invokeFindOne(4711L); + Optional invokeFindOne = factory.getInvokerFor(Product.class).invokeFindOne(4711L); - assertThat(invokeFindOne).isEqualTo(product); + assertThat(invokeFindOne).isEqualTo(Optional.of(product)); } @Test // DATACMNS-374, DATACMNS-589 diff --git a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java index f0be70624..2ca7a5d23 100755 --- a/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/ReflectionRepositoryInvokerUnitTests.java @@ -16,6 +16,7 @@ package org.springframework.data.repository.support; import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import static org.springframework.data.repository.support.RepositoryInvocationTestUtils.*; @@ -25,6 +26,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; +import java.util.Optional; import org.junit.Before; import org.junit.Test; @@ -239,6 +241,19 @@ public class ReflectionRepositoryInvokerUnitTests { } } + @Test // DATACMNS-867 + public void convertsWrapperTypeToJdkOptional() { + + GuavaRepository mock = mock(GuavaRepository.class); + when(mock.findOne(any())).thenReturn(com.google.common.base.Optional.of(new Domain())); + + RepositoryInvoker invoker = getInvokerFor(mock); + + Optional invokeFindOne = invoker.invokeFindOne(1L); + + assertThat(invokeFindOne).isPresent(); + } + private static RepositoryInvoker getInvokerFor(Object repository) { RepositoryMetadata metadata = new DefaultRepositoryMetadata(repository.getClass().getInterfaces()[0]); @@ -298,4 +313,9 @@ public class ReflectionRepositoryInvokerUnitTests { Domain findByClass(@Param("value") int value); } + + interface GuavaRepository extends Repository { + + com.google.common.base.Optional findOne(Long id); + } }