Skip to content

Commit

Permalink
Skip ajc-compiled aspects for ajc-compiled target classes
Browse files Browse the repository at this point in the history
Includes defensive ignoring of incompatible aspect types.

Closes gh-32970

(cherry picked from commit 0ea96b4)
  • Loading branch information
jhoeller committed Jun 6, 2024
1 parent 8b589db commit 628b050
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.IOException;
import java.io.ObjectInputStream;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Arrays;
Expand Down Expand Up @@ -85,6 +86,8 @@
public class AspectJExpressionPointcut extends AbstractExpressionPointcut
implements ClassFilter, IntroductionAwareMethodMatcher, BeanFactoryAware {

private static final String AJC_MAGIC = "ajc$";

private static final Set<PointcutPrimitive> SUPPORTED_PRIMITIVES = Set.of(
PointcutPrimitive.EXECUTION,
PointcutPrimitive.ARGS,
Expand All @@ -102,6 +105,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Nullable
private Class<?> pointcutDeclarationScope;

private boolean aspectCompiledByAjc;

private String[] pointcutParameterNames = new String[0];

private Class<?>[] pointcutParameterTypes = new Class<?>[0];
Expand Down Expand Up @@ -133,7 +138,7 @@ public AspectJExpressionPointcut() {
* @param paramTypes the parameter types for the pointcut
*/
public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames, Class<?>[] paramTypes) {
this.pointcutDeclarationScope = declarationScope;
setPointcutDeclarationScope(declarationScope);
if (paramNames.length != paramTypes.length) {
throw new IllegalStateException(
"Number of pointcut parameter names must match number of pointcut parameter types");
Expand All @@ -148,6 +153,7 @@ public AspectJExpressionPointcut(Class<?> declarationScope, String[] paramNames,
*/
public void setPointcutDeclarationScope(Class<?> pointcutDeclarationScope) {
this.pointcutDeclarationScope = pointcutDeclarationScope;
this.aspectCompiledByAjc = compiledByAjc(pointcutDeclarationScope);
}

/**
Expand Down Expand Up @@ -273,6 +279,11 @@ public PointcutExpression getPointcutExpression() {
@Override
public boolean matches(Class<?> targetClass) {
if (this.pointcutParsingFailed) {
// Pointcut parsing failed before below -> avoid trying again.
return false;
}
if (this.aspectCompiledByAjc && compiledByAjc(targetClass)) {
// ajc-compiled aspect class for ajc-compiled target class -> already weaved.
return false;
}

Expand Down Expand Up @@ -528,6 +539,15 @@ else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
return shadowMatch;
}

private static boolean compiledByAjc(Class<?> clazz) {
for (Field field : clazz.getDeclaredFields()) {
if (field.getName().startsWith(AJC_MAGIC)) {
return true;
}
}
return false;
}


@Override
public boolean equals(@Nullable Object other) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,9 +22,12 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.aspectj.lang.reflect.PerClauseKind;

import org.springframework.aop.Advisor;
import org.springframework.aop.framework.AopConfigException;
import org.springframework.beans.factory.BeanFactoryUtils;
import org.springframework.beans.factory.ListableBeanFactory;
import org.springframework.lang.Nullable;
Expand All @@ -40,6 +43,8 @@
*/
public class BeanFactoryAspectJAdvisorsBuilder {

private static final Log logger = LogFactory.getLog(BeanFactoryAspectJAdvisorsBuilder.class);

private final ListableBeanFactory beanFactory;

private final AspectJAdvisorFactory advisorFactory;
Expand Down Expand Up @@ -102,30 +107,37 @@ public List<Advisor> buildAspectJAdvisors() {
continue;
}
if (this.advisorFactory.isAspect(beanType)) {
aspectNames.add(beanName);
AspectMetadata amd = new AspectMetadata(beanType, beanName);
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
MetadataAwareAspectInstanceFactory factory =
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
if (this.beanFactory.isSingleton(beanName)) {
this.advisorsCache.put(beanName, classAdvisors);
try {
AspectMetadata amd = new AspectMetadata(beanType, beanName);
if (amd.getAjType().getPerClause().getKind() == PerClauseKind.SINGLETON) {
MetadataAwareAspectInstanceFactory factory =
new BeanFactoryAspectInstanceFactory(this.beanFactory, beanName);
List<Advisor> classAdvisors = this.advisorFactory.getAdvisors(factory);
if (this.beanFactory.isSingleton(beanName)) {
this.advisorsCache.put(beanName, classAdvisors);
}
else {
this.aspectFactoryCache.put(beanName, factory);
}
advisors.addAll(classAdvisors);
}
else {
// Per target or per this.
if (this.beanFactory.isSingleton(beanName)) {
throw new IllegalArgumentException("Bean with name '" + beanName +
"' is a singleton, but aspect instantiation model is not singleton");
}
MetadataAwareAspectInstanceFactory factory =
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
this.aspectFactoryCache.put(beanName, factory);
advisors.addAll(this.advisorFactory.getAdvisors(factory));
}
advisors.addAll(classAdvisors);
aspectNames.add(beanName);
}
else {
// Per target or per this.
if (this.beanFactory.isSingleton(beanName)) {
throw new IllegalArgumentException("Bean with name '" + beanName +
"' is a singleton, but aspect instantiation model is not singleton");
catch (IllegalArgumentException | IllegalStateException | AopConfigException ex) {
if (logger.isDebugEnabled()) {
logger.debug("Ignoring incompatible aspect [" + beanType.getName() + "]: " + ex);
}
MetadataAwareAspectInstanceFactory factory =
new PrototypeAspectInstanceFactory(this.beanFactory, beanName);
this.aspectFactoryCache.put(beanName, factory);
advisors.addAll(this.advisorFactory.getAdvisors(factory));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
<property name="foo" value="bar"/>
</bean>

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

<bean id="yetAnotherBean" class="java.lang.Object"/>
<bean id="yetAnotherBean" class="org.springframework.beans.factory.aspectj.ShouldBeConfiguredBySpring"/>

<bean id="configuredBean" class="org.springframework.beans.factory.aspectj.ShouldBeConfiguredBySpring" lazy-init="true"/>

</beans>

0 comments on commit 628b050

Please sign in to comment.