Skip to content

Commit 6e2032e

Browse files
mp911demp911de
committed
DATAJPA-1575 - Limit CrudMethodMetadataPostProcessor to implementation methods.
Also, merge CrudMethodMetadataPostProcessor with ExposeRepositoryInvocationInterceptor as exposing the MethodInvocation is only required for CrudMethodMetadata.
1 parent dfc7919 commit 6e2032e

File tree

2 files changed

+75
-123
lines changed

2 files changed

+75
-123
lines changed

‎src/main/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPostProcessor.java

Lines changed: 60 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@
1515
*/
1616
package org.springframework.data.jpa.repository.support;
1717

18-
import java.io.Serializable;
1918
import java.lang.reflect.Method;
2019
import java.util.Collections;
2120
import java.util.HashMap;
21+
import java.util.HashSet;
2222
import java.util.Map;
2323
import java.util.Optional;
24+
import java.util.Set;
2425
import java.util.concurrent.ConcurrentHashMap;
2526
import java.util.concurrent.ConcurrentMap;
2627
import java.util.function.Predicate;
@@ -30,14 +31,11 @@
3031

3132
import org.aopalliance.intercept.MethodInterceptor;
3233
import org.aopalliance.intercept.MethodInvocation;
33-
import org.springframework.aop.Advisor;
34+
3435
import org.springframework.aop.TargetSource;
3536
import org.springframework.aop.framework.ProxyFactory;
36-
import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
37-
import org.springframework.aop.support.DefaultPointcutAdvisor;
3837
import org.springframework.beans.factory.BeanClassLoaderAware;
3938
import org.springframework.core.NamedThreadLocal;
40-
import org.springframework.core.PriorityOrdered;
4139
import org.springframework.core.annotation.AnnotatedElementUtils;
4240
import org.springframework.core.annotation.AnnotationUtils;
4341
import org.springframework.data.jpa.repository.EntityGraph;
@@ -49,6 +47,7 @@
4947
import org.springframework.transaction.support.TransactionSynchronizationManager;
5048
import org.springframework.util.Assert;
5149
import org.springframework.util.ClassUtils;
50+
import org.springframework.util.ReflectionUtils;
5251

5352
/**
5453
* {@link RepositoryProxyPostProcessor} that sets up interceptors to read metadata information from the invoked method.
@@ -80,9 +79,7 @@ public void setBeanClassLoader(ClassLoader classLoader) {
8079
*/
8180
@Override
8281
public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) {
83-
84-
factory.addAdvisor(ExposeRepositoryInvocationInterceptor.ADVISOR);
85-
factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE);
82+
factory.addAdvice(new CrudMethodMetadataPopulatingMethodInterceptor(repositoryInformation));
8683
}
8784

