Skip to content

Commit 8e65122

Browse files
scottfredericksnicoll
scottfrederick
authored and
snicoll
committed
Improve testing of architecture checks
See gh-48139 Signed-off-by: Scott Frederick <scottyfred@gmail.com>
1 parent 5a20176 commit 8e65122

File tree

21 files changed

+781
-67
lines changed

21 files changed

+781
-67
lines changed

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

Lines changed: 16 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;
@@ -60,6 +59,8 @@
6059
import org.gradle.api.tasks.TaskAction;
6160
import org.gradle.api.tasks.VerificationException;
6261

62+
import org.springframework.boot.build.architecture.ArchitectureCheckAnnotations.Annotation;
63+
6364
/**
6465
* {@link Task} that checks for architecture problems.
6566
*
@@ -72,27 +73,26 @@
7273
*/
7374
public abstract class ArchitectureCheck extends DefaultTask {
7475

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-
8376
private FileCollection classes;
8477

8578
public ArchitectureCheck() {
8679
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));
80+
getAnnotationClasses().convention(ArchitectureCheckAnnotations.asMap());
8981
getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
9082
.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
9183
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))));
84+
getRules().addAll(whenMainSources(() -> ArchitectureRules.beanMethods(
85+
ArchitectureCheckAnnotations.classFor(getAnnotationClasses().get(), Annotation.CONDITIONAL_ON_CLASS))));
86+
getRules().addAll(whenMainSources(() -> ArchitectureRules.conditionalOnMissingBean(ArchitectureCheckAnnotations
87+
.classFor(getAnnotationClasses().get(), Annotation.CONDITIONAL_ON_MISSING_BEAN))));
88+
getRules().addAll(whenMainSources(() -> ArchitectureRules.configurationProperties(ArchitectureCheckAnnotations
89+
.classFor(getAnnotationClasses().get(), Annotation.CONFIGURATION_PROPERTIES))));
90+
getRules()
91+
.addAll(whenMainSources(() -> ArchitectureRules.configurationPropertiesBinding(ArchitectureCheckAnnotations
92+
.classFor(getAnnotationClasses().get(), Annotation.CONFIGURATION_PROPERTIES_BINDING))));
93+
getRules().addAll(
94+
whenMainSources(() -> ArchitectureRules.configurationPropertiesDeprecation(ArchitectureCheckAnnotations
95+
.classFor(getAnnotationClasses().get(), Annotation.DEPRECATED_CONFIGURATION_PROPERTY))));
9696
getRuleDescriptions().set(getRules().map(this::asDescriptions));
9797
}
9898

@@ -110,10 +110,6 @@ private List<String> asDescriptions(List<ArchRule> rules) {
110110
return rules.stream().map(ArchRule::getDescription).toList();
111111
}
112112

113-
private String annotationClassFor(String name, String defaultValue) {
114-
return getAnnotationClasses().get().getOrDefault(name, defaultValue);
115-
}
116-
117113
@TaskAction
118114
void checkArchitecture() throws Exception {
119115
withCompileClasspath(() -> {
@@ -204,7 +200,7 @@ final FileTree getInputClasses() {
204200
@Input // Use descriptions as input since rules aren't serializable
205201
abstract ListProperty<String> getRuleDescriptions();
206202

207-
@Input
203+
@Internal
208204
abstract MapProperty<String, String> getAnnotationClasses();
209205

210206
}

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
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+
*/
27+
public final class ArchitectureCheckAnnotations {
28+
29+
enum Annotation {
30+
31+
CONDITIONAL_ON_CLASS, CONDITIONAL_ON_MISSING_BEAN, CONFIGURATION_PROPERTIES, DEPRECATED_CONFIGURATION_PROPERTY,
32+
CONFIGURATION_PROPERTIES_BINDING
33+
34+
}
35+
36+
private ArchitectureCheckAnnotations() {
37+
}
38+
39+
private static final Map<String, String> annotationNameToClassName = Map.of(Annotation.CONDITIONAL_ON_CLASS.name(),
40+
"org.springframework.boot.autoconfigure.condition.ConditionalOnClass",
41+
Annotation.CONDITIONAL_ON_MISSING_BEAN.name(),
42+
"org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean",
43+
Annotation.CONFIGURATION_PROPERTIES.name(),
44+
"org.springframework.boot.context.properties.ConfigurationProperties",
45+
Annotation.DEPRECATED_CONFIGURATION_PROPERTY.name(),
46+
"org.springframework.boot.context.properties.DeprecatedConfigurationProperty",
47+
Annotation.CONFIGURATION_PROPERTIES_BINDING.name(),
48+
"org.springframework.boot.context.properties.ConfigurationPropertiesBinding");
49+
50+
static Map<String, String> asMap() {
51+
return annotationNameToClassName;
52+
}
53+
54+
static String classFor(Map<String, String> annotationProperty, Annotation annotation) {
55+
String name = annotation.name();
56+
return annotationProperty.getOrDefault(name, asMap().get(name));
57+
}
58+
59+
}

‎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)