refactor: clean up, cache already resolved GVKs

Author: metacosm

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

@@ -1,20 +1,19 @@
 package io.javaoperatorsdk.operator.processing;
 
+import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GroupVersionKindPlural;
 
 public class GroupVersionKind {
   private final String group;
   private final String version;
   private final String kind;
   private final String apiVersion;
+  protected final static Map<Class<? extends HasMetadata>, GroupVersionKind> CACHE =
+      new ConcurrentHashMap<>();
 
-  /**
-   * @deprecated Use {@link GroupVersionKindPlural#gvkFor(String, String)} instead
-   */
-  @Deprecated(forRemoval = true)
   public GroupVersionKind(String apiVersion, String kind) {
     this.kind = kind;
     String[] groupAndVersion = apiVersion.split("/");
@@ -29,13 +28,14 @@ public GroupVersionKind(String apiVersion, String kind) {
   }
 
   public static GroupVersionKind gvkFor(Class<? extends HasMetadata> resourceClass) {
-    return GroupVersionKindPlural.gvkFor(resourceClass);
+    return CACHE.computeIfAbsent(resourceClass, GroupVersionKind::computeGVK);
+  }
+
+  private static GroupVersionKind computeGVK(Class<? extends HasMetadata> rc) {
+    return new GroupVersionKind(HasMetadata.getGroup(rc),
+        HasMetadata.getVersion(rc), HasMetadata.getKind(rc));
   }
 
-  /**
-   * @deprecated Use {@link GroupVersionKindPlural#gvkFor(String, String, String)} instead
-   */
-  @Deprecated(forRemoval = true)
   public GroupVersionKind(String group, String version, String kind) {
     this.group = group;
     this.version = version;

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

@@ -11,7 +11,7 @@ public class GenericKubernetesDependentResource<P extends HasMetadata>
   private final GroupVersionKindPlural groupVersionKind;
 
   public GenericKubernetesDependentResource(GroupVersionKind groupVersionKind) {
-    this(new GroupVersionKindPlural(groupVersionKind));
+    this(GroupVersionKindPlural.from(groupVersionKind));
   }
 
   public GenericKubernetesDependentResource(GroupVersionKindPlural groupVersionKind) {

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

@@ -6,31 +6,83 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.processing.GroupVersionKind;
 
+/**
+ * An extension of {@link GroupVersionKind} that also records the associated plural form which is
+ * useful when dealing with Kubernetes RBACs. Downstream projects might leverage that information.
+ */
 public class GroupVersionKindPlural extends GroupVersionKind {
   private final String plural;
 
-  public GroupVersionKindPlural(String group, String version, String kind, String plural) {
+  protected GroupVersionKindPlural(String group, String version, String kind, String plural) {
     super(group, version, kind);
     this.plural = plural;
   }
 
-  public GroupVersionKindPlural(GroupVersionKind gvk) {
+  protected GroupVersionKindPlural(String apiVersion, String kind, String plural) {
+    super(apiVersion, kind);
+    this.plural = plural;
+  }
+
+  protected GroupVersionKindPlural(GroupVersionKind gvk, String plural) {
     this(gvk.getGroup(), gvk.getVersion(), gvk.getKind(),
-        gvk instanceof GroupVersionKindPlural ? ((GroupVersionKindPlural) gvk).plural : null);
+        plural != null ? plural
+            : (gvk instanceof GroupVersionKindPlural ? ((GroupVersionKindPlural) gvk).plural
+                : null));
   }
 
-  public static GroupVersionKind gvkFor(String group, String version, String kind) {
-    return new GroupVersionKind(group, version, kind);
+  /**
+   * Creates a new GroupVersionKindPlural from the specified {@link GroupVersionKind}.
+   *
+   * @param gvk a {@link GroupVersionKind} from which to create a new GroupVersionKindPlural object
+   * @return a new GroupVersionKindPlural object matching the specified {@link GroupVersionKind}
+   */
+  public static GroupVersionKindPlural from(GroupVersionKind gvk) {
+    return gvk instanceof GroupVersionKindPlural ? ((GroupVersionKindPlural) gvk)
+        : gvkWithPlural(gvk, null);
   }
 
-  public static GroupVersionKind gvkFor(String apiVersion, String kind) {
-    return new GroupVersionKind(apiVersion, kind);
+  /**
+   * Creates a new GroupVersionKindPlural based on the specified {@link GroupVersionKind} instance
+   * but specifying a plural form to use as well.
+   *
+   * @param gvk the base {@link GroupVersionKind} from which to derive a new GroupVersionKindPlural
+   * @param plural the plural form to use for the new instance or {@code null} if the default plural
+   *        form is desired. Note that the specified plural form will override any existing plural
+   *        form for the specified {@link GroupVersionKind} (in particular, if the specified
+   *        {@link GroupVersionKind} was already an instance of GroupVersionKindPlural, its plural
+   *        form will only be considered in the new instance if the specified plural form is
+   *        {@code null}
+   * @return a new GroupVersionKindPlural derived from the specified {@link GroupVersionKind} and
+   *         plural form
+   */
+  public static GroupVersionKindPlural gvkWithPlural(GroupVersionKind gvk, String plural) {
+    return new GroupVersionKindPlural(gvk, plural);
   }
 
+  /**
+   * Creates a new GroupVersionKindPlural instance extracting the information from the specified
+   * {@link HasMetadata} implementation
+   *
+   * @param resourceClass the {@link HasMetadata} from which group, version, kind and plural form
+   *        are extracted
+   * @return a new GroupVersionKindPlural instance based on the specified {@link HasMetadata}
+   *         implementation
+   */
   public static GroupVersionKindPlural gvkFor(Class<? extends HasMetadata> resourceClass) {
-    return new GroupVersionKindPlural(HasMetadata.getGroup(resourceClass),
-        HasMetadata.getVersion(resourceClass), HasMetadata.getKind(resourceClass),
-        HasMetadata.getPlural(resourceClass));
+    final var gvk = GroupVersionKind.gvkFor(resourceClass);
+    return gvkWithPlural(gvk, HasMetadata.getPlural(resourceClass));
+  }
+
+  /**
+   * Retrieves the default plural form for the specified kind.
+   *
+   * @param kind the kind for which we want to get the default plural form
+   * @return the default plural form for the specified kind
+   */
+  public static String getDefaultPluralFor(String kind) {
+    // todo: replace by Fabric8 version when available, see
+    // https://github.com/fabric8io/kubernetes-client/pull/6314
+    return kind != null ? Pluralize.toPlural(kind.toLowerCase()) : null;
   }
 
   /**
@@ -46,13 +98,14 @@ public Optional<String> getPlural() {
 
   /**
    * Returns the plural form associated with the kind if it was provided or a default, computed form
-   * via {@link Pluralize#toPlural(String)} (which should correspond to the actual plural form in
+   * via {@link #getDefaultPluralFor(String)} (which should correspond to the actual plural form in
    * most cases but might not always be correct, especially if the resource's creator defined an
    * exotic plural form via the CRD.
    *
    * @return the plural form associated with the kind if provided or a default plural form otherwise
    */
+  @SuppressWarnings("unused")
   public String getPluralOrDefault() {
-    return getPlural().orElse(Pluralize.toPlural(getKind()));
+    return getPlural().orElse(getDefaultPluralFor(getKind()));
   }
 }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java

@@ -12,19 +12,48 @@ class GroupVersionKindTest {
 
   @Test
   void testInitFromApiVersion() {
-    var gvk = GroupVersionKindPlural.gvkFor("v1", "ConfigMap");
+    var gvk = new GroupVersionKind("v1", "ConfigMap");
     assertThat(gvk.getGroup()).isNull();
     assertThat(gvk.getVersion()).isEqualTo("v1");
 
-    gvk = GroupVersionKindPlural.gvkFor("apps/v1", "Deployment");
+    gvk = new GroupVersionKind("apps/v1", "Deployment");
     assertThat(gvk.getGroup()).isEqualTo("apps");
     assertThat(gvk.getVersion()).isEqualTo("v1");
   }
 
   @Test
   void pluralShouldOnlyBeProvidedIfExplicitlySet() {
-    final var gvk = GroupVersionKindPlural.gvkFor(ConfigMap.class);
+    final var kind = "ConfigMap";
+    var gvk = GroupVersionKindPlural.from(new GroupVersionKind("v1", kind));
+    assertThat(gvk.getPlural()).isEmpty();
+    assertThat(gvk.getPluralOrDefault())
+        .isEqualTo(GroupVersionKindPlural.getDefaultPluralFor(kind));
+
+    gvk = GroupVersionKindPlural.from(GroupVersionKind.gvkFor(ConfigMap.class));
+    assertThat(gvk.getPlural()).isEmpty();
+    assertThat(gvk.getPluralOrDefault()).isEqualTo(HasMetadata.getPlural(ConfigMap.class));
+
+    gvk = GroupVersionKindPlural.gvkFor(ConfigMap.class);
+    assertThat(gvk.getPlural()).hasValue(HasMetadata.getPlural(ConfigMap.class));
+
+    gvk = GroupVersionKindPlural.from(gvk);
     assertThat(gvk.getPlural()).hasValue(HasMetadata.getPlural(ConfigMap.class));
   }
 
+  @Test
+  void pluralShouldBeEmptyIfNotProvided() {
+    final var kind = "MyKind";
+    var gvk =
+        GroupVersionKindPlural.gvkWithPlural(new GroupVersionKind("josdk.io", "v1", kind), null);
+    assertThat(gvk.getPlural()).isEmpty();
+    assertThat(gvk.getPluralOrDefault())
+        .isEqualTo(GroupVersionKindPlural.getDefaultPluralFor(kind));
+  }
+
+  @Test
+  void pluralShouldOverrideDefaultComputedVersionIfProvided() {
+    var gvk = GroupVersionKindPlural.gvkWithPlural(new GroupVersionKind("josdk.io", "v1", "MyKind"),
+        "MyPlural");
+    assertThat(gvk.getPlural()).hasValue("MyPlural");
+  }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/generickubernetesresource/generickubernetesdependentresourcemanaged/ConfigMapGenericKubernetesDependent.java

@@ -4,7 +4,6 @@
 import java.io.InputStream;
 import java.util.Map;
 
-import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
@@ -25,7 +24,7 @@ public class ConfigMapGenericKubernetesDependent extends
   public static final String KEY = "key";
 
   public ConfigMapGenericKubernetesDependent() {
-    super(GroupVersionKind.gvkFor(ConfigMap.class));
+    super(new GroupVersionKind("", VERSION, KIND));
   }
 
   @Override

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/generickubernetesresource/generickubernetesdependentstandalone/ConfigMapGenericKubernetesDependent.java

@@ -4,7 +4,6 @@
 import java.io.InputStream;
 import java.util.Map;
 
-import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
@@ -25,7 +24,7 @@ public class ConfigMapGenericKubernetesDependent extends
   public static final String KEY = "key";
 
   public ConfigMapGenericKubernetesDependent() {
-    super(GroupVersionKind.gvkFor(ConfigMap.class));
+    super(new GroupVersionKind("", VERSION, KIND));
   }
 
   @Override