diff --git a/core/src/main/java/io/dekorate/ResourceRegistry.java b/core/src/main/java/io/dekorate/ResourceRegistry.java index 91c7d3550..196c8a8a2 100644 --- a/core/src/main/java/io/dekorate/ResourceRegistry.java +++ b/core/src/main/java/io/dekorate/ResourceRegistry.java @@ -171,14 +171,12 @@ protected Map generate() { groupDecorators.forEach((group, decorators) -> { if (groups.containsKey(group)) { + log("Handling group '%s'", group); Set union = new TreeSet<>(); union.addAll(decorators); union.addAll(globalDecorators); for (Decorator d : sortDecorators(union)) { - if (isVerbose()) { - LOGGER.info("Applying decorator '%s'", d.getClass().getName()); - } - + log("Applying decorator '%s'", d.getClass().getName()); groups.get(group).accept(d); } } @@ -191,14 +189,12 @@ protected Map generate() { customDecorators.forEach((group, decorators) -> { if (customGroups.containsKey(group)) { + log("Handling group '%s'", group); Set union = new TreeSet<>(); union.addAll(decorators); union.addAll(globalDecorators); for (Decorator d : sortDecorators(union)) { - if (isVerbose()) { - LOGGER.info("Applying decorator '%s'", d.getClass().getName()); - } - + log("Applying decorator '%s'", d.getClass().getName()); customGroups.get(group).accept(d); } } @@ -227,4 +223,10 @@ public List getConfigReferences() { .map(WithConfigReferences.class::cast) .collect(Collectors.toList()); } + + private void log(String format, Object... args) { + if (isVerbose()) { + LOGGER.info(format, args); + } + } } diff --git a/core/src/main/java/io/dekorate/utils/TopologicalSort.java b/core/src/main/java/io/dekorate/utils/TopologicalSort.java index f3ae86f8d..3047e9705 100644 --- a/core/src/main/java/io/dekorate/utils/TopologicalSort.java +++ b/core/src/main/java/io/dekorate/utils/TopologicalSort.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeSet; import io.dekorate.Logger; import io.dekorate.LoggerFactory; @@ -32,7 +33,8 @@ private TopologicalSort() { */ public static List sortDecorators(Collection items) { Collection nodes = adaptToNodes(items); - return sortNodes(nodes); + // Order the nodes by the decorator class to guarantee the initial order. + return sortNodes(new TreeSet<>(nodes)); } /** @@ -169,7 +171,7 @@ private static void throwCycleDetectedException(List cycle) { /** * This class is internally used to represent a node of decorators and its dependencies. */ - private static class Node { + private static class Node implements Comparable { private final Class value; private List references = new ArrayList<>(); @@ -183,6 +185,11 @@ public Node(Class value) { public String toString() { return value.getName(); } + + @Override + public int compareTo(Node o) { + return value.getName().compareTo(o.value.toString()); + } } /** diff --git a/core/src/test/java/io/dekorate/utils/TopologicalSortTest.java b/core/src/test/java/io/dekorate/utils/TopologicalSortTest.java new file mode 100644 index 000000000..535fa4d89 --- /dev/null +++ b/core/src/test/java/io/dekorate/utils/TopologicalSortTest.java @@ -0,0 +1,121 @@ +package io.dekorate.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.dekorate.kubernetes.decorator.Decorator; + +public class TopologicalSortTest { + + private List sortedDecorators; + + @BeforeEach + public void setup() { + sortedDecorators = null; + } + + @Test + public void shouldGroupDecoratorsWithSameTypeAndShouldPreserveOrder() { + FooDecorator a1 = new FooDecorator(); + DummyDecorator b = new DummyDecorator().withAfter(FooDecorator.class); + FooDecorator a2 = new FooDecorator(); + + whenSort(b, a1, a2); + thenDecoratorsAre(a1, a2, b); + } + + @Test + public void shouldThrowCycleException() { + FooDecorator foo = new FooDecorator(); + DummyDecorator bar = new BarDecorator().withAfter(FooDecorator.class); + DummyDecorator special = new SpecialFooDecorator().withAfter(FooDecorator.class, BarDecorator.class); + + // it should fail because: + // bar -> foo + // special (it's also a foo) -> bar + assertThrows(RuntimeException.class, () -> whenSort(foo, bar, special)); + } + + @Test + public void shouldAvoidCycleErrorWhenDecoratorHasSameParent() { + FooDecorator foo = new FooDecorator(); + DummyDecorator bar = new BarDecorator().withBefore(SpecialFooDecorator.class).withAfter(FooDecorator.class); + DummyDecorator special = new SpecialFooDecorator().withAfter(FooDecorator.class, BarDecorator.class); + + whenSort(foo, bar, special); + thenDecoratorsAre(foo, bar, special); + } + + private void whenSort(Decorator decorator, Decorator... decorators) { + List unsortedDecorators = new ArrayList<>(); + unsortedDecorators.add(decorator); + if (decorators != null) { + for (Decorator rest : decorators) { + unsortedDecorators.add(rest); + } + } + + sortedDecorators = TopologicalSort.sortDecorators(unsortedDecorators); + } + + private void thenDecoratorsAre(Decorator... expectedDecorators) { + for (int position = 0; position < expectedDecorators.length; position++) { + Decorator expected = expectedDecorators[position]; + assertEquals(sortedDecorators.get(position), expected, "Unexpected order in decorators: " + sortedDecorators); + } + } + + static class FooDecorator extends DummyDecorator { + + } + + static class SpecialFooDecorator extends FooDecorator { + + } + + static class BarDecorator extends DummyDecorator { + + } + + static class DummyDecorator extends Decorator { + + private Class[] before; + private Class[] after; + + public DummyDecorator withAfter(Class... classes) { + after = classes; + return this; + } + + public DummyDecorator withBefore(Class... classes) { + before = classes; + return this; + } + + @Override + public void visit(Object o) { + + } + + @Override + public int compareTo(Object o) { + return 0; + } + + @Override + public Class[] before() { + return before; + } + + @Override + public Class[] after() { + return after; + } + } +}