Skip to content

Commit 6b9bc01

Browse files
wilkinsonawilkinsona
committed
Check that BPP and BFPP bean methods won't cause eager initialization
Closes gh-35164
1 parent 472afaf commit 6b9bc01

File tree

20 files changed

+682
-209
lines changed

20 files changed

+682
-209
lines changed

‎buildSrc/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ dependencies {
3838
testImplementation("org.assertj:assertj-core:3.11.1")
3939
testImplementation("org.apache.logging.log4j:log4j-core:2.17.1")
4040
testImplementation("org.junit.jupiter:junit-jupiter:5.6.0")
41+
implementation("org.springframework:spring-context")
4142
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
4243
}
4344

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

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
/*
2+
* Copyright 2022-2023 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.io.File;
20+
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Files;
23+
import java.nio.file.StandardOpenOption;
24+
import java.util.List;
25+
import java.util.stream.Collectors;
26+
import java.util.stream.Stream;
27+
28+
import com.tngtech.archunit.base.DescribedPredicate;
29+
import com.tngtech.archunit.core.domain.JavaClass;
30+
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
31+
import com.tngtech.archunit.core.domain.JavaClasses;
32+
import com.tngtech.archunit.core.domain.JavaMethod;
33+
import com.tngtech.archunit.core.domain.JavaParameter;
34+
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
35+
import com.tngtech.archunit.core.importer.ClassFileImporter;
36+
import com.tngtech.archunit.lang.ArchCondition;
37+
import com.tngtech.archunit.lang.ArchRule;
38+
import com.tngtech.archunit.lang.ConditionEvents;
39+
import com.tngtech.archunit.lang.EvaluationResult;
40+
import com.tngtech.archunit.lang.SimpleConditionEvent;
41+
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
42+
import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition;
43+
import org.gradle.api.DefaultTask;
44+
import org.gradle.api.GradleException;
45+
import org.gradle.api.Task;
46+
import org.gradle.api.file.DirectoryProperty;
47+
import org.gradle.api.file.FileCollection;
48+
import org.gradle.api.file.FileTree;
49+
import org.gradle.api.tasks.IgnoreEmptyDirectories;
50+
import org.gradle.api.tasks.InputFiles;
51+
import org.gradle.api.tasks.Internal;
52+
import org.gradle.api.tasks.OutputDirectory;
53+
import org.gradle.api.tasks.PathSensitive;
54+
import org.gradle.api.tasks.PathSensitivity;
55+
import org.gradle.api.tasks.SkipWhenEmpty;
56+
import org.gradle.api.tasks.TaskAction;
57+
58+
/**
59+
* {@link Task} that checks for architecture problems.
60+
*
61+
* @author Andy Wilkinson
62+
*/
63+
public abstract class ArchitectureCheck extends DefaultTask {
64+
65+
private FileCollection classes;
66+
67+
public ArchitectureCheck() {
68+
getOutputDirectory().convention(getProject().getLayout().getBuildDirectory().dir(getName()));
69+
}
70+
71+
@TaskAction
72+
void checkArchitecture() throws IOException {
73+
JavaClasses javaClasses = new ClassFileImporter()
74+
.importPaths(this.classes.getFiles().stream().map(File::toPath).collect(Collectors.toList()));
75+
List<EvaluationResult> violations = Stream.of(allPackagesShouldBeFreeOfTangles(),
76+
allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization(),
77+
allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters())
78+
.map((rule) -> rule.evaluate(javaClasses))
79+
.filter(EvaluationResult::hasViolation)
80+
.collect(Collectors.toList());
81+
File outputFile = getOutputDirectory().file("failure-report.txt").get().getAsFile();
82+
outputFile.getParentFile().mkdirs();
83+
if (!violations.isEmpty()) {
84+
StringBuilder report = new StringBuilder();
85+
for (EvaluationResult violation : violations) {
86+
report.append(violation.getFailureReport().toString());
87+
report.append(String.format("%n"));
88+
}
89+
Files.write(outputFile.toPath(), report.toString().getBytes(StandardCharsets.UTF_8),
90+
StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING);
91+
throw new GradleException("Architecture check failed. See '" + outputFile + "' for details.");
92+
}
93+
else {
94+
outputFile.createNewFile();
95+
}
96+
}
97+
98+
private ArchRule allPackagesShouldBeFreeOfTangles() {
99+
return SlicesRuleDefinition.slices().matching("(**)").should().beFreeOfCycles();
100+
}
101+
102+
private ArchRule allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization() {
103+
return ArchRuleDefinition.methods()
104+
.that()
105+
.areAnnotatedWith("org.springframework.context.annotation.Bean")
106+
.and()
107+
.haveRawReturnType(Predicates.assignableTo("org.springframework.beans.factory.config.BeanPostProcessor"))
108+
.should(onlyHaveParametersThatWillNotCauseEagerInitialization())
109+
.andShould()
110+
.beStatic()
111+
.allowEmptyShould(true);
112+
}
113+
114+
private ArchCondition<JavaMethod> onlyHaveParametersThatWillNotCauseEagerInitialization() {
115+
DescribedPredicate<CanBeAnnotated> notAnnotatedWithLazy = DescribedPredicate
116+
.not(CanBeAnnotated.Predicates.annotatedWith("org.springframework.context.annotation.Lazy"));
117+
DescribedPredicate<JavaClass> notOfASafeType = DescribedPredicate
118+
.not(Predicates.assignableTo("org.springframework.beans.factory.ObjectProvider")
119+
.or(Predicates.assignableTo("org.springframework.context.ApplicationContext"))
120+
.or(Predicates.assignableTo("org.springframework.core.env.Environment")));
121+
return new ArchCondition<JavaMethod>("not have parameters that will cause eager initialization") {
122+
123+
@Override
124+
public void check(JavaMethod item, ConditionEvents events) {
125+
item.getParameters()
126+
.stream()
127+
.filter(notAnnotatedWithLazy)
128+
.filter((parameter) -> notOfASafeType.test(parameter.getRawType()))
129+
.forEach((parameter) -> events.add(SimpleConditionEvent.violated(parameter,
130+
parameter.getDescription() + " will cause eager initialization as it is "
131+
+ notAnnotatedWithLazy.getDescription() + " and is "
132+
+ notOfASafeType.getDescription())));
133+
}
134+
135+
};
136+
}
137+
138+
private ArchRule allBeanFactoryPostProcessorBeanMethodsShouldBeStaticAndHaveNoParameters() {
139+
return ArchRuleDefinition.methods()
140+
.that()
141+
.areAnnotatedWith("org.springframework.context.annotation.Bean")
142+
.and()
143+
.haveRawReturnType(
144+
Predicates.assignableTo("org.springframework.beans.factory.config.BeanFactoryPostProcessor"))
145+
.should(haveNoParameters())
146+
.andShould()
147+
.beStatic()
148+
.allowEmptyShould(true);
149+
}
150+
151+
private ArchCondition<JavaMethod> haveNoParameters() {
152+
return new ArchCondition<JavaMethod>("have no parameters") {
153+
154+
@Override
155+
public void check(JavaMethod item, ConditionEvents events) {
156+
List<JavaParameter> parameters = item.getParameters();
157+
if (!parameters.isEmpty()) {
158+
events
159+
.add(SimpleConditionEvent.violated(item, item.getDescription() + " should have no parameters"));
160+
}
161+
}
162+
163+
};
164+
}
165+
166+
public void setClasses(FileCollection classes) {
167+
this.classes = classes;
168+
}
169+
170+
@Internal
171+
public FileCollection getClasses() {
172+
return this.classes;
173+
}
174+
175+
@InputFiles
176+
@SkipWhenEmpty
177+
@IgnoreEmptyDirectories
178+
@PathSensitive(PathSensitivity.RELATIVE)
179+
final FileTree getInputClasses() {
180+
return this.classes.getAsFileTree();
181+
}
182+
183+
@OutputDirectory
184+
public abstract DirectoryProperty getOutputDirectory();
185+
186+
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ public void apply(Project project) {
4444

4545
private void registerTasks(Project project) {
4646
JavaPluginExtension javaPluginExtension = project.getExtensions().getByType(JavaPluginExtension.class);
47-
List<TaskProvider<PackageTangleCheck>> packageTangleChecks = new ArrayList<>();
47+
List<TaskProvider<ArchitectureCheck>> packageTangleChecks = new ArrayList<>();
4848
for (SourceSet sourceSet : javaPluginExtension.getSourceSets()) {
49-
TaskProvider<PackageTangleCheck> checkPackageTangles = project.getTasks()
50-
.register("checkForPackageTangles" + StringUtils.capitalize(sourceSet.getName()),
51-
PackageTangleCheck.class, (task) -> {
49+
TaskProvider<ArchitectureCheck> checkPackageTangles = project.getTasks()
50+
.register("checkArchitecture" + StringUtils.capitalize(sourceSet.getName()), ArchitectureCheck.class,
51+
(task) -> {
5252
task.setClasses(sourceSet.getOutput().getClassesDirs());
53-
task.setDescription("Checks the classes of the " + sourceSet.getName()
54-
+ " source set for package tangles.");
53+
task.setDescription("Checks the architecture of the classes of the " + sourceSet.getName()
54+
+ " source set.");
5555
task.setGroup(LifecycleBasePlugin.VERIFICATION_GROUP);
5656
});
5757
packageTangleChecks.add(checkPackageTangles);

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

Lines changed: 0 additions & 102 deletions
This file was deleted.

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright 2023 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.stream.Stream;
20+
21+
import com.tngtech.archunit.base.DescribedPredicate;
22+
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
23+
import com.tngtech.archunit.lang.ArchRule;
24+
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
25+
import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition;
26+
27+
/**
28+
* Architecture rules evaluated by {@link ArchitectureCheck}.
29+
*
30+
* @author Andy Wilkinson
31+
*/
32+
final class Rules {
33+
34+
private Rules() {
35+
36+
}
37+
38+
static Stream<ArchRule> stream() {
39+
return Stream.of(allPackagesShouldBeFreeOfTangles(),
40+
allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization());
41+
}
42+
43+
static ArchRule allPackagesShouldBeFreeOfTangles() {
44+
return SlicesRuleDefinition.slices().matching("(**)").should().beFreeOfCycles();
45+
}
46+
47+
static ArchRule allBeanPostProcessorBeanMethodsShouldBeStaticAndHaveParametersThatWillNotCausePrematureInitialization() {
48+
return ArchRuleDefinition.methods()
49+
.that()
50+
.areAnnotatedWith("org.springframework.context.annotation.Bean")
51+
.and()
52+
.haveRawReturnType(Predicates.assignableTo("org.springframework.beans.factory.config.BeanPostProcessor"))
53+
.should()
54+
.beStatic()
55+
.andShould()
56+
.haveRawParameterTypes(DescribedPredicate
57+
.allElements(Predicates.assignableTo("org.springframework.beans.factory.ObjectProvider")
58+
.or(Predicates.assignableTo("org.springframework.context.ApplicationContext"))))
59+
.allowEmptyShould(true);
60+
}
61+
62+
}

0 commit comments

Comments
 (0)