Guarantee initial order for decorators to sort

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.getName());
+    }
   }
 
   /**

core/src/test/java/io/dekorate/utils/TopologicalSortTest.java

@@ -0,0 +1,129 @@
+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.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import io.dekorate.LoggerFactory;
+import io.dekorate.kubernetes.decorator.Decorator;
+import io.dekorate.logger.NoopLogger;
+
+public class TopologicalSortTest {
+
+  private List<Decorator> sortedDecorators;
+
+  @BeforeAll
+  public static void initLogger() {
+    LoggerFactory.setLogger(new NoopLogger());
+  }
+
+  @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;
+    }
+  }
+}