10 files changed, 32 insertions, 222 deletions

diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml
index f9b9cb093..24061f0f0 100644
--- a/src/mailman/config/configure.zcml
+++ b/src/mailman/config/configure.zcml
@@ -40,20 +40,6 @@
     factory="mailman.model.requests.ListRequests"
     />
 
-  <adapter
-    for="mailman.interfaces.database.IDatabase"
-    provides="mailman.interfaces.database.ITemporaryDatabase"
-    factory="mailman.database.sqlite.make_temporary"
-    name="sqlite"
-    />
-
-  <adapter
-    for="mailman.interfaces.database.IDatabase"
-    provides="mailman.interfaces.database.ITemporaryDatabase"
-    factory="mailman.database.postgresql.make_temporary"
-    name="postgres"
-    />
-
   <utility
     provides="mailman.interfaces.bounce.IBounceProcessor"
     factory="mailman.model.bounce.BounceProcessor"
@@ -72,12 +58,6 @@
     />
 
   <utility
-    provides="mailman.interfaces.database.IDatabaseFactory"
-    factory="mailman.database.factory.DatabaseTemporaryFactory"
-    name="temporary"
-    />
-
-  <utility
     provides="mailman.interfaces.domain.IDomainManager"
     factory="mailman.model.domain.DomainManager"
     />
diff --git a/src/mailman/database/base.py b/src/mailman/database/base.py
index c4b04b329..e360dcedf 100644
--- a/src/mailman/database/base.py
+++ b/src/mailman/database/base.py
@@ -45,10 +45,6 @@ class SABaseDatabase:
 
     Use this as a base class for your DB-Specific derived classes.
     """
-    # Tag used to distinguish the database being used.  Override this in base
-    # classes.
-    TAG = ''
-
     def __init__(self):
         self.url = None
         self.store = None
@@ -66,16 +62,6 @@ class SABaseDatabase:
         """See `IDatabase`."""
         self.store.rollback()
 
-    def _database_exists(self):
-        """Return True if the database exists and is initialized.
-
-        Return False when Mailman needs to create and initialize the
-        underlying database schema.
-
-        Base classes *must* override this.
-        """
-        raise NotImplementedError
-
     def _pre_reset(self, store):
         """Clean up method for testing.
 
