refactor: isolate index handling to BulkDependentResource interface

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

@@ -1,7 +1,5 @@
 package io.javaoperatorsdk.operator.processing.dependent;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Optional;
 
 import org.slf4j.Logger;
@@ -20,15 +18,15 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
     implements DependentResource<R, P> {
   private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);
 
-  protected final boolean creatable = this instanceof Creator;
-  protected final boolean updatable = this instanceof Updater;
-  protected final boolean bulk = this instanceof BulkDependentResource;
+  private final boolean creatable = this instanceof Creator;
+  private final boolean updatable = this instanceof Updater;
+  private final boolean bulk = this instanceof BulkDependentResource;
 
   protected Creator<R, P> creator;
   protected Updater<R, P> updater;
   protected BulkDependentResource<R, P> bulkDependentResource;
-
-  protected List<ResourceDiscriminator<R, P>> resourceDiscriminator = new ArrayList<>(1);
+  private ResourceDiscriminator<R, P> resourceDiscriminator;
+  private int currentCount;
 
   @SuppressWarnings("unchecked")
   public AbstractDependentResource() {
@@ -42,48 +40,33 @@ public AbstractDependentResource() {
   public ReconcileResult<R> reconcile(P primary, Context<P> context) {
     if (bulk) {
       final var count = bulkDependentResource.count(primary, context);
-      deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context);
-      adjustDiscriminators(count);
+      deleteBulkResourcesIfRequired(count, primary, context);
       @SuppressWarnings("unchecked")
       final ReconcileResult<R>[] results = new ReconcileResult[count];
       for (int i = 0; i < count; i++) {
         results[i] = reconcileIndexAware(primary, i, context);
       }
+      currentCount = count;
       return ReconcileResult.aggregatedResult(results);
     } else {
       return reconcileIndexAware(primary, 0, context);
     }
   }
 
-  protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary,
-      Context<P> context) {
-    if (targetCount >= actualCount) {
+  protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context<P> context) {
+    if (targetCount >= currentCount) {
       return;
     }
-    for (int i = targetCount; i < actualCount; i++) {
-      var resource = getSecondaryResourceIndexAware(primary, i, context);
+    for (int i = targetCount; i < currentCount; i++) {
+      var resource = bulkDependentResource.getSecondaryResource(primary, i, context);
       var index = i;
       resource.ifPresent(
           r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context));
     }
   }
 
-  private void adjustDiscriminators(int count) {
-    if (resourceDiscriminator.size() == count) {
-      return;
-    }
-    if (resourceDiscriminator.size() < count) {
-      for (int i = resourceDiscriminator.size(); i < count; i++) {
-        resourceDiscriminator.add(bulkDependentResource.getResourceDiscriminator(i));
-      }
-    }
-    if (resourceDiscriminator.size() > count) {
-      resourceDiscriminator.subList(count, resourceDiscriminator.size()).clear();
-    }
-  }
-
   protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> context) {
-    Optional<R> maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context)
+    Optional<R> maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context)
         : getSecondaryResource(primary, context);
     if (creatable || updatable) {
       if (maybeActual.isEmpty()) {
@@ -99,7 +82,7 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
         if (updatable) {
           final Matcher.Result<R> match;
           if (bulk) {
-            match = updater.match(actual, primary, i, context);
+            match = bulkDependentResource.match(actual, primary, i, context);
           } else {
             match = updater.match(actual, primary, context);
           }
@@ -124,17 +107,12 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
   }
 
   private R desiredIndexAware(P primary, int i, Context<P> context) {
-    return bulk ? desired(primary, i, context)
-        : desired(primary, context);
+    return bulk ? desired(primary, i, context) : desired(primary, context);
   }
 
   public Optional<R> getSecondaryResource(P primary, Context<P> context) {
-    return resourceDiscriminator.isEmpty() ? context.getSecondaryResource(resourceType())
-        : resourceDiscriminator.get(0).distinguish(resourceType(), primary, context);
-  }
-
-  protected Optional<R> getSecondaryResourceIndexAware(P primary, int index, Context<P> context) {
-    return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index));
+    return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
+        : resourceDiscriminator.distinguish(resourceType(), primary, context);
   }
 
   private void throwIfNull(R desired, P primary, String descriptor) {
@@ -195,28 +173,35 @@ protected R desired(P primary, Context<P> context) {
   }
 
   protected R desired(P primary, int index, Context<P> context) {
-    throw new IllegalStateException(
-        "Must be implemented for bulk DependentResource creation");
+    throw new IllegalStateException("Must be implemented for bulk DependentResource creation");
   }
 
-  public AbstractDependentResource<R, P> setResourceDiscriminator(
-      ResourceDiscriminator<R, P> resourceDiscriminator) {
-    if (resourceDiscriminator != null) {
-      this.resourceDiscriminator.add(resourceDiscriminator);
+  public void delete(P primary, Context<P> context) {
+    if (bulk) {
+      deleteBulkResourcesIfRequired(0, primary, context);
+    } else {
+      handleDelete(primary, context);
     }
-    return this;
   }
 
-  public ResourceDiscriminator<R, P> getResourceDiscriminator() {
-    if (this.resourceDiscriminator.isEmpty()) {
-      return null;
-    } else {
-      return this.resourceDiscriminator.get(0);
-    }
+  protected void handleDelete(P primary, Context<P> context) {
+    throw new IllegalStateException("delete method be implemented if Deleter trait is supported");
   }
 
-  protected int lastKnownBulkSize() {
-    return resourceDiscriminator.size();
+  public void setResourceDiscriminator(
+      ResourceDiscriminator<R, P> resourceDiscriminator) {
+    this.resourceDiscriminator = resourceDiscriminator;
   }
 
+  protected boolean isCreatable() {
+    return creatable;
+  }
+
+  protected boolean isUpdatable() {
+    return updatable;
+  }
+
+  protected boolean isBulk() {
+    return bulk;
+  }
 }

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

@@ -1,9 +1,11 @@
 package io.javaoperatorsdk.operator.processing.dependent;
 
+import java.util.Optional;
+
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
-import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
+import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 
 /**
  * Manages dynamic number of resources created for a primary resource. Since the point of a bulk
@@ -30,6 +32,21 @@ public interface BulkDependentResource<R, P extends HasMetadata> extends Creator
    */
   void deleteBulkResourceWithIndex(P primary, R resource, int i, Context<P> context);
 
-  ResourceDiscriminator<R, P> getResourceDiscriminator(int index);
+  /**
+   * Determines whether the specified secondary resource matches the desired state with target index
+   * of a bulk resource as defined from the specified primary resource, given the specified
+   * {@link Context}.
+   *
+   * @param actualResource the resource we want to determine whether it's matching the desired state
+   * @param primary the primary resource from which the desired state is inferred
+   * @param context the context in which the resource is being matched
+   * @return a {@link Result} encapsulating whether the resource matched its desired state and this
+   *         associated state if it was computed as part of the matching process. Use the static
+   *         convenience methods ({@link Result#nonComputed(boolean)} and
+   *         {@link Result#computed(boolean, Object)})
+   */
+  Result<R> match(R actualResource, P primary, int index, Context<P> context);
+
+  Optional<R> getSecondaryResource(P primary, int index, Context<P> context);
 
 }

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

@@ -13,8 +13,12 @@
 public interface BulkUpdater<R, P extends HasMetadata> extends Updater<R, P> {
 
   default Matcher.Result<R> match(R actualResource, P primary, Context<P> context) {
-    throw new IllegalStateException();
+    if (!(this instanceof BulkDependentResource)) {
+      throw new IllegalStateException(
+          BulkUpdater.class.getSimpleName() + " interface should only be implemented by "
+              + BulkDependentResource.class.getSimpleName() + " implementations");
+    }
+    throw new IllegalStateException("This method should not be called from a "
+        + BulkDependentResource.class.getSimpleName() + " implementation");
   }
-
-  Matcher.Result<R> match(R actualResource, P primary, int index, Context<P> context);
 }

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

@@ -16,10 +16,4 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
     var desired = abstractDependentResource.desired(primary, context);
     return Result.computed(actualResource.equals(desired), desired);
   }
-
-  @Override
-  public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-    var desired = abstractDependentResource.desired(primary, index, context);
-    return Result.computed(actualResource.equals(desired), desired);
-  }
 }

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

