From 22223f4386ae0aa53fb2632ac5c0c4b3d61705b2 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 12 Nov 2013 14:01:33 +0100 Subject: [PATCH] DATACMNS-393 - Improved CRUD method detection in DefaultCrudMethods. Slightly refined the detection algorithm to favor early returns and thus avoid unnecessary reflection lookups. Original pull request: #55. --- .../core/support/DefaultCrudMethods.java | 147 ++++++++---------- 1 file changed, 66 insertions(+), 81 deletions(-) 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 a2d296399..44659c45a 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 @@ -15,8 +15,12 @@ */ package org.springframework.data.repository.core.support; +import static org.springframework.util.ClassUtils.*; +import static org.springframework.util.ReflectionUtils.*; + import java.io.Serializable; import java.lang.reflect.Method; +import java.util.Arrays; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; @@ -26,8 +30,6 @@ import org.springframework.data.repository.core.CrudMethods; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; -import org.springframework.util.ReflectionUtils; /** * Default implementation to discover CRUD methods based on a given {@link RepositoryInformation}. Will detect methods @@ -57,7 +59,7 @@ class DefaultCrudMethods implements CrudMethods { */ public DefaultCrudMethods(RepositoryInformation information) { - Assert.notNull(information, "information must not be null!"); + Assert.notNull(information, "RepositoryInformation must not be null!"); this.findOneMethod = selectMostSuitableFindOneMethod(information); this.findAllMethod = selectMostSuitableFindAllMethod(information); @@ -68,129 +70,105 @@ class DefaultCrudMethods implements CrudMethods { /** * The most suitable save method is selected as follows: We prefer *
    - *
  1. a {@link RepositoryMetadata#getDomainType()} as first parameter over - *
  2. - *
  3. an {@link Object} as first parameter - *
  4. + *
  5. a {@link RepositoryMetadata#getDomainType()} as first parameter over
    6. + *
  6. an {@link Object} as first parameter
    7. *
* * @param information must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ + @SuppressWarnings("unchecked") private Method selectMostSuitableSaveMethod(RepositoryInformation information) { - Assert.notNull(information, "information must not be null!"); + for (Class type : Arrays.asList(information.getDomainType(), Object.class)) { + Method saveMethodCandidate = findMethod(information.getRepositoryInterface(), SAVE, type); + if (saveMethodCandidate != null) { + return getMostSpecificMethod(saveMethodCandidate, information.getRepositoryInterface()); + } + } - Method saveMethodCandidate = ReflectionUtils.findMethod(information.getRepositoryInterface(), SAVE, - information.getDomainType()); - return ClassUtils.getMostSpecificMethod( - saveMethodCandidate != null ? saveMethodCandidate : ReflectionUtils.findMethod( - information.getRepositoryInterface(), SAVE, Object.class), information.getRepositoryInterface()); + return null; } /** * The most suitable delete method is selected as follows: We prefer *
    - *
  1. a {@link RepositoryMetadata#getIdType()} as first parameter over - *
  2. - *
  3. a {@link Serializable} as first parameter over - *
  4. - *
  5. an {@link Iterable} as first parameter - *
  6. + *
  7. a {@link RepositoryMetadata#getIdType()} as first parameter over
    8. + *
  8. a {@link Serializable} as first parameter over
    9. + *
  9. an {@link Iterable} as first parameter
    10. *
* * @param information must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ + @SuppressWarnings("unchecked") private Method selectMostSuitableDeleteMethod(RepositoryInformation information) { - Assert.notNull(information, "information must not be null!"); + for (Class type : Arrays.asList(information.getIdType(), Serializable.class, Iterable.class)) { + Method candidate = findMethod(information.getRepositoryInterface(), DELETE, type); + if (candidate != null) { + return getMostSpecificMethod(candidate, information.getRepositoryInterface()); + } + } - Method deleteMethodCandiate = ReflectionUtils.findMethod(information.getRepositoryInterface(), DELETE, - information.getIdType()); - - deleteMethodCandiate = deleteMethodCandiate != null ? deleteMethodCandiate : ReflectionUtils.findMethod( - information.getRepositoryInterface(), DELETE, Serializable.class); - - deleteMethodCandiate = deleteMethodCandiate != null ? deleteMethodCandiate : ReflectionUtils.findMethod( - information.getRepositoryInterface(), DELETE, Iterable.class); - - return ClassUtils.getMostSpecificMethod(deleteMethodCandiate, information.getRepositoryInterface()); + return null; } /** * The most suitable findAll method is selected as follows: We prefer *
    - *
  1. a {@link Pageable} as first parameter over - *
  2. - *
  3. a {@link Sort} as first parameter over - *
  4. - *
  5. no parameters - *
  6. + *
  7. a {@link Pageable} as first parameter over
    8. + *
  8. a {@link Sort} as first parameter over
    9. + *
  9. no parameters
    10. *
* * @param information must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ + @SuppressWarnings("unchecked") private Method selectMostSuitableFindAllMethod(RepositoryInformation information) { - Assert.notNull(information, "information must not be null!"); - - Method findAllCandidateMethod = null; - if (ClassUtils.hasMethod(information.getRepositoryInterface(), FIND_ALL)) { - findAllCandidateMethod = ClassUtils.getMostSpecificMethod( - ReflectionUtils.findMethod(CrudRepository.class, FIND_ALL), information.getRepositoryInterface()); + for (Class type : Arrays.asList(Pageable.class, Sort.class)) { + if (hasMethod(information.getRepositoryInterface(), FIND_ALL, type)) { + Method candidate = findMethod(PagingAndSortingRepository.class, FIND_ALL, type); + if (candidate != null) { + return getMostSpecificMethod(candidate, information.getRepositoryInterface()); + } + } } - Method findAllPageableCandidateMethod = null; - if (ClassUtils.hasMethod(information.getRepositoryInterface(), FIND_ALL, Pageable.class)) { - findAllPageableCandidateMethod = ClassUtils.getMostSpecificMethod( - ReflectionUtils.findMethod(PagingAndSortingRepository.class, FIND_ALL, Pageable.class), - information.getRepositoryInterface()); + if (hasMethod(information.getRepositoryInterface(), FIND_ALL)) { + return getMostSpecificMethod(findMethod(CrudRepository.class, FIND_ALL), information.getRepositoryInterface()); } - Method findAllSortCandidateMethod = null; - if (ClassUtils.hasMethod(information.getRepositoryInterface(), FIND_ALL, Sort.class)) { - findAllSortCandidateMethod = ClassUtils.getMostSpecificMethod( - ReflectionUtils.findMethod(PagingAndSortingRepository.class, FIND_ALL, Sort.class), - information.getRepositoryInterface()); - } - - if (findAllPageableCandidateMethod != null) { - return findAllPageableCandidateMethod; - } - - if (findAllSortCandidateMethod != null) { - return findAllSortCandidateMethod; - } - - return findAllCandidateMethod; + return null; } /** * The most suitable findOne method is selected as follows: We prefer *
    - *
  1. a {@link RepositoryMetadata#getIdType()} as first parameter over - *
  2. - *
  3. a {@link Serializable} as first parameter - *
  4. + *
  5. a {@link RepositoryMetadata#getIdType()} as first parameter over
    6. + *
  6. a {@link Serializable} as first parameter
    7. *
* * @param information must not be {@literal null}. * @return the most suitable method or {@literal null} if no method could be found. */ + @SuppressWarnings("unchecked") private Method selectMostSuitableFindOneMethod(RepositoryInformation information) { - Assert.notNull(information, "information must not be null!"); + for (Class type : Arrays.asList(information.getIdType(), Serializable.class)) { + Method candidate = findMethod(information.getRepositoryInterface(), FIND_ONE, type); + if (candidate != null) { + return getMostSpecificMethod(candidate, information.getRepositoryInterface()); + } + } - Method findOneMethodCandidate = ReflectionUtils.findMethod(information.getRepositoryInterface(), FIND_ONE, - information.getIdType()); - return ClassUtils.getMostSpecificMethod( - findOneMethodCandidate != null ? findOneMethodCandidate : ReflectionUtils.findMethod( - information.getRepositoryInterface(), FIND_ONE, Serializable.class), information.getRepositoryInterface()); + return null; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#getSaveMethod() */ @Override @@ -198,7 +176,8 @@ class DefaultCrudMethods implements CrudMethods { return saveMethod; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#hasSaveMethod() */ @Override @@ -206,7 +185,8 @@ class DefaultCrudMethods implements CrudMethods { return saveMethod != null; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#getFindAllMethod() */ @Override @@ -214,7 +194,8 @@ class DefaultCrudMethods implements CrudMethods { return findAllMethod; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#hasFindAllMethod() */ @Override @@ -222,7 +203,8 @@ class DefaultCrudMethods implements CrudMethods { return findAllMethod != null; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#getFindOneMethod() */ @Override @@ -230,7 +212,8 @@ class DefaultCrudMethods implements CrudMethods { return findOneMethod; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#hasFindOneMethod() */ @Override @@ -238,7 +221,8 @@ class DefaultCrudMethods implements CrudMethods { return findOneMethod != null; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#hasDelete() */ @Override @@ -246,7 +230,8 @@ class DefaultCrudMethods implements CrudMethods { return this.deleteMethod != null; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.repository.core.support.CrudMethods#getDeleteMethod() */ @Override