@@ -103,10 +89,6 @@ class SABaseDatabase:
         """
         pass
 
-    # XXX Abhilash removed teh _prepare() method.  Is that because SA takes
-    # care of this for us?  If so, then the comment below must be updated.
-    # For reference, the SQLite bug is marked "won't fix".
-
     def initialize(self, debug=None):
         """See `IDatabase`."""
         # Calculate the engine url.
@@ -132,37 +114,3 @@ class SABaseDatabase:
         session = sessionmaker(bind=self.engine)
         self.store = session()
         self.store.commit()
-
-    # XXX We should probably rename load_migrations() and perhaps get rid of
-    # load_sql().  The latter is never called any more.
-
-    def load_migrations(self, until=None):
-        """Load schema migrations.
-
-        :param until: Load only the migrations up to the specified timestamp.
-            With default value of None, load all migrations.
-        :type until: string
-        """
-        from mailman.database.model import Model
-        Model.metadata.create_all(self.engine)
-
-    def load_sql(self, store, sql):
-        """Load the given SQL into the store.
-
-        :param store: The Storm store to load the schema into.
-        :type store: storm.locals.Store`
-        :param sql: The possibly multi-line SQL to load.
-        :type sql: string
-        """
-        # Discard all blank and comment lines.
-        lines = (line for line in sql.splitlines()
-                 if line.strip() != '' and line.strip()[:2] != '--')
-        sql = NL.join(lines)
-        for statement in sql.split(';'):
-            if statement.strip() != '':
-                store.execute(statement + ';')
-
-
-    @staticmethod
-    def _make_temporary():
-        raise NotImplementedError
diff --git a/src/mailman/database/factory.py b/src/mailman/database/factory.py
index 450672e5b..c06f75031 100644
--- a/src/mailman/database/factory.py
+++ b/src/mailman/database/factory.py
@@ -22,7 +22,6 @@ from __future__ import absolute_import, print_function, unicode_literals
 __metaclass__ = type
 __all__ = [
     'DatabaseFactory',
-    'DatabaseTemporaryFactory',
     'DatabaseTestingFactory',
     ]
 
@@ -31,13 +30,12 @@ import os
 import types
 
 from flufl.lock import Lock
-from zope.component import getAdapter
 from zope.interface import implementer
 from zope.interface.verify import verifyObject
 
 from mailman.config import config
-from mailman.interfaces.database import (
-    IDatabase, IDatabaseFactory, ITemporaryDatabase)
+from mailman.database.model import Model
+from mailman.interfaces.database import IDatabase, IDatabaseFactory
 from mailman.utilities.modules import call_name
 
 
@@ -54,7 +52,7 @@ class DatabaseFactory:
             database = call_name(database_class)
             verifyObject(IDatabase, database)
             database.initialize()
-            database.load_migrations()
+            Model.metadata.create_all(database.engine)
             database.commit()
             return database
 
@@ -82,24 +80,8 @@ class DatabaseTestingFactory:
         database = call_name(database_class)
         verifyObject(IDatabase, database)
         database.initialize()
-        database.load_migrations()
+        Model.metadata.create_all(database.engine)
         database.commit()
         # Make _reset() a bound method of the database instance.
         database._reset = types.MethodType(_reset, database)
         return database
-
-
-
-@implementer(IDatabaseFactory)
-class DatabaseTemporaryFactory:
-    """Create a temporary database for some of the migration tests."""
-
-    @staticmethod
-    def create():
-        """See `IDatabaseFactory`."""
-        database_class_name = config.database['class']
-        database = call_name(database_class_name)
-        verifyObject(IDatabase, database)
-        adapted_database = getAdapter(
-            database, ITemporaryDatabase, database.TAG)
-        return adapted_database
diff --git a/src/mailman/database/model.py b/src/mailman/database/model.py
index f8a15162c..06705dfe9 100644
--- a/src/mailman/database/model.py
+++ b/src/mailman/database/model.py
@@ -44,6 +44,8 @@ class ModelMeta:
         with contextlib.closing(config.db.engine.connect()) as connection:
             transaction = connection.begin()
             try:
+                # Delete all the tables in reverse foreign key dependency
+                # order.  http://tinyurl.com/on8dy6f
                 for table in reversed(Model.metadata.sorted_tables):
                     connection.execute(table.delete())
             except:
diff --git a/src/mailman/database/postgresql.py b/src/mailman/database/postgresql.py
index 1ee454074..59fff0865 100644
--- a/src/mailman/database/postgresql.py
+++ b/src/mailman/database/postgresql.py
@@ -22,34 +22,17 @@ from __future__ import absolute_import, print_function, unicode_literals
 __metaclass__ = type
 __all__ = [
     'PostgreSQLDatabase',
-    'make_temporary',
     ]
 
 
-import types
-
-from functools import partial
-from operator import attrgetter
-from urlparse import urlsplit, urlunsplit
-
 from mailman.database.base import SABaseDatabase
-from mailman.testing.helpers import configuration
+from operator import attrgetter
 
 
 
 class PostgreSQLDatabase(SABaseDatabase):
     """Database class for PostgreSQL."""
 
-    TAG = 'postgres'
-
-    def _database_exists(self, store):
-        """See `BaseDatabase`."""
-        table_query = ('SELECT table_name FROM information_schema.tables '
-                       "WHERE table_schema = 'public'")
-        results = store.execute(table_query)
-        table_names = set(item[0] for item in results)
-        return 'version' in table_names
-
     def _post_reset(self, store):
         """PostgreSQL-specific test suite cleanup.
 
@@ -69,37 +52,3 @@ class PostgreSQLDatabase(SABaseDatabase):
                               max("id") IS NOT null)
                        FROM "{0}";
                 """.format(model_class.__storm_table__))
-
-
-
-# Test suite adapter for ITemporaryDatabase.
-
-def _cleanup(self, store, tempdb_name):
-    from mailman.config import config
-    store.rollback()
-    store.close()
-    # From the original database connection, drop the now unused database.
-    config.db.store.execute('DROP DATABASE {0}'.format(tempdb_name))
-
-
-def make_temporary(database):
-    """Adapts by monkey patching an existing PostgreSQL IDatabase."""
-    from mailman.config import config
-    parts = urlsplit(config.database.url)
-    assert parts.scheme == 'postgres'
-    new_parts = list(parts)
-    new_parts[2] = '/mmtest'
-    url = urlunsplit(new_parts)
-    # Use the existing database connection to create a new testing
-    # database.
-    config.db.store.execute('ABORT;')
-    config.db.store.execute('CREATE DATABASE mmtest;')
-    with configuration('database', url=url):
-        database.initialize()
-    database._cleanup = types.MethodType(
-        partial(_cleanup, store=database.store, tempdb_name='mmtest'),
-        database)
-    # bool column values in PostgreSQL.
-    database.FALSE = 'False'
-    database.TRUE = 'True'
-    return database
diff --git a/src/mailman/database/sqlite.py b/src/mailman/database/sqlite.py
index b70e474cc..db7860390 100644
--- a/src/mailman/database/sqlite.py
+++ b/src/mailman/database/sqlite.py
@@ -22,63 +22,27 @@ from __future__ import absolute_import, print_function, unicode_literals
 __metaclass__ = type
 __all__ = [
     'SQLiteDatabase',
-    'make_temporary',
     ]
 
 
 import os
-import types
-import shutil
-import tempfile
-
-from functools import partial
-from urlparse import urlparse
 
 from mailman.database.base import SABaseDatabase
-from mailman.testing.helpers import configuration
+from urlparse import urlparse
 
 
 
 class SQLiteDatabase(SABaseDatabase):
     """Database class for SQLite."""
 
-    TAG = 'sqlite'
-
-    def _database_exists(self, store):
-        """See `BaseDatabase`."""
-        table_query = 'select tbl_name from sqlite_master;'
-        table_names = set(item[0] for item in
-                          store.execute(table_query))
-        return 'version' in table_names
-
     def _prepare(self, url):
         parts = urlparse(url)
         assert parts.scheme == 'sqlite', (
             'Database url mismatch (expected sqlite prefix): {0}'.format(url))
+        # Ensure that the SQLite database file has the proper permissions,
+        # since SQLite doesn't play nice with umask.
         path = os.path.normpath(parts.path)
         fd = os.open(path, os.O_WRONLY |  os.O_NONBLOCK | os.O_CREAT, 0o666)
         # Ignore errors
         if fd > 0:
             os.close(fd)
-
-
-
-# Test suite adapter for ITemporaryDatabase.
-
-def _cleanup(self, tempdir):
-    shutil.rmtree(tempdir)
-
-
-def make_temporary(database):
-    """Adapts by monkey patching an existing SQLite IDatabase."""
-    tempdir = tempfile.mkdtemp()
-    url = 'sqlite:///' + os.path.join(tempdir, 'mailman.db')
-    with configuration('database', url=url):
-        database.initialize()
-    database._cleanup = types.MethodType(
-        partial(_cleanup, tempdir=tempdir),
-        database)
-    # bool column values in SQLite must be integers.
-    database.FALSE = 0
-    database.TRUE = 1
-    return database
diff --git a/src/mailman/database/types.py b/src/mailman/database/types.py
index 380ce37dc..641e065ba 100644
--- a/src/mailman/database/types.py
+++ b/src/mailman/database/types.py
@@ -29,8 +29,7 @@ __all__ = [
 import uuid
 
 from sqlalchemy import Integer
-from sqlalchemy.dialects import postgresql
-from sqlalchemy.types import TypeDecorator, BINARY, CHAR
+from sqlalchemy.types import TypeDecorator, CHAR
 
 
 
@@ -59,45 +58,34 @@ class Enum(TypeDecorator):
 
 
 class UUID(TypeDecorator):
-    """Handle UUIds."""
+    """Platform-independent GUID type.
 
-    impl = BINARY(16)
-    python_type = uuid.UUID
+    Uses Postgresql's UUID type, otherwise uses
+    CHAR(32), storing as stringified hex values.
 
-    def __init__(self, binary=True, native=True):
-        self.binary = binary
-        self.native = native
+    """
+    impl = CHAR
 
     def load_dialect_impl(self, dialect):
-        if dialect.name == 'postgresql' and self.native:
-            # Use the native UUID type.
-            return dialect.type_descriptor(postgresql.UUID())
+        if dialect.name == 'postgresql':
+            return dialect.type_descriptor(UUID())
         else:
-            # Fallback to either a BINARY or a CHAR.
-            kind = self.impl if self.binary else CHAR(32)
-            return dialect.type_descriptor(kind)
-
-    @staticmethod
-    def _coerce(value):
-        if value and not isinstance(value, uuid.UUID):
-            try:
-                value = uuid.UUID(value)
-            except (TypeError, ValueError):
-                value = uuid.UUID(bytes=value)
-        return value
+            return dialect.type_descriptor(CHAR(32))
 
     def process_bind_param(self, value, dialect):
         if value is None:
             return value
-        if not isinstance(value, uuid.UUID):
-            value = self._coerce(value)
-        if self.native and dialect.name == 'postgresql':
+        elif dialect.name == 'postgresql':
             return str(value)
-        return value.bytes if self.binary else value.hex
+        else:
+            if not isinstance(value, uuid.UUID):
+                return "%.32x" % uuid.UUID(value)
+            else:
+                # hexstring
+                return "%.32x" % value
 
     def process_result_value(self, value, dialect):
         if value is None:
             return value
-        if self.native and dialect.name == 'postgresql':
+        else:
             return uuid.UUID(value)
-        return uuid.UUID(bytes=value) if self.binary else uuid.UUID(value)
diff --git a/src/mailman/interfaces/database.py b/src/mailman/interfaces/database.py
index c2997ba6b..bd436ed13 100644
--- a/src/mailman/interfaces/database.py
+++ b/src/mailman/interfaces/database.py
@@ -24,7 +24,6 @@ __all__ = [
     'DatabaseError',
     'IDatabase',
     'IDatabaseFactory',
-    'ITemporaryDatabase',
     ]
 
 
@@ -65,11 +64,6 @@ class IDatabase(Interface):
 
 
 
-class ITemporaryDatabase(Interface):
-    """Marker interface for test suite adaptation."""
-
-
-
 class IDatabaseFactory(Interface):
     "Interface for creating new databases."""
 
diff --git a/src/mailman/model/address.py b/src/mailman/model/address.py
index d078f28d5..20bd631f5 100644
--- a/src/mailman/model/address.py
+++ b/src/mailman/model/address.py
@@ -57,7 +57,7 @@ class Address(Model):
 
     preferences_id = Column(Integer, ForeignKey('preferences.id'))
     preferences = relationship(
-        'Preferences', backref=backref('Address', uselist=False))
+        'Preferences', backref=backref('address', uselist=False))
 
     def __init__(self, email, display_name):
         super(Address, self).__init__()
diff --git a/src/mailman/model/requests.py b/src/mailman/model/requests.py
index 335e1e002..7f996dded 100644
--- a/src/mailman/model/requests.py
+++ b/src/mailman/model/requests.py
@@ -105,6 +105,9 @@ class ListRequests:
             data_hash = token
         request = _Request(key, request_type, self.mailing_list, data_hash)
         store.add(request)
+        # XXX The caller needs a valid id immediately, so flush the changes
+        # now to the SA transaction context.  Otherwise .id would not be
+        # valid.  Hopefully this has no unintended side-effects.
         store.flush()
         return request.id
 
@@ -148,7 +151,7 @@ class _Request(Model):
 
     __tablename__ = 'request'
 
-    id = Column(Integer, primary_key=True)# TODO: ???, default=AutoReload)
+    id = Column(Integer, primary_key=True)
     key = Column(Unicode)
     request_type = Column(Enum(RequestType))
     data_hash = Column(LargeBinary)