Cache challenge data for faster loading of /api/v1/challenges (CTFd#2232)

* Improve response time of `/api/v1/challenges` and `/api/v1/challenges/[challenge_id]/solves`
* Rewrite and remove _build_solves_query to make it cacheable
* Closes CTFd#2209

Author: ColdHeat

CTFd/admin/__init__.py

@@ -26,7 +26,13 @@
 from CTFd.admin import submissions  # noqa: F401
 from CTFd.admin import teams  # noqa: F401
 from CTFd.admin import users  # noqa: F401
-from CTFd.cache import cache, clear_config, clear_pages, clear_standings
+from CTFd.cache import (
+    cache,
+    clear_challenges,
+    clear_config,
+    clear_pages,
+    clear_standings,
+)
 from CTFd.models import (
     Awards,
     Challenges,
@@ -238,6 +244,7 @@ def reset():
 
         clear_pages()
         clear_standings()
+        clear_challenges()
         clear_config()
 
         if logout is True:

CTFd/api/v1/challenges.py

@@ -1,15 +1,13 @@
-import datetime
 from typing import List
 
 from flask import abort, render_template, request, url_for
 from flask_restx import Namespace, Resource
-from sqlalchemy import func as sa_func
-from sqlalchemy.sql import and_, false, true
+from sqlalchemy.sql import and_
 
 from CTFd.api.v1.helpers.request import validate_args
 from CTFd.api.v1.helpers.schemas import sqlalchemy_to_pydantic
 from CTFd.api.v1.schemas import APIDetailedSuccessResponse, APIListSuccessResponse
-from CTFd.cache import clear_standings
+from CTFd.cache import clear_challenges, clear_standings
 from CTFd.constants import RawEnum
 from CTFd.models import ChallengeFiles as ChallengeFilesModel
 from CTFd.models import Challenges
@@ -22,12 +20,18 @@
 from CTFd.schemas.tags import TagSchema
 from CTFd.utils import config, get_config
 from CTFd.utils import user as current_user
+from CTFd.utils.challenges import (
+    get_all_challenges,
+    get_solve_counts_for_challenges,
+    get_solve_ids_for_user_id,
+    get_solves_for_challenge_id,
+)
 from CTFd.utils.config.visibility import (
     accounts_visible,
     challenges_visible,
     scores_visible,
 )
-from CTFd.utils.dates import ctf_ended, ctf_paused, ctftime, isoformat, unix_time_to_utc
+from CTFd.utils.dates import ctf_ended, ctf_paused, ctftime
 from CTFd.utils.decorators import (
     admins_only,
     during_ctf_time_only,
@@ -37,9 +41,7 @@
     check_challenge_visibility,
     check_score_visibility,
 )
-from CTFd.utils.helpers.models import build_model_filters
 from CTFd.utils.logging import log
-from CTFd.utils.modes import generate_account_url, get_model
 from CTFd.utils.security.signing import serialize
 from CTFd.utils.user import (
     authed,
@@ -77,60 +79,6 @@ class ChallengeListSuccessResponse(APIListSuccessResponse):
 )
 
 
-def _build_solves_query(extra_filters=(), admin_view=False):
-    """Returns queries and data that that are used for showing an account's solves.
-    It returns a tuple of
-        - SQLAlchemy query with (challenge_id, solve_count_for_challenge_id)
-        - Current user's solved challenge IDs
-    """
-    # This can return None (unauth) if visibility is set to public
-    user = get_current_user()
-    # We only set a condition for matching user solves if there is a user and
-    # they have an account ID (user mode or in a team in teams mode)
-    AccountModel = get_model()
-    if user is not None and user.account_id is not None:
-        user_solved_cond = Solves.account_id == user.account_id
-    else:
-        user_solved_cond = false()
-    # We have to filter solves to exclude any made after the current freeze
-    # time unless we're in an admin view as determined by the caller.
-    freeze = get_config("freeze")
-    if freeze and not admin_view:
-        freeze_cond = Solves.date < unix_time_to_utc(freeze)
-    else:
-        freeze_cond = true()
-    # Finally, we never count solves made by hidden or banned users/teams, even
-    # if we are an admin. This is to match the challenge detail API.
-    exclude_solves_cond = and_(
-        AccountModel.banned == false(), AccountModel.hidden == false(),
-    )
-    # This query counts the number of solves per challenge, as well as the sum
-    # of correct solves made by the current user per the condition above (which
-    # should probably only be 0 or 1!)
-    solves_q = (
-        db.session.query(Solves.challenge_id, sa_func.count(Solves.challenge_id),)
-        .join(AccountModel)
-        .filter(*extra_filters, freeze_cond, exclude_solves_cond)
-        .group_by(Solves.challenge_id)
-    )
-    # Also gather the user's solve items which can be different from above query
-    # For example, even if we are a hidden user, we should see that we have solved a challenge
-    # however as a hidden user we are not included in the count of the above query
-    if admin_view:
-        # If we're an admin we should show all challenges as solved to break through any requirements
-        challenges = Challenges.query.all()
-        solve_ids = {challenge.id for challenge in challenges}
-    else:
-        # If not an admin we calculate solves as normal
-        solve_ids = (
-            Solves.query.with_entities(Solves.challenge_id)
-            .filter(user_solved_cond)
-            .all()
-        )
-        solve_ids = {value for value, in solve_ids}
-    return solves_q, solve_ids
-
-
 @challenges_namespace.route("")
 class ChallengeList(Resource):
     @check_challenge_visibility
@@ -185,23 +133,22 @@ def get(self, query_args):
         # Build filtering queries
         q = query_args.pop("q", None)
         field = str(query_args.pop("field", None))
-        filters = build_model_filters(model=Challenges, query=q, field=field)
 
         # Admins get a shortcut to see all challenges despite pre-requisites
         admin_view = is_admin() and request.args.get("view") == "admin"
 
-        solve_counts = {}
-        # Build a query for to show challenge solve information. We only
-        # give an admin view if the request argument has been provided.
-        #
-        # NOTE: This is different behaviour to the challenge detail
-        # endpoint which only needs the current user to be an admin rather
-        # than also also having to provide `view=admin` as a query arg.
-        solves_q, user_solves = _build_solves_query(admin_view=admin_view)
+        # Get a cached mapping of challenge_id to solve_count
+        solve_counts = get_solve_counts_for_challenges(admin=admin_view)
+
+        # Get list of solve_ids for current user
+        if authed():
+            user = get_current_user()
+            user_solves = get_solve_ids_for_user_id(user_id=user.id)
+        else:
+            user_solves = set()
+
         # Aggregate the query results into the hashes defined at the top of
         # this block for later use
-        for chal_id, solve_count in solves_q:
-            solve_counts[chal_id] = solve_count
         if scores_visible() and accounts_visible():
             solve_count_dfl = 0
         else:
@@ -211,18 +158,7 @@ def get(self, query_args):
             # `None` for the solve count if visiblity checks fail
             solve_count_dfl = None
 
-        # Build the query for the challenges which may be listed
-        chal_q = Challenges.query
-        # Admins can see hidden and locked challenges in the admin view
-        if admin_view is False:
-            chal_q = chal_q.filter(
-                and_(Challenges.state != "hidden", Challenges.state != "locked")
-            )
-        chal_q = (
-            chal_q.filter_by(**query_args)
-            .filter(*filters)
-            .order_by(Challenges.value, Challenges.id)
-        )
+        chal_q = get_all_challenges(admin=admin_view, field=field, q=q, **query_args)
 
         # Iterate through the list of challenges, adding to the object which
         # will be JSONified back to the client
@@ -308,6 +244,9 @@ def post(self):
         challenge_class = get_chal_class(challenge_type)
         challenge = challenge_class.create(request)
         response = challenge_class.read(challenge)
+
+        clear_challenges()
+
         return {"success": True, "data": response}
 
 
@@ -453,13 +392,17 @@ def get(self, challenge_id):
 
         response = chal_class.read(challenge=chal)
 
-        solves_q, user_solves = _build_solves_query(
-            extra_filters=(Solves.challenge_id == chal.id,)
-        )
-        # If there are no solves for this challenge ID then we have 0 rows
-        maybe_row = solves_q.first()
-        if maybe_row:
-            challenge_id, solve_count = maybe_row
+        # Get list of solve_ids for current user
+        if authed():
+            user = get_current_user()
+            user_solves = get_solve_ids_for_user_id(user_id=user.id)
+        else:
+            user_solves = []
+
+        solves_count = get_solve_counts_for_challenges(challenge_id=chal.id)
+        if solves_count:
+            challenge_id = chal.id
+            solve_count = solves_count.get(chal.id)
             solved_by_user = challenge_id in user_solves
         else:
             solve_count, solved_by_user = 0, False
@@ -522,6 +465,10 @@ def patch(self, challenge_id):
         challenge_class = get_chal_class(challenge.type)
         challenge = challenge_class.update(challenge, request)
         response = challenge_class.read(challenge)
+
+        clear_standings()
+        clear_challenges()
+
         return {"success": True, "data": response}
 
     @admins_only
@@ -534,6 +481,9 @@ def delete(self, challenge_id):
         chal_class = get_chal_class(challenge.type)
         chal_class.delete(challenge)
 
+        clear_standings()
+        clear_challenges()
+
         return {"success": True}
 
 
@@ -675,6 +625,7 @@ def post(self):
                         user=user, team=team, challenge=challenge, request=request
                     )
                     clear_standings()
+                    clear_challenges()
 
                 log(
                     "submissions",
@@ -694,6 +645,7 @@ def post(self):
                         user=user, team=team, challenge=challenge, request=request
                     )
                     clear_standings()
+                    clear_challenges()
 
                 log(
                     "submissions",
@@ -762,41 +714,15 @@ def get(self, challenge_id):
         if challenge.state == "hidden" and is_admin() is False:
             abort(404)
 
-        Model = get_model()
-
-        # Note that we specifically query for the Solves.account.name
-        # attribute here because it is faster than having SQLAlchemy
-        # query for the attribute directly and it's unknown what the
-        # affects of changing the relationship lazy attribute would be
-        solves = (
-            Solves.query.add_columns(Model.name.label("account_name"))
-            .join(Model, Solves.account_id == Model.id)
-            .filter(
-                Solves.challenge_id == challenge_id,
-                Model.banned == False,
-                Model.hidden == False,
-            )
-            .order_by(Solves.date.asc())
-        )
-
         freeze = get_config("freeze")
         if freeze:
             preview = request.args.get("preview")
             if (is_admin() is False) or (is_admin() is True and preview):
-                dt = datetime.datetime.utcfromtimestamp(freeze)
-                solves = solves.filter(Solves.date < dt)
+                freeze = True
+            elif is_admin() is True:
+                freeze = False
 
-        for solve in solves:
-            # Seperate out the account name and the Solve object from the SQLAlchemy tuple
-            solve, account_name = solve
-            response.append(
-                {
-                    "account_id": solve.account_id,
-                    "name": account_name,
-                    "date": isoformat(solve.date),
-                    "account_url": generate_account_url(account_id=solve.account_id),
-                }
-            )
+        response = get_solves_for_challenge_id(challenge_id=challenge_id, freeze=freeze)
 
         return {"success": True, "data": response}
 

CTFd/api/v1/config.py

@@ -6,7 +6,7 @@
 from CTFd.api.v1.helpers.request import validate_args
 from CTFd.api.v1.helpers.schemas import sqlalchemy_to_pydantic
 from CTFd.api.v1.schemas import APIDetailedSuccessResponse, APIListSuccessResponse
-from CTFd.cache import clear_config, clear_standings
+from CTFd.cache import clear_challenges, clear_config, clear_standings
 from CTFd.constants import RawEnum
 from CTFd.models import Configs, Fields, db
 from CTFd.schemas.config import ConfigSchema
@@ -99,6 +99,7 @@ def post(self):
 
         clear_config()
         clear_standings()
+        clear_challenges()
 
         return {"success": True, "data": response.data}
 
@@ -119,6 +120,7 @@ def patch(self):
 
         clear_config()
         clear_standings()
+        clear_challenges()
 
         return {"success": True}
 
@@ -175,6 +177,7 @@ def patch(self, config_key):
 
         clear_config()
         clear_standings()
+        clear_challenges()
 
         return {"success": True, "data": response.data}
 
@@ -192,6 +195,7 @@ def delete(self, config_key):
 
         clear_config()
         clear_standings()
+        clear_challenges()
 
         return {"success": True}
 

CTFd/api/v1/submissions.py

@@ -8,7 +8,7 @@
     APIDetailedSuccessResponse,
     PaginatedAPIListSuccessResponse,
 )
-from CTFd.cache import clear_standings
+from CTFd.cache import clear_challenges, clear_standings
 from CTFd.constants import RawEnum
 from CTFd.models import Submissions, db
 from CTFd.schemas.submissions import SubmissionSchema
@@ -141,6 +141,8 @@ def post(self, json_args):
 
         # Delete standings cache
         clear_standings()
+        # Delete challenges cache
+        clear_challenges()
 
         return {"success": True, "data": response.data}
 
@@ -188,5 +190,6 @@ def delete(self, submission_id):
 
         # Delete standings cache
         clear_standings()
+        clear_challenges()
 
         return {"success": True}