Refactor to better handling the difference between a testing database and a

production database.

- Add an IDatabaseFactory interface with two named utility implementations.
  The initialization subsystem will either ask for the 'testing' or
  'production' factory utility depending on whether we're in the test suite or
  not.  The testing factory returns an IDatabase that can be _reset().

- initialize_2() now takes an optional `testing` argument, defaulting to
  False.  The test ConfigLayer will pass in True.

- Remove _reset() from the base database class.

- The ModelMeta now adds a PRESERVE attribute to database classes.  This
  defaults to False, meaning by default the test framework will reset the
  table.  The Version table is preserved because it records the schema
  migrations.

- Because of the above, we no longer need to support pre_reset() and
  post_reset() on migrations.

Also, bin/mailman should allow the standard configuration file search
algorithm to be used except when -C/--config is given.

13 files changed, 129 insertions, 137 deletions

diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py
index 94de65255..6b15c9838 100644
--- a/src/mailman/bin/mailman.py
+++ b/src/mailman/bin/mailman.py
@@ -90,13 +90,9 @@ def main():
         # No arguments or subcommands were given.
         parser.print_help()
         parser.exit()
-    # Before actually performing the subcommand, we need to initialize the
-    # Mailman system, and in particular, we must read the configuration file.
-    config_file = os.getenv('MAILMAN_CONFIG_FILE')
-    if config_file is None:
-        if args.config is not None:
-            config_file = os.path.abspath(os.path.expanduser(args.config))
-
-    initialize(config_file)
+    # Initialize the system.  Honor the -C flag if given.
+    config_path = (None if args.config is None
+                   else os.path.abspath(os.path.expanduser(args.config)))
+    initialize(config_path)
     # Perform the subcommand option.
     args.func(args)
diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml
index 8c362c31b..9e0d7c786 100644
--- a/src/mailman/config/configure.zcml
+++ b/src/mailman/config/configure.zcml
@@ -33,6 +33,18 @@
     />
 
   <utility
+    provides="mailman.interfaces.database.IDatabaseFactory"
+    factory="mailman.database.factory.DatabaseFactory"
+    name="production"
+    />
+
+  <utility
+    provides="mailman.interfaces.database.IDatabaseFactory"
+    factory="mailman.database.factory.DatabaseTestingFactory"
+    name="testing"
+    />
+
+  <utility
     provides="mailman.interfaces.domain.IDomainManager"
     factory="mailman.model.domain.DomainManager"
     />
diff --git a/src/mailman/core/initialize.py b/src/mailman/core/initialize.py
index b359928cc..3e927cc40 100644
--- a/src/mailman/core/initialize.py
+++ b/src/mailman/core/initialize.py
@@ -40,13 +40,13 @@ import os
 import sys
 
 from pkg_resources import resource_string
+from zope.component import getUtility
 from zope.configuration import xmlconfig
-from zope.interface.verify import verifyObject
 
 import mailman.config.config
 import mailman.core.logging
 
-from mailman.interfaces.database import IDatabase
+from mailman.interfaces.database import IDatabaseFactory
 from mailman.utilities.modules import call_name
 
 # The test infrastructure uses this to prevent the search and loading of any
@@ -125,7 +125,7 @@ def initialize_1(config_path=None):
     mailman.config.config.load(config_path)
 
 
