diff --git a/src/ahriman/core/database/migrations/__init__.py b/src/ahriman/core/database/migrations/__init__.py index 2ed44294..0adc24b8 100644 --- a/src/ahriman/core/database/migrations/__init__.py +++ b/src/ahriman/core/database/migrations/__init__.py @@ -62,24 +62,31 @@ class Migrations(LazyLogging): """ return Migrations(connection, configuration).run() - def migration(self, cursor: Cursor, migration: Migration) -> None: + def apply_migrations(self, migrations: list[Migration]) -> None: """ - perform single migration + perform migrations explicitly Args: - cursor(Cursor): connection cursor - migration(Migration): single migration to perform + migrations(list[Migration]): list of migrations to perform """ - self.logger.info("applying table migration %s at index %s", migration.name, migration.index) - for statement in migration.steps: - cursor.execute(statement) - self.logger.info("table migration %s at index %s has been applied", migration.name, migration.index) - - self.logger.info("perform data migration %s at index %s", migration.name, migration.index) - migration.migrate_data(self.connection, self.configuration) - self.logger.info( - "data migration %s at index %s has been performed", - migration.name, migration.index) + previous_isolation = self.connection.isolation_level + try: + self.connection.isolation_level = None + cursor = self.connection.cursor() + try: + cursor.execute("begin exclusive") + for migration in migrations: + self.perform_migration(cursor, migration) + except Exception: + self.logger.exception("migration failed with exception") + cursor.execute("rollback") + raise + else: + cursor.execute("commit") + finally: + cursor.close() + finally: + self.connection.isolation_level = previous_isolation def migrations(self) -> list[Migration]: """ @@ -114,6 +121,25 @@ class Migrations(LazyLogging): return migrations + def perform_migration(self, cursor: Cursor, migration: Migration) -> None: + """ + perform single migration + + Args: + cursor(Cursor): connection cursor + migration(Migration): single migration to perform + """ + self.logger.info("applying table migration %s at index %s", migration.name, migration.index) + for statement in migration.steps: + cursor.execute(statement) + self.logger.info("table migration %s at index %s has been applied", migration.name, migration.index) + + self.logger.info("perform data migration %s at index %s", migration.name, migration.index) + migration.migrate_data(self.connection, self.configuration) + self.logger.info( + "data migration %s at index %s has been performed", + migration.name, migration.index) + def run(self) -> MigrationResult: """ perform migrations @@ -122,6 +148,7 @@ class Migrations(LazyLogging): MigrationResult: current schema version """ migrations = self.migrations() + current_version = self.user_version() expected_version = len(migrations) result = MigrationResult(old_version=current_version, new_version=expected_version) @@ -130,25 +157,8 @@ class Migrations(LazyLogging): self.logger.info("no migrations required") return result - previous_isolation = self.connection.isolation_level - try: - self.connection.isolation_level = None - cursor = self.connection.cursor() - try: - cursor.execute("begin exclusive") - for migration in migrations[current_version:]: - self.migration(cursor, migration) - cursor.execute(f"pragma user_version = {expected_version}") # no support for ? placeholders - except Exception: - self.logger.exception("migration failed with exception") - cursor.execute("rollback") - raise - else: - cursor.execute("commit") - finally: - cursor.close() - finally: - self.connection.isolation_level = previous_isolation + self.apply_migrations(migrations[current_version:]) + self.connection.execute(f"pragma user_version = {expected_version}") # no support for ? placeholders self.logger.info("migrations have been performed from version %s to %s", result.old_version, result.new_version) return result diff --git a/tests/ahriman/core/database/migrations/test_migrations_init.py b/tests/ahriman/core/database/migrations/test_migrations_init.py index bbbefb93..8e1c19a5 100644 --- a/tests/ahriman/core/database/migrations/test_migrations_init.py +++ b/tests/ahriman/core/database/migrations/test_migrations_init.py @@ -19,17 +19,52 @@ def test_migrate(connection: Connection, configuration: Configuration, mocker: M run_mock.assert_called_once_with() -def test_migration(migrations: Migrations) -> None: +def test_apply_migrations(migrations: Migrations, mocker: MockerFixture) -> None: """ - must perform single migration + must apply list of migrations """ - migrate_data_mock = MagicMock() cursor = MagicMock() - migration = Migration(index=0, name="test", steps=["select 1"], migrate_data=migrate_data_mock) + migration = Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock()) + migrations.connection.cursor.return_value = cursor + migration_mock = mocker.patch("ahriman.core.database.migrations.Migrations.perform_migration") - migrations.migration(cursor, migration) - cursor.execute.assert_called_once_with("select 1") - migrate_data_mock.assert_called_once_with(migrations.connection, migrations.configuration) + migrations.apply_migrations([migration]) + cursor.execute.assert_has_calls([ + MockCall("begin exclusive"), + MockCall("commit"), + ]) + cursor.close.assert_called_once_with() + migration_mock.assert_called_once_with(cursor, migration) + + +def test_apply_migration_exception(migrations: Migrations, mocker: MockerFixture) -> None: + """ + must roll back and close cursor on exception during migration + """ + cursor = MagicMock() + mocker.patch("logging.Logger.info", side_effect=Exception()) + migrations.connection.cursor.return_value = cursor + + with pytest.raises(Exception): + migrations.apply_migrations([Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock())]) + cursor.execute.assert_has_calls([ + MockCall("begin exclusive"), + MockCall("rollback"), + ]) + cursor.close.assert_called_once_with() + + +def test_apply_migration_sql_exception(migrations: Migrations) -> None: + """ + must close cursor on general migration error + """ + cursor = MagicMock() + cursor.execute.side_effect = Exception() + migrations.connection.cursor.return_value = cursor + + with pytest.raises(Exception): + migrations.apply_migrations([Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock())]) + cursor.close.assert_called_once_with() def test_migrations(migrations: Migrations) -> None: @@ -39,6 +74,19 @@ def test_migrations(migrations: Migrations) -> None: assert migrations.migrations() +def test_perform_migration(migrations: Migrations) -> None: + """ + must perform single migration + """ + migrate_data_mock = MagicMock() + cursor = MagicMock() + migration = Migration(index=0, name="test", steps=["select 1"], migrate_data=migrate_data_mock) + + migrations.perform_migration(cursor, migration) + cursor.execute.assert_called_once_with("select 1") + migrate_data_mock.assert_called_once_with(migrations.connection, migrations.configuration) + + def test_run_skip(migrations: Migrations, mocker: MockerFixture) -> None: """ must skip migration if version is the same @@ -54,60 +102,15 @@ def test_run(migrations: Migrations, mocker: MockerFixture) -> None: must run migration """ migration = Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock()) - cursor = MagicMock() mocker.patch("ahriman.core.database.migrations.Migrations.user_version", return_value=0) mocker.patch("ahriman.core.database.migrations.Migrations.migrations", return_value=[migration]) - migrations.connection.cursor.return_value = cursor - migration_mock = mocker.patch("ahriman.core.database.migrations.Migrations.migration") validate_mock = mocker.patch("ahriman.models.migration_result.MigrationResult.validate") + apply_mock = mocker.patch("ahriman.core.database.migrations.Migrations.apply_migrations") migrations.run() + apply_mock.assert_called_once_with([migration]) validate_mock.assert_called_once_with() - cursor.execute.assert_has_calls([ - MockCall("begin exclusive"), - MockCall("pragma user_version = 1"), - MockCall("commit"), - ]) - cursor.close.assert_called_once_with() - migration_mock.assert_called_once_with(cursor, migration) - - -def test_run_migration_exception(migrations: Migrations, mocker: MockerFixture) -> None: - """ - must roll back and close cursor on exception during migration - """ - cursor = MagicMock() - mocker.patch("logging.Logger.info", side_effect=Exception()) - mocker.patch("ahriman.core.database.migrations.Migrations.user_version", return_value=0) - mocker.patch("ahriman.core.database.migrations.Migrations.migrations", - return_value=[Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock())]) - mocker.patch("ahriman.models.migration_result.MigrationResult.validate") - migrations.connection.cursor.return_value = cursor - - with pytest.raises(Exception): - migrations.run() - cursor.execute.assert_has_calls([ - MockCall("begin exclusive"), - MockCall("rollback"), - ]) - cursor.close.assert_called_once_with() - - -def test_run_sql_exception(migrations: Migrations, mocker: MockerFixture) -> None: - """ - must close cursor on general migration error - """ - cursor = MagicMock() - cursor.execute.side_effect = Exception() - mocker.patch("ahriman.core.database.migrations.Migrations.user_version", return_value=0) - mocker.patch("ahriman.core.database.migrations.Migrations.migrations", - return_value=[Migration(index=0, name="test", steps=["select 1"], migrate_data=MagicMock())]) - mocker.patch("ahriman.models.migration_result.MigrationResult.validate") - migrations.connection.cursor.return_value = cursor - - with pytest.raises(Exception): - migrations.run() - cursor.close.assert_called_once_with() + migrations.connection.execute.assert_called_once_with("pragma user_version = 1") def test_user_version(migrations: Migrations) -> None: