Fix AddProperty skipping property when child POM has relativePath#6943
Open
Fix AddProperty skipping property when child POM has relativePath#6943
Conversation
…ePath AddProperty blindly deferred to parent whenever <relativePath> had a non-empty value, even when the property didn't exist locally. This caused UpgradeDependencyVersion to fail when a managed dependency property needed to be overridden in a child POM with a local parent that itself inherited the property from a remote ancestor. Guard the consolidation branch with `parentValue != null` so it only fires when the property already exists in the current POM (something to actually consolidate), and falls through to AddPropertyVisitor otherwise.
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
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
Fixes
AddPropertyblindly deferring to parent POM whenever<relativePath>has a non-empty value, even when the property doesn't exist locallyThis caused
UpgradeDependencyVersionto fail when a managed dependency property needed to be overridden in a child POM that had a local parent inheriting the property from a remote ancestor (e.g. Spring Boot)Root cause fix: guard the consolidation branch with
parentValue != nullso it only fires when the property already exists in the current POMRelated to UpgradeDependencyVersion bug not working in the absence of <relativePath> #6910 — this addresses the root cause in
AddPropertyrather than working around it inUpgradeDependencyVersionTest plan
AddPropertyTest.addPropertyWhenLocalParentDoesNotDefineIt— child with relativePath, property not defined locally → property gets addedAddPropertyTest.consolidatesToParentWhenBothDefineProperty— both parent and child define property → child's property removed, parent updatedAddPropertyTest.addPropertyToChildWithImplicitRelativePath— no explicit<relativePath>tag → property added to both (no deferring)UpgradeDependencyVersionTest.upgradeVersionDefinedViaImplicitPropertyInRemoteParentOfLocalParent_withRelativePath— end-to-end integration testAddPropertyTest,ChangePropertyValueTest, andUpgradeDependencyVersionTesttests pass