From 38b3099ec5fe35c6b10033ccb0778e6d0c2ce092 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 8 Nov 2023 08:06:33 +0100 Subject: [PATCH] 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. --- ...efaultMethodInvokingMethodInterceptor.java | 163 ++---------------- ...hodInvokingMethodInterceptorUnitTests.java | 30 ++-- 2 files changed, 29 insertions(+), 164 deletions(-) diff --git a/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java b/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java index 738c14840..919b23631 100644 --- a/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java +++ b/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java @@ -19,7 +19,6 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodHandles.Lookup; import java.lang.invoke.MethodType; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.Map; @@ -28,7 +27,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.ProxyMethodInvocation; -import org.springframework.data.util.Lazy; import org.springframework.lang.Nullable; import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; @@ -44,7 +42,7 @@ import org.springframework.util.ReflectionUtils; */ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor { - private final MethodHandleLookup methodHandleLookup = MethodHandleLookup.getMethodHandleLookup(); + private static final Lookup LOOKUP = MethodHandles.lookup(); private final Map methodHandleCache = new ConcurrentReferenceHashMap<>(10, ReferenceType.WEAK); /** @@ -64,7 +62,7 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor @Nullable @Override - public Object invoke(@SuppressWarnings("null") MethodInvocation invocation) throws Throwable { + public Object invoke(MethodInvocation invocation) throws Throwable { Method method = invocation.getMethod(); @@ -84,7 +82,7 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor if (handle == null) { - handle = methodHandleLookup.lookup(method); + handle = lookup(method); methodHandleCache.put(method, handle); } @@ -92,153 +90,20 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor } /** - * Strategies for {@link MethodHandle} lookup. + * Lookup a {@link MethodHandle} given {@link Method} to look up. * - * @since 2.0 + * @param method must not be {@literal null}. + * @return the method handle. + * @throws ReflectiveOperationException */ - enum MethodHandleLookup { + private static MethodHandle lookup(Method method) throws ReflectiveOperationException { - /** - * Encapsulated {@link MethodHandle} lookup working on Java 9. - */ - ENCAPSULATED { + Lookup lookup = MethodHandles.privateLookupIn(method.getDeclaringClass(), LOOKUP); + MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes()); + Class declaringClass = method.getDeclaringClass(); - private final @Nullable Method privateLookupIn = ReflectionUtils.findMethod(MethodHandles.class, - "privateLookupIn", Class.class, Lookup.class); - - @Override - MethodHandle lookup(Method method) throws ReflectiveOperationException { - - if (privateLookupIn == null) { - throw new IllegalStateException("Could not obtain MethodHandles.privateLookupIn"); - } - - return doLookup(method, getLookup(method.getDeclaringClass(), privateLookupIn)); - } - - @Override - boolean isAvailable() { - return privateLookupIn != null; - } - - private Lookup getLookup(Class declaringClass, Method privateLookupIn) { - - Lookup lookup = MethodHandles.lookup(); - - try { - return (Lookup) privateLookupIn.invoke(MethodHandles.class, declaringClass, lookup); - } catch (ReflectiveOperationException e) { - return lookup; - } - } - }, - - /** - * Open (via reflection construction of {@link MethodHandles.Lookup}) method handle lookup. Works with Java 8 and - * with Java 9 permitting illegal access. - */ - OPEN { - - private final Lazy> constructor = Lazy.of(MethodHandleLookup::getLookupConstructor); - - @Override - MethodHandle lookup(Method method) throws ReflectiveOperationException { - - if (!isAvailable()) { - throw new IllegalStateException("Could not obtain MethodHandles.lookup constructor"); - } - - Constructor constructor = this.constructor.get(); - - return constructor.newInstance(method.getDeclaringClass()).unreflectSpecial(method, method.getDeclaringClass()); - } - - @Override - boolean isAvailable() { - return constructor.orElse(null) != null; - } - }, - - /** - * Fallback {@link MethodHandle} lookup using {@link MethodHandles#lookup() public lookup}. - * - * @since 2.1 - */ - FALLBACK { - - @Override - MethodHandle lookup(Method method) throws ReflectiveOperationException { - return doLookup(method, MethodHandles.lookup()); - } - - @Override - boolean isAvailable() { - return true; - } - }; - - private static MethodHandle doLookup(Method method, Lookup lookup) - throws NoSuchMethodException, IllegalAccessException { - - MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes()); - - if (Modifier.isStatic(method.getModifiers())) { - return lookup.findStatic(method.getDeclaringClass(), method.getName(), methodType); - } - - return lookup.findSpecial(method.getDeclaringClass(), method.getName(), methodType, method.getDeclaringClass()); - } - - /** - * Lookup a {@link MethodHandle} given {@link Method} to look up. - * - * @param method must not be {@literal null}. - * @return the method handle. - * @throws ReflectiveOperationException - */ - abstract MethodHandle lookup(Method method) throws ReflectiveOperationException; - - /** - * @return {@literal true} if the lookup is available. - */ - abstract boolean isAvailable(); - - /** - * Obtain the first available {@link MethodHandleLookup}. - * - * @return the {@link MethodHandleLookup} - * @throws IllegalStateException if no {@link MethodHandleLookup} is available. - */ - public static MethodHandleLookup getMethodHandleLookup() { - - for (MethodHandleLookup it : MethodHandleLookup.values()) { - - if (it.isAvailable()) { - return it; - } - } - - throw new IllegalStateException("No MethodHandleLookup available"); - } - - @Nullable - private static Constructor getLookupConstructor() { - - try { - - Constructor constructor = Lookup.class.getDeclaredConstructor(Class.class); - ReflectionUtils.makeAccessible(constructor); - - return constructor; - } catch (Exception ex) { - - // this is the signal that we are on Java 9 (encapsulated) and can't use the accessible constructor approach. - if (ex.getClass().getName().equals("java.lang.reflect.InaccessibleObjectException")) { - return null; - } - - throw new IllegalStateException(ex); - } - } + return Modifier.isStatic(method.getModifiers()) + ? lookup.findStatic(declaringClass, method.getName(), methodType) + : lookup.findSpecial(declaringClass, method.getName(), methodType, declaringClass); } } diff --git a/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java index 4e80a5554..4fce2f581 100644 --- a/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java +++ b/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java @@ -16,11 +16,9 @@ package org.springframework.data.projection; import static org.assertj.core.api.Assertions.*; -import static org.assertj.core.api.Assumptions.*; import org.junit.jupiter.api.Test; -import org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup; -import org.springframework.data.util.Version; +import org.springframework.aop.framework.ProxyFactory; /** * Unit tests for {@link DefaultMethodInvokingMethodInterceptor}. @@ -30,22 +28,24 @@ import org.springframework.data.util.Version; */ class DefaultMethodInvokingMethodInterceptorUnitTests { - @Test // DATACMNS-1376 - void shouldApplyEncapsulatedLookupOnJava9AndHigher() { + @Test // GH-2971 + void invokesDefaultMethodOnProxy() { - assumeThat(Version.javaVersion()).isGreaterThanOrEqualTo(Version.parse("9.0")); + ProxyFactory factory = new ProxyFactory(); + factory.setInterfaces(Sample.class); + factory.addAdvice(new DefaultMethodInvokingMethodInterceptor()); - assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.ENCAPSULATED); - assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isTrue(); + Object proxy = factory.getProxy(); + + assertThat(proxy).isInstanceOfSatisfying(Sample.class, it -> { + assertThat(it.sample()).isEqualTo("sample"); + }); } - @Test // DATACMNS-1376 - void shouldApplyOpenLookupOnJava8() { + interface Sample { - assumeThat(Version.javaVersion()).isLessThan(Version.parse("1.8.9999")); - - assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.OPEN); - assertThat(MethodHandleLookup.OPEN.isAvailable()).isTrue(); - assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isFalse(); + default String sample() { + return "sample"; + } } }