Skip to content

fix: Order milestone and release candidate versions in VersionComparator#15696

Open
jamesfredley wants to merge 6 commits into
7.0.xfrom
fix/14058-plugin-compatibility-milestone-snapshot
Open

fix: Order milestone and release candidate versions in VersionComparator#15696
jamesfredley wants to merge 6 commits into
7.0.xfrom
fix/14058-plugin-compatibility-milestone-snapshot

Conversation

@jamesfredley

@jamesfredley jamesfredley commented May 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #14058

Summary

grails.plugins.VersionComparator did not understand milestone (M) or release candidate (RC) version qualifiers. The plugin compatibility check in DefaultGrailsPluginManager delegates to this comparator, so plugins targeting (or applications running on) a milestone or release candidate produced incorrect "may not be compatible with this application" warnings. This PR makes the comparator qualifier-aware and adds the missing test coverage requested by the issue.

The bug

VersionComparator tokenised a version by splitting on . and keeping only purely numeric tokens (==~ /\d+/). -SNAPSHOT / .BUILD-SNAPSHOT were handled as a special case, but M/RC qualifiers were not, so the qualifier - and often the patch number - was silently discarded:

Input Parsed as (before) Problem
7.0.0-M1 [7, 0] patch 0 and -M1 both dropped
7.0.5-M1 [7, 0] patch 5 lost; compares as 7.0
7.0.0.M1 [7, 0, 0] M1 dropped; compares equal to final 7.0.0

This led to two concrete defects in the compatibility check:

  1. Lost patch number. A plugin requiring 7.0.3 > * running on 7.0.5-M1 was wrongly flagged incompatible, because 7.0.5-M1 was parsed as 7.0 (< 7.0.3).
  2. Milestones/RCs of a base version all compared equal, and equal to the final release. A plugin requiring the final 7.0.0 was treated as compatible with the pre-release 7.0.0-RC1, and milestone ordering carried no meaning.

Why this looked like it already had coverage

The codebase has three independent version representations. The well-tested milestone/RC ordering lives on classes the compatibility check never calls:

Class Handles M/RC? Tested for M/RC? Used by the compatibility check?
grails.plugins.VersionComparator No (fixed here) No (added here) Yes
org.grails.datastore.mapping.core.grailsversion.GrailsVersion / Snapshot Yes Yes No
grails.init.GrailsVersion (wrapper) Yes Yes No

The fix

VersionComparator now parses a version into its numeric components plus an optional qualifier:

  • Numeric components are compared first, zero padding the shorter side so 7.0 == 7.0.0 and 7.0.1 > 7.0.0-RC9.

  • When the numeric components are equal, the qualifier breaks the tie using the same ordering as GrailsVersion / Snapshot:

    7.0.0-M1 < 7.0.0-M2 < 7.0.0-RC1 < 7.0.0-RC2 < 7.0.0-SNAPSHOT < 7.0.0
    
  • Dotted (7.0.0.M1) and hyphenated (7.0.0-M1) forms are treated as equivalent, and qualifier matching is case insensitive (7.0.0-rc3 == 7.0.0-RC3).

  • Milestone and release candidate numbers are compared numerically, so 7.0.0-RC10 > 7.0.0-RC2.

  • Unrecognised qualifiers are still treated as a final release, preserving the previous "ignore unknown suffix" behaviour.

The protected deSnapshot / isSnapshot methods are retained for backwards compatibility.

Malformed-version errors preserved

VersionComparator is used by the plugin loading code, which expects an explicit InvalidVersionException (not a raw NumberFormatException) when a version is malformed. compare() wraps the per-side parsing in try/catch and rethrows the original side-aware message:

Cannot compare versions, left side [..] is invalid: ..
Cannot compare versions, right side [..] is invalid: ..

