Skip to content

Commit 6bd28a1

Browse files
committed
Simplify DefaultMethodInvokingMethodInterceptor.
As our baseline is now Java 17, we can remove all indirections to produce a Lookup previously needed to support Java 8 and 9. Fixes #2971.
1 parent be43b61 commit 6bd28a1

File tree

2 files changed

+29
-164
lines changed

2 files changed

+29
-164
lines changed

src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java

Lines changed: 14 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.lang.invoke.MethodHandles;
2020
import java.lang.invoke.MethodHandles.Lookup;
2121
import java.lang.invoke.MethodType;
22-
import java.lang.reflect.Constructor;
2322
import java.lang.reflect.Method;
2423
import java.lang.reflect.Modifier;
2524
import java.util.Map;
@@ -28,7 +27,6 @@
2827
import org.aopalliance.intercept.MethodInterceptor;
2928
import org.aopalliance.intercept.MethodInvocation;
3029
import org.springframework.aop.ProxyMethodInvocation;
31-
import org.springframework.data.util.Lazy;
3230
import org.springframework.lang.Nullable;
3331
import org.springframework.util.ConcurrentReferenceHashMap;
3432
import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType;
@@ -44,7 +42,7 @@
4442
*/
4543
public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor {
4644

47-
private final MethodHandleLookup methodHandleLookup = MethodHandleLookup.getMethodHandleLookup();
45+
private static final Lookup LOOKUP = MethodHandles.lookup();
4846
private final Map<Method, MethodHandle> methodHandleCache = new ConcurrentReferenceHashMap<>(10, ReferenceType.WEAK);
4947

5048
/**
@@ -64,7 +62,7 @@ public static boolean hasDefaultMethods(Class<?> interfaceClass) {
6462

6563
@Nullable
6664
@Override
67-
public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable {
65+
public Object invoke(MethodInvocation invocation) throws Throwable {
6866

6967
Method method = invocation.getMethod();
7068

@@ -84,161 +82,28 @@ private MethodHandle getMethodHandle(Method method) throws Exception {
8482

8583
if (handle == null) {
8684

87-
handle = methodHandleLookup.lookup(method);
85+
handle = lookup(method);
8886
methodHandleCache.put(method, handle);
8987
}
9088

9189
return handle;
9290
}
9391

9492
/**
95-
* Strategies for {@link MethodHandle} lookup.
93+
* Lookup a {@link MethodHandle} given {@link Method} to look up.
9694
*
97-
* @since 2.0
95+
* @param method must not be {@literal null}.
96+
* @return the method handle.
97+
* @throws ReflectiveOperationException
9898
*/
99-
enum MethodHandleLookup {
99+
private static MethodHandle lookup(Method method) throws ReflectiveOperationException {
100100

101-
/**
102-
* Encapsulated {@link MethodHandle} lookup working on Java 9.
103-
*/
104-
ENCAPSULATED {
101+
Lookup lookup = MethodHandles.privateLookupIn(method.getDeclaringClass(), LOOKUP);
102+
MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes());
103+
Class<?> declaringClass = method.getDeclaringClass();
105104

106-
private final @Nullable Method privateLookupIn = ReflectionUtils.findMethod(MethodHandles.class,
107-
"privateLookupIn", Class.class, Lookup.class);
108-
109-
@Override
110-
MethodHandle lookup(Method method) throws ReflectiveOperationException {
111-
112-
if (privateLookupIn == null) {
113-
throw new IllegalStateException("Could not obtain MethodHandles.privateLookupIn");
114-
}
115-
116-
return doLookup(method, getLookup(method.getDeclaringClass(), privateLookupIn));
117-
}
118-
119-
@Override
120-
boolean isAvailable() {
121-
return privateLookupIn != null;
122-
}
123-
124-
private Lookup getLookup(Class<?> declaringClass, Method privateLookupIn) {
125-
126-
Lookup lookup = MethodHandles.lookup();
127-
128-
try {
129-
return (Lookup) privateLookupIn.invoke(MethodHandles.class, declaringClass, lookup);
130-
} catch (ReflectiveOperationException e) {
131-
return lookup;
132-
}
133-
}
134-
},
135-
136-
/**
137-
* Open (via reflection construction of {@link MethodHandles.Lookup}) method handle lookup. Works with Java 8 and
138-
* with Java 9 permitting illegal access.
139-
*/
140-
OPEN {
141-
142-
private final Lazy<Constructor<Lookup>> constructor = Lazy.of(MethodHandleLookup::getLookupConstructor);
143-
144-
@Override
145-
MethodHandle lookup(Method method) throws ReflectiveOperationException {
146-
147-
if (!isAvailable()) {
148-
throw new IllegalStateException("Could not obtain MethodHandles.lookup constructor");
149-
}
150-
151-
Constructor<Lookup> constructor = this.constructor.get();
152-
153-
return constructor.newInstance(method.getDeclaringClass()).unreflectSpecial(method, method.getDeclaringClass());
154-
}
155-
156-
@Override
157-
boolean isAvailable() {
158-
return constructor.orElse(null) != null;
159-
}
160-
},
161-
162-
/**
163-
* Fallback {@link MethodHandle} lookup using {@link MethodHandles#lookup() public lookup}.
164-
*
165-
* @since 2.1
166-
*/
167-
FALLBACK {
168-
169-
@Override
170-
MethodHandle lookup(Method method) throws ReflectiveOperationException {
171-
return doLookup(method, MethodHandles.lookup());
172-
}
173-
174-
@Override
175-
boolean isAvailable() {
176-
return true;
177-
}
178-
};
179-
180-
private static MethodHandle doLookup(Method method, Lookup lookup)
181-
throws NoSuchMethodException, IllegalAccessException {
182-
183-
MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes());
184-
185-
if (Modifier.isStatic(method.getModifiers())) {
186-
return lookup.findStatic(method.getDeclaringClass(), method.getName(), methodType);
187-
}
188-
189-
return lookup.findSpecial(method.getDeclaringClass(), method.getName(), methodType, method.getDeclaringClass());
190-
}
191-
192-
/**
193-
* Lookup a {@link MethodHandle} given {@link Method} to look up.
194-
*
195-
* @param method must not be {@literal null}.
196-
* @return the method handle.
197-
* @throws ReflectiveOperationException
198-
*/
199-
abstract MethodHandle lookup(Method method) throws ReflectiveOperationException;
200-
201-
/**
202-
* @return {@literal true} if the lookup is available.
203-
*/
204-
abstract boolean isAvailable();
205-
206-
/**
207-
* Obtain the first available {@link MethodHandleLookup}.
208-
*
209-
* @return the {@link MethodHandleLookup}
210-
* @throws IllegalStateException if no {@link MethodHandleLookup} is available.
211-
*/
212-
public static MethodHandleLookup getMethodHandleLookup() {
213-
214-
for (MethodHandleLookup it : MethodHandleLookup.values()) {
215-
216-
if (it.isAvailable()) {
217-
return it;
218-
}
219-
}
220-
221-
throw new IllegalStateException("No MethodHandleLookup available");
222-
}
223-
224-
@Nullable
225-
private static Constructor<Lookup> getLookupConstructor() {
226-
227-
try {
228-
229-
Constructor<Lookup> constructor = Lookup.class.getDeclaredConstructor(Class.class);
230-
ReflectionUtils.makeAccessible(constructor);
231-
232-
return constructor;
233-
} catch (Exception ex) {
234-
235-
// this is the signal that we are on Java 9 (encapsulated) and can't use the accessible constructor approach.
236-
if (ex.getClass().getName().equals("java.lang.reflect.InaccessibleObjectException")) {
237-
return null;
238-
}
239-
240-
throw new IllegalStateException(ex);
241-
}
242-
}
105+
return Modifier.isStatic(method.getModifiers())
106+
? lookup.findStatic(declaringClass, method.getName(), methodType)
107+
: lookup.findSpecial(declaringClass, method.getName(), methodType, declaringClass);
243108
}
244109
}

