Skip to content

Commit e6bf44b

Browse files
mikereichemikereiche
committed
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.
1 parent 2d4ff0d commit e6bf44b

File tree

4 files changed

+182
-3
lines changed

4 files changed

+182
-3
lines changed

‎src/main/java/org/springframework/data/couchbase/repository/support/DynamicInvocationHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
102102
new DynamicInvocationHandler<>(target, options, (String) args[0], scope));
103103
}
104104

105-
Class<?>[] paramTypes = null;
105+
Class<?>[] paramTypes = new Class[0];
106106
if (args != null) {
107107
// the CouchbaseRepository methods - save(entity) etc - will have a parameter type of Object instead of entityType
108108
// so change the paramType to match
@@ -115,7 +115,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
115115
}
116116
}
117117

118-
Method theMethod = repositoryClass.getMethod(method.getName(), paramTypes);
118+
Method theMethod = FindMethod.findMethod(repositoryClass, method.getName(), paramTypes);
119119
Object result;
120120

121121
try {

‎src/main/java/org/springframework/data/couchbase/repository/support/FindMethod.java

+158
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/*
2+
* Copyright 2021-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.couchbase.repository.support;
17+
18+
import java.lang.reflect.Method;
19+
import java.util.Vector;
20+
21+
/**
22+
* From jonskeet.uk as provided in https://groups.google.com/g/comp.lang.java.programmer/c/khq5GGXIzC4
23+
*/
24+
public class FindMethod {
25+
/**
26+
* Finds the most specific applicable method
27+
*
28+
* @param source Class to find method in
29+
* @param name Name of method to find
30+
* @param parameterTypes Parameter types to search for
31+
*/
32+
public static Method findMethod(Class source, String name, Class[] parameterTypes) throws NoSuchMethodException {
33+
return internalFind(source.getMethods(), name, parameterTypes);
34+
}
35+
36+
/**
37+
* Finds the most specific applicable declared method
38+
*
39+
* @param source Class to find method in
40+
* @param name Name of method to find
41+
* @param parameterTypes Parameter types to search for
42+
*/
43+
public static Method findDeclaredMethod(Class source, String name, Class[] parameterTypes)
44+
throws NoSuchMethodException {
45+
return internalFind(source.getDeclaredMethods(), name, parameterTypes);
46+
}
47+
48+
/**
49+
* Internal method to find the most specific applicable method
50+
*/
51+
private static Method internalFind(Method[] toTest, String name, Class[] parameterTypes)
52+
throws NoSuchMethodException {
53+
int l = parameterTypes.length;
54+
55+
// First find the applicable methods
56+
Vector applicableMethods = new Vector();
57+
for (int i = 0; i < toTest.length; i++) {
58+
// Check the name matches
59+
if (!toTest[i].getName().equals(name)) {
60+
continue;
61+
}
62+
// Check the parameters match
63+
Class[] params = toTest[i].getParameterTypes();
64+
if (params.length != l) {
65+
continue;
66+
}
67+
int j;
68+
for (j = 0; j < l; j++) {
69+
if (!params[j].isAssignableFrom(parameterTypes[j]))
70+
break;
71+
}
72+
// If so, add it to the list
73+
if (j == l) {
74+
applicableMethods.add(toTest[i]);
75+
}
76+
}
77+
78+
/*
79+
* If we've got one or zero methods, we can finish
80+
* the job now.
81+
*/
82+
int size = applicableMethods.size();
83+
if (size == 0) {
84+
throw new NoSuchMethodException("No such method: " + name);
85+
}
86+
if (size == 1) {
87+
return (Method) applicableMethods.elementAt(0);
88+
}
89+
90+
/*
91+
* Now find the most specific method. Do this in a very primitive
92+
* way - check whether each method is maximally specific. If more
93+
* than one method is maximally specific, we'll throw an exception.
94+
* For a definition of maximally specific, see JLS section 15.11.2.2.
95+
*
96+
* I'm sure there are much quicker ways - and I could probably
97+
* set the second loop to be from i+1 to size. I'd rather not though,
98+
* until I'm sure...
99+
*/
100+
int maximallySpecific = -1; // Index of maximally specific method
101+
for (int i = 0; i < size; i++) {
102+
int j;
103+
// In terms of the JLS, current is T
104+
Method current = (Method) applicableMethods.elementAt(i);
105+
Class[] currentParams = current.getParameterTypes();
106+
Class currentDeclarer = current.getDeclaringClass();
107+
for (j = 0; j < size; j++) {
108+
if (i == j) {
109+
continue;
110+
}
111+
// In terms of the JLS, test is U
112+
Method test = (Method) applicableMethods.elementAt(j);
113+
Class[] testParams = test.getParameterTypes();
114+
Class testDeclarer = test.getDeclaringClass();
115+
116+
// Check if T is a subclass of U, breaking if not
117+
if (!testDeclarer.isAssignableFrom(currentDeclarer)) {
118+
break;
119+
}
120+
121+
// Check if each parameter in T is a subclass of the
122+
// equivalent parameter in U
123+
int k;
124+
for (k = 0; k < l; k++) {
125+
if (!testParams[k].isAssignableFrom(currentParams[k])) {
126+
break;
127+
}
128+
}
129+
if (k != l) {
130+
break;
131+
}
132+
}
133+
// Maximally specific!
134+
if (j == size) {
135+
if (maximallySpecific != -1) {
136+
// if one returns Iterable and the other returns List, use the List
137+
if (size == 2) {
138+
if (((Method) applicableMethods.elementAt(0)).getReturnType() == java.lang.Iterable.class
139+
&& ((Method) applicableMethods.elementAt(1)).getReturnType() == java.util.List.class) {
140+
return (Method) applicableMethods.elementAt(1);
141+
} else if (((Method) applicableMethods.elementAt(1)).getReturnType() == java.lang.Iterable.class
142+
&& ((Method) applicableMethods.elementAt(0)).getReturnType() == java.util.List.class) {
143+
return (Method) applicableMethods.elementAt(0);
144+
}
145+
}
146+
throw new NoSuchMethodException(
147+
"Ambiguous method search - more " + "than one maximally specific method");
148+
}
149+
maximallySpecific = i;
150+
}
151+
}
152+
if (maximallySpecific == -1) {
153+
throw new NoSuchMethodException("No maximally specific method.");
154+
}
155+
return (Method) applicableMethods.elementAt(maximallySpecific);
156+
157+
}
158+
}

‎src/test/java/org/springframework/data/couchbase/domain/AirlineRepository.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616

1717
package org.springframework.data.couchbase.domain;
1818

19+
import java.util.Collection;
1920
import java.util.List;
2021

2122
import org.springframework.data.couchbase.repository.CouchbaseRepository;
2223
import org.springframework.data.couchbase.repository.DynamicProxyable;
2324
import org.springframework.data.couchbase.repository.Query;
25+
import org.springframework.data.domain.Page;
26+
import org.springframework.data.domain.Pageable;
2427
import org.springframework.data.querydsl.QuerydslPredicateExecutor;
25-
import org.springframework.data.repository.PagingAndSortingRepository;
2628
import org.springframework.data.repository.query.Param;
2729
import org.springframework.stereotype.Repository;
2830

@@ -39,4 +41,5 @@ public interface AirlineRepository extends CouchbaseRepository<Airline, String>,
3941
@Query("select meta().id as _ID, meta().cas as _CAS, #{#n1ql.bucket}.* from #{#n1ql.bucket} where #{#n1ql.filter} and (name = $1)")
4042
List<Airline> getByName_3x(@Param("airline_name") String airlineName);
4143

44+
Page<Airline> findByHqCountryIn(@Param("hqCountry") Collection<String> hqCountry, Pageable pageable);
4245
}

