From d82a7d6e9ffcbca13f82ce3bf68f61e97679a366 Mon Sep 17 00:00:00 2001
From: Paul Campbell <pcampbell@kemitix.net>
Date: Tue, 24 May 2016 13:13:55 +0100
Subject: [PATCH] NoteItem: replace empty target node properly

---
 src/main/java/net/kemitix/node/NodeItem.java  | 42 +++++++---------
 .../java/net/kemitix/node/NodeItemTest.java   | 49 +++++++++++++++++++
 2 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/src/main/java/net/kemitix/node/NodeItem.java b/src/main/java/net/kemitix/node/NodeItem.java
index a756483..b77da2a 100644
--- a/src/main/java/net/kemitix/node/NodeItem.java
+++ b/src/main/java/net/kemitix/node/NodeItem.java
@@ -294,24 +294,26 @@ public class NodeItem<T> implements Node<T> {
     @Override
     public void placeNodeIn(final Node<T> nodeItem, final String... path) {
         if (path.length == 0) {
-            if (nodeItem.isNamed()) {
-                final Optional<Node<T>> childNamed = findChildNamed(
-                        nodeItem.getName());
-                if (childNamed.isPresent()) {
-                    final Node<T> existing = childNamed.get();
-                    if (!existing.isEmpty()) {
-                        throw new NodeException(
-                                "A non-empty node with that name already "
-                                        + "exists here");
-                    } else {
-                        existing.getChildren().forEach(nodeItem::addChild);
-                        existing.removeParent();
-                        nodeItem.setParent(this);
-                        return;
-                    }
-                }
+            if (!nodeItem.isNamed()) { // nothing to conflict with
+                addChild(nodeItem);
+                return;
+            }
+            final Optional<Node<T>> childNamed = findChildNamed(
+                    nodeItem.getName());
+            if (!childNamed.isPresent()) { // nothing with the same name exists
+                addChild(nodeItem);
+                return;
+            }
+            // we have an existing node with the same name
+            final Node<T> existing = childNamed.get();
+            if (!existing.isEmpty()) {
+                throw new NodeException(
+                        "A non-empty node with that name already exists here");
+            } else {
+                existing.getChildren().forEach(nodeItem::addChild);
+                existing.removeParent();
+                addChild(nodeItem);
             }
-            addChild(nodeItem);
             return;
         }
         String item = path[0];
@@ -321,12 +323,6 @@ public class NodeItem<T> implements Node<T> {
             child = new NodeItem<>(null, item, this);
         } else {
             child = childNamed.get();
-            if (child.isEmpty()) {
-                if (path.length == 1) {
-                    getChildren().forEach(nodeItem::addChild);
-                    nodeItem.setParent(this);
-                }
-            }
         }
         child.placeNodeIn(nodeItem, Arrays.copyOfRange(path, 1, path.length));
     }
diff --git a/src/test/java/net/kemitix/node/NodeItemTest.java b/src/test/java/net/kemitix/node/NodeItemTest.java
index 623ea94..5f5a5b0 100644
--- a/src/test/java/net/kemitix/node/NodeItemTest.java
+++ b/src/test/java/net/kemitix/node/NodeItemTest.java
@@ -744,4 +744,53 @@ public class NodeItemTest {
         //when
         node.removeParent();
     }
+
+    @Test
+    public void placeNodeInTreeWhereNonEmptyNodeWithSameNameExists() {
+        //given
+        exception.expect(NodeException.class);
+        exception.expectMessage(
+                "A non-empty node with that name already exists here");
+        node = new NodeItem<>(null);
+        val child = new NodeItem<String>(null, "child", node);
+        new NodeItem<>("data", "grandchild", child);
+        // root -> child -> grandchild
+        // only grandchild has data
+        //when
+        // attempt to add another node called 'grandchild' to 'child'
+        node.placeNodeIn(new NodeItem<>("cuckoo", "grandchild"), "child");
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void placeNodeInTreeWhenAddedNodeIsUnnamed() {
+        //given
+        node = new NodeItem<>(null);
+        final Node<String> newNode = new NodeItem<>(null);
+        //when
+        node.placeNodeIn(newNode);
+        //then
+        assertThat(node.getChildren()).containsOnly(newNode);
+    }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void placeNodeInTreeWhenEmptyChildWithTargetNameExists() {
+        //given
+        node = new NodeItem<>(null);
+        final NodeItem<String> child = new NodeItem<>(null, "child");
+        final NodeItem<String> target = new NodeItem<>(null, "target");
+        node.addChild(child);
+        child.addChild(target);
+        final NodeItem<String> addMe = new NodeItem<>("I'm new", "target");
+        assertThat(addMe.getParent()).isNull();
+        //when
+        // addMe should replace target as the sole descendant of child
+        node.placeNodeIn(addMe, "child");
+        //then
+        assertThat(child.getChildren()).as("child only contains new node")
+                                       .containsOnly(addMe);
+        assertThat(target.getParent()).as("old node is removed from tree")
+                                      .isNull();
+    }
 }