diff --git a/pokemon_v2/api.py b/pokemon_v2/api.py index 75039128c..b04d577b1 100644 --- a/pokemon_v2/api.py +++ b/pokemon_v2/api.py @@ -1021,7 +1021,7 @@ def get(self, request: Request, pokemon_id: int) -> Response: "encounterconditionvaluemap_set__encounter_condition_value", "encounterpokemondetail_set", ) - .order_by("location_area_id", "version_id", "encounter_slot_id") + .order_by("location_area_id", "version_id", "encounter_slot_id", "pk") ) grouped_data: list[dict[str, Any]] = [] diff --git a/pokemon_v2/models.py b/pokemon_v2/models.py index 7998e1a7c..a6b34e672 100644 --- a/pokemon_v2/models.py +++ b/pokemon_v2/models.py @@ -1,10 +1,12 @@ # pyright: reportIncompatibleVariableOverride=false from __future__ import annotations -from typing import Any +from typing import Any, ClassVar, TypeVar from django.db import models -from typing_extensions import override +from typing_extensions import Self, override + +_ModelT = TypeVar("_ModelT", bound=models.Model) __all__: tuple[str, ...] = ( "Ability", @@ -165,6 +167,7 @@ "PalPark", "PalParkArea", "PalParkAreaName", + "PokeApiManager", "PokeApiModel", "PokeathlonStat", "PokeathlonStatName", @@ -230,7 +233,24 @@ ############################ +class PokeApiManager(models.Manager[_ModelT]): + """Default manager ordering every queryset by primary key, so that the API + serializes lists deterministically. Call sites needing another order + override it with ``order_by()``. + + A ``distinct(*fields)`` call has to spell out a matching ``order_by()``, + since PostgreSQL requires the leading ``ORDER BY`` expressions to match the + ``DISTINCT ON`` ones. A plain ``distinct()`` is unaffected. + """ + + @override + def get_queryset(self) -> models.QuerySet[_ModelT]: + return super().get_queryset().order_by("pk") + + class PokeApiModel(models.Model): + objects: ClassVar[PokeApiManager[Self]] = PokeApiManager() + class Meta: abstract = True diff --git a/pokemon_v2/serializers.py b/pokemon_v2/serializers.py index 430c1c385..fb85c66cb 100644 --- a/pokemon_v2/serializers.py +++ b/pokemon_v2/serializers.py @@ -1053,7 +1053,7 @@ def get_method_rates(self, obj: LocationAreaEncounterRate) -> ReturnList[ReturnD rates = ( LocationAreaEncounterRate.objects.filter(location_area=obj, encounter_method__isnull=False) .select_related("encounter_method", "version") - .order_by("encounter_method_id") + .order_by("encounter_method_id", "version_id", "pk") ) grouped_rates: list[dict[str, Any]] = [ { @@ -1081,7 +1081,7 @@ def get_encounters(self, obj: LocationArea) -> ReturnList[ReturnDict[str, Any]]: "encounterconditionvaluemap_set", "encounterpokemondetail_set", ) - .order_by("pokemon_id", "version_id") + .order_by("pokemon_id", "version_id", "encounter_slot_id", "pk") ) grouped_data: list[dict[str, Any]] = [] @@ -1597,7 +1597,7 @@ def get_held_by_pokemon(self, obj: Item) -> ReturnList[ReturnDict[str, Any]]: pokemon_items = ( PokemonItem.objects.filter(item=obj) .select_related("pokemon", "version") - .order_by("pokemon_id", "version_id") + .order_by("pokemon_id", "version_id", "pk") ) grouped_data: list[dict[str, Any]] = [ { @@ -1739,7 +1739,9 @@ class Meta: @extend_schema_field(BerryFlavorBerryMapSerializer(many=True)) def get_berries_with_flavor(self, obj: BerryFlavor) -> ReturnList[ReturnDict[str, Any]]: flavor_map_objects = ( - BerryFlavorMap.objects.filter(berry_flavor=obj, potency__gt=0).select_related("berry").order_by("potency") + BerryFlavorMap.objects.filter(berry_flavor=obj, potency__gt=0) + .select_related("berry") + .order_by("potency", "berry_id", "pk") ) return cast( "ReturnList[ReturnDict[str, Any]]", @@ -2629,7 +2631,7 @@ def get_pokemon_form_sprites(self, obj: PokemonForm) -> dict[str, Any]: @extend_schema_field(PokemonFormTypeSerializer(many=True)) def get_pokemon_form_types(self, obj: PokemonForm) -> ReturnList[ReturnDict[str, Any]]: - form_types = PokemonFormType.objects.filter(pokemon_form=obj).select_related("type").order_by("slot") + form_types = PokemonFormType.objects.filter(pokemon_form=obj).select_related("type").order_by("slot", "pk") if form_types: return cast( @@ -2638,7 +2640,7 @@ def get_pokemon_form_types(self, obj: PokemonForm) -> ReturnList[ReturnDict[str, ) # Fall back to parent Pokemon's types if no form-specific types exist - pokemon_types = PokemonType.objects.filter(pokemon=obj.pokemon).select_related("type").order_by("slot") + pokemon_types = PokemonType.objects.filter(pokemon=obj.pokemon).select_related("type").order_by("slot", "pk") return cast( "ReturnList[ReturnDict[str, Any]]", PokemonTypeSerializer(pokemon_types, many=True, context=self.context).data, @@ -2946,7 +2948,7 @@ def get_pokemon_moves(self, obj: Pokemon) -> list[dict[str, Any]]: pokemon_moves = ( PokemonMove.objects.filter(pokemon=obj, move__isnull=False) .select_related("move", "version_group", "move_learn_method") - .order_by("move__id", "version_group_id") + .order_by("move__id", "version_group_id", "move_learn_method_id", "level", "pk") ) vg_cache: dict[int, Any] = {} @@ -2990,7 +2992,7 @@ def get_pokemon_held_items(self, obj: Pokemon) -> list[dict[str, Any]]: pokemon_items = ( PokemonItem.objects.filter(pokemon=obj, item__isnull=False) .select_related("item", "version") - .order_by("item__id", "version_id") + .order_by("item__id", "version_id", "pk") ) version_cache: dict[int, Any] = {} @@ -3033,7 +3035,7 @@ def get_past_pokemon_abilities(self, obj: Pokemon) -> list[dict[str, Any]]: past_abilities = ( PokemonAbilityPast.objects.filter(pokemon=obj, generation__isnull=False) .select_related("generation", "ability") - .order_by("generation_id") + .order_by("generation_id", "slot", "pk") ) final_data: list[dict[str, Any]] = [] @@ -3061,7 +3063,7 @@ def get_past_pokemon_stats(self, obj: Pokemon) -> list[dict[str, Any]]: past_stats = ( PokemonStatPast.objects.filter(pokemon=obj, generation__isnull=False) .select_related("generation", "stat") - .order_by("generation_id") + .order_by("generation_id", "stat_id", "pk") ) final_data: list[dict[str, Any]] = [] @@ -3086,7 +3088,7 @@ def get_past_pokemon_stats(self, obj: Pokemon) -> list[dict[str, Any]]: @extend_schema_field(PokemonTypeSerializer(many=True)) def get_pokemon_types(self, obj: Pokemon) -> ReturnList[ReturnDict[str, Any]]: - types = PokemonType.objects.filter(pokemon=obj).select_related("type").order_by("slot") + types = PokemonType.objects.filter(pokemon=obj).select_related("type").order_by("slot", "pk") return cast( "ReturnList[ReturnDict[str, Any]]", PokemonTypeSerializer(types, many=True, context=self.context).data, @@ -3097,7 +3099,7 @@ def get_past_pokemon_types(self, obj: Pokemon) -> list[dict[str, Any]]: past_types = ( PokemonTypePast.objects.filter(pokemon=obj, generation__isnull=False) .select_related("generation", "type") - .order_by("generation_id", "slot") + .order_by("generation_id", "slot", "pk") ) final_data: list[dict[str, Any]] = [] @@ -3361,7 +3363,7 @@ class Meta: @extend_schema_field(EvolutionChainLinkSerializer) def build_chain(self, obj: EvolutionChain) -> dict[str, Any]: - pokemon_objects = PokemonSpecies.objects.filter(evolution_chain=obj).order_by("order") + pokemon_objects = PokemonSpecies.objects.filter(evolution_chain=obj).order_by("order", "pk") summary_data = cast( "ReturnList[ReturnDict[str, Any]]", PokemonSpeciesSummarySerializer(pokemon_objects, many=True, context=self.context).data, @@ -3535,7 +3537,9 @@ class Meta: @extend_schema_field(PokemonDexNumberSerializer(many=True)) def get_pokedex_entries(self, obj: Pokedex) -> ReturnList[ReturnDict[str, Any]]: entries = ( - PokemonDexNumber.objects.filter(pokedex=obj).select_related("pokemon_species").order_by("pokedex_number") + PokemonDexNumber.objects.filter(pokedex=obj) + .select_related("pokemon_species") + .order_by("pokedex_number", "pokemon_species_id", "pk") ) return cast( "ReturnList[ReturnDict[str, Any]]", diff --git a/pokemon_v2/tests.py b/pokemon_v2/tests.py index 9b525a341..80e4c4903 100644 --- a/pokemon_v2/tests.py +++ b/pokemon_v2/tests.py @@ -1309,6 +1309,7 @@ def setup_pokemon_species_data( is_legendary=False, is_mythical=False, order=1, + pk=None, ): generation = generation or cls.setup_generation_data(name="gen for " + name) @@ -1321,6 +1322,7 @@ def setup_pokemon_species_data( pokemon_habitat = pokemon_habitat or cls.setup_pokemon_habitat_data(name="pkm hbtt for " + name) pokemon_species = PokemonSpecies.objects.create( + pk=pk, name=name, generation=generation, evolves_from_species=evolves_from_species, @@ -1573,10 +1575,11 @@ def setup_pokemon_item_data(cls, pokemon=None, item=None, version=None, rarity=5 return pokemon_item @classmethod - def setup_pokemon_move_data(cls, pokemon, move, version_group, level=0, order=1): - move_learn_method = cls.setup_move_learn_method_data(name="mv lrn mthd for pkmn") + def setup_pokemon_move_data(cls, pokemon, move, version_group, level=0, order=1, move_learn_method=None, pk=None): + move_learn_method = move_learn_method or cls.setup_move_learn_method_data(name="mv lrn mthd for pkmn") pokemon_move = PokemonMove.objects.create( + pk=pk, pokemon=pokemon, version_group=version_group, move=move, @@ -2686,6 +2689,29 @@ def test_growth_rate_api(self): "{}{}/pokemon-species/{}/".format(TEST_HOST, API_V2, pokemon_species.pk), ) + def test_reverse_relation_lists_are_ordered_by_pk(self): + # Lists coming straight from a reverse relation have no order_by() of + # their own, so they fall back to the manager ordering by pk. The pks + # are inserted out of order, since an unordered query returns them in + # insertion order and would pass either way. + growth_rate = self.setup_growth_rate_data(name="grth rt for ordering") + species = [ + self.setup_pokemon_species_data( + pk=pk, + growth_rate=growth_rate, + name="pkmn spcs for ordering {}".format(pk), + ) + for pk in (30, 10, 20) + ] + + response = self.client.get("{}/growth-rate/{}/".format(API_V2, growth_rate.pk)) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + [entry["name"] for entry in response.data["pokemon_species"]], + [entry.name for entry in sorted(species, key=lambda entry: entry.pk)], + ) + # Location Tests def test_location_api(self): location = self.setup_location_data(name="base lctn") @@ -4591,6 +4617,38 @@ def test_pokemon_moves_with_non_contiguous_version_group_and_method_ids(self): "{}{}/move-learn-method/{}/".format(TEST_HOST, API_V2, pokemon_move.move_learn_method.pk), ) + def test_pokemon_moves_version_group_details_are_deterministically_ordered(self): + # A pokemon can learn the same move in the same version group more than + # once, so these rows tie on every other order_by() field and fall back + # to the pk. The pks are inserted out of order on purpose. + pokemon_species = self.setup_pokemon_species_data(name="pkmn spcs for mv ordering") + pokemon = self.setup_pokemon_data(pokemon_species=pokemon_species, name="pkmn for mv ordering") + self.setup_pokemon_sprites_data(pokemon=pokemon) + self.setup_pokemon_cries_data(pokemon, latest=True, legacy=True) + + move = self.setup_move_data(name="mv for mv ordering") + version_group = self.setup_version_group_data(name="ver grp for mv ordering") + move_learn_method = self.setup_move_learn_method_data(name="mv lrn mthd for mv ordering") + + for pk, order in ((30, 3), (10, 1), (20, 2)): + self.setup_pokemon_move_data( + pk=pk, + pokemon=pokemon, + move=move, + version_group=version_group, + move_learn_method=move_learn_method, + level=5, + order=order, + ) + + response = self.client.get("{}/pokemon/{}/".format(API_V2, pokemon.pk), headers={"host": "testserver"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + [detail["order"] for detail in response.data["moves"][0]["version_group_details"]], + [1, 2, 3], + ) + def test_pokemon_form_api(self): pokemon_species = self.setup_pokemon_species_data() pokemon = self.setup_pokemon_data(pokemon_species=pokemon_species)