Skip to content

Commit c767d6f

Browse files
committed
GH-1299 - Prevent stack overflow during dependency lookup for modules involved in a cycle.
During dependency lookups, we now avoid recursing into modules which we've already seen.
1 parent 3072d60 commit c767d6f

File tree

9 files changed

+208
-22
lines changed

9 files changed

+208
-22
lines changed

spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,7 @@
2424
import static org.springframework.modulith.core.Types.SpringDataTypes.*;
2525
import static org.springframework.modulith.core.Types.SpringTypes.*;
2626

27-
import java.util.Arrays;
28-
import java.util.Collection;
29-
import java.util.Collections;
30-
import java.util.Iterator;
31-
import java.util.List;
32-
import java.util.Map;
33-
import java.util.Objects;
34-
import java.util.Optional;
27+
import java.util.*;
3528
import java.util.function.Predicate;
3629
import java.util.function.Supplier;
3730
import java.util.stream.Collectors;
@@ -599,12 +592,11 @@ public ApplicationModuleDependencies getDependencies(ApplicationModules modules,
599592
Assert.notNull(depth, "DependencyDepth must not be null!");
600593
Assert.notNull(types, "DependencyTypes must not be null!");
601594

602-
return getAllModuleDependencies(modules) //
603-
.filter(it -> types.length == 0 ? true : Arrays.stream(types).anyMatch(it::hasType)) //
604-
.distinct() //
605-
.flatMap(it -> DefaultApplicationModuleDependency.of(it, modules)) //
606-
.distinct() //
607-
.flatMap(it -> resolveRecurseively(modules, it, depth, types)) //
595+
if (depth == DependencyDepth.NONE) {
596+
return ApplicationModuleDependencies.NONE;
597+
}
598+
599+
return getDependencies(modules, new DependencyTraversal(depth, types))
608600
.collect(Collectors.collectingAndThen(Collectors.toList(), ApplicationModuleDependencies::of));
609601
}
610602

@@ -846,14 +838,26 @@ private Classes findAggregateRoots(Classes source) {
846838
.collect(Classes.toClasses());
847839
}
848840

