simplify implementation of 'isNewer'

Author: russgold

operator/src/main/java/oracle/kubernetes/operator/helpers/KubernetesUtils.java

@@ -6,7 +6,6 @@
 
 import io.kubernetes.client.models.V1ObjectMeta;
 import java.lang.reflect.Field;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import javax.json.JsonPatchBuilder;
@@ -26,45 +25,6 @@ static <K, V> boolean mapEquals(Map<K, V> first, Map<K, V> second) {
     return Objects.equals(first, second) || (MapUtils.isEmpty(first) && MapUtils.isEmpty(second));
   }
 
-  /**
-   * Returns true if the labels on the current artifact metadata match those on the build version.
-   * This excludes any weblogic-specific labels, as identified by the existence of the weblogic.
-   * prefix.
-   *
-   * @param build the desired version of the metadata
-   * @param current the current version of the metadata
-   * @return true if the labels match
-   */
-  static boolean areLabelsValid(V1ObjectMeta build, V1ObjectMeta current) {
-    return mapEquals(getCustomerLabels(current), getCustomerLabels(build));
-  }
-
-  private static Map<String, String> getCustomerLabels(V1ObjectMeta metadata) {
-    Map<String, String> result = new HashMap<>();
-    for (Map.Entry<String, String> entry : metadata.getLabels().entrySet()) {
-      if (!isOperatorLabel(entry)) {
-        result.put(entry.getKey(), entry.getValue());
-      }
-    }
-    return result;
-  }
-
-  private static boolean isOperatorLabel(Map.Entry<String, String> label) {
-    return label.getKey().startsWith("weblogic.");
-  }
-
-  /**
-   * Returns true if the annotations on the current artifact metadata match those on the build
-   * version.
-   *
-   * @param build the desired version of the metadata
-   * @param current the current version of the metadata
-   * @return true if the annotations match
-   */
-  static boolean areAnnotationsValid(V1ObjectMeta build, V1ObjectMeta current) {
-    return mapEquals(current.getAnnotations(), build.getAnnotations());
-  }
-
   /**
    * Returns true if the current map is missing values from the required map. This method is
    * typically used to compare labels and annotations against specifications derived from the
@@ -100,7 +60,7 @@ private static boolean hasAllRequiredNames(Map<String, ?> current, Map<String, ?
    * @param current a map of the values found in a Kubernetes resource
    * @param required a map of the values specified for the resource by the domain
    */
-  public static void addPatches(
+  static void addPatches(
       JsonPatchBuilder patchBuilder,
       String basePath,
       Map<String, String> current,
@@ -144,7 +104,9 @@ static V1ObjectMeta getResourceMetadata(Object resource) {
   }
 
   /**
-   * Returns true if the first metadata indicates a newer resource than does the second.
+   * Returns true if the first metadata indicates a newer resource than does the second. 'Newer'
+   * indicates that the creation time is later. If two items have the same creation time, a higher
+   * resource version indicates the newer resource.
    *
    * @param first the first item to compare
    * @param second the second item to compare
@@ -157,11 +119,16 @@ public static boolean isFirstNewer(V1ObjectMeta first, V1ObjectMeta second) {
     DateTime time1 = first.getCreationTimestamp();
     DateTime time2 = second.getCreationTimestamp();
 
-    if (time2.isAfter(time1)) {
+    if (time1.isAfter(time2)) {
+      return true;
+    } else if (time2.isAfter(time1)) {
       return false;
+    } else {
+      return getResourceVersion(first) > getResourceVersion(second);
     }
-    return Integer.parseInt(second.getResourceVersion())
-            < Integer.parseInt(first.getResourceVersion())
-        || time2.isBefore(time1);
+  }
+
+  private static int getResourceVersion(V1ObjectMeta metadata) {
+    return Integer.parseInt(metadata.getResourceVersion());
   }
 }

operator/src/test/java/oracle/kubernetes/operator/helpers/KubernetesUtilsTest.java

@@ -0,0 +1,42 @@
+// Copyright 2019, Oracle Corporation and/or its affiliates.  All rights reserved.
+// Licensed under the Universal Permissive License v 1.0 as shown at
+// http://oss.oracle.com/licenses/upl.
+
+package oracle.kubernetes.operator.helpers;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.*;
+
+import io.kubernetes.client.models.V1ObjectMeta;
+import org.joda.time.DateTime;
+import org.junit.Test;
+
+public class KubernetesUtilsTest {
+
+  @Test
+  public void whenCreationTimesDiffer_metadataWithLaterTimeIsNewer() {
+    V1ObjectMeta meta1 = new V1ObjectMeta().creationTimestamp(new DateTime(2)).resourceVersion("2");
+    V1ObjectMeta meta2 = new V1ObjectMeta().creationTimestamp(new DateTime(1)).resourceVersion("1");
+
+    assertThat(KubernetesUtils.isFirstNewer(meta1, meta2), is(true));
+    assertThat(KubernetesUtils.isFirstNewer(meta2, meta1), is(false));
+  }
+
+  @Test
+  public void whenCreationTimesMatch_metadataWithHigherResourceVersionIsNewer() {
+    V1ObjectMeta meta1 = new V1ObjectMeta().creationTimestamp(new DateTime(1)).resourceVersion("2");
+    V1ObjectMeta meta2 = new V1ObjectMeta().creationTimestamp(new DateTime(1)).resourceVersion("1");
+
+    assertThat(KubernetesUtils.isFirstNewer(meta1, meta2), is(true));
+    assertThat(KubernetesUtils.isFirstNewer(meta2, meta1), is(false));
+  }
+
+  @Test
+  public void whenCreationTimesAndResourceVersionsMatch_neitherIsNewer() {
+    V1ObjectMeta meta1 = new V1ObjectMeta().creationTimestamp(new DateTime(1)).resourceVersion("2");
+    V1ObjectMeta meta2 = new V1ObjectMeta().creationTimestamp(new DateTime(1)).resourceVersion("2");
+
+    assertThat(KubernetesUtils.isFirstNewer(meta2, meta1), is(false));
+    assertThat(KubernetesUtils.isFirstNewer(meta1, meta2), is(false));
+  }
+}