-def initialize_2(debug=False, propagate_logs=None):
+def initialize_2(debug=False, propagate_logs=None, testing=False):
     """Second initialization step.
 
     * Database
@@ -149,13 +149,8 @@ def initialize_2(debug=False, propagate_logs=None):
         call_name(config.mailman.pre_hook)
     # Instantiate the database class, ensure that it's of the right type, and
     # initialize it.  Then stash the object on our configuration object.
-    database_class = config.database['class']
-    database = call_name(database_class)
-    verifyObject(IDatabase, database)
-    database.initialize(debug)
-    database.load_migrations()
-    database.commit()
-    config.db = database
+    utility_name = ('testing' if testing else 'production')
+    config.db = getUtility(IDatabaseFactory, utility_name).create()
     # Initialize the rules and chains.  Do the imports here so as to avoid
     # circular imports.
     from mailman.app.commands import initialize as initialize_commands
diff --git a/src/mailman/database/base.py b/src/mailman/database/base.py
index af035914b..39ce017c5 100644
--- a/src/mailman/database/base.py
+++ b/src/mailman/database/base.py
@@ -233,13 +233,6 @@ class StormBaseDatabase:
         # is used by the test suite to reset the database between tests.
         store.add(Version(component=version, version=module_path))
 
-    def _reset(self):
-        """See `IDatabase`."""
-        from mailman.database.model import ModelMeta
-        self.store.rollback()
-        ModelMeta._reset(self.store)
-        self.store.commit()
-
     @staticmethod
     def _make_temporary():
         raise NotImplementedError
diff --git a/src/mailman/database/factory.py b/src/mailman/database/factory.py
new file mode 100644
index 000000000..4783410cc
--- /dev/null
+++ b/src/mailman/database/factory.py
@@ -0,0 +1,80 @@
+# Copyright (C) 2012 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/>.
+
+"""Database factory."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'DatabaseFactory',
+    'DatabaseTestingFactory',
+    ]
+
+
+import types
+
+from zope.interface import implementer
+from zope.interface.verify import verifyObject
+
+from mailman.config import config
+from mailman.interfaces.database import IDatabase, IDatabaseFactory
+from mailman.utilities.modules import call_name
+
+
+
+@implementer(IDatabaseFactory)
+class DatabaseFactory:
+    """Create a new database."""
+
+    @staticmethod
+    def create():
+        """See `IDatabaseFactory`."""
+        database_class = config.database['class']
+        database = call_name(database_class)
+        verifyObject(IDatabase, database)
+        database.initialize()
+        database.load_migrations()
+        database.commit()
+        return database
+
+
+
+def _reset(self):
+    """See `IDatabase`."""
+    from mailman.database.model import ModelMeta
+    self.store.rollback()
+    ModelMeta._reset(self.store)
+    self.store.commit()
+
+
+@implementer(IDatabaseFactory)
+class DatabaseTestingFactory:
+    """Create a new database for testing."""
+
+    @staticmethod
+    def create():
+        """See `IDatabaseFactory`."""
+        database_class = config.database['class']
+        database = call_name(database_class)
+        verifyObject(IDatabase, database)
+        database.initialize()
+        database.load_migrations()
+        database.commit()
+        # Make _reset() a bound method of the database instance.
+        database._reset = types.MethodType(_reset, database)
+        return database
diff --git a/src/mailman/database/model.py b/src/mailman/database/model.py
index c45517c9b..227543351 100644
--- a/src/mailman/database/model.py
+++ b/src/mailman/database/model.py
@@ -25,8 +25,6 @@ __all__ = [
     ]
 
 
-import sys
-
 from operator import attrgetter
 
 from storm.properties import PropertyPublisherMeta
@@ -43,6 +41,9 @@ class ModelMeta(PropertyPublisherMeta):
         # property to enforce our table naming convention.
         self.__storm_table__ = name.lower()
         super(ModelMeta, self).__init__(name, bases, dict)
+        # By default, the table should be reset by the testing framework.
+        if not hasattr(self, 'PRESERVE'):
+            self.PRESERVE = False
         # Register the model class so that it can be more easily cleared.
         # This is required by the test framework.
         if name == 'Model':
@@ -52,38 +53,13 @@ class ModelMeta(PropertyPublisherMeta):
     @staticmethod
     def _reset(store):
         from mailman.config import config
-        from mailman.model.version import Version
         config.db._pre_reset(store)
-        # Give each schema migration a chance to do its pre-reset.  See below
-        # for calling its post reset too.
-        versions = sorted(version.version for version in
-                          store.find(Version, component='schema'))
-        migrations = {}
-        for version in versions:
-            # We have to give the migrations module that loaded this version a
-            # chance to do both pre- and post-reset operations.  The following
-            # find the actual the module path for the migration.  See
-            # StormBaseDatabase.load_schema().
-            migration = store.find(Version, component=version).one()
-            if migration is None:
-                continue
-            migrations[version] = module_path = migration.version
-            module = sys.modules[module_path]
-            pre_reset = getattr(module, 'pre_reset', None)
-            if pre_reset is not None:
-                pre_reset(store)
         # Make sure this is deterministic, by sorting on the storm table name.
         classes = sorted(ModelMeta._class_registry,
                          key=attrgetter('__storm_table__'))
         for model_class in classes:
-            store.find(model_class).remove()
-        # Now give each migration a chance to do post-reset operations.
-        for version in versions:
-            module = sys.modules[migrations[version]]
-            post_reset = getattr(module, 'post_reset', None)
-            if post_reset is not None:
-                post_reset(store)
-        config.db._post_reset(store)
+            if not model_class.PRESERVE:
+                store.find(model_class).remove()
 
 
 
diff --git a/src/mailman/database/schema/mm_00000000000000_base.py b/src/mailman/database/schema/mm_00000000000000_base.py
index f98b30a8d..0dcd28edd 100644
--- a/src/mailman/database/schema/mm_00000000000000_base.py
+++ b/src/mailman/database/schema/mm_00000000000000_base.py
@@ -21,8 +21,6 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
-    'post_reset',
-    'pre_reset',
     'upgrade',
     ]
 
@@ -35,14 +33,3 @@ _helper = None
 def upgrade(database, store, version, module_path):
     filename = '{0}.sql'.format(database.TAG)
     database.load_schema(store, version, filename, module_path)
