diff --git a/src/ahriman/application/lock.py b/src/ahriman/application/lock.py index 7cae1b5f..526dbd56 100644 --- a/src/ahriman/application/lock.py +++ b/src/ahriman/application/lock.py @@ -114,30 +114,37 @@ class Lock(LazyLogging): return self._pid_file = self.path.open("a+") - def _watch(self) -> None: + def _watch(self) -> bool: """ watch until lock disappear + + Returns: + bool: True in case if file is locked and False otherwise """ # there are reasons why we are not using inotify here. First of all, if we would use it, it would bring to # race conditions because multiple processes will be notified at the same time. Secondly, it is good library, # but platform-specific, and we only need to check if file exists if self._pid_file is None: - return + return False waiter = Waiter(self.wait_timeout) - waiter.wait(lambda fd: not self.perform_lock(fd), self._pid_file.fileno()) + return bool(waiter.wait(lambda fd: not self.perform_lock(fd), self._pid_file.fileno())) - def _write(self) -> None: + def _write(self, *, is_locked: bool = False) -> None: """ write pid to the lock file + Args: + is_locked(bool, optional): indicates if file was already locked or not (Default value = False) + Raises: DuplicateRunError: if it cannot lock PID file """ if self._pid_file is None: return - if not self.perform_lock(self._pid_file.fileno()): - raise DuplicateRunError + if not is_locked: + if not self.perform_lock(self._pid_file.fileno()): + raise DuplicateRunError self._pid_file.seek(0) # reset position and remove file content if any self._pid_file.truncate() @@ -182,8 +189,8 @@ class Lock(LazyLogging): if self.force: # remove lock if force flag is set self.clear() self._open() - self._watch() - self._write() + is_locked = self._watch() + self._write(is_locked=is_locked) def __enter__(self) -> Self: """ diff --git a/src/ahriman/models/waiter.py b/src/ahriman/models/waiter.py index 7f804315..8a22c008 100644 --- a/src/ahriman/models/waiter.py +++ b/src/ahriman/models/waiter.py @@ -21,27 +21,87 @@ import time from collections.abc import Callable from dataclasses import dataclass, field -from typing import ParamSpec +from typing import Literal, ParamSpec Params = ParamSpec("Params") +@dataclass(frozen=True) +class WaiterResult: + """ + representation of a waiter result. This class should not be used directly, use derivatives instead + + Attributes: + took(float): consumed time in seconds + """ + + took: float + + def __bool__(self) -> bool: + """ + indicates whether the waiter completed with success or not + + Raises: + NotImplementedError: not implemented method + """ + raise NotImplementedError + + def __float__(self) -> float: + """ + extract time spent to retrieve the result in seconds + + Returns: + float: consumed time in seconds + """ + return self.took + + +class WaiterTaskFinished(WaiterResult): + """ + a waiter result used to notify that the task has been completed successfully + """ + + def __bool__(self) -> Literal[True]: + """ + indicates whether the waiter completed with success or not + + Returns: + Literal[True]: always False + """ + return True + + +class WaiterTimedOut(WaiterResult): + """ + a waiter result used to notify that the waiter run out of time + """ + + def __bool__(self) -> Literal[False]: + """ + indicates whether the waiter completed with success or not + + Returns: + Literal[False]: always False + """ + return False + + @dataclass(frozen=True) class Waiter: """ simple waiter implementation Attributes: - interval(int): interval in seconds between checks + interval(float): interval in seconds between checks start_time(float): monotonic time of the waiter start. More likely must not be assigned explicitly - wait_timeout(int): timeout in seconds to wait for. Negative value will result in immediate exit. Zero value + wait_timeout(float): timeout in seconds to wait for. Negative value will result in immediate exit. Zero value means infinite timeout """ - wait_timeout: int + wait_timeout: float start_time: float = field(default_factory=time.monotonic, kw_only=True) - interval: int = field(default=10, kw_only=True) + interval: float = field(default=10, kw_only=True) def is_timed_out(self) -> bool: """ @@ -51,10 +111,10 @@ class Waiter: bool: True in case current monotonic time is more than :attr:`start_time` and :attr:`wait_timeout` doesn't equal to 0 """ - since_start: float = time.monotonic() - self.start_time + since_start = time.monotonic() - self.start_time return self.wait_timeout != 0 and since_start > self.wait_timeout - def wait(self, in_progress: Callable[Params, bool], *args: Params.args, **kwargs: Params.kwargs) -> float: + def wait(self, in_progress: Callable[Params, bool], *args: Params.args, **kwargs: Params.kwargs) -> WaiterResult: """ wait until requirements are not met @@ -64,9 +124,12 @@ class Waiter: **kwargs(Params.kwargs): keyword arguments for check call Returns: - float: consumed time in seconds + WaiterResult: consumed time in seconds """ - while not self.is_timed_out() and in_progress(*args, **kwargs): + while not (timed_out := self.is_timed_out()) and in_progress(*args, **kwargs): time.sleep(self.interval) + took = time.monotonic() - self.start_time - return time.monotonic() - self.start_time + if timed_out: + return WaiterTimedOut(took) + return WaiterTaskFinished(took) diff --git a/tests/ahriman/application/test_lock.py b/tests/ahriman/application/test_lock.py index 428d8ffb..2db2c8cd 100644 --- a/tests/ahriman/application/test_lock.py +++ b/tests/ahriman/application/test_lock.py @@ -98,7 +98,7 @@ def test_write(lock: Lock) -> None: """ with NamedTemporaryFile("a+") as pid_file: lock._pid_file = pid_file - lock._write() + lock._write(is_locked=False) assert int(lock._pid_file.readline()) == os.getpid() @@ -107,7 +107,7 @@ def test_write_skip(lock: Lock) -> None: """ must skip write to file if no path set """ - lock._write() + lock._write(is_locked=False) def test_write_locked(lock: Lock, mocker: MockerFixture) -> None: @@ -117,7 +117,18 @@ def test_write_locked(lock: Lock, mocker: MockerFixture) -> None: mocker.patch("ahriman.application.lock.Lock.perform_lock", return_value=False) with pytest.raises(DuplicateRunError): lock._pid_file = MagicMock() - lock._write() + lock._write(is_locked=False) + + +def test_write_locked_before(lock: Lock, mocker: MockerFixture) -> None: + """ + must skip lock in case if file was locked before + """ + lock_mock = mocker.patch("ahriman.application.lock.Lock.perform_lock") + lock._pid_file = MagicMock() + + lock._write(is_locked=True) + lock_mock.assert_not_called() def test_check_user(lock: Lock, mocker: MockerFixture) -> None: @@ -226,14 +237,14 @@ def test_lock(lock: Lock, mocker: MockerFixture) -> None: """ clear_mock = mocker.patch("ahriman.application.lock.Lock.clear") open_mock = mocker.patch("ahriman.application.lock.Lock._open") - watch_mock = mocker.patch("ahriman.application.lock.Lock._watch") + watch_mock = mocker.patch("ahriman.application.lock.Lock._watch", return_value=True) write_mock = mocker.patch("ahriman.application.lock.Lock._write") lock.lock() clear_mock.assert_not_called() open_mock.assert_called_once_with() watch_mock.assert_called_once_with() - write_mock.assert_called_once_with() + write_mock.assert_called_once_with(is_locked=True) def test_lock_clear(lock: Lock, mocker: MockerFixture) -> None: diff --git a/tests/ahriman/models/test_waiter.py b/tests/ahriman/models/test_waiter.py index b98e757c..5af432a2 100644 --- a/tests/ahriman/models/test_waiter.py +++ b/tests/ahriman/models/test_waiter.py @@ -1,6 +1,36 @@ +import pytest import time -from ahriman.models.waiter import Waiter +from ahriman.models.waiter import Waiter, WaiterResult, WaiterTaskFinished, WaiterTimedOut + + +def test_result_to_float() -> None: + """ + must convert waiter result to float + """ + assert float(WaiterResult(4.2)) == 4.2 + + +def test_result_not_implemented() -> None: + """ + must raise NotImplementedError for abstract class + """ + with pytest.raises(NotImplementedError): + assert bool(WaiterResult(4.2)) + + +def test_result_success_to_bool() -> None: + """ + must convert success waiter result to bool + """ + assert bool(WaiterTaskFinished(4.2)) + + +def test_result_failure_to_bool() -> None: + """ + must convert failure waiter result to bool + """ + assert not bool(WaiterTimedOut(4.2)) def test_is_timed_out() -> None: @@ -22,8 +52,26 @@ def test_is_timed_out_infinite() -> None: def test_wait() -> None: """ - must wait until file will disappear + must wait for success result """ results = iter([True, False]) - waiter = Waiter(1, interval=1) - assert waiter.wait(lambda: next(results)) > 0 + waiter = Waiter(1, interval=0.1) + assert float(waiter.wait(lambda: next(results))) > 0 + + +def test_wait_timeout() -> None: + """ + must return WaiterTimedOut on timeout + """ + results = iter([True, False]) + waiter = Waiter(-1, interval=0.1) + assert isinstance(waiter.wait(lambda: next(results)), WaiterTimedOut) + + +def test_wait_success() -> None: + """ + must return WaiterTaskFinished on success + """ + results = iter([True, False]) + waiter = Waiter(1, interval=0.1) + assert isinstance(waiter.wait(lambda: next(results)), WaiterTaskFinished)