- Make kirovy exception handler more generic

- Add `/maps/img/id` edit endpoint
- Add `editable_fields` to serializers, which allows for fields to be set during creation, but never allowed to be updated.

Author: alexlambson

kirovy/constants/api_codes.py

@@ -32,3 +32,8 @@ class FileUploadApiCodes(enum.StrEnum):
     e.g. a temporary map does not support custom image uploads.
     """
     TOO_LARGE = "file-too-large"
+
+
+class GenericApiCodes(enum.StrEnum):
+    CANNOT_UPDATE_FIELD = "field-cannot-be-updated-after-creation"
+    """attr: Some fields are not allowed to be edited via any API endpoint."""

kirovy/exception_handler.py

@@ -1,7 +1,6 @@
-from rest_framework import status
 from rest_framework.views import exception_handler
 
-from kirovy.exceptions.view_exceptions import KirovyValidationError
+from kirovy.exceptions.view_exceptions import KirovyAPIException
 from kirovy.objects import ui_objects
 from kirovy.response import KirovyResponse
 
@@ -23,7 +22,7 @@ def kirovy_exception_handler(exception: Exception, context) -> KirovyResponse[ui
         Returns the ``KirovyResponse`` if the exception is one we defined.
         Otherwise, it calls the base DRF exception handler :func:`rest_framework.views.exception_handler`.
     """
-    if isinstance(exception, KirovyValidationError):
-        return KirovyResponse(exception.as_error_response_data(), status=status.HTTP_400_BAD_REQUEST)
+    if isinstance(exception, KirovyAPIException):
+        return KirovyResponse(exception.as_error_response_data(), status=exception.status_code)
 
     return exception_handler(exception, context)

kirovy/exceptions/view_exceptions.py

@@ -1,3 +1,4 @@
+from django.utils.encoding import force_str
 from rest_framework import status
 from rest_framework.exceptions import APIException as _DRFAPIException
 from django.utils.translation import gettext_lazy as _
@@ -6,17 +7,8 @@
 from kirovy.objects import ui_objects
 
 
-class KirovyValidationError(_DRFAPIException):
-    """A custom exception that easily converts to the standard ``ErrorResponseData``
-
-    See: :class:`kirovy.objects.ui_objects.ErrorResponseData`
-
-    This exception is meant to be used within serializers or views.
-    """
-
-    status_code = status.HTTP_400_BAD_REQUEST
-    default_detail = _("Invalid input.")
-    default_code = "invalid"
+class KirovyAPIException(_DRFAPIException):
+    status_code: _t.ClassVar[int] = status.HTTP_500_INTERNAL_SERVER_ERROR
     additional: _t.DictStrAny | None = None
     code: str | None
     """attr: Some kind of string that the UI will recognize. e.g. ``file-too-large``.
@@ -41,3 +33,29 @@ def __init__(self, detail: str | None = None, code: str | None = None, additiona
 
     def as_error_response_data(self) -> ui_objects.ErrorResponseData:
         return ui_objects.ErrorResponseData(message=self.detail, code=self.code, additional=self.additional)
+
+
+class KirovyValidationError(KirovyAPIException):
+    """A custom exception that easily converts to the standard ``ErrorResponseData``
+
+    See: :class:`kirovy.objects.ui_objects.ErrorResponseData`
+
+    This exception is meant to be used within serializers or views.
+    """
+
+    status_code = status.HTTP_400_BAD_REQUEST
+    default_detail = _("Invalid input.")
+    default_code = "invalid"
+
+
+class KirovyMethodNotAllowed(KirovyAPIException):
+    status_code = status.HTTP_405_METHOD_NOT_ALLOWED
+    default_detail = _('Method "{method}" not allowed.')
+    default_code = "method_not_allowed"
+
+    def __init__(
+        self, method, detail: str | None = None, code: str | None = None, additional: _t.DictStrAny | None = None
+    ):
+        if detail is None:
+            detail = force_str(self.default_detail).format(method=method)
+        super().__init__(detail, code, additional)

kirovy/serializers/__init__.py

@@ -1,5 +1,7 @@
 from rest_framework import serializers
 
+from kirovy.constants import api_codes
+from kirovy.exceptions.view_exceptions import KirovyValidationError
 from kirovy.models import CncUser
 from kirovy import typing as t
 from kirovy.request import KirovyRequest
@@ -21,6 +23,7 @@ class KirovySerializer(serializers.Serializer):
     class Meta:
         exclude = ["last_modified_by"]
         fields = "__all__"
+        editable_fields: t.ClassVar[set[str]] = set()
 
     def get_fields(self):
         """Get fields based on permission level.
