Skip to content

Commit 6bdf47b

Browse files
snicollsnicoll
committed
Merge pull request #48139 from scottfrederick
* pr/48139: Polish "Improve testing of architecture checks" Improve testing of architecture checks Closes gh-48139
2 parents 5a20176 + d9bf313 commit 6bdf47b

File tree

21 files changed

+795
-67
lines changed

21 files changed

+795
-67
lines changed

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

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.ArrayList;
2727
import java.util.Collections;
2828
import java.util.List;
29-
import java.util.Map;
3029
import java.util.concurrent.Callable;
3130
import java.util.function.Supplier;
3231
import java.util.stream.Stream;
@@ -72,27 +71,26 @@
7271
*/
7372
public abstract class ArchitectureCheck extends DefaultTask {
7473

75-
static final String CONDITIONAL_ON_CLASS = "ConditionalOnClass";
76-
77-
static final String DEPRECATED_CONFIGURATION_PROPERTY = "DeprecatedConfigurationProperty";
78-
79-
private static final String CONDITIONAL_ON_CLASS_ANNOTATION = "org.springframework.boot.autoconfigure.condition.ConditionalOnClass";
80-
81-
private static final String DEPRECATED_CONFIGURATION_PROPERTY_ANNOTATION = "org.springframework.boot.context.properties.DeprecatedConfigurationProperty";
82-
8374
private FileCollection classes;
8475

8576
public ArchitectureCheck() {
8677
getOutputDirectory().convention(getProject().getLayout().getBuildDirectory().dir(getName()));
87-
getAnnotationClasses().convention(Map.of(CONDITIONAL_ON_CLASS, CONDITIONAL_ON_CLASS_ANNOTATION,
88-
DEPRECATED_CONFIGURATION_PROPERTY, DEPRECATED_CONFIGURATION_PROPERTY_ANNOTATION));
78+
getAnnotationClasses().convention(ArchitectureCheckAnnotation.asMap());
8979
getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
9080
.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
9181
getRules().addAll(ArchitectureRules.standard());
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))));
82+
getRules().addAll(whenMainSources(() -> ArchitectureRules.beanMethods(ArchitectureCheckAnnotation
83+
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONDITIONAL_ON_CLASS))));
84+
getRules().addAll(whenMainSources(() -> ArchitectureRules.conditionalOnMissingBean(ArchitectureCheckAnnotation
85+
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONDITIONAL_ON_MISSING_BEAN))));
86+
getRules().addAll(whenMainSources(() -> ArchitectureRules.configurationProperties(ArchitectureCheckAnnotation
87+
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES))));
88+
getRules().addAll(whenMainSources(
89+
() -> ArchitectureRules.configurationPropertiesBinding(ArchitectureCheckAnnotation.classFor(
90+
getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES_BINDING))));
91+
getRules().addAll(whenMainSources(
92+
() -> ArchitectureRules.configurationPropertiesDeprecation(ArchitectureCheckAnnotation.classFor(
93+
getAnnotationClasses().get(), ArchitectureCheckAnnotation.DEPRECATED_CONFIGURATION_PROPERTY))));
9694
getRuleDescriptions().set(getRules().map(this::asDescriptions));
9795
}
9896

@@ -110,10 +108,6 @@ private List<String> asDescriptions(List<ArchRule> rules) {
110108
return rules.stream().map(ArchRule::getDescription).toList();
111109
}
112110

