refactor: move getResourceClassResolver to BaseConfigurationService (#…

…2451) The class resolving mechanism is only needed in that class right now Signed-off-by: Chris Laprun <claprun@redhat.com>
operator-framework · Jul 9, 2024 · da7b033 · da7b033
1 parent 07b5195
commit da7b033
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 104 deletions.
diff --git a/...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java
@@ -39,6 +39,7 @@ public class BaseConfigurationService extends AbstractConfigurationService {
 
   private static final String LOGGER_NAME = "Default ConfigurationService implementation";
   private static final Logger logger = LoggerFactory.getLogger(LOGGER_NAME);
+  private static final ResourceClassResolver DEFAULT_RESOLVER = new DefaultResourceClassResolver();
 
   public BaseConfigurationService(Version version) {
     this(version, null);
@@ -56,6 +57,89 @@ public BaseConfigurationService() {
     this(Utils.VERSION);
   }
 
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  private static List<DependentResourceSpec> dependentResources(
+      Workflow annotation,
+      ControllerConfiguration<?> controllerConfiguration) {
+    final var dependents = annotation.dependents();
+
+
+    if (dependents == null || dependents.length == 0) {
+      return Collections.emptyList();
+    }
+
+    final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
+    for (Dependent dependent : dependents) {
+      final Class<? extends DependentResource> dependentType = dependent.type();
+
+      final var dependentName = getName(dependent.name(), dependentType);
+      var spec = specsMap.get(dependentName);
+      if (spec != null) {
+        throw new IllegalArgumentException(
+            "A DependentResource named '" + dependentName + "' already exists: " + spec);
+      }
+
+      final var name = controllerConfiguration.getName();
+
+      var eventSourceName = dependent.useEventSourceWithName();
+      eventSourceName = Constants.NO_VALUE_SET.equals(eventSourceName) ? null : eventSourceName;
+      final var context = Utils.contextFor(name, dependentType, null);
+      spec = new DependentResourceSpec(dependentType, dependentName,
+          Set.of(dependent.dependsOn()),
+          Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
+          Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
+          Utils.instantiate(dependent.deletePostcondition(), Condition.class, context),
+          Utils.instantiate(dependent.activationCondition(), Condition.class, context),
+          eventSourceName);
+
+      // extract potential configuration
+      DependentResourceConfigurationResolver.configureSpecFromConfigured(spec,
+          controllerConfiguration,
+          dependentType);
+
+      specsMap.put(dependentName, spec);
+    }
+    return specsMap.values().stream().toList();
+  }
+
+  private static <T> T valueOrDefault(
+      io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration controllerConfiguration,
+      Function<io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration, T> mapper,
+      T defaultValue) {
+    if (controllerConfiguration == null) {
+      return defaultValue;
+    } else {
+      return mapper.apply(controllerConfiguration);
+    }
+  }
+
+  @SuppressWarnings("rawtypes")
+  private static String getName(String name, Class<? extends DependentResource> dependentType) {
+    if (name.isBlank()) {
+      name = DependentResource.defaultNameFor(dependentType);
+    }
+    return name;
+  }
+
+  @SuppressWarnings("unused")
+  private static <T> Configurator<T> configuratorFor(Class<T> instanceType,
+      Reconciler<?> reconciler) {
+    return instance -> configureFromAnnotatedReconciler(instance, reconciler);
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  private static void configureFromAnnotatedReconciler(Object instance, Reconciler<?> reconciler) {
+    if (instance instanceof AnnotationConfigurable configurable) {
+      final Class<? extends Annotation> configurationClass =
+          (Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
+              instance.getClass(), AnnotationConfigurable.class);
+      final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
+      if (configAnnotation != null) {
+        configurable.initFrom(configAnnotation);
+      }
+    }
+  }
+
   @Override
   protected void logMissingReconcilerWarning(String reconcilerKey, String reconcilersNameMessage) {
     logger.warn("Configuration for reconciler '{}' was not found. {}", reconcilerKey,
@@ -95,6 +179,15 @@ public <R extends HasMetadata> ControllerConfiguration<R> getConfigurationFor(
     return config;
   }
 
+  /**
+   * Override if a different class resolution is needed
+   *
+   * @return the custom {@link ResourceClassResolver} implementation to use
+   */
+  protected ResourceClassResolver getResourceClassResolver() {
+    return DEFAULT_RESOLVER;
+  }
+
   @SuppressWarnings({"unchecked", "rawtypes"})
   protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconciler<P> reconciler) {
     final var annotation = reconciler.getClass().getAnnotation(
@@ -109,7 +202,7 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
               " annotation for reconciler:  " + reconciler);
     }
     Class<Reconciler<P>> reconcilerClass = (Class<Reconciler<P>>) reconciler.getClass();
-    final var resourceClass = getResourceClassResolver().getResourceClass(reconcilerClass);
+    final var resourceClass = getResourceClassResolver().getPrimaryResourceClass(reconcilerClass);
 
     final var name = ReconcilerUtils.getNameFor(reconciler);
     final var generationAware = valueOrDefault(
@@ -192,51 +285,6 @@ public boolean handleExceptionsInReconciler() {
     return config;
   }
 
-  @SuppressWarnings({"unchecked", "rawtypes"})
-  private static List<DependentResourceSpec> dependentResources(
-      Workflow annotation,
-      ControllerConfiguration<?> controllerConfiguration) {
-    final var dependents = annotation.dependents();
-
-
-    if (dependents == null || dependents.length == 0) {
-      return Collections.emptyList();
-    }
-
-    final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
-    for (Dependent dependent : dependents) {
-      final Class<? extends DependentResource> dependentType = dependent.type();
-
-      final var dependentName = getName(dependent.name(), dependentType);
-      var spec = specsMap.get(dependentName);
-      if (spec != null) {
-        throw new IllegalArgumentException(
-            "A DependentResource named '" + dependentName + "' already exists: " + spec);
-      }
-
-      final var name = controllerConfiguration.getName();
-
-      var eventSourceName = dependent.useEventSourceWithName();
-      eventSourceName = Constants.NO_VALUE_SET.equals(eventSourceName) ? null : eventSourceName;
-      final var context = Utils.contextFor(name, dependentType, null);
-      spec = new DependentResourceSpec(dependentType, dependentName,
-          Set.of(dependent.dependsOn()),
-          Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
-          Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
-          Utils.instantiate(dependent.deletePostcondition(), Condition.class, context),
-          Utils.instantiate(dependent.activationCondition(), Condition.class, context),
-          eventSourceName);
-
-      // extract potential configuration
-      DependentResourceConfigurationResolver.configureSpecFromConfigured(spec,
-          controllerConfiguration,
-          dependentType);
-
-      specsMap.put(dependentName, spec);
-    }
-    return specsMap.values().stream().toList();
-  }
-
   protected boolean createIfNeeded() {
     return true;
   }
@@ -245,42 +293,4 @@ protected boolean createIfNeeded() {
   public boolean checkCRDAndValidateLocalModel() {
     return Utils.shouldCheckCRDAndValidateLocalModel();
   }
-
-  private static <T> T valueOrDefault(
-      io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration controllerConfiguration,
-      Function<io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration, T> mapper,
-      T defaultValue) {
-    if (controllerConfiguration == null) {
-      return defaultValue;
-    } else {
-      return mapper.apply(controllerConfiguration);
-    }
-  }
-
-  @SuppressWarnings("rawtypes")
-  private static String getName(String name, Class<? extends DependentResource> dependentType) {
-    if (name.isBlank()) {
-      name = DependentResource.defaultNameFor(dependentType);
-    }
-    return name;
-  }
-
-  @SuppressWarnings("unused")
-  private static <T> Configurator<T> configuratorFor(Class<T> instanceType,
-      Reconciler<?> reconciler) {
-    return instance -> configureFromAnnotatedReconciler(instance, reconciler);
-  }
-
-  @SuppressWarnings({"unchecked", "rawtypes"})
-  private static void configureFromAnnotatedReconciler(Object instance, Reconciler<?> reconciler) {
-    if (instance instanceof AnnotationConfigurable configurable) {
-      final Class<? extends Annotation> configurationClass =
-          (Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
-              instance.getClass(), AnnotationConfigurable.class);
-      final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
-      if (configAnnotation != null) {
-        configurable.initFrom(configAnnotation);
-      }
-    }
-  }
 }
diff --git a/...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
@@ -233,10 +233,6 @@ default ManagedWorkflowFactory getWorkflowFactory() {
     return ManagedWorkflowFactory.DEFAULT;
   }
 
-  default ResourceClassResolver getResourceClassResolver() {
-    return new DefaultResourceClassResolver();
-  }
-
   /**
    * Creates a new {@link ConfigurationService} instance used to configure an
    * {@link io.javaoperatorsdk.operator.Operator} instance, starting from the specified base

diff --git a/...e/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/...e/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
@@ -14,7 +14,7 @@
 import io.javaoperatorsdk.operator.api.monitoring.Metrics;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
 
-@SuppressWarnings("unused")
+@SuppressWarnings({"unused", "UnusedReturnValue"})
 public class ConfigurationServiceOverrider {
 
   private static final Logger log = LoggerFactory.getLogger(ConfigurationServiceOverrider.class);
@@ -32,7 +32,6 @@ public class ConfigurationServiceOverrider {
   private InformerStoppedHandler informerStoppedHandler;
   private Boolean stopOnInformerErrorDuringStartup;
   private Duration cacheSyncTimeout;
-  private ResourceClassResolver resourceClassResolver;
   private Boolean ssaBasedCreateUpdateMatchForDependentResources;
   private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
   private Boolean previousAnnotationForDependentResources;
@@ -128,12 +127,6 @@ public ConfigurationServiceOverrider withCacheSyncTimeout(Duration cacheSyncTime
     return this;
   }
 
-  public ConfigurationServiceOverrider withResourceClassResolver(
-      ResourceClassResolver resourceClassResolver) {
-    this.resourceClassResolver = resourceClassResolver;
-    return this;
-  }
-
   public ConfigurationServiceOverrider withSSABasedCreateUpdateMatchForDependentResources(
       boolean value) {
     this.ssaBasedCreateUpdateMatchForDependentResources = value;
@@ -258,12 +251,6 @@ public Duration cacheSyncTimeout() {
         return overriddenValueOrDefault(cacheSyncTimeout, ConfigurationService::cacheSyncTimeout);
       }
 
-      @Override
-      public ResourceClassResolver getResourceClassResolver() {
-        return overriddenValueOrDefault(resourceClassResolver,
-            ConfigurationService::getResourceClassResolver);
-      }
-
       @Override
       public boolean ssaBasedCreateUpdateMatchForDependentResources() {
         return overriddenValueOrDefault(ssaBasedCreateUpdateMatchForDependentResources,

diff --git a/...re/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceClassResolver.java b/...re/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceClassResolver.java
@@ -7,7 +7,7 @@ public class DefaultResourceClassResolver implements ResourceClassResolver {
 
   @SuppressWarnings("unchecked")
   @Override
-  public <R extends HasMetadata> Class<R> getResourceClass(
+  public <R extends HasMetadata> Class<R> getPrimaryResourceClass(
       Class<? extends Reconciler<R>> reconcilerClass) {
     return (Class<R>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(reconcilerClass,
         Reconciler.class);

diff --git a/...work-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceClassResolver.java b/...work-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceClassResolver.java
@@ -5,6 +5,7 @@
 
 public interface ResourceClassResolver {
 
-  <R extends HasMetadata> Class<R> getResourceClass(Class<? extends Reconciler<R>> reconcilerClass);
+  <P extends HasMetadata> Class<P> getPrimaryResourceClass(
+      Class<? extends Reconciler<P>> reconcilerClass);
 
 }