Skip to content

Commit e1c33a7

Browse files
scottfredericksnicoll
authored andcommitted
Add since attribute to uses of DeprecatedConfigurationProperty
Add an architecture rule to ensure that all usages of `@DeprecatedConfigurationProperty` in the Spring Boot codebase include the `since` attribute. Add the `since` attribute to the few places where it was not already included. See gh-47980 Signed-off-by: Scott Frederick <[email protected]>
1 parent 685c66a commit e1c33a7

File tree

8 files changed

+188
-23
lines changed

8 files changed

+188
-23
lines changed

buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.ArrayList;
2727
import java.util.Collections;
2828
import java.util.List;
29+
import java.util.Map;
2930
import java.util.concurrent.Callable;
3031
import java.util.function.Supplier;
3132
import java.util.stream.Stream;
@@ -42,6 +43,7 @@
4243
import org.gradle.api.file.FileCollection;
4344
import org.gradle.api.file.FileTree;
4445
import org.gradle.api.provider.ListProperty;
46+
import org.gradle.api.provider.MapProperty;
4547
import org.gradle.api.provider.Property;
4648
import org.gradle.api.provider.Provider;
4749
import org.gradle.api.tasks.Classpath;
@@ -70,19 +72,27 @@
7072
*/
7173
public abstract class ArchitectureCheck extends DefaultTask {
7274

75+
static final String CONDITIONAL_ON_CLASS = "ConditionalOnClass";
76+
77+
static final String DEPRECATED_CONFIGURATION_PROPERTY = "DeprecatedConfigurationProperty";
78+
7379
private static final String CONDITIONAL_ON_CLASS_ANNOTATION = "org.springframework.boot.autoconfigure.condition.ConditionalOnClass";
7480

81+
private static final String DEPRECATED_CONFIGURATION_PROPERTY_ANNOTATION = "org.springframework.boot.context.properties.DeprecatedConfigurationProperty";
82+
7583
private FileCollection classes;
7684

7785
public ArchitectureCheck() {
7886
getOutputDirectory().convention(getProject().getLayout().getBuildDirectory().dir(getName()));
79-
getConditionalOnClassAnnotation().convention(CONDITIONAL_ON_CLASS_ANNOTATION);
87+
getAnnotationClasses().convention(Map.of(CONDITIONAL_ON_CLASS, CONDITIONAL_ON_CLASS_ANNOTATION,
88+
DEPRECATED_CONFIGURATION_PROPERTY, DEPRECATED_CONFIGURATION_PROPERTY_ANNOTATION));
8089
getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
8190
.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
8291
getRules().addAll(ArchitectureRules.standard());
83-
getRules().addAll(whenMainSources(() -> List
84-
.of(ArchitectureRules.allBeanMethodsShouldReturnNonPrivateType(), ArchitectureRules
85-
.allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(getConditionalOnClassAnnotation().get()))));
92+
getRules().addAll(whenMainSources(() -> ArchitectureRules
93+
.beanMethods(annotationClassFor(CONDITIONAL_ON_CLASS, CONDITIONAL_ON_CLASS_ANNOTATION))));
94+
getRules().addAll(whenMainSources(() -> ArchitectureRules.configurationProperties(
95+
annotationClassFor(DEPRECATED_CONFIGURATION_PROPERTY, DEPRECATED_CONFIGURATION_PROPERTY_ANNOTATION))));
8696
getRuleDescriptions().set(getRules().map(this::asDescriptions));
8797
}
8898

@@ -100,6 +110,10 @@ private List<String> asDescriptions(List<ArchRule> rules) {
100110
return rules.stream().map(ArchRule::getDescription).toList();
101111
}
102112

113+
private String annotationClassFor(String name, String defaultValue) {
114+
return getAnnotationClasses().get().getOrDefault(name, defaultValue);
115+
}
116+
103117
@TaskAction
104118
void checkArchitecture() throws Exception {
105119
withCompileClasspath(() -> {
@@ -190,7 +204,7 @@ final FileTree getInputClasses() {
190204
@Input // Use descriptions as input since rules aren't serializable
191205
abstract ListProperty<String> getRuleDescriptions();
192206

193-
@Internal
194-
abstract Property<String> getConditionalOnClassAnnotation();
207+
@Input
208+
abstract MapProperty<String, String> getAnnotationClasses();
195209

196210
}

buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureRules.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,16 @@ static List<ArchRule> standard() {
101101
return List.copyOf(rules);
102102
}
103103

104-
static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
104+
static List<ArchRule> beanMethods(String annotationName) {
105+
return List.of(allBeanMethodsShouldReturnNonPrivateType(),
106+
allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(annotationName));
107+
}
108+
109+
static List<ArchRule> configurationProperties(String annotationName) {
110+
return List.of(allDeprecatedConfigurationPropertiesShouldIncludeSince(annotationName));
111+
}
112+
113+
private static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
105114
return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").should(check(
106115
"not return types declared with the %s modifier, as such types are incompatible with Spring AOT processing"
107116
.formatted(JavaModifier.PRIVATE),
@@ -115,7 +124,7 @@ static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
115124
.allowEmptyShould(true);
116125
}
117126

118-
static ArchRule allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(String annotationName) {
127+
private static ArchRule allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(String annotationName) {
119128
return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").should()
120129
.notBeAnnotatedWith(annotationName)
121130
.because("@ConditionalOnClass on @Bean methods is ineffective - it doesn't prevent "
@@ -348,6 +357,20 @@ private static ArchRule allConfigurationPropertiesBindingBeanMethodsShouldBeStat
348357
.allowEmptyShould(true);
349358
}
350359

360+
private static ArchRule allDeprecatedConfigurationPropertiesShouldIncludeSince(String annotationName) {
361+
return methodsThatAreAnnotatedWith(annotationName)
362+
.should(check("include a non-empty 'since' attribute", (method, events) -> {
363+
JavaAnnotation<JavaMethod> annotation = method.getAnnotationOfType(annotationName);
364+
Map<String, Object> properties = annotation.getProperties();
365+
Object since = properties.get("since");
366+
if (!(since instanceof String) || ((String) since).isEmpty()) {
367+
addViolation(events, method, annotation.getDescription()
368+
+ " should include a non-empty 'since' attribute of @DeprecatedConfigurationProperty");
369+
}
370+
}))
371+
.allowEmptyShould(true);
372+
}
373+
351374
private static boolean containsOnlySingleType(JavaType[] types, JavaType type) {
352375
return types.length == 1 && type.equals(types[0]);
353376
}

buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,17 @@
1919
import java.io.IOException;
2020
import java.nio.charset.StandardCharsets;
2121
import java.nio.file.Files;
22+
import java.nio.file.NoSuchFileException;
2223
import java.nio.file.Path;
2324
import java.nio.file.Paths;
2425
import java.util.Arrays;
26+
import java.util.HashMap;
2527
import java.util.LinkedHashMap;
2628
import java.util.LinkedHashSet;
2729
import java.util.Map;
2830
import java.util.Set;
2931
import java.util.function.UnaryOperator;
32+
import java.util.stream.Collectors;
3033

3134
import org.gradle.api.tasks.SourceSet;
3235
import org.gradle.testkit.runner.BuildResult;
@@ -41,6 +44,7 @@
4144
import org.junit.jupiter.params.provider.EnumSource;
4245

4346
import org.springframework.boot.build.architecture.annotations.TestConditionalOnClass;
47+
import org.springframework.boot.build.architecture.annotations.TestDeprecatedConfigurationProperty;
4448
import org.springframework.util.ClassUtils;
4549
import org.springframework.util.FileSystemUtils;
4650
import org.springframework.util.StringUtils;
@@ -316,6 +320,16 @@ void whenConditionalOnClassUsedOnBeanMethodsWithTestSourcesShouldSucceedAndWrite
316320
build(gradleBuild, Task.CHECK_ARCHITECTURE_TEST);
317321
}
318322

323+
@Test
324+
void whenDeprecatedConfigurationPropertyIsMissingSinceShouldFailAndWriteReport() throws IOException {
325+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties", "annotations");
326+
GradleBuild gradleBuild = this.gradleBuild.withDependencies(SPRING_CONTEXT)
327+
.withDeprecatedConfigurationPropertyAnnotation(TestDeprecatedConfigurationProperty.class.getName());
328+
buildAndFail(gradleBuild, Task.CHECK_ARCHITECTURE_MAIN,
329+
"should include a non-empty 'since' attribute of @DeprecatedConfigurationProperty",
330+
"DeprecatedConfigurationPropertySince.getProperty");
331+
}
332+
319333
private void prepareTask(Task task, String... sourceDirectories) throws IOException {
320334
for (String sourceDirectory : sourceDirectories) {
321335
FileSystemUtils.copyRecursively(
@@ -348,7 +362,12 @@ private void buildAndFail(GradleBuild gradleBuild, Task task, String... messages
348362
try {
349363
BuildResult buildResult = gradleBuild.buildAndFail(task.toString());
350364
assertThat(buildResult.taskPaths(TaskOutcome.FAILED)).as(buildResult.getOutput()).contains(":" + task);
351-
assertThat(task.getFailureReport(gradleBuild.getProjectDir())).contains(messages);
365+
try {
366+
assertThat(task.getFailureReport(gradleBuild.getProjectDir())).contains(messages);
367+
}
368+
catch (NoSuchFileException ex) {
369+
throw new AssertionError("Expected failure report not found\n" + buildResult.getOutput());
370+
}
352371
}
353372
catch (UnexpectedBuildSuccess ex) {
354373
throw new AssertionError("Expected build to fail but it succeeded\n" + ex.getBuildResult().getOutput(), ex);
@@ -410,9 +429,18 @@ GradleBuild withProhibitObjectsRequireNonNull(Boolean prohibitObjectsRequireNonN
410429
return this;
411430
}
412431

413-
GradleBuild withConditionalOnClassAnnotation(String annotationName) {
432+
GradleBuild withConditionalOnClassAnnotation(String annotationClass) {
433+
for (Task task : Task.values()) {
434+
configureTask(task, (configuration) -> configuration
435+
.withAnnotation(ArchitectureCheck.CONDITIONAL_ON_CLASS, annotationClass));
436+
}
437+
return this;
438+
}
439+
440+
GradleBuild withDeprecatedConfigurationPropertyAnnotation(String annotationClass) {
414441
for (Task task : Task.values()) {
415-
configureTask(task, (configuration) -> configuration.withConditionalOnClassAnnotation(annotationName));
442+
configureTask(task, (configuration) -> configuration
443+
.withAnnotation(ArchitectureCheck.DEPRECATED_CONFIGURATION_PROPERTY, annotationClass));
416444
}
417445
return this;
418446
}
@@ -454,18 +482,18 @@ private GradleRunner prepareRunner(String... arguments) throws IOException {
454482
for (String dependency : this.dependencies) {
455483
buildFile.append("\n implementation ").append(StringUtils.quote(dependency));
456484
}
457-
buildFile.append("}\n");
485+
buildFile.append("\n}\n\n");
458486
}
459487
this.taskConfigurations.forEach((task, configuration) -> {
460488
buildFile.append(task).append(" {");
461-
if (configuration.conditionalOnClassAnnotation() != null) {
462-
buildFile.append("\n conditionalOnClassAnnotation = ")
463-
.append(StringUtils.quote(configuration.conditionalOnClassAnnotation()));
464-
}
465489
if (configuration.prohibitObjectsRequireNonNull() != null) {
466490
buildFile.append("\n prohibitObjectsRequireNonNull = ")
467491
.append(configuration.prohibitObjectsRequireNonNull());
468492
}
493+
if (configuration.annotations() != null && !configuration.annotations().isEmpty()) {
494+
buildFile.append("\n annotationClasses = ")
495+
.append(toGroovyMapString(configuration.annotations()));
496+
}
469497
buildFile.append("\n}\n");
470498
});
471499
Files.writeString(this.projectDir.resolve("build.gradle"), buildFile, StandardCharsets.UTF_8);
@@ -475,15 +503,31 @@ private GradleRunner prepareRunner(String... arguments) throws IOException {
475503
.withPluginClasspath();
476504
}
477505

478-
private record TaskConfiguration(Boolean prohibitObjectsRequireNonNull, String conditionalOnClassAnnotation) {
506+
static String toGroovyMapString(Map<String, String> map) {
507+
return map.entrySet()
508+
.stream()
509+
.map((entry) -> "'" + entry.getKey() + "' : '" + entry.getValue() + "'")
510+
.collect(Collectors.joining(", ", "[", "]"));
511+
}
479512

480-
private TaskConfiguration withConditionalOnClassAnnotation(String annotationName) {
481-
return new TaskConfiguration(this.prohibitObjectsRequireNonNull, annotationName);
513+
private record TaskConfiguration(Boolean prohibitObjectsRequireNonNull, Map<String, String> annotations) {
514+
515+
public TaskConfiguration {
516+
if (annotations == null) {
517+
annotations = new HashMap<>();
518+
}
482519
}
483520

484521
private TaskConfiguration withProhibitObjectsRequireNonNull(Boolean prohibitObjectsRequireNonNull) {
485-
return new TaskConfiguration(prohibitObjectsRequireNonNull, this.conditionalOnClassAnnotation);
522+
return new TaskConfiguration(prohibitObjectsRequireNonNull, this.annotations);
486523
}
524+
525+
private TaskConfiguration withAnnotation(String name, String annotationClass) {
526+
Map<String, String> map = new HashMap<>(this.annotations);
527+
map.put(name, annotationClass);
528+
return new TaskConfiguration(this.prohibitObjectsRequireNonNull, map);
529+
}
530+
487531
}
488532

489533
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.build.architecture.annotations;
18+
19+
import java.lang.annotation.ElementType;
20+
import java.lang.annotation.Retention;
21+
import java.lang.annotation.RetentionPolicy;
22+
import java.lang.annotation.Target;
23+
24+
/**
25+
* {@code @DeprecatedConfigurationProperty} analogue for architecture checks.
26+
*/
27+
@Target(ElementType.METHOD)
28+
@Retention(RetentionPolicy.RUNTIME)
29+
public @interface TestDeprecatedConfigurationProperty {
30+
31+
/**
32+
* The reason for the deprecation.
33+
* @return the deprecation reason
34+
*/
35+
String reason() default "";
36+
37+
/**
38+
* The field that should be used instead (if any).
39+
* @return the replacement field
40+
*/
41+
String replacement() default "";
42+
43+
/**
44+
* The version in which the property became deprecated.
45+
* @return the version
46+
*/
47+
String since() default "";
48+
49+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.build.architecture.configurationproperties;
18+
19+
import org.springframework.boot.build.architecture.annotations.TestDeprecatedConfigurationProperty;
20+
21+
public class DeprecatedConfigurationPropertySince {
22+
23+
private String property;
24+
25+
@TestDeprecatedConfigurationProperty(reason = "no longer used")
26+
@Deprecated
27+
public String getProperty() {
28+
return this.property;
29+
}
30+
31+
public void setProperty(String property) {
32+
this.property = property;
33+
}
34+
35+
}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ public void setOutputQueryResults(Boolean outputQueryResults) {
794794
this.outputQueryResults = outputQueryResults;
795795
}
796796

797-
@DeprecatedConfigurationProperty(replacement = "spring.flyway.sqlserver.kerberos-login-file")
797+
@DeprecatedConfigurationProperty(replacement = "spring.flyway.sqlserver.kerberos-login-file", since = "3.2.0")
798798
@Deprecated(since = "3.2.0", forRemoval = true)
799799
public String getSqlServerKerberosLoginFile() {
800800
return getSqlserver().getKerberosLoginFile();

spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/appendix/configurationmetadata/format/property/MyProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public void setName(String name) {
3333
}
3434

3535
@Deprecated
36-
@DeprecatedConfigurationProperty(replacement = "my.app.name")
36+
@DeprecatedConfigurationProperty(replacement = "my.app.name", since = "1.2.0")
3737
public String getTarget() {
3838
return this.name;
3939
}

spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/appendix/configurationmetadata/format/property/MyProperties.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import org.springframework.boot.context.properties.DeprecatedConfigurationProper
2323
class MyProperties(val name: String?) {
2424

2525
var target: String? = null
26-
@Deprecated("") @DeprecatedConfigurationProperty(replacement = "my.app.name") get
26+
@Deprecated("") @DeprecatedConfigurationProperty(replacement = "my.app.name", since = "1.2.0") get
2727
@Deprecated("") set
2828

2929
}

0 commit comments

Comments
 (0)