From 4de18d813453ee101a81d7a44727e4722966a061 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Sat, 18 Sep 2021 23:30:51 -0700 Subject: [PATCH] fix(FastAPI): voted/notified query efficiency Previously, we were running a single ORM query for every single package to check for its voted or notified states. Now, we perform a single ORM query for each of the set of voted or notified packages in relation with the request user. This improves performance drastically at the expense of some manual code additions and set-dependency; i.e. we add a bit more complexity and roundabout way of getting our data. Closes: https://gitlab.archlinux.org/archlinux/aurweb/-/issues/102 Signed-off-by: Kevin Morris --- aurweb/packages/util.py | 50 +++++++++++++++++++++++- aurweb/routers/html.py | 35 +++++++++++++++-- templates/dashboard.html | 17 ++++++-- templates/partials/packages/results.html | 8 ++-- test/test_packages_util.py | 27 +++++++++++++ 5 files changed, 126 insertions(+), 11 deletions(-) diff --git a/aurweb/packages/util.py b/aurweb/packages/util.py index 036e3441..18ac7a5a 100644 --- a/aurweb/packages/util.py +++ b/aurweb/packages/util.py @@ -1,5 +1,6 @@ +from collections import defaultdict from http import HTTPStatus -from typing import List +from typing import Dict, List import orjson @@ -11,8 +12,11 @@ from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider from aurweb.models.package import Package from aurweb.models.package_base import PackageBase from aurweb.models.package_dependency import PackageDependency +from aurweb.models.package_notification import PackageNotification from aurweb.models.package_relation import PackageRelation +from aurweb.models.package_vote import PackageVote from aurweb.models.relation_type import PROVIDES_ID, RelationType +from aurweb.models.user import User from aurweb.redis import redis_connection from aurweb.templates import register_filter @@ -164,3 +168,47 @@ def updated_packages(limit: int = 0, cache_ttl: int = 600) -> List[Package]: # Return the deserialized list of packages. return packages + + +def query_voted(query: List[Package], user: User) -> Dict[int, bool]: + """ Produce a dictionary of package base ID keys to boolean values, + which indicate whether or not the package base has a vote record + related to user. + + :param query: A collection of Package models + :param user: The user that is being notified or not + :return: Vote state dict (PackageBase.ID: int -> bool) + """ + output = defaultdict(bool) + query_set = {pkg.PackageBase.ID for pkg in query} + voted = db.query(PackageVote).join( + PackageBase, + PackageBase.ID.in_(query_set) + ).filter( + PackageVote.UsersID == user.ID + ) + for vote in voted: + output[vote.PackageBase.ID] = True + return output + + +def query_notified(query: List[Package], user: User) -> Dict[int, bool]: + """ Produce a dictionary of package base ID keys to boolean values, + which indicate whether or not the package base has a notification + record related to user. + + :param query: A collection of Package models + :param user: The user that is being notified or not + :return: Notification state dict (PackageBase.ID: int -> bool) + """ + output = defaultdict(bool) + query_set = {pkg.PackageBase.ID for pkg in query} + notified = db.query(PackageNotification).join( + PackageBase, + PackageBase.ID.in_(query_set) + ).filter( + PackageNotification.UserID == user.ID + ) + for notify in notified: + output[notify.PackageBase.ID] = True + return output diff --git a/aurweb/routers/html.py b/aurweb/routers/html.py index c3fd3db1..3d44cf87 100644 --- a/aurweb/routers/html.py +++ b/aurweb/routers/html.py @@ -19,7 +19,7 @@ from aurweb.models.package_base import PackageBase from aurweb.models.package_comaintainer import PackageComaintainer from aurweb.models.package_request import PackageRequest from aurweb.models.user import User -from aurweb.packages.util import updated_packages +from aurweb.packages.util import query_notified, query_voted, updated_packages from aurweb.templates import make_context, render_template router = APIRouter() @@ -145,14 +145,25 @@ async def index(request: Request): PackageBase.MaintainerUID == request.user.ID ) + # Packages maintained by the user that have been flagged. context["flagged_packages"] = maintained.filter( PackageBase.OutOfDateTS.isnot(None) ).order_by( PackageBase.ModifiedTS.desc(), Package.Name.asc() ).limit(50).all() + # Flagged packages that request.user has voted for. + context["flagged_packages_voted"] = query_voted( + context.get("flagged_packages"), request.user) + + # Flagged packages that request.user is being notified about. + context["flagged_packages_notified"] = query_notified( + context.get("flagged_packages"), request.user) + archive_time = aurweb.config.getint('options', 'request_archive_time') start = now - archive_time + + # Package requests created by request.user. context["package_requests"] = request.user.package_requests.filter( PackageRequest.RequestTS >= start ).limit(50).all() @@ -162,13 +173,31 @@ async def index(request: Request): PackageBase.ModifiedTS.desc(), Package.Name.desc() ).limit(50).all() + # Packages that request.user has voted for. + context["packages_voted"] = query_voted( + context.get("packages"), request.user) + + # Packages that request.user is being notified about. + context["packages_notified"] = query_notified( + context.get("packages"), request.user) + # Any packages that the request user comaintains. context["comaintained"] = packages.join( - PackageComaintainer).filter( - PackageComaintainer.UsersID == request.user.ID).order_by( + PackageComaintainer + ).filter( + PackageComaintainer.UsersID == request.user.ID + ).order_by( PackageBase.ModifiedTS.desc(), Package.Name.desc() ).limit(50).all() + # Comaintained packages that request.user has voted for. + context["comaintained_voted"] = query_voted( + context.get("comaintained"), request.user) + + # Comaintained packages that request.user is being notified about. + context["comaintained_notified"] = query_notified( + context.get("comaintained"), request.user) + return render_template(request, "index.html", context) diff --git a/templates/dashboard.html b/templates/dashboard.html index 5ad89992..ec998187 100644 --- a/templates/dashboard.html +++ b/templates/dashboard.html @@ -5,7 +5,11 @@ {% if not flagged_packages %}

