Cleaned up patch version comparison to simplify code complexity

Author: ddsharpe

imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/AruPatch.java

@@ -1,11 +1,10 @@
-// Copyright (c) 2020, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2020, 2024, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package com.oracle.weblogic.imagetool.aru;
 
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.xml.xpath.XPathExpressionException;
@@ -22,11 +21,11 @@
  * Metadata for a patch, as defined by ARU.
  * Simple bean for holding metadata obtained from ARU for a given patch ID and version.
  */
-public class AruPatch implements Comparable<AruPatch> {
+public class AruPatch {
     private static final LoggingFacade logger = LoggingFactory.getLogger(AruPatch.class);
 
     private String patchId;
-    private Version version;
+    private String version;
     private String description;
     private String product;
     private String release;
@@ -51,15 +50,11 @@ public AruPatch patchId(String value) {
      * @return The string value of the version found in ARU.
      */
     public String version() {
-        if (version != null) {
-            return version.toString();
-        } else {
-            return null;
-        }
+        return version;
     }
 
     public AruPatch version(String value) {
-        version = new Version(value);
+        version = value;
         return this;
     }
 
@@ -294,27 +289,4 @@ private static AruPatch selectPatchOffline(List<AruPatch> patches, String provid
     public String toString() {
         return patchId + " - " + description;
     }
-
-    @Override
-    public int compareTo(AruPatch obj) {
-        return version.compareTo(obj.version);
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) {
-            return true;
-        }
-        if (o == null || getClass() != o.getClass()) {
-            return false;
-        }
-        AruPatch aruPatch = (AruPatch) o;
-        return Objects.equals(patchId, aruPatch.patchId) && Objects.equals(version, aruPatch.version)
-            && Objects.equals(release, aruPatch.release);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(patchId, version, release);
-    }
 }

imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/Version.java

@@ -1,4 +1,4 @@
-// Copyright (c) 2020, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2020, 2024, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package com.oracle.weblogic.imagetool.cachestore;
@@ -110,8 +110,11 @@ private static AruPatch getAruPatchOnline(String patchNumber, String providedVer
                 throw new VersionNotFoundException(patchNumber, providedVersion, patches);
             }
         } else {
-            // Select the newest (highest numbered) patch
-            selectedPatch = patches.stream().max(Comparator.naturalOrder()).orElse(null);
+            // Compare the ARU OPatch patches using the patch version field, like 12.2.1.4.0
+            Comparator<AruPatch> patchVersionComparator =
+                Comparator.comparing(AruPatch::version, Utils::compareVersionsNullsFirst);
+            // Select the newest (highest version) OPatch install/patch
+            selectedPatch = patches.stream().max(patchVersionComparator).orElse(null);
         }
         return selectedPatch;
     }

imagetool/src/main/java/com/oracle/weblogic/imagetool/cachestore/OPatchFile.java

@@ -1,4 +1,4 @@
-// Copyright (c) 2019, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2019, 2024, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package com.oracle.weblogic.imagetool.util;
@@ -182,18 +182,12 @@ private static HttpRequestRetryHandler retryHandler() {
                 // Do not retry if over max retries
                 return false;
             }
