From fc8f6c298530ebbd05bbb3e6950e2d07d579220e Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Fri, 10 Nov 2023 17:09:01 +0200 Subject: [PATCH] feat: extend result class --- .../application/application_packages.py | 10 +- src/ahriman/core/exceptions.py | 15 --- src/ahriman/core/report/email.py | 2 +- src/ahriman/core/report/html.py | 2 +- src/ahriman/core/repository/executor.py | 16 +-- src/ahriman/models/result.py | 119 ++++++++++++++---- .../application/test_application_packages.py | 2 +- .../application/handlers/test_handler_add.py | 2 +- .../handlers/test_handler_rebuild.py | 2 +- .../handlers/test_handler_update.py | 2 +- tests/ahriman/conftest.py | 2 +- .../core/report/test_jinja_template.py | 4 +- tests/ahriman/models/test_result.py | 64 +++++----- 13 files changed, 155 insertions(+), 87 deletions(-) diff --git a/src/ahriman/application/application/application_packages.py b/src/ahriman/application/application/application_packages.py index 32428c2a..68884a67 100644 --- a/src/ahriman/application/application/application_packages.py +++ b/src/ahriman/application/application/application_packages.py @@ -167,12 +167,16 @@ class ApplicationPackages(ApplicationProperties): """ raise NotImplementedError - def remove(self, names: Iterable[str]) -> None: + def remove(self, names: Iterable[str]) -> Result: """ remove packages from repository Args: names(Iterable[str]): list of packages (either base or name) to remove + + Returns: + Result: removal result """ - self.repository.process_remove(names) - self.on_result(Result()) + result = self.repository.process_remove(names) + self.on_result(result) + return result diff --git a/src/ahriman/core/exceptions.py b/src/ahriman/core/exceptions.py index 569c5fa1..43efbd50 100644 --- a/src/ahriman/core/exceptions.py +++ b/src/ahriman/core/exceptions.py @@ -316,21 +316,6 @@ class UnknownPackageError(ValueError): ValueError.__init__(self, f"Package base {package_base} is unknown") -class UnprocessedPackageStatusError(ValueError): - """ - exception for merging invalid statues - """ - - def __init__(self, package_base: str) -> None: - """ - default constructor - - Args: - package_base(str): package base name - """ - ValueError.__init__(self, f"Package base {package_base} had status failed, but new status is success") - - class UnsafeRunError(RuntimeError): """ exception which will be raised in case if user is not owner of repository diff --git a/src/ahriman/core/report/email.py b/src/ahriman/core/report/email.py index e42b827a..cc8cc579 100644 --- a/src/ahriman/core/report/email.py +++ b/src/ahriman/core/report/email.py @@ -120,6 +120,6 @@ class Email(Report, JinjaTemplate): text = self.make_html(result, self.template) attachments = {} if self.template_full is not None: - attachments["index.html"] = self.make_html(Result(success=packages), self.template_full) + attachments["index.html"] = self.make_html(Result(updated=packages), self.template_full) self._send(text, attachments) diff --git a/src/ahriman/core/report/html.py b/src/ahriman/core/report/html.py index 55deb739..393460e2 100644 --- a/src/ahriman/core/report/html.py +++ b/src/ahriman/core/report/html.py @@ -58,5 +58,5 @@ class HTML(Report, JinjaTemplate): packages(list[Package]): list of packages to generate report result(Result): build result """ - html = self.make_html(Result(success=packages), self.template) + html = self.make_html(Result(updated=packages), self.template) self.report_path.write_text(html, encoding="utf8") diff --git a/src/ahriman/core/repository/executor.py b/src/ahriman/core/repository/executor.py index a5266468..a39a41f3 100644 --- a/src/ahriman/core/repository/executor.py +++ b/src/ahriman/core/repository/executor.py @@ -98,7 +98,7 @@ class Executor(Cleaner): try: packager = self.packager(packagers, single.base) build_single(single, Path(dir_name), packager.packager_id) - result.add_success(single) + result.add_updated(single) except Exception: self.reporter.set_failed(single.base) result.add_failed(single) @@ -106,7 +106,7 @@ class Executor(Cleaner): return result - def process_remove(self, packages: Iterable[str]) -> Path: + def process_remove(self, packages: Iterable[str]) -> Result: """ remove packages from list @@ -114,7 +114,7 @@ class Executor(Cleaner): packages(Iterable[str]): list of package names or bases to remove Returns: - Path: path to repository database + Result: remove result """ def remove_base(package_base: str) -> None: try: @@ -126,9 +126,9 @@ class Executor(Cleaner): except Exception: self.logger.exception("could not remove base %s", package_base) - def remove_package(package: str, fn: Path) -> None: + def remove_package(package: str, archive_path: Path) -> None: try: - self.repo.remove(package, fn) # remove the package itself + self.repo.remove(package, archive_path) # remove the package itself except Exception: self.logger.exception("could not remove %s", package) @@ -136,6 +136,7 @@ class Executor(Cleaner): bases_to_remove: list[str] = [] # build package list based on user input + result = Result() requested = set(packages) for local in self.packages(): if local.base in packages or all(package in requested for package in local.packages): @@ -145,6 +146,7 @@ class Executor(Cleaner): if properties.filepath is not None }) bases_to_remove.append(local.base) + result.add_removed(local) elif requested.intersection(local.packages.keys()): packages_to_remove.update({ package: properties.filepath @@ -167,7 +169,7 @@ class Executor(Cleaner): for package in bases_to_remove: remove_base(package) - return self.repo.repo_path + return result def process_update(self, packages: Iterable[Path], packagers: Packagers | None = None) -> Result: """ @@ -219,7 +221,7 @@ class Executor(Cleaner): rename(description, local.base) update_single(description.filename, local.base, packager.key) self.reporter.set_success(local) - result.add_success(local) + result.add_updated(local) current_package_archives: set[str] = set() if local.base in current_packages: diff --git a/src/ahriman/models/result.py b/src/ahriman/models/result.py index ca328013..21b194f5 100644 --- a/src/ahriman/models/result.py +++ b/src/ahriman/models/result.py @@ -19,28 +19,50 @@ # from __future__ import annotations -from collections.abc import Iterable -from typing import Any +from collections.abc import Iterable, Callable +from typing import Any, Self -from ahriman.core.exceptions import UnprocessedPackageStatusError from ahriman.models.package import Package class Result: """ build result class holder + + Attributes: + STATUS_PRIORITIES(list[str]): (class attribute) list of statues according to their priorities """ - def __init__(self, success: Iterable[Package] | None = None, failed: Iterable[Package] | None = None) -> None: + STATUS_PRIORITIES = [ + "failed", + "removed", + "updated", + "added", + ] + + def __init__(self, *, added: Iterable[Package] | None = None, updated: Iterable[Package] | None = None, + removed: Iterable[Package] | None = None, failed: Iterable[Package] | None = None) -> None: """ default constructor Args: - success(Iterable[Package] | None, optional): initial list of successes packages (Default value = None) + addded(Iterable[Package] | None, optional): initial list of successfully added packages + (Default value = None) + updated(Iterable[Package] | None, optional): initial list of successfully updated packages + (Default value = None) + removed(Iterable[Package] | None, optional): initial list of successfully removed packages + (Default value = None) failed(Iterable[Package] | None, optional): initial list of failed packages (Default value = None) """ - success = success or [] - self._success = {package.base: package for package in success} + added = added or [] + self._added = {package.base: package for package in added} + + updated = updated or [] + self._updated = {package.base: package for package in updated} + + removed = removed or [] + self._removed = {package.base: package for package in removed} + failed = failed or [] self._failed = {package.base: package for package in failed} @@ -62,7 +84,17 @@ class Result: Returns: bool: True in case if success list is empty and False otherwise """ - return not bool(self._success) + return not self._added and not self._updated + + @property + def removed(self) -> list[Package]: + """ + get list of removed packages + + Returns: + list[Package]: list of packages successfully removed + """ + return list(self._removed.values()) @property def success(self) -> list[Package]: @@ -72,7 +104,16 @@ class Result: Returns: list[Package]: list of packages with success result """ - return list(self._success.values()) + return list(self._added.values()) + list(self._updated.values()) + + def add_added(self, package: Package) -> None: + """ + add new package to new packages list + + Args: + package(Package): package removed + """ + self._added[package.base] = package def add_failed(self, package: Package) -> None: """ @@ -83,17 +124,26 @@ class Result: """ self._failed[package.base] = package - def add_success(self, package: Package) -> None: + def add_removed(self, package: Package) -> None: + """ + add new package to removed list + + Args: + package(Package): package removed + """ + self._removed[package.base] = package + + def add_updated(self, package: Package) -> None: """ add new package to success built Args: package(Package): package built """ - self._success[package.base] = package + self._updated[package.base] = package # pylint: disable=protected-access - def merge(self, other: Result) -> Result: + def merge(self, other: Result) -> Self: """ merge other result into this one. This method assumes that other has fresh info about status and override it @@ -101,19 +151,35 @@ class Result: other(Result): instance of the newest result Returns: - Result: updated instance - - Raises: - UnprocessedPackageStatusError: if there is previously failed package which is masked as success + Self: updated instance """ - for base, package in other._failed.items(): - if base in self._success: - del self._success[base] - self.add_failed(package) - for base, package in other._success.items(): - if base in self._failed: - raise UnprocessedPackageStatusError(base) - self.add_success(package) + for status in self.STATUS_PRIORITIES: + new_packages: Iterable[Package] = getattr(other, f"_{status}", {}).values() + insert_package: Callable[[Package], None] = getattr(self, f"add_{status}") + for package in new_packages: + insert_package(package) + + return self.refine() + + def refine(self) -> Self: + """ + merge packages between different results (e.g. remove failed from added, etc.) removing duplicates + + Returns: + Self: updated instance + """ + for index, base_status in enumerate(self.STATUS_PRIORITIES): + # extract top-level packages + base_packages: Iterable[str] = getattr(self, f"_{base_status}", {}).keys() + # extract packages for each bottom-level + for status in self.STATUS_PRIORITIES[index + 1:]: + packages: dict[str, Package] = getattr(self, f"_{status}", {}) + + # if there is top-level package in bottom-level, then remove it + for base in base_packages: + if base in packages: + del packages[base] + return self # required for tests at least @@ -129,4 +195,7 @@ class Result: """ if not isinstance(other, Result): return False - return self.success == other.success and self.failed == other.failed + return self._added == other._added \ + and self._removed == other._removed \ + and self._updated == other._updated \ + and self._failed == other._failed diff --git a/tests/ahriman/application/application/test_application_packages.py b/tests/ahriman/application/application/test_application_packages.py index 56cfb121..0b7ec150 100644 --- a/tests/ahriman/application/application/test_application_packages.py +++ b/tests/ahriman/application/application/test_application_packages.py @@ -228,7 +228,7 @@ def test_remove(application_packages: ApplicationPackages, mocker: MockerFixture """ must remove package """ - executor_mock = mocker.patch("ahriman.core.repository.executor.Executor.process_remove") + executor_mock = mocker.patch("ahriman.core.repository.executor.Executor.process_remove", return_value=Result()) on_result_mock = mocker.patch("ahriman.application.application.application_packages.ApplicationPackages.on_result") application_packages.remove([]) diff --git a/tests/ahriman/application/handlers/test_handler_add.py b/tests/ahriman/application/handlers/test_handler_add.py index c09e767f..0be3a281 100644 --- a/tests/ahriman/application/handlers/test_handler_add.py +++ b/tests/ahriman/application/handlers/test_handler_add.py @@ -77,7 +77,7 @@ def test_run_with_updates(args: argparse.Namespace, configuration: Configuration args = _default_args(args) args.now = True result = Result() - result.add_success(package_ahriman) + result.add_updated(package_ahriman) mocker.patch("ahriman.application.application.Application.add") mocker.patch("ahriman.core.repository.Repository.load", return_value=repository) application_mock = mocker.patch("ahriman.application.application.Application.update", return_value=result) diff --git a/tests/ahriman/application/handlers/test_handler_rebuild.py b/tests/ahriman/application/handlers/test_handler_rebuild.py index d7d1122b..6ba76ae1 100644 --- a/tests/ahriman/application/handlers/test_handler_rebuild.py +++ b/tests/ahriman/application/handlers/test_handler_rebuild.py @@ -41,7 +41,7 @@ def test_run(args: argparse.Namespace, package_ahriman: Package, configuration: """ args = _default_args(args) result = Result() - result.add_success(package_ahriman) + result.add_updated(package_ahriman) mocker.patch("ahriman.core.repository.Repository.load", return_value=repository) extract_mock = mocker.patch("ahriman.application.handlers.Rebuild.extract_packages", return_value=[package_ahriman]) application_packages_mock = mocker.patch("ahriman.core.repository.repository.Repository.packages_depend_on", diff --git a/tests/ahriman/application/handlers/test_handler_update.py b/tests/ahriman/application/handlers/test_handler_update.py index 40f61fdf..0bf501fa 100644 --- a/tests/ahriman/application/handlers/test_handler_update.py +++ b/tests/ahriman/application/handlers/test_handler_update.py @@ -44,7 +44,7 @@ def test_run(args: argparse.Namespace, package_ahriman: Package, configuration: """ args = _default_args(args) result = Result() - result.add_success(package_ahriman) + result.add_updated(package_ahriman) mocker.patch("ahriman.core.repository.Repository.load", return_value=repository) application_mock = mocker.patch("ahriman.application.application.Application.update", return_value=result) check_mock = mocker.patch("ahriman.application.handlers.Handler.check_if_empty") diff --git a/tests/ahriman/conftest.py b/tests/ahriman/conftest.py index b6037f5a..1d5f7654 100644 --- a/tests/ahriman/conftest.py +++ b/tests/ahriman/conftest.py @@ -526,7 +526,7 @@ def result(package_ahriman: Package) -> Result: Result: result test instance """ result = Result() - result.add_success(package_ahriman) + result.add_updated(package_ahriman) return result diff --git a/tests/ahriman/core/report/test_jinja_template.py b/tests/ahriman/core/report/test_jinja_template.py index 65cc5c23..b9f845f5 100644 --- a/tests/ahriman/core/report/test_jinja_template.py +++ b/tests/ahriman/core/report/test_jinja_template.py @@ -11,7 +11,7 @@ def test_generate(configuration: Configuration, package_ahriman: Package) -> Non name = configuration.getpath("html", "template") _, repository_id = configuration.check_loaded() report = JinjaTemplate(repository_id, configuration, "html") - assert report.make_html(Result(success=[package_ahriman]), name) + assert report.make_html(Result(updated=[package_ahriman]), name) def test_generate_from_path(configuration: Configuration, package_ahriman: Package) -> None: @@ -21,4 +21,4 @@ def test_generate_from_path(configuration: Configuration, package_ahriman: Packa path = configuration.getpath("html", "templates") / configuration.get("html", "template") _, repository_id = configuration.check_loaded() report = JinjaTemplate(repository_id, configuration, "html") - assert report.make_html(Result(success=[package_ahriman]), path) + assert report.make_html(Result(updated=[package_ahriman]), path) diff --git a/tests/ahriman/models/test_result.py b/tests/ahriman/models/test_result.py index 51ef1c06..3543c6c7 100644 --- a/tests/ahriman/models/test_result.py +++ b/tests/ahriman/models/test_result.py @@ -1,6 +1,3 @@ -import pytest - -from ahriman.core.exceptions import UnprocessedPackageStatusError from ahriman.models.package import Package from ahriman.models.result import Result @@ -18,7 +15,7 @@ def test_non_empty_success(package_ahriman: Package) -> None: must be non-empty if there is success build """ result = Result() - result.add_success(package_ahriman) + result.add_updated(package_ahriman) assert not result.is_empty @@ -37,11 +34,22 @@ def test_non_empty_full(package_ahriman: Package) -> None: """ result = Result() result.add_failed(package_ahriman) - result.add_success(package_ahriman) + result.add_updated(package_ahriman) assert not result.is_empty +def test_add_added(package_ahriman: Package) -> None: + """ + must add package to new packages list + """ + result = Result() + result.add_added(package_ahriman) + assert not result.failed + assert not result.removed + assert result.success == [package_ahriman] + + def test_add_failed(package_ahriman: Package) -> None: """ must add package to failed list @@ -49,17 +57,30 @@ def test_add_failed(package_ahriman: Package) -> None: result = Result() result.add_failed(package_ahriman) assert result.failed == [package_ahriman] + assert not result.removed assert not result.success -def test_add_success(package_ahriman: Package) -> None: +def test_add_removed(package_ahriman: Package) -> None: + """ + must add package to removed list + """ + result = Result() + result.add_removed(package_ahriman) + assert not result.failed + assert result.removed == [package_ahriman] + assert not result.success + + +def test_add_updated(package_ahriman: Package) -> None: """ must add package to success list """ result = Result() - result.add_success(package_ahriman) - assert result.success == [package_ahriman] + result.add_updated(package_ahriman) assert not result.failed + assert not result.removed + assert result.success == [package_ahriman] def test_merge(package_ahriman: Package, package_python_schedule: Package) -> None: @@ -67,9 +88,9 @@ def test_merge(package_ahriman: Package, package_python_schedule: Package) -> No must merge success packages """ left = Result() - left.add_success(package_ahriman) + left.add_updated(package_ahriman) right = Result() - right.add_success(package_python_schedule) + right.add_updated(package_python_schedule) result = left.merge(right) assert result.success == [package_ahriman, package_python_schedule] @@ -81,7 +102,7 @@ def test_merge_failed(package_ahriman: Package) -> None: must merge and remove failed packages from success list """ left = Result() - left.add_success(package_ahriman) + left.add_updated(package_ahriman) right = Result() right.add_failed(package_ahriman) @@ -90,28 +111,15 @@ def test_merge_failed(package_ahriman: Package) -> None: assert not left.success -def test_merge_exception(package_ahriman: Package) -> None: - """ - must raise exception in case if package was failed - """ - left = Result() - left.add_failed(package_ahriman) - right = Result() - right.add_success(package_ahriman) - - with pytest.raises(UnprocessedPackageStatusError): - left.merge(right) - - def test_eq(package_ahriman: Package, package_python_schedule: Package) -> None: """ must return True for same objects """ left = Result() - left.add_success(package_ahriman) + left.add_updated(package_ahriman) left.add_failed(package_python_schedule) right = Result() - right.add_success(package_ahriman) + right.add_updated(package_ahriman) right.add_failed(package_python_schedule) assert left == right @@ -122,7 +130,7 @@ def test_eq_false(package_ahriman: Package) -> None: must return False in case if lists do not match """ left = Result() - left.add_success(package_ahriman) + left.add_updated(package_ahriman) right = Result() right.add_failed(package_ahriman) @@ -144,7 +152,7 @@ def test_eq_false_success(package_ahriman: Package) -> None: must return False in case if success does not match """ left = Result() - left.add_success(package_ahriman) + left.add_updated(package_ahriman) assert left != Result()