refactor: reduce redundancy by removing duplicate configuration recording

Author: metacosm

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java

@@ -1,10 +1,7 @@
 package io.javaoperatorsdk.operator.processing.event.source.controller;
 
-import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Function;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -21,12 +18,8 @@
 import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource;
 
 import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException;
-import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
-import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
-import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
-import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.onUpdateFinalizerNeededAndApplied;
-import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.onUpdateGenerationAware;
-import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.onUpdateMarkedForDeletion;
+import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.*;
+import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.*;
 
 public class ControllerResourceEventSource<T extends HasMetadata>
     extends ManagedInformerEventSource<T, T, ControllerConfiguration<T>>
@@ -137,9 +130,4 @@ public void setOnDeleteFilter(OnDeleteFilter<? super T> onDeleteFilter) {
     throw new IllegalStateException(
         "onDeleteFilter is not supported for controller resource event source");
   }
-
-  @Override
-  public void addIndexers(Map<String, Function<T, List<String>>> indexers) {
-    manager().addIndexers(indexers);
-  }
 }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

@@ -1,10 +1,6 @@
 package io.javaoperatorsdk.operator.processing.event.source.informer;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
+import java.util.*;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -76,7 +72,6 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata>
 
   private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class);
 
-  private final InformerConfiguration<R> configuration;
   // always called from a synchronized method
   private final EventRecorder<R> eventRecorder = new EventRecorder<>();
   // we need direct control for the indexer to propagate the just update resource also to the index
