diff options
21 files changed, 233 insertions, 167 deletions
diff --git a/src/mailman/commands/cli_migrate.py b/src/mailman/commands/cli_migrate.py deleted file mode 100644 index d51aaa209..000000000 --- a/src/mailman/commands/cli_migrate.py +++ /dev/null @@ -1,60 +0,0 @@ -# Copyright (C) 2010-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/>. - -"""bin/mailman migrate.""" - -from __future__ import absolute_import, print_function, unicode_literals - -__metaclass__ = type -__all__ = [ - 'Migrate', - ] - - -from alembic import command -from zope.interface import implementer - -from mailman.core.i18n import _ -from mailman.database.alembic import alembic_cfg -from mailman.interfaces.command import ICLISubCommand - - - -@implementer(ICLISubCommand) -class Migrate: - """Migrate the Mailman database to the latest schema.""" - - name = 'migrate' - - def add(self, parser, command_parser): - """See `ICLISubCommand`.""" - command_parser.add_argument( - '-a', '--autogenerate', - action='store_true', help=_("""\ - Autogenerate the migration script using Alembic.""")) - command_parser.add_argument( - '-q', '--quiet', - action='store_true', default=False, - help=('Produce less output.')) - - def process(self, args): - if args.autogenerate: - command.revision(alembic_cfg, autogenerate=True) - else: - command.upgrade(alembic_cfg, 'head') - if not args.quiet: - print('Updated the database schema.') diff --git a/src/mailman/commands/docs/conf.rst b/src/mailman/commands/docs/conf.rst index 2f708edc5..3203271f9 100644 --- a/src/mailman/commands/docs/conf.rst +++ b/src/mailman/commands/docs/conf.rst @@ -22,7 +22,7 @@ To get a list of all key-value pairs of any section, you need to call the command without any options. >>> command.process(FakeArgs) - [logging.archiver] path: mailman.log + [alembic] script_location: mailman.database:alembic ... [passwords] password_length: 8 ... diff --git a/src/mailman/config/alembic.cfg b/src/mailman/config/alembic.cfg deleted file mode 100644 index b1f53a3d2..000000000 --- a/src/mailman/config/alembic.cfg +++ /dev/null @@ -1,57 +0,0 @@ -# A generic, single database configuration. - -[alembic] -# path to migration scripts -script_location = mailman.database:alembic - -# template used to generate migration files -# file_template = %%(rev)s_%%(slug)s - -# max length of characters to apply to the -# "slug" field -#truncate_slug_length = 40 - -# set to 'true' to run the environment during -# the 'revision' command, regardless of autogenerate -# revision_environment = false - -# set to 'true' to allow .pyc and .pyo files without -# a source .py file to be detected as revisions in the -# versions/ directory -# sourceless = false - - -# Logging configuration -[loggers] -keys = root,sqlalchemy,alembic - -[handlers] -keys = console - -[formatters] -keys = generic - -[logger_root] -level = WARN -handlers = console -qualname = - -[logger_sqlalchemy] -level = WARN -handlers = console -qualname = sqlalchemy.engine - -[logger_alembic] -level = WARN -handlers = console -qualname = alembic - -[handler_console] -class = StreamHandler -args = (sys.stderr,) -level = WARN -formatter = generic - -[formatter_generic] -format = %(levelname)-5.5s [%(name)s] %(message)s -datefmt = %H:%M:%S diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index ac09c1b07..d28d2cf88 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -204,11 +204,6 @@ class: mailman.database.sqlite.SQLiteDatabase url: sqlite:///$DATA_DIR/mailman.db debug: no -# Where can we find the Alembic migration scripts? The `python:` schema means -# that this is a module path relative to the Mailman 3 source installation. -alembic_scripts: mailman.database:alembic - - [logging.template] # This defines various log settings. The options available are: # @@ -309,6 +304,9 @@ failure: $msgid delivery to $recip failed with code $smtpcode, $smtpmsg [logging.vette] +[logging.database] +level: warn + [webservice] # The hostname at which admin web service resources are exposed. @@ -645,3 +643,8 @@ rewrite_duplicate_headers: CC X-Original-CC Content-Transfer-Encoding X-Original-Content-Transfer-Encoding MIME-Version X-MIME-Version + + +[alembic] +# Path to Alembic migration scripts. +script_location: mailman.database:alembic diff --git a/src/mailman/database/alembic/__init__.py b/src/mailman/database/alembic/__init__.py index ffd3af6df..9ac7f1311 100644 --- a/src/mailman/database/alembic/__init__.py +++ b/src/mailman/database/alembic/__init__.py @@ -29,4 +29,4 @@ from alembic.config import Config from mailman.utilities.modules import expand_path -alembic_cfg = Config(expand_path('python:mailman.config.alembic')) +alembic_cfg = Config(expand_path("python:mailman.config.schema")) diff --git a/src/mailman/database/alembic/versions/429e08420177_initial.py b/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py index 14f22079b..3feb24fff 100644 --- a/src/mailman/database/alembic/versions/429e08420177_initial.py +++ b/src/mailman/database/alembic/versions/51b7f92bd06c_initial.py @@ -22,29 +22,45 @@ 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 the `version` table left over from Storm (if it exists). +It also removes schema items left over from Storm. -Revision ID: 429e08420177 +Revision ID: 51b7f92bd06c Revises: None -Create Date: 2014-10-02 10:18:17.333354 +Create Date: 2014-10-10 09:53:35.624472 """ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ + 'downgrade', + 'upgrade', ] -# Revision identifiers, used by Alembic. -revision = '429e08420177' -down_revision = None from alembic import op +import sqlalchemy as sa + + +# Revision identifiers, used by Alembic. +revision = '51b7f92bd06c' +down_revision = None def upgrade(): 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') def downgrade(): - pass + 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)) diff --git a/src/mailman/database/base.py b/src/mailman/database/base.py index 2f69ed6ad..e360dcedf 100644 --- a/src/mailman/database/base.py +++ b/src/mailman/database/base.py @@ -25,13 +25,11 @@ __all__ = [ import logging -from alembic import command from sqlalchemy import create_engine 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,14 +89,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 9cbe1088f..64174449d 100644 --- a/src/mailman/database/factory.py +++ b/src/mailman/database/factory.py @@ -28,8 +28,8 @@ __all__ = [ import os import types +import alembic.command -from alembic import command from alembic.migration import MigrationContext from alembic.script import ScriptDirectory from flufl.lock import Lock @@ -84,6 +84,8 @@ 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 setup_database(self): @@ -99,7 +101,8 @@ class SchemaManager: if storm_version is None: # Initial database creation. Model.metadata.create_all(self._database.engine) - command.stamp(alembic_cfg, 'head') + self._database.commit() + alembic.command.stamp(alembic_cfg, 'head') else: # The database was previously managed by Storm. if storm_version.version < LAST_STORM_SCHEMA_VERSION: @@ -107,9 +110,9 @@ class SchemaManager: 'Upgrades skipping beta versions is not supported.') # Run migrations to remove the Storm-specific table and upgrade # to SQLAlchemy and Alembic. - command.upgrade(alembic_cfg, 'head') + alembic.command.upgrade(alembic_cfg, 'head') elif current_rev != head_rev: - command.upgrade(alembic_cfg, 'head') + alembic.command.upgrade(alembic_cfg, 'head') 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..d7c4d8503 --- /dev/null +++ b/src/mailman/database/tests/test_factory.py @@ -0,0 +1,160 @@ +# 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__ = [ + 'TestSchemaManager', + ] + + +import unittest +import alembic.command + +from mock import patch +from sqlalchemy import MetaData, Table, Column, Integer, Unicode +from sqlalchemy.exc import ProgrammingError, OperationalError +from sqlalchemy.schema import Index + +from mailman.config import config +from mailman.database.alembic import alembic_cfg +from mailman.database.factory import LAST_STORM_SCHEMA_VERSION, SchemaManager +from mailman.database.model import Model +from mailman.interfaces.database import DatabaseError +from mailman.testing.layers import ConfigLayer + + + +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) + for tablename in ('alembic_version', 'version'): + if tablename in md.tables: + md.tables[tablename].drop(config.db.engine) + self.schema_mgr = SchemaManager(config.db) + + def tearDown(self): + self._drop_storm_database() + # Restore a virgin database. + 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_database(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() + # Other Storm specific changes, those SQL statements hopefully work on + # all DB engines... + config.db.engine.execute( + 'ALTER TABLE mailinglist ADD COLUMN acceptable_aliases_id INT') + Index('ix_user__user_id').drop(bind=config.db.engine) + # Don't pollute our main metadata object, create a new one. + md = MetaData() + user_table = Model.metadata.tables['user'].tometadata(md) + Index('ix_user_user_id', user_table.c._user_id).create( + bind=config.db.engine) + config.db.commit() + + def _drop_storm_database(self): + """Remove the leftovers from a Storm DB. + + A drop_all() must be issued afterwards. + """ + if 'version' in Model.metadata.tables: + version = Model.metadata.tables['version'] + version.drop(config.db.engine, checkfirst=True) + Model.metadata.remove(version) + try: + Index('ix_user_user_id').drop(bind=config.db.engine) + except (ProgrammingError, OperationalError): + # Nonexistent. PostgreSQL raises a ProgrammingError, while SQLite + # raises an OperationalError. + pass + config.db.commit() + + def test_current_database(self): + # The database is already at the latest version. + alembic.command.stamp(alembic_cfg, 'head') + with patch('alembic.command') as alembic_command: + self.schema_mgr.setup_database() + self.assertFalse(alembic_command.stamp.called) + self.assertFalse(alembic_command.upgrade.called) + + @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.setup_database() + self.assertFalse(alembic_command.upgrade.called) + self.assertTrue(self._table_exists('mailinglist')) + self.assertTrue(self._table_exists('alembic_version')) + + @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(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')) + + @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.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. + 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() + 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/model/address.py b/src/mailman/model/address.py index 20bd631f5..cc4ab6fd0 100644 --- a/src/mailman/model/address.py +++ b/src/mailman/model/address.py @@ -53,9 +53,9 @@ class Address(Model): _verified_on = Column('verified_on', DateTime) registered_on = Column(DateTime) - user_id = Column(Integer, ForeignKey('user.id')) + user_id = Column(Integer, ForeignKey('user.id'), index=True) - preferences_id = Column(Integer, ForeignKey('preferences.id')) + preferences_id = Column(Integer, ForeignKey('preferences.id'), index=True) preferences = relationship( 'Preferences', backref=backref('address', uselist=False)) diff --git a/src/mailman/model/autorespond.py b/src/mailman/model/autorespond.py index c74434f7b..2293f5dcd 100644 --- a/src/mailman/model/autorespond.py +++ b/src/mailman/model/autorespond.py @@ -44,14 +44,14 @@ from mailman.utilities.datetime import today class AutoResponseRecord(Model): """See `IAutoResponseRecord`.""" - __tablename__ = 'autorespondrecord' + __tablename__ = 'autoresponserecord' id = Column(Integer, primary_key=True) - address_id = Column(Integer, ForeignKey('address.id')) + address_id = Column(Integer, ForeignKey('address.id'), index=True) address = relationship('Address') - mailing_list_id = Column(Integer, ForeignKey('mailinglist.id')) + mailing_list_id = Column(Integer, ForeignKey('mailinglist.id'), index=True) mailing_list = relationship('MailingList') response_type = Column(Enum(Response)) diff --git a/src/mailman/model/language.py b/src/mailman/model/language.py index 15450c936..f4d48fc97 100644 --- a/src/mailman/model/language.py +++ b/src/mailman/model/language.py @@ -28,8 +28,8 @@ __all__ = [ from sqlalchemy import Column, Integer, Unicode from zope.interface import implementer -from mailman.database import Model -from mailman.interfaces import ILanguage +from mailman.database.model import Model +from mailman.interfaces.languages import ILanguage diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index d00cf3d31..4792c3f48 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -504,9 +504,11 @@ class AcceptableAlias(Model): id = Column(Integer, primary_key=True) - mailing_list_id = Column(Integer, ForeignKey('mailinglist.id')) + mailing_list_id = Column( + Integer, ForeignKey('mailinglist.id'), + index=True, nullable=False) mailing_list = relationship('MailingList', backref='acceptable_alias') - alias = Column(Unicode) + alias = Column(Unicode, index=True, nullable=False) def __init__(self, mailing_list, alias): self.mailing_list = mailing_list @@ -558,9 +560,12 @@ class ListArchiver(Model): id = Column(Integer, primary_key=True) - mailing_list_id = Column(Integer, ForeignKey('mailinglist.id')) + mailing_list_id = Column( + Integer, ForeignKey('mailinglist.id'), + index=True, nullable=False) mailing_list = relationship('MailingList') - name = Column(Unicode) + + name = Column(Unicode, nullable=False) _is_enabled = Column(Boolean) def __init__(self, mailing_list, archiver_name, system_archiver): diff --git a/src/mailman/model/mime.py b/src/mailman/model/mime.py index 906af91ea..dc6a54437 100644 --- a/src/mailman/model/mime.py +++ b/src/mailman/model/mime.py @@ -43,7 +43,7 @@ class ContentFilter(Model): id = Column(Integer, primary_key=True) - mailing_list_id = Column(Integer, ForeignKey('mailinglist.id')) + mailing_list_id = Column(Integer, ForeignKey('mailinglist.id'), index=True) mailing_list = relationship('MailingList') filter_type = Column(Enum(FilterType)) diff --git a/src/mailman/model/pending.py b/src/mailman/model/pending.py index 691e94fd9..49b12c16a 100644 --- a/src/mailman/model/pending.py +++ b/src/mailman/model/pending.py @@ -56,7 +56,7 @@ class PendedKeyValue(Model): id = Column(Integer, primary_key=True) key = Column(Unicode) value = Column(Unicode) - pended_id = Column(Integer, ForeignKey('pended.id')) + pended_id = Column(Integer, ForeignKey('pended.id'), index=True) def __init__(self, key, value): self.key = key diff --git a/src/mailman/model/requests.py b/src/mailman/model/requests.py index 7f996dded..6b130196d 100644 --- a/src/mailman/model/requests.py +++ b/src/mailman/model/requests.py @@ -149,14 +149,14 @@ class ListRequests: class _Request(Model): """Table for mailing list hold requests.""" - __tablename__ = 'request' + __tablename__ = '_request' id = Column(Integer, primary_key=True) key = Column(Unicode) request_type = Column(Enum(RequestType)) data_hash = Column(LargeBinary) - mailing_list_id = Column(Integer, ForeignKey('mailinglist.id')) + mailing_list_id = Column(Integer, ForeignKey('mailinglist.id'), index=True) mailing_list = relationship('MailingList') def __init__(self, key, request_type, mailing_list, data_hash): diff --git a/src/mailman/model/uid.py b/src/mailman/model/uid.py index 29d8e7021..72ddd7b5a 100644 --- a/src/mailman/model/uid.py +++ b/src/mailman/model/uid.py @@ -50,7 +50,7 @@ class UID(Model): __tablename__ = 'uid' id = Column(Integer, primary_key=True) - uid = Column(UUID) + uid = Column(UUID, index=True) @dbconnection def __init__(self, store, uid): diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py index 576015dbe..ab581fdc8 100644 --- a/src/mailman/model/user.py +++ b/src/mailman/model/user.py @@ -56,8 +56,8 @@ class User(Model): id = Column(Integer, primary_key=True) display_name = Column(Unicode) - _password = Column('password', LargeBinary) # TODO : was RawStr() - _user_id = Column(UUID) + _password = Column('password', LargeBinary) + _user_id = Column(UUID, index=True) _created_on = Column(DateTime) addresses = relationship( @@ -66,12 +66,15 @@ class User(Model): _preferred_address_id = Column( Integer, - ForeignKey('address.id', use_alter=True, name='_preferred_address')) + ForeignKey('address.id', use_alter=True, + name='_preferred_address', + ondelete='SET NULL')) + _preferred_address = relationship( 'Address', primaryjoin=(_preferred_address_id==Address.id), post_update=True) - preferences_id = Column(Integer, ForeignKey('preferences.id')) + preferences_id = Column(Integer, ForeignKey('preferences.id'), index=True) preferences = relationship( 'Preferences', backref=backref('user', uselist=False)) diff --git a/src/mailman/testing/layers.py b/src/mailman/testing/layers.py index 4a9841fc2..38aa3907f 100644 --- a/src/mailman/testing/layers.py +++ b/src/mailman/testing/layers.py @@ -48,6 +48,7 @@ import tempfile from lazr.config import as_boolean from pkg_resources import resource_string from textwrap import dedent +from zope import event from zope.component import getUtility from mailman.config import config @@ -98,7 +99,9 @@ class ConfigLayer(MockAndMonkeyLayer): # Set up the basic configuration stuff. Turn off path creation until # we've pushed the testing config. config.create_paths = False - initialize.initialize_1(INHIBIT_CONFIG_FILE) + if not event.subscribers: + # Only if not yet initialized by another layer. + initialize.initialize_1(INHIBIT_CONFIG_FILE) assert cls.var_dir is None, 'Layer already set up' # Calculate a temporary VAR_DIR directory so that run-time artifacts # of the tests won't tread on the installation's data. This also diff --git a/src/mailman/testing/testing.cfg b/src/mailman/testing/testing.cfg index eba10dd3b..43ae6e67e 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: postgres://$USER:$USER@localhost/mailman_test +# url: postgresql://$USER:$USER@localhost/mailman_test [mailman] site_owner: noreply@example.com |