refactor: minor clean-ups

Author: metacosm

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/informers/cache/Cache.java

@@ -31,17 +31,17 @@
 public interface Cache<T> extends Indexer<T> {
 
   // NAMESPACE_INDEX is the default index function for caching objects
-  public static final String NAMESPACE_INDEX = "namespace";
+  String NAMESPACE_INDEX = "namespace";
 
   /**
-   * It's is a convenient default KeyFunc which know show to make keys for API
+   * A convenient default KeyFunc which knows how to make keys for API
    * objects which implement HasMetadata interface. The key uses the format
    * namespace/name unless namespace is empty, then it's just name
    *
    * @param obj specific object
    * @return the key
    */
-  public static String metaNamespaceKeyFunc(HasMetadata obj) {
+  static String metaNamespaceKeyFunc(HasMetadata obj) {
     if (obj == null) {
       return "";
     }
@@ -53,7 +53,7 @@ public static String metaNamespaceKeyFunc(HasMetadata obj) {
     return namespaceKeyFunc(metadata.getNamespace(), metadata.getName());
   }
 
-  public static String metaUidKeyFunc(HasMetadata obj) {
+  static String metaUidKeyFunc(HasMetadata obj) {
     if (obj == null || obj.getMetadata() == null) {
       return "";
     }
@@ -66,21 +66,22 @@ public static String metaUidKeyFunc(HasMetadata obj) {
    *
    * @see #metaNamespaceKeyFunc
    */
-  public static String namespaceKeyFunc(String objectNamespace, String objectName) {
+  static String namespaceKeyFunc(String objectNamespace, String objectName) {
     if (Utils.isNullOrEmpty(objectNamespace)) {
       return objectName;
     }
     return objectNamespace + "/" + objectName;
   }
 
   /**
-   * It is a default index function that indexes based on an object's namespace
+   * Default index function that indexes based on an object's namespace
    *
    * @param obj the specific object
    * @return the indexed value
    */
-  public static List<String> metaNamespaceIndexFunc(HasMetadata obj) {
-    return Optional.ofNullable(obj).map(HasMetadata::getMetadata)
-        .map(metadata -> Collections.singletonList(metadata.getNamespace())).orElse(Collections.emptyList());
+  static List<String> metaNamespaceIndexFunc(HasMetadata obj) {
+    return Optional.ofNullable(obj)
+        .map(hm -> List.of(hm.getMetadata().getNamespace()))
+        .orElse(Collections.emptyList());
   }
 }

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/informers/cache/ItemStore.java

@@ -25,10 +25,10 @@
  * all entries until they are deleted. At its simplest this is just a map coupled with a key function.
  * <p>
  * Modifications to this store once the informer is running, by anything other than the informer will alter the event stream. If
- * for example an item is not found, an subsequent update from the api version will send notifications to
+ * for example an item is not found, any subsequent update from the api version will send notifications to
  * {@link ResourceEventHandler}s as an add.
  * <p>
- * Direct modifications to this store by anything other than the informer will not updated indexes nor emit events.
+ * Direct modifications to this store by anything other than the informer will not update indexes nor emit events.
  * <p>
  * The implementation should be safe with respect to concurrency. Modifications from the informer
  * will be single threaded, but not necessarily the same thread. Reads may be concurrent with writes.
@@ -60,8 +60,6 @@ public interface ItemStore<V> {
    * <br>
    * If false, then the initial add events must be processed as they
    * occur - meaning that the store state may not be complete.
-   *
-   * @return
    */
   default boolean isFullState() {
     return true;

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/informers/cache/ReducedStateItemStore.java

@@ -37,11 +37,12 @@
 public class ReducedStateItemStore<V extends HasMetadata> implements ItemStore<V> {
 
   private static final String METADATA = "metadata";
+  private static final Object[] NOT_FOUND_KEY_VALUE = new Object[1];
   private final ConcurrentHashMap<String, Object[]> store = new ConcurrentHashMap<>();
   private final List<String[]> fields = new ArrayList<>();
   private final Class<V> typeClass;
   private final KeyState keyState;
-  private KubernetesSerialization serialization;
+  private final KubernetesSerialization serialization;
 
   public static class KeyState {
 
@@ -64,17 +65,20 @@ public KeyState(Function<HasMetadata, String> keyFunction, Function<String, Stri
 
   }
 
+  private static final String NAMESPACE = "namespace";
+  private static final String NAME = "name";
   public static final KeyState NAME_KEY_STATE = new KeyState(Cache::metaNamespaceKeyFunc,
       k -> {
-        int index = k.indexOf("/");
+        int index = k.indexOf('/');
         if (index == -1) {
           return new String[] { null, k };
         }
         return new String[] { k.substring(0, index), k.substring(index + 1) };
-      }, new String[] { METADATA, "namespace" }, new String[] { METADATA, "name" });
+      }, new String[] { METADATA, NAMESPACE }, new String[] { METADATA, NAME });
 
+  private static final String UID = "uid";
   public static final KeyState UID_KEY_STATE = new KeyState(Cache::metaUidKeyFunc,
-      k -> new String[] { k }, new String[] { METADATA, "uid" });
+      k -> new String[] { k }, new String[] { METADATA, UID });
 
   /**
    * Create a state store with only the fields specified.
@@ -106,14 +110,15 @@ public ReducedStateItemStore(KeyState keyState, Class<V> typeClass, KubernetesSe
     this.keyState = keyState;
     fields.add(new String[] { METADATA, "resourceVersion" });
     if (valueFields != null) {
-      for (int i = 0; i < valueFields.length; i++) {
-        fields.add(valueFields[i].split("\\."));
+      for (String valueField : valueFields) {
+        fields.add(valueField.split("\\."));
       }
     }
     this.typeClass = typeClass;
     this.serialization = serialization;
   }
 
+  @SuppressWarnings("unchecked")
   Object[] store(V value) {
     if (value == null) {
       return null;
@@ -134,6 +139,7 @@ V restore(String key, Object[] values) {
     return serialization.convertValue(raw, typeClass);
   }
 
+  @SuppressWarnings("unchecked")
   private static void applyFields(Object[] values, Map<String, Object> raw, List<String[]> fields) {
     for (int i = 0; i < fields.size(); i++) {
       Object value = values[i];
@@ -175,7 +181,7 @@ public V get(String key) {
   }
 
   public String getResourceVersion(String key) {
-    return (String) store.getOrDefault(key, new Object[1])[0];
+    return (String) store.getOrDefault(key, NOT_FOUND_KEY_VALUE)[0];
   }
 
   @Override

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/informers/cache/ReducedStateItemStoreTest.java

@@ -34,25 +34,27 @@ void testStoreRestore() {
     Pod pod = new PodBuilder().withNewSpec().endSpec().withNewMetadata().withUid("x").withName("y").addToLabels("one", "1")
         .addToLabels("two", "2").withResourceVersion("2").endMetadata().withNewStatus().endStatus().build();
 
+    final var uid = pod.getMetadata().getUid();
+
     Object[] values = store.store(pod);
 
     assertEquals(3, values.length);
     assertEquals("2", values[0]); // always the resourceVersion
     assertEquals(pod.getMetadata().getLabels(), values[1]);
     assertNull(values[2]);
 
-    Pod restored = store.restore("x", values);
+    Pod restored = store.restore(uid, values);
 
     assertNull(restored.getSpec());
     assertNull(restored.getStatus());
-    assertEquals("x", restored.getMetadata().getUid());
+    assertEquals(uid, restored.getMetadata().getUid());
     assertEquals(pod.getMetadata().getLabels(), restored.getMetadata().getLabels());
 
-    assertNull(store.put("x", pod));
-    assertNotNull(store.get("x"));
-    assertEquals("2", store.getResourceVersion("x"));
+    assertNull(store.put(uid, pod));
+    assertNotNull(store.get(uid));
+    assertEquals("2", store.getResourceVersion(uid));
     assertEquals(1, store.size());
-    assertNotNull(store.remove("x"));
+    assertNotNull(store.remove(uid));
   }
 
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/informers/impl/cache/CacheImpl.java

@@ -16,11 +16,9 @@
 package io.fabric8.kubernetes.client.informers.impl.cache;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.fabric8.kubernetes.api.model.ObjectMeta;
 import io.fabric8.kubernetes.client.informers.cache.BasicItemStore;
 import io.fabric8.kubernetes.client.informers.cache.Cache;
 import io.fabric8.kubernetes.client.informers.cache.ItemStore;
-import io.fabric8.kubernetes.client.utils.Utils;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -45,36 +43,14 @@
  */
 public class CacheImpl<T extends HasMetadata> implements Cache<T> {
 
-  private static class Index {
-    private Map<Object, Set<String>> values = new ConcurrentHashMap<Object, Set<String>>();
-
-    public void update(String indexKey, String key, boolean remove) {
-      if (remove) {
-        values.computeIfPresent(indexKey == null ? this : indexKey, (k, v) -> {
-          v.remove(key);
-          return v.isEmpty() ? null : v;
-        });
-      } else {
-        values.computeIfAbsent(indexKey == null ? this : indexKey, k -> ConcurrentHashMap.newKeySet()).add(key);
-      }
-    }
-
-    public Set<String> get(String indexKey) {
-      return values.getOrDefault(indexKey == null ? this : indexKey, Collections.emptySet());
-    }
-  }
-
   // NAMESPACE_INDEX is the default index function for caching objects
   public static final String NAMESPACE_INDEX = "namespace";
-
   // indexers stores index functions by their names
   private final Map<String, Function<T, List<String>>> indexers = Collections.synchronizedMap(new HashMap<>());
-
-  // items stores object instances
-  private ItemStore<T> items;
-
   // indices stores objects' key by their indices
   private final ConcurrentMap<String, Index> indices = new ConcurrentHashMap<>();
+  // items stores object instances
+  private ItemStore<T> items;
 
   public CacheImpl() {
     this(NAMESPACE_INDEX, Cache::metaNamespaceIndexFunc, Cache::metaNamespaceKeyFunc);
@@ -85,6 +61,30 @@ public CacheImpl(String indexName, Function<T, List<String>> indexFunc, Function
     addIndexFunc(indexName, indexFunc);
   }
 
+  /**
+   * @deprecated use {@link Cache#metaNamespaceKeyFunc(HasMetadata)} instead
+   */
+  public static String metaNamespaceKeyFunc(Object obj) {
+    if (obj == null) {
+      return "";
+    }
+    return Cache.metaNamespaceKeyFunc((HasMetadata) obj);
+  }
+
+  /**
+   * @deprecated Use {@link Cache#namespaceKeyFunc(String, String)} instead
+   */
+  public static String namespaceKeyFunc(String objectNamespace, String objectName) {
+    return Cache.namespaceKeyFunc(objectNamespace, objectName);
+  }
+
+  /**
+   * @deprecated Use {@link Cache#metaNamespaceKeyFunc(HasMetadata)} instead
+   */
+  public static List<String> metaNamespaceIndexFunc(Object obj) {
+    return Cache.metaNamespaceIndexFunc((HasMetadata) obj);
+  }
+
   public void setItemStore(ItemStore<T> items) {
     this.items = items;
   }
@@ -263,7 +263,6 @@ public List<T> byIndex(String indexName, String indexKey) {
    * UpdateIndices modifies the objects location in the managed indexes, if there is
    * an update, you must provide an oldObj
    *
-   *
    * @param oldObj old object
    * @param newObj new object
    * @param key the key
@@ -312,63 +311,6 @@ public synchronized CacheImpl<T> addIndexFunc(String indexName, Function<T, List
     return this;
   }
 
-  /**
-   * It's is a convenient default KeyFunc which know show to make keys for API
-   * objects which implement HasMetadata interface. The key uses the format
-   * namespace/name unless namespace is empty, then it's just name
-   *
-   * @param obj specific object
-   * @return the key
-   */
-  public static String metaNamespaceKeyFunc(Object obj) {
-    if (obj == null) {
-      return "";
-    }
-    ObjectMeta metadata = null;
-    if (obj instanceof String) {
-      return (String) obj;
-    } else if (obj instanceof ObjectMeta) {
-      metadata = (ObjectMeta) obj;
-    } else if (obj instanceof HasMetadata) {
-      metadata = ((HasMetadata) obj).getMetadata();
-    }
-    if (metadata == null) {
-      throw new RuntimeException("Object is bad :" + obj);
-    }
-
-    return namespaceKeyFunc(metadata.getNamespace(), metadata.getName());
-  }
-
-  /**
-   * Default index function that indexes based on an object's namespace and name.
-   *
-   * @see #metaNamespaceKeyFunc
-   */
-  public static String namespaceKeyFunc(String objectNamespace, String objectName) {
-    if (Utils.isNullOrEmpty(objectNamespace)) {
-      return objectName;
-    }
-    return objectNamespace + "/" + objectName;
-  }
-
-  /**
-   * It is a default index function that indexes based on an object's namespace
-   *
-   * @param obj the specific object
-   * @return the indexed value
-   */
-  public static List<String> metaNamespaceIndexFunc(Object obj) {
-    final ObjectMeta metadata;
-    if (obj instanceof HasMetadata) {
-      metadata = ((HasMetadata) obj).getMetadata();
-    } else if (obj instanceof ObjectMeta) {
-      metadata = (ObjectMeta) obj;
-    } else {
-      metadata = null;
-    }
-    return metadata == null ? Collections.emptyList() : Collections.singletonList(metadata.getNamespace());
-  }
-
   @Override
   public synchronized void removeIndexer(String name) {
     this.indices.remove(name);
@@ -383,4 +325,23 @@ public Object getLockObject() {
     return this;
   }
 
+  private static class Index {
+    private final Map<Object, Set<String>> values = new ConcurrentHashMap<>();
+
+    public void update(String indexKey, String key, boolean remove) {
+      if (remove) {
+        values.computeIfPresent(indexKey == null ? this : indexKey, (k, v) -> {
+          v.remove(key);
+          return v.isEmpty() ? null : v;
+        });
+      } else {
+        values.computeIfAbsent(indexKey == null ? this : indexKey, k -> ConcurrentHashMap.newKeySet()).add(key);
+      }
+    }
+
+    public Set<String> get(String indexKey) {
+      return values.getOrDefault(indexKey == null ? this : indexKey, Collections.emptySet());
+    }
+  }
+
 }