bug: fix removal of the packages

It has been broken since reporter improvements, because it effectivelly
1) didn't call remove functions in database
2) used empty repository identifier for web service

With those changes it also raises exception when you try to call id on
empty identifier
This commit is contained in:
Evgenii Alekseev 2024-09-04 21:31:52 +03:00
parent fd8c8a00d0
commit 4b2f6bbee9
11 changed files with 41 additions and 22 deletions

View File

@ -75,7 +75,9 @@ class Lock(LazyLogging):
""" """
self.path: Path | None = None self.path: Path | None = None
if args.lock is not None: if args.lock is not None:
self.path = args.lock.with_stem(f"{args.lock.stem}_{repository_id.id}") self.path = args.lock
if not repository_id.is_empty:
self.path = self.path.with_stem(f"{args.lock.stem}_{repository_id.id}")
if not self.path.is_absolute(): if not self.path.is_absolute():
# prepend full path to the lock file # prepend full path to the lock file
self.path = Path("/") / "run" / "ahriman" / self.path self.path = Path("/") / "run" / "ahriman" / self.path

View File

@ -27,6 +27,7 @@ from ahriman.core.configuration import Configuration
from ahriman.core.database.migrations import Migrations from ahriman.core.database.migrations import Migrations
from ahriman.core.database.operations import AuthOperations, BuildOperations, ChangesOperations, \ from ahriman.core.database.operations import AuthOperations, BuildOperations, ChangesOperations, \
DependenciesOperations, LogsOperations, PackageOperations, PatchOperations DependenciesOperations, LogsOperations, PackageOperations, PatchOperations
from ahriman.models.repository_id import RepositoryId
# pylint: disable=too-many-ancestors # pylint: disable=too-many-ancestors
@ -102,23 +103,26 @@ class SQLite(
self.with_connection(lambda connection: Migrations.migrate(connection, configuration)) self.with_connection(lambda connection: Migrations.migrate(connection, configuration))
paths.chown(self.path) paths.chown(self.path)
def package_clear(self, package_base: str) -> None: def package_clear(self, package_base: str, repository_id: RepositoryId | None = None) -> None:
""" """
completely remove package from all tables completely remove package from all tables
Args: Args:
package_base(str): package base to remove package_base(str): package base to remove
repository_id(RepositoryId, optional): repository unique identifier override (Default value = None)
Examples: Examples:
This method completely removes the package from all tables and must be used, e.g. on package removal:: This method completely removes the package from all tables and must be used, e.g. on package removal::
>>> database.package_clear("ahriman") >>> database.package_clear("ahriman")
""" """
self.build_queue_clear(package_base) self.build_queue_clear(package_base, repository_id)
self.patches_remove(package_base, []) self.patches_remove(package_base, None)
self.logs_remove(package_base, None) self.logs_remove(package_base, None, repository_id)
self.changes_remove(package_base) self.changes_remove(package_base, repository_id)
self.dependencies_remove(package_base) self.dependencies_remove(package_base, repository_id)
self.package_remove(package_base, repository_id)
# remove local cache too # remove local cache too
self._repository_paths.tree_clear(package_base) self._repository_paths.tree_clear(package_base)

View File

@ -184,7 +184,7 @@ class LocalClient(Client):
Args: Args:
package_base(str): package base to remove package_base(str): package base to remove
""" """
self.database.package_clear(package_base) self.database.package_clear(package_base, self.repository_id)
def package_status_update(self, package_base: str, status: BuildStatusEnum) -> None: def package_status_update(self, package_base: str, status: BuildStatusEnum) -> None:
""" """

View File

@ -140,7 +140,6 @@ class Watcher(LazyLogging):
with self._lock: with self._lock:
self._known.pop(package_base, None) self._known.pop(package_base, None)
self.client.package_remove(package_base) self.client.package_remove(package_base)
self.package_logs_remove(package_base, None)
def package_status_update(self, package_base: str, status: BuildStatusEnum) -> None: def package_status_update(self, package_base: str, status: BuildStatusEnum) -> None:
""" """

View File

@ -41,9 +41,12 @@ class RepositoryId:
Returns: Returns:
str: unique id for this repository str: unique id for this repository
Raises:
ValueError: if repository identifier is empty
""" """
if self.is_empty: if self.is_empty:
return "" raise ValueError("Repository ID is called on empty repository identifier")
return f"{self.architecture}-{self.name}" # basically the same as used for command line return f"{self.architecture}-{self.name}" # basically the same as used for command line
@property @property

View File

@ -14,6 +14,7 @@ from ahriman.core.configuration import Configuration
from ahriman.core.exceptions import DuplicateRunError, UnsafeRunError from ahriman.core.exceptions import DuplicateRunError, UnsafeRunError
from ahriman.models.build_status import BuildStatus, BuildStatusEnum from ahriman.models.build_status import BuildStatus, BuildStatusEnum
from ahriman.models.internal_status import InternalStatus from ahriman.models.internal_status import InternalStatus
from ahriman.models.repository_id import RepositoryId
def test_path(args: argparse.Namespace, configuration: Configuration) -> None: def test_path(args: argparse.Namespace, configuration: Configuration) -> None:
@ -30,6 +31,8 @@ def test_path(args: argparse.Namespace, configuration: Configuration) -> None:
args.lock = Path("ahriman.pid") args.lock = Path("ahriman.pid")
assert Lock(args, repository_id, configuration).path == Path("/run/ahriman/ahriman_x86_64-aur-clone.pid") assert Lock(args, repository_id, configuration).path == Path("/run/ahriman/ahriman_x86_64-aur-clone.pid")
assert Lock(args, RepositoryId("", ""), configuration).path == Path("/run/ahriman/ahriman.pid")
with pytest.raises(ValueError): with pytest.raises(ValueError):
args.lock = Path("/") args.lock = Path("/")
assert Lock(args, repository_id, configuration).path # special case assert Lock(args, repository_id, configuration).path # special case

View File

@ -4,6 +4,7 @@ from pytest_mock import MockerFixture
from ahriman.core.configuration import Configuration from ahriman.core.configuration import Configuration
from ahriman.core.database import SQLite from ahriman.core.database import SQLite
from ahriman.models.repository_id import RepositoryId
def test_load(configuration: Configuration, mocker: MockerFixture) -> None: def test_load(configuration: Configuration, mocker: MockerFixture) -> None:
@ -35,7 +36,7 @@ def test_init_skip_migration(database: SQLite, configuration: Configuration, moc
migrate_schema_mock.assert_not_called() migrate_schema_mock.assert_not_called()
def test_package_clear(database: SQLite, mocker: MockerFixture) -> None: def test_package_clear(database: SQLite, repository_id: RepositoryId, mocker: MockerFixture) -> None:
""" """
must clear package data must clear package data
""" """
@ -44,12 +45,14 @@ def test_package_clear(database: SQLite, mocker: MockerFixture) -> None:
logs_mock = mocker.patch("ahriman.core.database.SQLite.logs_remove") logs_mock = mocker.patch("ahriman.core.database.SQLite.logs_remove")
changes_mock = mocker.patch("ahriman.core.database.SQLite.changes_remove") changes_mock = mocker.patch("ahriman.core.database.SQLite.changes_remove")
dependencies_mock = mocker.patch("ahriman.core.database.SQLite.dependencies_remove") dependencies_mock = mocker.patch("ahriman.core.database.SQLite.dependencies_remove")
package_mock = mocker.patch("ahriman.core.database.SQLite.package_remove")
tree_clear_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.tree_clear") tree_clear_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.tree_clear")
database.package_clear("package") database.package_clear("package", repository_id)
build_queue_mock.assert_called_once_with("package") build_queue_mock.assert_called_once_with("package", repository_id)
patches_mock.assert_called_once_with("package", []) patches_mock.assert_called_once_with("package", None)
logs_mock.assert_called_once_with("package", None) logs_mock.assert_called_once_with("package", None, repository_id)
changes_mock.assert_called_once_with("package") changes_mock.assert_called_once_with("package", repository_id)
dependencies_mock.assert_called_once_with("package") dependencies_mock.assert_called_once_with("package", repository_id)
package_mock.assert_called_once_with("package", repository_id)
tree_clear_mock.assert_called_once_with("package") tree_clear_mock.assert_called_once_with("package")

View File

@ -158,7 +158,7 @@ def test_package_remove(local_client: LocalClient, package_ahriman: Package, moc
""" """
package_mock = mocker.patch("ahriman.core.database.SQLite.package_clear") package_mock = mocker.patch("ahriman.core.database.SQLite.package_clear")
local_client.package_remove(package_ahriman.base) local_client.package_remove(package_ahriman.base)
package_mock.assert_called_once_with(package_ahriman.base) package_mock.assert_called_once_with(package_ahriman.base, local_client.repository_id)
def test_package_status_update(local_client: LocalClient, package_ahriman: Package, mocker: MockerFixture) -> None: def test_package_status_update(local_client: LocalClient, package_ahriman: Package, mocker: MockerFixture) -> None:

View File

@ -101,13 +101,11 @@ def test_package_remove(watcher: Watcher, package_ahriman: Package, mocker: Mock
must remove package base must remove package base
""" """
cache_mock = mocker.patch("ahriman.core.status.local_client.LocalClient.package_remove") cache_mock = mocker.patch("ahriman.core.status.local_client.LocalClient.package_remove")
logs_mock = mocker.patch("ahriman.core.status.watcher.Watcher.package_logs_remove", create=True)
watcher._known = {package_ahriman.base: (package_ahriman, BuildStatus())} watcher._known = {package_ahriman.base: (package_ahriman, BuildStatus())}
watcher.package_remove(package_ahriman.base) watcher.package_remove(package_ahriman.base)
assert not watcher._known assert not watcher._known
cache_mock.assert_called_once_with(package_ahriman.base) cache_mock.assert_called_once_with(package_ahriman.base)
logs_mock.assert_called_once_with(package_ahriman.base, None)
def test_package_remove_unknown(watcher: Watcher, package_ahriman: Package, mocker: MockerFixture) -> None: def test_package_remove_unknown(watcher: Watcher, package_ahriman: Package, mocker: MockerFixture) -> None:

View File

@ -7,10 +7,17 @@ def test_id() -> None:
""" """
must correctly generate id must correctly generate id
""" """
assert RepositoryId("", "").id == ""
assert RepositoryId("arch", "repo").id == "arch-repo" assert RepositoryId("arch", "repo").id == "arch-repo"
def test_id_empty() -> None:
"""
must raise exception on empty identifier
"""
with pytest.raises(ValueError):
assert RepositoryId("", "").id
def test_is_empty() -> None: def test_is_empty() -> None:
""" """
must check if repository id is empty or not must check if repository id is empty or not

View File

@ -201,7 +201,7 @@ def test_service_not_found(base: BaseView) -> None:
must raise HTTPNotFound if no repository found must raise HTTPNotFound if no repository found
""" """
with pytest.raises(HTTPNotFound): with pytest.raises(HTTPNotFound):
base.service(RepositoryId("", "")) base.service(RepositoryId("repo", "arch"))
def test_service_package(base: BaseView, repository_id: RepositoryId, mocker: MockerFixture) -> None: def test_service_package(base: BaseView, repository_id: RepositoryId, mocker: MockerFixture) -> None: