Resource version comparison utility (#2988)

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Co-authored-by: Steven Hawkins <shawkins@redhat.com>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/NonComparableResourceVersionException.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright Java Operator SDK Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.javaoperatorsdk.operator.api.reconciler;
+
+import io.javaoperatorsdk.operator.OperatorException;
+
+public class NonComparableResourceVersionException extends OperatorException {
+
+  public NonComparableResourceVersionException(String message) {
+    super(message);
+  }
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java

@@ -450,4 +450,58 @@ public static <P extends HasMetadata> P addFinalizerWithSSA(
           e);
     }
   }
+
+  public static int compareResourceVersions(String v1, String v2) {
+    var v1Length = v1.length();
+    if (v1Length == 0) {
+      throw new NonComparableResourceVersionException("Resource version (1) is empty");
+    }
+    var v2Length = v2.length();
+    if (v2Length == 0) {
+      throw new NonComparableResourceVersionException("Resource version (2) is empty");
+    }
+    var maxLength = Math.max(v1Length, v2Length);
+    boolean v1LeadingZero = true;
+    boolean v2LeadingZero = true;
+    int comparison = 0;
+    for (int i = 0; i < maxLength; i++) {
+      char char1 = 0;
+      if (i < v1Length) {
+        char1 = v1.charAt(i);
+        if (v1LeadingZero) {
+          if (char1 == '0') {
+            throw new NonComparableResourceVersionException(
+                "Resource version (1) cannot begin with 0");
+          }
+          v1LeadingZero = false;
+        }
+        if (!Character.isDigit(char1)) {
+          throw new NonComparableResourceVersionException(
+              "Non numeric characters in resource version (1): " + char1);
+        }
+      }
+      if (i < v2Length) {
+        var char2 = v2.charAt(i);
+        if (v2LeadingZero) {
+          if (char2 == '0') {
+            throw new NonComparableResourceVersionException(
+                "Resource version (2) cannot begin with 0");
+          }
+          v2LeadingZero = false;
+        }
+        if (!Character.isDigit(char2)) {
+          throw new NonComparableResourceVersionException(
+              "Non numeric characters in resource version (2): " + char2);
+        }
+        if (char1 == 0) {
+          comparison = -1;
+        } else if (comparison == 0) {
+          comparison = Character.compare(char1, char2);
+        }
+      } else {
+        comparison = 1;
+      }
+    }
+    return comparison;
+  }
 }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtilsTest.java

@@ -20,6 +20,8 @@
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
@@ -37,6 +39,7 @@
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
 import static io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils.DEFAULT_MAX_RETRY;
+import static io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils.compareResourceVersions;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
@@ -47,6 +50,8 @@
 
 class PrimaryUpdateAndCacheUtilsTest {
 
+  private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtilsTest.class);
+
   Context<TestCustomResource> context = mock(Context.class);
   KubernetesClient client = mock(KubernetesClient.class);
   Resource resource = mock(Resource.class);
@@ -176,4 +181,52 @@ void cachePollTimeouts() {
                     10L));
     assertThat(ex.getMessage()).contains("Timeout");
   }
+
+  @Test
+  public void compareResourceVersionsTest() {
+    assertThat(compareResourceVersions("11", "22")).isNegative();
+    assertThat(compareResourceVersions("22", "11")).isPositive();
+    assertThat(compareResourceVersions("1", "1")).isZero();
+    assertThat(compareResourceVersions("11", "11")).isZero();
+    assertThat(compareResourceVersions("123", "2")).isPositive();
+    assertThat(compareResourceVersions("3", "211")).isNegative();
+
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("aa", "22"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("11", "ba"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("", "22"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("11", ""));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("01", "123"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("123", "01"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("3213", "123a"));
+    assertThrows(
+        NonComparableResourceVersionException.class, () -> compareResourceVersions("321", "123a"));
+  }
+
+  // naive performance test that compares the work case scenario for the parsing and non-parsing
+  // variants
+  @Test
+  public void compareResourcePerformanceTest() {
+    var execNum = 30000000;
+    var startTime = System.currentTimeMillis();
+    for (int i = 0; i < execNum; i++) {
+      var res = compareResourceVersions("123456788", "123456789");
+    }
+    var dur1 = System.currentTimeMillis() - startTime;
+    log.info("Duration without parsing: {}", dur1);
+    startTime = System.currentTimeMillis();
+    for (int i = 0; i < execNum; i++) {
+      var res = Long.parseLong("123456788") > Long.parseLong("123456789");
+    }
+    var dur2 = System.currentTimeMillis() - startTime;
+    log.info("Duration with parsing:   {}", dur2);
+
+    assertThat(dur1).isLessThan(dur2);
+  }
 }