[java] fix comparator of Docker versions

[asolntsev]

Generally, people consider "numeric component > alphabetic component".

For example,
1.2.alpha < 1.2.beta < 1.2.3.alpha < 1.2.3

💥 What does this PR do?

fixes Version.java

Before this change, Version.java was thinking that:

• 1.2.3.4 > 1.2.4
• 1.2.beta > 1.2.3.alpha
• 1.2.3.alpha > 1.2.3

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Fix Docker Version comparator for numeric vs alphabetic segments
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Description

• Fix Version ordering so numeric segments sort higher than alphabetic segments.
• Correct isLessThan/isGreaterThan early-return logic to respect first differing segment.
• Add regression tests for differing segment counts and mixed numeric/alphabetic versions.

Diagram

graph TD
  A["Docker module"] --> B["Version"] --> C["compare()"] --> D{"Segment types?"}
  D -- "both numeric" --> E["toLong + Long.compare"]
  D -- "mixed" --> F["numeric > alpha"]
  D -- "both alpha" --> G["String.compareTo"]
  H["VersionTest"] --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt a standard version comparator (e.g., Maven ComparableVersion / semver lib)

• ➕ Mature handling of mixed numeric/alpha segments and edge cases
• ➕ Reduces custom comparison logic maintenance
• ➖ Introduces new dependency (or heavier utility) for a small internal need
• ➖ Docker version strings may not align with strict semver expectations

Recommendation: The PR’s approach is appropriate for Selenium’s lightweight internal Docker version handling: it keeps comparison logic local, clarifies numeric-vs-alpha precedence, and adds focused regression tests. A full-featured version library is a viable alternative but likely not worth the dependency/behavioral tradeoffs for this narrow use case.

Files changed (2) +56 / -44

Bug fix (1) +34 / -11

Version.javaImplement numeric>alphabetic segment ordering and fix relational comparisons +34/-11

Implement numeric>alphabetic segment ordering and fix relational comparisons

• Fixes isLessThan/isGreaterThan to return on the first non-zero segment comparison. Reworks compare() to classify segments as numeric vs alphabetic (treating missing segments as numeric 0) so numeric components sort after alphabetic components, and adds small helpers for digit-checking and parsing.

java/src/org/openqa/selenium/docker/Version.java

Tests (1) +22 / -33

VersionTest.javaAdd regression coverage for mixed numeric/alphabetic Docker version ordering +22/-33

Add regression coverage for mixed numeric/alphabetic Docker version ordering

• Extends the parameterized dataset to cover differing segment counts and mixed numeric/alphabetic cases (e.g., alpha/beta ordering and numeric segments outranking alphabetic). Improves assertions by checking symmetry (v1 vs v2 and v2 vs v1) and simplifies non-numeric tests into the shared parameterized suite.

java/test/org/openqa/selenium/docker/VersionTest.java

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. isLessThan missing Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality

Description

The modified public boolean isLessThan(Version other) and `public boolean isGreaterThan(Version
other)` methods have no Javadoc blocks, leaving their purpose and contract undocumented for callers.
This violates the requirement that all changed public methods include complete Javadoc with
@param/@return tags.

Code

java/src/org/openqa/selenium/docker/Version.java[R56-63]

+      int cmp = compare(segments, other.segments, i);
+      if (cmp < 0) {
+        return true;
+      }
+      if (cmp > 0) {
        return false;
      }
    }

Evidence

PR Compliance ID 330201 requires a Javadoc block with complete tags for each changed public method.
In Version.java, the updated isLessThan and isGreaterThan implementations appear without any
preceding /** ... */ Javadoc, so the other parameter and the boolean return value are not
documented, demonstrating non-compliance.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/docker/Version.java[52-66]
java/src/org/openqa/selenium/docker/Version.java[68-82]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`public boolean isLessThan(Version other)` and `public boolean isGreaterThan(Version other)` were modified in this PR but have no Javadoc. Compliance requires a Javadoc block immediately above changed public methods, including a purpose sentence plus `@param` and `@return` tags.

## Issue Context
This rule applies to changed `public` methods in non-test Java code.

## Fix Focus Areas
- java/src/org/openqa/selenium/docker/Version.java[52-66]
- java/src/org/openqa/selenium/docker/Version.java[68-82]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Uncaught parseLong overflow ✓ Resolved 🐞 Bug ☼ Reliability

Description

Version.compare now parses digit-only segments via Long.parseLong without guarding against overflow,
so large numeric components can throw NumberFormatException during comparisons.
VersionCommand.getDockerProtocol does not catch NumberFormatException, so this can break Docker API
version negotiation at runtime if Docker returns an unexpectedly large numeric
ApiVersion/MinAPIVersion segment.

Code

java/src/org/openqa/selenium/docker/Version.java[R92-115]

+    String mine = index < ours.length ? ours[index] : "";
+    String others = index < theirs.length ? theirs[index] : "";
+    boolean mineIsNumber = isNumber(mine) || mine.isEmpty();
+    boolean othersIsNumber = isNumber(others) || others.isEmpty();
+
+    if (mineIsNumber && othersIsNumber) {
+      return Long.compare(toLong(mine), toLong(others));
+    }
+    if (mineIsNumber) {
+      return 1;
    }
+    if (othersIsNumber) {
+      return -1;
+    }
+    return mine.compareTo(others);
+  }
+
+  private boolean isNumber(String value) {
+    return value.chars().allMatch(Character::isDigit);
+  }
+
+  private long toLong(String mine) {
+    return mine.isEmpty() ? 0L : parseLong(mine);
  }

Evidence

toLong calls parseLong without catching NumberFormatException, so an overflowing digit-only
segment will throw. VersionCommand.getDockerProtocol() compares versions and does not catch
NumberFormatException, so such an exception can propagate and break version negotiation.

java/src/org/openqa/selenium/docker/Version.java[88-115]
java/src/org/openqa/selenium/docker/VersionCommand.java[86-114]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Version.compare()` treats digit-only components as numbers and calls `Long.parseLong()` (via `toLong()`), but no longer catches `NumberFormatException`. If a numeric segment overflows `long` (or otherwise fails to parse), version comparison throws and can bubble up into Docker protocol selection.

### Issue Context
This class is used by `VersionCommand.getDockerProtocol()` to compare Docker API versions returned by `/version`. That method’s catch block does **not** include `NumberFormatException`, so a thrown parse exception can escape and fail protocol negotiation.

### Fix Focus Areas
- java/src/org/openqa/selenium/docker/Version.java[91-115]
- java/src/org/openqa/selenium/docker/VersionCommand.java[86-113]

### Suggested fix
In `compare()` when both segments are numeric:
- Either wrap `parseLong` in a try/catch and on `NumberFormatException` fall back to a safe numeric comparison that doesn’t overflow (e.g., compare by string length, then lexicographically), **or**
- Parse using `BigInteger` for numeric segments.

Add a unit test covering very large digit-only segments (bigger than `Long.MAX_VALUE`) to ensure comparisons do not throw and maintain consistent ordering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit b96bcab