diff --git a/CHANGELOG.md b/CHANGELOG.md index 445950ff..54b6ce3d 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/CTFd/api/v1/scoreboard.py b/CTFd/api/v1/scoreboard.py index 5d44952a..85c23a93 100644 --- a/CTFd/api/v1/scoreboard.py +++ b/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 diff --git a/CTFd/models/__init__.py b/CTFd/models/__init__.py index e2d47f88..b87c191f 100644 --- a/CTFd/models/__init__.py +++ b/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 diff --git a/CTFd/utils/scores/__init__.py b/CTFd/utils/scores/__init__.py index 12598f7c..f49dfaaa 100644 --- a/CTFd/utils/scores/__init__.py +++ b/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)