The milestone/release-candidate qualifier number is parsed eagerly inside parse(), so a qualifier-number overflow (for example 7.0.0-RC99999999999999999) is reported through the same InvalidVersionException rather than escaping as a raw NumberFormatException. Unrecognised qualifiers are treated as a final release and never trigger qualifier-number parsing.

Behaviour preserved

All pre-existing VersionComparatorSpec and DefaultGrailsPluginManagerSpec cases are unchanged and still pass (numeric comparisons, BUILD-SNAPSHOT < release, the existing range checks, etc.).

Blast radius

VersionComparator has four callers, all verified safe:

  • GrailsVersionUtils.isValidVersion - already strips qualifiers via trimTag() before comparing, so its behaviour is unchanged.
  • GrailsVersionUtils.isVersionGreaterThan - no callers.
  • RegexUrlMapping - compares user-defined API versions (plain numeric, no M/RC).
  • DefaultGrailsPluginManager.isCompatiblePlugin - the path this PR fixes.

Tests

  • VersionComparatorSpec: pre-release vs final ordering, patch preservation (7.0.5-M1 > 7.0.0), milestone/RC numeric ordering (7.0.0-RC10 > 7.0.0-RC2), the full tier order, dotted/hyphenated and snapshot-form equivalence, numeric-base dominance, a sort assertion over a mixed list, plus explicit InvalidVersionException assertions (stable message prefix) for numeric-component and milestone/RC qualifier-number overflow on both the left and right sides.
  • DefaultGrailsPluginManagerSpec: isCompatiblePlugin rows for milestone, release candidate and snapshot versions on both the application and the plugin.

Verification

  • ./gradlew :grails-core:test --tests "grails.plugins.VersionComparatorSpec" --tests "grails.plugins.DefaultGrailsPluginManagerSpec" - passing.
  • ./gradlew :grails-bootstrap:codeStyle :grails-core:codeStyle - passing.

Future work

The milestone/RC/snapshot ordering is intentionally re-implemented here rather than reused from GrailsVersion/Snapshot, because grails-bootstrap cannot reasonably depend on grails-datastore-core and GrailsVersion is stricter than this comparator needs (it rejects 2-part versions such as 4.0). Consolidating these into a single shared implementation is tracked in #15695.

VersionComparator discarded any non-numeric version token, so milestone
(M) and release candidate (RC) qualifiers were ignored and the patch
number was lost for hyphenated forms (e.g. 7.0.5-M1 was parsed as 7.0).
As a result the plugin compatibility check in DefaultGrailsPluginManager
emitted incorrect "may not be compatible" warnings when an application
or plugin targeted a milestone or release candidate of Grails.

Versions are now parsed into their numeric components plus an optional
qualifier. The numeric components are compared first (zero padded), then
the qualifier breaks ties following the same ordering as GrailsVersion
and Snapshot: M < RC < SNAPSHOT < final release. The dotted (7.0.0.M1)
and hyphenated (7.0.0-M1) qualifier forms are treated as equivalent and
matching is case insensitive. Unrecognised qualifiers continue to be
treated as a final release to preserve backwards compatible behaviour.

Adds coverage to VersionComparatorSpec for the new ordering and to
DefaultGrailsPluginManagerSpec for milestone, release candidate and
snapshot compatibility checks.

Fixes #14058

Assisted-by: claude-code:claude-4.8-opus

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes plugin compatibility version comparisons by making grails.plugins.VersionComparator understand milestone (M) and release candidate (RC) qualifiers (plus snapshot ordering), aligning pre-release ordering with existing Grails version semantics and adding regression test coverage for the compatibility check path.

