Merge pull request #59 from Workiva/invalid-tree-node-heights

DT-23492 Type errors from invalid node heights
Workiva · Oct 31, 2023 · fb17c5b · fb17c5b
2 parents 6759a6a + 7b11400
commit fb17c5b
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 3 deletions.
diff --git a/lib/src/r_tree/node.dart b/lib/src/r_tree/node.dart
@@ -124,7 +124,7 @@ abstract class Node<E> extends RTreeContributor {
     addChild(seeds.seed1);
 
     Node<E> splitNode = createNewNode();
-    splitNode.height = height + 1;
+    splitNode.height = height;
     splitNode.addChild(seeds.seed2);
 
     _reassignRemainingChildren(remainingChildren, splitNode);

diff --git a/lib/src/r_tree/non_leaf_node.dart b/lib/src/r_tree/non_leaf_node.dart
@@ -79,6 +79,8 @@ class NonLeafNode<E> extends Node<E> {
     for (var child in childrenToRemove) {
       removeChild(child);
     }
+
+    _recalculateHeight();
   }
 
   addChild(Node<E> child) {
@@ -93,6 +95,8 @@ class NonLeafNode<E> extends Node<E> {
     if (_childNodes.length == 0) {
       _convertToLeafNode();
     }
+
+    _recalculateHeight();
   }
 
   clearChildren() {
@@ -124,4 +128,12 @@ class NonLeafNode<E> extends Node<E> {
     nonLeafParent.removeChild(this);
     nonLeafParent.addChild(newLeafNode);
   }
+
+  _recalculateHeight() {
+    final maxChildHeight = _childNodes.fold(0, (int greatestHeight, childNode) {
+      return max(greatestHeight, childNode.height);
+    });
+
+    height = 1 + maxChildHeight;
+  }
 }
diff --git a/test/r_tree/r_tree_test.dart b/test/r_tree/r_tree_test.dart
@@ -13,11 +13,12 @@ main() {
         RTreeDatum<String> item = RTreeDatum<String>(Rectangle(0, 0, 1, 1), 'Item 1');
 
         tree.insert(item);
+        assertTreeHeightValidity(tree);
+
         var items = tree.search(item.rect, shouldInclude: (_) => false);
         expect(items, isEmpty);
 
         items = tree.search(item.rect);
-
         expect(items.length, equals(1));
         expect(items.elementAt(0).value, equals('Item 1'));
 
@@ -27,13 +28,15 @@ main() {
           tree.insert(RTreeDatum<String>(Rectangle(0, 0, 1, 1), 'Item 4'));
           tree.insert(RTreeDatum<String>(Rectangle(0, 0, 1, 1), 'Item 5'));
         });
+        assertTreeHeightValidity(tree);
 
         items = tree.search(item.rect);
         expect(items.length, equals(5));
 
         items.forEach((item) {
           tree.remove(item);
         });
+        assertTreeHeightValidity(tree);
 
         items = tree.search((item.rect));
         expect(items.isEmpty, isTrue);
@@ -61,6 +64,7 @@ main() {
           }
 
           addMethod.method(tree, itemsToInsert);
+          assertTreeHeightValidity(tree);
 
           var items = tree.search(Rectangle(0, 0, 1, 3)); // A1:A3
           expect(items.length, equals(1));
@@ -91,6 +95,7 @@ main() {
           }
 
           addMethod.method(tree, itemsToInsert);
+          assertTreeHeightValidity(tree);
 
           var items = tree.search(Rectangle(0, 2, 1, 1));
           expect(items.length, equals(1));
@@ -122,6 +127,7 @@ main() {
           }
 
           addMethod.method(tree, itemsToInsert);
+          assertTreeHeightValidity(tree);
 
           var items = tree.search(Rectangle(31, 27, 1, 1));
           expect(items.length, equals(1));
@@ -134,25 +140,32 @@ main() {
     });
 
     group('Remove', () {
-      test('remove should only remove first occurance of item', () {
+      test('remove should only remove first occurrence of item', () {
         RTree tree = RTree(3);
         RTreeDatum<String> item = RTreeDatum<String>(Rectangle(0, 0, 1, 1), 'Item 1');
 
         tree.insert(item);
         tree.insert(item);
+        assertTreeHeightValidity(tree);
 
         var items = tree.search(item.rect);
         expect(items.length, equals(2));
 
         tree.remove(item);
+        assertTreeHeightValidity(tree);
+
         items = tree.search(item.rect);
         expect(items.length, equals(1));
 
         tree.remove(item);
+        assertTreeHeightValidity(tree);
+
         items = tree.search(item.rect);
         expect(items.length, equals(0));
 
         tree.insert(item);
+        assertTreeHeightValidity(tree);
+
         items = tree.search(item.rect);
         expect(items.length, equals(1));
       });
@@ -168,11 +181,13 @@ main() {
             tree.insert(itemMap[itemId]);
           }
         }
+        assertTreeHeightValidity(tree);
 
         var items = tree.search(itemMap['Item 0:0'].rect);
         expect(items.length, equals(1));
 
         tree.remove(itemMap['Item 0:0']);
+        assertTreeHeightValidity(tree);
 
         items = tree.search(itemMap['Item 0:0'].rect);
         expect(items.length, equals(0));
@@ -181,6 +196,7 @@ main() {
         expect(items.length, equals(1));
 
         tree.remove(itemMap['Item 13:41']);
+        assertTreeHeightValidity(tree);
 
         items = tree.search(itemMap['Item 13:41'].rect);
         expect(items.length, equals(0));
@@ -197,30 +213,119 @@ main() {
             tree.insert(item);
           }
         }
+        assertTreeHeightValidity(tree);
 
         var items = tree.search(Rectangle(0, 0, 50, 50));
         expect(items.length, equals(2500));
 
         data.forEach((RTreeDatum item) {
           tree.remove(item);
         });
+        assertTreeHeightValidity(tree);
 
         items = tree.search(Rectangle(0, 0, 50, 50));
         expect(items.length, equals(0));
 
         //test inserting after removal to ensure new root leaf node functions correctly
         tree.insert(RTreeDatum<String>(Rectangle(0, 0, 1, 1), 'New Initial Item'));
+        assertTreeHeightValidity(tree);
 
         items = tree.search(Rectangle(0, 0, 50, 50));
 
         items.forEach((datum) {
           expect(datum.value, equals('New Initial Item'));
         });
       });
+
+      test('remove all items and then reload', () {
+        final tree = RTree(3);
+
+        var items = <RTreeDatum<String>>[];
+        for (var i = 0; i < 20; i++) {
+          final item = RTreeDatum(Rectangle(0, i, 1, 1), 'Item $i');
+          items.add(item);
+          tree.insert(item);
+        }
+        assertTreeHeightValidity(tree);
+
+        var searchResult = tree.search(Rectangle(0, 0, 1, 20));
+        expect(searchResult, hasLength(20));
+
+        for (final item in items) {
+          tree.remove(item);
+        }
+        assertTreeHeightValidity(tree);
+
+        searchResult = tree.search(Rectangle(0, 0, 1, 20));
+        expect(searchResult, isEmpty);
+
+        tree.load(items.sublist(0, 3));
+        assertTreeHeightValidity(tree);
+
+        searchResult = tree.search(Rectangle(0, 0, 1, 20));
+        expect(searchResult, hasLength(3));
+      });
     });
   });
 }
 
