Avoid duplicate JCacheOperationSource bean registration in <cache:annotation-driven /> #27499
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In our application we use xml context and <cache:annotation-driven /> declaration. Also we disable bean definition duplication by setting GenericApplicationContext.setAllowBeanDefinitionOverriding(false) in ApplicationContextInitializer. Such combination leads to BeanDefinitionOverrideException because bean "org.springframework.cache.jcache.interceptor.DefaultJCacheOperationSource" is registered twice: String sourceName = parserContext.getReaderContext().registerWithGeneratedName(sourceDef); parserContext.registerBeanComponent(new BeanComponentDefinition(sourceDef, sourceName));
|
@nivolg Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
spring-projects-issues
added
the
status: waiting-for-triage
|
@pivotal-cla This is an Obvious Fix |
|
@nivolg This Pull Request contains an obvious fix. Signing the Contributor License Agreement is not necessary. |
|
Thanks for the report and PR @nivolg. I wasn't able to reproduce the problem with a vanilla project started from start.spring.io. Please look at it and let us know what is missing. If it turns out that a change is required in Spring Framework, I unfortunately disagree that this is an obvious fix so you'll have to sign the CLA. |
snicoll
added
the
status: waiting-for-feedback
|
@nivolg Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
status: feedback-provided
|
Thanks for quick response. I can't access your vanilla project repopsitory. But I made example project to reproduce the issue. I missed an important details in description: aspectj and jcache are used together. Also I signed CLA. |
|
@nivolg bummer, it got created as private by default, sorry about that. Thank you for the sample, I missed the aspectJ bits. |
|
Thanks for the repro project. I have confirmed that the following exception occurs. But I have not yet confirmed the proposed fix. We'll look into this. |
sbrannen
added
in: core
sbrannen
reviewed
Oct 12, 2021
| @@ -245,16 +245,20 @@ private static void registerCacheAdvisor(Element element, ParserContext parserCo | |||
| 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); | |||
There was a problem hiding this comment.
Suggested change
| String sourceName = parserContext.getReaderContext().registerWithGeneratedName(sourceDef); | |
| String sourceName = parserContext.getReaderContext().generateBeanName(sourceDef); |
This would be a simpler fix, since it avoids the undesired side effect of registering the bean when generating the name (and the rest of the code can remain unchanged), but I think your CompositeComponentDefinition proposal is more in line with how components are handled for XML namespaces. So we'll go with that.
There was a problem hiding this comment.
Thanks for review. I understand what you mean.
I thought about such simpler fix but read the methods above and decided that CompositeComponentDefinition is required for some reasons I don't know.
There was a problem hiding this comment.
The CompositeComponentDefinition is not technically required.
It's really just there for tooling support. Without it, the application would work the same.
sbrannen
changed the title
sbrannen
added a commit
that referenced
this pull request
Oct 12, 2021
sbrannen
added
the
for: backport-to-5.2.x
spring-projects-issues
added
status: backported
sbrannen
pushed a commit
to sbrannen/spring-framework
that referenced
this pull request
Oct 12, 2021
…otation-driven /> In our application we use XML context and <cache:annotation-driven /> 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 spring-projectsgh-27499
sbrannen
added a commit
to sbrannen/spring-framework
that referenced
this pull request
Oct 12, 2021
This was referenced Oct 14, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello.
In our application we use XML context and
<cache:annotation-driven mode="aspectj"/>declaration in combination with JCache API. Also we disable bean definition overrides by settingGenericApplicationContext.setAllowBeanDefinitionOverriding(false)in anApplicationContextInitializer.Such combination leads to
BeanDefinitionOverrideExceptionbecause the bean "org.springframework.cache.jcache.interceptor.DefaultJCacheOperationSource" is registered twice: