Skip to content

Commit

Permalink
Allow @ConstructorBinding to be optional
Browse files Browse the repository at this point in the history
This commit makes @ConstructorBinding optional for a type
that has a single parameterized constructor. An @Autowired annotation
on any of the constructors indicates that the type should not be constructor
bound.

Since @ConstructorBinding is now deduced for a single parameterized constructor,
the annotation is no longer needed at the type level.

Closes gh-23216
  • Loading branch information
mbhave committed Feb 16, 2022
1 parent bc2c637 commit 44b88cc
Show file tree
Hide file tree
Showing 44 changed files with 692 additions and 390 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -52,7 +51,6 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.beans.BeanUtils;
import org.springframework.beans.BeansException;
import org.springframework.boot.actuate.endpoint.SanitizableData;
import org.springframework.boot.actuate.endpoint.Sanitizer;
Expand All @@ -63,7 +61,8 @@
import org.springframework.boot.context.properties.BoundConfigurationProperties;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConfigurationPropertiesBean;
import org.springframework.boot.context.properties.ConstructorBinding;
import org.springframework.boot.context.properties.ConfigurationPropertiesBindConstructorProvider;
import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.Name;
import org.springframework.boot.context.properties.source.ConfigurationProperty;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
Expand All @@ -72,11 +71,9 @@
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.KotlinDetector;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.env.PropertySource;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;
Expand Down Expand Up @@ -472,7 +469,9 @@ public List<BeanPropertyWriter> changeProperties(SerializationConfig config, Bea
List<BeanPropertyWriter> beanProperties) {
List<BeanPropertyWriter> result = new ArrayList<>();
Class<?> beanClass = beanDesc.getType().getRawClass();
Constructor<?> bindConstructor = findBindConstructor(ClassUtils.getUserClass(beanClass));
Bindable<?> bindable = Bindable.of(ClassUtils.getUserClass(beanClass));
Constructor<?> bindConstructor = ConfigurationPropertiesBindConstructorProvider.INSTANCE
.getBindConstructor(bindable, false);
for (BeanPropertyWriter writer : beanProperties) {
if (isCandidate(beanDesc, writer, bindConstructor)) {
result.add(writer);
Expand Down Expand Up @@ -540,34 +539,6 @@ private String determineAccessorSuffix(String propertyName) {
return StringUtils.capitalize(propertyName);
}

private Constructor<?> findBindConstructor(Class<?> type) {
boolean classConstructorBinding = MergedAnnotations
.from(type, SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES)
.isPresent(ConstructorBinding.class);
if (KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(type)) {
Constructor<?> constructor = BeanUtils.findPrimaryConstructor(type);
if (constructor != null) {
return findBindConstructor(classConstructorBinding, constructor);
}
}
return findBindConstructor(classConstructorBinding, type.getDeclaredConstructors());
}

private Constructor<?> findBindConstructor(boolean classConstructorBinding, Constructor<?>... candidates) {
List<Constructor<?>> candidateConstructors = Arrays.stream(candidates)
.filter((constructor) -> constructor.getParameterCount() > 0).collect(Collectors.toList());
List<Constructor<?>> flaggedConstructors = candidateConstructors.stream()
.filter((candidate) -> MergedAnnotations.from(candidate).isPresent(ConstructorBinding.class))
.collect(Collectors.toList());
if (flaggedConstructors.size() == 1) {
return flaggedConstructors.get(0);
}
if (classConstructorBinding && candidateConstructors.size() == 1) {
return candidateConstructors.get(0);
}
return null;
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.actuate.context.properties.ConfigurationPropertiesReportEndpoint.ConfigurationPropertiesBeanDescriptor;
import org.springframework.boot.actuate.context.properties.ConfigurationPropertiesReportEndpoint.ContextConfigurationProperties;
import org.springframework.boot.actuate.endpoint.SanitizingFunction;
Expand Down Expand Up @@ -72,6 +73,12 @@ void descriptorWithJavaBeanBindMethodDetectsRelevantProperties() {
(properties) -> assertThat(properties).containsOnlyKeys("dbPassword", "myTestProperty", "duration")));
}

@Test
void descriptorWithAutowiredConstructorBindMethodDetectsRelevantProperties() {
this.contextRunner.withUserConfiguration(AutowiredPropertiesConfiguration.class)
.run(assertProperties("autowired", (properties) -> assertThat(properties).containsOnlyKeys("counter")));
}

@Test
void descriptorWithValueObjectBindMethodDetectsRelevantProperties() {
this.contextRunner.withUserConfiguration(ImmutablePropertiesConfiguration.class).run(assertProperties(
Expand Down Expand Up @@ -489,7 +496,6 @@ static class ImmutablePropertiesConfiguration {
}

@ConfigurationProperties(prefix = "immutable")
@ConstructorBinding
public static class ImmutableProperties {

private final String dbPassword;
Expand Down Expand Up @@ -540,7 +546,6 @@ static class MultiConstructorPropertiesConfiguration {
}

@ConfigurationProperties(prefix = "multiconstructor")
@ConstructorBinding
public static class MultiConstructorProperties {

private final String name;
Expand Down Expand Up @@ -568,14 +573,50 @@ public int getCounter() {

}

@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(AutowiredProperties.class)
static class AutowiredPropertiesConfiguration {

@Bean
String hello() {
return "hello";
}

}

@ConfigurationProperties(prefix = "autowired")
public static class AutowiredProperties {

private final String name;

private int counter;

@Autowired
AutowiredProperties(String name) {
this.name = name;
}

public String getName() {
return this.name;
}

public int getCounter() {
return this.counter;
}

public void setCounter(int counter) {
this.counter = counter;
}

}

@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(ImmutableNestedProperties.class)
static class ImmutableNestedPropertiesConfiguration {

}

@ConfigurationProperties("immutablenested")
@ConstructorBinding
public static class ImmutableNestedProperties {

private final String name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.springframework.boot.actuate.context.properties;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConstructorBinding;
import org.springframework.validation.annotation.Validated;

/**
Expand All @@ -27,7 +26,6 @@
* @author Madhura Bhave
*/
@Validated
@ConstructorBinding
@ConfigurationProperties(prefix = "validated")
public class ValidatedConstructorBindingProperties {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.springframework.boot.autoconfigure.diagnostics.analyzer;

import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -41,15 +40,10 @@
import org.springframework.boot.autoconfigure.condition.ConditionEvaluationReport.ConditionAndOutcome;
import org.springframework.boot.autoconfigure.condition.ConditionEvaluationReport.ConditionAndOutcomes;
import org.springframework.boot.autoconfigure.condition.ConditionOutcome;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConstructorBinding;
import org.springframework.boot.diagnostics.FailureAnalysis;
import org.springframework.boot.diagnostics.analyzer.AbstractInjectionFailureAnalyzer;
import org.springframework.context.annotation.Bean;
import org.springframework.core.KotlinDetector;
import org.springframework.core.ResolvableType;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.type.MethodMetadata;
import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
import org.springframework.core.type.classreading.MetadataReader;
Expand Down Expand Up @@ -115,27 +109,6 @@ protected FailureAnalysis analyze(Throwable rootFailure, NoSuchBeanDefinitionExc
(!autoConfigurationResults.isEmpty() || !userConfigurationResults.isEmpty())
? "revisiting the entries above or defining" : "defining",
getBeanDescription(cause));
if (injectionPoint != null && injectionPoint.getMember() instanceof Constructor) {
Constructor<?> constructor = (Constructor<?>) injectionPoint.getMember();
Class<?> declaringClass = constructor.getDeclaringClass();
MergedAnnotation<ConfigurationProperties> configurationProperties = MergedAnnotations.from(declaringClass)
.get(ConfigurationProperties.class);
if (configurationProperties.isPresent()) {
if (KotlinDetector.isKotlinType(declaringClass) && !KotlinDetector.isKotlinReflectPresent()) {
action = String.format(
"%s%nConsider adding a dependency on kotlin-reflect so that the constructor used for @%s can be located. Also, ensure that @%s is present on '%s' if you intended to use constructor-based "
+ "configuration property binding.",
action, ConstructorBinding.class.getSimpleName(), ConstructorBinding.class.getSimpleName(),
constructor.getName());
}
else {
action = String.format(
"%s%nConsider adding @%s to %s if you intended to use constructor-based "
+ "configuration property binding.",
action, ConstructorBinding.class.getSimpleName(), constructor.getName());
}
}
}
return new FailureAnalysis(message.toString(), action, cause);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2020 the original author or authors.
* Copyright 2012-2022 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 @@ -30,8 +30,6 @@
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.diagnostics.FailureAnalysis;
import org.springframework.boot.diagnostics.LoggingFailureAnalysisReporter;
import org.springframework.boot.system.JavaVersion;
Expand Down Expand Up @@ -164,16 +162,6 @@ private String determineAnnotationValuePattern() {
return "@org.springframework.beans.factory.annotation.Qualifier\\(value=\"*alpha\"*\\)";
}

@Test
void failureAnalysisForConfigurationPropertiesThatMaybeShouldHaveBeenConstructorBound() {
FailureAnalysis analysis = analyzeFailure(
createFailure(ConstructorBoundConfigurationPropertiesConfiguration.class));
assertThat(analysis.getAction()).startsWith(
String.format("Consider defining a bean of type '%s' in your configuration.", String.class.getName()));
assertThat(analysis.getAction()).contains(
"Consider adding @ConstructorBinding to " + NeedsConstructorBindingProperties.class.getName());
}

private void assertDescriptionConstructorMissingType(FailureAnalysis analysis, Class<?> component, int index,
Class<?> type) {
String expected = String.format(
Expand Down Expand Up @@ -379,25 +367,4 @@ static class StringNameHandler {

}

@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties(NeedsConstructorBindingProperties.class)
static class ConstructorBoundConfigurationPropertiesConfiguration {

}

@ConfigurationProperties("test")
static class NeedsConstructorBindingProperties {

private final String name;

NeedsConstructorBindingProperties(String name) {
this.name = name;
}

String getName() {
return this.name;
}

}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ You could also let the AspectJ plugin run all the processing and disable annotat
=== Automatic Metadata Generation
The processor picks up both classes and methods that are annotated with `@ConfigurationProperties`.

If the class is also annotated with `@ConstructorBinding`, a single constructor is expected and one property is created per constructor parameter.
If the class has a single parameterized constructor, one property is created per constructor parameter, unless the constructor is annotated with `@Autowired`.
If the class has a constructor explicitly annotated with `@ConstructorBinding`, one property is created per constructor parameter for that constructor.
Otherwise, properties are discovered through the presence of standard getters and setters with special handling for collection and map types (that is detected even if only a getter is present).
The annotation processor also supports the use of the `@Data`, `@Value`, `@Getter`, and `@Setter` lombok annotations.

Expand Down
Loading

0 comments on commit 44b88cc

Please sign in to comment.