From: Jiri Kuncar <jiri.kuncar@cern.ch>
Date: Fri, 18 Nov 2016 18:40:24 +0000 (-0500)
Subject: Detect and ignore duplicate revision files on read
X-Git-Tag: rel_0_8_9~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=685931d33854c887689754a4e8d8db061702bf6e;p=thirdparty%2Fsqlalchemy%2Falembic.git

Detect and ignore duplicate revision files on read

Added an additional check when reading in revision files to detect
if the same file is being read twice; this can occur if the same directory
or a symlink equivalent is present more than once in version_locations.
A warning is now emitted and the file is skipped.  Pull request courtesy
Jiri Kuncar.

Change-Id: I10ffc8eff420d18c55d3533afb9d5935bbadfe32
Pull-request: https://github.com/zzzeek/alembic/pull/30
---

diff --git a/alembic/script/base.py b/alembic/script/base.py
index a9b6e7df..3a0a6fd9 100644
--- a/alembic/script/base.py
+++ b/alembic/script/base.py
@@ -82,8 +82,17 @@ class ScriptDirectory(object):
         else:
             paths = [self.versions]
 
+        dupes = set()
         for vers in paths:
             for file_ in os.listdir(vers):
+                path = os.path.realpath(os.path.join(vers, file_))
+                if path in dupes:
+                    util.warn(
+                        "File %s loaded twice! ignoring. Please ensure "
+                        "version_locations is unique." % path
+                    )
+                    continue
+                dupes.add(path)
                 script = Script._from_filename(self, vers, file_)
                 if script is None:
                     continue
diff --git a/alembic/testing/env.py b/alembic/testing/env.py
index 015d0bd4..91c7fa53 100644
--- a/alembic/testing/env.py
+++ b/alembic/testing/env.py
@@ -115,7 +115,7 @@ datefmt = %%H:%%M:%%S
 
 
 
-def _multi_dir_testing_config(sourceless=False):
+def _multi_dir_testing_config(sourceless=False, extra_version_location=''):
     dir_ = os.path.join(_get_staging_directory(), 'scripts')
     url = "sqlite:///%s/foo.db" % dir_
 
@@ -124,7 +124,7 @@ def _multi_dir_testing_config(sourceless=False):
 script_location = %s
 sqlalchemy.url = %s
 sourceless = %s
-version_locations = %%(here)s/model1/ %%(here)s/model2/ %%(here)s/model3/
+version_locations = %%(here)s/model1/ %%(here)s/model2/ %%(here)s/model3/ %s
 
 [loggers]
 keys = root
@@ -149,7 +149,8 @@ keys = generic
 [formatter_generic]
 format = %%(levelname)-5.5s [%%(name)s] %%(message)s
 datefmt = %%H:%%M:%%S
-    """ % (dir_, url, "true" if sourceless else "false"))
+    """ % (dir_, url, "true" if sourceless else "false",
+           extra_version_location))
 
 
 def _no_sql_testing_config(dialect="postgresql", directives=""):
diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index f8a89be2..f10a61ea 100644
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -6,6 +6,15 @@ Changelog
 .. changelog::
     :version: 0.8.9
 
+    .. change::
+      :tags: bug, revisioning
+
+      Added an additional check when reading in revision files to detect
+      if the same file is being read twice; this can occur if the same directory
+      or a symlink equivalent is present more than once in version_locations.
+      A warning is now emitted and the file is skipped.  Pull request courtesy
+      Jiri Kuncar.
+
     .. change::
       :tags: bug, autogenerate
       :tickets: 395
diff --git a/tests/test_script_production.py b/tests/test_script_production.py
index 66e311d1..d32a94c7 100644
--- a/tests/test_script_production.py
+++ b/tests/test_script_production.py
@@ -1,5 +1,5 @@
 from alembic.testing.fixtures import TestBase
-from alembic.testing import eq_, ne_, assert_raises_message, is_
+from alembic.testing import eq_, ne_, assert_raises_message, is_, assertions
 from alembic.testing.env import clear_staging_env, staging_env, \
     _get_staging_directory, _no_sql_testing_config, env_file_fixture, \
     script_file_fixture, _testing_config, _sqlite_testing_config, \
@@ -837,3 +837,63 @@ context.configure(dialect_name='sqlite', template_args={"somearg":"somevalue"})
             contents = open(m.group(1)).read()
             os.remove(m.group(1))
             assert "<% z = x + y %>" in contents
+
+
+class DuplicateVersionLocationsTest(TestBase):
+
+    def setUp(self):
+        self.env = staging_env()
+        self.cfg = _multi_dir_testing_config(
+            # this is a duplicate of one of the paths
+            # already present in this fixture
+            extra_version_location='%(here)s/model1'
+        )
+
+        script = ScriptDirectory.from_config(self.cfg)
+        self.model1 = util.rev_id()
+        self.model2 = util.rev_id()
+        self.model3 = util.rev_id()
+        for model, name in [
+            (self.model1, "model1"),
+            (self.model2, "model2"),
+            (self.model3, "model3"),
+        ]:
+            script.generate_revision(
+                model, name, refresh=True,
+                version_path=os.path.join(_get_staging_directory(), name),
+                head="base")
+            write_script(script, model, """\
+"%s"
+revision = '%s'
+down_revision = None
+branch_labels = ['%s']
+
+from alembic import op
+
+def upgrade():
+    pass
+
+def downgrade():
+    pass
+
+""" % (name, model, name))
+
+    def tearDown(self):
+        clear_staging_env()
+
+    def test_env_emits_warning(self):
+        with assertions.expect_warnings(
+            "File %s loaded twice! ignoring. "
+            "Please ensure version_locations is unique" % (
+                os.path.realpath(os.path.join(
+                _get_staging_directory(),
+                "model1",
+                "%s_model1.py" % self.model1
+                )))
+        ):
+            script = ScriptDirectory.from_config(self.cfg)
+            script.revision_map.heads
+            eq_(
+                [rev.revision for rev in script.walk_revisions()],
+                [self.model1, self.model2, self.model3]
+            )
\ No newline at end of file