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 <kevr@0cost.org>

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.

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))

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.

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)

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:

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):

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)

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"
 

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)