Separate out the migration test methods so that the ones testing that the

columns got renamed don't in fact check the data in those tables.  Better
separation of concerns.

Rename mailman.database.transaction.txn to ...transactional and use it in a
couple of additional spots.

6 files changed, 45 insertions, 34 deletions

diff --git a/src/mailman/database/tests/data/__init__.py b/src/mailman/database/tests/data/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/src/mailman/database/tests/data/__init__.py
diff --git a/src/mailman/database/tests/data/mailman_01.db b/src/mailman/database/tests/data/mailman_01.db
new file mode 100644
index 000000000..1ff8d8343
--- /dev/null
+++ b/src/mailman/database/tests/data/mailman_01.db
diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py
index bf3decc88..8b4dd1f43 100644
--- a/src/mailman/database/tests/test_migrations.py
+++ b/src/mailman/database/tests/test_migrations.py
@@ -31,12 +31,14 @@ import sqlite3
 import tempfile
 import unittest
 
+from pkg_resources import resource_filename
 from zope.component import getUtility
 
-from mailman.app.lifecycle import create_list
 from mailman.config import config
 from mailman.interfaces.domain import IDomainManager
-from mailman.testing.helpers import configuration, temporary_db
+from mailman.interfaces.listmanager import IListManager
+from mailman.interfaces.mailinglist import IAcceptableAliasSet
+from mailman.testing.helpers import configuration
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.modules import call_name
 
@@ -66,7 +68,8 @@ class TestMigration20120407(unittest.TestCase):
         shutil.rmtree(self._tempdir)
 
     def test_sqlite_base(self):
-        # Test the SQLite base schema.
+        # Test that before the migration, the old table columns are present
+        # and the new database columns are not.
         url = 'sqlite:///' + os.path.join(self._tempdir, 'mailman.db')
         database_class = config.database['class']
         database = call_name(database_class)
@@ -74,14 +77,6 @@ class TestMigration20120407(unittest.TestCase):
             database.initialize()
         # Load all the database SQL to just before ours.
         database.load_migrations('20120406999999')
-        # Populate the test database with a domain and a mailing list.
-        with temporary_db(database):
-            getUtility(IDomainManager).add(
-                'example.com', 'An example domain.',
-                'http://lists.example.com', 'postmaster@example.com')
-            mlist = create_list('test@example.com')
-            del mlist
-        database.commit()
         # Verify that the database has not yet been migrated.
         for missing in ('archive_policy',
                         'nntp_prefix_subject_too'):
@@ -100,7 +95,8 @@ class TestMigration20120407(unittest.TestCase):
                 'select {0} from mailinglist;'.format(present))
 
     def test_sqlite_migration(self):
-        # Test the SQLite base schema.
+        # Test that after the migration, the old table columns are missing
+        # and the new database columns are present.
         url = 'sqlite:///' + os.path.join(self._tempdir, 'mailman.db')
         database_class = config.database['class']
         database = call_name(database_class)
@@ -108,14 +104,6 @@ class TestMigration20120407(unittest.TestCase):
             database.initialize()
         # Load all the database SQL to just before ours.
         database.load_migrations('20120406999999')
-        # Populate the test database with a domain and a mailing list.
-        with temporary_db(database):
-            getUtility(IDomainManager).add(
-                'example.com', 'An example domain.',
-                'http://lists.example.com', 'postmaster@example.com')
-            mlist = create_list('test@example.com')
-            del mlist
-        database.commit()
         # Load all migrations, up to and including this one.
         database.load_migrations('20120407000000')
         # Verify that the database has been migrated.
@@ -134,3 +122,31 @@ class TestMigration20120407(unittest.TestCase):
             self.assertRaises(sqlite3.OperationalError,
                               database.store.execute,
                               'select {0} from mailinglist;'.format(missing))
+
+    def test_data_after_migration(self):
+        # Ensure that the existing data and foreign key references are
+        # preserved across a migration.  Unfortunately, this requires sample
+        # data, which kind of sucks.
+        dst = os.path.join(self._tempdir, 'mailman.db')
+        src = resource_filename('mailman.database.tests.data', 'mailman_01.db')
+        shutil.copyfile(src, dst)
+        url = 'sqlite:///' + dst
+        database_class = config.database['class']
+        database = call_name(database_class)
+        with configuration('database', url=url):
+            # Initialize the database and perform the migrations.
+            database.initialize()
+            database.load_migrations('20120407000000')
+            # Check that the domains survived the migration.  This table was
+            # not touched so it should be fine.
+            domains = list(getUtility(IDomainManager))
+            self.assertEqual(len(domains), 1)
+            self.assertEqual(domains[0].mail_host, 'example.com')
+            # There should be exactly one mailing list defined.
+            mlists = list(getUtility(IListManager).mailing_lists)
+            self.assertEqual(len(mlists), 1)
+            # Get the mailing list object and check its acceptable aliases.
+            # This tests that foreign keys continue to work.
+            aliases_set = IAcceptableAliasSet(mlists)
+            self.assertEqual(set(aliases_set.aliases),
+                             set(['foo@example.com', 'bar@example.com']))
diff --git a/src/mailman/database/transaction.py b/src/mailman/database/transaction.py
index 47e58d3e2..7a6ba00af 100644
--- a/src/mailman/database/transaction.py
+++ b/src/mailman/database/transaction.py
@@ -17,11 +17,11 @@
 
 """Transactional support."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
-    'txn',
+    'transactional',
     ]
 
 
@@ -29,7 +29,7 @@ from mailman.config import config
 
 
 
-class txn(object):
+class transactional:
     """Decorator for transactional support.
 
     When the function this decorator wraps exits cleanly, the current
diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py
index 36e8ae5ac..a735d2012 100644
--- a/src/mailman/rest/wsgiapp.py
+++ b/src/mailman/rest/wsgiapp.py
@@ -33,6 +33,7 @@ from wsgiref.simple_server import WSGIRequestHandler
 from wsgiref.simple_server import make_server as wsgi_server
 
 from mailman.config import config
+from mailman.database.transaction import transactional
 from mailman.rest.root import Root
 
 
@@ -51,17 +52,11 @@ class AdminWebServiceWSGIRequestHandler(WSGIRequestHandler):
 class AdminWebServiceApplication(RestishApp):
     """Connect the restish WSGI application to Mailman's database."""
 
+    @transactional
     def __call__(self, environ, start_response):
         """See `RestishApp`."""
-        try:
-            response = super(AdminWebServiceApplication, self).__call__(
-                environ, start_response)
-        except:
-            config.db.abort()
-            raise
-        else:
-            config.db.commit()
-            return response
+        return super(AdminWebServiceApplication, self).__call__(
+            environ, start_response)
 
 
 
diff --git a/src/mailman/runners/lmtp.py b/src/mailman/runners/lmtp.py
index 45fa5a783..249ab94c1 100644
--- a/src/mailman/runners/lmtp.py
+++ b/src/mailman/runners/lmtp.py
@@ -41,7 +41,7 @@ from zope.component import getUtility
 
 from mailman.config import config
 from mailman.core.runner import Runner
-from mailman.database.transaction import txn
+from mailman.database.transaction import transactional
 from mailman.email.message import Message
 from mailman.interfaces.listmanager import IListManager
 from mailman.utilities.datetime import now
@@ -154,7 +154,7 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
         Channel(self, conn, addr)
         slog.debug('LMTP accept from %s', addr)
 
-    @txn
+    @transactional
     def process_message(self, peer, mailfrom, rcpttos, data):
         try:
             # Refresh the list of list names every time we process a message