From eecb5fbc10b1d08360dff7c3064e7bf5ee5f4e94 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 11:23:46 +0100 Subject: [PATCH 01/11] Add cached_property methods to NationState to improve performance --- core/models/nation.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/core/models/nation.py b/core/models/nation.py index a4a0895..7d2f091 100644 --- a/core/models/nation.py +++ b/core/models/nation.py @@ -2,6 +2,7 @@ from django.contrib.auth.models import User from django.db import models +from django.utils.functional import cached_property from core.models.base import PerTurnModel, Phase, SurrenderStatus @@ -160,6 +161,14 @@ def supply_centers(self): territory__supply_center=True ) + @cached_property + def num_supply_centers(self): + return self.supply_centers.count() + + @cached_property + def num_pieces(self): + return self.pieces.count() + @property def unoccupied_controlled_home_supply_centers(self): """ @@ -187,7 +196,7 @@ def supply_delta(self): Returns: * `int` """ - return self.supply_centers.count() - self.pieces.count() + return self.num_supply_centers - self.num_pieces @property def num_builds(self): @@ -233,18 +242,22 @@ def pieces_to_order(self): raise Exception('Should not be called during build phase') return self.turn.piecestates.filter(piece__nation=self.nation) + @cached_property + def num_pieces_to_order(self): + return self.pieces_to_order.count() + @property def num_orders(self): if self.turn.phase == Phase.BUILD: return max(self.num_builds, self.num_disbands) - return self.pieces_to_order.count() + return self.num_pieces_to_order @property def num_orders_remaining(self): if self.turn.phase == Phase.BUILD: num_orders = max(self.num_builds, self.num_disbands) return max(0, num_orders - self.orders.count()) - return self.pieces_to_order.count() - self.orders.count() + return self.num_pieces_to_order - self.orders.count() def copy_to_new_turn(self, turn): """ From d96eb0e315f82a94ead83c8b63065dfea6a6e374 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 11:24:38 +0100 Subject: [PATCH 02/11] Add permission for user is nation state --- service/permissions.py | 6 ++++++ service/tests/test_permissions.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 service/tests/test_permissions.py diff --git a/service/permissions.py b/service/permissions.py index 5cdb363..993ff5f 100644 --- a/service/permissions.py +++ b/service/permissions.py @@ -22,3 +22,9 @@ def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return True return obj.created_by == request.user + + +class UserIsNationState(permissions.BasePermission): + + def has_object_permission(self, request, view, obj): + return request.user == obj.user diff --git a/service/tests/test_permissions.py b/service/tests/test_permissions.py new file mode 100644 index 0000000..d3b9976 --- /dev/null +++ b/service/tests/test_permissions.py @@ -0,0 +1,29 @@ +from django.test import RequestFactory, TestCase + +from service import permissions +from core import models +from core.tests import DiplomacyTestCaseMixin + + +class TestUserIsNationState(TestCase, DiplomacyTestCaseMixin): + + permission_class = permissions.UserIsNationState + + def setUp(self): + factory = RequestFactory() + self.request = factory.get('/') + self.user = self.create_test_user() + self.request.user = self.user + self.view = None + + def check_object_permission(self, obj): + permission = self.permission_class() + return permission.has_object_permission(self.request, self.view, obj) + + def test_nation_state_is_user(self): + nation_state = models.NationState(user=self.user) + self.assertTrue(self.check_object_permission(nation_state)) + + def test_nation_state_not_user(self): + nation_state = models.NationState() + self.assertFalse(self.check_object_permission(nation_state)) From a3ac1274f3494992d52b10689b97f8d80083763e Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 11:25:58 +0100 Subject: [PATCH 03/11] Add separate views/serializers to improve performance of GameDetailView --- service/serializers.py | 154 +++++++++++------------ service/tests/test_serializers.py | 39 ++++++ service/tests/test_views.py | 200 +++++++++++++++++++++++++++++- service/urls.py | 10 ++ service/views.py | 42 ++++--- 5 files changed, 347 insertions(+), 98 deletions(-) create mode 100644 service/tests/test_serializers.py diff --git a/service/serializers.py b/service/serializers.py index 287e866..32b699c 100644 --- a/service/serializers.py +++ b/service/serializers.py @@ -196,8 +196,6 @@ class Meta: class PublicNationStateSerializer(serializers.ModelSerializer): - orders_finalized = serializers.SerializerMethodField() - num_supply_centers = serializers.SerializerMethodField() surrenders = SurrenderSerializer(many=True, read_only=True) class Meta: @@ -206,35 +204,11 @@ class Meta: 'id', 'user', 'nation', - 'orders_finalized', - 'num_orders', - 'num_supply_centers', - 'supply_delta', - 'num_builds', - 'num_disbands', 'surrenders', ) - read_only_fields = ('nation', ) - - def get_num_supply_centers(self, nation_state): - return nation_state.supply_centers.count() - - # TODO make orders finalized a separate view - def get_orders_finalized(self, nation_state): - request = self.context.get('request') - if request: - user = request.user - if user.id == nation_state.user.id: - return nation_state.orders_finalized - return None - - def get_num_orders_remaining(self, nation_state): - request = self.context.get('request') - if request: - user = request.user - if user.id == nation_state.user.id: - return nation_state.num_orders_remaining - return None + read_only_fields = ( + 'nation', + ) def update(self, instance, validated_data): """ @@ -274,39 +248,6 @@ def update(self, nation_state, validated_data): return nation_state -class PrivateNationStateSerializer(serializers.ModelSerializer): - - build_territories = serializers.SerializerMethodField() - surrenders = SurrenderSerializer(many=True) - - class Meta: - model = models.NationState - fields = ( - 'id', - 'user', - 'nation', - 'orders_finalized', - 'num_orders_remaining', - 'supply_delta', - 'build_territories', - 'num_builds', - 'num_disbands', - 'surrenders', - ) - - def get_build_territories(self, nation_state): - """ - Get a list of territory ids for each territory in which the user can - build. - """ - if nation_state.turn.phase != Phase.BUILD: - return None - return [ - ts.territory.id - for ts in nation_state.unoccupied_controlled_home_supply_centers - ] - - class VariantSerializer(serializers.ModelSerializer): territories = TerritorySerializer(many=True) @@ -406,38 +347,63 @@ def validate(self, data): class DrawSerializer(serializers.ModelSerializer): - draw_responses = DrawResponseSerializer(many=True, - source='drawresponse_set') + draw_responses = DrawResponseSerializer( + many=True, + source='drawresponse_set', + ) class Meta: model = models.Draw - fields = ('id', 'turn', 'nations', 'proposed_by', 'proposed_by_user', - 'status', 'proposed_at', 'resolved_at', 'draw_responses') + fields = ( + 'id', + 'turn', + 'nations', + 'proposed_by', + 'proposed_by_user', + 'status', + 'proposed_at', + 'resolved_at', + 'draw_responses' + ) class TurnSerializer(serializers.ModelSerializer): - territory_states = TerritoryStateSerializer(many=True, - source='territorystates') - piece_states = PieceStateSerializer(many=True, source='piecestates') - nation_states = PublicNationStateSerializer(many=True, - source='nationstates') - orders = OrderSerializer(many=True, source='public_orders') - phase_display = serializers.CharField(source='get_phase_display') - season_display = serializers.CharField(source='get_season_display') - next_turn = serializers.SerializerMethodField() - previous_turn = serializers.SerializerMethodField() + territory_states = TerritoryStateSerializer( + many=True, + source='territorystates' + ) + piece_states = PieceStateSerializer( + many=True, + source='piecestates' + ) + nation_states = PublicNationStateSerializer( + many=True, + source='nationstates' + ) + orders = OrderSerializer( + many=True, + source='public_orders' + ) + phase_display = serializers.CharField( + source='get_phase_display' + ) + season_display = serializers.CharField( + source='get_season_display' + ) draws = DrawSerializer(many=True) turn_end = serializers.SerializerMethodField() + next_turn = serializers.SerializerMethodField() + previous_turn = serializers.SerializerMethodField() class Meta: model = models.Turn fields = ( 'id', 'game', + 'current_turn', 'next_turn', 'previous_turn', - 'current_turn', 'year', 'season', 'season_display', @@ -451,6 +417,11 @@ class Meta: 'turn_end', ) + def get_turn_end(self, turn): + if turn.turn_end: + return turn.turn_end.datetime + return None + def get_next_turn(self, obj): turn = models.Turn.get_next(obj) return getattr(turn, 'id', None) @@ -459,11 +430,6 @@ def get_previous_turn(self, obj): turn = models.Turn.get_previous(obj) return getattr(turn, 'id', None) - def get_turn_end(self, turn): - if turn.turn_end: - return turn.turn_end.datetime - return None - class ListNationStatesSerializer(serializers.ModelSerializer): @@ -649,3 +615,27 @@ class Meta: 'status', 'participants', ) + + +class NationStateOrdersFinalizedSerializer(serializers.ModelSerializer): + + class Meta: + model = models.NationState + fields = ( + 'id', + 'orders_finalized', + ) + + +class NationStateOrdersStatusSerializer(serializers.ModelSerializer): + + class Meta: + model = models.NationState + fields = ( + 'id', + 'num_orders', + 'num_supply_centers', + 'supply_delta', + 'num_builds', + 'num_disbands', + ) diff --git a/service/tests/test_serializers.py b/service/tests/test_serializers.py new file mode 100644 index 0000000..4c6d23d --- /dev/null +++ b/service/tests/test_serializers.py @@ -0,0 +1,39 @@ +from django.test import TestCase + +from service import serializers +from core.tests import DiplomacyTestCaseMixin +from core.models.base import GameStatus + + +class TesNationStateOrdersStatusSerializer(TestCase, DiplomacyTestCaseMixin): + + expected_num_queries = 4 + serializer_class = serializers.NationStateOrdersStatusSerializer + + def setUp(self): + self.variant = self.create_test_variant() + self.game = self.create_test_game( + variant=self.variant, + status=GameStatus.ACTIVE, + ) + self.nation = self.create_test_nation(variant=self.variant) + self.turn = self.create_test_turn(game=self.game) + self.user = self.create_test_user() + self.nation_state = self.create_test_nation_state( + nation=self.nation, + turn=self.turn, + user=self.user, + ) + + def serialize_object(self, obj): + return self.serializer_class(obj).data + + def test_nation_state(self): + with self.assertNumQueries(self.expected_num_queries): + data = self.serialize_object(self.nation_state) + self.assertEqual(data['id'], self.nation_state.id) + self.assertEqual(data['num_orders'], self.nation_state.num_orders) + self.assertEqual(data['num_supply_centers'], self.nation_state.num_supply_centers) + self.assertEqual(data['supply_delta'], self.nation_state.supply_delta) + self.assertEqual(data['num_builds'], self.nation_state.num_builds) + self.assertEqual(data['num_disbands'], self.nation_state.num_disbands) diff --git a/service/tests/test_views.py b/service/tests/test_views.py index c3ee70f..bf7caeb 100644 --- a/service/tests/test_views.py +++ b/service/tests/test_views.py @@ -1,4 +1,4 @@ -from unittest import mock +from unittest import mock, skip from unittest.mock import patch from django.urls import reverse @@ -25,7 +25,9 @@ def set_processed(self, processed_at=None): class BaseTestCase(APITestCase, DiplomacyTestCaseMixin): - pass + + def get_url(self, *args, **kwargs): + return reverse(self.url_name, args=args, kwargs=kwargs) class TestListGames(BaseTestCase): @@ -1562,3 +1564,197 @@ def test_cancel_response(self): response = self.client.delete(self.url, self.data, format='json') self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertEqual(models.DrawResponse.objects.count(), 0) + + +class TestNationStateOrdersFinalized(BaseTestCase): + + url_name = 'nation_state_orders_finalized' + + def setUp(self): + self.variant = self.create_test_variant() + self.game = self.create_test_game( + variant=self.variant, + status=GameStatus.ACTIVE, + ) + self.nation = self.create_test_nation(variant=self.variant) + self.turn = self.create_test_turn(game=self.game) + self.user = self.create_test_user() + self.nation_state = self.create_test_nation_state( + nation=self.nation, + turn=self.turn, + user=self.user, + ) + + def test_get_authorized(self): + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['orders_finalized'], False) + + def test_get_unauthorized(self): + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_not_user_nation(self): + other_user = self.create_test_user() + self.nation_state.user = other_user + self.nation_state.save() + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_not_found(self): + self.nation_state.delete() + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + +class TestNationStateOrdersStatus(BaseTestCase): + + url_name = 'nation_state_orders_status' + + def setUp(self): + self.variant = self.create_test_variant() + self.game = self.create_test_game( + variant=self.variant, + status=GameStatus.ACTIVE, + ) + self.nation = self.create_test_nation(variant=self.variant) + self.turn = self.create_test_turn(game=self.game) + self.user = self.create_test_user() + self.nation_state = self.create_test_nation_state( + nation=self.nation, + turn=self.turn, + user=self.user, + ) + + def test_get_authorized(self): + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertTrue(response.data) + + def test_get_unauthorized(self): + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_not_user_nation(self): + other_user = self.create_test_user() + self.nation_state.user = other_user + self.nation_state.save() + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_not_found(self): + self.nation_state.delete() + self.client.force_authenticate(user=self.user) + url = self.get_url(pk=self.turn.id) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + +class TestGameStateViewOptimization(BaseTestCase): + + expected_num_queries = 14 + + def setUp(self): + self.user = factories.UserFactory() + self.client.force_authenticate(user=self.user) + self.variant = self.create_test_variant(name='test') + self.game = self.create_test_game( + variant=self.variant, + status=GameStatus.ACTIVE, + created_by=self.user, + ) + self.nation = self.create_test_nation( + variant=self.variant, + ) + self.territory = self.create_test_territory( + variant=self.variant, + ) + self.piece = self.create_test_piece( + game=self.game, + nation=self.nation, + ) + self.url = reverse('game-state', args=[self.game.slug]) + + def create_turn(self): + turn = self.create_test_turn( + game=self.game, + phase=Phase.ORDER, + season=Season.SPRING, + ) + self.create_test_order( + turn=turn, + nation=self.nation, + source=self.territory, + ) + self.create_test_order( + turn=turn, + nation=self.nation, + source=self.territory, + ) + self.create_test_piece_state( + turn=turn, + piece=self.piece, + territory=self.territory, + ) + self.create_test_piece_state( + turn=turn, + piece=self.piece, + territory=self.territory, + must_retreat=True, + ) + self.create_test_territory_state( + territory=self.territory, + turn=turn, + ) + self.create_test_territory_state( + territory=self.territory, + turn=turn, + ) + self.create_test_nation_state( + nation=self.nation, + turn=turn, + user=self.user, + ) + self.create_test_nation_state( + nation=self.nation, + turn=turn, + user=self.user, + ) + self.create_test_draw( + turn=turn, + proposed_by=self.nation, + proposed_by_user=self.user, + ) + self.create_test_draw( + turn=turn, + proposed_by=self.nation, + proposed_by_user=self.user, + ) + + @skip + def test_get_game_state(self): + self.create_turn() + with self.assertNumQueries(self.expected_num_queries): + response = self.client.get(self.url, format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) + + @skip + def test_get_game_state_multiple_turns(self): + self.create_turn() + self.create_turn() + self.create_turn() + with self.assertNumQueries(self.expected_num_queries): + response = self.client.get(self.url, format='json') + self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/service/urls.py b/service/urls.py index ddf271b..95ba6ec 100644 --- a/service/urls.py +++ b/service/urls.py @@ -50,6 +50,16 @@ views.GameStateView.as_view(), name='game-state' ), + re_path( + r'game/(?P[0-9]+)/orders_finalized$', + views.NationStateOrdersFinalized.as_view(), + name='nation_state_orders_finalized' + ), + re_path( + r'game/(?P[0-9]+)/orders_status$', + views.NationStateOrdersStatus.as_view(), + name='nation_state_orders_status' + ), path( 'variants', views.ListVariants.as_view(), diff --git a/service/views.py b/service/views.py index c280a53..1a73357 100644 --- a/service/views.py +++ b/service/views.py @@ -10,7 +10,7 @@ from service import serializers from service.decorators import convert_query_params_to_snake_case from service.mixins import CamelCase -from service.permissions import IsAuthenticated +from service.permissions import IsAuthenticated, UserIsNationState # NOTE this could possibly be replaced by using options @@ -111,28 +111,23 @@ def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) -class GameStateView(CamelCase, BaseMixin, generics.RetrieveAPIView): - - previous_orders = models.Order.objects.filter( - turn__current_turn=False, - ) +class GameStateView(CamelCase, generics.RetrieveAPIView): permission_classes = [IsAuthenticated] serializer_class = serializers.GameStateSerializer queryset = ( models.Game.objects.all() + .select_related('variant') .prefetch_related( 'participants', 'pieces', - 'turns__nationstates__user', + 'turns__draws__drawresponse_set', + 'turns__draws__nations', + 'turns__orders', 'turns__nationstates__surrenders', + 'turns__nationstates__user', 'turns__piecestates', - 'turns__territorystates__territory', + 'turns__territorystates', 'turns__turnend', - Prefetch( - 'turns__orders', - queryset=previous_orders, - to_attr='previous_orders' - ), ) ) lookup_field = 'slug' @@ -248,7 +243,7 @@ class ToggleFinalizeOrdersView(CamelCase, generics.UpdateAPIView): def check_object_permissions(self, request, obj): if request.user != obj.user: raise exceptions.PermissionDenied( - detail='Cannot finalize orders for other nation.' + 'Cannot finalize orders for other nation.' ) @@ -400,3 +395,22 @@ def check_object_permissions(self, request, draw_response): raise exceptions.PermissionDenied( detail='Cannot cancel another nation\'s draw response.' ) + + +class NationStateFromTurnMixin: + permission_classes = [IsAuthenticated, UserIsNationState] + queryset = models.NationState.objects.all() + lookup_field = 'turn__id' + lookup_url_kwarg = 'pk' + + def get_queryset(self): + queryset = super().get_queryset() + return queryset.filter(user=self.request.user) + + +class NationStateOrdersFinalized(CamelCase, NationStateFromTurnMixin, generics.RetrieveAPIView): + serializer_class = serializers.NationStateOrdersFinalizedSerializer + + +class NationStateOrdersStatus(CamelCase, NationStateFromTurnMixin, generics.RetrieveAPIView): + serializer_class = serializers.NationStateOrdersStatusSerializer From 456c63e6a84a3e783bbd0649e84afce15005320c Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 11:28:27 +0100 Subject: [PATCH 04/11] LintinG --- service/serializers.py | 2 +- service/views.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/service/serializers.py b/service/serializers.py index 32b699c..e8dba4d 100644 --- a/service/serializers.py +++ b/service/serializers.py @@ -5,7 +5,7 @@ from core import models from core.game import process_turn -from core.models.base import DrawStatus, OrderType, Phase, SurrenderStatus +from core.models.base import DrawStatus, OrderType, SurrenderStatus from . import validators as custom_validators diff --git a/service/views.py b/service/views.py index 1a73357..9e3ca69 100644 --- a/service/views.py +++ b/service/views.py @@ -1,4 +1,3 @@ -from django.db.models import Prefetch from django_filters.rest_framework import DjangoFilterBackend from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator From 0cf305b223ab18554cf756338cf742daa90d7099 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 11:45:06 +0100 Subject: [PATCH 05/11] Extend token expiry to 30 days --- project/settings/base.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/project/settings/base.py b/project/settings/base.py index fc7b863..e06a300 100644 --- a/project/settings/base.py +++ b/project/settings/base.py @@ -1,4 +1,5 @@ import os +from datetime import timedelta # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -164,3 +165,7 @@ CLIENT_URL = 'https://diplomacy-react.netlify.app' CORS_ALLOW_ALL_ORIGINS = True + +REST_KNOX = { + 'TOKEN_TTL': timedelta(days=30), +} From 3fb98f91ef4ab32e19083e92a3c6bac7234a4873 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:38:17 +0100 Subject: [PATCH 06/11] Set DummyHold order to fail if piece must retreat --- adjudicator/order.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/adjudicator/order.py b/adjudicator/order.py index e639d1f..12559a3 100644 --- a/adjudicator/order.py +++ b/adjudicator/order.py @@ -117,7 +117,12 @@ def __init__(self, state, nation, source, **kwargs): self.nation = nation self.source = source self.state = state - self.outcome = Outcomes.SUCCEEDS + + @property + def outcome(self): + if self.piece.retreating: + return Outcomes.FAILS + return Outcomes.SUCCEEDS class Hold(Order): From fc692aab10ffa61b42b5929a17bfdbeb0a096d9d Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:39:22 +0100 Subject: [PATCH 07/11] Add functionality to adjudicator to destroy piece if failed order --- adjudicator/processor.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/adjudicator/processor.py b/adjudicator/processor.py index 6c815d0..658c373 100644 --- a/adjudicator/processor.py +++ b/adjudicator/processor.py @@ -102,6 +102,14 @@ def process(state): else: build.outcome = Outcomes.FAILS + if state.phase == Phase.RETREAT: + for piece in state.pieces: + if piece.retreating and (piece.order.outcome == Outcomes.FAILS): + piece.destroyed = True + piece.destroyed_message = ( + 'Destroyed because piece must retreat but retreat order failed.' + ) + # TODO test # TODO split into sub function # Set captured_by for territories if fall orders From fcc2bad48532a775af71c88dd05cfb6f515c3a5c Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:40:56 +0100 Subject: [PATCH 08/11] Add turn_destroyed field to piece_state --- core/migrations/0009_piece_turn_destroyed.py | 19 +++++++++++++++++++ core/models/piece.py | 10 ++++++++++ 2 files changed, 29 insertions(+) create mode 100644 core/migrations/0009_piece_turn_destroyed.py diff --git a/core/migrations/0009_piece_turn_destroyed.py b/core/migrations/0009_piece_turn_destroyed.py new file mode 100644 index 0000000..6905ee2 --- /dev/null +++ b/core/migrations/0009_piece_turn_destroyed.py @@ -0,0 +1,19 @@ +# Generated by Django 3.1.7 on 2021-04-29 15:19 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0008_auto_20210426_1518'), + ] + + operations = [ + migrations.AddField( + model_name='piece', + name='turn_destroyed', + field=models.ForeignKey(blank=True, help_text='The turn during which this piece was destroyed.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='core.turn'), + ), + ] diff --git a/core/models/piece.py b/core/models/piece.py index c1e9d8a..f4e1ea3 100644 --- a/core/models/piece.py +++ b/core/models/piece.py @@ -51,6 +51,16 @@ class Piece(HygienicModel): 'be a retreat/disband phase or a build/disband phase.' ) ) + turn_destroyed = models.ForeignKey( + 'Turn', + null=True, + blank=True, + on_delete=models.CASCADE, + related_name='+', + help_text=_( + 'The turn during which this piece was destroyed.' + ) + ) def __str__(self): return f'{self.type} ({self.nation}) {self.id}' From 33fef26e6970902e49ef3b1ca91925a19bb09c61 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:42:14 +0100 Subject: [PATCH 09/11] Add adjudicator tests to test failed/illegal retreats --- core/tests/test_adjudicator.py | 242 +++++++++++++++++++++++++++++---- 1 file changed, 213 insertions(+), 29 deletions(-) diff --git a/core/tests/test_adjudicator.py b/core/tests/test_adjudicator.py index 29363fb..7d42936 100644 --- a/core/tests/test_adjudicator.py +++ b/core/tests/test_adjudicator.py @@ -3,7 +3,7 @@ from adjudicator.check import ArmyMovesToAdjacentTerritoryNotConvoy from core import models from core.game import process_turn -from core.models.base import GameStatus, OrderType, PieceType, Phase, Season +from core.models.base import GameStatus, OrderType, OutcomeType, PieceType, Phase, Season from . import DiplomacyTestCaseMixin @@ -28,14 +28,18 @@ def setUp(self): self.turkey = models.Nation.objects.get(id='standard-turkey') self.patch_process_turn_apply_async() + self.livonia = models.Territory.objects.get(id='standard-livonia') + self.norway = models.Territory.objects.get(id='standard-norway') + self.st_petersburg = models.Territory.objects.get(id='standard-st-petersburg') + self.st_petersburg_south_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-south-coast') + self.st_petersburg_north_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-north-coast') + def test_move_st_petersburg_south_coast_to_gulf_of_bothnia(self): piece = models.Piece.objects.create( game=self.game, type=PieceType.FLEET, nation=self.russia, ) - st_petersburg = models.Territory.objects.get(id='standard-st-petersburg') - st_petersburg_south_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-south-coast') gulf_of_bothnia = models.Territory.objects.get(id='standard-gulf-of-bothnia') for nation in models.Nation.objects.all(): models.NationState.objects.create( @@ -48,14 +52,14 @@ def test_move_st_petersburg_south_coast_to_gulf_of_bothnia(self): turn=self.turn, ) models.PieceState.objects.create( - named_coast=st_petersburg_south_coast, + named_coast=self.st_petersburg_south_coast, piece=piece, - territory=st_petersburg, + territory=self.st_petersburg, turn=self.turn, ) order = models.Order.objects.create( nation=self.russia, - source=st_petersburg, + source=self.st_petersburg, target=gulf_of_bothnia, turn=self.turn, type=OrderType.MOVE, @@ -65,14 +69,11 @@ def test_move_st_petersburg_south_coast_to_gulf_of_bothnia(self): self.assertFalse(order.illegal) def test_st_petersburg_and_norway_shared_coast(self): - st_petersburg = models.Territory.objects.get(id='standard-st-petersburg') - st_petersburg_north_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-north-coast') - norway = models.Territory.objects.get(id='standard-norway') - self.assertTrue(st_petersburg in norway.shared_coasts.all()) - self.assertTrue(st_petersburg in norway.neighbours.all()) - self.assertTrue(norway in st_petersburg.shared_coasts.all()) - self.assertTrue(norway in st_petersburg.neighbours.all()) - self.assertTrue(norway in st_petersburg_north_coast.neighbours.all()) + self.assertTrue(self.st_petersburg in self.norway.shared_coasts.all()) + self.assertTrue(self.st_petersburg in self.norway.neighbours.all()) + self.assertTrue(self.norway in self.st_petersburg.shared_coasts.all()) + self.assertTrue(self.norway in self.st_petersburg.neighbours.all()) + self.assertTrue(self.norway in self.st_petersburg_north_coast.neighbours.all()) def test_move_st_petersburg_north_coast_to_norway(self): piece = models.Piece.objects.create( @@ -80,9 +81,6 @@ def test_move_st_petersburg_north_coast_to_norway(self): type=PieceType.FLEET, nation=self.russia, ) - st_petersburg = models.Territory.objects.get(id='standard-st-petersburg') - st_petersburg_north_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-north-coast') - norway = models.Territory.objects.get(id='standard-norway') for nation in models.Nation.objects.all(): models.NationState.objects.create( nation=nation, @@ -94,15 +92,15 @@ def test_move_st_petersburg_north_coast_to_norway(self): turn=self.turn, ) models.PieceState.objects.create( - named_coast=st_petersburg_north_coast, + named_coast=self.st_petersburg_north_coast, piece=piece, - territory=st_petersburg, + territory=self.st_petersburg, turn=self.turn, ) order = models.Order.objects.create( nation=self.russia, - source=st_petersburg, - target=norway, + source=self.st_petersburg, + target=self.norway, turn=self.turn, type=OrderType.MOVE, ) @@ -190,8 +188,6 @@ def test_build_fleet_st_petersburg_north_coast(self): self.turn.season = Season.FALL self.turn.phase = Phase.BUILD self.turn.save() - st_petersburg = models.Territory.objects.get(id='standard-st-petersburg') - st_petersburg_north_coast = models.NamedCoast.objects.get(id='standard-st-petersburg-north-coast') for nation in models.Nation.objects.all(): models.NationState.objects.create( nation=nation, @@ -199,19 +195,19 @@ def test_build_fleet_st_petersburg_north_coast(self): ) models.TerritoryState.objects.create( controlled_by=self.russia, - territory=st_petersburg, + territory=self.st_petersburg, turn=self.turn, ) - for territory in models.Territory.objects.exclude(id=st_petersburg.id): + for territory in models.Territory.objects.exclude(id=self.st_petersburg.id): models.TerritoryState.objects.create( territory=territory, turn=self.turn, ) order = models.Order.objects.create( nation=self.russia, - source=st_petersburg, + source=self.st_petersburg, piece_type=PieceType.FLEET, - target_coast=st_petersburg_north_coast, + target_coast=self.st_petersburg_north_coast, turn=self.turn, type=OrderType.BUILD, ) @@ -219,7 +215,195 @@ def test_build_fleet_st_petersburg_north_coast(self): order.refresh_from_db() self.assertFalse(order.illegal) new_turn.piecestates.get( - territory=st_petersburg, - named_coast=st_petersburg_north_coast, + territory=self.st_petersburg, + named_coast=self.st_petersburg_north_coast, piece__nation=self.russia ) + + def test_illegal_retreat_removes_piece(self): + self.turn.phase = Phase.RETREAT + self.turn.save() + for nation in models.Nation.objects.all(): + models.NationState.objects.create( + nation=nation, + turn=self.turn, + ) + for territory in models.Territory.objects.all(): + models.TerritoryState.objects.create( + territory=territory, + turn=self.turn, + ) + piece = models.Piece.objects.create( + game=self.game, + type=PieceType.ARMY, + nation=self.russia, + ) + piece_state = models.PieceState.objects.create( + piece=piece, + territory=self.norway, + turn=self.turn, + must_retreat=True, + ) + order = models.Order.objects.create( + nation=self.russia, + source=self.norway, + target=self.livonia, + turn=self.turn, + type=OrderType.RETREAT, + ) + new_turn = process_turn(self.turn) + order.refresh_from_db() + piece.refresh_from_db() + piece_state.refresh_from_db() + + self.assertTrue(order.illegal) + self.assertTrue(piece_state.destroyed) + self.assertTrue(piece.turn_destroyed, self.turn) + self.assertEqual(new_turn.piecestates.count(), 0) + + def test_contested_retreat_removes_piece(self): + self.turn.phase = Phase.RETREAT + self.turn.save() + for nation in models.Nation.objects.all(): + models.NationState.objects.create( + nation=nation, + turn=self.turn, + ) + for territory in models.Territory.objects.all(): + models.TerritoryState.objects.create( + territory=territory, + turn=self.turn, + ) + st_petersburg_state = models.TerritoryState.objects.get(territory=self.st_petersburg) + st_petersburg_state.contested = True + st_petersburg_state.save() + piece = models.Piece.objects.create( + game=self.game, + type=PieceType.ARMY, + nation=self.russia, + ) + piece_state = models.PieceState.objects.create( + piece=piece, + territory=self.norway, + turn=self.turn, + must_retreat=True, + ) + order = models.Order.objects.create( + nation=self.russia, + source=self.norway, + target=self.st_petersburg, + turn=self.turn, + type=OrderType.RETREAT, + ) + new_turn = process_turn(self.turn) + order.refresh_from_db() + piece.refresh_from_db() + piece_state.refresh_from_db() + + self.assertTrue(order.illegal) + self.assertEqual(order.outcome, OutcomeType.FAILS) + self.assertTrue(piece_state.destroyed) + self.assertTrue(piece.turn_destroyed, self.turn) + self.assertEqual(new_turn.piecestates.count(), 0) + + def test_failed_retreat_removes_piece(self): + self.turn.phase = Phase.RETREAT + self.turn.save() + for nation in models.Nation.objects.all(): + models.NationState.objects.create( + nation=nation, + turn=self.turn, + ) + for territory in models.Territory.objects.all(): + models.TerritoryState.objects.create( + territory=territory, + turn=self.turn, + ) + piece_norway = models.Piece.objects.create( + game=self.game, + type=PieceType.ARMY, + nation=self.russia, + ) + piece_state_norway = models.PieceState.objects.create( + piece=piece_norway, + territory=self.norway, + turn=self.turn, + must_retreat=True, + ) + piece_livonia = models.Piece.objects.create( + game=self.game, + type=PieceType.ARMY, + nation=self.russia, + ) + piece_state_livonia = models.PieceState.objects.create( + piece=piece_livonia, + territory=self.livonia, + turn=self.turn, + must_retreat=True, + ) + order_norway = models.Order.objects.create( + nation=self.russia, + source=self.norway, + target=self.st_petersburg, + turn=self.turn, + type=OrderType.RETREAT, + ) + order_livonia = models.Order.objects.create( + nation=self.russia, + source=self.livonia, + target=self.st_petersburg, + turn=self.turn, + type=OrderType.RETREAT, + ) + new_turn = process_turn(self.turn) + order_norway.refresh_from_db() + order_livonia.refresh_from_db() + piece_norway.refresh_from_db() + piece_livonia.refresh_from_db() + piece_state_norway.refresh_from_db() + piece_state_livonia.refresh_from_db() + + self.assertFalse(order_livonia.illegal) + self.assertFalse(order_norway.illegal) + self.assertEqual(order_livonia.outcome, OutcomeType.FAILS) + self.assertEqual(order_norway.outcome, OutcomeType.FAILS) + self.assertTrue(piece_state_norway.destroyed) + self.assertTrue(piece_norway.turn_destroyed, self.turn) + self.assertTrue(piece_state_livonia.destroyed) + self.assertTrue(piece_livonia.turn_destroyed, self.turn) + self.assertEqual(new_turn.piecestates.count(), 0) + + def test_no_order_removes_piece(self): + self.turn.phase = Phase.RETREAT + self.turn.save() + for nation in models.Nation.objects.all(): + models.NationState.objects.create( + nation=nation, + turn=self.turn, + ) + for territory in models.Territory.objects.all(): + models.TerritoryState.objects.create( + territory=territory, + turn=self.turn, + ) + st_petersburg_state = models.TerritoryState.objects.get(territory=self.st_petersburg) + st_petersburg_state.contested = True + st_petersburg_state.save() + piece = models.Piece.objects.create( + game=self.game, + type=PieceType.ARMY, + nation=self.russia, + ) + piece_state = models.PieceState.objects.create( + piece=piece, + territory=self.norway, + turn=self.turn, + must_retreat=True, + ) + new_turn = process_turn(self.turn) + piece.refresh_from_db() + piece_state.refresh_from_db() + + self.assertTrue(piece_state.destroyed) + self.assertTrue(piece.turn_destroyed, self.turn) + self.assertEqual(new_turn.piecestates.count(), 0) From a1c95535a518626e2d6bd64d5828c367b4b53885 Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:45:13 +0100 Subject: [PATCH 10/11] Remove skipped tests --- service/tests/test_views.py | 98 ------------------------------------- 1 file changed, 98 deletions(-) diff --git a/service/tests/test_views.py b/service/tests/test_views.py index bf7caeb..d70b182 100644 --- a/service/tests/test_views.py +++ b/service/tests/test_views.py @@ -1660,101 +1660,3 @@ def test_not_found(self): url = self.get_url(pk=self.turn.id) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - - -class TestGameStateViewOptimization(BaseTestCase): - - expected_num_queries = 14 - - def setUp(self): - self.user = factories.UserFactory() - self.client.force_authenticate(user=self.user) - self.variant = self.create_test_variant(name='test') - self.game = self.create_test_game( - variant=self.variant, - status=GameStatus.ACTIVE, - created_by=self.user, - ) - self.nation = self.create_test_nation( - variant=self.variant, - ) - self.territory = self.create_test_territory( - variant=self.variant, - ) - self.piece = self.create_test_piece( - game=self.game, - nation=self.nation, - ) - self.url = reverse('game-state', args=[self.game.slug]) - - def create_turn(self): - turn = self.create_test_turn( - game=self.game, - phase=Phase.ORDER, - season=Season.SPRING, - ) - self.create_test_order( - turn=turn, - nation=self.nation, - source=self.territory, - ) - self.create_test_order( - turn=turn, - nation=self.nation, - source=self.territory, - ) - self.create_test_piece_state( - turn=turn, - piece=self.piece, - territory=self.territory, - ) - self.create_test_piece_state( - turn=turn, - piece=self.piece, - territory=self.territory, - must_retreat=True, - ) - self.create_test_territory_state( - territory=self.territory, - turn=turn, - ) - self.create_test_territory_state( - territory=self.territory, - turn=turn, - ) - self.create_test_nation_state( - nation=self.nation, - turn=turn, - user=self.user, - ) - self.create_test_nation_state( - nation=self.nation, - turn=turn, - user=self.user, - ) - self.create_test_draw( - turn=turn, - proposed_by=self.nation, - proposed_by_user=self.user, - ) - self.create_test_draw( - turn=turn, - proposed_by=self.nation, - proposed_by_user=self.user, - ) - - @skip - def test_get_game_state(self): - self.create_turn() - with self.assertNumQueries(self.expected_num_queries): - response = self.client.get(self.url, format='json') - self.assertEqual(response.status_code, status.HTTP_200_OK) - - @skip - def test_get_game_state_multiple_turns(self): - self.create_turn() - self.create_turn() - self.create_turn() - with self.assertNumQueries(self.expected_num_queries): - response = self.client.get(self.url, format='json') - self.assertEqual(response.status_code, status.HTTP_200_OK) From 1e0a32cd925d8dc14b161ab72c00ad984afcff9e Mon Sep 17 00:00:00 2001 From: johnm Date: Thu, 29 Apr 2021 16:47:17 +0100 Subject: [PATCH 11/11] LintinG --- service/tests/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/tests/test_views.py b/service/tests/test_views.py index d70b182..d8a9466 100644 --- a/service/tests/test_views.py +++ b/service/tests/test_views.py @@ -1,4 +1,4 @@ -from unittest import mock, skip +from unittest import mock from unittest.mock import patch from django.urls import reverse