apply data migration in the same transaction block with schema migration

src/ahriman/core/database/data/__init__.py

@@ -24,11 +24,10 @@ from ahriman.core.database.data.package_statuses import migrate_package_statuses
 from ahriman.core.database.data.patches import migrate_patches
 from ahriman.core.database.data.users import migrate_users_data
 from ahriman.models.migration_result import MigrationResult
-from ahriman.models.repository_paths import RepositoryPaths
 
 
-def migrate_data(result: MigrationResult, connection: Connection,
+def migrate_data(
-                 configuration: Configuration, paths: RepositoryPaths) -> None:
+        result: MigrationResult, connection: Connection, configuration: Configuration) -> None:
     """
     perform data migration
 
@@ -36,10 +35,11 @@ def migrate_data(result: MigrationResult, connection: Connection,
         result(MigrationResult): result of the schema migration
         connection(Connection): database connection
         configuration(Configuration): configuration instance
-        paths(RepositoryPaths): repository paths instance
     """
     # initial data migration
+    repository_paths = configuration.repository_paths
+
     if result.old_version <= 0:
-        migrate_package_statuses(connection, paths)
+        migrate_package_statuses(connection, repository_paths)
-        migrate_patches(connection, paths)
+        migrate_patches(connection, repository_paths)
         migrate_users_data(connection, configuration)

src/ahriman/core/database/data/package_statuses.py

@@ -77,6 +77,3 @@ def migrate_package_statuses(connection: Connection, paths: RepositoryPaths) ->
         status = BuildStatus.from_json(item["status"])
         insert_base(package, status)
         insert_packages(package)
-
-    connection.commit()
-    cache_path.unlink()

src/ahriman/core/database/data/patches.py

@@ -42,5 +42,3 @@ def migrate_patches(connection: Connection, paths: RepositoryPaths) -> None:
         connection.execute(
             """insert into patches (package_base, patch) values (:package_base, :patch)""",
             {"package_base": package.name, "patch": content})
-
-    connection.commit()

src/ahriman/core/database/data/users.py

@@ -38,5 +38,3 @@ def migrate_users_data(connection: Connection, configuration: Configuration) ->
             connection.execute(
                 """insert into users (username, access, password) values (:username, :access, :password)""",
                 {"username": option.lower(), "access": access, "password": value})
-
-    connection.commit()

src/ahriman/core/database/migrations/__init__.py

@@ -27,6 +27,8 @@ from pkgutil import iter_modules
 from sqlite3 import Connection
 from typing import List, Type
 
+from ahriman.core.configuration import Configuration
+from ahriman.core.database.data import migrate_data
 from ahriman.models.migration import Migration
 from ahriman.models.migration_result import MigrationResult
 
@@ -37,32 +39,36 @@ class Migrations:
     idea comes from https://www.ash.dev/blog/simple-migration-system-in-sqlite/
 
     Attributes:
+        configuration(Configuration): configuration instance
         connection(Connection): database connection
         logger(logging.Logger): class logger
     """
 
-    def __init__(self, connection: Connection) -> None:
+    def __init__(self, connection: Connection, configuration: Configuration) -> None:
         """
         default constructor
 
         Args:
             connection(Connection): database connection
+            configuration(Configuration): configuration instance
         """
         self.connection = connection
+        self.configuration = configuration
         self.logger = logging.getLogger("database")
 
     @classmethod
-    def migrate(cls: Type[Migrations], connection: Connection) -> MigrationResult:
+    def migrate(cls: Type[Migrations], connection: Connection, configuration: Configuration) -> MigrationResult:
         """
         perform migrations implicitly
 
         Args:
             connection(Connection): database connection
+            configuration(Configuration): configuration instance
 
         Returns:
             MigrationResult: current schema version
         """
-        return cls(connection).run()
+        return cls(connection, configuration).run()
 
     def migrations(self) -> List[Migration]:
         """
@@ -112,6 +118,8 @@ class Migrations:
                         cursor.execute(statement)
                     self.logger.info("migration %s at index %s has been applied", migration.name, migration.index)
 
