From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 17 Sep 2019 22:25:41 +0000 (-0400)
Subject: Only allow partial revision match for > 3 characters
X-Git-Tag: rel_1_2_0~12^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b162e7d8fb6ecb7e65b56f33803eb70c2249a273;p=thirdparty%2Fsqlalchemy%2Falembic.git

Only allow partial revision match for > 3 characters

Made the command interface revision lookup behavior more strict in that an
Alembic revision number is only resolved based on a partial match rules if
it has at least four characters, to prevent simple typographical issues
from inadvertently  running migrations.

Change-Id: I9c4c87bb3fdb78d2dd264f37c99422df309c4e5c
Fixes: #534
---

diff --git a/alembic/script/revision.py b/alembic/script/revision.py
index 43c757ef..22481a08 100644
--- a/alembic/script/revision.py
+++ b/alembic/script/revision.py
@@ -377,13 +377,23 @@ class RevisionMap(object):
             revs = [
                 x
                 for x in self._revision_map
-                if x and x.startswith(resolved_id)
+                if x and len(x) > 3 and x.startswith(resolved_id)
             ]
+
             if branch_rev:
                 revs = self.filter_for_lineage(revs, check_branch)
             if not revs:
                 raise ResolutionError(
-                    "No such revision or branch '%s'" % resolved_id,
+                    "No such revision or branch '%s'%s"
+                    % (
+                        resolved_id,
+                        (
+                            "; please ensure at least four characters are "
+                            "present for partial revision identifier matches"
+                            if len(resolved_id) < 4
+                            else ""
+                        ),
+                    ),
                     resolved_id,
                 )
             elif len(revs) > 1:
@@ -477,7 +487,7 @@ class RevisionMap(object):
                 and id_
                 and not isinstance(id_[0], compat.string_types)
             )
-            or not isinstance(id_, compat.string_types + (tuple, ))
+            or not isinstance(id_, compat.string_types + (tuple,))
         ):
             raise RevisionError(
                 "revision identifier %r is not a string; ensure database "
diff --git a/docs/build/unreleased/534.rst b/docs/build/unreleased/534.rst
new file mode 100644
index 00000000..5bf93617
--- /dev/null
+++ b/docs/build/unreleased/534.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: usecase, commands
+    :tickets: 534
+
+    Made the command interface revision lookup behavior more strict in that an
+    Alembic revision number is only resolved based on a partial match rules if
+    it has at least four characters, to prevent simple typographical issues
+    from inadvertently  running migrations.
diff --git a/tests/test_revision.py b/tests/test_revision.py
index 41d8b42a..bf433f51 100644
--- a/tests/test_revision.py
+++ b/tests/test_revision.py
@@ -23,24 +23,27 @@ class APITest(TestBase):
             RevisionError,
             "revision identifier b'12345' is not a string; "
             "ensure database driver settings are correct",
-            map_.get_revisions, b'12345'
+            map_.get_revisions,
+            b"12345",
         )
 
         assert_raises_message(
             RevisionError,
             "revision identifier b'12345' is not a string; "
             "ensure database driver settings are correct",
-            map_.get_revision, b'12345'
+            map_.get_revision,
+            b"12345",
         )
 
         assert_raises_message(
             RevisionError,
             r"revision identifier \(b'12345',\) is not a string; "
             "ensure database driver settings are correct",
-            map_.get_revision, (b'12345', )
+            map_.get_revision,
+            (b"12345",),
         )
 
-        map_.get_revision(("a", ))
+        map_.get_revision(("a",))
         map_.get_revision("a")
 
     def test_add_revision_one_head(self):
@@ -336,6 +339,16 @@ class LabeledBranchTest(DownIterateTest):
         eq_(self.map.get_revision("ebranch@some").revision, "someothername")
         eq_(self.map.get_revision("abranch@some").revision, "somelongername")
 
+    def test_partial_id_resolve_too_short(self):
+        assert_raises_message(
+            RevisionError,
+            "No such revision or branch 'sos'; please ensure at least "
+            "four characters are present for partial revision identifier "
+            "matches",
+            self.map.get_revision,
+            "ebranch@sos",
+        )
+
     def test_branch_at_heads(self):
         eq_(self.map.get_revision("abranch@heads").revision, "c")
 
@@ -368,12 +381,24 @@ class LabeledBranchTest(DownIterateTest):
             "abranch@d",
         )
 
+    def test_actually_short_rev_name(self):
+        eq_(self.map.get_revision("e").revision, "e")
+
     def test_no_revision_exists(self):
         assert_raises_message(
             RevisionError,
-            "No such revision or branch 'q'",
+            "No such revision or branch 'qprstuv'$",
+            self.map.get_revision,
+            "abranch@qprstuv",
+        )
+
+        assert_raises_message(
+            RevisionError,
+            "No such revision or branch 'qpr'; please ensure at least "
+            "four characters are present for partial revision identifier "
+            "matches$",
             self.map.get_revision,
-            "abranch@q",
+            "abranch@qpr",
         )
 
     def test_not_actually_a_branch(self):