Skip to content

Commit

Permalink
bug: fix removal of the packages
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arcan1s committed Sep 4, 2024
1 parent fd8c8a0 commit 4b2f6bb
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 22 deletions.
4 changes: 3 additions & 1 deletion src/ahriman/application/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ def __init__(self, args: argparse.Namespace, repository_id: RepositoryId, config
"""
self.path: Path | None = 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():
# prepend full path to the lock file
self.path = Path("/") / "run" / "ahriman" / self.path
Expand Down
16 changes: 10 additions & 6 deletions src/ahriman/core/database/sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from ahriman.core.database.migrations import Migrations
from ahriman.core.database.operations import AuthOperations, BuildOperations, ChangesOperations, \
DependenciesOperations, LogsOperations, PackageOperations, PatchOperations
from ahriman.models.repository_id import RepositoryId


# pylint: disable=too-many-ancestors
Expand Down Expand Up @@ -102,23 +103,26 @@ def init(self, configuration: Configuration) -> None:
self.with_connection(lambda connection: Migrations.migrate(connection, configuration))
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
Args:
package_base(str): package base to remove
repository_id(RepositoryId, optional): repository unique identifier override (Default value = None)
Examples:
This method completely removes the package from all tables and must be used, e.g. on package removal::
>>> database.package_clear("ahriman")
"""
self.build_queue_clear(package_base)
self.patches_remove(package_base, [])
self.logs_remove(package_base, None)
self.changes_remove(package_base)
self.dependencies_remove(package_base)
self.build_queue_clear(package_base, repository_id)
self.patches_remove(package_base, None)
self.logs_remove(package_base, None, repository_id)
self.changes_remove(package_base, repository_id)
self.dependencies_remove(package_base, repository_id)

self.package_remove(package_base, repository_id)

# remove local cache too
self._repository_paths.tree_clear(package_base)
2 changes: 1 addition & 1 deletion src/ahriman/core/status/local_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def package_remove(self, package_base: str) -> None:
Args:
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:
"""
Expand Down
1 change: 0 additions & 1 deletion src/ahriman/core/status/watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ def package_remove(self, package_base: str) -> None:
with self._lock:
self._known.pop(package_base, None)
self.client.package_remove(package_base)
self.package_logs_remove(package_base, None)

def package_status_update(self, package_base: str, status: BuildStatusEnum) -> None:
"""
Expand Down
5 changes: 4 additions & 1 deletion src/ahriman/models/repository_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ def id(self) -> str:
Returns:
str: unique id for this repository
Raises:
ValueError: if repository identifier 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

@property
Expand Down
3 changes: 3 additions & 0 deletions tests/ahriman/application/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from ahriman.core.exceptions import DuplicateRunError, UnsafeRunError
from ahriman.models.build_status import BuildStatus, BuildStatusEnum
from ahriman.models.internal_status import InternalStatus
from ahriman.models.repository_id import RepositoryId


def test_path(args: argparse.Namespace, configuration: Configuration) -> None:
Expand All @@ -30,6 +31,8 @@ def test_path(args: argparse.Namespace, configuration: Configuration) -> None:
args.lock = Path("ahriman.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):
args.lock = Path("/")
assert Lock(args, repository_id, configuration).path # special case
Expand Down
17 changes: 10 additions & 7 deletions tests/ahriman/core/database/test_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from ahriman.core.configuration import Configuration
from ahriman.core.database import SQLite
from ahriman.models.repository_id import RepositoryId


def test_load(configuration: Configuration, mocker: MockerFixture) -> None:
Expand Down Expand Up @@ -35,7 +36,7 @@ def test_init_skip_migration(database: SQLite, configuration: Configuration, moc
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
"""
Expand All @@ -44,12 +45,14 @@ def test_package_clear(database: SQLite, mocker: MockerFixture) -> None:
logs_mock = mocker.patch("ahriman.core.database.SQLite.logs_remove")
changes_mock = mocker.patch("ahriman.core.database.SQLite.changes_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")

database.package_clear("package")
build_queue_mock.assert_called_once_with("package")
patches_mock.assert_called_once_with("package", [])
logs_mock.assert_called_once_with("package", None)
changes_mock.assert_called_once_with("package")
dependencies_mock.assert_called_once_with("package")
database.package_clear("package", repository_id)
build_queue_mock.assert_called_once_with("package", repository_id)
patches_mock.assert_called_once_with("package", None)
logs_mock.assert_called_once_with("package", None, repository_id)
changes_mock.assert_called_once_with("package", repository_id)
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")
2 changes: 1 addition & 1 deletion tests/ahriman/core/status/test_local_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
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:
Expand Down
2 changes: 0 additions & 2 deletions tests/ahriman/core/status/test_watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,11 @@ def test_package_remove(watcher: Watcher, package_ahriman: Package, mocker: Mock
must remove package base
"""
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.package_remove(package_ahriman.base)
assert not watcher._known
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:
Expand Down
9 changes: 8 additions & 1 deletion tests/ahriman/models/test_repository_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@ def test_id() -> None:
"""
must correctly generate id
"""
assert RepositoryId("", "").id == ""
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:
"""
must check if repository id is empty or not
Expand Down
2 changes: 1 addition & 1 deletion tests/ahriman/web/views/test_view_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ def test_service_not_found(base: BaseView) -> None:
must raise HTTPNotFound if no repository found
"""
with pytest.raises(HTTPNotFound):
base.service(RepositoryId("", ""))
base.service(RepositoryId("repo", "arch"))


def test_service_package(base: BaseView, repository_id: RepositoryId, mocker: MockerFixture) -> None:
Expand Down

0 comments on commit 4b2f6bb

Please sign in to comment.