From: Simon Bowly <simon.bowly@gmail.com>
Date: Thu, 20 May 2021 21:46:04 +0000 (+1000)
Subject: De-duplicate Revision.down_revision helpers.
X-Git-Tag: rel_1_6_3~1^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=72235a2550c08eb7871576ce7b2283b111f796d2;p=thirdparty%2Fsqlalchemy%2Falembic.git

De-duplicate Revision.down_revision helpers.

Fixes an issue when downgrading a revision where dependencies
are duplicated (e.g. by specifying the down_revision in depends_on).
This causes the down migrations to fail to remove the head, which is
caught by an assertion.

Added logic to de-duplicate the entries in _normalized_down_revisions
and _all_down_revisions without altering revision order.

This change revises the previous approach taken in
8d5a9a1b0d32fff5726010afffa48cc0fb738238 to address the problem
more generally outside of the topological algorithm.

Fixes: #843

Change-Id: I3517fb102b38c783c9a61bfb8dabc02c1fc89c4b
---

diff --git a/alembic/script/revision.py b/alembic/script/revision.py
index f2055663..f2b4f706 100644
--- a/alembic/script/revision.py
+++ b/alembic/script/revision.py
@@ -863,15 +863,10 @@ class RevisionMap(object):
 
                 candidate_rev = id_to_rev[candidate]
 
-                # immediate ancestor nodes, use a set to uniquify
-                _u = set()
                 heads_to_add = [
                     r
                     for r in candidate_rev._normalized_down_revisions
-                    if r in todo
-                    and r not in current_heads
-                    and r not in _u
-                    and (_u.add(r) or True)
+                    if r in todo and r not in current_heads
                 ]
 
                 if not heads_to_add:
@@ -1424,7 +1419,7 @@ class Revision(object):
 
     @property
     def _all_down_revisions(self):
-        return (
+        return util.dedupe_tuple(
             util.to_tuple(self.down_revision, default=())
             + self._resolved_dependencies
         )
@@ -1435,7 +1430,7 @@ class Revision(object):
         that are still dependencies of ancestors.
 
         """
-        return (
+        return util.dedupe_tuple(
             util.to_tuple(self.down_revision, default=())
             + self._normalized_resolved_dependencies
         )
diff --git a/tests/test_revision.py b/tests/test_revision.py
index ef1202a0..c2c1410f 100644
--- a/tests/test_revision.py
+++ b/tests/test_revision.py
@@ -1575,7 +1575,7 @@ class NormalizedDownRevTest(DownIterateTest):
                 Revision("b2", "b1", dependencies="a3"),
                 Revision("b3", "b2"),
                 Revision("b4", "b3", dependencies="a3"),
-                Revision("b5", "b4"),
+                Revision("b5", "b4", dependencies="b4"),
             ]
         )
 
@@ -1587,6 +1587,11 @@ class NormalizedDownRevTest(DownIterateTest):
         # "a3" is not included because ancestor b2 is also dependent
         eq_(b4._normalized_down_revisions, ("b3",))
 
+    def test_dupe_dependency(self):
+        b5 = self.map.get_revision("b5")
+        eq_(b5._all_down_revisions, ("b4",))
+        eq_(b5._normalized_down_revisions, ("b4",))
+
     def test_branch_traversal(self):
         self._assert_iteration(
             "b4",
diff --git a/tests/test_version_traversal.py b/tests/test_version_traversal.py
index d3709d2a..e9399df6 100644
--- a/tests/test_version_traversal.py
+++ b/tests/test_version_traversal.py
@@ -1147,6 +1147,14 @@ class DependsOnOwnDownrevTest(MigrationTest):
             set(["a2"]),
         )
 
+    def test_traverse_down(self):
+        self._assert_downgrade(
+            self.a1.revision,
+            self.a2.revision,
+            [self.down_(self.a2)],
+            set(["a1"]),
+        )
+
 
 class DependsOnBranchTestFour(MigrationTest):
     @classmethod