feat(updaters): add Maven pom.xml updater with property indirection - #596
Merged
Conversation
Reviewer's GuideAdds a Maven pom.xml updater that uses fast-xml-parser for structural understanding but applies position-based string replacements to preserve formatting, with support for property-based version indirection (including recursive chains, circular detection, and dependencyManagement) and an accompanying high-coverage test suite with XML fixtures. Sequence diagram for updateMavenVersions Maven pom.xml update flowsequenceDiagram
participant Caller
participant updateMavenVersions
participant XMLParser
participant resolvePropertyChain
participant findDependencyVersionPosition
participant findPropertyPosition
Caller->>updateMavenVersions: updateMavenVersions(pomContent, versionChanges)
alt no_versionChanges
updateMavenVersions-->>Caller: {content: pomContent, applied: [], skipped: []}
else has_versionChanges
updateMavenVersions->>XMLParser: new XMLParser(options)
updateMavenVersions->>XMLParser: parse(pomContent)
alt parse_error
updateMavenVersions-->>Caller: {content: pomContent, applied: [], skipped: [reason Failed to parse pom.xml]}
else parse_ok
updateMavenVersions->>findDependencyVersionPosition: findDependencyVersionPosition(pomContent, groupId, artifactId)
alt version_not_found
updateMavenVersions-->>Caller: skipped reason Dependency not found or no version
else version_found
updateMavenVersions->>updateMavenVersions: read currentVersion
alt currentVersion_is_property
updateMavenVersions->>resolvePropertyChain: resolvePropertyChain(propName, properties)
alt resolve_error_or_cycle
updateMavenVersions-->>Caller: skipped reason from resolvePropertyChain
else resolved_ok
updateMavenVersions->>findPropertyPosition: findPropertyPosition(pomContent, terminalPropName)
alt property_position_not_found
updateMavenVersions-->>Caller: skipped reason Could not locate property
else property_position_found
updateMavenVersions->>updateMavenVersions: queue property replacement in replacementsByPosition
end
end
else direct_version
updateMavenVersions->>updateMavenVersions: queue direct version replacement
end
updateMavenVersions->>updateMavenVersions: sort replacementsByPosition descending
updateMavenVersions->>updateMavenVersions: splice pomContent to apply replacements
updateMavenVersions-->>Caller: {content: result, applied, skipped}
end
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When multiple versionChanges target the same direct dependency (same groupId/artifactId) with different newVersion values, the last one silently wins; consider adding a conflict check similar to the property case so callers get an explicit skipped reason instead of an implicit override.
- The string-based tag searches (e.g. findDependencyVersionPosition/findPropertyPosition) assume simple
<tag>/</tag>forms without attributes or namespace prefixes, which may break on real-world POMs; consider relaxing these to handle<tag ...>and prefixed tags (or leveraging the parsed XML structure to locate positions more robustly).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When multiple versionChanges target the same direct dependency (same groupId/artifactId) with different newVersion values, the last one silently wins; consider adding a conflict check similar to the property case so callers get an explicit skipped reason instead of an implicit override.
- The string-based tag searches (e.g. findDependencyVersionPosition/findPropertyPosition) assume simple `<tag>`/`</tag>` forms without attributes or namespace prefixes, which may break on real-world POMs; consider relaxing these to handle `<tag ...>` and prefixed tags (or leveraging the parsed XML structure to locate positions more robustly).
## Individual Comments
### Comment 1
<location path="test/updaters/maven_updater.test.js" line_range="331" />
<code_context>
+ });
+ });
+
+ suite('idempotency', () => {
+
+ /** Verifies that running updateMavenVersions twice with the same input is idempotent. */
</code_context>
<issue_to_address>
**suggestion (testing):** Expand idempotency tests to cover the applied/skipped metadata, not just the final content.
Since callers may depend on `applied`/`skipped` metadata, it would be valuable to assert how those fields behave on the second run—for example, whether we still report the same `applied` entries or instead mark everything as `skipped` once versions already match. Encoding this explicitly in a test will document the intended contract and guard against regressions in this behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #596 +/- ##
==========================================
- Coverage 91.01% 90.84% -0.18%
==========================================
Files 38 39 +1
Lines 8001 8363 +362
Branches 1395 1453 +58
==========================================
+ Hits 7282 7597 +315
- Misses 719 766 +47
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Implements TC-5411
Adds a new `updateMavenVersions()` function that modifies dependency
versions in pom.xml files while preserving all formatting, comments,
whitespace, and indentation.
Key capabilities:
- Direct <version> element replacement via position-based string splice
- ${property} indirection: traces property references through
<properties> and updates the property value instead
- Recursive property chain resolution (${a} -> ${b} -> value)
- Circular property reference detection with skip reporting
- Fail-safe: unreachable dependencies reported in skipped[]
- Idempotent: running twice with same input produces same output
- Covers both <dependencies> and <dependencyManagement> sections
Uses fast-xml-parser (existing dependency) for XML structure
understanding, then performs character-offset string splice to
avoid altering any non-version content.
Assisted-by: Claude Code
The direct-version path silently let the last entry win when the same dependency appeared twice in versionChanges with different newVersion values, while reporting both as applied. Align with the property path by checking replacementsByPosition before inserting and skipping conflicts with a clear reason. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review —
|
Collaborator
Author
Verification Report for TC-5411 (commit dcfb3d3)
Overall: WARNCommit This comment was AI-generated by sdlc-workflow/verify-pr v0.13.7. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/updaters/maven_updater.jsmodule withupdateMavenVersions()API${property}indirection with recursive resolution and cycle detection<dependencyManagement>sectionsImplements TC-5411
Test plan
npm run lintpassesnpm testpassesSummary by Sourcery
Add a Maven pom.xml updater that performs structure-aware, formatting-preserving dependency version updates with robust handling of property indirection and edge cases, backed by comprehensive tests and fixtures.
New Features:
Enhancements:
Tests: