merge branch from abompard

5 files changed, 148 insertions, 19 deletions

diff --git a/src/mailman/database/alembic/env.py b/src/mailman/database/alembic/env.py
index 269dcf835..56f673803 100644
--- a/src/mailman/database/alembic/env.py
+++ b/src/mailman/database/alembic/env.py
@@ -28,7 +28,6 @@ __all__ = [
 
 from alembic import context
 from contextlib import closing
-from logging.config import fileConfig
 from sqlalchemy import create_engine
 
 from mailman.core import initialize
@@ -38,8 +37,6 @@ from mailman.database.model import Model
 from mailman.utilities.string import expand
 
 
-fileConfig(alembic_cfg.config_file_name)
-
 
 def run_migrations_offline():
     """Run migrations in 'offline' mode.
diff --git a/src/mailman/database/base.py b/src/mailman/database/base.py
index dcaedade0..6e86faf04 100644
--- a/src/mailman/database/base.py
+++ b/src/mailman/database/base.py
@@ -31,7 +31,6 @@ from sqlalchemy.orm import sessionmaker
 from zope.interface import implementer
 
 from mailman.config import config
-from mailman.database.alembic import alembic_cfg
 from mailman.interfaces.database import IDatabase
 from mailman.utilities.string import expand
 
@@ -91,15 +90,6 @@ class SABaseDatabase:
         """
         pass
 
-    def stamp(self, debug=False):
-        """Stamp the database with the latest Alembic version."""
-        # Newly created databases don't need migrations from Alembic, since
-        # create_all() ceates the latest schema.  This patches the database
-        # with the latest Alembic version to add an entry in the
-        # alembic_version table.
-        command.stamp(alembic_cfg, 'head')
-
-
     def initialize(self, debug=None):
         """See `IDatabase`."""
         # Calculate the engine url.
diff --git a/src/mailman/database/factory.py b/src/mailman/database/factory.py
index 469ed5d18..ea2048143 100644
--- a/src/mailman/database/factory.py
+++ b/src/mailman/database/factory.py
@@ -70,8 +70,7 @@ class SchemaManager:
 
     def __init__(self, database):
         self.database = database
-        self.alembic_cfg = alembic_cfg
-        self.script = ScriptDirectory.from_config(self.alembic_cfg)
+        self.script = ScriptDirectory.from_config(alembic_cfg)
 
     def get_storm_schema_version(self):
         md = MetaData()
@@ -82,8 +81,18 @@ class SchemaManager:
         last_version = self.database.store.query(Version.c.version).filter(
                 Version.c.component == "schema"
                 ).order_by(Version.c.version.desc()).first()
+        # Don't leave open transactions or they will block any schema change
+        self.database.commit()
         return last_version
 
+    def _create(self):
+        # initial DB creation
+        Model.metadata.create_all(self.database.engine)
+        command.stamp(alembic_cfg, "head")
+
+    def _upgrade(self):
+        command.upgrade(alembic_cfg, "head")
+
     def setup_db(self):
         context = MigrationContext.configure(self.database.store.connection())
         current_rev = context.get_current_revision()
@@ -95,8 +104,7 @@ class SchemaManager:
             storm_version = self.get_storm_schema_version()
             if storm_version is None:
                 # initial DB creation
-                Model.metadata.create_all(self.database.engine)
-                command.stamp(self.alembic_cfg, "head")
+                self._create()
             else:
                 # DB from a previous version managed by Storm
                 if storm_version.version < self.LAST_STORM_SCHEMA_VERSION:
@@ -106,9 +114,9 @@ class SchemaManager:
                             "Mailman beta release")
                 # Run migrations to remove the Storm-specific table and
                 # upgrade to SQLAlchemy & Alembic
-                command.upgrade(self.alembic_cfg, "head")
+                self._upgrade()
         elif current_rev != head_rev:
-            command.upgrade(self.alembic_cfg, "head")
+            self._upgrade()
         return head_rev
 
 
