Skip to content

Commit

Permalink
ensure downrev/dependencies in tuple form before using "in"
Browse files Browse the repository at this point in the history
Fixed regression where new "loop detection" feature introduced in
:ticket:`757` produced false positives for revision names that have
overlapping substrings between revision number and down revision and/or
dependency, if the downrev/dependency were not in sequence form.

Fixes: #784
Change-Id: I9f31298c91ee2d3c1c63fd1f52bf8e0309b6ad88
  • Loading branch information
zzzeek committed Jan 20, 2021
1 parent e43d175 commit 3c0dac8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
6 changes: 4 additions & 2 deletions alembic/script/revision.py
Original file line number Diff line number Diff line change
Expand Up @@ -1037,9 +1037,11 @@ def verify_rev_id(cls, revision):
def __init__(
self, revision, down_revision, dependencies=None, branch_labels=None
):
if down_revision and revision in down_revision:
if down_revision and revision in util.to_tuple(down_revision):
raise LoopDetected(revision)
elif dependencies is not None and revision in dependencies:
elif dependencies is not None and revision in util.to_tuple(
dependencies
):
raise DependencyLoopDetected(revision)

self.verify_rev_id(revision)
Expand Down
8 changes: 8 additions & 0 deletions docs/build/unreleased/784.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.. change::
:tags: bug, versioning, regression
:tickets: 784

Fixed regression where new "loop detection" feature introduced in
:ticket:`757` produced false positives for revision names that have
overlapping substrings between revision number and down revision and/or
dependency, if the downrev/dependency were not in sequence form.
18 changes: 17 additions & 1 deletion tests/test_revision.py
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ def _assert_raises_revision_map_dep_cycle(self, map_, revisions):
)


class GraphWithLoopTest(InvalidRevisionMapTest):
class GraphWithLoopTest(DownIterateTest, InvalidRevisionMapTest):
def test_revision_map_solitary_loop(self):
map_ = RevisionMap(
lambda: [
Expand All @@ -1297,6 +1297,22 @@ def test_revision_map_solitary_loop(self):
)
self._assert_raises_revision_map_loop(map_, "a")

def test_revision_map_no_loop_w_overlapping_substrings(self):
r1 = Revision("user_foo", None)
r2 = Revision("user", "user_foo")

self.map = RevisionMap(lambda: [r1, r2])

self._assert_iteration("heads", None, ["user", "user_foo"])

def test_revision_map_no_loop_w_overlapping_substrings_dependencies(self):
r1 = Revision("user_foo", None)
r2 = Revision("user", None, dependencies="user_foo")

self.map = RevisionMap(lambda: [r1, r2])

self._assert_iteration("heads", None, ["user", "user_foo"])

def test_revision_map_base_loop(self):
map_ = RevisionMap(
lambda: [
Expand Down

0 comments on commit 3c0dac8

Please sign in to comment.