9 files changed, 138 insertions, 112 deletions

diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py
index 0ba88e089..52cac414f 100644
--- a/src/mailman/config/config.py
+++ b/src/mailman/config/config.py
@@ -87,7 +87,6 @@ class Configuration:
         self.pipelines = {}
         self.commands = {}
         self.password_context = None
-        self.initialized = False
 
     def _clear(self):
         """Clear the cached configuration variables."""
@@ -137,7 +136,6 @@ class Configuration:
         # Expand and set up all directories.
         self._expand_paths()
         self.ensure_directories_exist()
-        self.initialized = True
         notify(ConfigurationUpdatedEvent(self))
 
     def _expand_paths(self):
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index 3e628fa47..be5cdb395 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -226,6 +226,7 @@ debug: no
 # - archiver        --  All archiver output
 # - bounce          --  All bounce processing logs go here
 # - config          --  Configuration issues
+# - database        --  Database logging (SQLAlchemy and Alembic)
 # - debug           --  Only used for development
 # - error           --  All exceptions go to this log
 # - fromusenet      --  Information related to the Usenet to Mailman gateway
@@ -254,6 +255,8 @@ path: bounce.log
 
 [logging.config]
 
+[logging.database]
+
 [logging.debug]
 path: debug.log
 level: info
diff --git a/src/mailman/core/logging.py b/src/mailman/core/logging.py
index 591f3c996..f4b9a1da1 100644
--- a/src/mailman/core/logging.py
+++ b/src/mailman/core/logging.py
@@ -104,6 +104,27 @@ class ReopenableFileHandler(logging.Handler):
 
 
 
