From 3c080a00f3de5be7dceda779fc8c4eddeed7294c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= <csviri@gmail.com>
Date: Tue, 6 Feb 2024 12:18:30 +0100
Subject: [PATCH] fix: multiple dependents of same type exceptions (#2226)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Co-authored-by: Chris Laprun <claprun@redhat.com>
---
 .../dependent/workflow/WorkflowResult.java    | 20 +++++-
 .../workflow/WorkflowResultTest.java          | 65 +++++++++++++++++++
 2 files changed, 82 insertions(+), 3 deletions(-)
 create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java

diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java
index ba9e0f5755..77fc2dcbfe 100644
--- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java
+++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java
@@ -1,8 +1,8 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.stream.Collectors;
 
 import io.javaoperatorsdk.operator.AggregatedOperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
@@ -10,6 +10,7 @@
 @SuppressWarnings("rawtypes")
 class WorkflowResult {
 
+  private static final String NUMBER_DELIMITER = "_";
   private final Map<DependentResource, Exception> erroredDependents;
 
   WorkflowResult(Map<DependentResource, Exception> erroredDependents) {
@@ -36,9 +37,22 @@ public boolean erroredDependentsExist() {
 
   public void throwAggregateExceptionIfErrorsPresent() {
     if (erroredDependentsExist()) {
+      Map<String, Exception> exceptionMap = new HashMap<>();
+      Map<String, Integer> numberOfClasses = new HashMap<>();
+
+      for (Entry<DependentResource, Exception> entry : erroredDependents.entrySet()) {
+        String name = entry.getKey().getClass().getName();
+        var num = numberOfClasses.getOrDefault(name, 0);
+        if (num > 0) {
+          exceptionMap.put(name + NUMBER_DELIMITER + num, entry.getValue());
+        } else {
+          exceptionMap.put(name, entry.getValue());
+        }
+        numberOfClasses.put(name, num + 1);
+      }
+
       throw new AggregatedOperatorException("Exception(s) during workflow execution.",
-          erroredDependents.entrySet().stream()
-              .collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue)));
+          exceptionMap);
     }
   }
 }
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java
new file mode 100644
index 0000000000..48cf3fa75a
--- /dev/null
+++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java
@@ -0,0 +1,65 @@
+package io.javaoperatorsdk.operator.processing.dependent.workflow;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.AggregatedOperatorException;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class WorkflowResultTest {
+
+  @Test
+  void throwsExceptionWithoutNumberingIfAllDifferentClass() {
+    var res = new WorkflowResult(Map.of(new DependentA(), new RuntimeException(),
+        new DependentB(), new RuntimeException()));
+    try {
+      res.throwAggregateExceptionIfErrorsPresent();
+    } catch (AggregatedOperatorException e) {
+      assertThat(e.getAggregatedExceptions()).containsOnlyKeys(DependentA.class.getName(),
+          DependentB.class.getName());
+    }
+  }
+
+  @Test
+  void numbersDependentClassNamesIfMoreOfSameType() {
+    var res = new WorkflowResult(Map.of(new DependentA(), new RuntimeException(),
+        new DependentA(), new RuntimeException()));
+    try {
+      res.throwAggregateExceptionIfErrorsPresent();
+    } catch (AggregatedOperatorException e) {
+      assertThat(e.getAggregatedExceptions()).hasSize(2);
+    }
+  }
+
+  @SuppressWarnings("rawtypes")
+  static class DependentA implements DependentResource {
+    @Override
+    public ReconcileResult reconcile(HasMetadata primary, Context context) {
+      return null;
+    }
+
+    @Override
+    public Class resourceType() {
+      return null;
+    }
+  }
+
+  @SuppressWarnings("rawtypes")
+  static class DependentB implements DependentResource {
+    @Override
+    public ReconcileResult reconcile(HasMetadata primary, Context context) {
+      return null;
+    }
+
+    @Override
+    public Class resourceType() {
+      return null;
+    }
+  }
+}