Raise for non-string revision identifier

Added an assertion in :meth:`.RevisionMap.get_revisions` and other methods
which ensures revision numbers are passed as strings or collections of
strings.   Driver issues particularly on MySQL may inadvertently be passing
bytes here which leads to failures later on.

Change-Id: Id80c958f0a082fed26cac2cf838cb7507b8d814c
Fixes: #551

diff --git a/alembic/script/revision.py b/alembic/script/revision.py
index af08688d994ad012fac6d481114fcd5629a5affa..43c757efad2d166e0897c8fd362dbaaa8ef47a25 100644 (file)
--- a/alembic/script/revision.py
+++ b/alembic/script/revision.py
@@ -314,6 +314,7 @@ class RevisionMap(object):
         full revision.
 
         """
+
         if isinstance(id_, (list, tuple, set, frozenset)):
             return sum([self.get_revisions(id_elem) for id_elem in id_], ())
         else:
@@ -469,6 +470,20 @@ class RevisionMap(object):
     def _resolve_revision_number(self, id_):
         if isinstance(id_, compat.string_types) and "@" in id_:
             branch_label, id_ = id_.split("@", 1)
+
+        elif id_ is not None and (
+            (
+                isinstance(id_, tuple)
+                and id_
+                and not isinstance(id_[0], compat.string_types)
+            )
+            or not isinstance(id_, compat.string_types + (tuple, ))
+        ):
+            raise RevisionError(
+                "revision identifier %r is not a string; ensure database "
+                "driver settings are correct" % (id_,)
+            )
+
         else:
             branch_label = None
 

diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py
index 24f86672e0cfcda82c07960dc32ccf05150e3c87..4e88a9b0163cc4b0d75d4eb8d7fb2a5ea4344ca8 100644 (file)
--- a/alembic/testing/requirements.py
+++ b/alembic/testing/requirements.py
@@ -1,3 +1,5 @@
+import sys
+
 from alembic import util
 from alembic.util import sqla_compat
 from . import exclusions
@@ -153,6 +155,12 @@ class SuiteRequirements(Requirements):
             "SQLAlchemy 1.2.16, 1.3.0b2 or greater required",
         )
 
+    @property
+    def python3(self):
+        return exclusions.skip_if(
+            lambda: sys.version_info < (3,), "Python version 3.xx is required."
+        )
+
     @property
     def pep3147(self):
 

diff --git a/docs/build/unreleased/551.rst b/docs/build/unreleased/551.rst
new file mode 100644 (file)
index 0000000..043a162
--- /dev/null
+++ b/docs/build/unreleased/551.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, operations, mysql
+    :tickets: 551
+
+    Added an assertion in :meth:`.RevisionMap.get_revisions` and other methods
+    which ensures revision numbers are passed as strings or collections of
+    strings.   Driver issues particularly on MySQL may inadvertently be passing
+    bytes here which leads to failures later on.
\ No newline at end of file

diff --git a/tests/test_revision.py b/tests/test_revision.py
index 20eb309a602dc537f39b695670de13d2505c46e0..41d8b42ad8f2f2a014f89515859c5b41e37631eb 100644 (file)
--- a/tests/test_revision.py
+++ b/tests/test_revision.py
@@ -3,12 +3,46 @@ from alembic.script.revision import Revision
 from alembic.script.revision import RevisionError
 from alembic.script.revision import RevisionMap
 from alembic.testing import assert_raises_message
+from alembic.testing import config
 from alembic.testing import eq_
 from alembic.testing.fixtures import TestBase
 from . import _large_map
 
 
 class APITest(TestBase):
+    @config.requirements.python3
+    def test_invalid_datatype(self):
+        map_ = RevisionMap(
+            lambda: [
+                Revision("a", ()),
+                Revision("b", ("a",)),
+                Revision("c", ("b",)),
+            ]
+        )
+        assert_raises_message(
+            RevisionError,
+            "revision identifier b'12345' is not a string; "
+            "ensure database driver settings are correct",
+            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'
+        )
+
+        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(("a", ))
+        map_.get_revision("a")
+
     def test_add_revision_one_head(self):
         map_ = RevisionMap(
             lambda: [