Skip to content

Commit cfd1393

Browse files
committed
Add ArchRule that checks that the type of exposed bean is not private
Signed-off-by: Dmytro Nosan <[email protected]>
1 parent f2d0712 commit cfd1393

File tree

16 files changed

+148
-25
lines changed

16 files changed

+148
-25
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@
3333
import com.tngtech.archunit.core.domain.JavaClass;
3434
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
3535
import com.tngtech.archunit.core.domain.JavaMethod;
36+
import com.tngtech.archunit.core.domain.JavaModifier;
3637
import com.tngtech.archunit.core.domain.JavaParameter;
3738
import com.tngtech.archunit.core.domain.JavaType;
3839
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
40+
import com.tngtech.archunit.core.domain.properties.HasModifiers;
3941
import com.tngtech.archunit.core.domain.properties.HasName;
4042
import com.tngtech.archunit.core.domain.properties.HasOwner;
4143
import com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With;
@@ -90,6 +92,7 @@ static List<ArchRule> standard() {
9092
rules.add(conditionalOnMissingBeanShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodReturnType());
9193
rules.add(enumSourceShouldNotSpecifyOnlyATypeThatIsTheSameAsMethodParameterType());
9294
rules.add(allConfigurationPropertiesBindingBeanMethodsShouldBeStatic());
95+
rules.add(allBeanMethodsShouldReturnNonPrivateType());
9396
return List.copyOf(rules);
9497
}
9598

@@ -106,6 +109,13 @@ private static ArchRule allBeanPostProcessorBeanMethodsShouldBeStaticAndNotCause
106109
.allowEmptyShould(true);
107110
}
108111

112+
private static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
113+
return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").should()
114+
.haveRawReturnType(DescribedPredicate.not(HasModifiers.Predicates.modifier(JavaModifier.PRIVATE)))
115+
.as("@Bean methods must not return types declared with the private modifier, as such types are incompatible with Spring AOT processing")
116+
.allowEmptyShould(true);
117+
}
118+
109119
private static ArchCondition<JavaMethod> onlyHaveParametersThatWillNotCauseEagerInitialization() {
110120
return check("not have parameters that will cause eager initialization",
111121
ArchitectureRules::allBeanPostProcessorBeanMethodsShouldBeStaticAndNotCausePrematureInitialization);

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package org.springframework.boot.build.architecture;
1818

19+
import java.io.FileNotFoundException;
1920
import java.io.IOException;
21+
import java.nio.charset.StandardCharsets;
2022
import java.nio.file.Files;
2123
import java.nio.file.Path;
2224
import java.util.function.Consumer;
@@ -160,6 +162,17 @@ void whenClassCallsStringToUpperCaseWithLocaleShouldNotFail() throws IOException
160162
runGradleWithCompiledClasses("string/toUpperCaseWithLocale", shouldHaveEmptyFailureReport());
161163
}
162164

