diff --git a/src/ahriman/web/views/base.py b/src/ahriman/web/views/base.py index 1ca1e440..de4b6cab 100644 --- a/src/ahriman/web/views/base.py +++ b/src/ahriman/web/views/base.py @@ -18,7 +18,7 @@ # along with this program. If not, see . # from aiohttp_cors import CorsViewMixin # type: ignore[import] -from aiohttp.web import Request, StreamResponse, View +from aiohttp.web import HTTPBadRequest, Request, StreamResponse, View from collections.abc import Awaitable, Callable from typing import Any, TypeVar @@ -184,6 +184,30 @@ class BaseView(View, CorsViewMixin): self._raise_allowed_methods() + def page(self) -> tuple[int, int]: + """ + parse limit and offset and return values + + Returns: + tuple[int, int]: limit and offset from request + + Raises: + HTTPBadRequest: if supplied parameters are invalid + """ + try: + limit = int(self.request.query.getone("limit", default=-1)) + offset = int(self.request.query.getone("offset", default=0)) + except Exception as ex: + raise HTTPBadRequest(reason=str(ex)) + + # some checks + if limit < -1: + raise HTTPBadRequest(reason=f"Limit must be -1 or non-negative, got {limit}") + if offset < 0: + raise HTTPBadRequest(reason=f"Offset must be non-negative, got {offset}") + + return limit, offset + async def username(self) -> str | None: """ extract username from request if any diff --git a/src/ahriman/web/views/v1/status/packages.py b/src/ahriman/web/views/v1/status/packages.py index dc26a7de..604352d0 100644 --- a/src/ahriman/web/views/v1/status/packages.py +++ b/src/ahriman/web/views/v1/status/packages.py @@ -18,11 +18,15 @@ # along with this program. If not, see . # import aiohttp_apispec # type: ignore[import] +import itertools +from collections.abc import Callable from aiohttp.web import HTTPNoContent, Response, json_response +from ahriman.models.build_status import BuildStatus +from ahriman.models.package import Package from ahriman.models.user_access import UserAccess -from ahriman.web.schemas import AuthSchema, ErrorSchema, PackageStatusSchema +from ahriman.web.schemas import AuthSchema, ErrorSchema, PackageStatusSchema, PaginationSchema from ahriman.web.views.base import BaseView @@ -41,9 +45,10 @@ class PackagesView(BaseView): @aiohttp_apispec.docs( tags=["Packages"], summary="Get packages list", - description="Retrieve all packages and their descriptors", + description="Retrieve packages and their descriptors", responses={ 200: {"description": "Success response", "schema": PackageStatusSchema(many=True)}, + 400: {"description": "Bad data is supplied", "schema": ErrorSchema}, 401: {"description": "Authorization required", "schema": ErrorSchema}, 403: {"description": "Access is forbidden", "schema": ErrorSchema}, 500: {"description": "Internal server error", "schema": ErrorSchema}, @@ -51,6 +56,7 @@ class PackagesView(BaseView): security=[{"token": [GET_PERMISSION]}], ) @aiohttp_apispec.cookies_schema(AuthSchema) + @aiohttp_apispec.querystring_schema(PaginationSchema) async def get(self) -> Response: """ get current packages status @@ -58,12 +64,17 @@ class PackagesView(BaseView): Returns: Response: 200 with package description on success """ + limit, offset = self.page() + stop = offset + limit if limit >= 0 else None + + comparator: Callable[[tuple[Package, BuildStatus]], str] = lambda pair: pair[0].base response = [ { "package": package.view(), "status": status.view() - } for package, status in self.service.packages + } for package, status in itertools.islice(sorted(self.service.packages, key=comparator), offset, stop) ] + return json_response(response) @aiohttp_apispec.docs( diff --git a/src/ahriman/web/views/v2/status/logs.py b/src/ahriman/web/views/v2/status/logs.py index 505f56d8..45ed66f5 100644 --- a/src/ahriman/web/views/v2/status/logs.py +++ b/src/ahriman/web/views/v2/status/logs.py @@ -19,7 +19,7 @@ # import aiohttp_apispec # type: ignore[import] -from aiohttp.web import HTTPBadRequest, HTTPNotFound, Response, json_response +from aiohttp.web import HTTPNotFound, Response, json_response from ahriman.core.exceptions import UnknownPackageError from ahriman.models.user_access import UserAccess @@ -63,15 +63,10 @@ class LogsView(BaseView): Response: 200 with package logs on success Raises: - HTTPBadRequest: if supplied parameters are invalid HTTPNotFound: if package base is unknown """ package_base = self.request.match_info["package"] - try: - limit = int(self.request.query.getone("limit", default=-1)) - offset = int(self.request.query.getone("offset", default=0)) - except Exception as ex: - raise HTTPBadRequest(reason=str(ex)) + limit, offset = self.page() try: _, status = self.service.package_get(package_base) diff --git a/tests/ahriman/web/conftest.py b/tests/ahriman/web/conftest.py index 2480a451..05b522ee 100644 --- a/tests/ahriman/web/conftest.py +++ b/tests/ahriman/web/conftest.py @@ -21,7 +21,7 @@ from ahriman.web.web import setup_service @pytest.helpers.register -def request(application: Application, path: str, method: str, json: Any = None, data: Any = None, +def request(application: Application, path: str, method: str, params: Any = None, json: Any = None, data: Any = None, extra: dict[str, Any] | None = None, resource: Resource | None = None) -> MagicMock: """ request generator helper @@ -30,6 +30,7 @@ def request(application: Application, path: str, method: str, json: Any = None, application(Application): application fixture path(str): path for the request method(str): method for the request + params(Any, optional): query parameters (Default value = None) json(Any, optional): json payload of the request (Default value = None) data(Any, optional): form data payload of the request (Default value = None) extra(dict[str, Any] | None, optional): extra info which will be injected for ``get_extra_info`` command @@ -42,6 +43,7 @@ def request(application: Application, path: str, method: str, json: Any = None, request_mock.app = application request_mock.path = path request_mock.method = method + request_mock.query = params request_mock.json = json request_mock.post = data diff --git a/tests/ahriman/web/views/test_view_base.py b/tests/ahriman/web/views/test_view_base.py index b670e346..4433dc18 100644 --- a/tests/ahriman/web/views/test_view_base.py +++ b/tests/ahriman/web/views/test_view_base.py @@ -2,6 +2,7 @@ import pytest from multidict import MultiDict from aiohttp.test_utils import TestClient +from aiohttp.web import HTTPBadRequest from pytest_mock import MockerFixture from unittest.mock import AsyncMock @@ -150,6 +151,41 @@ async def test_head_not_allowed(client: TestClient) -> None: assert response.status == 405 +def test_page(base: BaseView) -> None: + """ + must extract page from query parameters + """ + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(limit=2, offset=3)) + assert base.page() == (2, 3) + + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(offset=3)) + assert base.page() == (-1, 3) + + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(limit=2)) + assert base.page() == (2, 0) + + +def test_page_bad_request(base: BaseView) -> None: + """ + must raise HTTPBadRequest in case if parameters are invalid + """ + with pytest.raises(HTTPBadRequest): + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(limit="string")) + base.page() + + with pytest.raises(HTTPBadRequest): + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(offset="string")) + base.page() + + with pytest.raises(HTTPBadRequest): + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(limit=-2)) + base.page() + + with pytest.raises(HTTPBadRequest): + base._request = pytest.helpers.request(base.request.app, "", "", params=MultiDict(offset=-1)) + base.page() + + async def test_username(base: BaseView, mocker: MockerFixture) -> None: """ must return identity of logged-in user diff --git a/tests/ahriman/web/views/v1/status/test_view_v1_status_packages.py b/tests/ahriman/web/views/v1/status/test_view_v1_status_packages.py index a3d4b982..fabde948 100644 --- a/tests/ahriman/web/views/v1/status/test_view_v1_status_packages.py +++ b/tests/ahriman/web/views/v1/status/test_view_v1_status_packages.py @@ -32,7 +32,7 @@ async def test_get(client: TestClient, package_ahriman: Package, package_python_ response_schema = pytest.helpers.schema_response(PackagesView.get) response = await client.get("/api/v1/packages") - assert response.ok + assert response.status == 200 json = await response.json() assert not response_schema.validate(json, many=True) @@ -41,6 +41,30 @@ async def test_get(client: TestClient, package_ahriman: Package, package_python_ assert {package.base for package in packages} == {package_ahriman.base, package_python_schedule.base} +async def test_get_with_pagination(client: TestClient, package_ahriman: Package, + package_python_schedule: Package) -> None: + """ + must return paginated status for packages + """ + await client.post(f"/api/v1/packages/{package_ahriman.base}", + json={"status": BuildStatusEnum.Success.value, "package": package_ahriman.view()}) + await client.post(f"/api/v1/packages/{package_python_schedule.base}", + json={"status": BuildStatusEnum.Success.value, "package": package_python_schedule.view()}) + request_schema = pytest.helpers.schema_request(PackagesView.get, location="querystring") + response_schema = pytest.helpers.schema_response(PackagesView.get) + + payload = {"limit": 1, "offset": 1} + assert not request_schema.validate(payload) + response = await client.get("/api/v1/packages", params=payload) + assert response.status == 200 + json = await response.json() + assert not response_schema.validate(json, many=True) + + packages = [Package.from_json(item["package"]) for item in json] + assert packages + assert {package.base for package in packages} == {package_python_schedule.base} + + async def test_post(client: TestClient, mocker: MockerFixture) -> None: """ must be able to reload packages