Changes:

  • Update VersionComparator parsing/comparison to preserve numeric parts (including patch) and apply qualifier tier ordering (M < RC < SNAPSHOT < final).
  • Expand VersionComparatorSpec to cover milestone/RC/snapshot ordering, numeric-suffix ordering (e.g., RC10 > RC2), and mixed-version sorting.
  • Expand DefaultGrailsPluginManagerSpec to cover isCompatiblePlugin behavior for milestone/RC/snapshot app/plugin versions (issue #14058).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
grails-bootstrap/src/main/groovy/grails/plugins/VersionComparator.groovy Implements qualifier-aware parsing and ordering for version comparison used by plugin compatibility checks.
grails-core/src/test/groovy/grails/plugins/VersionComparatorSpec.groovy Adds comparator regression tests for pre-release qualifiers and sorting behavior.
grails-core/src/test/groovy/grails/plugins/DefaultGrailsPluginManagerSpec.groovy Adds compatibility-check rows covering milestone/RC/snapshot versions to prevent regressions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread grails-bootstrap/src/main/groovy/grails/plugins/VersionComparator.groovy Outdated
Comment thread grails-bootstrap/src/main/groovy/grails/plugins/VersionComparator.groovy Outdated
…dering docs

Addresses review feedback on the milestone/release candidate ordering change:

- compareQualifiers now only compares trailing qualifier numbers for the
  milestone and release candidate tiers. Unrecognised qualifiers stay equal
  to the final release, so e.g. 7.0.0-FOO2 no longer sorts newer than 7.0.0.
- Corrects the VersionComparator class documentation: it orders versions
  from oldest to newest, matching the Comparator contract and the spec.
- Adds VersionComparatorSpec cases asserting that unrecognised qualifiers,
  including ones with a trailing number, compare equal to the final release.

Assisted-by: claude-code:claude-4.8-opus

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VersionComparator is used by plugin loading code and we need to continue to have explicit error messages if a version is malformed. It looks like we removed the InvalidVersionException instead.

…nComparator

The qualifier-aware rewrite of VersionComparator replaced the guarded integer
parsing with unguarded Integer.parseInt calls, so malformed or overflowing
version numbers leaked a raw NumberFormatException instead of the explicit
InvalidVersionException that the plugin loading code relies on.

Wrap the per-side parse calls in compare to rethrow NumberFormatException as
InvalidVersionException with the original left/right context message, and parse
the milestone/release-candidate qualifier number eagerly inside parse so a
qualifier-number overflow is reported the same way. Unrecognised qualifiers are
still treated as a final release and never trigger qualifier-number parsing.

Add regression tests covering numeric-component and qualifier-number overflow on
both the left and right sides.

Assisted-by: claude-code:claude-4.8-opus
@jamesfredley

Copy link
Copy Markdown
Contributor Author

Thanks @jdaugherty - good catch. The rewrite did drop the InvalidVersionException: the new parse() called Integer.parseInt() unguarded, so a malformed or overflowing version leaked a raw NumberFormatException instead of the explicit, side-aware diagnostic the plugin loading code expects.

Restored in cd75a0f:

  • compare() now wraps the per-side parse() calls in try/catch (NumberFormatException) and rethrows the original InvalidVersionException messages (Cannot compare versions, left side [..] is invalid: .. / right side), exactly as before.
  • The milestone/RC qualifier number is now parsed eagerly inside parse(), so it runs inside the same per-side try/catch. Previously the only remaining parseInt for M/RC suffixes ran later in compareQualifiers(), outside any guard, and could still leak a raw NumberFormatException (for example 7.0.0-RC99999999999999999). Unrecognised qualifiers are still treated as a final release and never trigger qualifier-number parsing.
  • Added VersionComparatorSpec regression tests asserting the explicit exception and the stable message prefix for both numeric-component and qualifier-number overflow, on the left and right sides.

./gradlew :grails-bootstrap:codeStyle :grails-core:codeStyle and the full VersionComparatorSpec / DefaultGrailsPluginManagerSpec suites pass.

@jamesfredley jamesfredley requested a review from jdaugherty June 3, 2026 21:49
@testlens-app

testlens-app Bot commented Jun 3, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 6f0861b
▶️ Tests: 34740 executed
⚪️ Checks: 33/33 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Test Plugin Compatibility Check with Milestone and Snapshot Versions

3 participants