849-
private Stream<ApplicationModuleDependency> resolveRecurseively(ApplicationModules modules,
850-
DefaultApplicationModuleDependency dependency, DependencyDepth depth, DependencyType... type) {
841+
private Stream<ApplicationModuleDependency> getDependencies(ApplicationModules modules,
842+
DependencyTraversal traversal) {
843+
844+
return getAllModuleDependencies(modules) //
845+
.filter(traversal::include) //
846+
.distinct() //
847+
.flatMap(it -> DefaultApplicationModuleDependency.of(it, modules)) //
848+
.distinct() //
849+
.flatMap(it -> resolveRecursively(modules, it, traversal)); //
850+
}
851+
852+
private static Stream<ApplicationModuleDependency> resolveRecursively(ApplicationModules modules,
853+
ApplicationModuleDependency dependency, DependencyTraversal seen) {
854+
855+
if (!seen.register(dependency.getTargetModule())) {
856+
return Stream.of(dependency);
857+
}
851858

852-
var tail = depth == DependencyDepth.ALL
853-
? dependency.getTargetModule()
854-
.getDependencies(modules, depth, type)
855-
.stream()
856-
.distinct()
859+
var tail = seen.hasDepth(DependencyDepth.ALL)
860+
? dependency.getTargetModule().getDependencies(modules, seen).distinct()
857861
: Stream.<ApplicationModuleDependency> empty();
858862

859863
return Stream.concat(Stream.of(dependency), tail);
@@ -1329,6 +1333,10 @@ boolean hasType(DependencyType type) {
13291333
return this.type.equals(type);
13301334
}
13311335

1336+
boolean hasAnyType(Collection<DependencyType> candidates) {
1337+
return candidates.contains(type);
1338+
}
1339+
13321340
Violations isValidDependencyWithin(ApplicationModules modules) {
13331341

13341342
var originModule = getExistingModuleOf(source, modules);
@@ -1617,6 +1625,43 @@ private static String getDescriptionFor(JavaMember member) {
16171625
}
16181626
}
16191627

1628+
/**
1629+
* A helper type to keep track of a traversal configuration and seen {@link ApplicationModule}s.
1630+
*
1631+
* @author Oliver Drotbohm
1632+
*/
1633+
private static class DependencyTraversal {
1634+
1635+
private final Set<ApplicationModule> seen = new HashSet<>();
1636+
private final DependencyDepth depth;
1637+
private final Set<DependencyType> types;
1638+
1639+
DependencyTraversal(DependencyDepth depth, DependencyType... types) {
1640+
1641+
this.depth = depth;
1642+
this.types = Set.of(types);
1643+
}
1644+
1645+
boolean include(QualifiedDependency dependency) {
1646+
return types.isEmpty() || dependency.hasAnyType(types);
1647+
}
1648+
1649+
boolean hasDepth(DependencyDepth depth) {
1650+
return this.depth == depth;
1651+
}
1652+
1653+
boolean register(ApplicationModule module) {
1654+
1655+
if (seen.contains(module)) {
1656+
return false;
1657+
}
1658+
1659+
seen.add(module);
1660+
1661+
return true;
1662+
}
1663+
}
1664+
16201665
private static class DefaultApplicationModuleDependency
16211666
implements ApplicationModuleDependency {
16221667

spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModuleDependencies.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.modulith.core;
1717

1818
import java.util.Collection;
19+
import java.util.Collections;
1920
import java.util.HashSet;
2021
import java.util.List;
2122
import java.util.function.Function;
@@ -31,6 +32,8 @@
3132
*/
3233
public class ApplicationModuleDependencies {
3334

35+
static final ApplicationModuleDependencies NONE = new ApplicationModuleDependencies(Collections.emptyList());
36+
3437
private final List<ApplicationModuleDependency> dependencies;
3538
private final Collection<ApplicationModule> modules;
3639

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2025 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+
package example.cycle;
17+
18+
/**
19+
* @author Oliver Drotbohm
20+
*/
21+
public class Cycle {
22+
23+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2025 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+
package example.cycle.a;
17+
18+
import example.cycle.b.CycleB;
19+
20+
/**
21+
* @author Oliver Drotbohm
22+
*/
23+
public class CycleA {
24+
private CycleB cycleB;
25+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2025 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+
package example.cycle.b;
17+
18+
import example.cycle.c.CycleC;
19+
20+
/**
21+
* @author Oliver Drotbohm
22+
*/
23+
public class CycleB {
24+
private CycleC cycleC;
25+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2025 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+
package example.cycle.c;
17+
18+
import example.cycle.d.CycleD;
19+
20+
/**
21+
* @author Oliver Drotbohm
22+
*/
23+
public class CycleC {
24+
CycleD cycleD;
25+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright 2025 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+
package example.cycle.d;
17+
18+
import example.cycle.a.CycleA;
19+
import example.cycle.b.CycleB;
20+
21+
/**
22+
* @author Oliver Drotbohm
23+
*/
24+
public class CycleD {
25+
CycleB cycleB;
26+
CycleA cycleA;
27+
}

spring-modulith-core/src/test/java/org/springframework/modulith/core/ApplicationModulesUnitTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*/
2727
class ApplicationModulesUnitTests {
2828

29-
ApplicationModules modules = TestUtils.of("example", "example.ninvalid");
29+
ApplicationModules modules = TestUtils.of("example", "example.ninvalid", "example.cycle..");
3030

3131
@Test // GH 578
3232
void discoversComplexModuleArrangement() {

spring-modulith-core/src/test/java/org/springframework/modulith/core/ModuleUnitTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,17 @@ void wildcardedDeclaredDependencyAllowsDependenciesToAllNamedInterfaces() {
105105
assertThat(dependency.contains(SpiType.class)).isTrue();
106106
assertThat(dependency.contains(ApiType.class)).isTrue();
107107
}
108+
109+
@Test // GH-1299
110+
void obtainsDependenciesForCyclicArrangement() {
111+
112+
var modules = TestUtils.of("example.cycle");
113+
114+
modules.forEach(it -> {
115+
116+
assertThat(it.getDependencies(modules, DependencyDepth.ALL).uniqueModules())
117+
.extracting(ApplicationModule::getIdentifier)
118+
.contains(it.getIdentifier());
119+
});
120+
}
108121
}

0 commit comments

Comments
 (0)