src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@
1616
package org.springframework.data.projection;
1717

1818
import static org.assertj.core.api.Assertions.*;
19-
import static org.assertj.core.api.Assumptions.*;
2019

2120
import org.junit.jupiter.api.Test;
22-
import org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup;
23-
import org.springframework.data.util.Version;
21+
import org.springframework.aop.framework.ProxyFactory;
2422

2523
/**
2624
* Unit tests for {@link DefaultMethodInvokingMethodInterceptor}.
@@ -30,22 +28,24 @@
3028
*/
3129
class DefaultMethodInvokingMethodInterceptorUnitTests {
3230

33-
@Test // DATACMNS-1376
34-
void shouldApplyEncapsulatedLookupOnJava9AndHigher() {
31+
@Test // GH-2971
32+
void invokesDefaultMethodOnProxy() {
3533

36-
assumeThat(Version.javaVersion()).isGreaterThanOrEqualTo(Version.parse("9.0"));
34+
ProxyFactory factory = new ProxyFactory();
35+
factory.setInterfaces(Sample.class);
36+
factory.addAdvice(new DefaultMethodInvokingMethodInterceptor());
3737

38-
assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.ENCAPSULATED);
39-
assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isTrue();
40-
}
38+
Object proxy = factory.getProxy();
4139

42-
@Test // DATACMNS-1376
43-
void shouldApplyOpenLookupOnJava8() {
40+
assertThat(proxy).isInstanceOfSatisfying(Sample.class, it -> {
41+
assertThat(it.sample()).isEqualTo("sample");
42+
});
43+
}
4444

45-
assumeThat(Version.javaVersion()).isLessThan(Version.parse("1.8.9999"));
45+
interface Sample {
4646

47-
assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.OPEN);
48-
assertThat(MethodHandleLookup.OPEN.isAvailable()).isTrue();
49-
assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isFalse();
47+
default String sample() {
48+
return "sample";
49+
}
5050
}
5151
}

0 commit comments

Comments
 (0)