+def _init_logger(propagate, sub_name, log, logger_config):
+    # Get settings from log configuration file (or defaults).
+    log_format = logger_config.format
+    log_datefmt = logger_config.datefmt
+    # Propagation to the root logger is how we handle logging to stderr
+    # when the runners are not run as a subprocess of 'bin/mailman start'.
+    log.propagate = (as_boolean(logger_config.propagate)
+                     if propagate is None else propagate)
+    # Set the logger's level.
+    log.setLevel(as_log_level(logger_config.level))
+    # Create a formatter for this logger, then a handler, and link the
+    # formatter to the handler.
+    formatter = logging.Formatter(fmt=log_format, datefmt=log_datefmt)
+    path_str = logger_config.path
+    path_abs = os.path.normpath(os.path.join(config.LOG_DIR, path_str))
+    handler = ReopenableFileHandler(sub_name, path_abs)
+    _handlers[sub_name] = handler
+    handler.setFormatter(formatter)
+    log.addHandler(handler)
+
+
 def initialize(propagate=None):
     """Initialize all logs.
 
@@ -126,32 +147,18 @@ def initialize(propagate=None):
             continue
         if sub_name == 'locks':
             log = logging.getLogger('flufl.lock')
-        elif sub_name == 'database':
+        if sub_name == 'database':
+            # Set both the SQLAlchemy and Alembic logs to the mailman.database
+            # log configuration, essentially ignoring the alembic.cfg
+            # settings.  Do the SQLAlchemy one first, then let the Alembic one
+            # fall through to the common code path.
             log = logging.getLogger('sqlalchemy')
-        elif sub_name == 'dbmigration':
+            _init_logger(propagate, sub_name, log, logger_config)
             log = logging.getLogger('alembic')
         else:
             logger_name = 'mailman.' + sub_name
             log = logging.getLogger(logger_name)
-        # Get settings from log configuration file (or defaults).
-        log_format = logger_config.format
-        log_datefmt = logger_config.datefmt
-        # Propagation to the root logger is how we handle logging to stderr
-        # when the runners are not run as a subprocess of 'bin/mailman start'.
-        log.propagate = (as_boolean(logger_config.propagate)
-                         if propagate is None else propagate)
-        # Set the logger's level.
-        log.setLevel(as_log_level(logger_config.level))
-        # Create a formatter for this logger, then a handler, and link the
-        # formatter to the handler.
-        formatter = logging.Formatter(fmt=log_format, datefmt=log_datefmt)
-        path_str = logger_config.path
-        path_abs = os.path.normpath(os.path.join(config.LOG_DIR, path_str))
-        handler = ReopenableFileHandler(sub_name, path_abs)
-        _handlers[sub_name] = handler
-        handler.setFormatter(formatter)
-        log.addHandler(handler)
-
+        _init_logger(propagate, sub_name, log, logger_config)
 
 
 def reopen():
diff --git a/src/mailman/database/alembic/__init__.py b/src/mailman/database/alembic/__init__.py
index a2f7418ba..9ac7f1311 100644
--- a/src/mailman/database/alembic/__init__.py
+++ b/src/mailman/database/alembic/__init__.py
@@ -15,18 +15,18 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"Alembic config init."
+"""Alembic configuration initization."""
 
 from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
-    'alembic_cfg'
-]
+    'alembic_cfg',
+    ]
 
 
 from alembic.config import Config
 from mailman.utilities.modules import expand_path
 
 
-alembic_cfg=Config(expand_path("python:mailman.config.schema"))
+alembic_cfg = Config(expand_path("python:mailman.config.schema"))
diff --git a/src/mailman/database/alembic/env.py b/src/mailman/database/alembic/env.py
index d1a1c557c..125868566 100644
--- a/src/mailman/database/alembic/env.py
+++ b/src/mailman/database/alembic/env.py
@@ -30,15 +30,10 @@ from alembic import context
 from contextlib import closing
 from sqlalchemy import create_engine
 
-from mailman.core import initialize
 from mailman.config import config
-from mailman.database.alembic import alembic_cfg
 from mailman.database.model import Model
 from mailman.utilities.string import expand
 
-if not config.initialized:
-    initialize.initialize_1(context.config.config_file_name)
-
 
 
 def run_migrations_offline():
diff --git a/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py b/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py
index 226bff7f6..f29809523 100644
--- a/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py
+++ b/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py
@@ -1,11 +1,40 @@
-"""initial
+# Copyright (C) 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/>.
+
+"""Initial migration.
+
+This empty migration file makes sure there is always an alembic_version
+in the database.  As a consequence, if the database version is reported
+as None, it means the database needs to be created from scratch with
+SQLAlchemy itself.
+
+It also removes schema items left over from Storm.
 
 Revision ID: 51b7f92bd06c
 Revises: None
 Create Date: 2014-10-10 09:53:35.624472
-
 """
 
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    ]
+
 # revision identifiers, used by Alembic.
 revision = '51b7f92bd06c'
 down_revision = None
@@ -15,20 +44,16 @@ import sqlalchemy as sa
 
 
 def upgrade():
-    ### commands auto generated by Alembic - please adjust! ###
     op.drop_table('version')
     if op.get_bind().dialect.name != "sqlite":
         # SQLite does not support dropping columns
         op.drop_column('mailinglist', 'acceptable_aliases_id')
     op.create_index(op.f('ix_user__user_id'), 'user', ['_user_id'], unique=False)
     op.drop_index('ix_user_user_id', table_name='user')
-    ### end Alembic commands ###
 
 
 def downgrade():
-    ### commands auto generated by Alembic - please adjust! ###
     op.create_table('version')
     op.create_index('ix_user_user_id', 'user', ['_user_id'], unique=False)
     op.drop_index(op.f('ix_user__user_id'), table_name='user')
     op.add_column('mailinglist', sa.Column('acceptable_aliases_id', sa.INTEGER(), nullable=True))
