Skip to content

Commit 546aeb1

Browse files
Throw CycleException when cycles are detected
This changes the DependencyGraph's topological sort to throw a dedicated exception class so that it can be better handled.
1 parent 2529ebd commit 546aeb1

File tree

9 files changed

+33
-9
lines changed

9 files changed

+33
-9
lines changed

smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import software.amazon.smithy.model.node.ObjectNode;
3434
import software.amazon.smithy.model.transform.ModelTransformer;
3535
import software.amazon.smithy.model.validation.ValidatedResult;
36+
import software.amazon.smithy.utils.CycleException;
3637
import software.amazon.smithy.utils.DependencyGraph;
3738
import software.amazon.smithy.utils.Pair;
3839
import software.amazon.smithy.utils.SmithyBuilder;
@@ -242,7 +243,7 @@ private List<ResolvedPlugin> resolvePlugins(String projectionName, ProjectionCon
242243
List<PluginId> sorted;
243244
try {
244245
sorted = dependencyGraph.toSortedList();
245-
} catch (IllegalStateException e) {
246+
} catch (CycleException e) {
246247
throw new SmithyBuildException(e.getMessage(), e);
247248
}
248249

smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ public void detectsPluginCycles() throws Exception {
624624
.build());
625625

626626
SmithyBuildException e = assertThrows(SmithyBuildException.class, builder::build);
627-
assertThat(e.getMessage(), containsString("Cycle(s) detected in dependency graph"));
627+
assertThat(e.getMessage(), containsString("Cycle(s) detected"));
628628
}
629629

630630
@Test

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin1.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ public void execute(PluginContext context) {
2020
} catch (InterruptedException ignored) {}
2121
}
2222

23+
// This is made serial to protect against test failures if we decide later to
24+
// have plugins run in parallel. These plugins MUST run serially with respect
25+
// to each other to function.
2326
@Override
2427
public boolean isSerial() {
2528
return true;

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin2.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public List<String> runBefore() {
2727
return ListUtils.of("timestampPlugin1");
2828
}
2929

30+
// This is made serial to protect against test failures if we decide later to
31+
// have plugins run in parallel. These plugins MUST run serially with respect
32+
// to each other to function.
3033
@Override
3134
public boolean isSerial() {
3235
return true;

smithy-build/src/test/java/software/amazon/smithy/build/TimestampPlugin3.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public List<String> runAfter() {
2727
return ListUtils.of("timestampPlugin1");
2828
}
2929

30+
// This is made serial to protect against test failures if we decide later to
31+
// have plugins run in parallel. These plugins MUST run serially with respect
32+
// to each other to function.
3033
@Override
3134
public boolean isSerial() {
3235
return true;

smithy-codegen-core/src/main/java/software/amazon/smithy/codegen/core/IntegrationTopologicalSort.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import java.util.List;
1111
import java.util.Map;
1212
import java.util.logging.Logger;
13+
import software.amazon.smithy.utils.CycleException;
1314
import software.amazon.smithy.utils.DependencyGraph;
1415

1516
final class IntegrationTopologicalSort<I extends SmithyIntegration<?, ?, ?>> {
@@ -46,7 +47,7 @@ List<I> sort() {
4647
List<String> sorted;
4748
try {
4849
sorted = dependencyGraph.toSortedList(this::compareIntegrations);
49-
} catch (IllegalStateException e) {
50+
} catch (CycleException e) {
5051
throw new IllegalArgumentException(e);
5152
}
5253
for (String name : sorted) {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
package software.amazon.smithy.utils;
6+
7+
import java.util.Collection;
8+
import java.util.stream.Collectors;
9+
10+
public class CycleException extends RuntimeException {
11+
public CycleException(Collection<?> cyclicNodes) {
12+
super(String.format("Cycle(s) detected amongst: [%s]",
13+
cyclicNodes.stream().map(Object::toString).collect(Collectors.joining(", "))));
14+
}
15+
}

smithy-utils/src/main/java/software/amazon/smithy/utils/DependencyGraph.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,15 +349,13 @@ private List<T> toSortedList(Queue<T> satisfied) {
349349

350350
// Check for cycles.
351351
if (result.size() != reverseDependencies.size()) {
352-
List<String> remaining = new ArrayList<>(reverseDependencies.size() - result.size());
352+
List<T> remaining = new ArrayList<>(reverseDependencies.size() - result.size());
353353
for (T node : reverseDependencies.keySet()) {
354354
if (!result.contains(node)) {
355-
remaining.add(node.toString());
355+
remaining.add(node);
356356
}
357357
}
358-
throw new IllegalStateException(
359-
String.format("Cycle(s) detected in dependency graph while attempting to sort among [%s]",
360-
String.join(", ", remaining)));
358+
throw new CycleException(remaining);
361359
}
362360

363361
return result;

smithy-utils/src/test/java/software/amazon/smithy/utils/DependencyGraphTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ public void sortedListThrowsErrorOnCycle() {
233233
graph.addDependency("spam", "eggs");
234234
graph.addDependency("eggs", "spam");
235235

236-
assertThrows(IllegalStateException.class, graph::toSortedList);
236+
assertThrows(CycleException.class, graph::toSortedList);
237237
}
238238

239239
@Test

0 commit comments

Comments
 (0)