From e6bf44b9c3a42669f9c4204a0cbca4bfb0aee515 Mon Sep 17 00:00:00 2001 From: Michael Reiche <48999328+mikereiche@users.noreply.github.com> Date: Mon, 11 Sep 2023 19:13:55 -0700 Subject: [PATCH] Allow non-exact argument type matching for DynamicProxyable. (#1819) Allow non-exact (isAssignableFrom) argument matching for methods called by DyanmicProxyable. If there are exactly two candidates, and one returns an Iterable and the other returns a List, use the method that returns the List. This is due to CouchbaseRepository defining findAll() methods as List, while PagingAndSortyRepository defines findAll() to return an Iterable. This needs to be addressed separately. Closes #1818. --- .../support/DynamicInvocationHandler.java | 4 +- .../repository/support/FindMethod.java | 158 ++++++++++++++++++ .../couchbase/domain/AirlineRepository.java | 5 +- ...chbaseRepositoryQueryIntegrationTests.java | 18 ++ 4 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/springframework/data/couchbase/repository/support/FindMethod.java diff --git a/src/main/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java b/src/main/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java index a60c5af6..08fd56c4 100644 --- a/src/main/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java +++ b/src/main/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java @@ -102,7 +102,7 @@ public class DynamicInvocationHandler implements InvocationHandler { new DynamicInvocationHandler<>(target, options, (String) args[0], scope)); } - Class[] paramTypes = null; + Class[] paramTypes = new Class[0]; if (args != null) { // the CouchbaseRepository methods - save(entity) etc - will have a parameter type of Object instead of entityType // so change the paramType to match @@ -115,7 +115,7 @@ public class DynamicInvocationHandler implements InvocationHandler { } } - Method theMethod = repositoryClass.getMethod(method.getName(), paramTypes); + Method theMethod = FindMethod.findMethod(repositoryClass, method.getName(), paramTypes); Object result; try { diff --git a/src/main/java/org/springframework/data/couchbase/repository/support/FindMethod.java b/src/main/java/org/springframework/data/couchbase/repository/support/FindMethod.java new file mode 100644 index 00000000..d45eff57 --- /dev/null +++ b/src/main/java/org/springframework/data/couchbase/repository/support/FindMethod.java @@ -0,0 +1,158 @@ +/* + * Copyright 2021-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.couchbase.repository.support; + +import java.lang.reflect.Method; +import java.util.Vector; + +/** + * From jonskeet.uk as provided in https://groups.google.com/g/comp.lang.java.programmer/c/khq5GGXIzC4 + */ +public class FindMethod { + /** + * Finds the most specific applicable method + * + * @param source Class to find method in + * @param name Name of method to find + * @param parameterTypes Parameter types to search for + */ + public static Method findMethod(Class source, String name, Class[] parameterTypes) throws NoSuchMethodException { + return internalFind(source.getMethods(), name, parameterTypes); + } + + /** + * Finds the most specific applicable declared method + * + * @param source Class to find method in + * @param name Name of method to find + * @param parameterTypes Parameter types to search for + */ + public static Method findDeclaredMethod(Class source, String name, Class[] parameterTypes) + throws NoSuchMethodException { + return internalFind(source.getDeclaredMethods(), name, parameterTypes); + } + + /** + * Internal method to find the most specific applicable method + */ + private static Method internalFind(Method[] toTest, String name, Class[] parameterTypes) + throws NoSuchMethodException { + int l = parameterTypes.length; + + // First find the applicable methods + Vector applicableMethods = new Vector(); + for (int i = 0; i < toTest.length; i++) { + // Check the name matches + if (!toTest[i].getName().equals(name)) { + continue; + } + // Check the parameters match + Class[] params = toTest[i].getParameterTypes(); + if (params.length != l) { + continue; + } + int j; + for (j = 0; j < l; j++) { + if (!params[j].isAssignableFrom(parameterTypes[j])) + break; + } + // If so, add it to the list + if (j == l) { + applicableMethods.add(toTest[i]); + } + } + + /* + * If we've got one or zero methods, we can finish + * the job now. + */ + int size = applicableMethods.size(); + if (size == 0) { + throw new NoSuchMethodException("No such method: " + name); + } + if (size == 1) { + return (Method) applicableMethods.elementAt(0); + } + + /* + * Now find the most specific method. Do this in a very primitive + * way - check whether each method is maximally specific. If more + * than one method is maximally specific, we'll throw an exception. + * For a definition of maximally specific, see JLS section 15.11.2.2. + * + * I'm sure there are much quicker ways - and I could probably + * set the second loop to be from i+1 to size. I'd rather not though, + * until I'm sure... + */ + int maximallySpecific = -1; // Index of maximally specific method + for (int i = 0; i < size; i++) { + int j; + // In terms of the JLS, current is T + Method current = (Method) applicableMethods.elementAt(i); + Class[] currentParams = current.getParameterTypes(); + Class currentDeclarer = current.getDeclaringClass(); + for (j = 0; j < size; j++) { + if (i == j) { + continue; + } + // In terms of the JLS, test is U + Method test = (Method) applicableMethods.elementAt(j); + Class[] testParams = test.getParameterTypes(); + Class testDeclarer = test.getDeclaringClass(); + + // Check if T is a subclass of U, breaking if not + if (!testDeclarer.isAssignableFrom(currentDeclarer)) { + break; + } + + // Check if each parameter in T is a subclass of the + // equivalent parameter in U + int k; + for (k = 0; k < l; k++) { + if (!testParams[k].isAssignableFrom(currentParams[k])) { + break; + } + } + if (k != l) { + break; + } + } + // Maximally specific! + if (j == size) { + if (maximallySpecific != -1) { + // if one returns Iterable and the other returns List, use the List + if (size == 2) { + if (((Method) applicableMethods.elementAt(0)).getReturnType() == java.lang.Iterable.class + && ((Method) applicableMethods.elementAt(1)).getReturnType() == java.util.List.class) { + return (Method) applicableMethods.elementAt(1); + } else if (((Method) applicableMethods.elementAt(1)).getReturnType() == java.lang.Iterable.class + && ((Method) applicableMethods.elementAt(0)).getReturnType() == java.util.List.class) { + return (Method) applicableMethods.elementAt(0); + } + } + throw new NoSuchMethodException( + "Ambiguous method search - more " + "than one maximally specific method"); + } + maximallySpecific = i; + } + } + if (maximallySpecific == -1) { + throw new NoSuchMethodException("No maximally specific method."); + } + return (Method) applicableMethods.elementAt(maximallySpecific); + + } +} diff --git a/src/test/java/org/springframework/data/couchbase/domain/AirlineRepository.java b/src/test/java/org/springframework/data/couchbase/domain/AirlineRepository.java index 8d1e964b..814b6360 100644 --- a/src/test/java/org/springframework/data/couchbase/domain/AirlineRepository.java +++ b/src/test/java/org/springframework/data/couchbase/domain/AirlineRepository.java @@ -16,13 +16,15 @@ package org.springframework.data.couchbase.domain; +import java.util.Collection; import java.util.List; import org.springframework.data.couchbase.repository.CouchbaseRepository; import org.springframework.data.couchbase.repository.DynamicProxyable; import org.springframework.data.couchbase.repository.Query; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.data.querydsl.QuerydslPredicateExecutor; -import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; @@ -39,4 +41,5 @@ public interface AirlineRepository extends CouchbaseRepository, @Query("select meta().id as _ID, meta().cas as _CAS, #{#n1ql.bucket}.* from #{#n1ql.bucket} where #{#n1ql.filter} and (name = $1)") List getByName_3x(@Param("airline_name") String airlineName); + Page findByHqCountryIn(@Param("hqCountry") Collection hqCountry, Pageable pageable); } diff --git a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java b/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java index 47b3fe2a..38556901 100644 --- a/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java @@ -66,6 +66,7 @@ import org.springframework.data.couchbase.core.query.N1QLExpression; import org.springframework.data.couchbase.core.query.QueryCriteria; import org.springframework.data.couchbase.domain.Address; import org.springframework.data.couchbase.domain.AirlineRepository; +import org.springframework.data.couchbase.domain.Airline; import org.springframework.data.couchbase.domain.Airport; import org.springframework.data.couchbase.domain.AirportJsonValue; import org.springframework.data.couchbase.domain.AirportJsonValueRepository; @@ -292,6 +293,23 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr } + @Test + void issuePageableDynamicProxyParameter() { + Airline airline = null; + try { + airline = new Airline("airline::USA", "US Air", "US"); + airlineRepository.withScope("_default").save(airline); + java.util.Collection countries = new LinkedList(); + countries.add(airline.getHqCountry()); + Pageable pageable = PageRequest.of(0, 1, Sort.by("hqCountry")); + Page airports2 = airlineRepository.withScope("_default").withOptions(QueryOptions.queryOptions().scanConsistency(REQUEST_PLUS)).findByHqCountryIn(countries, pageable); + assertEquals(1, airports2.getTotalElements()); + } finally { + airlineRepository.withScope("_default").delete(airline); + } + + } + @Test void findBySimpleProperty() { Airport vie = null;