From aff9ac72ec5e7ea426b963f9d2d2a613b67ef880 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 --- .../annotation/ConfigurationClass.java | 16 ++++++-- .../annotation/ConfigurationClassParser.java | 37 +++++++++++++------ .../ConfigurationClassPostProcessorTests.java | 24 +++++++++++- ...alidConfigurationClassDefinitionTests.java | 12 +++--- .../annotation/ValueCglibConfiguration.java | 9 ++++- 5 files changed, 75 insertions(+), 23 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 7664cb69813..53daf3105ba 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. @@ -203,6 +203,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); } @@ -223,8 +232,9 @@ final class ConfigurationClass { void validate(ProblemReporter problemReporter) { Map attributes = this.metadata.getAnnotationAttributes(Configuration.class.getName()); - // A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false - if (attributes != null && (Boolean) attributes.get("proxyBeanMethods") && this.metadata.isFinal()) { + // A configuration class may not be final (CGLIB limitation) unless it does not have to proxy bean methods + if (attributes != null && (Boolean) attributes.get("proxyBeanMethods") && hasNonStaticBeanMethods() && + this.metadata.isFinal()) { problemReporter.error(new FinalConfigurationProblem()); } 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 7fd102e2a9d..ba3f926fd74 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-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. @@ -167,14 +167,22 @@ class ConfigurationClassParser { for (BeanDefinitionHolder holder : configCandidates) { BeanDefinition bd = holder.getBeanDefinition(); try { + ConfigurationClass configClass; if (bd instanceof AnnotatedBeanDefinition annotatedBeanDef) { - parse(annotatedBeanDef, holder.getBeanName()); + configClass = parse(annotatedBeanDef, 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) { @@ -189,20 +197,25 @@ class ConfigurationClassParser { this.deferredImportSelectorHandler.process(); } - private void parse(AnnotatedBeanDefinition beanDef, String beanName) { - processConfigurationClass( - new ConfigurationClass(beanDef.getMetadata(), beanName, (beanDef instanceof ScannedGenericBeanDefinition)), - DEFAULT_EXCLUSION_FILTER); + private ConfigurationClass parse(AnnotatedBeanDefinition beanDef, String beanName) { + ConfigurationClass configClass = new ConfigurationClass( + beanDef.getMetadata(), beanName, (beanDef instanceof ScannedGenericBeanDefinition)); + processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER); + return configClass; } - private void parse(Class clazz, String beanName) { - processConfigurationClass(new ConfigurationClass(clazz, beanName), DEFAULT_EXCLUSION_FILTER); + private ConfigurationClass parse(Class clazz, String beanName) { + ConfigurationClass configClass = new ConfigurationClass(clazz, beanName); + processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER); + return configClass; } - final void parse(@Nullable String className, String beanName) throws IOException { + 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); + ConfigurationClass configClass = new ConfigurationClass(reader, beanName); + processConfigurationClass(configClass, DEFAULT_EXCLUSION_FILTER); + return configClass; } /** 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 ef810f56a5f..122111aee98 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")); @@ -1166,7 +1182,7 @@ class ConfigurationClassPostProcessorTests { } @Configuration - static class StaticSingletonBeanConfig { + static final class StaticSingletonBeanConfig { @Bean public static Foo foo() { @@ -1179,6 +1195,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 8b1094ed6e1..9f535f63b16 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"); } } diff --git a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/context/annotation/ValueCglibConfiguration.java b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/context/annotation/ValueCglibConfiguration.java index 506e99e56a9..317fa2d2784 100644 --- a/spring-context/src/testFixtures/java/org/springframework/context/testfixture/context/annotation/ValueCglibConfiguration.java +++ b/spring-context/src/testFixtures/java/org/springframework/context/testfixture/context/annotation/ValueCglibConfiguration.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. @@ -17,6 +17,7 @@ package org.springframework.context.testfixture.context.annotation; import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @Configuration @@ -31,4 +32,10 @@ public class ValueCglibConfiguration { public String getName() { return this.name; } + + @Bean + public String dummy() { + return "dummy"; + } + }