From fd2c400c4e7b57ac14fa2da770a547e15ab32ef7 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 14 Nov 2016 08:16:08 +0100 Subject: [PATCH] DATACMNS-867 - Optionals in CrudMethods API. --- .../data/repository/core/CrudMethods.java | 9 +-- .../support/AbstractRepositoryMetadata.java | 21 +++---- .../core/support/DefaultCrudMethods.java | 55 +++++++++---------- .../support/CrudRepositoryInvoker.java | 5 +- .../support/ReflectionRepositoryInvoker.java | 26 +++++---- .../support/DefaultCrudMethodsUnitTests.java | 29 ++++++---- 6 files changed, 74 insertions(+), 71 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/core/CrudMethods.java b/src/main/java/org/springframework/data/repository/core/CrudMethods.java index 2dc8bf63f..ee3392081 100644 --- a/src/main/java/org/springframework/data/repository/core/CrudMethods.java +++ b/src/main/java/org/springframework/data/repository/core/CrudMethods.java @@ -16,6 +16,7 @@ package org.springframework.data.repository.core; import java.lang.reflect.Method; +import java.util.Optional; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; @@ -37,7 +38,7 @@ public interface CrudMethods { * @return the method to save entities or {@literal null} if noen exposed. * @see #hasSaveMethod() */ - Method getSaveMethod(); + Optional getSaveMethod(); /** * Returns whether the repository exposes a save method at all. @@ -53,7 +54,7 @@ public interface CrudMethods { * @return the find all method of the repository or {@literal null} if not available. * @see #hasFindAllMethod() */ - Method getFindAllMethod(); + Optional getFindAllMethod(); /** * Returns whether the repository exposes a find all method at all. @@ -69,7 +70,7 @@ public interface CrudMethods { * @return the find one method of the repository or {@literal null} if not available. * @see #hasFindOneMethod() */ - Method getFindOneMethod(); + Optional getFindOneMethod(); /** * Returns whether the repository exposes a find one method. @@ -84,7 +85,7 @@ public interface CrudMethods { * @return the delete method of the repository or {@literal null} if not available. * @see #hasDelete() */ - Method getDeleteMethod(); + Optional getDeleteMethod(); /** * Returns whether the repository esposes a delete method. diff --git a/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java b/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java index b7594ff43..7dccbad56 100644 --- a/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java @@ -27,6 +27,7 @@ import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.util.QueryExecutionConverters; import org.springframework.data.repository.util.ReactiveWrappers; import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.Lazy; import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -41,7 +42,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata { private final TypeInformation typeInformation; private final Class repositoryInterface; - private CrudMethods crudMethods; + private final Lazy crudMethods; /** * Creates a new {@link AbstractRepositoryMetadata}. @@ -55,6 +56,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata { this.repositoryInterface = repositoryInterface; this.typeInformation = ClassTypeInformation.from(repositoryInterface); + this.crudMethods = Lazy.of(() -> new DefaultCrudMethods(this)); } /** @@ -94,12 +96,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata { */ @Override public CrudMethods getCrudMethods() { - - if (this.crudMethods == null) { - this.crudMethods = new DefaultCrudMethods(this); - } - - return this.crudMethods; + return this.crudMethods.get(); } /* @@ -109,13 +106,9 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata { @Override public boolean isPagingRepository() { - Method findAllMethod = getCrudMethods().getFindAllMethod(); - - if (findAllMethod == null) { - return false; - } - - return Arrays.asList(findAllMethod.getParameterTypes()).contains(Pageable.class); + return getCrudMethods().getFindAllMethod()// + .map(it -> Arrays.asList(it.getParameterTypes()).contains(Pageable.class))// + .orElse(false); } /* diff --git a/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java b/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java index 40692b840..f8c88d4d1 100644 --- a/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java +++ b/src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java @@ -21,6 +21,7 @@ import static org.springframework.util.ReflectionUtils.*; 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; @@ -48,10 +49,10 @@ public class DefaultCrudMethods implements CrudMethods { private static final String FIND_ALL = "findAll"; private static final String DELETE = "delete"; - private final Method findAllMethod; - private final Method findOneMethod; - private final Method saveMethod; - private final Method deleteMethod; + private final Optional findAllMethod; + private final Optional findOneMethod; + private final Optional saveMethod; + private final Optional deleteMethod; /** * Creates a new {@link DefaultCrudMethods} using the given {@link RepositoryMetadata}. @@ -78,7 +79,7 @@ public class DefaultCrudMethods implements CrudMethods { * @param metadata must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ - private Method selectMostSuitableSaveMethod(RepositoryMetadata metadata) { + private Optional selectMostSuitableSaveMethod(RepositoryMetadata metadata) { for (Class type : asList(metadata.getDomainType(), Object.class)) { @@ -89,7 +90,7 @@ public class DefaultCrudMethods implements CrudMethods { } } - return null; + return Optional.empty(); } /** @@ -104,7 +105,7 @@ public class DefaultCrudMethods implements CrudMethods { * @param metadata must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ - private Method selectMostSuitableDeleteMethod(RepositoryMetadata metadata) { + private Optional selectMostSuitableDeleteMethod(RepositoryMetadata metadata) { for (Class type : asList(metadata.getDomainType(), metadata.getIdType(), Serializable.class, Iterable.class)) { @@ -115,7 +116,7 @@ public class DefaultCrudMethods implements CrudMethods { } } - return null; + return Optional.empty(); } /** @@ -129,7 +130,7 @@ public class DefaultCrudMethods implements CrudMethods { * @param metadata must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ - private Method selectMostSuitableFindAllMethod(RepositoryMetadata metadata) { + private Optional selectMostSuitableFindAllMethod(RepositoryMetadata metadata) { for (Class type : asList(Pageable.class, Sort.class)) { @@ -147,7 +148,7 @@ public class DefaultCrudMethods implements CrudMethods { return getMostSpecificMethod(findMethod(CrudRepository.class, FIND_ALL), metadata.getRepositoryInterface()); } - return null; + return Optional.empty(); } /** @@ -160,7 +161,7 @@ public class DefaultCrudMethods implements CrudMethods { * @param metadata must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ - private Method selectMostSuitableFindOneMethod(RepositoryMetadata metadata) { + private Optional selectMostSuitableFindOneMethod(RepositoryMetadata metadata) { for (Class type : asList(metadata.getIdType(), Serializable.class)) { @@ -171,7 +172,7 @@ public class DefaultCrudMethods implements CrudMethods { } } - return null; + return Optional.empty(); } /** @@ -183,16 +184,12 @@ public class DefaultCrudMethods implements CrudMethods { * @see ClassUtils#getMostSpecificMethod(Method, Class) * @return */ - private static Method getMostSpecificMethod(Method method, Class type) { + private static Optional getMostSpecificMethod(Method method, Class type) { - Method result = ClassUtils.getMostSpecificMethod(method, type); - - if (result == null) { - return null; - } - - ReflectionUtils.makeAccessible(result); - return result; + return Optional.ofNullable(ClassUtils.getMostSpecificMethod(method, type)).map(it -> { + ReflectionUtils.makeAccessible(it); + return it; + }); } /* @@ -200,7 +197,7 @@ public class DefaultCrudMethods implements CrudMethods { * @see org.springframework.data.repository.core.support.CrudMethods#getSaveMethod() */ @Override - public Method getSaveMethod() { + public Optional getSaveMethod() { return saveMethod; } @@ -210,7 +207,7 @@ public class DefaultCrudMethods implements CrudMethods { */ @Override public boolean hasSaveMethod() { - return saveMethod != null; + return saveMethod.isPresent(); } /* @@ -218,7 +215,7 @@ public class DefaultCrudMethods implements CrudMethods { * @see org.springframework.data.repository.core.support.CrudMethods#getFindAllMethod() */ @Override - public Method getFindAllMethod() { + public Optional getFindAllMethod() { return findAllMethod; } @@ -228,7 +225,7 @@ public class DefaultCrudMethods implements CrudMethods { */ @Override public boolean hasFindAllMethod() { - return findAllMethod != null; + return findAllMethod.isPresent(); } /* @@ -236,7 +233,7 @@ public class DefaultCrudMethods implements CrudMethods { * @see org.springframework.data.repository.core.support.CrudMethods#getFindOneMethod() */ @Override - public Method getFindOneMethod() { + public Optional getFindOneMethod() { return findOneMethod; } @@ -246,7 +243,7 @@ public class DefaultCrudMethods implements CrudMethods { */ @Override public boolean hasFindOneMethod() { - return findOneMethod != null; + return findOneMethod.isPresent(); } /* @@ -255,7 +252,7 @@ public class DefaultCrudMethods implements CrudMethods { */ @Override public boolean hasDelete() { - return this.deleteMethod != null; + return this.deleteMethod.isPresent(); } /* @@ -263,7 +260,7 @@ public class DefaultCrudMethods implements CrudMethods { * @see org.springframework.data.repository.core.support.CrudMethods#getDeleteMethod() */ @Override - public Method getDeleteMethod() { + public Optional getDeleteMethod() { return this.deleteMethod; } } 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 7a809224d..ab0ad4256 100644 --- a/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.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.core.convert.ConversionService; import org.springframework.data.domain.Pageable; @@ -114,7 +115,7 @@ class CrudRepositoryInvoker extends ReflectionRepositoryInvoker { } } - private boolean isRedeclaredMethod(Method method) { - return !method.getDeclaringClass().equals(CrudRepository.class); + protected boolean isRedeclaredMethod(Optional method) { + return method.map(it -> !it.getDeclaringClass().equals(CrudRepository.class)).orElse(false); } } 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 615ef00e2..944f3d3f5 100644 --- a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java @@ -113,10 +113,12 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { * @see org.springframework.data.rest.core.invoke.RepositoryInvoker#invokeSave(java.lang.Object) */ @Override + @SuppressWarnings("unchecked") public T invokeSave(T object) { - Assert.state(hasSaveMethod(), "Repository doesn't have a save-method declared!"); - return invoke(methods.getSaveMethod(), object); + return methods.getSaveMethod()// + .map(it -> (T) invoke(it, object))// + .orElseThrow(() -> new IllegalStateException("Repository doesn't have a save-method declared!")); } /* @@ -133,10 +135,12 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { * @see org.springframework.data.rest.core.invoke.RepositoryInvoker#invokeFindOne(java.io.Serializable) */ @Override + @SuppressWarnings("unchecked") public T invokeFindOne(Serializable id) { - Assert.state(hasFindOneMethod(), "Repository doesn't have a find-one-method declared!"); - return invoke(methods.getFindOneMethod(), convertId(id)); + return methods.getFindOneMethod()// + .map(it -> (T) invoke(it, convertId(id)))// + .orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-one-method declared!")); } /* @@ -156,9 +160,9 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { public void invokeDelete(Serializable id) { Assert.notNull(id, "Identifier must not be null!"); - Assert.state(hasDeleteMethod(), "Repository doesn't have a delete-method declared!"); - Method method = methods.getDeleteMethod(); + Method method = methods.getDeleteMethod() + .orElseThrow(() -> new IllegalStateException("Repository doesn't have a delete-method declared!")); Class parameterType = method.getParameterTypes()[0]; List> idTypes = Arrays.asList(idType, Serializable.class); @@ -260,9 +264,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { protected Iterable invokePagedFindAllReflectively(Pageable pageable) { - Assert.state(hasFindAllMethod(), "Repository doesn't have a find-all-method declared!"); - - Method method = methods.getFindAllMethod(); + Method method = methods.getFindAllMethod() + .orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-all-method declared!")); Class[] types = method.getParameterTypes(); if (types.length == 0) { @@ -278,9 +281,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { protected Iterable invokeSortedFindAllReflectively(Sort sort) { - Assert.state(hasFindAllMethod(), "Repository doesn't have a find-all-method declared!"); - - Method method = methods.getFindAllMethod(); + Method method = methods.getFindAllMethod() + .orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-all-method declared!")); Class[] types = method.getParameterTypes(); if (types.length == 0) { diff --git a/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java b/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java index f2cd424c0..a1e0cb8da 100755 --- a/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java +++ b/src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.*; import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.List; import java.util.Optional; @@ -92,7 +93,7 @@ public class DefaultCrudMethodsUnitTests { public void doesNotDetectInvalidlyDeclaredMethods() throws Exception { Class type = RepositoryWithInvalidPagingFindAll.class; - assertFindAllMethodOn(type, null); + assertFindAllMethodOn(type, Optional.empty()); } @Test // DATACMNS-393 @@ -132,10 +133,14 @@ public class DefaultCrudMethodsUnitTests { CrudMethods methods = getMethodsFor(RepositoryWithAllCrudMethodOverloaded.class); - assertThat(methods.getSaveMethod().isAccessible()).isTrue(); - assertThat(methods.getDeleteMethod().isAccessible()).isTrue(); - assertThat(methods.getFindAllMethod().isAccessible()).isTrue(); - assertThat(methods.getFindOneMethod().isAccessible()).isTrue(); + Arrays + .asList(methods.getSaveMethod(), methods.getDeleteMethod(), methods.getFindAllMethod(), + methods.getFindOneMethod())// + .forEach(method -> { + assertThat(method).hasValueSatisfying(it -> { + assertThat(it.isAccessible()).isTrue(); + }); + }); } private static CrudMethods getMethodsFor(Class repositoryInterface) { @@ -148,10 +153,14 @@ public class DefaultCrudMethodsUnitTests { } private static void assertFindAllMethodOn(Class type, Method method) { + assertFindAllMethodOn(type, Optional.ofNullable(method)); + } + + private static void assertFindAllMethodOn(Class type, Optional method) { CrudMethods methods = getMethodsFor(type); - assertThat(methods.hasFindAllMethod()).isEqualTo(method != null); + assertThat(methods.hasFindAllMethod()).isEqualTo(method.isPresent()); assertThat(methods.getFindAllMethod()).isEqualTo(method); } @@ -160,7 +169,7 @@ public class DefaultCrudMethodsUnitTests { CrudMethods methods = getMethodsFor(type); assertThat(methods.hasFindOneMethod()).isEqualTo(method != null); - assertThat(methods.getFindOneMethod()).isEqualTo(method); + assertThat(methods.getFindOneMethod()).hasValue(method); } private static void assertDeleteMethodOn(Class type, Method method) { @@ -168,7 +177,7 @@ public class DefaultCrudMethodsUnitTests { CrudMethods methods = getMethodsFor(type); assertThat(methods.hasDelete()).isEqualTo(method != null); - assertThat(methods.getDeleteMethod()).isEqualTo(method); + assertThat(methods.getDeleteMethod()).hasValue(method); } private static void assertSaveMethodOn(Class type, Method method) { @@ -176,7 +185,7 @@ public class DefaultCrudMethodsUnitTests { CrudMethods methods = getMethodsFor(type); assertThat(methods.hasSaveMethod()).isEqualTo(method != null); - assertThat(methods.getSaveMethod()).isEqualTo(method); + assertThat(methods.getSaveMethod()).hasValue(method); } private static void assertSaveMethodPresent(Class type, boolean present) { @@ -184,7 +193,7 @@ public class DefaultCrudMethodsUnitTests { CrudMethods methods = getMethodsFor(type); assertThat(methods.hasSaveMethod()).isEqualTo(present); - assertThat(methods.getSaveMethod()).matches(method -> present ? method != null : method == null); + assertThat(methods.getSaveMethod().isPresent()).isEqualTo(present); } interface Domain {}