Skip to content

Commit

Permalink
Avoid unnecessary CGLIB processing on configuration classes
Browse files Browse the repository at this point in the history 
Closes gh-34486

(cherry picked from commit aff9ac7)
  • Loading branch information
@jhoeller
jhoeller committed Feb 25, 2025
1 parent c02d07f commit ea419d2
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 40 deletions.
13 changes: 11 additions & 2 deletions spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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 Down Expand Up @@ -191,6 +191,15 @@ Set<BeanMethod> getBeanMethods() {
return this.beanMethods;
}

boolean hasNonStaticBeanMethods() {
for (BeanMethod beanMethod : this.beanMethods) {
if (!beanMethod.getMetadata().isStatic()) {
return true;
}
}
return false;
}

void addImportedResource(String importedResource, Class<? extends BeanDefinitionReader> readerClass) {
this.importedResources.put(importedResource, readerClass);
}
Expand All @@ -212,7 +221,7 @@ void validate(ProblemReporter problemReporter) {

// A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false
if (attributes != null && (Boolean) attributes.get("proxyBeanMethods")) {
if (this.metadata.isFinal()) {
if (hasNonStaticBeanMethods() && this.metadata.isFinal()) {
problemReporter.error(new FinalConfigurationProblem());
}
for (BeanMethod beanMethod : this.beanMethods) {
Expand Down
75 changes: 44 additions & 31 deletions ...ontext/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 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 Down Expand Up @@ -81,10 +81,9 @@
* any number of ConfigurationClass objects because one Configuration class may import
* another using the {@link Import} annotation).
*
* <p>This class helps separate the concern of parsing the structure of a Configuration
* class from the concern of registering BeanDefinition objects based on the content of
* that model (with the exception of {@code @ComponentScan} annotations which need to be
* registered immediately).
* <p>This class helps separate the concern of parsing the structure of a Configuration class
* from the concern of registering BeanDefinition objects based on the content of that model
* (except {@code @ComponentScan} annotations which need to be registered immediately).
*
* <p>This ASM-based implementation avoids reflection and eager class loading in order to
* interoperate effectively with lazy class loading in a Spring ApplicationContext.
Expand Down Expand Up @@ -161,14 +160,22 @@ public void parse(Set<BeanDefinitionHolder> configCandidates) {
for (BeanDefinitionHolder holder : configCandidates) {
BeanDefinition bd = holder.getBeanDefinition();
try {
ConfigurationClass configClass;
if (bd instanceof AnnotatedBeanDefinition annotatedBeanDef) {
parse(annotatedBeanDef.getMetadata(), holder.getBeanName());
configClass = parse(annotatedBeanDef.getMetadata(), holder.getBeanName());
}
else if (bd instanceof AbstractBeanDefinition abstractBeanDef && abstractBeanDef.hasBeanClass()) {
parse(abstractBeanDef.getBeanClass(), holder.getBeanName());
configClass = parse(abstractBeanDef.getBeanClass(), holder.getBeanName());
}
else {
parse(bd.getBeanClassName(), holder.getBeanName());
configClass = parse(bd.getBeanClassName(), holder.getBeanName());
}

// Downgrade to lite (no enhancement) in case of no instance-level @Bean methods.
if (!configClass.hasNonStaticBeanMethods() && ConfigurationClassUtils.CONFIGURATION_CLASS_FULL.equals(
bd.getAttribute(ConfigurationClassUtils.CONFIGURATION_CLASS_ATTRIBUTE))) {
bd.setAttribute(ConfigurationClassUtils.CONFIGURATION_CLASS_ATTRIBUTE,
ConfigurationClassUtils.CONFIGURATION_CLASS_LITE);
}
}
catch (BeanDefinitionStoreException ex) {
Expand All @@ -183,31 +190,37 @@ else if (bd instanceof AbstractBeanDefinition abstractBeanDef && abstractBeanDef
this.deferredImportSelectorHandler.process();
}

protected final void parse(@Nullable String className, String beanName) throws IOException {
Assert.notNull(className, "No bean class name for configuration class bean definition");
MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className);
processConfigurationClass(new ConfigurationClass(reader, beanName), DEFAULT_EXCLUSION_FILTER);
final ConfigurationClass parse(AnnotationMetadata metadata, String beanName) {
ConfigurationClass configClass = new ConfigurationClass(metadata, beanName);
processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER);
return configClass;
}

protected final void parse(Class<?> clazz, String beanName) throws IOException {
processConfigurationClass(new ConfigurationClass(clazz, beanName), DEFAULT_EXCLUSION_FILTER);
final ConfigurationClass parse(Class<?> clazz, String beanName) {
ConfigurationClass configClass = new ConfigurationClass(clazz, beanName);
processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER);
return configClass;
}

protected final void parse(AnnotationMetadata metadata, String beanName) throws IOException {
processConfigurationClass(new ConfigurationClass(metadata, beanName), DEFAULT_EXCLUSION_FILTER);
final ConfigurationClass parse(@Nullable String className, String beanName) throws IOException {
Assert.notNull(className, "No bean class name for configuration class bean definition");
MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className);
ConfigurationClass configClass = new ConfigurationClass(reader, beanName);
processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER);
return configClass;
}

/**
* Validate each {@link ConfigurationClass} object.
* @see ConfigurationClass#validate
*/
public void validate() {
void validate() {
for (ConfigurationClass configClass : this.configurationClasses.keySet()) {
configClass.validate(this.problemReporter);
}
}

public Set<ConfigurationClass> getConfigurationClasses() {
Set<ConfigurationClass> getConfigurationClasses() {
return this.configurationClasses.keySet();
}

Expand All @@ -216,7 +229,7 @@ List<PropertySourceDescriptor> getPropertySourceDescriptors() {
Collections.emptyList());
}

protected void processConfigurationClass(ConfigurationClass configClass, Predicate<String> filter) throws IOException {
protected void processConfigurationClass(ConfigurationClass configClass, Predicate<String> filter) {
if (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) {
return;
}
Expand Down Expand Up @@ -448,7 +461,7 @@ private Set<MethodMetadata> retrieveBeanMethodMetadata(SourceClass sourceClass)


/**
* Returns {@code @Import} class, considering all meta-annotations.
* Returns {@code @Import} classes, considering all meta-annotations.
*/
private Set<SourceClass> getImports(SourceClass sourceClass) throws IOException {
Set<SourceClass> imports = new LinkedHashSet<>();
Expand Down Expand Up @@ -636,7 +649,7 @@ private static class ImportStack extends ArrayDeque<ConfigurationClass> implemen

private final MultiValueMap<String, AnnotationMetadata> imports = new LinkedMultiValueMap<>();

public void registerImport(AnnotationMetadata importingClass, String importedClass) {
void registerImport(AnnotationMetadata importingClass, String importedClass) {
this.imports.add(importedClass, importingClass);
}

Expand Down Expand Up @@ -691,7 +704,7 @@ private class DeferredImportSelectorHandler {
* @param configClass the source configuration class
* @param importSelector the selector to handle
*/
public void handle(ConfigurationClass configClass, DeferredImportSelector importSelector) {
void handle(ConfigurationClass configClass, DeferredImportSelector importSelector) {
DeferredImportSelectorHolder holder = new DeferredImportSelectorHolder(configClass, importSelector);
if (this.deferredImportSelectors == null) {
DeferredImportSelectorGroupingHandler handler = new DeferredImportSelectorGroupingHandler();
Expand All @@ -703,7 +716,7 @@ public void handle(ConfigurationClass configClass, DeferredImportSelector import
}
}

public void process() {
void process() {
List<DeferredImportSelectorHolder> deferredImports = this.deferredImportSelectors;
this.deferredImportSelectors = null;
try {
Expand All @@ -727,7 +740,7 @@ private class DeferredImportSelectorGroupingHandler {

private final Map<AnnotationMetadata, ConfigurationClass> configurationClasses = new HashMap<>();

public void register(DeferredImportSelectorHolder deferredImport) {
void register(DeferredImportSelectorHolder deferredImport) {
Class<? extends Group> group = deferredImport.getImportSelector().getImportGroup();
DeferredImportSelectorGrouping grouping = this.groupings.computeIfAbsent(
(group != null ? group : deferredImport),
Expand All @@ -737,7 +750,7 @@ public void register(DeferredImportSelectorHolder deferredImport) {
deferredImport.getConfigurationClass());
}

public void processGroupImports() {
void processGroupImports() {
for (DeferredImportSelectorGrouping grouping : this.groupings.values()) {
Predicate<String> exclusionFilter = grouping.getCandidateFilter();
grouping.getImports().forEach(entry -> {
Expand Down Expand Up @@ -775,16 +788,16 @@ private static class DeferredImportSelectorHolder {

private final DeferredImportSelector importSelector;

public DeferredImportSelectorHolder(ConfigurationClass configClass, DeferredImportSelector selector) {
DeferredImportSelectorHolder(ConfigurationClass configClass, DeferredImportSelector selector) {
this.configurationClass = configClass;
this.importSelector = selector;
}

public ConfigurationClass getConfigurationClass() {
ConfigurationClass getConfigurationClass() {
return this.configurationClass;
}

public DeferredImportSelector getImportSelector() {
DeferredImportSelector getImportSelector() {
return this.importSelector;
}
}
Expand All @@ -800,23 +813,23 @@ private static class DeferredImportSelectorGrouping {
this.group = group;
}

public void add(DeferredImportSelectorHolder deferredImport) {
void add(DeferredImportSelectorHolder deferredImport) {
this.deferredImports.add(deferredImport);
}

/**
* Return the imports defined by the group.
* @return each import with its associated configuration class
*/
public Iterable<Group.Entry> getImports() {
Iterable<Group.Entry> getImports() {
for (DeferredImportSelectorHolder deferredImport : this.deferredImports) {
this.group.process(deferredImport.getConfigurationClass().getMetadata(),
deferredImport.getImportSelector());
}
return this.group.selectImports();
}

public Predicate<String> getCandidateFilter() {
Predicate<String> getCandidateFilter() {
Predicate<String> mergedFilter = DEFAULT_EXCLUSION_FILTER;
for (DeferredImportSelectorHolder deferredImport : this.deferredImports) {
Predicate<String> selectorFilter = deferredImport.getImportSelector().getExclusionFilter();
Expand Down
24 changes: 22 additions & 2 deletions ...est/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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 Down Expand Up @@ -67,6 +67,7 @@
import org.springframework.core.task.SyncTaskExecutor;
import org.springframework.stereotype.Component;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -104,6 +105,7 @@ void enhancementIsPresentBecauseSingletonSemanticsAreRespected() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).contains(ClassUtils.CGLIB_CLASS_SEPARATOR);
Foo foo = beanFactory.getBean("foo", Foo.class);
Bar bar = beanFactory.getBean("bar", Bar.class);
assertThat(bar.foo).isSameAs(foo);
Expand All @@ -118,6 +120,7 @@ void enhancementIsPresentBecauseSingletonSemanticsAreRespectedUsingAsm() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).contains(ClassUtils.CGLIB_CLASS_SEPARATOR);
Foo foo = beanFactory.getBean("foo", Foo.class);
Bar bar = beanFactory.getBean("bar", Bar.class);
assertThat(bar.foo).isSameAs(foo);
Expand All @@ -132,6 +135,7 @@ void enhancementIsNotPresentForProxyBeanMethodsFlagSetToFalse() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).doesNotContain(ClassUtils.CGLIB_CLASS_SEPARATOR);
Foo foo = beanFactory.getBean("foo", Foo.class);
Bar bar = beanFactory.getBean("bar", Bar.class);
assertThat(bar.foo).isNotSameAs(foo);
Expand All @@ -143,6 +147,7 @@ void enhancementIsNotPresentForProxyBeanMethodsFlagSetToFalseUsingAsm() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).doesNotContain(ClassUtils.CGLIB_CLASS_SEPARATOR);
Foo foo = beanFactory.getBean("foo", Foo.class);
Bar bar = beanFactory.getBean("bar", Bar.class);
assertThat(bar.foo).isNotSameAs(foo);
Expand All @@ -154,6 +159,7 @@ void enhancementIsNotPresentForStaticMethods() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).doesNotContain(ClassUtils.CGLIB_CLASS_SEPARATOR);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("foo")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("bar")).hasBeanClass()).isTrue();
Foo foo = beanFactory.getBean("foo", Foo.class);
Expand All @@ -167,13 +173,23 @@ void enhancementIsNotPresentForStaticMethodsUsingAsm() {
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).doesNotContain(ClassUtils.CGLIB_CLASS_SEPARATOR);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("foo")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("bar")).hasBeanClass()).isTrue();
Foo foo = beanFactory.getBean("foo", Foo.class);
Bar bar = beanFactory.getBean("bar", Bar.class);
assertThat(bar.foo).isNotSameAs(foo);
}

@Test
void enhancementIsNotPresentWithEmptyConfig() {
beanFactory.registerBeanDefinition("config", new RootBeanDefinition(EmptyConfig.class));
ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
pp.postProcessBeanFactory(beanFactory);
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).hasBeanClass()).isTrue();
assertThat(((RootBeanDefinition) beanFactory.getBeanDefinition("config")).getBeanClass().getName()).doesNotContain(ClassUtils.CGLIB_CLASS_SEPARATOR);
}

@Test
void configurationIntrospectionOfInnerClassesWorksWithDotNameSyntax() {
beanFactory.registerBeanDefinition("config", new RootBeanDefinition(getClass().getName() + ".SingletonBeanConfig"));
Expand Down Expand Up @@ -1158,7 +1174,7 @@ static class NonEnhancedSingletonBeanConfig {
}

@Configuration
static class StaticSingletonBeanConfig {
static final class StaticSingletonBeanConfig {

@Bean
public static Foo foo() {
Expand All @@ -1171,6 +1187,10 @@ public static Bar bar() {
}
}

@Configuration
static final class EmptyConfig {
}

@Configuration
@Order(2)
static class OverridingSingletonBeanConfig {
Expand Down
12 changes: 7 additions & 5 deletions ...java/org/springframework/context/annotation/InvalidConfigurationClassDefinitionTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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 Down Expand Up @@ -37,16 +37,18 @@ class InvalidConfigurationClassDefinitionTests {
@Test
void configurationClassesMayNotBeFinal() {
@Configuration
final class Config { }
final class Config {
@Bean String dummy() { return "dummy"; }
}

BeanDefinition configBeanDef = rootBeanDefinition(Config.class).getBeanDefinition();
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
beanFactory.registerBeanDefinition("config", configBeanDef);

ConfigurationClassPostProcessor pp = new ConfigurationClassPostProcessor();
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
pp.postProcessBeanFactory(beanFactory))
.withMessageContaining("Remove the final modifier");
assertThatExceptionOfType(BeanDefinitionParsingException.class)
.isThrownBy(() -> pp.postProcessBeanFactory(beanFactory))
.withMessageContaining("Remove the final modifier");
}

}

0 comments on commit ea419d2

Please sign in to comment.