Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xpp3DomUtils#mergeIntoXpp3Dom() must not override the dominant value in case it is empty #216

Closed
kwin opened this issue Sep 12, 2022 · 1 comment · Fixed by #217
Closed

Comments

@kwin
Copy link
Contributor

kwin commented Sep 12, 2022

Although that behaviour is also stated in its javadoc it is highly unexpected. Rather a dominant empty value element should take precedence over the same named recessive element value. This would be necessary for https://issues.apache.org/jira/browse/MNG-6434 to work as expected.

@kwin kwin changed the title Xpp3DomUtils.mergeIntoXpp3Dom overrides the dominant value in case it is empty Xpp3DomUtils.mergeIntoXpp3Dom must not overwrite the dominant value in case it is empty Sep 12, 2022
kwin added a commit to kwin/plexus-utils that referenced this issue Sep 12, 2022
@michael-o
Copy link
Member

...an empty string is still a valid string...unless you use Oracle :-)

@michael-o michael-o changed the title Xpp3DomUtils.mergeIntoXpp3Dom must not overwrite the dominant value in case it is empty Xpp3DomUtils#mergeIntoXpp3Dom() must not override the dominant value in case it is empty Oct 10, 2022
kwin added a commit to kwin/plexus-utils that referenced this issue Oct 11, 2022
kwin added a commit to kwin/plexus-utils that referenced this issue Oct 11, 2022
michael-o pushed a commit to kwin/plexus-utils that referenced this issue Oct 15, 2022
gnodet added a commit that referenced this issue Apr 5, 2023
The behavior has changed with #216 but plexus-utils contains duplicate merge code in Xpp3Dom and Xpp3DomUtils and the two pieces have divered.  This duplication is removed by the PR by switching to the XmlNode implementation in both cases, but we now need to fix the expectations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants