Skip to content

Commit c428779

Browse files
snicollsnicoll
committed
Polish contribution
This commit applies the same rules to ConfigurationPropertiesSource. See gh-50367
1 parent 2b11785 commit c428779

11 files changed

Lines changed: 316 additions & 28 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ public ArchitectureCheck() {
8787
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONDITIONAL_ON_MISSING_BEAN))));
8888
getRules().addAll(whenMainSources(() -> ArchitectureRules.configurationProperties(ArchitectureCheckAnnotation
8989
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES))));
90+
getRules().addAll(whenMainSources(() -> ArchitectureRules.configurationProperties(ArchitectureCheckAnnotation
91+
.classFor(getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES_SOURCE))));
9092
getRules().addAll(whenMainSources(
9193
() -> ArchitectureRules.configurationPropertiesBinding(ArchitectureCheckAnnotation.classFor(
9294
getAnnotationClasses().get(), ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES_BINDING))));

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ public enum ArchitectureCheckAnnotation {
4242
*/
4343
CONFIGURATION_PROPERTIES,
4444

45+
/**
46+
* Configuration properties source.
47+
*/
48+
CONFIGURATION_PROPERTIES_SOURCE,
49+
4550
/**
4651
* Deprecated configuration property.
4752
*/
@@ -56,6 +61,8 @@ public enum ArchitectureCheckAnnotation {
5661
"org.springframework.boot.autoconfigure.condition.ConditionalOnClass", CONDITIONAL_ON_MISSING_BEAN.name(),
5762
"org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean",
5863
CONFIGURATION_PROPERTIES.name(), "org.springframework.boot.context.properties.ConfigurationProperties",
64+
CONFIGURATION_PROPERTIES_SOURCE.name(),
65+
"org.springframework.boot.context.properties.ConfigurationPropertiesSource",
5966
DEPRECATED_CONFIGURATION_PROPERTY.name(),
6067
"org.springframework.boot.context.properties.DeprecatedConfigurationProperty",
6168
CONFIGURATION_PROPERTIES_BINDING.name(),

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

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ static List<ArchRule> conditionalOnMissingBean(String annotationClass) {
131131
static List<ArchRule> configurationProperties(String annotationClass) {
132132
return List.of(classLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(annotationClass),
133133
methodLevelConfigurationPropertiesShouldNotSpecifyOnlyPrefixAttribute(annotationClass),
134-
initializedConfigurationPropertiesShouldUse("java.util.Set", "java.util.LinkedHashSet",
135-
annotationClass),
136-
initializedConfigurationPropertiesShouldUse("java.util.Map",
137-
List.of("java.util.LinkedHashMap", "java.util.EnumMap"), annotationClass));
134+
configurationPropertyOfTypeShouldUseImplementation(annotationClass, "java.util.Set",
135+
List.of("java.util.LinkedHashSet")),
136+
configurationPropertyOfTypeShouldUseImplementation(annotationClass, "java.util.Map",
137+
List.of("java.util.LinkedHashMap", "java.util.EnumMap")));
138138
}
139139

140140
static List<ArchRule> configurationPropertiesBinding(String annotationClass) {
@@ -436,42 +436,36 @@ private static ArchRule allDeprecatedConfigurationPropertiesShouldIncludeSince(S
436436
.allowEmptyShould(true);
437437
}
438438

439-
private static ArchRule initializedConfigurationPropertiesShouldUse(String propertyType, String implementationType,
440-
String annotationClass) {
441-
return initializedConfigurationPropertiesShouldUse(propertyType, List.of(implementationType), annotationClass);
442-
}
443-
444-
private static ArchRule initializedConfigurationPropertiesShouldUse(String propertyType,
445-
List<String> implementationTypes, String annotationClass) {
439+
private static ArchRule configurationPropertyOfTypeShouldUseImplementation(String annotationClass,
440+
String propertyType, List<String> allowedImplementations) {
446441
return ArchRuleDefinition.classes()
447442
.that()
448443
.areAnnotatedWith(annotationClass)
449-
.or(areNestedInConfigurationPropertiesClasses(annotationClass))
450-
.should(useImplementationForInitializedProperties(propertyType, implementationTypes))
451-
.because("@ConfigurationProperties classes should preserve the property type's expected implementation")
444+
.or(areNestedInClassAnnotatedWith(annotationClass))
445+
.should(useAllowedImplementationForProperties(propertyType, allowedImplementations))
452446
.allowEmptyShould(true);
453447
}
454448

455-
private static ArchCondition<JavaClass> useImplementationForInitializedProperties(String propertyType,
456-
List<String> implementationTypes) {
457-
String description = implementationTypes.stream().collect(Collectors.joining(" or "));
458-
return check("use %s for initialized %s properties".formatted(description, propertyType),
449+
private static ArchCondition<JavaClass> useAllowedImplementationForProperties(String propertyType,
450+
List<String> allowedImplementations) {
451+
return check(
452+
"use %s for properties of type %s".formatted(String.join(" or ", allowedImplementations), propertyType),
459453
(javaClass, events) -> javaClass.getFields()
460454
.stream()
461455
.filter((field) -> propertyType.equals(field.getRawType().getName()))
462-
.forEach((field) -> checkPropertyInitializer(field, implementationTypes, events)));
456+
.forEach((field) -> checkPropertyImplementation(field, allowedImplementations, events)));
463457
}
464458

465-
private static void checkPropertyInitializer(JavaField field, List<String> implementationTypes,
459+
private static void checkPropertyImplementation(JavaField field, List<String> allowedImplementations,
466460
ConditionEvents events) {
467-
String description = implementationTypes.stream().collect(Collectors.joining(" or "));
468461
field.getAccessesToSelf()
469462
.stream()
470463
.filter((access) -> access.getAccessType() == AccessType.SET)
471464
.flatMap((access) -> initializerConstructorCalls(access).stream())
472-
.filter((call) -> !implementationTypes.contains(call.getTargetOwner().getName()))
473-
.forEach((call) -> addViolation(events, field, "%s should be initialized with %s instead of %s"
474-
.formatted(field.getDescription(), description, call.getTargetOwner().getName())));
465+
.filter((call) -> !allowedImplementations.contains(call.getTargetOwner().getName()))
466+
.forEach((call) -> addViolation(events, field,
467+
"%s should be initialized with %s instead of %s".formatted(field.getDescription(),
468+
String.join(" or ", allowedImplementations), call.getTargetOwner().getName())));
475469
}
476470

477471
private static List<JavaConstructorCall> initializerConstructorCalls(JavaFieldAccess fieldAccess) {
@@ -482,11 +476,11 @@ private static List<JavaConstructorCall> initializerConstructorCalls(JavaFieldAc
482476
.toList();
483477
}
484478

485-
private static DescribedPredicate<JavaClass> areNestedInConfigurationPropertiesClasses(String annotationClass) {
486-
return DescribedPredicate.describe("are nested in @ConfigurationProperties",
479+
private static DescribedPredicate<JavaClass> areNestedInClassAnnotatedWith(String annotationClass) {
480+
return DescribedPredicate.describe("one of its inner class",
487481
(javaClass) -> javaClass.getEnclosingClass()
488482
.map((enclosing) -> enclosing.isAnnotatedWith(annotationClass)
489-
|| areNestedInConfigurationPropertiesClasses(annotationClass).test(enclosing))
483+
|| areNestedInClassAnnotatedWith(annotationClass).test(enclosing))
490484
.orElse(false));
491485
}
492486

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.springframework.boot.build.architecture.annotations.TestConditionalOnMissingBean;
4848
import org.springframework.boot.build.architecture.annotations.TestConfigurationProperties;
4949
import org.springframework.boot.build.architecture.annotations.TestConfigurationPropertiesBinding;
50+
import org.springframework.boot.build.architecture.annotations.TestConfigurationPropertiesSource;
5051
import org.springframework.boot.build.architecture.annotations.TestDeprecatedConfigurationProperty;
5152
import org.springframework.util.ClassUtils;
5253
import org.springframework.util.FileSystemUtils;
@@ -339,34 +340,69 @@ void whenConfigurationPropertiesUsesHashMapShouldFailAndWriteReport() throws IOE
339340
Task.CHECK_ARCHITECTURE_MAIN, "should be initialized with java.util.LinkedHashMap");
340341
}
341342

343+
@Test
344+
void whenConfigurationPropertiesSourceUsesHashMapShouldFailAndWriteReport() throws IOException {
345+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationpropertiessource/hashmap", "annotations");
346+
buildAndFail(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesSourceAnnotation(),
347+
Task.CHECK_ARCHITECTURE_MAIN, "should be initialized with java.util.LinkedHashMap");
348+
}
349+
342350
@Test
343351
void whenConfigurationPropertiesUsesHashSetShouldFailAndWriteReport() throws IOException {
344352
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties/hashset", "annotations");
345353
buildAndFail(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesAnnotation(),
346354
Task.CHECK_ARCHITECTURE_MAIN, "should be initialized with java.util.LinkedHashSet");
347355
}
348356

357+
@Test
358+
void whenConfigurationPropertiesSourceUsesHashSetShouldFailAndWriteReport() throws IOException {
359+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationpropertiessource/hashset", "annotations");
360+
buildAndFail(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesSourceAnnotation(),
361+
Task.CHECK_ARCHITECTURE_MAIN, "should be initialized with java.util.LinkedHashSet");
362+
}
363+
349364
@Test
350365
void whenConfigurationPropertiesUsesLinkedHashMapShouldSucceedAndWriteEmptyReport() throws IOException {
351366
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties/linkedhashmap", "annotations");
352367
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesAnnotation(),
353368
Task.CHECK_ARCHITECTURE_MAIN);
354369
}
355370

371+
@Test
372+
void whenConfigurationPropertiesSourceUsesLinkedHashMapShouldSucceedAndWriteEmptyReport() throws IOException {
373+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationpropertiessource/linkedhashmap", "annotations");
374+
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesSourceAnnotation(),
375+
Task.CHECK_ARCHITECTURE_MAIN);
376+
}
377+
356378
@Test
357379
void whenConfigurationPropertiesUsesEnumMapShouldSucceedAndWriteEmptyReport() throws IOException {
358380
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties/enummap", "annotations");
359381
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesAnnotation(),
360382
Task.CHECK_ARCHITECTURE_MAIN);
361383
}
362384

385+
@Test
386+
void whenConfigurationPropertiesSourceUsesEnumMapShouldSucceedAndWriteEmptyReport() throws IOException {
387+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationpropertiessource/enummap", "annotations");
388+
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesSourceAnnotation(),
389+
Task.CHECK_ARCHITECTURE_MAIN);
390+
}
391+
363392
@Test
364393
void whenConfigurationPropertiesUsesLinkedHashSetShouldSucceedAndWriteEmptyReport() throws IOException {
365394
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties/linkedhashset", "annotations");
366395
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesAnnotation(),
367396
Task.CHECK_ARCHITECTURE_MAIN);
368397
}
369398

399+
@Test
400+
void whenConfigurationPropertiesSourceUsesLinkedHashSetShouldSucceedAndWriteEmptyReport() throws IOException {
401+
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationpropertiessource/linkedhashset", "annotations");
402+
build(this.gradleBuild.withDependencies(SPRING_CONTEXT).withConfigurationPropertiesSourceAnnotation(),
403+
Task.CHECK_ARCHITECTURE_MAIN);
404+
}
405+
370406
@Test
371407
void whenConfigurationPropertiesBindingBeanMethodIsNotStaticShouldFailAndWriteReport() throws IOException {
372408
prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "configurationproperties/bindingnonstatic", "annotations");
@@ -604,6 +640,12 @@ GradleBuild withConfigurationPropertiesAnnotation() {
604640
return this;
605641
}
606642

643+
GradleBuild withConfigurationPropertiesSourceAnnotation() {
644+
configureTasks(ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES_SOURCE.name(),
645+
TestConfigurationPropertiesSource.class.getName());
646+
return this;
647+
}
648+
607649
GradleBuild withConfigurationPropertiesBindingAnnotation() {
608650
configureTasks(ArchitectureCheckAnnotation.CONFIGURATION_PROPERTIES_BINDING.name(),
609651
TestConfigurationPropertiesBinding.class.getName());

‎buildSrc/src/test/java/org/springframework/boot/build/architecture/annotations/TestConfigurationPropertiesSource.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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+
@Target({ ElementType.TYPE, ElementType.METHOD })
25+
@Retention(RetentionPolicy.RUNTIME)
26+
public @interface TestConfigurationPropertiesSource {
27+
28+
}

‎buildSrc/src/test/java/org/springframework/boot/build/architecture/configurationpropertiessource/enummap/ConfigurationPropertiesWithEnumMap.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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.configurationpropertiessource.enummap;
18+
19+
import java.util.EnumMap;
20+
import java.util.Map;
21+
22+
import org.springframework.boot.build.architecture.annotations.TestConfigurationPropertiesSource;
23+
24+
/**
25+
* Test {@link TestConfigurationPropertiesSource} using {@link EnumMap}.
26+
*
27+
* @author Stephane Nicoll
28+
*/
29+
@TestConfigurationPropertiesSource
30+
public class ConfigurationPropertiesWithEnumMap {
31+
32+
private Map<Example, String> properties = new EnumMap<>(Example.class);
33+
34+
public Map<Example, String> getProperties() {
35+
return this.properties;
36+
}
37+
38+
public void setProperties(Map<Example, String> properties) {
39+
this.properties = properties;
40+
}
41+
42+
enum Example {
43+
44+
ONE
45+
46+
}
47+
48+
}

‎buildSrc/src/test/java/org/springframework/boot/build/architecture/configurationpropertiessource/hashmap/ConfigurationPropertiesWithHashMap.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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.configurationpropertiessource.hashmap;
18+
19+
import java.util.HashMap;
20+
import java.util.Map;
21+
22+
import org.springframework.boot.build.architecture.annotations.TestConfigurationPropertiesSource;
23+
24+
/**
25+
* Test {@link TestConfigurationPropertiesSource} using {@link HashMap}.
26+
*
27+
* @author Stephane Nicoll
28+
*/
29+
@TestConfigurationPropertiesSource
30+
public class ConfigurationPropertiesWithHashMap {
31+
32+
private Map<String, String> properties = new HashMap<>();
33+
34+
public Map<String, String> getProperties() {
35+
return this.properties;
36+
}
37+
38+
public void setProperties(Map<String, String> properties) {
39+
this.properties = properties;
40+
}
41+
42+
}

‎buildSrc/src/test/java/org/springframework/boot/build/architecture/configurationpropertiessource/hashset/ConfigurationPropertiesWithHashSet.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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.configurationpropertiessource.hashset;
18+
19+
import java.util.HashSet;
20+
import java.util.Set;
21+
22+
import org.springframework.boot.build.architecture.annotations.TestConfigurationPropertiesSource;
23+
24+
/**
25+
* Test {@link TestConfigurationPropertiesSource} using {@link HashSet}.
26+
*
27+
* @author Stephane Nicoll
28+
*/
29+
@TestConfigurationPropertiesSource
30+
public class ConfigurationPropertiesWithHashSet {
31+
32+
private Set<String> items = new HashSet<>();
33+
34+
public Set<String> getItems() {
35+
return this.items;
36+
}
37+
38+
public void setItems(Set<String> items) {
39+
this.items = items;
40+
}
41+
42+
}

0 commit comments

Comments
 (0)