Skip to content

Commit 63568e6

Browse files
jhoellerjhoeller
committed
Skip ajc-compiled aspects for ajc-compiled target classes
Includes defensive ignoring of incompatible aspect types. Closes gh-32970 (cherry picked from commit 0ea96b4)
1 parent d410485 commit 63568e6

3 files changed

Lines changed: 56 additions & 22 deletions

File tree

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.io.IOException;
2020
import java.io.ObjectInputStream;
21+
import java.lang.reflect.Field;
2122
import java.lang.reflect.Method;
2223
import java.lang.reflect.Proxy;
2324
import java.util.Arrays;
@@ -86,6 +87,8 @@
8687
public class AspectJExpressionPointcut extends AbstractExpressionPointcut
8788
implements ClassFilter, IntroductionAwareMethodMatcher, BeanFactoryAware {
8889

90+
private static final String AJC_MAGIC = "ajc$";
91+
8992
private static final Set<PointcutPrimitive> SUPPORTED_PRIMITIVES = new HashSet<>();
9093

9194
static {
@@ -107,6 +110,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
107110
@Nullable
108111
private Class<?> pointcutDeclarationScope;
109112

113+
private boolean aspectCompiledByAjc;
114+
110115
private String[] pointcutParameterNames = new String[0];
111116

112117
private Class<?>[] pointcutParameterTypes = new Class<?>[0];
@@ -138,7 +143,7 @@ public AspectJExpressionPointcut() {
138143
* @param paramTypes the parameter types for the pointcut
139144
*/
140145
public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames, Class<?>[] paramTypes) {
141-
this.pointcutDeclarationScope = declarationScope;
146+
setPointcutDeclarationScope(declarationScope);
142147
if (paramNames.length != paramTypes.length) {
143148
throw new IllegalStateException(
144149
"Number of pointcut parameter names must match number of pointcut parameter types");
@@ -153,6 +158,7 @@ public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames,
153158
*/
154159
public void setPointcutDeclarationScope(Class<?> pointcutDeclarationScope) {
155160
this.pointcutDeclarationScope = pointcutDeclarationScope;
161+
this.aspectCompiledByAjc = compiledByAjc(pointcutDeclarationScope);
156162
}
157163

158164
/**
@@ -278,6 +284,11 @@ public PointcutExpression getPointcutExpression() {
278284
@Override
279285
public boolean matches(Class<?> targetClass) {
280286
if (this.pointcutParsingFailed) {
287+
// Pointcut parsing failed before below -> avoid trying again.
288+
return false;
289+
}
290+
if (this.aspectCompiledByAjc && compiledByAjc(targetClass)) {
291+
// ajc-compiled aspect class for ajc-compiled target class -> already weaved.
281292
return false;
282293
}
283294

@@ -532,6 +543,15 @@ else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
532543
return shadowMatch;
533544
}
534545

546+
private static boolean compiledByAjc(Class<?> clazz) {
547+
for (Field field : clazz.getDeclaredFields()) {
548+
if (field.getName().startsWith(AJC_MAGIC)) {
549+
return true;
550+
}
551+
}
552+
return false;
553+
}
554+
535555

536556
@Override
537557
public boolean equals(@Nullable Object other) {

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

Lines changed: 31 additions & 19 deletions
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;
@@ -102,30 +107,37 @@ public List<Advisor> buildAspectJAdvisors() {
102107
continue;
103108
}
104109
if (this.advisorFactory.isAspect(beanType)) {
105-
aspectNames.add(beanName);
106-
AspectMetadata amd = new AspectMetadata(beanType, beanName);
107-
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
108-
MetadataAwareAspectInstanceFactory factory =
109-
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
110-
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
111-
if (this.beanFactory.isSingleton(beanName)) {
112-
this.advisorsCache.put(beanName, classAdvisors);
110+
try {
111+
AspectMetadata amd = new AspectMetadata(beanType, beanName);
112+
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
113+
MetadataAwareAspectInstanceFactory factory =
114+
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
115+
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
116+
if (this.beanFactory.isSingleton(beanName)) {
117+
this.advisorsCache.put(beanName, classAdvisors);
118+
}
119+
else {
120+
this.aspectFactoryCache.put(beanName, factory);
121+
}
122+
advisors.addAll(classAdvisors);
113123
}
114124
else {
125+
// Per target or per this.
126+
if (this.beanFactory.isSingleton(beanName)) {
127+
throw new IllegalArgumentException("Bean with name '" + beanName +
128+
"' is a singleton, but aspect instantiation model is not singleton");
129+
}
130+
MetadataAwareAspectInstanceFactory factory =
131+
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
115132
this.aspectFactoryCache.put(beanName, factory);
133+
advisors.addAll(this.advisorFactory.getAdvisors(factory));
116134
}
117-
advisors.addAll(classAdvisors);
135+
aspectNames.add(beanName);
118136
}
119-
else {
120-
// Per target or per this.
121-
if (this.beanFactory.isSingleton(beanName)) {
122-
throw new IllegalArgumentException("Bean with name '" + beanName +
123-
"' is a singleton, but aspect instantiation model is not singleton");
137+
catch (IllegalArgumentException | IllegalStateException | AopConfigException ex) {
138+
if (logger.isDebugEnabled()) {
139+
logger.debug("Ignoring incompatible aspect [" + beanType.getName() + "]: " + ex);
124140
}
125-
MetadataAwareAspectInstanceFactory factory =
126-
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
127-
this.aspectFactoryCache.put(beanName, factory);
128-
advisors.addAll(this.advisorFactory.getAdvisors(factory));
129141
}
130142
}
131143
}

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

Lines changed: 4 additions & 2 deletions
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>

0 commit comments

Comments
 (0)