jamesfredley opened a new pull request, #15693:
URL: https://github.com/apache/grails-core/pull/15693
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.
## 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, plus a sort assertion over a mixed list.
- `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:codenarcMain` - passing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