165+
@Test
166+
void whenBeanMethodExposePrivateTypeeShouldNotFailAndWriteReport() throws IOException {
167+
runGradleWithCompiledClasses("beans/privatebean", shouldHaveFailureReportWithMessage(
168+
"@Bean methods must not return types declared with the private modifier, as such types are incompatible with Spring AOT processing"));
169+
}
170+
171+
@Test
172+
void whenBeanMethodExposeNonPrivateTypeeShouldNotFail() throws IOException {
173+
runGradleWithCompiledClasses("beans/regular", shouldHaveEmptyFailureReport());
174+
}
175+
163176
@Test
164177
void whenBeanPostProcessorBeanMethodIsNotStaticWithExternalClass() throws IOException {
165178
Files.writeString(this.buildFile, """
@@ -196,17 +209,22 @@ IntegrationMBeanExporter integrationMBeanExporter() {
196209

197210
private Consumer<GradleRunner> shouldHaveEmptyFailureReport() {
198211
return (gradleRunner) -> {
199-
assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
200-
.contains("Task :checkArchitectureMain");
201-
assertThat(failureReport()).isEmptyFile();
212+
try {
213+
assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
214+
.contains("Task :checkArchitectureMain");
215+
assertThat(failureReport()).isEmpty();
216+
}
217+
catch (Exception ex) {
218+
throw new AssertionError("Expected build to succeed but it failed\n" + failureReport(), ex);
219+
}
202220
};
203221
}
204222

205223
private Consumer<GradleRunner> shouldHaveFailureReportWithMessage(String message) {
206224
return (gradleRunner) -> {
207225
assertThat(gradleRunner.buildAndFail().getOutput()).contains("BUILD FAILED")
208226
.contains("Task :checkArchitectureMain FAILED");
209-
assertThat(failureReport()).content().contains(message);
227+
assertThat(failureReport()).contains(message);
210228
};
211229
}
212230

@@ -235,8 +253,17 @@ private void runGradle(Consumer<GradleRunner> callback) {
235253
.withPluginClasspath());
236254
}
237255

238-
private Path failureReport() {
239-
return this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
256+
private String failureReport() {
257+
try {
258+
Path failureReport = this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
259+
return Files.readString(failureReport, StandardCharsets.UTF_8);
260+
}
261+
catch (FileNotFoundException ex) {
262+
return "Failure report does not exist";
263+
}
264+
catch (IOException ex) {
265+
return "Failure report could not be read: " + ex.getMessage();
266+
}
240267
}
241268

242269
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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.beans.privatebean;
18+
19+
import org.springframework.context.annotation.Bean;
20+
import org.springframework.context.annotation.Configuration;
21+
22+
@Configuration(proxyBeanMethods = false)
23+
class PrivateBean {
24+
25+
@Bean
26+
static MyBean myBean() {
27+
return new MyBean();
28+
}
29+
30+
private static final class MyBean {
31+
32+
}
33+
34+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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.beans.regular;
18+
19+
import org.springframework.context.annotation.Bean;
20+
import org.springframework.context.annotation.Configuration;
21+
22+
@Configuration(proxyBeanMethods = false)
23+
class RegularBean {
24+
25+
@Bean
26+
static PackagePrivate packagePrivateBean() {
27+
return new PackagePrivate();
28+
}
29+
30+
@Bean
31+
static Protected protectedBean() {
32+
return new Protected();
33+
}
34+
35+
@Bean
36+
static Public publicBean() {
37+
return new Public();
38+
}
39+
40+
static final class PackagePrivate {
41+
42+
}
43+
44+
protected static final class Protected {
45+
46+
}
47+
48+
public static final class Public {
49+
50+
}
51+
52+
}

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/data/RepositoryMetricsAutoConfigurationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ TestRepositoryTagsProvider tagsProvider() {
161161

162162
}
163163

164-
private static final class TestRepositoryTagsProvider implements RepositoryTagsProvider {
164+
static final class TestRepositoryTagsProvider implements RepositoryTagsProvider {
165165

166166
@Override
167167
public Iterable<Tag> repositoryTags(RepositoryMethodInvocation invocation) {

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationAutoConfigurationTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ MeterObservationHandler<Context> customMeterObservationHandler1(CalledHandlers c
566566

567567
}
568568

569-
private static class CustomTracingObservationHandler implements TracingObservationHandler<Context> {
569+
static class CustomTracingObservationHandler implements TracingObservationHandler<Context> {
570570

571571
private final Tracer tracer = mock(Tracer.class, Answers.RETURNS_MOCKS);
572572

@@ -600,7 +600,7 @@ public boolean supportsContext(Context context) {
600600

601601
}
602602

603-
private static class ObservationHandlerWithCustomContext implements ObservationHandler<CustomContext> {
603+
static class ObservationHandlerWithCustomContext implements ObservationHandler<CustomContext> {
604604

605605
private final CalledHandlers calledHandlers;
606606

@@ -624,7 +624,7 @@ private static final class CustomContext extends Context {
624624

625625
}
626626

627-
private static final class CalledHandlers {
627+
static final class CalledHandlers {
628628

629629
private final List<ObservationHandler<?>> calledHandlers = new ArrayList<>();
630630

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/observation/ObservationRegistryConfigurerIntegrationTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ Customizer2 customizer2(CalledCustomizers calledCustomizers) {
7373

7474
}
7575

76-
private static final class CalledCustomizers {
76+
static final class CalledCustomizers {
7777

7878
private final List<ObservationRegistryCustomizer<?>> customizers = new ArrayList<>();
7979

@@ -87,7 +87,7 @@ List<ObservationRegistryCustomizer<?>> getCustomizers() {
8787

8888
}
8989

90-
private static class Customizer1 implements ObservationRegistryCustomizer<ObservationRegistry> {
90+
static class Customizer1 implements ObservationRegistryCustomizer<ObservationRegistry> {
9191

9292
private final CalledCustomizers calledCustomizers;
9393

@@ -102,7 +102,7 @@ public void customize(ObservationRegistry registry) {
102102

103103
}
104104

105-
private static class Customizer2 implements ObservationRegistryCustomizer<ObservationRegistry> {
105+
static class Customizer2 implements ObservationRegistryCustomizer<ObservationRegistry> {
106106

107107
private final CalledCustomizers calledCustomizers;
108108

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/quartz/QuartzEndpointAutoConfigurationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ CustomEndpoint customEndpoint() {
124124

125125
}
126126

127-
private static final class CustomEndpoint extends QuartzEndpoint {
127+
static final class CustomEndpoint extends QuartzEndpoint {
128128

129129
private CustomEndpoint() {
130130
super(mock(Scheduler.class), Collections.emptyList());

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/scheduling/ScheduledTasksEndpointAutoConfigurationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ CustomEndpoint customEndpoint() {
7272

7373
}
7474

75-
private static final class CustomEndpoint extends ScheduledTasksEndpoint {
75+
static final class CustomEndpoint extends ScheduledTasksEndpoint {
7676

7777
private CustomEndpoint() {
7878
super(Collections.emptyList());

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/OpenTelemetryTracingAutoConfigurationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ InMemoryRecordingSpanExporter spanExporter() {
530530

531531
}
532532

533-
private static final class InMemoryRecordingSpanExporter implements SpanExporter {
533+
static final class InMemoryRecordingSpanExporter implements SpanExporter {
534534

535535
private final List<SpanData> exportedSpans = new ArrayList<>();
536536

0 commit comments

Comments
 (0)