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

MoveStrategy.MERGE does not work if the target node does not exist #181

Closed
SizableShrimp opened this issue Nov 6, 2020 · 0 comments
Closed
Assignees
Milestone

Comments

@SizableShrimp
Copy link

The current implementation of MoveStrategy.MERGE uses ConfigurationNode#mergeFrom(ConfigurationNode) to merge nodes that have their path changed in a TransformAction. The problem with this implementation is in this line from SingleConfigurationTransformation:

this.strategy.move(node, start.node(transformedPath));

If the transformedPath does not exist within the node structure, then a virtual node is returned. When this virtual node is passed through MoveStrategy.OVERWRITE, it is run through ConfigurationNode#from(ConfigurationNode). This method properly attaches the node if necessary to its parent and then overwrites the node. However, MoveStrategy.MERGE and therefore ConfigurationNode#mergeFrom(ConfigurationNode), does not actually attempt to attach the node if necessary back to its parent on merge. This causes the merged node to never actually update in the structure of the start node, and therefore breaks if trying to use MoveStrategy.MERGE when the target node is virtual. This seems like unintended behavior.

Consider the following situation: A developer might want to use MoveStrategy.MERGE for their transformation builder because they want to move and merge one node. However, they might also want to move a separate node to a new target path even if that path doesn't exist (e.g. oldlocation to newlocation). Yet, with MoveStrategy.MERGE in use, the target node (newlocation) doesn't exist and is virtual, and oldlocation is merged into newlocation. This newlocation virtual node is never actually attached back to the parent. Being required to use MoveStrategy.OVERWRITE to be able to move old nodes to new, non-existent locations seems like an oversight in the library.

@SizableShrimp SizableShrimp changed the title MoveStrategy.MERGE does not work if the moved node does not exist MoveStrategy.MERGE does not work if the target node does not exist Nov 6, 2020
@zml2008 zml2008 self-assigned this Nov 14, 2020
@zml2008 zml2008 added this to the 4.0 milestone Nov 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants