Guarantee initial order for decorators to sort

Plus, add some test coverage with basic scenarios.

core/src/main/java/io/dekorate/ResourceRegistry.java

@@ -171,14 +171,12 @@ protected Map<String, KubernetesList> generate() {
 
  groupDecorators.forEach((group, decorators) -> {
  if (groups.containsKey(group)) {
+ log("Handling group '%s'", group);
  Set<Decorator> 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<String, KubernetesList> generate() {
 
  customDecorators.forEach((group, decorators) -> {
  if (customGroups.containsKey(group)) {
+ log("Handling group '%s'", group);
  Set<Decorator> 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<WithConfigReferences> getConfigReferences() {
  .map(WithConfigReferences.class::cast)
  .collect(Collectors.toList());
  }
+
+ private void log(String format, Object... args) {
+ if (isVerbose()) {
+ LOGGER.info(format, args);
+ }
+ }
 }

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<Decorator> sortDecorators(Collection<Decorator> items) {
  Collection<Node> 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<Node> 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<Node> {
  private final Class value;
 
  private List<Decorator> 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());
+ }
  }
 
  /**

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<Decorator> 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<Decorator> 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<? extends Decorator>[] before() {
+ return before;
+ }
+
+ @Override
+ public Class<? extends Decorator>[] after() {
+ return after;
+ }
+ }
+}