From 50ccb1bfcd93318508ec656e2cfe513b2e2bc6d2 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 30 Sep 2021 22:40:21 +0300 Subject: [PATCH 1/2] Avoid duplicate JCacheOperationSource bean registration in In our application we use XML context and declaration. Also we disable bean definition duplication by setting GenericApplicationContext.setAllowBeanDefinitionOverriding(false) in an ApplicationContextInitializer. This combination leads to a BeanDefinitionOverrideException because the DefaultJCacheOperationSource bean is registered twice. - once for: parserContext.getReaderContext().registerWithGeneratedName(sourceDef); - once for: parserContext.registerBeanComponent(new BeanComponentDefinition(sourceDef, sourceName)); This commit refactors JCacheCachingConfigurer.registerCacheAspect(...) so that the JCacheOperationSource bean is registered only once. Closes gh-27499 --- .../AnnotationDrivenCacheBeanDefinitionParser.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java b/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java index 41c0bbc7b6d..c7466616d49 100644 --- a/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java +++ b/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java @@ -245,16 +245,20 @@ class AnnotationDrivenCacheBeanDefinitionParser implements BeanDefinitionParser private static void registerCacheAspect(Element element, ParserContext parserContext) { if (!parserContext.getRegistry().containsBeanDefinition(CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)) { Object eleSource = parserContext.extractSource(element); + + BeanDefinition sourceDef = createJCacheOperationSourceBeanDefinition(element, eleSource); + String sourceName = parserContext.getReaderContext().registerWithGeneratedName(sourceDef); + RootBeanDefinition def = new RootBeanDefinition(); def.setBeanClassName(JCACHE_ASPECT_CLASS_NAME); def.setFactoryMethodName("aspectOf"); - BeanDefinition sourceDef = createJCacheOperationSourceBeanDefinition(element, eleSource); - String sourceName = - parserContext.getReaderContext().registerWithGeneratedName(sourceDef); def.getPropertyValues().add("cacheOperationSource", new RuntimeBeanReference(sourceName)); + parserContext.getRegistry().registerBeanDefinition(CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME, def); - parserContext.registerBeanComponent(new BeanComponentDefinition(sourceDef, sourceName)); - parserContext.registerBeanComponent(new BeanComponentDefinition(def, CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)); + CompositeComponentDefinition compositeDef = new CompositeComponentDefinition(element.getTagName(), eleSource); + compositeDef.addNestedComponent(new BeanComponentDefinition(sourceDef, sourceName)); + compositeDef.addNestedComponent(new BeanComponentDefinition(def, CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)); + parserContext.registerComponent(compositeDef); } } From 5bd90538b32a84fd84026bceeae17b901cc39910 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 12 Oct 2021 12:22:24 +0200 Subject: [PATCH 2/2] Introduce test for gh-27499 and polish contribution --- .../JCacheAspectJNamespaceConfigTests.java | 11 ++++++--- ...tationDrivenCacheBeanDefinitionParser.java | 24 +++++++++---------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/spring-aspects/src/test/java/org/springframework/cache/aspectj/JCacheAspectJNamespaceConfigTests.java b/spring-aspects/src/test/java/org/springframework/cache/aspectj/JCacheAspectJNamespaceConfigTests.java index 81ba6d0faff..c755d8c3f4a 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/aspectj/JCacheAspectJNamespaceConfigTests.java +++ b/spring-aspects/src/test/java/org/springframework/cache/aspectj/JCacheAspectJNamespaceConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2021 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,13 +22,18 @@ import org.springframework.contextsupport.testfixture.jcache.AbstractJCacheAnnot /** * @author Stephane Nicoll + * @author Sam Brannen */ public class JCacheAspectJNamespaceConfigTests extends AbstractJCacheAnnotationTests { @Override protected ApplicationContext getApplicationContext() { - return new GenericXmlApplicationContext( - "/org/springframework/cache/config/annotation-jcache-aspectj.xml"); + GenericXmlApplicationContext context = new GenericXmlApplicationContext(); + // Disallow bean definition overriding to test https://github.com/spring-projects/spring-framework/pull/27499 + context.setAllowBeanDefinitionOverriding(false); + context.load("/org/springframework/cache/config/annotation-jcache-aspectj.xml"); + context.refresh(); + return context; } } diff --git a/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java b/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java index c7466616d49..f2768059a0d 100644 --- a/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java +++ b/spring-context/src/main/java/org/springframework/cache/config/AnnotationDrivenCacheBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2021 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. @@ -244,20 +244,20 @@ class AnnotationDrivenCacheBeanDefinitionParser implements BeanDefinitionParser private static void registerCacheAspect(Element element, ParserContext parserContext) { if (!parserContext.getRegistry().containsBeanDefinition(CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)) { - Object eleSource = parserContext.extractSource(element); + Object source = parserContext.extractSource(element); - BeanDefinition sourceDef = createJCacheOperationSourceBeanDefinition(element, eleSource); - String sourceName = parserContext.getReaderContext().registerWithGeneratedName(sourceDef); + BeanDefinition cacheOperationSourceDef = createJCacheOperationSourceBeanDefinition(element, source); + String cacheOperationSourceName = parserContext.getReaderContext().registerWithGeneratedName(cacheOperationSourceDef); - RootBeanDefinition def = new RootBeanDefinition(); - def.setBeanClassName(JCACHE_ASPECT_CLASS_NAME); - def.setFactoryMethodName("aspectOf"); - def.getPropertyValues().add("cacheOperationSource", new RuntimeBeanReference(sourceName)); - parserContext.getRegistry().registerBeanDefinition(CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME, def); + RootBeanDefinition jcacheAspectDef = new RootBeanDefinition(); + jcacheAspectDef.setBeanClassName(JCACHE_ASPECT_CLASS_NAME); + jcacheAspectDef.setFactoryMethodName("aspectOf"); + jcacheAspectDef.getPropertyValues().add("cacheOperationSource", new RuntimeBeanReference(cacheOperationSourceName)); + parserContext.getRegistry().registerBeanDefinition(CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME, jcacheAspectDef); - CompositeComponentDefinition compositeDef = new CompositeComponentDefinition(element.getTagName(), eleSource); - compositeDef.addNestedComponent(new BeanComponentDefinition(sourceDef, sourceName)); - compositeDef.addNestedComponent(new BeanComponentDefinition(def, CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)); + CompositeComponentDefinition compositeDef = new CompositeComponentDefinition(element.getTagName(), source); + compositeDef.addNestedComponent(new BeanComponentDefinition(cacheOperationSourceDef, cacheOperationSourceName)); + compositeDef.addNestedComponent(new BeanComponentDefinition(jcacheAspectDef, CacheManagementConfigUtils.JCACHE_ASPECT_BEAN_NAME)); parserContext.registerComponent(compositeDef); } }