@@ -95,19 +95,4 @@ public Optional<T> computedDesired() {
    *         {@link Result#computed(boolean, Object)})
    */
   Result<R> match(R actualResource, P primary, Context<P> context);
-
-  /**
-   * Determines whether the specified secondary resource matches the desired state with target index
-   * of a bulk resource as defined from the specified primary resource, given the specified
-   * {@link Context}.
-   *
-   * @param actualResource the resource we want to determine whether it's matching the desired state
-   * @param primary the primary resource from which the desired state is inferred
-   * @param context the context in which the resource is being matched
-   * @return a {@link Result} encapsulating whether the resource matched its desired state and this
-   *         associated state if it was computed as part of the matching process. Use the static
-   *         convenience methods ({@link Result#nonComputed(boolean)} and
-   *         {@link Result#computed(boolean, Object)})
-   */
-  Result<R> match(R actualResource, P primary, int index, Context<P> context);
 }

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

@@ -8,8 +8,4 @@ public interface Updater<R, P extends HasMetadata> {
   R update(R actual, R desired, P primary, Context<P> context);
 
   Result<R> match(R actualResource, P primary, Context<P> context);
-
-  default Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-    throw new IllegalStateException("Implement this for bulk matching");
-  }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java

@@ -23,47 +23,7 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> depen
   @SuppressWarnings({"unchecked", "rawtypes"})
   static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
       Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
-    if (Secret.class.isAssignableFrom(resourceType)) {
-      return new Matcher<>() {
-        @Override
-        public Result<R> match(R actualResource, P primary, Context<P> context) {
-          final var desired = dependentResource.desired(primary, context);
-          return Result.computed(
-              ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
-              desired);
-        }
-
-        @Override
-        public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-          final var desired = dependentResource.desired(primary, index, context);
-          return Result.computed(
-              ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
-              desired);
-        }
-      };
-    } else if (ConfigMap.class.isAssignableFrom(resourceType)) {
-      return new Matcher<>() {
-        @Override
-        public Result<R> match(R actualResource, P primary, Context<P> context) {
-          final var desired = dependentResource.desired(primary, context);
-          return Result.computed(
-              ResourceComparators.compareConfigMapData((ConfigMap) desired,
-                  (ConfigMap) actualResource),
-              desired);
-        }
-
-        @Override
-        public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-          final var desired = dependentResource.desired(primary, index, context);
-          return Result.computed(
-              ResourceComparators.compareConfigMapData((ConfigMap) desired,
-                  (ConfigMap) actualResource),
-              desired);
-        }
-      };
-    } else {
-      return new GenericKubernetesResourceMatcher(dependentResource);
-    }
+    return new GenericKubernetesResourceMatcher(dependentResource);
   }
 
   @Override
