feat: implement support for converter overriding, add tests

Author: metacosm

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java

@@ -237,7 +237,6 @@ public static String contextFor(ControllerConfiguration<?> controllerConfigurati
     }
     context += "reconciler: " + controllerConfiguration.getName();
 
-
     return context;
   }
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolver.java

@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.api.config.dependent;
 
+import java.lang.annotation.Annotation;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -14,7 +15,10 @@ public class DependentResourceConfigurationResolver {
 
   private DependentResourceConfigurationResolver() {}
 
-  private static final Map<Class, ConfigurationConverter> converters = new HashMap<>();
+  private static final Map<Class<? extends DependentResource>, ConverterAnnotationPair> converters =
+      new HashMap<>();
+  private static final Map<Class<? extends ConfigurationConverter>, ConfigurationConverter> knownConverters =
+      new HashMap<>();
 
   public static <C extends ControllerConfiguration<? extends HasMetadata>> void configure(
       DependentResource dependentResource, DependentResourceSpec spec, C parentConfiguration) {
@@ -38,39 +42,140 @@ public static <C extends ControllerConfiguration<? extends HasMetadata>> Object
     }
 
     // find Configured-annotated class if it exists
-    final var dependentResourceClass = spec.getDependentResourceClass();
-    Class<?> currentClass = dependentResourceClass;
-    Configured configured = null;
-    while (!Object.class.equals(currentClass)) {
+    return extractConfigurationFromConfigured(spec.getDependentResourceClass(),
+        parentConfiguration);
+  }
+
+  public static <C extends ControllerConfiguration<? extends HasMetadata>> Object extractConfigurationFromConfigured(
+      Class<? extends DependentResource> dependentResourceClass, C parentConfiguration) {
+    var converterAnnotationPair = converters.get(dependentResourceClass);
+
+    Annotation configAnnotation;
+    if (converterAnnotationPair == null) {
+      var configuredClassPair = getConfigured(dependentResourceClass);
+      if (configuredClassPair == null) {
+        return null;
+      }
+
+      // check if we already have a converter registered for the found Configured annotated class
+      converterAnnotationPair = converters.get(configuredClassPair.annotatedClass);
+      if (converterAnnotationPair == null) {
+        final var configured = configuredClassPair.configured;
+        converterAnnotationPair =
+            getOrCreateConverter(dependentResourceClass, parentConfiguration,
+                configured.converter(),
+                configured.by());
+      } else {
+        // only register the converter pair for this dependent resource class as well
+        converters.put(dependentResourceClass, converterAnnotationPair);
+      }
+    }
+
+    // find the associated configuration annotation
+    configAnnotation =
+        dependentResourceClass.getAnnotation(converterAnnotationPair.annotationClass);
+    final var converter = converterAnnotationPair.converter;
+
+    // always called even if the annotation is null so that implementations can provide default
+    // values
+    return converter.configFrom(configAnnotation, parentConfiguration, dependentResourceClass);
+  }
+
+  private static ConfiguredClassPair getConfigured(
+      Class<? extends DependentResource> dependentResourceClass) {
+    Class<? extends DependentResource> currentClass = dependentResourceClass;
+    Configured configured;
+    ConfiguredClassPair result = null;
+    while (DependentResource.class.isAssignableFrom(currentClass)) {
       configured = currentClass.getAnnotation(Configured.class);
       if (configured != null) {
+        result = new ConfiguredClassPair(configured, currentClass);
         break;
       }
-      currentClass = currentClass.getSuperclass();
+      currentClass = (Class<? extends DependentResource>) currentClass.getSuperclass();
     }
+    return result;
+  }
 
+  private static <C extends ControllerConfiguration<? extends HasMetadata>> ConverterAnnotationPair getOrCreateConverter(
+      Class<? extends DependentResource> dependentResourceClass, C parentConfiguration,
+      Class<? extends ConfigurationConverter> converterClass,
+      Class<? extends Annotation> annotationClass) {
+    var converterPair = converters.get(dependentResourceClass);
+    if (converterPair == null) {
+      // only instantiate a new converter if we haven't done so already for this converter type
+      var converter = knownConverters.get(converterClass);
+      if (converter == null) {
+        converter = Utils.instantiate(converterClass,
+            ConfigurationConverter.class,
+            Utils.contextFor(parentConfiguration, dependentResourceClass, Configured.class));
+        knownConverters.put(converterClass, converter);
+      }
+      // record dependent class - converter association for faster future retrieval
+      converterPair = new ConverterAnnotationPair(converter, annotationClass);
+      converters.put(dependentResourceClass, converterPair);
+    }
+    return converterPair;
+  }
+
+  static ConfigurationConverter getConverter(
+      Class<? extends DependentResource> dependentResourceClass) {
+    final var converterAnnotationPair = converters.get(dependentResourceClass);
+    return converterAnnotationPair != null ? converterAnnotationPair.converter : null;
+  }
+
+  @SuppressWarnings("unused")
+  public static void registerConverter(Class<? extends DependentResource> dependentResourceClass,
+      ConfigurationConverter converter) {
+    var configured = getConfigured(dependentResourceClass);
     if (configured == null) {
-      return null;
+      throw new IllegalArgumentException("There is no @" + Configured.class.getSimpleName()
+          + " annotation on " + dependentResourceClass.getName()
+          + " or its superclasses and thus doesn't need to be associated with a converter");
     }
+
     // find the associated configuration annotation
-    final var configAnnotation = dependentResourceClass.getAnnotation(configured.by());
-    final var converter = getConverter(dependentResourceClass, parentConfiguration, configured);
-    // always called even if the annotation is null so that implementations can provide default
-    // values
-    return converter.configFrom(configAnnotation, parentConfiguration, dependentResourceClass);
+    final var toRegister = new ConverterAnnotationPair(converter, configured.configured.by());
+    final Class<? extends ConfigurationConverter> converterClass = converter.getClass();
+    converters.put(dependentResourceClass, toRegister);
+
+    // also register the Configured-annotated class if not the one we're registering
+    if (!dependentResourceClass.equals(configured.annotatedClass)) {
+      converters.put(configured.annotatedClass, toRegister);
+    }
+
+    knownConverters.put(converterClass, converter);
   }
 
-  static <C extends ControllerConfiguration<? extends HasMetadata>> ConfigurationConverter getConverter(
-      Class<? extends DependentResource> dependentResourceClass, C parentConfiguration,
-      Configured configured) {
-    var converter = converters.get(dependentResourceClass);
-    if (converter == null) {
-      converter = Utils.instantiate(configured.converter(),
-          ConfigurationConverter.class,
-          Utils.contextFor(parentConfiguration, dependentResourceClass, Configured.class));
-      converters.put(dependentResourceClass, converter);
+  private static class ConfiguredClassPair {
+    private final Configured configured;
+    private final Class<? extends DependentResource> annotatedClass;
+
+    private ConfiguredClassPair(Configured configured,
+        Class<? extends DependentResource> annotatedClass) {
+      this.configured = configured;
+      this.annotatedClass = annotatedClass;
+    }
+
+    @Override
+    public String toString() {
+      return annotatedClass.getName() + " -> " + configured;
     }
-    return converter;
   }
 
+  private static class ConverterAnnotationPair {
+    private final ConfigurationConverter converter;
+    private final Class<? extends Annotation> annotationClass;
+
+    private ConverterAnnotationPair(ConfigurationConverter converter,
+        Class<? extends Annotation> annotationClass) {
+      this.converter = converter;
+      this.annotationClass = annotationClass;
+    }
+
+    @Override
+    public String toString() {
+      return converter.toString() + " -> " + annotationClass.getName();
+    }
+  }
 }

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

@@ -17,7 +17,6 @@
 public class KubernetesDependentConverter<R extends HasMetadata, P extends HasMetadata> implements
     ConfigurationConverter<KubernetesDependent, KubernetesDependentResourceConfig<R>, KubernetesDependentResource<R, P>> {
 
-
   @Override
   @SuppressWarnings({"unchecked", "rawtypes"})
   public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent configAnnotation,