8885
/**
@@ -107,11 +104,37 @@ CrudMethodMetadata getCrudMethodMetadata() {
107104
* @author Oliver Gierke
108105
* @author Thomas Darimont
109106
*/
110-
enum CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor {
107+
static class CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor {
111108

112-
INSTANCE;
109+
private static final ThreadLocal<MethodInvocation> currentInvocation = new NamedThreadLocal<>(
110+
"Current AOP method invocation");
113111

114112
private final ConcurrentMap<Method, CrudMethodMetadata> metadataCache = new ConcurrentHashMap<>();
113+
private final Set<Method> implementations = new HashSet<>();
114+
115+
CrudMethodMetadataPopulatingMethodInterceptor(RepositoryInformation repositoryInformation) {
116+
117+
ReflectionUtils.doWithMethods(repositoryInformation.getRepositoryInterface(), implementations::add,
118+
method -> !repositoryInformation.isQueryMethod(method));
119+
}
120+
121+
/**
122+
* Return the AOP Alliance {@link MethodInvocation} object associated with the current invocation.
123+
*
124+
* @return the invocation object associated with the current invocation.
125+
* @throws IllegalStateException if there is no AOP invocation in progress, or if the
126+
* {@link CrudMethodMetadataPopulatingMethodInterceptor} was not added to this interceptor chain.
127+
*/
128+
static MethodInvocation currentInvocation() throws IllegalStateException {
129+
130+
MethodInvocation mi = currentInvocation.get();
131+
132+
if (mi == null)
133+
throw new IllegalStateException(
134+
"No MethodInvocation found: Check that an AOP invocation is in progress, and that the "
135+
+ "CrudMethodMetadataPopulatingMethodInterceptor is upfront in the interceptor chain.");
136+
return mi;
137+
}
115138

116139
/*
117140
* (non-Javadoc)
@@ -121,30 +144,43 @@ enum CrudMethodMetadataPopulatingMethodInterceptor implements MethodInterceptor
121144
public Object invoke(MethodInvocation invocation) throws Throwable {
122145

123146
Method method = invocation.getMethod();
124-
CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method);
125147

126-
if (metadata != null) {
148+
if (!implementations.contains(method)) {
127149
return invocation.proceed();
128150
}
129151

130-
CrudMethodMetadata methodMetadata = metadataCache.get(method);
152+
MethodInvocation oldInvocation = currentInvocation.get();
153+
currentInvocation.set(invocation);
131154

132-
if (methodMetadata == null) {
155+
try {
133156

134-
methodMetadata = new DefaultCrudMethodMetadata(method);
135-
CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata);
157+
CrudMethodMetadata metadata = (CrudMethodMetadata) TransactionSynchronizationManager.getResource(method);
136158

137-
if (tmp != null) {
138-
methodMetadata = tmp;
159+
if (metadata != null) {
160+
return invocation.proceed();
139161
}
140-
}
141162

142-
TransactionSynchronizationManager.bindResource(method, methodMetadata);
163+
CrudMethodMetadata methodMetadata = metadataCache.get(method);
143164

144-
try {
145-
return invocation.proceed();
165+
if (methodMetadata == null) {
166+
167+
methodMetadata = new DefaultCrudMethodMetadata(method);
168+
CrudMethodMetadata tmp = metadataCache.putIfAbsent(method, methodMetadata);
169+
170+
if (tmp != null) {
171+
methodMetadata = tmp;
172+
}
173+
}
174+
175+
TransactionSynchronizationManager.bindResource(method, methodMetadata);
176+
177+
try {
178+
return invocation.proceed();
179+
} finally {
180+
TransactionSynchronizationManager.unbindResource(method);
181+
}
146182
} finally {
147-
TransactionSynchronizationManager.unbindResource(method);
183+
currentInvocation.set(oldInvocation);
148184
}
149185
}
150186
}
@@ -285,7 +321,7 @@ public boolean isStatic() {
285321
@Override
286322
public Object getTarget() {
287323

288-
MethodInvocation invocation = ExposeRepositoryInvocationInterceptor.currentInvocation();
324+
MethodInvocation invocation = CrudMethodMetadataPopulatingMethodInterceptor.currentInvocation();
289325
return TransactionSynchronizationManager.getResource(invocation.getMethod());
290326
}
291327

@@ -296,95 +332,4 @@ public Object getTarget() {
296332
@Override
297333
public void releaseTarget(Object target) {}
298334
}
299-
300-
/**
301-
* Own copy of {@link ExposeInvocationInterceptor} scoped to repository proxy method usage to not conflict with
302-
* {@link ExposeInvocationInterceptor} that might expose nested proxy calls to e.g. proxied transaction managers.
303-
*
304-
* @author Mark Paluch
305-
* @since 2.2.0
306-
* @see ExposeInvocationInterceptor
307-
*/
308-
@SuppressWarnings("serial")
309-
static class ExposeRepositoryInvocationInterceptor implements MethodInterceptor, PriorityOrdered, Serializable {
310-
311-
/**
312-
* Singleton instance of this class
313-
*/
314-
static final ExposeRepositoryInvocationInterceptor INSTANCE = new ExposeRepositoryInvocationInterceptor();
315-
316-
private static final ThreadLocal<MethodInvocation> invocation = new NamedThreadLocal<>(
317-
"Current AOP method invocation");
318-
319-
/**
320-
* Singleton advisor for this class. Use in preference to {@code INSTANCE} when using Spring AOP, as it prevents the
321-
* need to create a new Advisor to wrap the instance.
322-
*/
323-
static final Advisor ADVISOR = new DefaultPointcutAdvisor(INSTANCE) {
324-
@Override
325-
public String toString() {
326-
return ExposeRepositoryInvocationInterceptor.class.getName() + ".ADVISOR";
327-
}
328-
};
329-
330-
/**
331-
* Ensures that only the canonical instance can be created.
332-
*/
333-
private ExposeRepositoryInvocationInterceptor() {}
334-
335-
/**
336-
* Return the AOP Alliance {@link MethodInvocation} object associated with the current invocation.
337-
*
338-
* @return the invocation object associated with the current invocation.
339-
* @throws IllegalStateException if there is no AOP invocation in progress, or if the
340-
* {@link ExposeRepositoryInvocationInterceptor} was not added to this interceptor chain.
341-
*/
342-
static MethodInvocation currentInvocation() throws IllegalStateException {
343-
344-
MethodInvocation mi = invocation.get();
345-
346-
if (mi == null)
347-
throw new IllegalStateException(
348-
"No MethodInvocation found: Check that an AOP invocation is in progress, and that the "
349-
+ "ExposeRepositoryInvocationInterceptor is upfront in the interceptor chain. Specifically, note that "
350-
+ "advices with order HIGHEST_PRECEDENCE will execute before ExposeRepositoryMethodInvocationInterceptor!");
351-
return mi;
352-
}
353-
354-
/*
355-
* (non-Javadoc)
356-
* @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation)
357-
*/
358-
@Override
359-
public Object invoke(MethodInvocation mi) throws Throwable {
360-
361-
MethodInvocation oldInvocation = invocation.get();
362-
invocation.set(mi);
363-
364-
try {
365-
return mi.proceed();
366-
} finally {
367-
invocation.set(oldInvocation);
368-
}
369-
}
370-
371-
/*
372-
* (non-Javadoc)
373-
* @see org.springframework.core.Ordered#getOrder()
374-
*/
375-
@Override
376-
public int getOrder() {
377-
return PriorityOrdered.HIGHEST_PRECEDENCE + 1;
378-
}
379-
380-
/**
381-
* Required to support serialization. Replaces with canonical instance on deserialization, protecting Singleton
382-
* pattern.
383-
* <p>
384-
* Alternative to overriding the {@code equals} method.
385-
*/
386-
private Object readResolve() {
387-
return INSTANCE;
388-
}
389-
}
390335
}

‎src/test/java/org/springframework/data/jpa/repository/support/CrudMethodMetadataPopulatingMethodInterceptorUnitTests.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import org.springframework.aop.framework.ProxyFactory;
3232
import org.springframework.data.jpa.repository.Lock;
3333
import org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor.CrudMethodMetadataPopulatingMethodInterceptor;
34-
import org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor.ExposeRepositoryInvocationInterceptor;
34+
import org.springframework.data.repository.core.RepositoryInformation;
3535
import org.springframework.transaction.support.TransactionSynchronizationManager;
3636

3737
/**
@@ -45,13 +45,14 @@
4545
public class CrudMethodMetadataPopulatingMethodInterceptorUnitTests {
4646

4747
@Mock MethodInvocation invocation;
48+
@Mock RepositoryInformation information;
4849

49-
private static Sample expectLockModeType(final CrudMethodMetadata metadata, final LockModeType type) {
50+
private static Sample expectLockModeType(CrudMethodMetadata metadata, RepositoryInformation information,
51+
LockModeType type) {
5052

5153
ProxyFactory factory = new ProxyFactory(new Object());
5254
factory.addInterface(Sample.class);
53-
factory.addAdvice(ExposeRepositoryInvocationInterceptor.INSTANCE);
54-
factory.addAdvice(CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE);
55+
factory.addAdvice(new CrudMethodMetadataPopulatingMethodInterceptor(information));
5556
factory.addAdvice(new MethodInterceptor() {
5657

5758
@Override
@@ -65,29 +66,35 @@ public Object invoke(MethodInvocation invocation) {
6566
}
6667

6768
@Test // DATAJPA-268
69+
@SuppressWarnings("unchecked")
6870
public void cleansUpBoundResources() throws Throwable {
6971

7072
Method method = prepareMethodInvocation("someMethod");
73+
when(information.isQueryMethod(method)).thenReturn(false);
74+
when(information.getRepositoryInterface()).thenReturn((Class) Sample.class);
7175

72-
CrudMethodMetadataPopulatingMethodInterceptor interceptor = CrudMethodMetadataPopulatingMethodInterceptor.INSTANCE;
76+
CrudMethodMetadataPopulatingMethodInterceptor interceptor = new CrudMethodMetadataPopulatingMethodInterceptor(
77+
information);
7378
interceptor.invoke(invocation);
7479

7580
assertThat(TransactionSynchronizationManager.getResource(method)).isNull();
7681
}
7782

7883
@Test // DATAJPA-839, DATAJPA-1368
84+
@SuppressWarnings("unchecked")
7985
public void looksUpCrudMethodMetadataForEveryInvocation() {
8086

8187
CrudMethodMetadata metadata = new CrudMethodMetadataPostProcessor().getCrudMethodMetadata();
88+
when(information.isQueryMethod(any())).thenReturn(false);
89+
when(information.getRepositoryInterface()).thenReturn((Class) Sample.class);
8290

83-
expectLockModeType(metadata, LockModeType.OPTIMISTIC).someMethod();
84-
expectLockModeType(metadata, LockModeType.PESSIMISTIC_READ).someOtherMethod();
91+
expectLockModeType(metadata, information, LockModeType.OPTIMISTIC).someMethod();
92+
expectLockModeType(metadata, information, LockModeType.PESSIMISTIC_READ).someOtherMethod();
8593
}
8694

8795
private Method prepareMethodInvocation(String name) throws Throwable {
8896

8997
Method method = Sample.class.getMethod(name);
90-
ExposeRepositoryInvocationInterceptor.INSTANCE.invoke(invocation);
9198
when(invocation.getMethod()).thenReturn(method);
9299

93100
return method;

0 commit comments

Comments
 (0)