fix eclipse-theia#2499: fixed Tree construction

Signed-off-by: Anton Kosyakov <[email protected]>

packages/core/src/browser/tree/tree.spec.ts

@@ -0,0 +1,149 @@
+/********************************************************************************
+ * Copyright (C) 2018 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import * as assert from 'assert';
+import { TreeNode, CompositeTreeNode } from './tree';
+
+describe('Tree', () => {
+
+    it('addChildren', () => {
+        assertTreeNode(`{
+  "id": "parent",
+  "name": "parent",
+  "children": [
+    {
+      "id": "foo",
+      "name": "foo",
+      "parent": "parent",
+      "nextSibling": "bar"
+    },
+    {
+      "id": "bar",
+      "name": "bar",
+      "parent": "parent",
+      "previousSibling": "foo",
+      "nextSibling": "baz"
+    },
+    {
+      "id": "baz",
+      "name": "baz",
+      "parent": "parent",
+      "previousSibling": "bar"
+    }
+  ]
+}`, getNode());
+    });
+
+    it('removeChild - first', () => {
+        const node = getNode();
+        CompositeTreeNode.removeChild(node, node.children[0]);
+        assertTreeNode(`{
+  "id": "parent",
+  "name": "parent",
+  "children": [
+    {
+      "id": "bar",
+      "name": "bar",
+      "parent": "parent",
+      "nextSibling": "baz"
+    },
+    {
+      "id": "baz",
+      "name": "baz",
+      "parent": "parent",
+      "previousSibling": "bar"
+    }
+  ]
+}`, node);
+    });
+
+    it('removeChild - second', () => {
+        const node = getNode();
+        CompositeTreeNode.removeChild(node, node.children[1]);
+        assertTreeNode(`{
+  "id": "parent",
+  "name": "parent",
+  "children": [
+    {
+      "id": "foo",
+      "name": "foo",
+      "parent": "parent",
+      "nextSibling": "baz"
+    },
+    {
+      "id": "baz",
+      "name": "baz",
+      "parent": "parent",
+      "previousSibling": "foo"
+    }
+  ]
+}`, node);
+    });
+
+    it('removeChild - thrid', () => {
+        const node = getNode();
+        CompositeTreeNode.removeChild(node, node.children[2]);
+        assertTreeNode(`{
+  "id": "parent",
+  "name": "parent",
+  "children": [
+    {
+      "id": "foo",
+      "name": "foo",
+      "parent": "parent",
+      "nextSibling": "bar"
+    },
+    {
+      "id": "bar",
+      "name": "bar",
+      "parent": "parent",
+      "previousSibling": "foo"
+    }
+  ]
+}`, node);
+    });
+
+    function getNode(): CompositeTreeNode {
+        return CompositeTreeNode.addChildren({
+            id: 'parent',
+            name: 'parent',
+            children: [],
+            parent: undefined
+        }, [{
+            id: 'foo',
+            name: 'foo',
+            parent: undefined
+        }, {
+            id: 'bar',
+            name: 'bar',
+            parent: undefined
+        }, {
+            id: 'baz',
+            name: 'baz',
+            parent: undefined
+        }]);
+    }
+
+    function assertTreeNode(expectation: string, node: TreeNode): void {
+        assert.deepEqual(expectation, JSON.stringify(node, (key: keyof CompositeTreeNode, value: any) => {
+            if (key === 'parent' || key === 'previousSibling' || key === 'nextSibling') {
+                return value && value.id;
+            }
+            return value;
+        }, 2));
+    }
+
+});

packages/core/src/browser/tree/tree.ts

@@ -144,6 +144,54 @@ export namespace CompositeTreeNode {
         }
         return parent.children.findIndex(child => TreeNode.equals(node, child));
     }
+
+    export function addChildren(parent: CompositeTreeNode, children: TreeNode[]): CompositeTreeNode {
+        for (const child of children) {
+            addChild(parent, child);
+        }
+        return parent;
+    }
+
+    export function addChild(parent: CompositeTreeNode, child: TreeNode): CompositeTreeNode {
+        const children = parent.children as TreeNode[];
+        const index = children.findIndex(value => value.id === child.id);
+        if (index !== -1) {
+            children.splice(index, 1, child);
+            setParent(child, index, parent);
+        } else {
+            children.push(child);
+            setParent(child, parent.children.length - 1, parent);
+        }
+        return parent;
+    }
+
+    export function removeChild(parent: CompositeTreeNode, child: TreeNode): void {
+        const children = parent.children as TreeNode[];
+        const index = children.findIndex(value => value.id === child.id);
+        if (index === -1) {
+            return;
+        }
+        children.splice(index, 1);
+        const { previousSibling, nextSibling } = child;
+        if (previousSibling) {
+            Object.assign(previousSibling, { nextSibling });
+        }
+        if (nextSibling) {
+            Object.assign(nextSibling, { previousSibling });
+        }
+    }
+
+    export function setParent(child: TreeNode, index: number, parent: CompositeTreeNode): void {
+        const previousSibling = parent.children[index - 1];
+        const nextSibling = parent.children[index + 1];
+        Object.assign(child, { parent, previousSibling, nextSibling });
+        if (previousSibling) {
+            Object.assign(previousSibling, { nextSibling: child });
+        }
+        if (nextSibling) {
+            Object.assign(nextSibling, { previousSibling: child });
+        }
+    }
 }
 
 /**
@@ -246,34 +294,10 @@ export class TreeImpl implements Tree {
         if (CompositeTreeNode.is(node)) {
             const { children } = node;
             children.forEach((child, index) => {
-                this.setParent(child, index, node);
+                CompositeTreeNode.setParent(child, index, node);
                 this.addNode(child);
             });
         }
     }
 
-    protected setParent(child: TreeNode, index: number, parent: CompositeTreeNode): void {
-        const previousSibling = parent.children[index - 1];
-        const nextSibling = parent.children[index + 1];
-        Object.assign(child, { parent, previousSibling, nextSibling });
-    }
-
-    protected addChild(parent: CompositeTreeNode, child: TreeNode): void {
-        const index = parent.children.findIndex(value => value.id === child.id);
-        if (index !== -1) {
-            (parent.children as TreeNode[]).splice(index, 1, child);
-            this.setParent(child, index, parent);
-        } else {
-            (parent.children as TreeNode[]).push(child);
-            this.setParent(child, parent.children.length - 1, parent);
-        }
-    }
-
-    protected removeChild(parent: CompositeTreeNode, child: TreeNode): void {
-        const index = parent.children.findIndex(value => value.id === child.id);
-        if (index !== -1) {
-            (parent.children as TreeNode[]).splice(index, 1);
-        }
-    }
-
 }

packages/markers/src/browser/marker-tree.ts

@@ -55,14 +55,14 @@ export abstract class MarkerTree<T extends object> extends TreeImpl {
         const markers = this.markerManager.findMarkers({ uri });
         if (markers.length <= 0) {
             if (MarkerInfoNode.is(existing)) {
-                this.removeChild(existing.parent, existing);
+                CompositeTreeNode.removeChild(existing.parent, existing);
                 this.removeNode(existing);
                 this.fireChanged();
             }
             return;
         }
         const node = MarkerInfoNode.is(existing) ? existing : await this.createMarkerInfo(id, uri);
-        this.addChild(node.parent, node);
+        CompositeTreeNode.addChild(node.parent, node);
         const children = this.getMarkerNodes(node, markers);
         node.numberOfMarkers = markers.length;
         this.setChildren(node, children);