Revert "refactor: isolate index handling to BulkDependentResource interface"

This reverts commit e2e6b18.

Author: csviri

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

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

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

@@ -1,11 +1,9 @@
 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
@@ -32,21 +30,6 @@ public interface BulkDependentResource<R, P extends HasMetadata> extends Creator
    */
   void deleteBulkResourceWithIndex(P primary, R resource, int i, 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);
-
-  Optional<R> getSecondaryResource(P primary, int index, Context<P> context);
+  ResourceDiscriminator<R, P> getResourceDiscriminator(int index);
 
 }

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

@@ -13,12 +13,8 @@
 public interface BulkUpdater<R, P extends HasMetadata> extends Updater<R, P> {
 
   default Matcher.Result<R> match(R actualResource, P primary, Context<P> context) {
-    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");
+    throw new IllegalStateException();
   }
+
+  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,4 +16,10 @@ 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,4 +95,19 @@ 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,4 +8,8 @@ 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

@@ -24,19 +24,42 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> depen
   static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
       Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
     if (Secret.class.isAssignableFrom(resourceType)) {
-      return (actualResource, primary, context) -> {
-        final var desired = dependentResource.desired(primary, context);
-        return Result.computed(
-            ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
-            desired);
+      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 (actualResource, primary, context) -> {
-        final var desired = dependentResource.desired(primary, context);
-        return Result.computed(
-            ResourceComparators.compareConfigMapData((ConfigMap) desired,
-                (ConfigMap) actualResource),
-            desired);
+      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);
@@ -49,6 +72,12 @@ 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) {
     if (considerMetadata) {

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

@@ -135,19 +135,23 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
   }
 
   public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
-    final var desired = desired(primary, index, context);
-    return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
+    return matcher.match(actualResource, primary, index, context);
   }
 
-  protected void handleDelete(P primary, Context<P> context) {
-    var resource = getSecondaryResource(primary, context);
-    resource.ifPresent(r -> client.resource(r).delete());
+  public void delete(P primary, Context<P> context) {
+    if (bulk) {
+      deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context);
+    } else {
+      var resource = getSecondaryResource(primary, context);
+      resource.ifPresent(r -> client.resource(r).delete());
+    }
   }
 
   public void deleteBulkResourceWithIndex(P primary, R resource, int i, Context<P> context) {
     client.resource(resource).delete();
   }
 
+  @SuppressWarnings("unchecked")
   protected Resource<R> prepare(R desired, P primary, String actionName) {
     log.debug("{} target resource with type: {}, with id: {}",
         actionName,
@@ -188,7 +192,7 @@ protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> cont
   }
 
   private boolean useDefaultAnnotationsToIdentifyPrimary() {
-    return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && isCreatable();
+    return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable;
   }
 
   private void addDefaultSecondaryToPrimaryMapperAnnotations(R desired, P primary) {