From 2df0eb3b44f585ac32295a2958a8d987653a827f Mon Sep 17 00:00:00 2001 From: Nate Rook Date: Tue, 10 Sep 2024 14:14:20 -0400 Subject: [PATCH] Use new rank view everywhere the old annotate_with_rank call was used. (#509) This should fix #507, as well as simplifying some code to no longer need rank_replays.py. --- project/thscoreboard/replays/models.py | 32 +----------- project/thscoreboard/replays/rank_replays.py | 20 ------- .../thscoreboard/replays/replays_to_json.py | 7 ++- project/thscoreboard/replays/test_models.py | 28 ++-------- .../thscoreboard/replays/test_rank_replays.py | 52 ------------------- .../replays/test_replays_to_json.py | 6 +-- project/thscoreboard/replays/views/index.py | 6 +-- .../thscoreboard/replays/views/replay_list.py | 2 +- project/thscoreboard/replays/views/user.py | 9 ++-- project/thscoreboard/users/views/rankings.py | 11 ++-- 10 files changed, 27 insertions(+), 146 deletions(-) delete mode 100644 project/thscoreboard/replays/rank_replays.py delete mode 100644 project/thscoreboard/replays/test_rank_replays.py diff --git a/project/thscoreboard/replays/models.py b/project/thscoreboard/replays/models.py index 4efe77e7..3552513c 100644 --- a/project/thscoreboard/replays/models.py +++ b/project/thscoreboard/replays/models.py @@ -9,8 +9,7 @@ from django.utils import formats from django.utils import timezone from django.utils.translation import pgettext_lazy -from django.db.models import Q, F, Window, QuerySet, When, Case, Value -from django.db.models.functions import RowNumber +from django.db.models import Q, QuerySet from replays import game_ids from replays import limits @@ -197,35 +196,6 @@ def filter_visible(self) -> "ReplayQuerySet": return self.filter(Q(user__is_active=True) | Q(imported_username__isnull=False)) - def annotate_with_rank(self) -> "ReplayQuerySet": - """Annotate each standard replay with a rank, starting from 1 descending, with - separate ranks for each difficulty and shot. Set rank to -1 for other replays. - """ - - return self.annotate( - rank=Case( - When( - Q(category=Category.STANDARD) & Q(replay_type=ReplayType.FULL_GAME), - then=Window( - expression=RowNumber(), - order_by=[ - F("score").desc(), - F("created"), - F("id"), - ], - partition_by=[ - F("shot_id"), - F("difficulty"), - F("shot__game_id"), - F("route"), - ], - ), - ), - default=Value(-1), - output_field=models.IntegerField(), - ) - ) - def ghosts_of(self, replay_hash: bytes) -> "ReplayQuerySet": """Matches ghosts of a given replay file. diff --git a/project/thscoreboard/replays/rank_replays.py b/project/thscoreboard/replays/rank_replays.py deleted file mode 100644 index 97691231..00000000 --- a/project/thscoreboard/replays/rank_replays.py +++ /dev/null @@ -1,20 +0,0 @@ -from replays import models -from replays.models import ReplayQuerySet - - -def add_global_rank_annotations(replays: ReplayQuerySet) -> ReplayQuerySet: - """Ranks replays in the replay query set against all ranked replays""" - top_3_replays = ( - models.Replay.objects.filter(category=models.Category.STANDARD) - .filter(replay_type=1) - .filter(is_listed=True) - .annotate_with_rank() - .filter(rank__lte=3) - .all() - ) - rank_dict = {replay.id: replay.rank for replay in top_3_replays} - - for replay in replays: - replay.rank = rank_dict.get(replay.id, -1) - - return replays diff --git a/project/thscoreboard/replays/replays_to_json.py b/project/thscoreboard/replays/replays_to_json.py index 9da30df4..7cd35f9c 100644 --- a/project/thscoreboard/replays/replays_to_json.py +++ b/project/thscoreboard/replays/replays_to_json.py @@ -20,7 +20,12 @@ def _get_shot_name(self, shot: models.Shot) -> str: def convert_replay_to_dict(self, replay: models.Replay) -> dict: shot = replay.shot game = self._get_game(shot) - score_prefix = _get_medal_emoji(replay.rank) if hasattr(replay, "rank") else "" + + rank = replay.GetRank() + if rank is None: + score_prefix = "" + else: + score_prefix = _get_medal_emoji(rank) json_dict = {} json_dict["Id"] = replay.id diff --git a/project/thscoreboard/replays/test_models.py b/project/thscoreboard/replays/test_models.py index e0d53252..e7c7a0fd 100644 --- a/project/thscoreboard/replays/test_models.py +++ b/project/thscoreboard/replays/test_models.py @@ -133,17 +133,11 @@ def testRanks(self): ) replays = ( - models.Replay.objects.order_by("-score") - .select_related("rank_view") - .annotate_with_rank() - .all() + models.Replay.objects.order_by("-score").select_related("rank_view").all() ) - self.assertEquals(replays[0].rank, 1) self.assertEquals(replays[0].GetRank(), 1) - self.assertEquals(replays[1].rank, 2) self.assertEquals(replays[1].GetRank(), 2) - self.assertEquals(replays[2].rank, 1) self.assertEquals(replays[2].GetRank(), 1) def testRanksTasReplay(self): @@ -154,9 +148,8 @@ def testRanksTasReplay(self): category=models.Category.TAS, ) - replay = models.Replay.objects.order_by("-score").annotate_with_rank().first() + replay = models.Replay.objects.order_by("-score").first() - self.assertEquals(replay.rank, -1) self.assertIsNone(replay.GetRank()) def testRanksBreakTiesUsingUploadDate(self): @@ -190,17 +183,11 @@ def testRanksBreakTiesUsingUploadDate(self): ) replays = ( - models.Replay.objects.select_related("rank_view") - .order_by("created") - .annotate_with_rank() - .all() + models.Replay.objects.select_related("rank_view").order_by("created").all() ) - self.assertEquals(replays[0].rank, 1) self.assertEquals(replays[0].GetRank(), 1) - self.assertEquals(replays[1].rank, 2) self.assertEquals(replays[1].GetRank(), 2) - self.assertEquals(replays[2].rank, 3) self.assertEquals(replays[2].GetRank(), 3) def testStagePracticeReplaysAreUnranked(self) -> None: @@ -210,12 +197,7 @@ def testStagePracticeReplaysAreUnranked(self) -> None: replay_type=models.ReplayType.STAGE_PRACTICE, ) - replay = ( - models.Replay.objects.select_related("rank_view") - .annotate_with_rank() - .first() - ) - self.assertEquals(replay.rank, -1) + replay = models.Replay.objects.select_related("rank_view").first() self.assertIsNone(replay.GetRank()) def testRankCountsTasSeparately(self): @@ -253,8 +235,6 @@ def testRankCountsTasSeparately(self): .all() ) - # Note that .annotate_with_rank() performs incorrectly with this query, - # which is why we're moving to the join-with-view implementation instead. self.assertEqual(len(replays), 3) (returned_standard_1, returned_standard_2, returned_tas) = replays self.assertEqual(returned_standard_1.category, models.Category.STANDARD) diff --git a/project/thscoreboard/replays/test_rank_replays.py b/project/thscoreboard/replays/test_rank_replays.py deleted file mode 100644 index ca91d218..00000000 --- a/project/thscoreboard/replays/test_rank_replays.py +++ /dev/null @@ -1,52 +0,0 @@ -from unittest.mock import patch - -from replays.rank_replays import add_global_rank_annotations -from replays.testing import test_case -from replays import models -from replays.testing import test_replays - - -class GlobalRankAnnotationsTestCase(test_case.ReplayTestCase): - def setUp(self): - super().setUp() - self.user = self.createUser("some-user") - - def _create_n_replays(self, n: int, is_tas: bool = False) -> None: - with patch("replays.constant_helpers.CalculateReplayFileHash") as mocked_hash: - for i in range(n): - mocked_hash.return_value = bytes(i) - test_replays.CreateAsPublishedReplay( - "th10_normal", - user=self.user, - score=1_000_000_000 - 100_000_000 * i, - difficulty=0, - category=models.Category.TAS - if is_tas - else models.Category.STANDARD, - ) - - def testRanksAgainstAllReplays(self): - self._create_n_replays(3) - replay_subset = models.Replay.objects.order_by("-score")[1:] - add_global_rank_annotations(replay_subset) - - self.assertEqual(replay_subset[0].rank, 2) - self.assertEqual(replay_subset[1].rank, 3) - - def testGivesNegativeRankToReplaysOutsideTopThree(self): - self._create_n_replays(5) - replay_subset = models.Replay.objects.order_by("-score")[3:] - add_global_rank_annotations(replay_subset) - - self.assertEqual(replay_subset[0].rank, -1) - self.assertEqual(replay_subset[1].rank, -1) - - def testIgnoresUnrankedGlobalReplays(self): - self._create_n_replays(3, is_tas=True) - test_replays.CreateAsPublishedReplay( - "th10_normal", user=self.user, score=0, difficulty=0 - ) - replay_subset = models.Replay.objects.order_by("-score")[3:] - add_global_rank_annotations(replay_subset) - - self.assertEqual(replay_subset[0].rank, 1) diff --git a/project/thscoreboard/replays/test_replays_to_json.py b/project/thscoreboard/replays/test_replays_to_json.py index da390f46..b39e07b2 100644 --- a/project/thscoreboard/replays/test_replays_to_json.py +++ b/project/thscoreboard/replays/test_replays_to_json.py @@ -62,7 +62,7 @@ def testConvertReplaysToJsonString(self): imported_username="あ", ) - replays = models.Replay.objects.order_by("-score").annotate_with_rank() + replays = models.Replay.objects.order_by("-score") with test_utilities.OverrideTranslations(): converter = ReplayToJsonConverter() @@ -106,7 +106,7 @@ def testMedalEmojisSingleShot(self): difficulty=0, ) - replays = models.Replay.objects.order_by("-score").annotate_with_rank() + replays = models.Replay.objects.order_by("-score") with test_utilities.OverrideTranslations(): converter = ReplayToJsonConverter() json_data = [converter.convert_replay_to_dict(replay) for replay in replays] @@ -130,7 +130,7 @@ def testMedalEmojisMultipleShots(self): difficulty=0, ) - replays = models.Replay.objects.order_by("-score").annotate_with_rank() + replays = models.Replay.objects.order_by("-score") with test_utilities.OverrideTranslations(): converter = ReplayToJsonConverter() diff --git a/project/thscoreboard/replays/views/index.py b/project/thscoreboard/replays/views/index.py index 47689218..b1fd9a1c 100644 --- a/project/thscoreboard/replays/views/index.py +++ b/project/thscoreboard/replays/views/index.py @@ -11,12 +11,10 @@ @http_decorators.require_safe def index_json(request): recent_replays = ( - models.Replay.objects.filter( - category__in=[models.Category.STANDARD, models.Category.TAS] - ) + models.Replay.objects.select_related("rank_view") + .filter(category__in=[models.Category.STANDARD, models.Category.TAS]) .filter(is_listed=True) .filter_visible() - .annotate_with_rank() .order_by("-created")[:50] ) replay_jsons = convert_replays_to_json_bytes(recent_replays) diff --git a/project/thscoreboard/replays/views/replay_list.py b/project/thscoreboard/replays/views/replay_list.py index e9e67443..6fbd1e01 100644 --- a/project/thscoreboard/replays/views/replay_list.py +++ b/project/thscoreboard/replays/views/replay_list.py @@ -147,13 +147,13 @@ def _get_all_replay_for_game(game_id: str) -> Manager[models.Replay]: return ( models.Replay.objects.prefetch_related("shot") .prefetch_related("route") + .select_related("rank_view") .filter(category=models.Category.STANDARD) .filter(shot__game=game_id) .filter(replay_type=1) .filter(is_listed=True) .filter_visible() .order_by("-score") - .annotate_with_rank() ) diff --git a/project/thscoreboard/replays/views/user.py b/project/thscoreboard/replays/views/user.py index 34d7c51d..517e52a9 100644 --- a/project/thscoreboard/replays/views/user.py +++ b/project/thscoreboard/replays/views/user.py @@ -1,11 +1,9 @@ """The public page for a user's information.""" - from django.contrib import auth from django.shortcuts import get_object_or_404, render from django.views.decorators import http as http_decorators -from replays.rank_replays import add_global_rank_annotations from replays import models from replays.replays_to_json import convert_replays_to_json_bytes from replays.views.replay_table_helpers import stream_json_bytes_to_http_reponse @@ -14,10 +12,11 @@ @http_decorators.require_safe def user_page_json(request, username: str): user = get_object_or_404(auth.get_user_model(), username=username, is_active=True) - user_replays = models.Replay.objects.filter(user=user).order_by( - "shot__game_id", "shot_id", "created" + user_replays = ( + models.Replay.objects.select_related("rank_view") + .filter(user=user) + .order_by("shot__game_id", "shot_id", "created") ) - add_global_rank_annotations(user_replays) replay_jsons = convert_replays_to_json_bytes(user_replays) return stream_json_bytes_to_http_reponse(replay_jsons) diff --git a/project/thscoreboard/users/views/rankings.py b/project/thscoreboard/users/views/rankings.py index 92dc60b3..d0d60a2f 100644 --- a/project/thscoreboard/users/views/rankings.py +++ b/project/thscoreboard/users/views/rankings.py @@ -52,19 +52,20 @@ def _get_all_player_rankings_for_games( rankings = defaultdict(lambda: RankCount(0, 0, 0)) game_ids = [game.game_id for game in games] top_3_replays = ( - replay_models.Replay.objects.filter(category=replay_models.Category.STANDARD) + replay_models.Replay.objects.select_related("rank_view") + .filter(category=replay_models.Category.STANDARD) .filter(shot__game__in=game_ids) .filter(replay_type=1) .filter(is_listed=True) - .annotate_with_rank() - .filter(rank__lte=3) + .filter(rank_view__place__lte=3) ) for replay in top_3_replays: player = replay.user if replay.user is not None else replay.imported_username - if replay.rank == 1: + rank = replay.GetRank() + if rank == 1: rankings[player].first_place_count += 1 - elif replay.rank == 2: + elif rank == 2: rankings[player].second_place_count += 1 else: rankings[player].third_place_count += 1