+                migrate_data(result, self.connection, self.configuration)
+
                 cursor.execute(f"pragma user_version = {expected_version}")  # no support for ? placeholders
             except Exception:
                 self.logger.exception("migration failed with exception")

src/ahriman/core/database/sqlite.py

@@ -23,11 +23,9 @@ import json
 import sqlite3
 
 from pathlib import Path
-from sqlite3 import Connection
 from typing import Type
 
 from ahriman.core.configuration import Configuration
-from ahriman.core.database.data import migrate_data
 from ahriman.core.database.migrations import Migrations
 from ahriman.core.database.operations.auth_operations import AuthOperations
 from ahriman.core.database.operations.build_operations import BuildOperations
@@ -83,9 +81,5 @@ class SQLite(AuthOperations, BuildOperations, PackageOperations, PatchOperations
 
         paths = configuration.repository_paths
 
-        def run(connection: Connection) -> None:
+        self.with_connection(lambda conn: Migrations.migrate(conn, configuration))
-            result = Migrations.migrate(connection)
-            migrate_data(result, connection, configuration, paths)
-
-        self.with_connection(run)
         paths.chown(self.path)

tests/ahriman/core/database/data/test_data_init.py

@@ -16,20 +16,19 @@ def test_migrate_data_initial(connection: Connection, configuration: Configurati
     patches = mocker.patch("ahriman.core.database.data.migrate_patches")
     users = mocker.patch("ahriman.core.database.data.migrate_users_data")
 
-    migrate_data(MigrationResult(old_version=0, new_version=900), connection, configuration, repository_paths)
+    migrate_data(MigrationResult(old_version=0, new_version=900), connection, configuration)
     packages.assert_called_once_with(connection, repository_paths)
     patches.assert_called_once_with(connection, repository_paths)
     users.assert_called_once_with(connection, configuration)
 
 
-def test_migrate_data_skip(connection: Connection, configuration: Configuration,
+def test_migrate_data_skip(connection: Connection, configuration: Configuration, mocker: MockerFixture) -> None:
-                           repository_paths: RepositoryPaths, mocker: MockerFixture) -> None:
     """
     must not migrate data if version is up-to-date
     """
     packages = mocker.patch("ahriman.core.database.data.migrate_package_statuses")
     users = mocker.patch("ahriman.core.database.data.migrate_users_data")
 
-    migrate_data(MigrationResult(old_version=900, new_version=900), connection, configuration, repository_paths)
+    migrate_data(MigrationResult(old_version=900, new_version=900), connection, configuration)
     packages.assert_not_called()
     users.assert_not_called()

tests/ahriman/core/database/data/test_package_statuses.py

@@ -19,10 +19,8 @@ def test_migrate_package_statuses(connection: Connection, package_ahriman: Packa
     mocker.patch("pathlib.Path.is_file", return_value=True)
     mocker.patch("pathlib.Path.open")
     mocker.patch("json.load", return_value=response)
-    unlink_mock = mocker.patch("pathlib.Path.unlink")
 
     migrate_package_statuses(connection, repository_paths)
-    unlink_mock.assert_called_once_with()
     connection.execute.assert_has_calls([
         mock.call(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int)),
         mock.call(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int)),
@@ -30,7 +28,6 @@ def test_migrate_package_statuses(connection: Connection, package_ahriman: Packa
     connection.executemany.assert_has_calls([
         mock.call(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int)),
     ])
-    connection.commit.assert_called_once_with()
 
 
 def test_migrate_package_statuses_skip(connection: Connection, repository_paths: RepositoryPaths,
@@ -40,4 +37,3 @@ def test_migrate_package_statuses_skip(connection: Connection, repository_paths:
     """
     mocker.patch("pathlib.Path.is_file", return_value=False)
     migrate_package_statuses(connection, repository_paths)
-    connection.commit.assert_not_called()

tests/ahriman/core/database/data/test_patches.py

@@ -23,7 +23,6 @@ def test_migrate_patches(connection: Connection, repository_paths: RepositoryPat
     iterdir_mock.assert_called_once_with()
     read_mock.assert_called_once_with(encoding="utf8")
     connection.execute.assert_called_once_with(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int))
-    connection.commit.assert_called_once_with()
 
 
 def test_migrate_patches_skip(connection: Connection, repository_paths: RepositoryPaths,

tests/ahriman/core/database/data/test_users.py

@@ -19,4 +19,3 @@ def test_migrate_users_data(connection: Connection, configuration: Configuration
         mock.call(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int)),
         mock.call(pytest.helpers.anyvar(str, strict=True), pytest.helpers.anyvar(int)),
     ])
-    connection.commit.assert_called_once_with()

tests/ahriman/core/database/migrations/conftest.py

@@ -2,18 +2,20 @@ import pytest
 
 from sqlite3 import Connection
 
+from ahriman.core.configuration import Configuration
 from ahriman.core.database.migrations import Migrations
 
 
 @pytest.fixture
-def migrations(connection: Connection) -> Migrations:
+def migrations(connection: Connection, configuration: Configuration) -> Migrations:
     """
     fixture for migrations object
 
     Args:
         connection(Connection): sqlite connection fixture
+        configuration(Configuration): configuration fixture
 
     Returns:
         Migrations: migrations test instance
     """
-    return Migrations(connection)
+    return Migrations(connection, configuration)

tests/ahriman/core/database/migrations/test_migrations_init.py

@@ -5,17 +5,18 @@ from sqlite3 import Connection
 from unittest import mock
 from unittest.mock import MagicMock
 
+from ahriman.core.configuration import Configuration
 from ahriman.core.database.migrations import Migrations
 from ahriman.models.migration import Migration
 from ahriman.models.migration_result import MigrationResult
 
 
-def test_migrate(connection: Connection, mocker: MockerFixture) -> None:
+def test_migrate(connection: Connection, configuration: Configuration, mocker: MockerFixture) -> None:
     """
     must perform migrations
     """
     run_mock = mocker.patch("ahriman.core.database.migrations.Migrations.run")
-    Migrations.migrate(connection)
+    Migrations.migrate(connection, configuration)
     run_mock.assert_called_once_with()
 
 
@@ -46,6 +47,7 @@ def test_run(migrations: Migrations, mocker: MockerFixture) -> None:
                  return_value=[Migration(0, "test", ["select 1"])])
     migrations.connection.cursor.return_value = cursor
     validate_mock = mocker.patch("ahriman.models.migration_result.MigrationResult.validate")
+    migrate_data_mock = mocker.patch("ahriman.core.database.migrations.migrate_data")
 
     migrations.run()
     validate_mock.assert_called_once_with()
@@ -56,6 +58,7 @@ def test_run(migrations: Migrations, mocker: MockerFixture) -> None:
         mock.call("commit"),
     ])
     cursor.close.assert_called_once_with()
+    migrate_data_mock.assert_called_once_with(MigrationResult(0, 1), migrations.connection, migrations.configuration)
 
 
 def test_run_migration_exception(migrations: Migrations, mocker: MockerFixture) -> None:

tests/ahriman/core/database/test_sqlite.py

@@ -20,9 +20,5 @@ def test_init(database: SQLite, configuration: Configuration, mocker: MockerFixt
     must run migrations on init
     """
     migrate_schema_mock = mocker.patch("ahriman.core.database.migrations.Migrations.migrate")
-    migrate_data_mock = mocker.patch("ahriman.core.database.sqlite.migrate_data")
-
     database.init(configuration)
-    migrate_schema_mock.assert_called_once_with(pytest.helpers.anyvar(int))
+    migrate_schema_mock.assert_called_once_with(pytest.helpers.anyvar(int), configuration)
-    migrate_data_mock.assert_called_once_with(
-        pytest.helpers.anyvar(int), pytest.helpers.anyvar(int), configuration, configuration.repository_paths)