Improve response times of /api/v1/scoreboard significantly (#1470)

* Improve response times of `/api/v1/scoreboard` significantly by avoiding hitting the database to get every team member's score
* Fix issue where a hidden/banned user's score could be revealed as a member of a team

From tests I was able to cut this down from 11s to 0.5s. This endpoint also will still be heavily cached which should improve performance for a lot of users.

CHANGELOG.md

@@ -1,4 +1,4 @@
-2.5.0 / 2020-06-02
+2.5.0 / 2020-06-04
 ==================
 
 **General**
@@ -9,13 +9,15 @@
 * Add `/api/v1/challenges?view=admin` to allow admin users to see all challenges regardless of their visibility state
 * Add `/api/v1/users?view=admin` to allow admin users to see all users regardless of their hidden/banned state
 * Add `/api/v1/teams?view=admin` to allow admin users to see all teams regardless of their hidden/banned state
-* The scoreboard endpoints `/api/v1/scoreboard` & `/api/v1/scoreboard/top/[count]` should now be more performant because score and place for Users/Teams are now cached
+* The scoreboard endpoint `/api/v1/scoreboard` is now significantly more performant due to better response generation
+* The scoreboard endpoint `/api/v1/scoreboard` will no longer show hidden/banned users in a non-hidden team
 
 **Deployment**
 * `docker-compose` now provides a basic nginx configuration and deploys nginx on port 80
 
 **Miscellaneous**
 * The `get_config` and `get_page` config utilities now use SQLAlchemy Core instead of SQLAlchemy ORM for slight speedups
+* The `get_team_standings` and `get_user_standings` functions now return more data (id, oauth_id, name, score for regular users and banned, hidden as well for admins)
 * Update Flask-Migrate to 2.5.3 and regenerate the migration environment. Fixes using `%` signs in database passwords.
 
 

CTFd/api/v1/scoreboard.py

@@ -1,5 +1,7 @@
 from flask_restx import Namespace, Resource
 
+from sqlalchemy.orm import joinedload
+
 from CTFd.cache import cache, make_cache_key
 from CTFd.models import Awards, Solves, Teams
 from CTFd.utils import get_config
@@ -9,7 +11,7 @@ from CTFd.utils.decorators.visibility import (
     check_score_visibility,
 )
 from CTFd.utils.modes import TEAMS_MODE, generate_account_url, get_mode_as_word
-from CTFd.utils.scores import get_standings
+from CTFd.utils.scores import get_standings, get_user_standings
 
 scoreboard_namespace = Namespace(
     "scoreboard", description="Endpoint to retrieve scores"
@@ -31,9 +33,23 @@ class ScoreboardList(Resource):
             team_ids = []
             for team in standings:
                 team_ids.append(team.account_id)
-            teams = Teams.query.filter(Teams.id.in_(team_ids)).all()
+
+            # Get team objects with members explicitly loaded in
+            teams = (
+                Teams.query.options(joinedload(Teams.members))
+                .filter(Teams.id.in_(team_ids))
+                .all()
+            )
+
+            # Sort according to team_ids order
             teams = [next(t for t in teams if t.id == id) for id in team_ids]
 
+            # Get user_standings as a dict so that we can more quickly get member scores
+            user_standings = get_user_standings()
+            users = {}
+            for u in user_standings:
+                users[u.user_id] = u
+
         for i, x in enumerate(standings):
             entry = {
                 "pos": i + 1,
@@ -47,15 +63,30 @@ class ScoreboardList(Resource):
 
             if mode == TEAMS_MODE:
                 members = []
+
+                # This code looks like it would be slow
+                # but it is faster than accessing each member's score individually
                 for member in teams[i].members:
-                    members.append(
-                        {
-                            "id": member.id,
-                            "oauth_id": member.oauth_id,
-                            "name": member.name,
-                            "score": int(member.score),
-                        }
-                    )
+                    user = users.get(member.id)
+                    if user:
+                        members.append(
+                            {
+                                "id": user.user_id,
+                                "oauth_id": user.oauth_id,
+                                "name": user.name,
+                                "score": int(user.score),
+                            }
+                        )
+                    else:
+                        if member.hidden is False and member.banned is False:
+                            members.append(
+                                {
+                                    "id": member.id,
+                                    "oauth_id": member.oauth_id,
+                                    "name": member.name,
+                                    "score": 0,
+                                }
+                            )
 
                 entry["members"] = members
 

CTFd/models/__init__.py

@@ -368,12 +368,13 @@ class Users(db.Model):
 
         standings = get_user_standings(admin=admin)
 
-        try:
-            n = standings.index((self.id,)) + 1
-            if numeric:
-                return n
-            return ordinalize(n)
-        except ValueError:
+        for i, user in enumerate(standings):
+            if user.user_id == self.id:
+                n = i + 1
+                if numeric:
+                    return n
+                return ordinalize(n)
+        else:
             return None
 
 
@@ -394,7 +395,9 @@ class Teams(db.Model):
     password = db.Column(db.String(128))
     secret = db.Column(db.String(128))
 
-    members = db.relationship("Users", backref="team", foreign_keys="Users.team_id")
+    members = db.relationship(
+        "Users", backref="team", foreign_keys="Users.team_id", lazy="joined"
+    )
 
     # Supplementary attributes
     website = db.Column(db.String(128))
@@ -509,12 +512,13 @@ class Teams(db.Model):
 
         standings = get_team_standings(admin=admin)
 
-        try:
-            n = standings.index((self.id,)) + 1
-            if numeric:
-                return n
-            return ordinalize(n)
-        except ValueError:
+        for i, team in enumerate(standings):
+            if team.team_id == self.id:
+                n = i + 1
+                if numeric:
+                    return n
+                return ordinalize(n)
+        else:
             return None
 
 

CTFd/utils/scores/__init__.py

@@ -159,13 +159,25 @@ def get_team_standings(count=None, admin=False):
 
     if admin:
         standings_query = (
-            db.session.query(Teams.id.label("team_id"))
+            db.session.query(
+                Teams.id.label("team_id"),
+                Teams.oauth_id.label("oauth_id"),
+                Teams.name.label("name"),
+                Teams.hidden,
+                Teams.banned,
+                sumscores.columns.score,
+            )
             .join(sumscores, Teams.id == sumscores.columns.team_id)
             .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
         )
     else:
         standings_query = (
-            db.session.query(Teams.id.label("team_id"))
+            db.session.query(
+                Teams.id.label("team_id"),
+                Teams.oauth_id.label("oauth_id"),
+                Teams.name.label("name"),
+                sumscores.columns.score,
+            )
             .join(sumscores, Teams.id == sumscores.columns.team_id)
             .filter(Teams.banned == False)
             .filter(Teams.hidden == False)
@@ -225,13 +237,25 @@ def get_user_standings(count=None, admin=False):
 
     if admin:
         standings_query = (
-            db.session.query(Users.id.label("user_id"))
+            db.session.query(
+                Users.id.label("user_id"),
+                Users.oauth_id.label("oauth_id"),
+                Users.name.label("name"),
+                Users.hidden,
+                Users.banned,
+                sumscores.columns.score,
+            )
             .join(sumscores, Users.id == sumscores.columns.user_id)
             .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
         )
     else:
         standings_query = (
-            db.session.query(Users.id.label("user_id"))
+            db.session.query(
+                Users.id.label("user_id"),
+                Users.oauth_id.label("oauth_id"),
+                Users.name.label("name"),
+                sumscores.columns.score,
+            )
             .join(sumscores, Users.id == sumscores.columns.user_id)
             .filter(Users.banned == False, Users.hidden == False)
             .order_by(sumscores.columns.score.desc(), sumscores.columns.id)