fix: explicitly mark as visited

Signed-off-by: Chris Laprun <[email protected]>

Author: metacosm

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java

@@ -72,11 +72,8 @@ protected boolean noMoreExecutionsScheduled() {
   }
 
   protected boolean alreadyVisited(DependentResourceNode<?, P> dependentResourceNode) {
-    return results.containsKey(dependentResourceNode);
-  }
-
-  protected void markAsVisited(DependentResourceNode<?, P> dependentResourceNode) {
-    createOrGetResultFor(dependentResourceNode);
+    final WorkflowResult.DetailBuilder<?> builder = results.get(dependentResourceNode);
+    return builder != null && builder.isVisited();
   }
 
   protected boolean postDeleteConditionNotMet(DependentResourceNode<?, P> dependentResourceNode) {
@@ -162,7 +159,7 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
     }
   }
 
-  protected Map<DependentResource, WorkflowResult.Detail> asDetails() {
+  protected Map<DependentResource, WorkflowResult.Detail<?>> asDetails() {
     return results.entrySet().stream()
         .collect(
             Collectors.toMap(e -> e.getKey().getDependentResource(), e -> e.getValue().build()));

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ResultCondition.java

@@ -7,8 +7,6 @@
 public interface ResultCondition<R, P extends HasMetadata, T> extends Condition<R, P> {
   Result<T> detailedIsMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
 
-  Object NULL = new Object();
-
   @Override
   default boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context) {
     return detailedIsMet(dependentResource, primary, context).isSuccess();
@@ -18,31 +16,45 @@ interface Result<T> {
     Result metWithoutResult = new Result() {
       @Override
       public Object getResult() {
-        return NULL;
+        return null;
       }
 
       @Override
       public boolean isSuccess() {
         return true;
       }
+
+      @Override
+      public String toString() {
+        return asString();
+      }
     };
 
     Result unmetWithoutResult = new Result() {
       @Override
       public Object getResult() {
-        return NULL;
+        return null;
       }
 
       @Override
       public boolean isSuccess() {
         return false;
       }
+
+      @Override
+      public String toString() {
+        return asString();
+      }
     };
 
     static Result withoutResult(boolean success) {
       return success ? metWithoutResult : unmetWithoutResult;
     }
 
+    default String asString() {
+      return "Result: " + getResult() + " met: " + isSuccess();
+    }
+
     T getResult();
 
     boolean isSuccess();

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java

@@ -74,7 +74,7 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
       }
 
       if (deletePostConditionMet) {
-        markAsVisited(dependentResourceNode);
+        createOrGetResultFor(dependentResourceNode).markAsVisited();
         handleDependentCleaned(dependentResourceNode);
       }
     }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupResult.java

@@ -7,7 +7,7 @@
 
 @SuppressWarnings("rawtypes")
 public class WorkflowCleanupResult extends WorkflowResult {
-  WorkflowCleanupResult(Map<DependentResource, Detail> results) {
+  WorkflowCleanupResult(Map<DependentResource, Detail<?>> results) {
     super(results);
   }
 
@@ -16,7 +16,7 @@ public List<DependentResource> getDeleteCalledOnDependents() {
   }
 
   public List<DependentResource> getPostConditionNotMetDependents() {
-    return listFilteredBy(detail -> !Detail.isConditionMet(detail.deletePostconditionResult()));
+    return listFilteredBy(detail -> !detail.isConditionWithTypeMet(Condition.Type.DELETE));
   }
 
   public boolean allPostConditionsMet() {

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java

@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -140,12 +141,13 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
       log.debug(
           "Reconciling for primary: {} node: {} ", primaryID, dependentResourceNode);
       ReconcileResult reconcileResult = dependentResource.reconcile(primary, context);
-      createOrGetResultFor(dependentResourceNode).withReconcileResult(reconcileResult);
+      final var detailBuilder = createOrGetResultFor(dependentResourceNode);
+      detailBuilder.withReconcileResult(reconcileResult);
 
       if (isConditionMet(dependentResourceNode.getReconcilePrecondition(), dependentResourceNode)) {
         log.debug("Setting already reconciled for: {} primaryID: {}",
             dependentResourceNode, primaryID);
-        markAsVisited(dependentResourceNode);
+        detailBuilder.markAsVisited();
         handleDependentsReconcile(dependentResourceNode);
       } else {
         log.debug("Setting already reconciled but not ready for: {}", dependentResourceNode);
@@ -162,8 +164,6 @@ private NodeDeleteExecutor(DependentResourceNode<R, P> dependentResourceNode) {
     @Override
     @SuppressWarnings("unchecked")
     protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
-      var deletePostCondition = dependentResourceNode.getDeletePostcondition();
-
       final var dependentResource = dependentResourceNode.getDependentResource();
       boolean deletePostConditionMet = true;
       if (isConditionMet(dependentResourceNode.getActivationCondition(), dependentResourceNode)) {
@@ -173,11 +173,12 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
         if (dependentResource instanceof Deleter) {
           ((Deleter<P>) dependentResource).delete(primary, context);
         }
-        deletePostConditionMet = isConditionMet(deletePostCondition, dependentResourceNode);
+        deletePostConditionMet =
+            isConditionMet(dependentResourceNode.getDeletePostcondition(), dependentResourceNode);
       }
 
       if (deletePostConditionMet) {
-        markAsVisited(dependentResourceNode);
+        createOrGetResultFor(dependentResourceNode).markAsVisited();
         handleDependentDeleted(dependentResourceNode);
       }
     }
@@ -224,7 +225,7 @@ private void markDependentsForDelete(DependentResourceNode<?, P> dependentResour
       }
     } else {
       // this is for an edge case when there is only one resource but that is not active
-      markAsVisited(dependentResourceNode);
+      createOrGetResultFor(dependentResourceNode).markAsVisited();
       if (dependents.isEmpty()) {
         handleNodeExecutionFinish(dependentResourceNode);
       } else {

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileResult.java

@@ -9,7 +9,7 @@
 @SuppressWarnings("rawtypes")
 public class WorkflowReconcileResult extends WorkflowResult {
 
-  public WorkflowReconcileResult(Map<DependentResource, Detail> results) {
+  public WorkflowReconcileResult(Map<DependentResource, Detail<?>> results) {
     super(results);
   }
 
@@ -18,7 +18,7 @@ public List<DependentResource> getReconciledDependents() {
   }
 
   public List<DependentResource> getNotReadyDependents() {
-    return listFilteredBy(detail -> !Detail.isConditionMet(detail.reconcilePostconditionResult()));
+    return listFilteredBy(detail -> !detail.isConditionWithTypeMet(Condition.Type.READY));
   }
 
   public <T> T getNotReadyDependentResult(DependentResource dependentResource,

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResult.java

@@ -3,6 +3,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -13,10 +14,10 @@
 
 @SuppressWarnings("rawtypes")
 class WorkflowResult {
-  private final Map<DependentResource, Detail> results;
+  private final Map<DependentResource, Detail<?>> results;
   private Boolean hasErroredDependents;
 
-  WorkflowResult(Map<DependentResource, Detail> results) {
+  WorkflowResult(Map<DependentResource, Detail<?>> results) {
     this.results = results;
   }
 
@@ -25,11 +26,11 @@ public Map<DependentResource, Exception> getErroredDependents() {
         .collect(Collectors.toMap(Entry::getKey, entry -> entry.getValue().error));
   }
 
-  private Stream<Entry<DependentResource, Detail>> getErroredDependentsStream() {
+  private Stream<Entry<DependentResource, Detail<?>>> getErroredDependentsStream() {
     return results.entrySet().stream().filter(entry -> entry.getValue().error != null);
   }
 
-  protected Map<DependentResource, Detail> results() {
+  protected Map<DependentResource, Detail<?>> results() {
     return results;
   }
 
@@ -64,10 +65,11 @@ static class DetailBuilder<R> {
     private ResultCondition.Result readyPostconditionResult;
     private ResultCondition.Result reconcilePostconditionResult;
     private boolean deleted;
+    private boolean visited;
 
     Detail<R> build() {
       return new Detail<>(error, reconcileResult, activationConditionResult,
-          deletePostconditionResult, readyPostconditionResult, reconcilePostconditionResult, deleted);
+          deletePostconditionResult, readyPostconditionResult, reconcilePostconditionResult, deleted, visited);
     }
 
     DetailBuilder<R> withResultForCondition(DependentResourceNode.ConditionWithType conditionWithType, ResultCondition.Result conditionResult) {
@@ -107,6 +109,15 @@ public boolean hasPostDeleteConditionNotMet() {
     public boolean isNotReady() {
       return readyPostconditionResult != null && !readyPostconditionResult.isSuccess();
     }
+
+    DetailBuilder<R> markAsVisited() {
+      visited = true;
+      return this;
+    }
+
+    public boolean isVisited() {
+      return visited;
+    }
   }
 
 
@@ -115,10 +126,19 @@ record Detail<R>(Exception error, ReconcileResult<R> reconcileResult,
       ResultCondition.Result deletePostconditionResult,
       ResultCondition.Result readyPostconditionResult,
       ResultCondition.Result reconcilePostconditionResult,
-                   boolean deleted) {
+                   boolean deleted, boolean visited) {
+
+    boolean isConditionWithTypeMet(Condition.Type conditionType) {
+      return getResultForConditionWithType(conditionType).map(ResultCondition.Result::isSuccess).orElse(true);
+    }
 
-    static boolean isConditionMet(ResultCondition.Result conditionResult) {
-      return conditionResult != null && conditionResult.isSuccess();
+    Optional<ResultCondition.Result<?>> getResultForConditionWithType(Condition.Type conditionType) {
+      return switch (conditionType) {
+        case ACTIVATION -> Optional.ofNullable(activationConditionResult);
+        case DELETE -> Optional.ofNullable(deletePostconditionResult);
+        case READY -> Optional.ofNullable(readyPostconditionResult);
+        case RECONCILE -> Optional.ofNullable(reconcilePostconditionResult);
+      };
     }
   }
 }

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowResultTest.java

@@ -14,7 +14,8 @@
 
 class WorkflowResultTest {
   private final static WorkflowResult.Detail detail =
-      new WorkflowResult.Detail<>(new RuntimeException(), null, null, null, null, null, false);
+      new WorkflowResult.Detail<>(new RuntimeException(), null, null, null, null, null, false,
+          false);
 
   @Test
   void throwsExceptionWithoutNumberingIfAllDifferentClass() {