mirror of
https://github.com/spring-projects/spring-boot.git
synced 2026-09-17 12:09:16 +00:00
Add ArchRule that checks that the type of exposed bean is not private
See gh-46753 Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
This commit is contained in:
committed by
Stéphane Nicoll
parent
a5d8eeccba
commit
b88ad99f1a
+13
@@ -43,6 +43,7 @@ import org.gradle.api.file.FileCollection;
|
||||
import org.gradle.api.file.FileTree;
|
||||
import org.gradle.api.provider.ListProperty;
|
||||
import org.gradle.api.provider.Property;
|
||||
import org.gradle.api.provider.Provider;
|
||||
import org.gradle.api.tasks.Classpath;
|
||||
import org.gradle.api.tasks.IgnoreEmptyDirectories;
|
||||
import org.gradle.api.tasks.Input;
|
||||
@@ -53,6 +54,7 @@ import org.gradle.api.tasks.OutputDirectory;
|
||||
import org.gradle.api.tasks.PathSensitive;
|
||||
import org.gradle.api.tasks.PathSensitivity;
|
||||
import org.gradle.api.tasks.SkipWhenEmpty;
|
||||
import org.gradle.api.tasks.SourceSet;
|
||||
import org.gradle.api.tasks.TaskAction;
|
||||
import org.gradle.api.tasks.VerificationException;
|
||||
|
||||
@@ -75,9 +77,17 @@ public abstract class ArchitectureCheck extends DefaultTask {
|
||||
getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
|
||||
.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
|
||||
getRules().addAll(ArchitectureRules.standard());
|
||||
getRules().addAll(whenMainSources(
|
||||
() -> Collections.singletonList(ArchitectureRules.allBeanMethodsShouldReturnNonPrivateType())));
|
||||
getRuleDescriptions().set(getRules().map(this::asDescriptions));
|
||||
}
|
||||
|
||||
private Provider<List<ArchRule>> whenMainSources(Supplier<List<ArchRule>> rules) {
|
||||
return getSourceSet().convention(SourceSet.MAIN_SOURCE_SET_NAME)
|
||||
.map(SourceSet.MAIN_SOURCE_SET_NAME::equals)
|
||||
.map(whenTrue(rules));
|
||||
}
|
||||
|
||||
private Transformer<List<ArchRule>, Boolean> whenTrue(Supplier<List<ArchRule>> rules) {
|
||||
return (in) -> (!in) ? Collections.emptyList() : rules.get();
|
||||
}
|
||||
@@ -170,6 +180,9 @@ public abstract class ArchitectureCheck extends DefaultTask {
|
||||
@Internal
|
||||
public abstract Property<Boolean> getProhibitObjectsRequireNonNull();
|
||||
|
||||
@Internal
|
||||
abstract Property<String> getSourceSet();
|
||||
|
||||
@Input // Use descriptions as input since rules aren't serializable
|
||||
abstract ListProperty<String> getRuleDescriptions();
|
||||
|
||||
|
||||
+1
@@ -49,6 +49,7 @@ public class ArchitecturePlugin implements Plugin<Project> {
|
||||
TaskProvider<ArchitectureCheck> checkPackageTangles = project.getTasks()
|
||||
.register("checkArchitecture" + StringUtils.capitalize(sourceSet.getName()), ArchitectureCheck.class,
|
||||
(task) -> {
|
||||
task.getSourceSet().set(sourceSet.getName());
|
||||
task.getCompileClasspath().from(sourceSet.getCompileClasspath());
|
||||
task.setClasses(sourceSet.getOutput().getClassesDirs());
|
||||
task.getResourcesDirectory().set(sourceSet.getOutput().getResourcesDir());
|
||||
|
||||
+15
@@ -33,6 +33,7 @@ import com.tngtech.archunit.core.domain.JavaCall;
|
||||
import com.tngtech.archunit.core.domain.JavaClass;
|
||||
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
|
||||
import com.tngtech.archunit.core.domain.JavaMethod;
|
||||
import com.tngtech.archunit.core.domain.JavaModifier;
|
||||
import com.tngtech.archunit.core.domain.JavaParameter;
|
||||
import com.tngtech.archunit.core.domain.JavaType;
|
||||
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
|
||||
@@ -93,6 +94,20 @@ final class ArchitectureRules {
|
||||
return List.copyOf(rules);
|
||||
}
|
||||
|
||||
static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
|
||||
return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").should(check(
|
||||
"not return types declared with the %s modifier, as such types are incompatible with Spring AOT processing"
|
||||
.formatted(JavaModifier.PRIVATE),
|
||||
(method, events) -> {
|
||||
JavaClass returnType = method.getRawReturnType();
|
||||
if (returnType.getModifiers().contains(JavaModifier.PRIVATE)) {
|
||||
addViolation(events, method, "%s returns %s which is declared as %s".formatted(
|
||||
method.getDescription(), returnType.getDescription(), returnType.getModifiers()));
|
||||
}
|
||||
}))
|
||||
.allowEmptyShould(true);
|
||||
}
|
||||
|
||||
private static ArchRule allPackagesShouldBeFreeOfTangles() {
|
||||
return SlicesRuleDefinition.slices().matching("(**)").should().beFreeOfCycles();
|
||||
}
|
||||
|
||||
+38
-7
@@ -16,7 +16,9 @@
|
||||
|
||||
package org.springframework.boot.build.architecture;
|
||||
|
||||
import java.io.FileNotFoundException;
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.function.Consumer;
|
||||
@@ -160,6 +162,21 @@ class ArchitectureCheckTests {
|
||||
runGradleWithCompiledClasses("string/toUpperCaseWithLocale", shouldHaveEmptyFailureReport());
|
||||
}
|
||||
|
||||
@Test
|
||||
void whenBeanMethodExposePrivateTypeShouldFailAndWriteReport() throws IOException {
|
||||
runGradleWithCompiledClasses("beans/privatebean", shouldHaveFailureReportWithMessage(
|
||||
"methods that are annotated with @Bean should not return types declared with the PRIVATE modifier,"
|
||||
+ " as such types are incompatible with Spring AOT processing",
|
||||
"Method <org.springframework.boot.build.architecture.beans.privatebean.PrivateBean.myBean()> "
|
||||
+ "returns Class <org.springframework.boot.build.architecture.beans.privatebean.PrivateBean$MyBean>"
|
||||
+ " which is declared as [PRIVATE, STATIC, FINAL]"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void whenBeanMethodExposeNonPrivateTypeeShouldNotFail() throws IOException {
|
||||
runGradleWithCompiledClasses("beans/regular", shouldHaveEmptyFailureReport());
|
||||
}
|
||||
|
||||
@Test
|
||||
void whenBeanPostProcessorBeanMethodIsNotStaticWithExternalClass() throws IOException {
|
||||
Files.writeString(this.buildFile, """
|
||||
@@ -196,17 +213,22 @@ class ArchitectureCheckTests {
|
||||
|
||||
private Consumer<GradleRunner> shouldHaveEmptyFailureReport() {
|
||||
return (gradleRunner) -> {
|
||||
assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
|
||||
.contains("Task :checkArchitectureMain");
|
||||
assertThat(failureReport()).isEmptyFile();
|
||||
try {
|
||||
assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
|
||||
.contains("Task :checkArchitectureMain");
|
||||
assertThat(failureReport()).isEmpty();
|
||||
}
|
||||
catch (Exception ex) {
|
||||
throw new AssertionError("Expected build to succeed but it failed\n" + failureReport(), ex);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
private Consumer<GradleRunner> shouldHaveFailureReportWithMessage(String message) {
|
||||
private Consumer<GradleRunner> shouldHaveFailureReportWithMessage(String... messages) {
|
||||
return (gradleRunner) -> {
|
||||
assertThat(gradleRunner.buildAndFail().getOutput()).contains("BUILD FAILED")
|
||||
.contains("Task :checkArchitectureMain FAILED");
|
||||
assertThat(failureReport()).content().contains(message);
|
||||
assertThat(failureReport()).contains(messages);
|
||||
};
|
||||
}
|
||||
|
||||
@@ -235,8 +257,17 @@ class ArchitectureCheckTests {
|
||||
.withPluginClasspath());
|
||||
}
|
||||
|
||||
private Path failureReport() {
|
||||
return this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
|
||||
private String failureReport() {
|
||||
try {
|
||||
Path failureReport = this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
|
||||
return Files.readString(failureReport, StandardCharsets.UTF_8);
|
||||
}
|
||||
catch (FileNotFoundException ex) {
|
||||
return "Failure report does not exist";
|
||||
}
|
||||
catch (IOException ex) {
|
||||
return "Failure report could not be read: " + ex.getMessage();
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+34
@@ -0,0 +1,34 @@
|
||||
/*
|
||||
* Copyright 2012-present the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.boot.build.architecture.beans.privatebean;
|
||||
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
class PrivateBean {
|
||||
|
||||
@Bean
|
||||
static MyBean myBean() {
|
||||
return new MyBean();
|
||||
}
|
||||
|
||||
private static final class MyBean {
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
+52
@@ -0,0 +1,52 @@
|
||||
/*
|
||||
* Copyright 2012-present the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.boot.build.architecture.beans.regular;
|
||||
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
class RegularBean {
|
||||
|
||||
@Bean
|
||||
static PackagePrivate packagePrivateBean() {
|
||||
return new PackagePrivate();
|
||||
}
|
||||
|
||||
@Bean
|
||||
static Protected protectedBean() {
|
||||
return new Protected();
|
||||
}
|
||||
|
||||
@Bean
|
||||
static Public publicBean() {
|
||||
return new Public();
|
||||
}
|
||||
|
||||
static final class PackagePrivate {
|
||||
|
||||
}
|
||||
|
||||
protected static final class Protected {
|
||||
|
||||
}
|
||||
|
||||
public static final class Public {
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user