Browse Source

DATACMNS-867 - Optionals in CrudMethods API.

pull/194/head
Oliver Gierke 9 years ago
parent
commit
fd2c400c4e
  1. 9
      src/main/java/org/springframework/data/repository/core/CrudMethods.java
  2. 21
      src/main/java/org/springframework/data/repository/core/support/AbstractRepositoryMetadata.java
  3. 55
      src/main/java/org/springframework/data/repository/core/support/DefaultCrudMethods.java
  4. 5
      src/main/java/org/springframework/data/repository/support/CrudRepositoryInvoker.java
  5. 26
      src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java
  6. 29
      src/test/java/org/springframework/data/repository/core/support/DefaultCrudMethodsUnitTests.java

9
src/main/java/org/springframework/data/repository/core/CrudMethods.java

@ -16,6 +16,7 @@
package org.springframework.data.repository.core; package org.springframework.data.repository.core;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Optional;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort; 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. * @return the method to save entities or {@literal null} if noen exposed.
* @see #hasSaveMethod() * @see #hasSaveMethod()
*/ */
Method getSaveMethod(); Optional<Method> getSaveMethod();
/** /**
* Returns whether the repository exposes a save method at all. * 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. * @return the find all method of the repository or {@literal null} if not available.
* @see #hasFindAllMethod() * @see #hasFindAllMethod()
*/ */
Method getFindAllMethod(); Optional<Method> getFindAllMethod();
/** /**
* Returns whether the repository exposes a find all method at all. * 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. * @return the find one method of the repository or {@literal null} if not available.
* @see #hasFindOneMethod() * @see #hasFindOneMethod()
*/ */
Method getFindOneMethod(); Optional<Method> getFindOneMethod();
/** /**
* Returns whether the repository exposes a find one method. * 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. * @return the delete method of the repository or {@literal null} if not available.
* @see #hasDelete() * @see #hasDelete()
*/ */
Method getDeleteMethod(); Optional<Method> getDeleteMethod();
/** /**
* Returns whether the repository esposes a delete method. * Returns whether the repository esposes a delete method.

21
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.QueryExecutionConverters;
import org.springframework.data.repository.util.ReactiveWrappers; import org.springframework.data.repository.util.ReactiveWrappers;
import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.ClassTypeInformation;
import org.springframework.data.util.Lazy;
import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.ReflectionUtils;
import org.springframework.data.util.TypeInformation; import org.springframework.data.util.TypeInformation;
import org.springframework.util.Assert; import org.springframework.util.Assert;
@ -41,7 +42,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
private final TypeInformation<?> typeInformation; private final TypeInformation<?> typeInformation;
private final Class<?> repositoryInterface; private final Class<?> repositoryInterface;
private CrudMethods crudMethods; private final Lazy<CrudMethods> crudMethods;
/** /**
* Creates a new {@link AbstractRepositoryMetadata}. * Creates a new {@link AbstractRepositoryMetadata}.
@ -55,6 +56,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
this.repositoryInterface = repositoryInterface; this.repositoryInterface = repositoryInterface;
this.typeInformation = ClassTypeInformation.from(repositoryInterface); this.typeInformation = ClassTypeInformation.from(repositoryInterface);
this.crudMethods = Lazy.of(() -> new DefaultCrudMethods(this));
} }
/** /**
@ -94,12 +96,7 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
*/ */
@Override @Override
public CrudMethods getCrudMethods() { public CrudMethods getCrudMethods() {
return this.crudMethods.get();
if (this.crudMethods == null) {
this.crudMethods = new DefaultCrudMethods(this);
}
return this.crudMethods;
} }
/* /*
@ -109,13 +106,9 @@ public abstract class AbstractRepositoryMetadata implements RepositoryMetadata {
@Override @Override
public boolean isPagingRepository() { public boolean isPagingRepository() {
Method findAllMethod = getCrudMethods().getFindAllMethod(); return getCrudMethods().getFindAllMethod()//
.map(it -> Arrays.asList(it.getParameterTypes()).contains(Pageable.class))//
if (findAllMethod == null) { .orElse(false);
return false;
}
return Arrays.asList(findAllMethod.getParameterTypes()).contains(Pageable.class);
} }
/* /*

55
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.io.Serializable;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Optional;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort; 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 FIND_ALL = "findAll";
private static final String DELETE = "delete"; private static final String DELETE = "delete";
private final Method findAllMethod; private final Optional<Method> findAllMethod;
private final Method findOneMethod; private final Optional<Method> findOneMethod;
private final Method saveMethod; private final Optional<Method> saveMethod;
private final Method deleteMethod; private final Optional<Method> deleteMethod;
/** /**
* Creates a new {@link DefaultCrudMethods} using the given {@link RepositoryMetadata}. * 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}. * @param metadata must not be {@literal null}.
* @return the most suitable method or {@literal null} if no method could be found. * @return the most suitable method or {@literal null} if no method could be found.
*/ */
private Method selectMostSuitableSaveMethod(RepositoryMetadata metadata) { private Optional<Method> selectMostSuitableSaveMethod(RepositoryMetadata metadata) {
for (Class<?> type : asList(metadata.getDomainType(), Object.class)) { 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}. * @param metadata must not be {@literal null}.
* @return the most suitable method or {@literal null} if no method could be found. * @return the most suitable method or {@literal null} if no method could be found.
*/ */
private Method selectMostSuitableDeleteMethod(RepositoryMetadata metadata) { private Optional<Method> selectMostSuitableDeleteMethod(RepositoryMetadata metadata) {
for (Class<?> type : asList(metadata.getDomainType(), metadata.getIdType(), Serializable.class, Iterable.class)) { 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}. * @param metadata must not be {@literal null}.
* @return the most suitable method or {@literal null} if no method could be found. * @return the most suitable method or {@literal null} if no method could be found.
*/ */
private Method selectMostSuitableFindAllMethod(RepositoryMetadata metadata) { private Optional<Method> selectMostSuitableFindAllMethod(RepositoryMetadata metadata) {
for (Class<?> type : asList(Pageable.class, Sort.class)) { 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 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}. * @param metadata must not be {@literal null}.
* @return the most suitable method or {@literal null} if no method could be found. * @return the most suitable method or {@literal null} if no method could be found.
*/ */
private Method selectMostSuitableFindOneMethod(RepositoryMetadata metadata) { private Optional<Method> selectMostSuitableFindOneMethod(RepositoryMetadata metadata) {
for (Class<?> type : asList(metadata.getIdType(), Serializable.class)) { 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) * @see ClassUtils#getMostSpecificMethod(Method, Class)
* @return * @return
*/ */
private static Method getMostSpecificMethod(Method method, Class<?> type) { private static Optional<Method> getMostSpecificMethod(Method method, Class<?> type) {
Method result = ClassUtils.getMostSpecificMethod(method, type); return Optional.ofNullable(ClassUtils.getMostSpecificMethod(method, type)).map(it -> {
ReflectionUtils.makeAccessible(it);
if (result == null) { return it;
return null; });
}
ReflectionUtils.makeAccessible(result);
return result;
} }
/* /*
@ -200,7 +197,7 @@ public class DefaultCrudMethods implements CrudMethods {
* @see org.springframework.data.repository.core.support.CrudMethods#getSaveMethod() * @see org.springframework.data.repository.core.support.CrudMethods#getSaveMethod()
*/ */
@Override @Override
public Method getSaveMethod() { public Optional<Method> getSaveMethod() {
return saveMethod; return saveMethod;
} }
@ -210,7 +207,7 @@ public class DefaultCrudMethods implements CrudMethods {
*/ */
@Override @Override
public boolean hasSaveMethod() { 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() * @see org.springframework.data.repository.core.support.CrudMethods#getFindAllMethod()
*/ */
@Override @Override
public Method getFindAllMethod() { public Optional<Method> getFindAllMethod() {
return findAllMethod; return findAllMethod;
} }
@ -228,7 +225,7 @@ public class DefaultCrudMethods implements CrudMethods {
*/ */
@Override @Override
public boolean hasFindAllMethod() { 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() * @see org.springframework.data.repository.core.support.CrudMethods#getFindOneMethod()
*/ */
@Override @Override
public Method getFindOneMethod() { public Optional<Method> getFindOneMethod() {
return findOneMethod; return findOneMethod;
} }
@ -246,7 +243,7 @@ public class DefaultCrudMethods implements CrudMethods {
*/ */
@Override @Override
public boolean hasFindOneMethod() { public boolean hasFindOneMethod() {
return findOneMethod != null; return findOneMethod.isPresent();
} }
/* /*
@ -255,7 +252,7 @@ public class DefaultCrudMethods implements CrudMethods {
*/ */
@Override @Override
public boolean hasDelete() { 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() * @see org.springframework.data.repository.core.support.CrudMethods#getDeleteMethod()
*/ */
@Override @Override
public Method getDeleteMethod() { public Optional<Method> getDeleteMethod() {
return this.deleteMethod; return this.deleteMethod;
} }
} }

5
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.io.Serializable;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Optional;
import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConversionService;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
@ -114,7 +115,7 @@ class CrudRepositoryInvoker extends ReflectionRepositoryInvoker {
} }
} }
private boolean isRedeclaredMethod(Method method) { protected boolean isRedeclaredMethod(Optional<Method> method) {
return !method.getDeclaringClass().equals(CrudRepository.class); return method.map(it -> !it.getDeclaringClass().equals(CrudRepository.class)).orElse(false);
} }
} }

26
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) * @see org.springframework.data.rest.core.invoke.RepositoryInvoker#invokeSave(java.lang.Object)
*/ */
@Override @Override
@SuppressWarnings("unchecked")
public <T> T invokeSave(T object) { public <T> T invokeSave(T object) {
Assert.state(hasSaveMethod(), "Repository doesn't have a save-method declared!"); return methods.getSaveMethod()//
return invoke(methods.getSaveMethod(), object); .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) * @see org.springframework.data.rest.core.invoke.RepositoryInvoker#invokeFindOne(java.io.Serializable)
*/ */
@Override @Override
@SuppressWarnings("unchecked")
public <T> T invokeFindOne(Serializable id) { public <T> T invokeFindOne(Serializable id) {
Assert.state(hasFindOneMethod(), "Repository doesn't have a find-one-method declared!"); return methods.getFindOneMethod()//
return invoke(methods.getFindOneMethod(), convertId(id)); .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) { public void invokeDelete(Serializable id) {
Assert.notNull(id, "Identifier must not be null!"); 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]; Class<?> parameterType = method.getParameterTypes()[0];
List<Class<? extends Serializable>> idTypes = Arrays.asList(idType, Serializable.class); List<Class<? extends Serializable>> idTypes = Arrays.asList(idType, Serializable.class);
@ -260,9 +264,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker {
protected Iterable<Object> invokePagedFindAllReflectively(Pageable pageable) { protected Iterable<Object> invokePagedFindAllReflectively(Pageable pageable) {
Assert.state(hasFindAllMethod(), "Repository doesn't have a find-all-method declared!"); Method method = methods.getFindAllMethod()
.orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-all-method declared!"));
Method method = methods.getFindAllMethod();
Class<?>[] types = method.getParameterTypes(); Class<?>[] types = method.getParameterTypes();
if (types.length == 0) { if (types.length == 0) {
@ -278,9 +281,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker {
protected Iterable<Object> invokeSortedFindAllReflectively(Sort sort) { protected Iterable<Object> invokeSortedFindAllReflectively(Sort sort) {
Assert.state(hasFindAllMethod(), "Repository doesn't have a find-all-method declared!"); Method method = methods.getFindAllMethod()
.orElseThrow(() -> new IllegalStateException("Repository doesn't have a find-all-method declared!"));
Method method = methods.getFindAllMethod();
Class<?>[] types = method.getParameterTypes(); Class<?>[] types = method.getParameterTypes();
if (types.length == 0) { if (types.length == 0) {

29
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.io.Serializable;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
@ -92,7 +93,7 @@ public class DefaultCrudMethodsUnitTests {
public void doesNotDetectInvalidlyDeclaredMethods() throws Exception { public void doesNotDetectInvalidlyDeclaredMethods() throws Exception {
Class<RepositoryWithInvalidPagingFindAll> type = RepositoryWithInvalidPagingFindAll.class; Class<RepositoryWithInvalidPagingFindAll> type = RepositoryWithInvalidPagingFindAll.class;
assertFindAllMethodOn(type, null); assertFindAllMethodOn(type, Optional.empty());
} }
@Test // DATACMNS-393 @Test // DATACMNS-393
@ -132,10 +133,14 @@ public class DefaultCrudMethodsUnitTests {
CrudMethods methods = getMethodsFor(RepositoryWithAllCrudMethodOverloaded.class); CrudMethods methods = getMethodsFor(RepositoryWithAllCrudMethodOverloaded.class);
assertThat(methods.getSaveMethod().isAccessible()).isTrue(); Arrays
assertThat(methods.getDeleteMethod().isAccessible()).isTrue(); .asList(methods.getSaveMethod(), methods.getDeleteMethod(), methods.getFindAllMethod(),
assertThat(methods.getFindAllMethod().isAccessible()).isTrue(); methods.getFindOneMethod())//
assertThat(methods.getFindOneMethod().isAccessible()).isTrue(); .forEach(method -> {
assertThat(method).hasValueSatisfying(it -> {
assertThat(it.isAccessible()).isTrue();
});
});
} }
private static CrudMethods getMethodsFor(Class<?> repositoryInterface) { private static CrudMethods getMethodsFor(Class<?> repositoryInterface) {
@ -148,10 +153,14 @@ public class DefaultCrudMethodsUnitTests {
} }
private static void assertFindAllMethodOn(Class<?> type, Method method) { private static void assertFindAllMethodOn(Class<?> type, Method method) {
assertFindAllMethodOn(type, Optional.ofNullable(method));
}
private static void assertFindAllMethodOn(Class<?> type, Optional<Method> method) {
CrudMethods methods = getMethodsFor(type); CrudMethods methods = getMethodsFor(type);
assertThat(methods.hasFindAllMethod()).isEqualTo(method != null); assertThat(methods.hasFindAllMethod()).isEqualTo(method.isPresent());
assertThat(methods.getFindAllMethod()).isEqualTo(method); assertThat(methods.getFindAllMethod()).isEqualTo(method);
} }
@ -160,7 +169,7 @@ public class DefaultCrudMethodsUnitTests {
CrudMethods methods = getMethodsFor(type); CrudMethods methods = getMethodsFor(type);
assertThat(methods.hasFindOneMethod()).isEqualTo(method != null); assertThat(methods.hasFindOneMethod()).isEqualTo(method != null);
assertThat(methods.getFindOneMethod()).isEqualTo(method); assertThat(methods.getFindOneMethod()).hasValue(method);
} }
private static void assertDeleteMethodOn(Class<?> type, Method method) { private static void assertDeleteMethodOn(Class<?> type, Method method) {
@ -168,7 +177,7 @@ public class DefaultCrudMethodsUnitTests {
CrudMethods methods = getMethodsFor(type); CrudMethods methods = getMethodsFor(type);
assertThat(methods.hasDelete()).isEqualTo(method != null); assertThat(methods.hasDelete()).isEqualTo(method != null);
assertThat(methods.getDeleteMethod()).isEqualTo(method); assertThat(methods.getDeleteMethod()).hasValue(method);
} }
private static void assertSaveMethodOn(Class<?> type, Method method) { private static void assertSaveMethodOn(Class<?> type, Method method) {
@ -176,7 +185,7 @@ public class DefaultCrudMethodsUnitTests {
CrudMethods methods = getMethodsFor(type); CrudMethods methods = getMethodsFor(type);
assertThat(methods.hasSaveMethod()).isEqualTo(method != null); assertThat(methods.hasSaveMethod()).isEqualTo(method != null);
assertThat(methods.getSaveMethod()).isEqualTo(method); assertThat(methods.getSaveMethod()).hasValue(method);
} }
private static void assertSaveMethodPresent(Class<?> type, boolean present) { private static void assertSaveMethodPresent(Class<?> type, boolean present) {
@ -184,7 +193,7 @@ public class DefaultCrudMethodsUnitTests {
CrudMethods methods = getMethodsFor(type); CrudMethods methods = getMethodsFor(type);
assertThat(methods.hasSaveMethod()).isEqualTo(present); assertThat(methods.hasSaveMethod()).isEqualTo(present);
assertThat(methods.getSaveMethod()).matches(method -> present ? method != null : method == null); assertThat(methods.getSaveMethod().isPresent()).isEqualTo(present);
} }
interface Domain {} interface Domain {}

Loading…
Cancel
Save