@@ -34,6 +37,29 @@ def get_fields(self):
             fields.pop("last_modified_by_id", None)
         return fields
 
+    def to_internal_value(self, data: dict) -> dict:
+        """Convert the raw request data into data that can be used in a django model.
+
+        Enforces editable fields from :attr:`~kirovy.serializers.KirovySerializer.Meta.editable.fields`.
+        """
+        data = super().to_internal_value(data)
+        # Enforce editable fields.
+        if self.instance and hasattr(self.Meta, "editable_fields") and self.Meta.editable_fields:
+            updated_keys = set(data.keys())
+            if attempted_disallowed_updates := list(updated_keys - self.Meta.editable_fields):
+                # `to_internal_value` converts `data["thing_id"]` to `data["thing"]`.
+                # If you're getting this error on e.g. `thing_id`, but you specified `thing_id` as editable,
+                # then you need to change the name in `editable_fields` to `thing`.
+                raise KirovyValidationError(
+                    "Requested fields cannot be edited after creation",
+                    api_codes.GenericApiCodes.CANNOT_UPDATE_FIELD,
+                    additional={
+                        "can_update": list(self.Meta.editable_fields),
+                        "attempted": attempted_disallowed_updates,
+                    },
+                )
+        return data
+
 
 class CncNetUserOwnedModelSerializer(KirovySerializer):
     """Base serializer for any model that mixes in :class:`~kirovy.models.cnc_user.CncNetUserOwnedModel`"""

kirovy/serializers/cnc_map_serializers.py

@@ -109,6 +109,7 @@ class Meta:
         # We return the ID instead of the whole object.
         exclude = ["cnc_user", "cnc_game", "cnc_map", "file_extension", "hash_md5", "hash_sha512", "hash_sha1"]
         fields = "__all__"