diff --git a/src/mailman/database/tests/__init__.py b/src/mailman/database/tests/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/src/mailman/database/tests/__init__.py
diff --git a/src/mailman/database/tests/test_factory.py b/src/mailman/database/tests/test_factory.py
new file mode 100644
index 000000000..a87bca7be
--- /dev/null
+++ b/src/mailman/database/tests/test_factory.py
@@ -0,0 +1,134 @@
+# Copyright (C) 2013-2014 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test database schema migrations"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    ]
+
+
+import unittest
+import types
+
+import alembic.command
+from mock import Mock
+from sqlalchemy import MetaData, Table, Column, Integer, Unicode
+
+from mailman.config import config
+from mailman.testing.layers import ConfigLayer
+from mailman.database.factory import SchemaManager, _reset
+from mailman.database.sqlite import SQLiteDatabase
+from mailman.database.alembic import alembic_cfg
+from mailman.database.model import Model
+
+
+
+class TestSchemaManager(unittest.TestCase):
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        # Drop the existing database
+        Model.metadata.drop_all(config.db.engine)
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        if "alembic_version" in md.tables:
+            md.tables["alembic_version"].drop(config.db.engine)
+        self.schema_mgr = SchemaManager(config.db)
+
+    def tearDown(self):
+        if "version" in Model.metadata.tables:
+            version = Model.metadata.tables["version"]
+            version.drop(config.db.engine, checkfirst=True)
+            Model.metadata.remove(version)
+        # Restore a virgin DB
+        Model.metadata.create_all(config.db.engine)
+
+
+    def _table_exists(self, tablename):
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        return tablename in md.tables
+
+    def _create_storm_version_table(self, revision):
+        version_table = Table("version", Model.metadata,
+                Column("id", Integer, primary_key=True),
+                Column("component", Unicode),
+                Column("version", Unicode),
+                )
+        version_table.create(config.db.engine)
+        config.db.store.execute(version_table.insert().values(
+                component='schema', version=revision))
+        config.db.commit()
+
+
+    def test_current_db(self):
+        """The database is already at the latest version"""
+        alembic.command.stamp(alembic_cfg, "head")
+        self.schema_mgr._create = Mock()
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertFalse(self.schema_mgr._upgrade.called)
+
+    def test_initial(self):
+        """No existing database"""
+        self.assertFalse(self._table_exists("mailinglist"))
+        self.assertFalse(self._table_exists("alembic_version"))
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._upgrade.called)
+        self.assertTrue(self._table_exists("mailinglist"))
+        self.assertTrue(self._table_exists("alembic_version"))
+
+    def test_storm(self):
+        """Existing Storm database"""
+        Model.metadata.create_all(config.db.engine)
+        self._create_storm_version_table(
+                self.schema_mgr.LAST_STORM_SCHEMA_VERSION)
+        self.schema_mgr._create = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertTrue(self._table_exists("mailinglist")
+                    and self._table_exists("alembic_version")
+                    and not self._table_exists("version"))
+
+    def test_old_storm(self):
+        """Existing Storm database in an old version"""
+        Model.metadata.create_all(config.db.engine)
+        self._create_storm_version_table("001")
+        self.schema_mgr._create = Mock()
+        self.assertRaises(RuntimeError, self.schema_mgr.setup_db)
+        self.assertFalse(self.schema_mgr._create.called)
+
+    def test_old_db(self):
+        """The database is in an old revision, must upgrade"""
+        alembic.command.stamp(alembic_cfg, "head")
+        md = MetaData()
+        md.reflect(bind=config.db.engine)
+        config.db.store.execute(md.tables["alembic_version"].delete())
+        config.db.store.execute(md.tables["alembic_version"].insert().values(
+                version_num="dummyrevision"))
+        config.db.commit()
+        self.schema_mgr._create = Mock()
+        self.schema_mgr._upgrade = Mock()
+        self.schema_mgr.setup_db()
+        self.assertFalse(self.schema_mgr._create.called)
+        self.assertTrue(self.schema_mgr._upgrade.called)