ConfigNode copy now preserves the original properly

Fixed a possible future issue where tree up-traversal is broken
if a node in a tree is passed to a copy operation.
This was because the parent node was updated in the process of copy if
no old node was present.

Fixed by creating a new node when copying instead of reusing the
object being copied
This commit is contained in:
Rsl1122 2019-01-05 20:21:10 +02:00
parent f6384a49a4
commit 0dd3af9a68
2 changed files with 69 additions and 13 deletions

View File

@ -91,12 +91,16 @@ public class ConfigNode {
String key = parts[0];
String leftover = parts[1];
// Add a new child
ConfigNode child;
if (!childNodes.containsKey(key)) {
child = addChild(new ConfigNode(key, newParent, null));
} else {
child = childNodes.get(key);
}
// If the path ends return the leaf node
// Otherwise continue recursively.
return leftover.isEmpty() ? child : child.addNode(leftover);
}
throw new IllegalArgumentException("Can not add a node with empty path");
@ -123,6 +127,13 @@ public class ConfigNode {
updateParent(null);
}
/**
* Add a new child ConfigNode.
*
* @param child ConfigNode to add.
* If from another config tree, the parent is 'cut', which breaks the old tree traversal.
* @return Return the node given, now part of this tree.
*/
protected ConfigNode addChild(ConfigNode child) {
getNode(child.key).ifPresent(ConfigNode::remove);
childNodes.put(child.key, child);
@ -274,37 +285,38 @@ public class ConfigNode {
}
public void copyMissing(ConfigNode from) {
// Override comment conditionally
if (comment.size() < from.comment.size()) {
comment = from.comment;
}
// Override value conditionally
if (value == null && from.value != null) {
value = from.value;
}
// Copy all nodes from 'from'
for (String key : from.nodeOrder) {
ConfigNode newChild = from.childNodes.get(key);
if (childNodes.containsKey(key)) {
ConfigNode oldChild = childNodes.get(key);
oldChild.copyMissing(newChild);
} else {
addChild(newChild);
}
// Copy values recursively to children
ConfigNode created = addNode(key);
created.copyMissing(newChild);
}
}
public void copyAll(ConfigNode from) {
// Override comment and value unconditionally.
comment = from.comment;
value = from.value;
// Copy all nodes from 'from'
for (String key : from.nodeOrder) {
ConfigNode newChild = from.childNodes.get(key);
if (childNodes.containsKey(key)) {
ConfigNode oldChild = childNodes.get(key);
oldChild.copyAll(newChild);
} else {
addChild(newChild);
}
// Copy values recursively to children
ConfigNode created = addNode(key);
created.copyAll(newChild);
}
}

View File

@ -36,6 +36,7 @@ class ConfigNodeTest {
private static final String STRING_NODE_WITH_DOUBLE_QUOTES = "String_node_with_double_quotes";
private static final String FIRST_LEVEL = "1st_level";
private static final String SECOND_LEVEL = "2nd_level";
private static final String THIRD_LEVEL = "3rd_level";
private ConfigNode testTree;
@ -46,8 +47,10 @@ class ConfigNodeTest {
testTree.addChild(new ConfigNode(STRING_NODE_WITH_QUOTES, testTree, "\"'String'\""));
testTree.addChild(new ConfigNode(STRING_NODE_WITH_DOUBLE_QUOTES, testTree, "'\"String\"'"));
ConfigNode emptyNode = new ConfigNode(FIRST_LEVEL, testTree, null);
ConfigNode secondLevel = new ConfigNode(SECOND_LEVEL, emptyNode, "String");
testTree.addChild(emptyNode);
emptyNode.addChild(new ConfigNode(SECOND_LEVEL, emptyNode, "String"));
emptyNode.addChild(secondLevel);
secondLevel.addChild(new ConfigNode(THIRD_LEVEL, secondLevel, "3"));
assertTrue(testTree.childNodes.containsKey(SIMPLE_STRING_NODE), "Tree construction failed, addChild does not work.");
}
@ -288,4 +291,45 @@ class ConfigNodeTest {
testTree.set(SIMPLE_STRING_NODE, adding);
assertTrue(testTree.getNode(SIMPLE_STRING_NODE + "." + SECOND_LEVEL).isPresent());
}
@Test
void copyAllOverridesAllValues() {
ConfigNode overridingWith = testTree.getNode(FIRST_LEVEL).orElseThrow(() -> new AssertionError("Fail"));
ConfigNode added = testTree.addNode("Test." + SECOND_LEVEL + "." + THIRD_LEVEL);
added.set("ORIGINAL");
ConfigNode node = testTree.getNode("Test").orElseThrow(() -> new AssertionError("Fail"));
node.copyAll(overridingWith);
String original = testTree.getString(FIRST_LEVEL);
String copied = testTree.getString("Test");
assertEquals(original, copied);
original = testTree.getString(FIRST_LEVEL + "." + SECOND_LEVEL);
copied = testTree.getString("Test." + SECOND_LEVEL);
assertEquals(original, copied);
original = testTree.getString(FIRST_LEVEL + "." + SECOND_LEVEL + "." + THIRD_LEVEL);
copied = testTree.getString("Test." + SECOND_LEVEL + "." + THIRD_LEVEL);
assertEquals(original, copied);
}
@Test
void copyDefaultAddsProperValues() {
ConfigNode overridingWith = testTree.getNode(FIRST_LEVEL).orElseThrow(() -> new AssertionError("Fail"));
ConfigNode added = testTree.addNode("Test." + SECOND_LEVEL + "." + THIRD_LEVEL);
added.set("ORIGINAL");
ConfigNode node = testTree.getNode("Test").orElseThrow(() -> new AssertionError("Fail"));
node.copyMissing(overridingWith);
String original = testTree.getString(FIRST_LEVEL);
String copied = testTree.getString("Test");
assertEquals(original, copied);
original = testTree.getString(FIRST_LEVEL + "." + SECOND_LEVEL);
copied = testTree.getString("Test." + SECOND_LEVEL);
assertEquals(original, copied);
copied = testTree.getString("Test." + SECOND_LEVEL + "." + THIRD_LEVEL);
assertEquals("ORIGINAL", copied);
}
}