Skip to content

Commit 2aabe23

Browse files
jhoellerjhoeller
committed
Merge branch '6.1.x'
# Conflicts: # spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java
2 parents 7b9cbd7 + 2451bd6 commit 2aabe23

File tree

7 files changed

+79
-43
lines changed

7 files changed

+79
-43
lines changed

Diff for: ‎spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

+21-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.aop.aspectj;
1818

19+
import java.lang.reflect.Field;
1920
import java.lang.reflect.Method;
2021
import java.lang.reflect.Proxy;
2122
import java.util.Arrays;
@@ -82,6 +83,8 @@
8283
public class AspectJExpressionPointcut extends AbstractExpressionPointcut
8384
implements ClassFilter, IntroductionAwareMethodMatcher, BeanFactoryAware {
8485

86+
private static final String AJC_MAGIC = "ajc$";
87+
8588
private static final Set<PointcutPrimitive> SUPPORTED_PRIMITIVES = Set.of(
8689
PointcutPrimitive.EXECUTION,
8790
PointcutPrimitive.ARGS,
@@ -99,6 +102,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
99102
@Nullable
100103
private Class<?> pointcutDeclarationScope;
101104

105+
private boolean aspectCompiledByAjc;
106+
102107
private String[] pointcutParameterNames = new String[0];
103108

104109
private Class<?>[] pointcutParameterTypes = new Class<?>[0];
@@ -128,7 +133,7 @@ public AspectJExpressionPointcut() {
128133
* @param paramTypes the parameter types for the pointcut
129134
*/
130135
public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames, Class<?>[] paramTypes) {
131-
this.pointcutDeclarationScope = declarationScope;
136+
setPointcutDeclarationScope(declarationScope);
132137
if (paramNames.length != paramTypes.length) {
133138
throw new IllegalStateException(
134139
"Number of pointcut parameter names must match number of pointcut parameter types");
@@ -143,6 +148,7 @@ public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames,
143148
*/
144149
public void setPointcutDeclarationScope(Class<?> pointcutDeclarationScope) {
145150
this.pointcutDeclarationScope = pointcutDeclarationScope;
151+
this.aspectCompiledByAjc = compiledByAjc(pointcutDeclarationScope);
146152
}
147153

148154
/**
@@ -268,6 +274,11 @@ public PointcutExpression getPointcutExpression() {
268274
@Override
269275
public boolean matches(Class<?> targetClass) {
270276
if (this.pointcutParsingFailed) {
277+
// Pointcut parsing failed before below -> avoid trying again.
278+
return false;
279+
}
280+
if (this.aspectCompiledByAjc && compiledByAjc(targetClass)) {
281+
// ajc-compiled aspect class for ajc-compiled target class -> already weaved.
271282
return false;
272283
}
273284

@@ -523,6 +534,15 @@ private boolean containsAnnotationPointcut() {
523534
return resolveExpression().contains("@annotation");
524535
}
525536

537+
private static boolean compiledByAjc(Class<?> clazz) {
538+
for (Field field : clazz.getDeclaredFields()) {
539+
if (field.getName().startsWith(AJC_MAGIC)) {
540+
return true;
541+
}
542+
}
543+
return false;
544+
}
545+
526546

527547
@Override
528548
public boolean equals(@Nullable Object other) {

Diff for: ‎spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectJAdvisorsBuilder.java

+31-19
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,9 +22,12 @@
2222
import java.util.Map;
2323
import java.util.concurrent.ConcurrentHashMap;
2424

25+
import org.apache.commons.logging.Log;
26+
import org.apache.commons.logging.LogFactory;
2527
import org.aspectj.lang.reflect.PerClauseKind;
2628

2729
import org.springframework.aop.Advisor;
30+
import org.springframework.aop.framework.AopConfigException;
2831
import org.springframework.beans.factory.BeanFactoryUtils;
2932
import org.springframework.beans.factory.ListableBeanFactory;
3033
import org.springframework.lang.Nullable;
@@ -40,6 +43,8 @@
4043
*/
4144
public class BeanFactoryAspectJAdvisorsBuilder {
4245

46+
private static final Log logger = LogFactory.getLog(BeanFactoryAspectJAdvisorsBuilder.class);
47+
4348
private final ListableBeanFactory beanFactory;
4449

4550
private final AspectJAdvisorFactory advisorFactory;
@@ -103,30 +108,37 @@ public List<Advisor> buildAspectJAdvisors() {
103108
continue;
104109
}
105110
if (this.advisorFactory.isAspect(beanType)) {
106-
aspectNames.add(beanName);
107-
AspectMetadata amd = new AspectMetadata(beanType, beanName);
108-
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
109-
MetadataAwareAspectInstanceFactory factory =
110-
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
111-
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
112-
if (this.beanFactory.isSingleton(beanName)) {
113-
this.advisorsCache.put(beanName, classAdvisors);
111+
try {
112+
AspectMetadata amd = new AspectMetadata(beanType, beanName);
113+
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
114+
MetadataAwareAspectInstanceFactory factory =
115+
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
116+
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
117+
if (this.beanFactory.isSingleton(beanName)) {
118+
this.advisorsCache.put(beanName, classAdvisors);
119+
}
120+
else {
121+
this.aspectFactoryCache.put(beanName, factory);
122+
}
123+
advisors.addAll(classAdvisors);
114124
}
115125
else {
126+
// Per target or per this.
127+
if (this.beanFactory.isSingleton(beanName)) {
128+
throw new IllegalArgumentException("Bean with name '" + beanName +
129+
"' is a singleton, but aspect instantiation model is not singleton");
130+
}
131+
MetadataAwareAspectInstanceFactory factory =
132+
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
116133
this.aspectFactoryCache.put(beanName, factory);
134+
advisors.addAll(this.advisorFactory.getAdvisors(factory));
117135
}
118-
advisors.addAll(classAdvisors);
136+
aspectNames.add(beanName);
119137
}
120-
else {
121-
// Per target or per this.
122-
if (this.beanFactory.isSingleton(beanName)) {
123-
throw new IllegalArgumentException("Bean with name '" + beanName +
124-
"' is a singleton, but aspect instantiation model is not singleton");
138+
catch (IllegalArgumentException | IllegalStateException | AopConfigException ex) {
139+
if (logger.isDebugEnabled()) {
140+
logger.debug("Ignoring incompatible aspect [" + beanType.getName() + "]: " + ex);
125141
}
126-
MetadataAwareAspectInstanceFactory factory =
127-
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
128-
this.aspectFactoryCache.put(beanName, factory);
129-
advisors.addAll(this.advisorFactory.getAdvisors(factory));
130142
}
131143
}
132144
}

Diff for: ‎spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml

+4-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
<property name="foo" value="bar"/>
2222
</bean>
2323

24-
<bean id="otherBean" class="java.lang.Object"/>
24+
<bean id="otherBean" class="org.springframework.beans.factory.aspectj.ShouldBeConfiguredBySpring"/>
2525

26-
<bean id="yetAnotherBean" class="java.lang.Object"/>
26+
<bean id="yetAnotherBean" class="org.springframework.beans.factory.aspectj.ShouldBeConfiguredBySpring"/>
27+
28+
<bean id="configuredBean" class="org.springframework.beans.factory.aspectj.ShouldBeConfiguredBySpring" lazy-init="true"/>
2729

2830
</beans>

Diff for: ‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ private Object processCacheEvicts(Collection<CacheOperationContext> contexts, bo
635635
if (result instanceof CompletableFuture<?> future) {
636636
return future.whenComplete((value, ex) -> {
637637
if (ex == null) {
638-
performCacheEvicts(applicable, result);
638+
performCacheEvicts(applicable, value);
639639
}
640640
});
641641
}
@@ -1117,7 +1117,7 @@ public Object processCacheEvicts(List<CacheOperationContext> contexts, @Nullable
11171117
ReactiveAdapter adapter = (result != null ? this.registry.getAdapter(result.getClass()) : null);
11181118
if (adapter != null) {
11191119
return adapter.fromPublisher(Mono.from(adapter.toPublisher(result))
1120-
.doOnSuccess(value -> performCacheEvicts(contexts, result)));
1120+
.doOnSuccess(value -> performCacheEvicts(contexts, value)));
11211121
}
11221122
return NOT_HANDLED;
11231123
}

Diff for: ‎spring-context/src/test/java/org/springframework/cache/CacheReproTests.java

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -606,9 +606,9 @@ public CompletableFuture<TestBean> insertItem(TestBean item) {
606606
return CompletableFuture.completedFuture(item);
607607
}
608608

609-
@CacheEvict(cacheNames = "itemCache", allEntries = true)
610-
public CompletableFuture<Void> clear() {
611-
return CompletableFuture.completedFuture(null);
609+
@CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0")
610+
public CompletableFuture<Integer> clear() {
611+
return CompletableFuture.completedFuture(1);
612612
}
613613
}
614614

@@ -655,9 +655,9 @@ public Mono<TestBean> insertItem(TestBean item) {
655655
return Mono.just(item);
656656
}
657657

658-
@CacheEvict(cacheNames = "itemCache", allEntries = true)
659-
public Mono<Void> clear() {
660-
return Mono.empty();
658+
@CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0")
659+
public Mono<Integer> clear() {
660+
return Mono.just(1);
661661
}
662662
}
663663

@@ -706,9 +706,9 @@ public Flux<TestBean> insertItem(String id, List<TestBean> item) {
706706
return Flux.fromIterable(item);
707707
}
708708

709-
@CacheEvict(cacheNames = "itemCache", allEntries = true)
710-
public Flux<Void> clear() {
711-
return Flux.empty();
709+
@CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0")
710+
public Flux<Integer> clear() {
711+
return Flux.just(1);
712712
}
713713
}
714714

Diff for: ‎spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,7 +51,9 @@ class ReactiveCachingTests {
5151
LateCacheHitDeterminationConfig.class,
5252
LateCacheHitDeterminationWithValueWrapperConfig.class})
5353
void cacheHitDetermination(Class<?> configClass) {
54-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class);
54+
55+
AnnotationConfigApplicationContext ctx =
56+
new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class);
5557
ReactiveCacheableService service = ctx.getBean(ReactiveCacheableService.class);
5658

5759
Object key = new Object();
@@ -117,7 +119,9 @@ void cacheHitDetermination(Class<?> configClass) {
117119
LateCacheHitDeterminationConfig.class,
118120
LateCacheHitDeterminationWithValueWrapperConfig.class})
119121
void fluxCacheDoesntDependOnFirstRequest(Class<?> configClass) {
120-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class);
122+
123+
AnnotationConfigApplicationContext ctx =
124+
new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class);
121125
ReactiveCacheableService service = ctx.getBean(ReactiveCacheableService.class);
122126

123127
Object key = new Object();
@@ -135,6 +139,7 @@ void fluxCacheDoesntDependOnFirstRequest(Class<?> configClass) {
135139
ctx.close();
136140
}
137141

142+
138143
@CacheConfig(cacheNames = "first")
139144
static class ReactiveCacheableService {
140145

Diff for: ‎spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2023-2023 the original author or authors.
2+
* Copyright 2023-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,8 +28,7 @@
2828
import org.springframework.util.Assert;
2929

3030
/**
31-
* {@link ClientHttpRequestFactory} implementation based on the Java
32-
* {@link HttpClient}.
31+
* {@link ClientHttpRequestFactory} implementation based on the Java {@link HttpClient}.
3332
*
3433
* @author Marten Deinum
3534
* @author Arjen Poutsma
@@ -89,13 +88,11 @@ public void setReadTimeout(int readTimeout) {
8988
}
9089

9190
/**
92-
* Set the underlying {@code HttpClient}'s read timeout as a
93-
* {@code Duration}.
91+
* Set the underlying {@code HttpClient}'s read timeout as a {@code Duration}.
9492
* <p>Default is the system's default timeout.
9593
* @see java.net.http.HttpRequest.Builder#timeout
9694
*/
9795
public void setReadTimeout(Duration readTimeout) {
98-
Assert.notNull(readTimeout, "ReadTimeout must not be null");
9996
this.readTimeout = readTimeout;
10097
}
10198

0 commit comments

Comments
 (0)