+void assertTreeHeightValidity<E>(RTree<E> tree) {
+  try {
+    assertNodeHeightValidity(tree, tree.currentRootNode);
+  } on StateError catch (e) {
+    fail('${e.message}\nTree:\n${stringifyTree(tree)}');
+  }
+}
+
+int assertNodeHeightValidity<E>(RTree<E> tree, RTreeContributor contributor) {
+  if (contributor is LeafNode<E>) {
+    if (contributor.height != 1) {
+      throw StateError('Leaf height of ${contributor.height} should be 1.');
+    }
+
+    return 1;
+  } else if (contributor is NonLeafNode<E>) {
+    var maxChildHeight = 0;
+    if (contributor.children.isNotEmpty) {
+      for (final child in contributor.children) {
+        final childHeight = assertNodeHeightValidity(tree, child);
+        if (childHeight > maxChildHeight) {
+          maxChildHeight = childHeight;
+        }
+      }
+    }
+
+    final actualNodeHeight = 1 + maxChildHeight;
+    if (contributor.height != actualNodeHeight) {
+      throw StateError('Non-leaf height of ${contributor.height} should be $actualNodeHeight.');
+    }
+
+    return actualNodeHeight;
+  }
+
+  return 0;
+}
+
+// Serializes the tree in a human-readable form for debugging.
+String stringifyTree<E>(RTree<E> tree) {
+  final buffer = StringBuffer();
+  stringifyNode(buffer, tree.currentRootNode, 0);
+  return buffer.toString();
+}
+
+// Serializes the subtree from [contributor] in a humnan-readable form for debugging.
+void stringifyNode<E>(StringBuffer buffer, RTreeContributor contributor, int level) {
+  buffer.write('${' ' * level}${contributor.runtimeType}');
+  if (contributor is Node<E>) {
+    buffer.write('(height=${contributor.height}):\n');
+    for (final child in contributor.children) {
+      stringifyNode(buffer, child, level + 1);
+    }
+  } else if (contributor is RTreeDatum<E>) {
+    buffer.write(': ${contributor.value}\n');
+  }
+}
+
 class _InsertCase {
   final Function(RTree tree, Iterable<RTreeDatum<String>> toAdd) method;
   final String name;