tree: fix gap closing on branch deletion

ziggurat_foundations/exc.py

@@ -14,13 +14,17 @@ class ZigguratSessionException(ZigguratException):
     pass
 
 
-class ZigguratResourceTreeMissingException(ZigguratException):
+class ZugguratTreeException(ZigguratException):
     pass
 
 
-class ZigguratResourceTreePathException(ZigguratException):
+class ZigguratResourceTreeMissingException(ZugguratTreeException):
     pass
 
 
-class ZigguratResourceOutOfBoundaryException(ZigguratException):
+class ZigguratResourceTreePathException(ZugguratTreeException):
+    pass
+
+
+class ZigguratResourceOutOfBoundaryException(ZugguratTreeException):
     pass

ziggurat_foundations/models/services/resource_tree_postgres.py

@@ -62,6 +62,13 @@ def delete_branch(cls, resource_id=None, db_session=None, *args, **kwargs):
         :return:
         """
         tablename = cls.model.__table__.name
+
+        # lets lock rows to prevent bad tree states
+        resource = ResourceService.lock_resource_for_update(
+            resource_id=resource_id,
+            db_session=db_session)
+        parent_id = resource.parent_id
+        ordering = resource.ordering
         raw_q = """
             WITH RECURSIVE subtree AS (
                     SELECT res.resource_id
@@ -74,8 +81,14 @@ def delete_branch(cls, resource_id=None, db_session=None, *args, **kwargs):
             DELETE FROM resources where resource_id in (select * from subtree);
         """.format(tablename=tablename)
         db_session = get_db_session(db_session)
+        # this is required to force transaction managers to mark the session
+        # as "dirty"
+        dummy = db_session.query(cls.model)
+        dummy.filter(cls.model.resource_id == None).delete(
+            synchronize_session=False)
         text_obj = sa.text(raw_q)
         db_session.execute(text_obj, params={'resource_id': resource_id})
+        cls.shift_ordering_down(parent_id, ordering, db_session=db_session)
         return True
 
     @classmethod

ziggurat_foundations/tests/test_resource_tree_postgres.py

@@ -338,18 +338,21 @@ def test_move_from_root_deeper(self, db_session):
         assert [n['node'].ordering for n in tree_struct.values()] == [1, 2]
 
     @pytest.mark.skipif(not_postgres, reason="requires postgres")
-    @pytest.mark.parametrize("resource_id, expected", [
-        (1, [-1, 2, 4, 3, 10, 11, -2, -3]),
-        (9, [-1, 1, 5, 6, 7, 8, 2, 4, 3, 10, 11, -2, -3]),
-        (12, [-1, 1, 5, 6, 7, 9, 8, 2, 4, 3, 10, 11, -2, -3]),
-        (-1, [-2, -3]),
+    @pytest.mark.parametrize("resource_id, expected_ids, expected_order", [
+        (1, [-1, 2, 4, 3, 10, 11, -2, -3], None),
+        (9, [-1, 1, 5, 6, 7, 8, 2, 4, 3, 10, 11, -2, -3], None),
+        (12, [-1, 1, 5, 6, 7, 9, 8, 2, 4, 3, 10, 11, -2, -3], None),
+        (-1, [-2, -3], [1, 2]),
     ])
-    def test_delete_branches(self, db_session, resource_id, expected):
+    def test_delete_branches(self, db_session, resource_id, expected_ids,
+                             expected_order):
         create_default_tree(db_session)
         tree_service.delete_branch(resource_id, db_session=db_session)
 
         result = tree_service.from_parent_deeper(
             parent_id=None, db_session=db_session)
         row_ids = [r.Resource.resource_id for r in result]
-        print(row_ids)
-        assert row_ids == expected
+        ordering = [r.Resource.ordering for r in result]
+        assert row_ids == expected_ids
+        if expected_order:
+            assert ordering == expected_order