-    ### end Alembic commands ###
diff --git a/src/mailman/database/factory.py b/src/mailman/database/factory.py
index 8a5909283..6d8ae143c 100644
--- a/src/mailman/database/factory.py
+++ b/src/mailman/database/factory.py
@@ -29,7 +29,7 @@ __all__ = [
 import os
 import types
 
-from alembic import command
+import alembic.command
 from alembic.migration import MigrationContext
 from alembic.script import ScriptDirectory
 from flufl.lock import Lock
@@ -38,10 +38,14 @@ from zope.interface import implementer
 from zope.interface.verify import verifyObject
 
 from mailman.config import config
-from mailman.database.model import Model
 from mailman.database.alembic import alembic_cfg
-from mailman.interfaces.database import IDatabase, IDatabaseFactory
-from mailman.utilities.modules import call_name, expand_path
+from mailman.database.model import Model
+from mailman.interfaces.database import (
+    DatabaseError, IDatabase, IDatabaseFactory)
+from mailman.utilities.modules import call_name
+
+
+LAST_STORM_SCHEMA_VERSION = '20130406000000'
 
 
 
@@ -57,67 +61,58 @@ class DatabaseFactory:
             database = call_name(database_class)
             verifyObject(IDatabase, database)
             database.initialize()
-            schema_mgr = SchemaManager(database)
-            schema_mgr.setup_db()
+            SchemaManager(database).setup_database()
             database.commit()
             return database
 
 
 
 class SchemaManager:
-
-    LAST_STORM_SCHEMA_VERSION = '20130406000000'
+    "Manage schema migrations."""
 
     def __init__(self, database):
-        self.database = database
-        self.script = ScriptDirectory.from_config(alembic_cfg)
+        self._database = database
+        self._script = ScriptDirectory.from_config(alembic_cfg)
 
-    def get_storm_schema_version(self):
-        md = MetaData()
-        md.reflect(bind=self.database.engine)
-        if "version" not in md.tables:
+    def _get_storm_schema_version(self):
+        metadata = MetaData()
+        metadata.reflect(bind=self._database.engine)
+        if 'version' not in metadata.tables:
+            # There are no Storm artifacts left.
             return None
-        Version = md.tables["version"]
-        last_version = self.database.store.query(Version.c.version).filter(
-                Version.c.component == "schema"
-                ).order_by(Version.c.version.desc()).first()
+        Version = metadata.tables['version']
+        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()
+        self._database.commit()
         return last_version
 
-    def _create(self):
-        # initial DB creation
-        Model.metadata.create_all(self.database.engine)
-        self.database.commit()
-        command.stamp(alembic_cfg, "head")
-
-    def _upgrade(self):
-        command.upgrade(alembic_cfg, "head")
-
-    def setup_db(self):
-        context = MigrationContext.configure(self.database.store.connection())
+    def setup_database(self):
+        context = MigrationContext.configure(self._database.store.connection())
         current_rev = context.get_current_revision()
-        head_rev = self.script.get_current_head()
+        head_rev = self._script.get_current_head()
         if current_rev == head_rev:
-            return head_rev # already at the latest revision, nothing to do
-        if current_rev == None:
-            # no alembic information
-            storm_version = self.get_storm_schema_version()
+             # We're already at the latest revision so there's nothing to do.
+            return head_rev
+        if current_rev is None:
+            # No Alembic information is available.
+            storm_version = self._get_storm_schema_version()
             if storm_version is None:
-                # initial DB creation
-                self._create()
+                # Initial database creation.
+                Model.metadata.create_all(self._database.engine)
+                self._database.commit()
+                alembic.command.stamp(alembic_cfg, 'head')
             else:
-                # DB from a previous version managed by Storm
-                if storm_version.version < self.LAST_STORM_SCHEMA_VERSION:
-                    raise RuntimeError(
-                            "Upgrading while skipping beta version is "
-                            "unsupported, please install the previous "
-                            "Mailman beta release")
-                # Run migrations to remove the Storm-specific table and
-                # upgrade to SQLAlchemy & Alembic
-                self._upgrade()
+                # The database was previously managed by Storm.
+                if storm_version.version < LAST_STORM_SCHEMA_VERSION:
+                    raise DatabaseError(
+                        'Upgrades skipping beta versions is not supported.')
+                # Run migrations to remove the Storm-specific table and upgrade
+                # to SQLAlchemy and Alembic.
+                alembic.command.upgrade(alembic_cfg, 'head')
         elif current_rev != head_rev:
-            self._upgrade()
+            alembic.command.upgrade(alembic_cfg, 'head')
         return head_rev
 
 
diff --git a/src/mailman/database/tests/test_factory.py b/src/mailman/database/tests/test_factory.py
index bb37d01c9..461d57128 100644
--- a/src/mailman/database/tests/test_factory.py
+++ b/src/mailman/database/tests/test_factory.py
@@ -28,14 +28,16 @@ import unittest
 import types
 
 import alembic.command
-from mock import Mock
+from mock import patch
 from sqlalchemy import MetaData, Table, Column, Integer, Unicode
 from sqlalchemy.schema import Index
 from sqlalchemy.exc import ProgrammingError, OperationalError
 
 from mailman.config import config
+from mailman.interfaces.database import DatabaseError
 from mailman.testing.layers import ConfigLayer
-from mailman.database.factory import SchemaManager, _reset
+from mailman.database.factory import (
+    SchemaManager, _reset, LAST_STORM_SCHEMA_VERSION)
 from mailman.database.sqlite import SQLiteDatabase
 from mailman.database.alembic import alembic_cfg
 from mailman.database.model import Model
@@ -110,41 +112,40 @@ class TestSchemaManager(unittest.TestCase):
     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)
