arcanis/ahriman
1
0
Fork 0
mirror of https://github.com/arcan1s/ahriman.git synced 2025-10-31 13:53:41 +00:00
Code Issues Projects Releases Wiki Activity

implement atomic_move method, move files only with lock

Browse Source
This commit is contained in:
Evgenii Alekseev
2025-07-30 14:45:29 +03:00
parent 2983d7e61a
commit 1bcdce4e6e
10 changed files with 157 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
CONTRIBUTING.md
View File

@ -165,6 +165,11 @@ Again, the most checks can be performed by `tox` command, though some additional
# Blank line again and package imports
from ahriman.core.configuration import Configuration
# Multiline import example
from ahriman.core.database.operations import (
AuthOperations,
BuildOperations,
)
```
* One file should define only one class, exception is class satellites in case if file length remains less than 400 lines.

12
src/ahriman/core/database/sqlite.py
View File

@ -25,8 +25,16 @@ from typing import Self
from ahriman.core.configuration import Configuration
from ahriman.core.database.migrations import Migrations
from ahriman.core.database.operations import AuthOperations, BuildOperations, ChangesOperations, \
DependenciesOperations, EventOperations, LogsOperations, PackageOperations, PatchOperations
from ahriman.core.database.operations import (
AuthOperations,
BuildOperations,
ChangesOperations,
DependenciesOperations,
EventOperations,
LogsOperations,
PackageOperations,
PatchOperations,
)
from ahriman.models.repository_id import RepositoryId

10
src/ahriman/core/repository/executor.py
View File

@ -17,8 +17,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
import shutil # shutil.move is used here to ensure cross fs file movement
from collections.abc import Iterable
from pathlib import Path
from tempfile import TemporaryDirectory
@ -27,7 +25,7 @@ from ahriman.core.build_tools.package_archive import PackageArchive
from ahriman.core.build_tools.task import Task
from ahriman.core.repository.cleaner import Cleaner
from ahriman.core.repository.package_info import PackageInfo
from ahriman.core.utils import safe_filename
from ahriman.core.utils import atomic_move, safe_filename
from ahriman.models.changes import Changes
from ahriman.models.event import EventType
from ahriman.models.package import Package
@ -54,7 +52,7 @@ class Executor(PackageInfo, Cleaner):
return # suppress type checking, it never can be none actually
if (safe := safe_filename(description.filename)) != description.filename:
(self.paths.packages / description.filename).rename(self.paths.packages / safe)
atomic_move(self.paths.packages / description.filename, self.paths.packages / safe)
description.filename = safe
def _package_build(self, package: Package, path: Path, packager: str | None,
@ -81,7 +79,7 @@ class Executor(PackageInfo, Cleaner):
package.with_packages(built, self.pacman)
for src in built:
dst = self.paths.packages / src.name
shutil.move(src, dst)
atomic_move(src, dst)
return commit_sha
@ -130,7 +128,7 @@ class Executor(PackageInfo, Cleaner):
for src in files:
dst = self.paths.archive_for(package_base) / src.name
src.rename(dst) # move package to archive directory
atomic_move(src, dst) # move package to archive directory
if not (symlink := self.paths.repository / dst.name).exists():
symlink.symlink_to(dst.relative_to(symlink.parent, walk_up=True)) # create link to archive

31
src/ahriman/core/utils.py
View File

@ -19,12 +19,14 @@
#
# pylint: disable=too-many-lines
import datetime
import fcntl
import io
import itertools
import logging
import os
import re
import selectors
import shutil
import subprocess
from collections.abc import Callable, Generator, Iterable, Mapping
@ -39,6 +41,7 @@ from ahriman.models.repository_paths import RepositoryPaths
__all__ = [
"atomic_move",
"check_output",
"check_user",
"dataclass_view",
@ -65,6 +68,34 @@ __all__ = [
T = TypeVar("T")
def atomic_move(src: Path, dst: Path) -> None:
"""
move file from ``source`` location to ``destination``. This method uses lock and :func:`shutil.move` to ensure that
file will be copied (if not rename) atomically. This method blocks execution until lock is available
Args:
src(Path): path to the source file
dst(Path): path to the destination
Examples:
This method is a drop-in replacement for :func:`shutil.move` (except it doesn't allow to override copy method)
which first locking destination file. To use it simply call method with arguments::
>>> atomic_move(src, dst)
"""
lock_path = dst.with_name(f".{dst.name}")
try:
with lock_path.open("ab") as lock_file:
fd = lock_file.fileno()
try:
fcntl.flock(fd, fcntl.LOCK_EX) # lock file and wait lock is until available
shutil.move(src, dst)
finally:
fcntl.flock(fd, fcntl.LOCK_UN) # unlock file first
finally:
lock_path.unlink(missing_ok=True) # remove lock file at the end
# pylint: disable=too-many-locals
def check_output(*args: str, exception: Exception | Callable[[int, list[str], str, str], Exception] | None = None,
cwd: Path | None = None, input_data: str | None = None,

14
src/ahriman/web/middlewares/exception_handler.py
View File

@ -21,8 +21,18 @@ import aiohttp_jinja2
import logging
from aiohttp.typedefs import Middleware
from aiohttp.web import HTTPClientError, HTTPException, HTTPMethodNotAllowed, HTTPNoContent, HTTPServerError, \
HTTPUnauthorized, Request, StreamResponse, json_response, middleware
from aiohttp.web import (
HTTPClientError,
HTTPException,
HTTPMethodNotAllowed,
HTTPNoContent,
HTTPServerError,
HTTPUnauthorized,
Request,
StreamResponse,
json_response,
middleware,
)
from ahriman.web.middlewares import HandlerType

8
src/ahriman/web/views/v1/packages/package.py
View File

@ -25,8 +25,12 @@ from ahriman.models.build_status import BuildStatusEnum
from ahriman.models.package import Package
from ahriman.models.user_access import UserAccess
from ahriman.web.apispec.decorators import apidocs
from ahriman.web.schemas import PackageNameSchema, PackageStatusSchema, PackageStatusSimplifiedSchema, \
RepositoryIdSchema
from ahriman.web.schemas import (
PackageNameSchema,
PackageStatusSchema,
PackageStatusSimplifiedSchema,
RepositoryIdSchema,
)
from ahriman.web.views.base import BaseView
from ahriman.web.views.status_view_guard import StatusViewGuard

5
src/ahriman/web/views/v1/service/upload.py
View File

@ -26,6 +26,7 @@ from tempfile import NamedTemporaryFile
from typing import ClassVar
from ahriman.core.configuration import Configuration
from ahriman.core.utils import atomic_move
from ahriman.models.repository_paths import RepositoryPaths
from ahriman.models.user_access import UserAccess
from ahriman.web.apispec.decorators import apidocs
@ -152,10 +153,8 @@ class UploadView(BaseView):
files.append(await self.save_file(part, target, max_body_size=max_body_size))
# and now we can rename files, which is relatively fast operation
# it is probably good way to call lock here, however
for filename, current_location in files:
target_location = current_location.parent / filename
current_location.rename(target_location)
atomic_move(current_location, target_location)
raise HTTPCreated

21
tests/ahriman/core/repository/test_executor.py
View File

@ -20,10 +20,10 @@ def test_archive_rename(executor: Executor, package_ahriman: Package, mocker: Mo
path = "gconf-3.2.6+11+g07808097-10-x86_64.pkg.tar.zst"
safe_path = "gconf-3.2.6-11-g07808097-10-x86_64.pkg.tar.zst"
package_ahriman.packages[package_ahriman.base].filename = path
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.core.repository.executor.atomic_move")
executor._archive_rename(package_ahriman.packages[package_ahriman.base], package_ahriman.base)
rename_mock.assert_called_once_with(executor.paths.packages / safe_path)
rename_mock.assert_called_once_with(executor.paths.packages / path, executor.paths.packages / safe_path)
assert package_ahriman.packages[package_ahriman.base].filename == safe_path
@ -32,7 +32,7 @@ def test_archive_rename_empty_filename(executor: Executor, package_ahriman: Pack
must skip renaming if filename is not set
"""
package_ahriman.packages[package_ahriman.base].filename = None
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.core.repository.executor.atomic_move")
executor._archive_rename(package_ahriman.packages[package_ahriman.base], package_ahriman.base)
rename_mock.assert_not_called()
@ -46,13 +46,13 @@ def test_package_build(executor: Executor, package_ahriman: Package, mocker: Moc
status_client_mock = mocker.patch("ahriman.core.status.Client.set_building")
init_mock = mocker.patch("ahriman.core.build_tools.task.Task.init", return_value="sha")
with_packages_mock = mocker.patch("ahriman.models.package.Package.with_packages")
move_mock = mocker.patch("shutil.move")
rename_mock = mocker.patch("ahriman.core.repository.executor.atomic_move")
assert executor._package_build(package_ahriman, Path("local"), "packager", None) == "sha"
status_client_mock.assert_called_once_with(package_ahriman.base)
init_mock.assert_called_once_with(pytest.helpers.anyvar(int), pytest.helpers.anyvar(int), None)
with_packages_mock.assert_called_once_with([Path(package_ahriman.base)], executor.pacman)
move_mock.assert_called_once_with(Path(package_ahriman.base), executor.paths.packages / package_ahriman.base)
rename_mock.assert_called_once_with(Path(package_ahriman.base), executor.paths.packages / package_ahriman.base)
def test_package_remove(executor: Executor, package_ahriman: Package, mocker: MockerFixture) -> None:
@ -94,7 +94,7 @@ def test_package_update(executor: Executor, package_ahriman: Package, user: User
"""
must update built package in repository
"""
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.core.repository.executor.atomic_move")
symlink_mock = mocker.patch("pathlib.Path.symlink_to")
repo_add_mock = mocker.patch("ahriman.core.alpm.repo.Repo.add")
sign_package_mock = mocker.patch("ahriman.core.sign.gpg.GPG.process_sign_package", side_effect=lambda fn, _: [fn])
@ -102,7 +102,8 @@ def test_package_update(executor: Executor, package_ahriman: Package, user: User
executor._package_update(filepath, package_ahriman.base, user.key)
# must move files (once)
rename_mock.assert_called_once_with(executor.paths.archive_for(package_ahriman.base) / filepath)
rename_mock.assert_called_once_with(
executor.paths.packages / filepath, executor.paths.archive_for(package_ahriman.base) / filepath)
# must sign package
sign_package_mock.assert_called_once_with(executor.paths.packages / filepath, user.key)
# symlink to the archive
@ -122,7 +123,7 @@ def test_package_update_empty_filename(executor: Executor, package_ahriman: Pack
"""
must skip update for package which does not have path
"""
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.core.repository.executor.atomic_move")
executor._package_update(None, package_ahriman.base, None)
rename_mock.assert_not_called()
@ -155,7 +156,7 @@ def test_process_build_bump_pkgrel(executor: Executor, package_ahriman: Package,
"""
mocker.patch("ahriman.core.repository.executor.Executor.packages", return_value=[package_ahriman])
mocker.patch("ahriman.core.build_tools.task.Task.build", return_value=[Path(package_ahriman.base)])
mocker.patch("shutil.move")
mocker.patch("ahriman.core.repository.executor.atomic_move")
init_mock = mocker.patch("ahriman.core.build_tools.task.Task.init")
executor.process_build([package_ahriman], Packagers("packager"), bump_pkgrel=True)
@ -172,7 +173,7 @@ def test_process_build_failure(executor: Executor, package_ahriman: Package, moc
mocker.patch("ahriman.core.repository.executor.Executor.packages_built")
mocker.patch("ahriman.core.build_tools.task.Task.build", return_value=[Path(package_ahriman.base)])
mocker.patch("ahriman.core.build_tools.task.Task.init")
mocker.patch("shutil.move", side_effect=Exception)
mocker.patch("ahriman.core.repository.executor.atomic_move", side_effect=Exception)
status_client_mock = mocker.patch("ahriman.core.status.Client.set_failed")
executor.process_build([package_ahriman])

74
tests/ahriman/core/test_utils.py
View File

@ -1,4 +1,5 @@
import datetime
import fcntl
import logging
import os
import pytest
@ -9,15 +10,82 @@ from typing import Any
from unittest.mock import call as MockCall
from ahriman.core.exceptions import BuildError, CalledProcessError, OptionError, UnsafeRunError
from ahriman.core.utils import check_output, check_user, dataclass_view, enum_values, extract_user, filter_json, \
full_version, minmax, package_like, parse_version, partition, pretty_datetime, pretty_interval, pretty_size, \
safe_filename, srcinfo_property, srcinfo_property_list, trim_package, utcnow, walk
from ahriman.core.utils import (
atomic_move,
check_output,
check_user,
dataclass_view,
enum_values,
extract_user,
filter_json,
full_version,
minmax,
package_like,
parse_version,
partition,
pretty_datetime,
pretty_interval,
pretty_size,
safe_filename,
srcinfo_property,
srcinfo_property_list,
trim_package,
utcnow,
walk,
)
from ahriman.models.package import Package
from ahriman.models.package_source import PackageSource
from ahriman.models.repository_id import RepositoryId
from ahriman.models.repository_paths import RepositoryPaths
def test_atomic_move(mocker: MockerFixture) -> None:
"""
must move file with locking
"""
lock_mock = mocker.patch("fcntl.flock")
open_mock = mocker.patch("pathlib.Path.open", autospec=True)
move_mock = mocker.patch("shutil.move")
unlink_mock = mocker.patch("pathlib.Path.unlink")
atomic_move(Path("source"), Path("destination"))
open_mock.assert_called_once_with(Path(".destination"), "ab")
lock_mock.assert_has_calls([
MockCall(pytest.helpers.anyvar(int), fcntl.LOCK_EX),
MockCall(pytest.helpers.anyvar(int), fcntl.LOCK_UN),
])
move_mock.assert_called_once_with(Path("source"), Path("destination"))
unlink_mock.assert_called_once_with(missing_ok=True)
def test_atomic_move_remove_lock(mocker: MockerFixture) -> None:
"""
must remove lock file in case of exception
"""
mocker.patch("pathlib.Path.open", side_effect=Exception)
unlink_mock = mocker.patch("pathlib.Path.unlink")
with pytest.raises(Exception):
atomic_move(Path("source"), Path("destination"))
unlink_mock.assert_called_once_with(missing_ok=True)
def test_atomic_move_unlock(mocker: MockerFixture) -> None:
"""
must unlock file in case of exception
"""
mocker.patch("pathlib.Path.open")
mocker.patch("shutil.move", side_effect=Exception)
lock_mock = mocker.patch("fcntl.flock")
with pytest.raises(Exception):
atomic_move(Path("source"), Path("destination"))
lock_mock.assert_has_calls([
MockCall(pytest.helpers.anyvar(int), fcntl.LOCK_EX),
MockCall(pytest.helpers.anyvar(int), fcntl.LOCK_UN),
])
def test_check_output(mocker: MockerFixture) -> None:
"""
must run command and log result

10
tests/ahriman/web/views/v1/service/test_view_v1_service_upload.py
View File

@ -109,7 +109,7 @@ async def test_post(client: TestClient, repository_paths: RepositoryPaths, mocke
local = Path("local")
save_mock = pytest.helpers.patch_view(client.app, "save_file",
AsyncMock(return_value=("filename", local / ".filename")))
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.web.views.v1.service.upload.atomic_move")
# no content validation here because it has invalid schema
data = FormData()
@ -118,7 +118,7 @@ async def test_post(client: TestClient, repository_paths: RepositoryPaths, mocke
response = await client.post("/api/v1/service/upload", data=data)
assert response.ok
save_mock.assert_called_once_with(pytest.helpers.anyvar(int), repository_paths.packages, max_body_size=None)
rename_mock.assert_called_once_with(local / "filename")
rename_mock.assert_called_once_with(local / ".filename", local / "filename")
async def test_post_with_sig(client: TestClient, repository_paths: RepositoryPaths, mocker: MockerFixture) -> None:
@ -131,7 +131,7 @@ async def test_post_with_sig(client: TestClient, repository_paths: RepositoryPat
("filename", local / ".filename"),
("filename.sig", local / ".filename.sig"),
]))
rename_mock = mocker.patch("pathlib.Path.rename")
rename_mock = mocker.patch("ahriman.web.views.v1.service.upload.atomic_move")
# no content validation here because it has invalid schema
data = FormData()
@ -145,8 +145,8 @@ async def test_post_with_sig(client: TestClient, repository_paths: RepositoryPat
MockCall(pytest.helpers.anyvar(int), repository_paths.packages, max_body_size=None),
])
rename_mock.assert_has_calls([
MockCall(local / "filename"),
MockCall(local / "filename.sig"),
MockCall(local / ".filename", local / "filename"),
MockCall(local / ".filename.sig", local / "filename.sig"),
])