‎src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java

+18
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import org.springframework.data.couchbase.core.query.QueryCriteria;
6767
import org.springframework.data.couchbase.domain.Address;
6868
import org.springframework.data.couchbase.domain.AirlineRepository;
69+
import org.springframework.data.couchbase.domain.Airline;
6970
import org.springframework.data.couchbase.domain.Airport;
7071
import org.springframework.data.couchbase.domain.AirportJsonValue;
7172
import org.springframework.data.couchbase.domain.AirportJsonValueRepository;
@@ -292,6 +293,23 @@ void issue1304CollectionParameter() {
292293

293294
}
294295

296+
@Test
297+
void issuePageableDynamicProxyParameter() {
298+
Airline airline = null;
299+
try {
300+
airline = new Airline("airline::USA", "US Air", "US");
301+
airlineRepository.withScope("_default").save(airline);
302+
java.util.Collection<String> countries = new LinkedList<String>();
303+
countries.add(airline.getHqCountry());
304+
Pageable pageable = PageRequest.of(0, 1, Sort.by("hqCountry"));
305+
Page<Airline> airports2 = airlineRepository.withScope("_default").withOptions(QueryOptions.queryOptions().scanConsistency(REQUEST_PLUS)).findByHqCountryIn(countries, pageable);
306+
assertEquals(1, airports2.getTotalElements());
307+
} finally {
308+
airlineRepository.withScope("_default").delete(airline);
309+
}
310+
311+
}
312+
295313
@Test
296314
void findBySimpleProperty() {
297315
Airport vie = null;

0 commit comments

Comments
 (0)