{{ "No packages matched your search criteria." | tr }}

{% else %} - {% with table_id = "flagged-packages", packages = flagged_packages %} + {% with table_id = "flagged-packages", + packages = flagged_packages, + votes = flagged_packages_voted, + notified = flagged_packages_notified + %} {% include 'partials/packages/results.html' %} {% endwith %} {% endif %} @@ -30,7 +34,10 @@ {% if not packages %}

{{ "No packages matched your search criteria." | tr }}

{% else %} - {% with table_id = "my-packages" %} + {% with table_id = "my-packages", + votes = packages_voted, + notified = packages_notified + %} {% include 'partials/packages/results.html' %} {% endwith %} {% endif %} @@ -46,7 +53,11 @@ {% if not comaintained %}

{{ "No packages matched your search criteria." | tr }}

{% else %} - {% with table_id = "comaintained-packages", packages = comaintained %} + {% with table_id = "comaintained-packages", + packages = comaintained, + votes = comaintained_voted, + notified = comaintained_notified + %} {% include 'partials/packages/results.html' %} {% endwith %} {% endif %} diff --git a/templates/partials/packages/results.html b/templates/partials/packages/results.html index 80029d10..a9a206a1 100644 --- a/templates/partials/packages/results.html +++ b/templates/partials/packages/results.html @@ -28,14 +28,14 @@ {{ pkg.PackageBase.Popularity | number_format(2) }} {% if request.user.is_authenticated() %} - - {% if request.user.voted_for(pkg) %} + {# If I voted, display "Yes". #} + {% if pkg.PackageBase.ID in votes %} {{ "Yes" | tr }} {% endif %} - - {% if request.user.notified(pkg) %} + {# If I'm being notified, display "Yes". #} + {% if pkg.PackageBase.ID in notified %} {{ "Yes" | tr }} {% endif %} diff --git a/test/test_packages_util.py b/test/test_packages_util.py index 754e3b8d..1396734b 100644 --- a/test/test_packages_util.py +++ b/test/test_packages_util.py @@ -1,3 +1,5 @@ +from datetime import datetime + import pytest from fastapi.testclient import TestClient @@ -7,6 +9,8 @@ from aurweb.models.account_type import USER_ID, AccountType from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider from aurweb.models.package import Package from aurweb.models.package_base import PackageBase +from aurweb.models.package_notification import PackageNotification +from aurweb.models.package_vote import PackageVote from aurweb.models.user import User from aurweb.packages import util from aurweb.redis import kill_redis @@ -19,6 +23,8 @@ def setup(): User.__tablename__, Package.__tablename__, PackageBase.__tablename__, + PackageVote.__tablename__, + PackageNotification.__tablename__, OfficialProvider.__tablename__ ) @@ -71,3 +77,24 @@ def test_updated_packages(maintainer: User, package: Package): assert util.updated_packages(1, 0) == [expected] assert util.updated_packages(1, 600) == [expected] kill_redis() # Kill it again, in case other tests use a real instance. + + +def test_query_voted(maintainer: User, package: Package): + now = int(datetime.utcnow().timestamp()) + with db.begin(): + db.create(PackageVote, User=maintainer, VoteTS=now, + PackageBase=package.PackageBase) + + query = db.query(Package).filter(Package.ID == package.ID).all() + query_voted = util.query_voted(query, maintainer) + assert query_voted[package.PackageBase.ID] + + +def test_query_notified(maintainer: User, package: Package): + with db.begin(): + db.create(PackageNotification, User=maintainer, + PackageBase=package.PackageBase) + + query = db.query(Package).filter(Package.ID == package.ID).all() + query_notified = util.query_notified(query, maintainer) + assert query_notified[package.PackageBase.ID]