-            if (exception instanceof InterruptedIOException) {
-                // Timeout
-                return false;
-            }
-            if (exception instanceof UnknownHostException) {
-                // Unknown host
-                return false;
-            }
-            if (exception instanceof SSLException) {
-                // SSL handshake failed
+            if (exception instanceof InterruptedIOException   // Timeout
+                || exception instanceof UnknownHostException  // Unknown Host
+                || exception instanceof SSLException) {       // SSL handshake failed
                 return false;
             }
+
             HttpClientContext clientContext = HttpClientContext.adapt(context);
             HttpRequest request = clientContext.getRequest();
             // return true if it is okay to retry this request type

imagetool/src/main/java/com/oracle/weblogic/imagetool/util/HttpUtil.java

@@ -1,4 +1,4 @@
-// Copyright (c) 2019, 2021, Oracle and/or its affiliates.
+// Copyright (c) 2019, 2024, Oracle and/or its affiliates.
 // Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 package com.oracle.weblogic.imagetool.util;
@@ -303,6 +303,27 @@ public static void deleteFilesRecursively(String pathDir) throws IOException {
         logger.exiting();
     }
 
+    /**
+     * Compares two version strings.  A null-friendly comparator that considers null to be less than non-null.
+     * Any qualifiers are treated as older than the same version without
+     * a qualifier.  If both versions have qualifiers and are otherwise equal, they are compared using
+     * String.compareTo() to determine the result.
+     *
+     * @param thisVersion  - first version
+     * @param otherVersion  - second version
+     * @return returns 0 if the versions are equal, greater than zero if thisVersion is newer,
+     *     and less than zero if thisVersion is older.
+     */
+    public static int compareVersionsNullsFirst(String thisVersion, String otherVersion) {
+        if (isEmptyString(thisVersion)) {
+            return (isEmptyString(otherVersion)) ? 0 : -1;
+        } else if (isEmptyString(otherVersion)) {
+            return 1;
+        } else {
+            return compareVersions(thisVersion, otherVersion);
+        }
+    }
+
     /**
      * Compares two version strings.  Any qualifiers are treated as older than the same version without
      * a qualifier.  If both versions have qualifiers and are otherwise equal, they are compared using
@@ -314,8 +335,6 @@ public static void deleteFilesRecursively(String pathDir) throws IOException {
      *     and less than zero if thisVersion is older.
      */
     public static int compareVersions(String thisVersion, String otherVersion) {
-        int result = 0;
-
         if (isEmptyString(thisVersion) || isEmptyString(otherVersion)) {
             throw new IllegalArgumentException("cannot compare null strings");
         }
@@ -330,61 +349,60 @@ public static int compareVersions(String thisVersion, String otherVersion) {
 
         int fieldsToCompare = Math.min(thisVersionElements.length, otherVersionElements.length);
 
+        int result = 0;
         int idx;
         for (idx = 0; idx < fieldsToCompare; idx++) {
             int thisVersionNumber = Integer.parseInt(thisVersionElements[idx]);
             int otherVersionNumber = Integer.parseInt(otherVersionElements[idx]);
 
-            if (thisVersionNumber > otherVersionNumber) {
-                result = 1;
-                break;
-            } else if (thisVersionNumber < otherVersionNumber) {
-                result = -1;
-                break;
+            result = Integer.compare(thisVersionNumber, otherVersionNumber);
+            if (result != 0) {
+                return result;
             }
         }
 
         // Version fields compared so far are equal so check to see if one version number
         // has more fields than the other.
         //
-        if (result == 0 && thisVersionElements.length != otherVersionElements.length) {
+        if (thisVersionElements.length != otherVersionElements.length) {
             if (thisVersionElements.length > otherVersionElements.length) {
                 result = 1;
             } else {
                 result = -1;
             }
         }
 
+        if (result != 0) {
+            return result;
+        }
+
         // Finally, look to see if one or both versions have a qualifier if they are otherwise the same.
         //
-        if (result == 0) {
-            int useCase = 0;
-            if (thisVersion.indexOf('-') != -1) {
-                useCase += 1;
-            }
-            if (otherVersion.indexOf('-') != -1) {
-                useCase += 2;
-            }
-            switch (useCase) {
-                case 0:
-                    break;
+        int useCase = 0;
+        if (thisVersion.indexOf('-') != -1) {
+            useCase += 1;
+        }
+        if (otherVersion.indexOf('-') != -1) {
+            useCase += 2;
+        }
+        switch (useCase) {
+            case 1:
+                result = -1;
+                break;
 
-                case 1:
-                    result = -1;
-                    break;
+            case 2:
+                result = 1;
+                break;
 
-                case 2:
-                    result = 1;
-                    break;
+            case 3:
+                String thisQualifier = thisVersion.substring(thisVersion.indexOf('-'));
+                String otherQualifier = otherVersion.substring(otherVersion.indexOf('-'));
+                result = thisQualifier.compareTo(otherQualifier);
+                break;
 
-                case 3:
-                    String thisQualifier = thisVersion.substring(thisVersion.indexOf('-'));
-                    String otherQualifier = otherVersion.substring(otherVersion.indexOf('-'));
-                    result = thisQualifier.compareTo(otherQualifier);
-                    break;
-                default:
-                    break;
-            }
+            case 0:
+            default:
+                break;
         }
         return result;
     }