From ea419d2dcd5f1aa0e91744fd3305890e245032d1 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 25 Feb 2025 16:20:12 +0100 Subject: [PATCH] Avoid unnecessary CGLIB processing on configuration classes Closes gh-34486 (cherry picked from commit aff9ac72ec5e7ea426b963f9d2d2a613b67ef880) --- .../annotation/ConfigurationClass.java | 13 ++- .../annotation/ConfigurationClassParser.java | 79 +++++++++++-------- .../ConfigurationClassPostProcessorTests.java | 24 +++++- ...alidConfigurationClassDefinitionTests.java | 12 +-- 4 files changed, 86 insertions(+), 42 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java index f952bf666b..57375fcb9e 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java @@ -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. @@ -191,6 +191,15 @@ final class ConfigurationClass { return this.beanMethods; } + boolean hasNonStaticBeanMethods() { + for (BeanMethod beanMethod : this.beanMethods) { + if (!beanMethod.getMetadata().isStatic()) { + return true; + } + } + return false; + } + void addImportedResource(String importedResource, Class readerClass) { this.importedResources.put(importedResource, readerClass); } @@ -212,7 +221,7 @@ final class ConfigurationClass { // 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) { diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 9a7917011b..be21880217 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -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. @@ -81,10 +81,9 @@ import org.springframework.util.MultiValueMap; * any number of ConfigurationClass objects because one Configuration class may import * another using the {@link Import} annotation). * - *

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). + *

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). * *

This ASM-based implementation avoids reflection and eager class loading in order to * interoperate effectively with lazy class loading in a Spring ApplicationContext. @@ -161,14 +160,22 @@ class ConfigurationClassParser { 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) { @@ -183,31 +190,37 @@ class ConfigurationClassParser { this.deferredImportSelectorHandler.process(); } - protected final void parse(@Nullable String className, String beanName) throws IOException { + final ConfigurationClass parse(AnnotationMetadata metadata, String beanName) { + ConfigurationClass configClass = new ConfigurationClass(metadata, beanName); + processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER); + return configClass; + } + + final ConfigurationClass parse(Class clazz, String beanName) { + ConfigurationClass configClass = new ConfigurationClass(clazz, beanName); + processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER); + return configClass; + } + + 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); - processConfigurationClass(new ConfigurationClass(reader, beanName), DEFAULT_EXCLUSION_FILTER); - } - - protected final void parse(Class clazz, String beanName) throws IOException { - processConfigurationClass(new ConfigurationClass(clazz, beanName), DEFAULT_EXCLUSION_FILTER); - } - - protected final void parse(AnnotationMetadata metadata, String beanName) throws IOException { - processConfigurationClass(new ConfigurationClass(metadata, beanName), DEFAULT_EXCLUSION_FILTER); + 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 getConfigurationClasses() { + Set getConfigurationClasses() { return this.configurationClasses.keySet(); } @@ -216,7 +229,7 @@ class ConfigurationClassParser { Collections.emptyList()); } - protected void processConfigurationClass(ConfigurationClass configClass, Predicate filter) throws IOException { + protected void processConfigurationClass(ConfigurationClass configClass, Predicate filter) { if (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) { return; } @@ -448,7 +461,7 @@ class ConfigurationClassParser { /** - * Returns {@code @Import} class, considering all meta-annotations. + * Returns {@code @Import} classes, considering all meta-annotations. */ private Set getImports(SourceClass sourceClass) throws IOException { Set imports = new LinkedHashSet<>(); @@ -636,7 +649,7 @@ class ConfigurationClassParser { private final MultiValueMap imports = new LinkedMultiValueMap<>(); - public void registerImport(AnnotationMetadata importingClass, String importedClass) { + void registerImport(AnnotationMetadata importingClass, String importedClass) { this.imports.add(importedClass, importingClass); } @@ -691,7 +704,7 @@ class ConfigurationClassParser { * @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(); @@ -703,7 +716,7 @@ class ConfigurationClassParser { } } - public void process() { + void process() { List deferredImports = this.deferredImportSelectors; this.deferredImportSelectors = null; try { @@ -727,7 +740,7 @@ class ConfigurationClassParser { private final Map configurationClasses = new HashMap<>(); - public void register(DeferredImportSelectorHolder deferredImport) { + void register(DeferredImportSelectorHolder deferredImport) { Class group = deferredImport.getImportSelector().getImportGroup(); DeferredImportSelectorGrouping grouping = this.groupings.computeIfAbsent( (group != null ? group : deferredImport), @@ -737,7 +750,7 @@ class ConfigurationClassParser { deferredImport.getConfigurationClass()); } - public void processGroupImports() { + void processGroupImports() { for (DeferredImportSelectorGrouping grouping : this.groupings.values()) { Predicate exclusionFilter = grouping.getCandidateFilter(); grouping.getImports().forEach(entry -> { @@ -775,16 +788,16 @@ class ConfigurationClassParser { 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; } } @@ -800,7 +813,7 @@ class ConfigurationClassParser { this.group = group; } - public void add(DeferredImportSelectorHolder deferredImport) { + void add(DeferredImportSelectorHolder deferredImport) { this.deferredImports.add(deferredImport); } @@ -808,7 +821,7 @@ class ConfigurationClassParser { * Return the imports defined by the group. * @return each import with its associated configuration class */ - public Iterable getImports() { + Iterable getImports() { for (DeferredImportSelectorHolder deferredImport : this.deferredImports) { this.group.process(deferredImport.getConfigurationClass().getMetadata(), deferredImport.getImportSelector()); @@ -816,7 +829,7 @@ class ConfigurationClassParser { return this.group.selectImports(); } - public Predicate getCandidateFilter() { + Predicate getCandidateFilter() { Predicate mergedFilter = DEFAULT_EXCLUSION_FILTER; for (DeferredImportSelectorHolder deferredImport : this.deferredImports) { Predicate selectorFilter = deferredImport.getImportSelector().getExclusionFilter(); diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java index c205ac781a..806887a5f5 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassPostProcessorTests.java @@ -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. @@ -67,6 +67,7 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor; 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; @@ -104,6 +105,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -118,6 +120,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -132,6 +135,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -143,6 +147,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -154,6 +159,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -167,6 +173,7 @@ class ConfigurationClassPostProcessorTests { 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); @@ -174,6 +181,15 @@ class ConfigurationClassPostProcessorTests { 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")); @@ -1158,7 +1174,7 @@ class ConfigurationClassPostProcessorTests { } @Configuration - static class StaticSingletonBeanConfig { + static final class StaticSingletonBeanConfig { @Bean public static Foo foo() { @@ -1171,6 +1187,10 @@ class ConfigurationClassPostProcessorTests { } } + @Configuration + static final class EmptyConfig { + } + @Configuration @Order(2) static class OverridingSingletonBeanConfig { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/InvalidConfigurationClassDefinitionTests.java b/spring-context/src/test/java/org/springframework/context/annotation/InvalidConfigurationClassDefinitionTests.java index bd2bb4a410..4c1c8a8cf7 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/InvalidConfigurationClassDefinitionTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/InvalidConfigurationClassDefinitionTests.java @@ -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. @@ -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"); } }