@@ -72,14 +32,8 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
     return match(desired, actualResource, false);
   }
 
-  @Override
-  public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-    var desired = dependentResource.desired(primary, index, context);
-    return match(desired, actualResource, false);
-  }
-
-  public static <R extends HasMetadata> Result<R> match(
-      R desired, R actualResource, boolean considerMetadata) {
+  public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
+      boolean considerMetadata) {
     if (considerMetadata) {
       final var desiredMetadata = desired.getMetadata();
       final var actualMetadata = actualResource.getMetadata();
@@ -91,20 +45,30 @@ public static <R extends HasMetadata> Result<R> match(
       }
     }
 
-    final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper();
+    if (desired instanceof ConfigMap) {
+      return Result.computed(
+          ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actualResource),
+          desired);
+    } else if (desired instanceof Secret) {
+      return Result.computed(
+          ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
+          desired);
+    } else {
+      final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper();
 
-    // reflection will be replaced by this:
-    // https://github.com/fabric8io/kubernetes-client/issues/3816
-    var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
-    var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
-    var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
-    for (int i = 0; i < diffJsonPatch.size(); i++) {
-      String operation = diffJsonPatch.get(i).get("op").asText();
-      if (!operation.equals("add")) {
-        return Result.computed(false, desired);
+      // reflection will be replaced by this:
+      // https://github.com/fabric8io/kubernetes-client/issues/3816
+      var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
+      var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
+      var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
+      for (int i = 0; i < diffJsonPatch.size(); i++) {
+        String operation = diffJsonPatch.get(i).get("op").asText();
+        if (!operation.equals("add")) {
+          return Result.computed(false, desired);
+        }
       }
+      return Result.computed(true, desired);
     }
-    return Result.computed(true, desired);
   }
 
   /**