refactor: remove markedForDelete field

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

Author: metacosm

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

@@ -5,6 +5,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
@@ -72,13 +73,15 @@ protected boolean noMoreExecutionsScheduled() {
   }
 
   protected boolean alreadyVisited(DependentResourceNode<?, P> dependentResourceNode) {
-    final WorkflowResult.DetailBuilder<?> builder = results.get(dependentResourceNode);
-    return builder != null && builder.isVisited();
+    return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isVisited);
   }
 
-  protected boolean postDeleteConditionNotMet(DependentResourceNode<?, P> dependentResourceNode) {
-    final var builder = results.get(dependentResourceNode);
-    return builder != null && builder.hasPostDeleteConditionNotMet();
+  protected boolean postDeleteConditionNotMet(DependentResourceNode<?, P> drn) {
+    return getResultFlagFor(drn, WorkflowResult.DetailBuilder::hasPostDeleteConditionNotMet);
+  }
+
+  protected boolean isMarkedForDelete(DependentResourceNode<?, P> drn) {
+    return getResultFlagFor(drn, WorkflowResult.DetailBuilder::isMarkedForDelete);
   }
 
   protected WorkflowResult.DetailBuilder createOrGetResultFor(
@@ -87,6 +90,16 @@ protected WorkflowResult.DetailBuilder createOrGetResultFor(
         unused -> new WorkflowResult.DetailBuilder());
   }
 
+  protected Optional<WorkflowResult.DetailBuilder<?>> getResultFor(
+      DependentResourceNode<?, P> dependentResourceNode) {
+    return Optional.ofNullable(results.get(dependentResourceNode));
+  }
+
+  protected boolean getResultFlagFor(DependentResourceNode<?, P> dependentResourceNode,
+      Function<WorkflowResult.DetailBuilder<?>, Boolean> flag) {
+    return getResultFor(dependentResourceNode).map(flag).orElse(false);
+  }
+
   protected boolean isExecutingNow(DependentResourceNode<?, P> dependentResourceNode) {
     return actualExecutions.containsKey(dependentResourceNode);
   }
@@ -103,13 +116,11 @@ protected synchronized void handleExceptionInExecutor(
   }
 
   protected boolean isNotReady(DependentResourceNode<?, P> dependentResourceNode) {
-    final var builder = results.get(dependentResourceNode);
-    return builder != null && builder.isNotReady();
+    return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::isNotReady);
   }
 
   protected boolean isInError(DependentResourceNode<?, P> dependentResourceNode) {
-    final var builder = results.get(dependentResourceNode);
-    return builder != null && builder.hasError();
+   return getResultFlagFor(dependentResourceNode, WorkflowResult.DetailBuilder::hasError);
   }
 
   protected synchronized void handleNodeExecutionFinish(

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

@@ -84,7 +84,7 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode) {
         final var dependentResource = dependentResourceNode.getDependentResource();
         if (dependentResource.isDeletable()) {
           ((Deleter<P>) dependentResource).delete(primary, context);
-          createOrGetResultFor(dependentResourceNode).withDeleted(true);
+          createOrGetResultFor(dependentResourceNode).markAsDeleted();
         }
 
         deletePostConditionMet =

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

@@ -3,7 +3,6 @@
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -20,12 +19,8 @@ class WorkflowReconcileExecutor<P extends HasMetadata> extends AbstractWorkflowE
   private static final String RECONCILE = "reconcile";
   private static final String DELETE = "delete";
 
-  private final Set<DependentResourceNode> markedForDelete;
-
   public WorkflowReconcileExecutor(DefaultWorkflow<P> workflow, P primary, Context<P> context) {
     super(workflow, primary, context);
-    final var size = workflow.size();
-    markedForDelete = ConcurrentHashMap.newKeySet(size);
   }
 
   public synchronized WorkflowReconcileResult reconcile() {
@@ -47,7 +42,7 @@ private synchronized <R> void handleReconcile(DependentResourceNode<R, P> depend
     final var alreadyVisited = alreadyVisited(dependentResourceNode);
     final var executingNow = isExecutingNow(dependentResourceNode);
     final var isWaitingOnParents = !allParentsReconciledAndReady(dependentResourceNode);
-    final var isMarkedForDelete = markedForDelete.contains(dependentResourceNode);
+    final var isMarkedForDelete = isMarkedForDelete(dependentResourceNode);
     final var hasErroredParent = hasErroredParent(dependentResourceNode);
     if (isWaitingOnParents || alreadyVisited || executingNow || isMarkedForDelete
         || hasErroredParent) {
@@ -95,7 +90,7 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo
 
     final var alreadyVisited = alreadyVisited(dependentResourceNode);
     final var executingNow = isExecutingNow(dependentResourceNode);
-    final var isNotMarkedForDelete = !markedForDelete.contains(dependentResourceNode);
+    final var isNotMarkedForDelete = !isMarkedForDelete(dependentResourceNode);
     final var isWaitingOnDependents = !allDependentsDeletedAlready(dependentResourceNode);
     if (isNotMarkedForDelete || alreadyVisited || executingNow || isWaitingOnDependents) {
       if (log.isDebugEnabled()) {
@@ -216,7 +211,7 @@ private void markDependentsForDelete(DependentResourceNode<?, P> dependentResour
     // so if the activation condition was false, this node is not meant to be deleted.
     var dependents = dependentResourceNode.getParents();
     if (activationConditionMet) {
-      markedForDelete.add(dependentResourceNode);
+      createOrGetResultFor(dependentResourceNode).markForDelete();
       if (dependents.isEmpty()) {
         bottomNodes.add(dependentResourceNode);
       } else {

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) {
+  WorkflowReconcileResult(Map<DependentResource, Detail<?>> results) {
     super(results);
   }
 

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

@@ -67,10 +67,11 @@ static class DetailBuilder<R> {
     private ResultCondition.Result reconcilePostconditionResult;
     private boolean deleted;
     private boolean visited;
+    private boolean markedForDelete;
 
     Detail<R> build() {
       return new Detail<>(error, reconcileResult, activationConditionResult,
-          deletePostconditionResult, readyPostconditionResult, reconcilePostconditionResult, deleted, visited);
+          deletePostconditionResult, readyPostconditionResult, reconcilePostconditionResult, deleted, visited, markedForDelete);
     }
 
     DetailBuilder<R> withResultForCondition(DependentResourceNode.ConditionWithType conditionWithType, ResultCondition.Result conditionResult) {
@@ -94,8 +95,8 @@ DetailBuilder<R> withReconcileResult(ReconcileResult<R> reconcileResult) {
       return this;
     }
 
-    DetailBuilder<R> withDeleted(boolean deleted) {
-      this.deleted = deleted;
+    DetailBuilder<R> markAsDeleted() {
+      this.deleted = true;
       return this;
     }
 
@@ -119,6 +120,15 @@ DetailBuilder<R> markAsVisited() {
     public boolean isVisited() {
       return visited;
     }
+
+    public boolean isMarkedForDelete() {
+      return markedForDelete;
+    }
+
+    DetailBuilder<R> markForDelete() {
+      markedForDelete = true;
+      return this;
+    }
   }
 
 
@@ -127,7 +137,7 @@ record Detail<R>(Exception error, ReconcileResult<R> reconcileResult,
       ResultCondition.Result deletePostconditionResult,
       ResultCondition.Result readyPostconditionResult,
       ResultCondition.Result reconcilePostconditionResult,
-                   boolean deleted, boolean visited) {
+                   boolean deleted, boolean visited, boolean markedForDelete) {
 
     boolean isConditionWithTypeMet(Condition.Type conditionType) {
       return getResultForConditionWithType(conditionType).map(ResultCondition.Result::isSuccess).orElse(true);

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

@@ -13,9 +13,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 class WorkflowResultTest {
-  private final static WorkflowResult.Detail detail =
+  private final static WorkflowResult.Detail<?> detail =
       new WorkflowResult.Detail<>(new RuntimeException(), null, null, null, null, null, false,
-          false);
+          false, false);
 
   @Test
   void throwsExceptionWithoutNumberingIfAllDifferentClass() {