-
-
-
-## def pre_reset(store):
-##     global _helper
-##     from mailman.testing.database import ResetHelper
-##     _helper = ResetHelper(VERSION, store)
-
-
-## def post_reset(store):
-##     _helper.restore(store)
diff --git a/src/mailman/database/schema/mm_20120407000000.py b/src/mailman/database/schema/mm_20120407000000.py
index 017141683..9ed99e225 100644
--- a/src/mailman/database/schema/mm_20120407000000.py
+++ b/src/mailman/database/schema/mm_20120407000000.py
@@ -36,8 +36,6 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
-    'post_reset',
-    'pre_reset',
     'upgrade',
     ]
 
@@ -127,14 +125,3 @@ def upgrade_postgres(database, store, version, module_path):
     store.execute('ALTER TABLE mailinglist DROP COLUMN archive_private;')
     # Record the migration in the version table.
     database.load_schema(store, version, None, module_path)
-
-
-
-## def pre_reset(store):
-##     global _helper
-##     from mailman.testing.database import ResetHelper
-##     _helper = ResetHelper(VERSION, store)
-
-
-## def post_reset(store):
-##     _helper.restore(store)
diff --git a/src/mailman/interfaces/database.py b/src/mailman/interfaces/database.py
index 316d5be49..a74ddd71f 100644
--- a/src/mailman/interfaces/database.py
+++ b/src/mailman/interfaces/database.py
@@ -23,6 +23,7 @@ __metaclass__ = type
 __all__ = [
     'DatabaseError',
     'IDatabase',
+    'IDatabaseFactory',
     ]
 
 
@@ -49,12 +50,6 @@ class IDatabase(Interface):
             configuration file setting.
         """
 
-    def _reset():
-        """Reset the database to its pristine state.
-
-        This is only used by the test framework.
-        """
-
     def _make_temporary():
         """Make a temporary database.
 
@@ -79,3 +74,17 @@ class IDatabase(Interface):
 
     store = Attribute(
         """The underlying Storm store on which you can do queries.""")
+
+
+
+class IDatabaseFactory(Interface):
+    "Interface for creating new databases."""
+
+    def create():
+        """Return a new `IDatabase`.
+
+        The database will be initialized and all migrations will be loaded.
+
+        :return: A new database.
+        :rtype: IDatabase
+        """
diff --git a/src/mailman/model/version.py b/src/mailman/model/version.py
index d6a4f3938..bae9322ea 100644
--- a/src/mailman/model/version.py
+++ b/src/mailman/model/version.py
@@ -34,6 +34,10 @@ class Version(Model):
     component = Unicode()
     version = Unicode()
 
+    # The testing machinery will generally reset all tables, however because
+    # this table tracks schema migrations, we do not want to reset it.
+    PRESERVE = True
+
     def __init__(self, component, version):
         super(Version, self).__init__()
         self.component = component
diff --git a/src/mailman/testing/database.py b/src/mailman/testing/database.py
deleted file mode 100644
index b442031fa..000000000
--- a/src/mailman/testing/database.py
+++ /dev/null
@@ -1,47 +0,0 @@
-# Copyright (C) 2012 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/>.
-
-"""Database test helpers."""
-
-from __future__ import absolute_import, print_function, unicode_literals
-
-__metaclass__ = type
-__all__ = [
-    'ResetHelper',
-    ]
-
-
-from mailman.model.version import Version
-
-
-
-class ResetHelper:
-    """Help with database resets; used by schema migrations."""
-
-    def __init__(self, version, store):
-        self.version = version
-        # Save the entry in the Version table for the test suite reset.  This
-        # will be restored below.
-        result = store.find(Version, component=version).one()
-        self.saved = result.version
-
-    def restore(self, store):
-        # We need to preserve the Version table entry for this migration,
-        # since its existence defines the fact that the tables have been
-        # loaded.
-        store.add(Version(component='schema', version=self.version))
-        store.add(Version(component=self.version, version=self.saved))
diff --git a/src/mailman/testing/layers.py b/src/mailman/testing/layers.py
index bbef6d5f4..3a3e1f684 100644
--- a/src/mailman/testing/layers.py
+++ b/src/mailman/testing/layers.py
@@ -127,7 +127,7 @@ class ConfigLayer(MockAndMonkeyLayer):
         config.create_paths = True
         config.push('test config', test_config)
         # Initialize everything else.
-        initialize.initialize_2()
+        initialize.initialize_2(testing=True)
         initialize.initialize_3()
         # When stderr debugging is enabled, subprocess root loggers should
         # also be more verbose.
diff --git a/src/mailman/testing/testing.cfg b/src/mailman/testing/testing.cfg
index 0be01298b..141d74a8f 100644
--- a/src/mailman/testing/testing.cfg
+++ b/src/mailman/testing/testing.cfg
@@ -18,9 +18,9 @@
 # A testing configuration.
 
 # For testing against PostgreSQL.
-[database]
-class: mailman.database.postgresql.PostgreSQLDatabase
-url: postgres://barry:barry@localhost/mailman
+# [database]
+# class: mailman.database.postgresql.PostgreSQLDatabase
+# url: postgres://barry:barry@localhost/mailman
 
 [mailman]
 site_owner: noreply@example.com