113-
private String annotationClassFor(String name, String defaultValue) {
114-
return getAnnotationClasses().get().getOrDefault(name, defaultValue);
115-
}
116-
117111
@TaskAction
118112
void checkArchitecture() throws Exception {
119113
withCompileClasspath(() -> {
@@ -204,7 +198,7 @@ final FileTree getInputClasses() {
204198
@Input // Use descriptions as input since rules aren't serializable
205199
abstract ListProperty<String> getRuleDescriptions();
206200

207-
@Input
201+
@Internal
208202
abstract MapProperty<String, String> getAnnotationClasses();
209203

210204
}

‎buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheckAnnotation.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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;
18+
19+
import java.util.Map;
20+
21+
/**
22+
* Annotations used in architecture checks, which can be overridden in architecture rule
23+
* tests.
24+
*
25+
* @author Scott Frederick
26+
* @author Stephane Nicoll
27+
*/
28+
public enum ArchitectureCheckAnnotation {
29+
30+
/**
31+
* Condition on class check.
32+
*/
33+
CONDITIONAL_ON_CLASS,
34+
35+
/**
36+
* Condition on missing bean check.
37+
*/
38+
CONDITIONAL_ON_MISSING_BEAN,
39+
40+
/**
41+
* Configuration properties bean.
42+
*/
43+
CONFIGURATION_PROPERTIES,
44+
45+
/**
46+
* Deprecated configuration property.
47+
*/
48+
DEPRECATED_CONFIGURATION_PROPERTY,
49+
50+
/**
51+
* Custom implementation to bind configuration properties.
52+
*/
53+
CONFIGURATION_PROPERTIES_BINDING;
54+
55+
private static final Map<String, String> annotationNameToClassName = Map.of(CONDITIONAL_ON_CLASS.name(),
56+
"org.springframework.boot.autoconfigure.condition.ConditionalOnClass", CONDITIONAL_ON_MISSING_BEAN.name(),
57+
"org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean",
58+
CONFIGURATION_PROPERTIES.name(), "org.springframework.boot.context.properties.ConfigurationProperties",
59+
DEPRECATED_CONFIGURATION_PROPERTY.name(),
60+
"org.springframework.boot.context.properties.DeprecatedConfigurationProperty",
61+
CONFIGURATION_PROPERTIES_BINDING.name(),
62+
"org.springframework.boot.context.properties.ConfigurationPropertiesBinding");
63+
64+
static Map<String, String> asMap() {
65+
return annotationNameToClassName;
66+
}
67+
68+
static String classFor(Map<String, String> annotationProperty, ArchitectureCheckAnnotation annotation) {
69+
String name = annotation.name();
70+
return annotationProperty.getOrDefault(name, asMap().get(name));
71+
}
72+
73+
}

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

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -92,22 +92,32 @@ static List<ArchRule> standard() {
9292
rules.add(noClassesShouldLoadResourcesUsingResourceUtils());
9393
rules.add(noClassesShouldCallStringToUpperCaseWithoutLocale());
9494
rules.add(noClassesShouldCallStringToLowerCaseWithoutLocale());
95-
rules.add(conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType());
9695
rules.add(enumSourceShouldNotHaveValueThatIsTheSameAsTypeOfMethodsFirstParameter());
97-
rules.add(classLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute());
98-
rules.add(methodLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute());
9996
rules.add(conditionsShouldNotBePublic());
100-
rules.add(allConfigurationPropertiesBindingBeanMethodsShouldBeStatic());
10197
return List.copyOf(rules);
10298
}
10399

104-
static List<ArchRule> beanMethods(String annotationName) {
100+
static List<ArchRule> beanMethods(String annotationClass) {
105101
return List.of(allBeanMethodsShouldReturnNonPrivateType(),
106-
allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(annotationName));
102+
allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(annotationClass));
107103
}
108104

109-
static List<ArchRule> configurationProperties(String annotationName) {
110-
return List.of(allDeprecatedConfigurationPropertiesShouldIncludeSince(annotationName));
105+
static List<ArchRule> conditionalOnMissingBean(String annotationClass) {
106+
return List
107+
.of(conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType(annotationClass));
108+
}
109+
110+
static List<ArchRule> configurationProperties(String annotationClass) {
111+
return List.of(classLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(annotationClass),
112+
methodLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(annotationClass));
113+
}
114+
115+
static List<ArchRule> configurationPropertiesBinding(String annotationClass) {
116+
return List.of(allConfigurationPropertiesBindingBeanMethodsShouldBeStatic(annotationClass));
117+
}
118+
119+
static List<ArchRule> configurationPropertiesDeprecation(String annotationClass) {
120+
return List.of(allDeprecatedConfigurationPropertiesShouldIncludeSince(annotationClass));
111121
}
112122

113123
private static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
@@ -247,16 +257,17 @@ private static ArchRule noClassesShouldCallStringToLowerCaseWithoutLocale() {
247257
.because(shouldUse("String.toLowerCase(Locale.ROOT)"));
248258
}
249259

250-
private static ArchRule conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType() {
251-
return methodsThatAreAnnotatedWith("org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean")
252-
.should(notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType())
260+
private static ArchRule conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType(
261+
String annotation) {
262+
return methodsThatAreAnnotatedWith(annotation)
263+
.should(notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType(annotation))
253264
.allowEmptyShould(true);
254265
}
255266

256-
private static ArchCondition<? super JavaMethod> notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType() {
267+
private static ArchCondition<? super JavaMethod> notSpecifyOnlyATypeThatIsTheSameAsTheMethodReturnType(
268+
String annotation) {
257269
return check("not specify only a type that is the same as the method's return type", (item, events) -> {
258-
JavaAnnotation<JavaMethod> conditionalAnnotation = item
259-
.getAnnotationOfType("org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean");
270+
JavaAnnotation<JavaMethod> conditionalAnnotation = item.getAnnotationOfType(annotation);
260271
Map<String, Object> properties = conditionalAnnotation.getProperties();
261272
if (!hasProperty("type", properties) && !hasProperty("name", properties)) {
262273
conditionalAnnotation.get("value").ifPresent((value) -> {
@@ -274,7 +285,7 @@ private static boolean hasProperty(String name, Map<String, Object> properties)
274285
if (property == null) {
275286
return false;
276287
}
277-
return !property.getClass().isArray() || ((Object[]) property).length > 0;
288+
return (property.getClass().isArray()) ? ((Object[]) property).length > 0 : !property.toString().isEmpty();
278289
}
279290

280291
private static ArchRule enumSourceShouldNotHaveValueThatIsTheSameAsTypeOfMethodsFirstParameter() {
@@ -303,33 +314,37 @@ private static void notSpecifyOnlyATypeThatIsTheSameAsTheMethodParameterType(Jav
303314
});
304315
}
305316

306-
private static ArchRule classLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute() {
317+
private static ArchRule classLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(
318+
String annotationClass) {
307319
return ArchRuleDefinition.classes()
308320
.that()
309-
.areAnnotatedWith("org.springframework.boot.context.properties.ConfigurationProperties")
310-
.should(notSpecifyOnlyPrefixAttributeOfConfigurationProperties())
321+
.areAnnotatedWith(annotationClass)
322+
.should(notSpecifyOnlyPrefixAttributeOfConfigurationProperties(annotationClass))
311323
.allowEmptyShould(true);
312324
}
313325

314-
private static ArchRule methodLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute() {
326+
private static ArchRule methodLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(
327+
String annotationClass) {
315328
return ArchRuleDefinition.methods()
316329
.that()
317-
.areAnnotatedWith("org.springframework.boot.context.properties.ConfigurationProperties")
318-
.should(notSpecifyOnlyPrefixAttributeOfConfigurationProperties())
330+
.areAnnotatedWith(annotationClass)
331+
.should(notSpecifyOnlyPrefixAttributeOfConfigurationProperties(annotationClass))
319332
.allowEmptyShould(true);
320333
}
321334

322-
private static ArchCondition<? super HasAnnotations<?>> notSpecifyOnlyPrefixAttributeOfConfigurationProperties() {
323-
return check("not specify only prefix attribute of @ConfigurationProperties",
324-
ArchitectureRules::notSpecifyOnlyPrefixAttributeOfConfigurationProperties);
335+
private static ArchCondition<? super HasAnnotations<?>> notSpecifyOnlyPrefixAttributeOfConfigurationProperties(
336+
String annotationClass) {
337+
return check("not specify only prefix attribute of @ConfigurationProperties", (item,
338+
events) -> notSpecifyOnlyPrefixAttributeOfConfigurationProperties(annotationClass, item, events));
325339
}
326340

327-
private static void notSpecifyOnlyPrefixAttributeOfConfigurationProperties(HasAnnotations<?> item,
328-
ConditionEvents events) {
329-
JavaAnnotation<?> configurationPropertiesAnnotation = item
330-
.getAnnotationOfType("org.springframework.boot.context.properties.ConfigurationProperties");
341+
private static void notSpecifyOnlyPrefixAttributeOfConfigurationProperties(String annotationClass,
342+
HasAnnotations<?> item, ConditionEvents events) {
343+
JavaAnnotation<?> configurationPropertiesAnnotation = item.getAnnotationOfType(annotationClass);
331344
Map<String, Object> properties = configurationPropertiesAnnotation.getProperties();
332-
if (properties.size() == 1 && properties.containsKey("prefix")) {
345+
if (hasProperty("prefix", properties) && !hasProperty("value", properties)
346+
&& properties.get("ignoreInvalidFields").equals(false)
347+
&& properties.get("ignoreUnknownFields").equals(true)) {
333348
addViolation(events, item, configurationPropertiesAnnotation.getDescription()
334349
+ " should specify implicit 'value' attribute other than explicit 'prefix' attribute");
335350
}
@@ -349,9 +364,9 @@ private static ArchRule conditionsShouldNotBePublic() {
349364
.allowEmptyShould(true);
350365
}
351366

352-
private static ArchRule allConfigurationPropertiesBindingBeanMethodsShouldBeStatic() {
367+
private static ArchRule allConfigurationPropertiesBindingBeanMethodsShouldBeStatic(String annotationClass) {
353368
return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").and()
354-
.areAnnotatedWith("org.springframework.boot.context.properties.ConfigurationPropertiesBinding")
369+
.areAnnotatedWith(annotationClass)
355370
.should()
356371
.beStatic()
357372
.allowEmptyShould(true);

0 commit comments

Comments
 (0)