+        editable_fields: set[str] = {"name", "image_order"}
 
     width = serializers.IntegerField()
     """attr: The map height.

kirovy/urls.py

@@ -24,7 +24,7 @@
 import kirovy.views.map_image_views
 from kirovy.models import CncGame
 from kirovy.settings import settings_constants
-from kirovy.views import test, cnc_map_views, permission_views, admin_views, map_upload_views
+from kirovy.views import test, cnc_map_views, permission_views, admin_views, map_upload_views, map_image_views
 from kirovy import typing as t, constants
 
 _DjangoPath = URLPattern | URLResolver
@@ -109,7 +109,8 @@ def _get_url_patterns() -> list[_DjangoPath]:
     path("<uuid:pk>/", cnc_map_views.MapRetrieveUpdateView.as_view()),
     path("delete/<uuid:pk>/", cnc_map_views.MapDeleteView.as_view()),
     path("search/", cnc_map_views.MapListView.as_view()),
-    path("img/", kirovy.views.map_image_views.MapImageFileUploadView.as_view()),
+    path("img/", map_image_views.MapImageFileUploadView.as_view()),
+    path("img/<uuid:pk>/", map_image_views.MapImageFileRetrieveUpdate.as_view()),
     # path("img/<uuid:map_id>/", ...),
     # path("search/")
 ]

kirovy/views/base_views.py

@@ -6,21 +6,19 @@
 
 from django.core.files.uploadedfile import UploadedFile, InMemoryUploadedFile
 from rest_framework import (
-    exceptions as _e,
     generics as _g,
     permissions as _p,
     pagination as _pagination,
     status,
 )
 from rest_framework.generics import get_object_or_404
 from rest_framework.parsers import MultiPartParser
-from rest_framework.response import Response
 from rest_framework.views import APIView
 
 import kirovy.objects.ui_objects
 from kirovy import permissions, typing as t, logging
 from kirovy.constants import api_codes
-from kirovy.exceptions.view_exceptions import KirovyValidationError
+from kirovy.exceptions.view_exceptions import KirovyValidationError, KirovyMethodNotAllowed
 from kirovy.models import CncNetFileBaseModel
 from kirovy.models.cnc_game import GameScopedUserOwnedModel
 from kirovy.objects import ui_objects
@@ -124,16 +122,16 @@ def retrieve(self, request, *args, **kwargs):
             status=status.HTTP_200_OK,
         )
 
-    def put(self, request: KirovyRequest, *args, **kwargs) -> Response:
-        raise _e.MethodNotAllowed(
+    def put(self, request: KirovyRequest, *args, **kwargs) -> KirovyResponse:
+        raise KirovyMethodNotAllowed(
             "PUT",
             "PUT is not allowed. Only use PATCH and only send fields that were modified.",
         )
 
-    def delete(self, request: KirovyRequest, *args, **kwargs) -> Response:
-        raise _e.MethodNotAllowed(
+    def delete(self, request: KirovyRequest, *args, **kwargs) -> KirovyResponse:
+        raise KirovyMethodNotAllowed(
             "DELETE",
-            "DELETE is not allowed on this endpoint. Please use the delete endpoint.",
+            "DELETE is not allowed on this endpoint",
         )
 
 

kirovy/views/map_image_views.py

@@ -4,6 +4,7 @@
 from PIL import Image, UnidentifiedImageError
 from PIL.Image import DecompressionBombError
 from django.core.files.uploadedfile import UploadedFile, InMemoryUploadedFile
+from django.db.models import QuerySet
 
 from kirovy import permissions, typing as t
 from kirovy.constants import api_codes
@@ -75,3 +76,16 @@ def modify_uploaded_file(
             image.convert("RGB").save(image_io, format="JPEG", quality=95)
 
         return InMemoryUploadedFile(image_io, None, f"{filename}.jpg", "image/jpeg", image_io.tell(), None)
+
+
+class MapImageFileRetrieveUpdate(base_views.KirovyRetrieveUpdateView):
+    """Endpoint to edit the editable fields for a map image."""
+
+    serializer_class = cnc_map_serializers.CncMapImageFileSerializer
+
+    def get_queryset(self) -> QuerySet[CncMapImageFile]:
+        base = CncMapImageFile.objects.filter(cnc_map__is_banned=False)
+        if self.request.user.is_authenticated:
+            return base | CncMapImageFile.objects.filter(cnc_user_id=self.request.user.id)
+
+        return base

tests/fixtures/common_fixtures.py

@@ -17,7 +17,7 @@
 
 from kirovy import objects, typing as t, constants
 from kirovy.models import CncUser
-from kirovy.objects.ui_objects import ErrorResponseData
+from kirovy.objects.ui_objects import ErrorResponseData, BanData
 from kirovy.response import KirovyResponse
 
 
@@ -376,3 +376,21 @@ def client_admin(admin, create_client) -> KirovyClient:
 def client_god(god, create_client) -> KirovyClient:
     """Returns a client with an active god user."""
     return create_client(god)
+
+
+@pytest.fixture
+def ban_user(client_god):
+    """Return a function to ban a user."""
+
+    def _inner(user_to_ban: CncUser, ban_reason: str = "Silos Needed") -> CncUser:
+        response = client_god.post(
+            "/admin/ban/",
+            BanData(
+                object_type=BanData.Meta.ObjectType.USER, is_banned=True, note=ban_reason, object_id=str(user_to_ban.id)
+            ).model_dump_json(),
+        )
+        assert response.status_code == status.HTTP_200_OK
+        user_to_ban.refresh_from_db()
+        return user_to_ban
+
+    return _inner

tests/fixtures/map_fixtures.py

@@ -1,8 +1,9 @@
 from django.core.files import File
 from django.db.models import UUIDField
+from rest_framework import status
 
-from kirovy.models import CncGame
-from kirovy.models.cnc_map import CncMap, CncMapFile, MapCategory
+from kirovy.models import CncGame, CncUser
+from kirovy.models.cnc_map import CncMap, CncMapFile, MapCategory, CncMapImageFile
 from kirovy import typing as t
 import pytest
 
@@ -169,3 +170,39 @@ def banned_cheat_map(create_cnc_map, file_map_unfair) -> CncMap:
         is_temporary=True,
         file=file_map_unfair,
     )
+
+
+@pytest.fixture
+def create_cnc_map_image_file(create_client, create_cnc_map, create_kirovy_user):
+    """Return a function to add an image to a map."""
+
+    def _inner(
+        file: File,
+        cnc_map: CncMap | None = None,
+        user: CncUser | None = None,
+    ) -> CncMapImageFile:
+        if cnc_map and not user:
+            # The most common case is just passing a map.
+            user = cnc_map.cnc_user
+        else:
+            # Only passed a user, or passed neither.
+            if not user:
+                user = create_kirovy_user()
+            if not cnc_map:
+                cnc_map = create_cnc_map(user_id=user.id)
+
+        assert cnc_map and user
+        assert cnc_map.cnc_user_id == user.id
+
+        client = create_client(user)
+        response = client.post(
+            "/maps/img/",
+            {"file": file, "cnc_map_id": str(cnc_map.id)},
+            format="multipart",
+            content_type=None,
+        )
+        assert response.status_code == status.HTTP_201_CREATED
+
+        return CncMapImageFile.objects.get(id=response.data["result"]["file_id"])
+
+    return _inner