From a1f46611e1caee19af8fc84a452ed4484ada528a Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Sun, 2 Jan 2022 00:54:56 -0800 Subject: [PATCH] change(python): move request & pkgbase request routes Move package request routes and related routes to their respective routers. In addition, move some utility used for requests over from `aurweb.packages`. Introduced routers: - `aurweb.routers.requests` Introduced package: - `aurweb.requests` Introduced module: - `aurweb.requests.util` Changes: - Moved `aurweb.packages.validate` to `aurweb.pkgbase.validate` - Moved requests listing & request closure routes to `aurweb.routers.requests` - Moved pkgbase request creation route to `aurweb.routers.pkgbase` - Moved `get_pkgreq_by_id` from `aurweb.packages.util` to `aurweb.requests.util` and fixed its return type hint. Signed-off-by: Kevin Morris --- aurweb/asgi.py | 3 +- aurweb/packages/util.py | 8 - aurweb/{packages => pkgbase}/validate.py | 9 +- aurweb/requests/__init__.py | 0 aurweb/requests/util.py | 13 ++ aurweb/routers/packages.py | 177 +---------------------- aurweb/routers/pkgbase.py | 98 ++++++++++++- aurweb/routers/requests.py | 92 ++++++++++++ test/test_packages_util.py | 6 - test/test_requests.py | 7 + 10 files changed, 218 insertions(+), 195 deletions(-) rename aurweb/{packages => pkgbase}/validate.py (84%) create mode 100644 aurweb/requests/__init__.py create mode 100644 aurweb/requests/util.py create mode 100644 aurweb/routers/requests.py diff --git a/aurweb/asgi.py b/aurweb/asgi.py index 88e72052..15c2d40a 100644 --- a/aurweb/asgi.py +++ b/aurweb/asgi.py @@ -21,7 +21,7 @@ from aurweb.auth import BasicAuthBackend from aurweb.db import get_engine, query from aurweb.models import AcceptedTerm, Term from aurweb.prometheus import http_api_requests_total, http_requests_total, instrumentator -from aurweb.routers import accounts, auth, html, packages, pkgbase, rpc, rss, sso, trusted_user +from aurweb.routers import accounts, auth, html, packages, pkgbase, requests, rpc, rss, sso, trusted_user from aurweb.templates import make_context, render_template # Setup the FastAPI app. @@ -82,6 +82,7 @@ async def app_startup(): app.include_router(rss.router) app.include_router(packages.router) app.include_router(pkgbase.router) + app.include_router(requests.router) app.include_router(rpc.router) # Initialize the database engine and ORM. diff --git a/aurweb/packages/util.py b/aurweb/packages/util.py index 0224e738..4210f51e 100644 --- a/aurweb/packages/util.py +++ b/aurweb/packages/util.py @@ -115,14 +115,6 @@ def get_pkgbase_comment(pkgbase: models.PackageBase, id: int) \ return db.refresh(comment) -def get_pkgreq_by_id(id: int): - pkgreq = db.query(models.PackageRequest).filter( - models.PackageRequest.ID == id).first() - if not pkgreq: - raise HTTPException(status_code=HTTPStatus.NOT_FOUND) - return db.refresh(pkgreq) - - @register_filter("out_of_date") def out_of_date(packages: orm.Query) -> orm.Query: return packages.filter(models.PackageBase.OutOfDateTS.isnot(None)) diff --git a/aurweb/packages/validate.py b/aurweb/pkgbase/validate.py similarity index 84% rename from aurweb/packages/validate.py rename to aurweb/pkgbase/validate.py index e730e98b..8d05a3d7 100644 --- a/aurweb/packages/validate.py +++ b/aurweb/pkgbase/validate.py @@ -1,10 +1,11 @@ from typing import Any, Dict -from aurweb import db, models +from aurweb import db from aurweb.exceptions import ValidationError +from aurweb.models import PackageBase -def request(pkgbase: models.PackageBase, +def request(pkgbase: PackageBase, type: str, comments: str, merge_into: str, context: Dict[str, Any]) -> None: if not comments: @@ -17,8 +18,8 @@ def request(pkgbase: models.PackageBase, raise ValidationError( ['The "Merge into" field must not be empty.']) - target = db.query(models.PackageBase).filter( - models.PackageBase.Name == merge_into + target = db.query(PackageBase).filter( + PackageBase.Name == merge_into ).first() if not target: # TODO: This error needs to be translated. diff --git a/aurweb/requests/__init__.py b/aurweb/requests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/aurweb/requests/util.py b/aurweb/requests/util.py new file mode 100644 index 00000000..97c3447e --- /dev/null +++ b/aurweb/requests/util.py @@ -0,0 +1,13 @@ +from http import HTTPStatus + +from fastapi import HTTPException + +from aurweb import db +from aurweb.models import PackageRequest + + +def get_pkgreq_by_id(id: int) -> PackageRequest: + pkgreq = db.query(PackageRequest).filter(PackageRequest.ID == id).first() + if not pkgreq: + raise HTTPException(status_code=HTTPStatus.NOT_FOUND) + return db.refresh(pkgreq) diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index dbb4dc63..1930a7a2 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -1,28 +1,21 @@ from collections import defaultdict -from datetime import datetime from http import HTTPStatus from typing import Any, Dict, List -from fastapi import APIRouter, Form, Query, Request, Response -from fastapi.responses import RedirectResponse -from sqlalchemy import case +from fastapi import APIRouter, Form, Request, Response -import aurweb.filters -import aurweb.packages.util +import aurweb.filters # noqa: F401 from aurweb import config, db, defaults, logging, models, util from aurweb.auth import auth_required, creds -from aurweb.exceptions import InvariantError, ValidationError -from aurweb.models.package_request import ACCEPTED_ID, PENDING_ID, REJECTED_ID +from aurweb.exceptions import InvariantError from aurweb.models.relation_type import CONFLICTS_ID, PROVIDES_ID, REPLACES_ID from aurweb.packages import util as pkgutil -from aurweb.packages import validate from aurweb.packages.search import PackageSearch -from aurweb.packages.util import get_pkg_or_base, get_pkgreq_by_id +from aurweb.packages.util import get_pkg_or_base from aurweb.pkgbase import actions as pkgbase_actions from aurweb.pkgbase import util as pkgbaseutil -from aurweb.scripts import notify -from aurweb.templates import make_context, make_variable_context, render_template +from aurweb.templates import make_context, render_template logger = logging.get_logger(__name__) router = APIRouter() @@ -181,166 +174,6 @@ async def package(request: Request, name: str) -> Response: return render_template(request, "packages/show.html", context) -@router.get("/requests") -@auth_required() -async def requests(request: Request, - O: int = Query(default=defaults.O), - PP: int = Query(default=defaults.PP)): - context = make_context(request, "Requests") - - context["q"] = dict(request.query_params) - - O, PP = util.sanitize_params(O, PP) - context["O"] = O - context["PP"] = PP - - # A PackageRequest query, with left inner joined User and RequestType. - query = db.query(models.PackageRequest).join( - models.User, models.PackageRequest.UsersID == models.User.ID - ).join(models.RequestType) - - # If the request user is not elevated (TU or Dev), then - # filter PackageRequests which are owned by the request user. - if not request.user.is_elevated(): - query = query.filter(models.PackageRequest.UsersID == request.user.ID) - - context["total"] = query.count() - context["results"] = query.order_by( - # Order primarily by the Status column being PENDING_ID, - # and secondarily by RequestTS; both in descending order. - case([(models.PackageRequest.Status == PENDING_ID, 1)], else_=0).desc(), - models.PackageRequest.RequestTS.desc() - ).limit(PP).offset(O).all() - - return render_template(request, "requests.html", context) - - -@router.get("/pkgbase/{name}/request") -@auth_required() -async def package_request(request: Request, name: str): - pkgbase = get_pkg_or_base(name, models.PackageBase) - context = await make_variable_context(request, "Submit Request") - context["pkgbase"] = pkgbase - return render_template(request, "pkgbase/request.html", context) - - -@router.post("/pkgbase/{name}/request") -@auth_required() -async def pkgbase_request_post(request: Request, name: str, - type: str = Form(...), - merge_into: str = Form(default=None), - comments: str = Form(default=str())): - pkgbase = get_pkg_or_base(name, models.PackageBase) - - # Create our render context. - context = await make_variable_context(request, "Submit Request") - context["pkgbase"] = pkgbase - if type not in {"deletion", "merge", "orphan"}: - # In the case that someone crafted a POST request with an invalid - # type, just return them to the request form with BAD_REQUEST status. - return render_template(request, "pkgbase/request.html", context, - status_code=HTTPStatus.BAD_REQUEST) - - try: - validate.request(pkgbase, type, comments, merge_into, context) - except ValidationError as exc: - logger.error(f"Request Validation Error: {str(exc.data)}") - context["errors"] = exc.data - return render_template(request, "pkgbase/request.html", context) - - # All good. Create a new PackageRequest based on the given type. - now = int(datetime.utcnow().timestamp()) - reqtype = db.query(models.RequestType).filter( - models.RequestType.Name == type).first() - with db.begin(): - pkgreq = db.create(models.PackageRequest, - RequestType=reqtype, - User=request.user, - RequestTS=now, - PackageBase=pkgbase, - PackageBaseName=pkgbase.Name, - MergeBaseName=merge_into, - Comments=comments, - ClosureComment=str()) - - # Prepare notification object. - notif = notify.RequestOpenNotification( - request.user.ID, pkgreq.ID, reqtype.Name, - pkgreq.PackageBase.ID, merge_into=merge_into or None) - - # Send the notification now that we're out of the DB scope. - notif.send() - - auto_orphan_age = aurweb.config.getint("options", "auto_orphan_age") - auto_delete_age = aurweb.config.getint("options", "auto_delete_age") - - ood_ts = pkgbase.OutOfDateTS or 0 - flagged = ood_ts and (now - ood_ts) >= auto_orphan_age - is_maintainer = pkgbase.Maintainer == request.user - outdated = (now - pkgbase.SubmittedTS) <= auto_delete_age - - if type == "orphan" and flagged: - # This request should be auto-accepted. - with db.begin(): - pkgbase.Maintainer = None - pkgreq.Status = ACCEPTED_ID - notif = notify.RequestCloseNotification( - request.user.ID, pkgreq.ID, pkgreq.status_display()) - notif.send() - logger.debug(f"New request #{pkgreq.ID} is marked for auto-orphan.") - elif type == "deletion" and is_maintainer and outdated: - # This request should be auto-accepted. - notifs = pkgbase_actions.pkgbase_delete_instance( - request, pkgbase, comments=comments) - util.apply_all(notifs, lambda n: n.send()) - logger.debug(f"New request #{pkgreq.ID} is marked for auto-deletion.") - - # Redirect the submitting user to /packages. - return RedirectResponse("/packages", status_code=HTTPStatus.SEE_OTHER) - - -@router.get("/requests/{id}/close") -@auth_required() -async def requests_close(request: Request, id: int): - pkgreq = get_pkgreq_by_id(id) - if not request.user.is_elevated() and request.user != pkgreq.User: - # Request user doesn't have permission here: redirect to '/'. - return RedirectResponse("/", status_code=HTTPStatus.SEE_OTHER) - - context = make_context(request, "Close Request") - context["pkgreq"] = pkgreq - return render_template(request, "requests/close.html", context) - - -@router.post("/requests/{id}/close") -@auth_required() -async def requests_close_post(request: Request, id: int, - comments: str = Form(default=str())): - pkgreq = get_pkgreq_by_id(id) - - # `pkgreq`.User can close their own request. - approved = [pkgreq.User] - if not request.user.has_credential(creds.PKGREQ_CLOSE, approved=approved): - # Request user doesn't have permission here: redirect to '/'. - return RedirectResponse("/", status_code=HTTPStatus.SEE_OTHER) - - context = make_context(request, "Close Request") - context["pkgreq"] = pkgreq - - now = int(datetime.utcnow().timestamp()) - with db.begin(): - pkgreq.Closer = request.user - pkgreq.ClosureComment = comments - pkgreq.ClosedTS = now - pkgreq.Status = REJECTED_ID - - notify_ = notify.RequestCloseNotification( - request.user.ID, pkgreq.ID, pkgreq.status_display()) - notify_.send() - - return RedirectResponse("/requests", status_code=HTTPStatus.SEE_OTHER) - - async def packages_unflag(request: Request, package_ids: List[int] = [], **kwargs): if not package_ids: diff --git a/aurweb/routers/pkgbase.py b/aurweb/routers/pkgbase.py index 464d1eea..e9fdd337 100644 --- a/aurweb/routers/pkgbase.py +++ b/aurweb/routers/pkgbase.py @@ -5,21 +5,22 @@ from fastapi import APIRouter, Form, HTTPException, Query, Request, Response from fastapi.responses import JSONResponse, RedirectResponse from sqlalchemy import and_ -from aurweb import db, l10n, logging, templates, util +from aurweb import config, db, l10n, logging, templates, util from aurweb.auth import auth_required, creds -from aurweb.exceptions import InvariantError +from aurweb.exceptions import InvariantError, ValidationError from aurweb.models import PackageBase from aurweb.models.package_comment import PackageComment from aurweb.models.package_keyword import PackageKeyword from aurweb.models.package_notification import PackageNotification -from aurweb.models.package_request import PENDING_ID, PackageRequest +from aurweb.models.package_request import ACCEPTED_ID, PENDING_ID, PackageRequest from aurweb.models.package_vote import PackageVote from aurweb.models.request_type import DELETION_ID, MERGE_ID, ORPHAN_ID from aurweb.packages.requests import update_closure_comment from aurweb.packages.util import get_pkg_or_base, get_pkgbase_comment from aurweb.pkgbase import actions from aurweb.pkgbase import util as pkgbaseutil -from aurweb.scripts import popupdate +from aurweb.pkgbase import validate +from aurweb.scripts import notify, popupdate from aurweb.scripts.rendercomment import update_comment_render_fastapi from aurweb.templates import make_variable_context, render_template @@ -641,6 +642,95 @@ async def pkgbase_comaintainers_post(request: Request, name: str, status_code=HTTPStatus.SEE_OTHER) +@router.get("/pkgbase/{name}/request") +@auth_required() +async def pkgbase_request(request: Request, name: str): + pkgbase = get_pkg_or_base(name, PackageBase) + context = await make_variable_context(request, "Submit Request") + context["pkgbase"] = pkgbase + return render_template(request, "pkgbase/request.html", context) + + +@router.post("/pkgbase/{name}/request") +@auth_required() +async def pkgbase_request_post(request: Request, name: str, + type: str = Form(...), + merge_into: str = Form(default=None), + comments: str = Form(default=str())): + pkgbase = get_pkg_or_base(name, PackageBase) + + # Create our render context. + context = await make_variable_context(request, "Submit Request") + context["pkgbase"] = pkgbase + + types = { + "deletion": DELETION_ID, + "merge": MERGE_ID, + "orphan": ORPHAN_ID + } + + if type not in types: + # In the case that someone crafted a POST request with an invalid + # type, just return them to the request form with BAD_REQUEST status. + return render_template(request, "pkgbase/request.html", context, + status_code=HTTPStatus.BAD_REQUEST) + + try: + validate.request(pkgbase, type, comments, merge_into, context) + except ValidationError as exc: + logger.error(f"Request Validation Error: {str(exc.data)}") + context["errors"] = exc.data + return render_template(request, "pkgbase/request.html", context) + + # All good. Create a new PackageRequest based on the given type. + now = int(datetime.utcnow().timestamp()) + with db.begin(): + pkgreq = db.create(PackageRequest, + ReqTypeID=types.get(type), + User=request.user, + RequestTS=now, + PackageBase=pkgbase, + PackageBaseName=pkgbase.Name, + MergeBaseName=merge_into, + Comments=comments, + ClosureComment=str()) + + # Prepare notification object. + notif = notify.RequestOpenNotification( + request.user.ID, pkgreq.ID, type, + pkgreq.PackageBase.ID, merge_into=merge_into or None) + + # Send the notification now that we're out of the DB scope. + notif.send() + + auto_orphan_age = config.getint("options", "auto_orphan_age") + auto_delete_age = config.getint("options", "auto_delete_age") + + ood_ts = pkgbase.OutOfDateTS or 0 + flagged = ood_ts and (now - ood_ts) >= auto_orphan_age + is_maintainer = pkgbase.Maintainer == request.user + outdated = (now - pkgbase.SubmittedTS) <= auto_delete_age + + if type == "orphan" and flagged: + # This request should be auto-accepted. + with db.begin(): + pkgbase.Maintainer = None + pkgreq.Status = ACCEPTED_ID + notif = notify.RequestCloseNotification( + request.user.ID, pkgreq.ID, pkgreq.status_display()) + notif.send() + logger.debug(f"New request #{pkgreq.ID} is marked for auto-orphan.") + elif type == "deletion" and is_maintainer and outdated: + # This request should be auto-accepted. + notifs = actions.pkgbase_delete_instance( + request, pkgbase, comments=comments) + util.apply_all(notifs, lambda n: n.send()) + logger.debug(f"New request #{pkgreq.ID} is marked for auto-deletion.") + + # Redirect the submitting user to /packages. + return RedirectResponse("/packages", status_code=HTTPStatus.SEE_OTHER) + + @router.get("/pkgbase/{name}/delete") @auth_required() async def pkgbase_delete_get(request: Request, name: str): diff --git a/aurweb/routers/requests.py b/aurweb/routers/requests.py new file mode 100644 index 00000000..2c18c66a --- /dev/null +++ b/aurweb/routers/requests.py @@ -0,0 +1,92 @@ +from datetime import datetime +from http import HTTPStatus + +from fastapi import APIRouter, Form, Query, Request +from fastapi.responses import RedirectResponse +from sqlalchemy import case + +from aurweb import db, defaults, util +from aurweb.auth import auth_required, creds +from aurweb.models import PackageRequest, User +from aurweb.models.package_request import PENDING_ID, REJECTED_ID +from aurweb.requests.util import get_pkgreq_by_id +from aurweb.scripts import notify +from aurweb.templates import make_context, render_template + +router = APIRouter() + + +@router.get("/requests") +@auth_required() +async def requests(request: Request, + O: int = Query(default=defaults.O), + PP: int = Query(default=defaults.PP)): + context = make_context(request, "Requests") + + context["q"] = dict(request.query_params) + + O, PP = util.sanitize_params(O, PP) + context["O"] = O + context["PP"] = PP + + # A PackageRequest query, with left inner joined User and RequestType. + query = db.query(PackageRequest).join( + User, User.ID == PackageRequest.UsersID) + + # If the request user is not elevated (TU or Dev), then + # filter PackageRequests which are owned by the request user. + if not request.user.is_elevated(): + query = query.filter(PackageRequest.UsersID == request.user.ID) + + context["total"] = query.count() + context["results"] = query.order_by( + # Order primarily by the Status column being PENDING_ID, + # and secondarily by RequestTS; both in descending order. + case([(PackageRequest.Status == PENDING_ID, 1)], else_=0).desc(), + PackageRequest.RequestTS.desc() + ).limit(PP).offset(O).all() + + return render_template(request, "requests.html", context) + + +@router.get("/requests/{id}/close") +@auth_required() +async def request_close(request: Request, id: int): + + pkgreq = get_pkgreq_by_id(id) + if not request.user.is_elevated() and request.user != pkgreq.User: + # Request user doesn't have permission here: redirect to '/'. + return RedirectResponse("/", status_code=HTTPStatus.SEE_OTHER) + + context = make_context(request, "Close Request") + context["pkgreq"] = pkgreq + return render_template(request, "requests/close.html", context) + + +@router.post("/requests/{id}/close") +@auth_required() +async def request_close_post(request: Request, id: int, + comments: str = Form(default=str())): + pkgreq = get_pkgreq_by_id(id) + + # `pkgreq`.User can close their own request. + approved = [pkgreq.User] + if not request.user.has_credential(creds.PKGREQ_CLOSE, approved=approved): + # Request user doesn't have permission here: redirect to '/'. + return RedirectResponse("/", status_code=HTTPStatus.SEE_OTHER) + + context = make_context(request, "Close Request") + context["pkgreq"] = pkgreq + + now = int(datetime.utcnow().timestamp()) + with db.begin(): + pkgreq.Closer = request.user + pkgreq.ClosureComment = comments + pkgreq.ClosedTS = now + pkgreq.Status = REJECTED_ID + + notify_ = notify.RequestCloseNotification( + request.user.ID, pkgreq.ID, pkgreq.status_display()) + notify_.send() + + return RedirectResponse("/requests", status_code=HTTPStatus.SEE_OTHER) diff --git a/test/test_packages_util.py b/test/test_packages_util.py index 3474f847..2c2ed5e9 100644 --- a/test/test_packages_util.py +++ b/test/test_packages_util.py @@ -2,7 +2,6 @@ from datetime import datetime import pytest -from fastapi import HTTPException from fastapi.testclient import TestClient from aurweb import asgi, config, db @@ -98,11 +97,6 @@ def test_query_notified(maintainer: User, package: Package): assert query_notified[package.PackageBase.ID] -def test_pkgreq_by_id_not_found(): - with pytest.raises(HTTPException): - util.get_pkgreq_by_id(0) - - def test_source_uri_file(package: Package): FILE = "test_file" diff --git a/test/test_requests.py b/test/test_requests.py index 8228e171..8ca8f401 100644 --- a/test/test_requests.py +++ b/test/test_requests.py @@ -6,6 +6,7 @@ from logging import DEBUG import pytest +from fastapi import HTTPException from fastapi.testclient import TestClient from aurweb import asgi, config, db @@ -15,6 +16,7 @@ from aurweb.models.package_notification import PackageNotification from aurweb.models.package_request import ACCEPTED_ID, PENDING_ID, REJECTED_ID from aurweb.models.request_type import DELETION_ID, MERGE_ID, ORPHAN_ID from aurweb.packages.requests import ClosureFactory +from aurweb.requests.util import get_pkgreq_by_id from aurweb.testing.email import Email from aurweb.testing.html import get_errors from aurweb.testing.requests import Request @@ -583,3 +585,8 @@ def test_closure_factory_invalid_reqtype_id(): automated.get_closure(666, None, None, None, ACCEPTED_ID) with pytest.raises(NotImplementedError, match=match): automated.get_closure(666, None, None, None, REJECTED_ID) + + +def test_pkgreq_by_id_not_found(): + with pytest.raises(HTTPException): + get_pkgreq_by_id(0)