+        with patch("alembic.command") as alembic_command:
+            self.schema_mgr.setup_database()
+            self.assertFalse(alembic_command.stamp.called)
+            self.assertFalse(alembic_command.upgrade.called)
 
-    def test_initial(self):
+    @patch("alembic.command")
+    def test_initial(self, alembic_command):
         """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.schema_mgr.setup_database()
+        self.assertFalse(alembic_command.upgrade.called)
         self.assertTrue(self._table_exists("mailinglist"))
         self.assertTrue(self._table_exists("alembic_version"))
 
-    def test_storm(self):
+    @patch("alembic.command.stamp")
+    def test_storm(self, alembic_command_stamp):
         """Existing Storm database"""
         Model.metadata.create_all(config.db.engine)
-        self._create_storm_database(
-                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._create_storm_database(LAST_STORM_SCHEMA_VERSION)
+        self.schema_mgr.setup_database()
+        self.assertFalse(alembic_command_stamp.called)
         self.assertTrue(self._table_exists("mailinglist")
                     and self._table_exists("alembic_version")
                     and not self._table_exists("version"))
 
-    def test_old_storm(self):
+    @patch("alembic.command")
+    def test_old_storm(self, alembic_command):
         """Existing Storm database in an old version"""
         Model.metadata.create_all(config.db.engine)
         self._create_storm_database("001")
-        self.schema_mgr._create = Mock()
-        self.assertRaises(RuntimeError, self.schema_mgr.setup_db)
-        self.assertFalse(self.schema_mgr._create.called)
+        self.assertRaises(DatabaseError, self.schema_mgr.setup_database)
+        self.assertFalse(alembic_command.stamp.called)
+        self.assertFalse(alembic_command.upgrade.called)
 
     def test_old_db(self):
         """The database is in an old revision, must upgrade"""
@@ -155,8 +156,7 @@ class TestSchemaManager(unittest.TestCase):
         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)
+        with patch("alembic.command") as alembic_command:
+            self.schema_mgr.setup_database()
+            self.assertFalse(alembic_command.stamp.called)
+            self.assertTrue(alembic_command.upgrade.called)
diff --git a/src/mailman/testing/testing.cfg b/src/mailman/testing/testing.cfg
index ea2df6988..466e4e219 100644
--- a/src/mailman/testing/testing.cfg
+++ b/src/mailman/testing/testing.cfg
@@ -20,7 +20,7 @@
 # For testing against PostgreSQL.
 # [database]
 # class: mailman.database.postgresql.PostgreSQLDatabase
-# url: postgresql://maxking:maxking@localhost/mailman_test
+# url: postgresql://$USER:$USER@localhost/mailman_test
 
 [mailman]
 site_owner: noreply@example.com
@@ -87,3 +87,6 @@ charset: euc-jp
 [language.fr]
 description: French
 charset: iso-8859-1
+
+[alembic]
+script_location = mailman.database:alembic