@@ -91,8 +86,6 @@ public InformerEventSource(
 
   public InformerEventSource(InformerConfiguration<R> configuration, KubernetesClient client) {
     super(client.resources(configuration.getResourceClass()), configuration);
-    this.configuration = configuration;
-
 
     // If there is a primary to secondary mapper there is no need for primary to secondary index.
     primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper();
@@ -193,7 +186,7 @@ private boolean temporaryCacheHasResourceWithSameVersionAs(R resource) {
 
   private void propagateEvent(R object) {
     var primaryResourceIdSet =
-        configuration.getSecondaryToPrimaryMapper().toPrimaryResourceIDs(object);
+        configuration().getSecondaryToPrimaryMapper().toPrimaryResourceIDs(object);
     if (primaryResourceIdSet.isEmpty()) {
       return;
     }
@@ -217,8 +210,7 @@ public Set<R> getSecondaryResources(P primary) {
     Set<ResourceID> secondaryIDs;
     if (useSecondaryToPrimaryIndex()) {
       var primaryResourceID = ResourceID.fromResource(primary);
-      secondaryIDs =
-          primaryToSecondaryIndex.getSecondaryResources(primaryResourceID);
+      secondaryIDs = primaryToSecondaryIndex.getSecondaryResources(primaryResourceID);
       log.debug(
           "Using PrimaryToSecondaryIndex to find secondary resources for primary: {}. Found secondary ids: {} ",
           primaryResourceID, secondaryIDs);
@@ -232,8 +224,16 @@ public Set<R> getSecondaryResources(P primary) {
         .collect(Collectors.toSet());
   }
 
+  /**
+   * Returns the configuration object for the informer.
+   *
+   * @return the informer configuration object
+   *
+   * @deprecated Use {@link #configuration()} instead
+   */
+  @Deprecated(forRemoval = true)
   public InformerConfiguration<R> getConfiguration() {
-    return configuration;
+    return configuration();
   }
 
   @Override
@@ -334,7 +334,7 @@ public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resour
 
   @Override
   public boolean allowsNamespaceChanges() {
-    return getConfiguration().followControllerNamespaceChanges();
+    return configuration().followControllerNamespaceChanges();
   }
 
 
@@ -364,7 +364,7 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) {
   public void setConfigurationService(ConfigurationService configurationService) {
     super.setConfigurationService(configurationService);
 
-    cache.addIndexers(indexerBuffer);
+    super.addIndexers(indexerBuffer);
     indexerBuffer = new HashMap<>();
   }
 

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java

@@ -1,11 +1,6 @@
 package io.javaoperatorsdk.operator.processing.event.source.informer;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -23,7 +18,6 @@
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
-import io.javaoperatorsdk.operator.api.config.Cloner;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ResourceConfiguration;
 import io.javaoperatorsdk.operator.health.InformerHealthIndicator;
@@ -40,19 +34,21 @@ public class InformerManager<T extends HasMetadata, C extends ResourceConfigurat
   private static final Logger log = LoggerFactory.getLogger(InformerManager.class);
 
   private final Map<String, InformerWrapper<T>> sources = new ConcurrentHashMap<>();
-  private Cloner cloner;
   private final C configuration;
   private final MixedOperation<T, KubernetesResourceList<T>, Resource<T>> client;
   private final ResourceEventHandler<T> eventHandler;
   private final Map<String, Function<T, List<String>>> indexers = new HashMap<>();
-  private final ConfigurationService configurationService;
+  private ConfigurationService configurationService;
 
-  public InformerManager(MixedOperation<T, KubernetesResourceList<T>, Resource<T>> client,
-      C configuration, ConfigurationService configurationService,
+  InformerManager(MixedOperation<T, KubernetesResourceList<T>, Resource<T>> client,
+      C configuration,
       ResourceEventHandler<T> eventHandler) {
     this.client = client;
     this.configuration = configuration;
     this.eventHandler = eventHandler;
+  }
+
+  void setConfigurationService(ConfigurationService configurationService) {
     this.configurationService = configurationService;
   }
 
@@ -74,7 +70,6 @@ private void initSources() {
     if (!sources.isEmpty()) {
       throw new IllegalStateException("Some sources already initialized.");
     }
-    cloner = configurationService.getResourceCloner();
     final var targetNamespaces = configuration.getEffectiveNamespaces(configurationService);
     if (ResourceConfiguration.allNamespacesWatched(targetNamespaces)) {
       var source = createEventSourceForNamespace(WATCH_ALL_NAMESPACES);
@@ -175,7 +170,7 @@ public Stream<T> list(String namespace, Predicate<T> predicate) {
   public Optional<T> get(ResourceID resourceID) {
     return getSource(resourceID.getNamespace().orElse(WATCH_ALL_NAMESPACES))
         .flatMap(source -> source.get(resourceID))
-        .map(cloner::clone);
+        .map(r -> configurationService.getResourceCloner().clone(r));
   }
 
   @Override

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java

@@ -36,18 +36,17 @@ public abstract class ManagedInformerEventSource<R extends HasMetadata, P extend
     InformerWrappingEventSourceHealthIndicator<R>, Configurable<C> {
 
   private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class);
+  private final InformerManager<R, C> cache;
 
   protected TemporaryResourceCache<R> temporaryResourceCache;
-  protected InformerManager<R, C> cache;
-  protected C configuration;
   protected MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client;
 
   protected ManagedInformerEventSource(
       MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client, C configuration) {
     super(configuration.getResourceClass());
     this.client = client;
     temporaryResourceCache = new TemporaryResourceCache<>(this);
-    this.configuration = configuration;
+    this.cache = new InformerManager<>(client, configuration, this);
   }
 
   @Override
@@ -128,7 +127,9 @@ void setTemporalResourceCache(TemporaryResourceCache<R> temporaryResourceCache)
     this.temporaryResourceCache = temporaryResourceCache;
   }
 
-  public abstract void addIndexers(Map<String, Function<R, List<String>>> indexers);
+  public void addIndexers(Map<String, Function<R, List<String>>> indexers) {
+    cache.addIndexers(indexers);
+  }
 
   public List<R> byIndex(String indexName, String indexKey) {
     return manager().byIndex(indexName, indexKey);
@@ -156,7 +157,7 @@ public Status getStatus() {
 
   @Override
   public ResourceConfiguration<R> getInformerConfiguration() {
-    return configuration;
+    return configuration();
   }
 
   @Override
@@ -167,11 +168,11 @@ public C configuration() {
   @Override
   public String toString() {
     return getClass().getSimpleName() + "{" +
-        "resourceClass: " + configuration.getResourceClass().getSimpleName() +
+        "resourceClass: " + configuration().getResourceClass().getSimpleName() +
         "}";
   }
 
   public void setConfigurationService(ConfigurationService configurationService) {
-    cache = new InformerManager<>(client, configuration, configurationService, this);
+    cache.setConfigurationService(configurationService);
   }
 }

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java

@@ -23,16 +23,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
 
 @SuppressWarnings({"rawtypes", "unchecked"})
 class EventSourceManagerTest {
@@ -166,7 +158,7 @@ void changesNamespacesOnControllerAndInformerEventSources() {
     when(informerConfigurationMock.followControllerNamespaceChanges()).thenReturn(true);
     InformerEventSource informerEventSource = mock(InformerEventSource.class);
     when(informerEventSource.resourceType()).thenReturn(TestCustomResource.class);
-    when(informerEventSource.getConfiguration()).thenReturn(informerConfigurationMock);
+    when(informerEventSource.configuration()).thenReturn(informerConfigurationMock);
     when(informerEventSource.allowsNamespaceChanges()).thenCallRealMethod();
     manager.registerEventSource("ies", informerEventSource);