refactor: streamline migrations

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
+            migrations(list[Migration]): list of migrations to perform
-            migration(Migration): single migration to perform
         """
-        self.logger.info("applying table migration %s at index %s", migration.name, migration.index)
+        previous_isolation = self.connection.isolation_level
-        for statement in migration.steps:
+        try:
-            cursor.execute(statement)
+            self.connection.isolation_level = None
-        self.logger.info("table migration %s at index %s has been applied", migration.name, migration.index)
+            cursor = self.connection.cursor()
-
+            try:
-        self.logger.info("perform data migration %s at index %s", migration.name, migration.index)
+                cursor.execute("begin exclusive")
-        migration.migrate_data(self.connection, self.configuration)
+                for migration in migrations:
-        self.logger.info(
+                    self.perform_migration(cursor, migration)
-            "data migration %s at index %s has been performed",
+            except Exception:
-            migration.name, migration.index)
+                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
+        self.apply_migrations(migrations[current_version:])
-        try:
+        self.connection.execute(f"pragma user_version = {expected_version}")  # no support for ? placeholders
-            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.logger.info("migrations have been performed from version %s to %s", result.old_version, result.new_version)
         return result

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)
+    migrations.apply_migrations([migration])
-    cursor.execute.assert_called_once_with("select 1")
+    cursor.execute.assert_has_calls([
-    migrate_data_mock.assert_called_once_with(migrations.connection, migrations.configuration)
+        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([
+    migrations.connection.execute.assert_called_once_with("pragma user_version = 1")
-        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()
 
 
 def test_user_version(migrations: Migrations) -> None: