diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index 1a08bb454d..a8c3d0990c 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -16,6 +16,7 @@ package org.springframework.aop.aspectj; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Arrays; @@ -82,6 +83,8 @@ import org.springframework.util.StringUtils; public class AspectJExpressionPointcut extends AbstractExpressionPointcut implements ClassFilter, IntroductionAwareMethodMatcher, BeanFactoryAware { + private static final String AJC_MAGIC = "ajc$"; + private static final Set SUPPORTED_PRIMITIVES = Set.of( PointcutPrimitive.EXECUTION, PointcutPrimitive.ARGS, @@ -99,6 +102,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]; @@ -128,7 +133,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut * @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"); @@ -143,6 +148,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut */ public void setPointcutDeclarationScope(Class pointcutDeclarationScope) { this.pointcutDeclarationScope = pointcutDeclarationScope; + this.aspectCompiledByAjc = compiledByAjc(pointcutDeclarationScope); } /** @@ -268,6 +274,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @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; } @@ -523,6 +534,15 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut return resolveExpression().contains("@annotation"); } + 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) { diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectJAdvisorsBuilder.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectJAdvisorsBuilder.java index 2ac439af1e..21bc248e50 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectJAdvisorsBuilder.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/BeanFactoryAspectJAdvisorsBuilder.java @@ -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. @@ -22,9 +22,12 @@ import java.util.List; 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; @@ -40,6 +43,8 @@ import org.springframework.util.Assert; */ public class BeanFactoryAspectJAdvisorsBuilder { + private static final Log logger = LogFactory.getLog(BeanFactoryAspectJAdvisorsBuilder.class); + private final ListableBeanFactory beanFactory; private final AspectJAdvisorFactory advisorFactory; @@ -103,30 +108,37 @@ public class BeanFactoryAspectJAdvisorsBuilder { 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 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 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)); } } } diff --git a/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml b/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml index bc5e5258f8..e6c494c4f9 100644 --- a/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml +++ b/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml @@ -21,8 +21,10 @@ - + - + + + diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 475f6e47b9..b77b8ca6f8 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -635,7 +635,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker if (result instanceof CompletableFuture future) { return future.whenComplete((value, ex) -> { if (ex == null) { - performCacheEvicts(applicable, result); + performCacheEvicts(applicable, value); } }); } @@ -1117,7 +1117,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker ReactiveAdapter adapter = (result != null ? this.registry.getAdapter(result.getClass()) : null); if (adapter != null) { return adapter.fromPublisher(Mono.from(adapter.toPublisher(result)) - .doOnSuccess(value -> performCacheEvicts(contexts, result))); + .doOnSuccess(value -> performCacheEvicts(contexts, value))); } return NOT_HANDLED; } diff --git a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java index c67102f7da..9d358b81cd 100644 --- a/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java +++ b/spring-context/src/test/java/org/springframework/cache/CacheReproTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -606,9 +606,9 @@ class CacheReproTests { return CompletableFuture.completedFuture(item); } - @CacheEvict(cacheNames = "itemCache", allEntries = true) - public CompletableFuture clear() { - return CompletableFuture.completedFuture(null); + @CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0") + public CompletableFuture clear() { + return CompletableFuture.completedFuture(1); } } @@ -655,9 +655,9 @@ class CacheReproTests { return Mono.just(item); } - @CacheEvict(cacheNames = "itemCache", allEntries = true) - public Mono clear() { - return Mono.empty(); + @CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0") + public Mono clear() { + return Mono.just(1); } } @@ -706,9 +706,9 @@ class CacheReproTests { return Flux.fromIterable(item); } - @CacheEvict(cacheNames = "itemCache", allEntries = true) - public Flux clear() { - return Flux.empty(); + @CacheEvict(cacheNames = "itemCache", allEntries = true, condition = "#result > 0") + public Flux clear() { + return Flux.just(1); } } diff --git a/spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java b/spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java index 5c04f80a51..06130951af 100644 --- a/spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java +++ b/spring-context/src/test/java/org/springframework/cache/annotation/ReactiveCachingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -51,7 +51,9 @@ class ReactiveCachingTests { LateCacheHitDeterminationConfig.class, LateCacheHitDeterminationWithValueWrapperConfig.class}) void cacheHitDetermination(Class configClass) { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class); + + AnnotationConfigApplicationContext ctx = + new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class); ReactiveCacheableService service = ctx.getBean(ReactiveCacheableService.class); Object key = new Object(); @@ -117,7 +119,9 @@ class ReactiveCachingTests { LateCacheHitDeterminationConfig.class, LateCacheHitDeterminationWithValueWrapperConfig.class}) void fluxCacheDoesntDependOnFirstRequest(Class configClass) { - AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class); + + AnnotationConfigApplicationContext ctx = + new AnnotationConfigApplicationContext(configClass, ReactiveCacheableService.class); ReactiveCacheableService service = ctx.getBean(ReactiveCacheableService.class); Object key = new Object(); @@ -135,6 +139,7 @@ class ReactiveCachingTests { ctx.close(); } + @CacheConfig(cacheNames = "first") static class ReactiveCacheableService { diff --git a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java index 3d0d0c0819..6597048cb1 100644 --- a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2023-2023 the original author or authors. + * Copyright 2023-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. @@ -28,8 +28,7 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** - * {@link ClientHttpRequestFactory} implementation based on the Java - * {@link HttpClient}. + * {@link ClientHttpRequestFactory} implementation based on the Java {@link HttpClient}. * * @author Marten Deinum * @author Arjen Poutsma @@ -89,13 +88,11 @@ public class JdkClientHttpRequestFactory implements ClientHttpRequestFactory { } /** - * Set the underlying {@code HttpClient}'s read timeout as a - * {@code Duration}. + * Set the underlying {@code HttpClient}'s read timeout as a {@code Duration}. *

Default is the system's default timeout. * @see java.net.http.HttpRequest.Builder#timeout */ public void setReadTimeout(Duration readTimeout) { - Assert.notNull(readTimeout, "ReadTimeout must not be null"); this.readTimeout = readTimeout; }