From 2ce9b6f763001d7b90f9dc6812197762125f2702 Mon Sep 17 00:00:00 2001 From: Alex Lambson Date: Thu, 24 Oct 2024 10:17:28 -0600 Subject: [PATCH] feat: Mvp for image upload saving (#7) * feat: Mvp for image upload saving Save extracted images, and make models for map previews * feat: Map previews Make development server serve files * feat: Map details - Implement map detail view - Better docs for permissions regarding editing - Check for object ban status in `CanEdit` permissions so that banned objects can't be edited by the offender - Fix pytest debugging by adding `setuptools` as a req. - Test that an uploaded map can be retrieved via the API --- .github/workflows/ci.yml | 4 +- docs/file_uploads.md | 21 +++++ kirovy/migrations/0007_jpg_png.py | 59 +++++++++++++ ...8_alter_cncfileextension_extension_type.py | 21 +++++ kirovy/migrations/0009_mappreview.py | 75 +++++++++++++++++ kirovy/models/__init__.py | 1 + kirovy/models/cnc_game.py | 8 ++ kirovy/models/cnc_map.py | 17 +--- kirovy/models/file_base.py | 14 +++- kirovy/models/map_preview.py | 68 +++++++++++++++ kirovy/permissions.py | 15 +++- kirovy/settings/__init__.py | 2 +- kirovy/settings/{base.py => _base.py} | 9 ++ kirovy/settings/testing.py | 3 +- kirovy/urls.py | 27 ++++-- kirovy/views/base_views.py | 12 ++- kirovy/views/cnc_map_views.py | 82 +++++++++++++++++-- requirements-dev.txt | 1 + tests/fixtures/common_fixtures.py | 2 +- tests/test_views/test_map_upload.py | 56 ++++++++++++- 20 files changed, 453 insertions(+), 44 deletions(-) create mode 100644 docs/file_uploads.md create mode 100644 kirovy/migrations/0007_jpg_png.py create mode 100644 kirovy/migrations/0008_alter_cncfileextension_extension_type.py create mode 100644 kirovy/migrations/0009_mappreview.py create mode 100644 kirovy/models/map_preview.py rename kirovy/settings/{base.py => _base.py} (91%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cff93d5..0d00866 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: run: cp ci.env .env - name: Build docker images - run: docker-compose build + run: docker compose build - name: Run PyTest - run: docker-compose run test + run: docker compose run test diff --git a/docs/file_uploads.md b/docs/file_uploads.md new file mode 100644 index 0000000..3720f67 --- /dev/null +++ b/docs/file_uploads.md @@ -0,0 +1,21 @@ +# File uploads + +All file uploads should go into the `kirovy.models.file_base.CncNetFileBaseModel` class. + +This class uses Django's [upload_to](https://docs.djangoproject.com/en/5.0/ref/models/fields/#django.db.models.FileField.upload_to) +logic to automatically place the files. By default, files will go to: + +- `{seetings.MEDIA_ROOT}/{game_slug}/{object.UPLOAD_TYPE}/{object.id}/filename.ext` + +An example of a default upload path would be: + +- `/uploaded_media/yr/uncategorized_uploads/1234/conscript_sprites.shf` + +## Customizing the upload path for a subclass + +Controlling where a file is saved can be easily done by changing `UPLOAD_TYPE: str` for the subclass. +The default value is `uncategorized_uploads`. + +If you need even more control, then override `kirovy.models.file_base.CncNetFileBaseModel.generate_upload_to` with your +own function. Files will still always be placed in `settings.MEDIA_ROOT`, but `generate_upload_to` can control +everything about the upload path after that application-wide root path. diff --git a/kirovy/migrations/0007_jpg_png.py b/kirovy/migrations/0007_jpg_png.py new file mode 100644 index 0000000..3962a41 --- /dev/null +++ b/kirovy/migrations/0007_jpg_png.py @@ -0,0 +1,59 @@ +# Generated by Django 4.2.11 on 2024-08-15 03:35 + +from django.db import migrations +from django.db.backends.postgresql.schema import DatabaseSchemaEditor +from django.db.migrations.state import StateApps + +from kirovy import typing +from kirovy.models import CncFileExtension as _Ext, CncUser as _User + + +def _forward(apps: StateApps, schema_editor: DatabaseSchemaEditor): + + # This is necessary in case later migrations make schema changes to these models. + # Importing them normally will use the latest schema state and will crash if those + # migrations are after this one. + CncFileExtension: typing.Type[_Ext] = apps.get_model("kirovy", "CncFileExtension") + CncUser: typing.Type[_User] = apps.get_model("kirovy", "CncUser") + + migration_user = CncUser.objects.get_or_create_migration_user() + + jpg = CncFileExtension( + extension="jpg", + extension_type=_Ext.ExtensionTypes.IMAGE.value, + about="Jpg files are used for previews on the website and in the client.", + last_modified_by_id=migration_user.id, + ) + jpg.save() + + jpeg = CncFileExtension( + extension="jpeg", + extension_type=_Ext.ExtensionTypes.IMAGE.value, + about="Jpeg files are used for previews on the website and in the client.", + last_modified_by_id=migration_user.id, + ) + jpeg.save() + + png = CncFileExtension( + extension="png", + extension_type=_Ext.ExtensionTypes.IMAGE.value, + about="PNG files are used for previews on the website and in the client.", + last_modified_by_id=migration_user.id, + ) + png.save() + + +def _backward(apps: StateApps, schema_editor: DatabaseSchemaEditor): + """Deleting the games on accident could be devastating to the db so no.""" + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ("kirovy", "0006_cncmap_parent"), + ] + + operations = [ + migrations.RunPython(_forward, reverse_code=_backward, elidable=False), + ] diff --git a/kirovy/migrations/0008_alter_cncfileextension_extension_type.py b/kirovy/migrations/0008_alter_cncfileextension_extension_type.py new file mode 100644 index 0000000..ba77dde --- /dev/null +++ b/kirovy/migrations/0008_alter_cncfileextension_extension_type.py @@ -0,0 +1,21 @@ +# Generated by Django 4.2.11 on 2024-08-15 03:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("kirovy", "0007_jpg_png"), + ] + + operations = [ + migrations.AlterField( + model_name="cncfileextension", + name="extension_type", + field=models.CharField( + choices=[("map", "map"), ("assets", "assets"), ("image", "image")], + max_length=32, + ), + ), + ] diff --git a/kirovy/migrations/0009_mappreview.py b/kirovy/migrations/0009_mappreview.py new file mode 100644 index 0000000..5fb5e30 --- /dev/null +++ b/kirovy/migrations/0009_mappreview.py @@ -0,0 +1,75 @@ +# Generated by Django 4.2.11 on 2024-08-15 04:00 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import kirovy.models.file_base +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ("kirovy", "0008_alter_cncfileextension_extension_type"), + ] + + operations = [ + migrations.CreateModel( + name="MapPreview", + fields=[ + ( + "id", + models.UUIDField( + default=uuid.uuid4, + editable=False, + primary_key=True, + serialize=False, + ), + ), + ("created", models.DateTimeField(auto_now_add=True, null=True)), + ("modified", models.DateTimeField(auto_now=True, null=True)), + ("name", models.CharField(max_length=255)), + ( + "file", + models.FileField( + upload_to=kirovy.models.file_base._generate_upload_to + ), + ), + ("hash_md5", models.CharField(max_length=32)), + ("hash_sha512", models.CharField(max_length=512)), + ("is_extracted", models.BooleanField()), + ( + "cnc_game", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, to="kirovy.cncgame" + ), + ), + ( + "cnc_map_file", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="kirovy.cncmapfile", + ), + ), + ( + "file_extension", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to="kirovy.cncfileextension", + ), + ), + ( + "last_modified_by", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="modified_%(class)s_set", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "abstract": False, + }, + ), + ] diff --git a/kirovy/models/__init__.py b/kirovy/models/__init__.py index 8c9e8e0..8277506 100644 --- a/kirovy/models/__init__.py +++ b/kirovy/models/__init__.py @@ -2,3 +2,4 @@ from .cnc_map import CncMap, CncMapFile, MapCategory from .cnc_user import CncUser from .file_base import CncNetFileBaseModel +from .map_preview import MapPreview diff --git a/kirovy/models/cnc_game.py b/kirovy/models/cnc_game.py index ca1b059..6b814c5 100644 --- a/kirovy/models/cnc_game.py +++ b/kirovy/models/cnc_game.py @@ -26,6 +26,11 @@ class CncFileExtension(CncNetBaseModel): """File extension types for Command & Conquer games and what they do. Useful page: https://modenc.renegadeprojects.com/File_Types + + .. note:: + + These extension objects are only necessary for user-uploaded files. Don't worry about all of this + overhead for any files committed to the repository. """ class ExtensionTypes(models.TextChoices): @@ -37,6 +42,9 @@ class ExtensionTypes(models.TextChoices): ASSETS = "assets", "assets" """This file extension represents some kind of game asset to support a map, e.g. a ``.mix`` file.""" + IMAGE = "image", "image" + """This file extension represents some kind of image uploaded by a user to display on the website.""" + extension = models.CharField( max_length=32, unique=True, validators=[is_valid_extension], blank=False ) diff --git a/kirovy/models/cnc_map.py b/kirovy/models/cnc_map.py index b8767fb..97aad6b 100644 --- a/kirovy/models/cnc_map.py +++ b/kirovy/models/cnc_map.py @@ -62,8 +62,6 @@ class CncMap(cnc_user.CncNetUserOwnedModel): map_name = models.CharField(max_length=128, null=False, blank=False) description = models.CharField(max_length=4096, null=False, blank=False) - cnc_game = models.ForeignKey(game_models.CncGame, models.PROTECT, null=False) - categories = models.ManyToManyField(MapCategory) is_legacy = models.BooleanField( default=False, help_text="If true, this is an upload from the old cncnet database.", @@ -115,6 +113,8 @@ class CncMap(cnc_user.CncNetUserOwnedModel): help_text="If true, then the map file has been uploaded, but the map info has not been set yet.", ) + cnc_game = models.ForeignKey(game_models.CncGame, models.PROTECT, null=False) + categories = models.ManyToManyField(MapCategory) parent = models.ForeignKey( "CncMap", on_delete=models.SET_NULL, @@ -184,20 +184,9 @@ class Meta: def save(self, *args, **kwargs): if not self.version: self.version = self.cnc_map.next_version_number() + self.name = self.cnc_map.generate_versioned_name_for_file() super().save(*args, **kwargs) - def get_map_upload_path(self, filename: str) -> pathlib.Path: - """Generate the upload path for the map file. - - :param filename: - The filename that the user uploaded. - :return: - Path to store the map file in. - This path is not guaranteed to exist because we use this function on first-save. - """ - directory = self.cnc_map.get_map_directory_path() - return pathlib.Path(directory, filename) - @staticmethod def generate_upload_to(instance: "CncMapFile", filename: str) -> pathlib.Path: """Generate the path to upload map files to. diff --git a/kirovy/models/file_base.py b/kirovy/models/file_base.py index 91fdb32..7588a93 100644 --- a/kirovy/models/file_base.py +++ b/kirovy/models/file_base.py @@ -47,7 +47,9 @@ class Meta: ) """What type of file extension this object is.""" - ALLOWED_EXTENSION_TYPES = set(game_models.CncFileExtension.ExtensionTypes.values) + ALLOWED_EXTENSION_TYPES: t.Set[str] = set( + game_models.CncFileExtension.ExtensionTypes.values + ) """Used to make sure e.g. a ``.mix`` doesn't get uploaded as a ``CncMapFile``. These are checked against :attr:`kirovy.models.cnc_game.CncFileExtension.extension_type`. @@ -67,7 +69,15 @@ class Meta: def validate_file_extension( self, file_extension: game_models.CncFileExtension ) -> None: - if file_extension.extension.lower() not in self.cnc_game.allowed_extensions_set: + # Images are allowed for all games. + is_image = ( + self.file_extension.extension_type + == self.file_extension.ExtensionTypes.IMAGE + ) + is_allowed_for_game = ( + file_extension.extension.lower() in self.cnc_game.allowed_extensions_set + ) + if not is_allowed_for_game and not is_image: raise validators.ValidationError( f'"{file_extension.extension}" is not a valid file extension for game "{self.cnc_game.full_name}".' ) diff --git a/kirovy/models/map_preview.py b/kirovy/models/map_preview.py new file mode 100644 index 0000000..92d88f5 --- /dev/null +++ b/kirovy/models/map_preview.py @@ -0,0 +1,68 @@ +import pathlib +import uuid + +from django.db import models + +from kirovy import typing as t +from kirovy.models import cnc_map, file_base, cnc_user, cnc_game + + +class MapPreview(file_base.CncNetFileBaseModel): + """An image preview for a C&C Map upload. + + .. note:: + + This class has no user link. The link to the user is via + :attr:`kirovy.models.map_preview.CncMapPreview.cnc_map`. + + .. note:: + + Map previews are uploaded to the same directory as map files using + :func:`kirovy.models.cnc_map.CncMap.get_map_directory_path` through this class's + :attr:`kirovy.models.map_preview.CncMapPreview.cnc_map`. + + """ + + cnc_map_file = models.ForeignKey(cnc_map.CncMapFile, on_delete=models.CASCADE) + """The map file that this preview belongs to. We link to the file so that we can have version-specific previews.""" + + ALLOWED_EXTENSION_TYPES = { + cnc_game.CncFileExtension.ExtensionTypes.IMAGE.value, + } + + is_extracted = models.BooleanField(null=False, blank=False) + """If true, then this image was extracted from the uploaded map file, usually generated by FinalAlert. + + This will always be false for games released after Yuri's Revenge because Generals and beyond do not pack the + preview image into the map files. + """ + + def save(self, *args, **kwargs): + self.name = self.cnc_map_file.name + self.cnc_game = self.cnc_map_file.cnc_game + return super().save(*args, **kwargs) + + @staticmethod + def generate_upload_to(instance: "MapPreview", filename: str) -> pathlib.Path: + """Generate the path to upload map previews to. + + Gets called by :func:`kirovy.models.file_base._generate_upload_to` when ``CncMapFile.save`` is called. + See [the django docs for file fields](https://docs.djangoproject.com/en/5.0/ref/models/fields/#filefield). + ``upload_to`` is set in :attr:`kirovy.models.file_base.CncNetFileBaseModel.file`, which calls + ``_generate_upload_to``, which calls this function. + + :param instance: + Acts as ``self``. The map preview object that we are creating an upload path for. + :param filename: + The filename of the uploaded file. + :return: + Path to upload map to relative to :attr:`~kirovy.settings.base.MEDIA_ROOT`. + """ + filename = pathlib.Path(filename) + filename_uuid = str(uuid.uuid4()).replace("-", "") + final_file_name = f"{instance.name}_{filename_uuid}{filename.suffix}" + + # e.g. "yr/maps/CNC_NET_MAP_ID_HEX/ra2_map_file_name_UUID.png + return pathlib.Path( + instance.cnc_map_file.cnc_map.get_map_directory_path(), final_file_name + ) diff --git a/kirovy/permissions.py b/kirovy/permissions.py index 3bef5ed..f07734f 100644 --- a/kirovy/permissions.py +++ b/kirovy/permissions.py @@ -47,11 +47,19 @@ def has_permission(self, request: KirovyRequest, view: View) -> bool: class CanEdit(CanUpload): - """ + """Check editing permissions. + Users can edit their own uploads. - Staff can edit user uploads. + Staff can edit all uploads. Users that have been banned cannot edit anymore. Just in case they feel like defacing content in retaliation. + + Checking if the **user** is banned happens in :func:`kirovy.permissions.CanUpload.has_permission` which runs + *before* ``has_object_permissions``. Checking if the **object** is banned is done via checking for an ` + `is_banned`` attribute. + [DRF permission docs](https://www.django-rest-framework.org/api-guide/permissions/#custom-permissions). + + The edit check flow for users: ``Is the user banned -> Is the object banned -> Does the user own the object`` """ def has_object_permission( @@ -62,7 +70,8 @@ def has_object_permission( # Check if this model type is owned by users. if isinstance(obj, cnc_user.CncNetUserOwnedModel): - return request.user == obj.cnc_user + obj_is_banned = hasattr(obj, "is_banned") and obj.is_banned + return request.user == obj.cnc_user and not obj_is_banned return False diff --git a/kirovy/settings/__init__.py b/kirovy/settings/__init__.py index 4fa7e7f..d2be0a7 100644 --- a/kirovy/settings/__init__.py +++ b/kirovy/settings/__init__.py @@ -1 +1 @@ -from kirovy.settings.base import * +from ._base import * diff --git a/kirovy/settings/base.py b/kirovy/settings/_base.py similarity index 91% rename from kirovy/settings/base.py rename to kirovy/settings/_base.py index 8523782..c9e079d 100644 --- a/kirovy/settings/base.py +++ b/kirovy/settings/_base.py @@ -131,12 +131,21 @@ # https://docs.djangoproject.com/en/4.1/howto/static-files/ STATIC_URL = "static/" +"""str: The static directory for serving web files. This is for assets for the website, **not** for uploads.""" + CNC_GAME_IMAGE_DIRECTORY = "game_images/" +""" +str: The directory inside of :attr:`~kirovy.settings._base.STATIC_URL` where we store game-specific and mod-specific +logos and backgrounds. +""" MEDIA_ROOT = get_env_var("MEDIA_ROOT") """:attr: The directory where all user uploads will be stored.""" +MEDIA_URL = get_env_var("MEDIA_URL", default="downloads/") +"""str: The URL path that ``settings.MEDIA_ROOT`` files will be served from.""" + CNC_MAP_DIRECTORY = "maps" """:attr: The directory, beneath the game slug, where map files will be stored.""" diff --git a/kirovy/settings/testing.py b/kirovy/settings/testing.py index 196f3cf..9f6582b 100644 --- a/kirovy/settings/testing.py +++ b/kirovy/settings/testing.py @@ -1,7 +1,8 @@ -from kirovy.settings.base import * +from kirovy.settings._base import * TESTING_API_USERNAME = get_env_var("TESTING_API_USERNAME", "seth@brotherhood.nod") TESTING_API_PASSWORD = get_env_var("TESTING_API_PASSWORD", "ripbozo1") +DEBUG = True DATABASES = { "default": { diff --git a/kirovy/urls.py b/kirovy/urls.py index bfd9cd3..9fdffca 100644 --- a/kirovy/urls.py +++ b/kirovy/urls.py @@ -13,6 +13,8 @@ 1. Import the include() function: from django.urls import include, path 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) """ +from django.conf import settings +from django.conf.urls.static import static from django.contrib import admin from django.urls import path, include @@ -26,14 +28,20 @@ def _get_url_patterns() -> t.List[path]: I added this because I wanted to have the root URLs at the top of the file, but I didn't want to have other url files. """ - return [ - path("admin/", admin.site.urls), - path("test/jwt", test.TestJwt.as_view()), - path("ui-permissions/", permission_views.ListPermissionForAuthUser.as_view()), - path("maps/", include(map_patterns)), - # path("users//", ...), # will show which files a user has uploaded. - # path("games/", ...), # get games. - ] + return ( + [ + path("admin/", admin.site.urls), + path("test/jwt", test.TestJwt.as_view()), + path( + "ui-permissions/", permission_views.ListPermissionForAuthUser.as_view() + ), + path("maps/", include(map_patterns)), + # path("users//", ...), # will show which files a user has uploaded. + # path("games/", ...), # get games. + ] + + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT) + + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) + ) map_patterns = [ @@ -41,7 +49,8 @@ def _get_url_patterns() -> t.List[path]: # path("categories/game//", ...), path("categories/", cnc_map_views.MapCategoryListCreateView.as_view()), path("upload/", cnc_map_views.MapFileUploadView.as_view()), - # path("/", ...), + path("/", cnc_map_views.MapRetrieveUpdateView.as_view()), + # path("img//"), # path("img//", ...), # path("search/") ] diff --git a/kirovy/views/base_views.py b/kirovy/views/base_views.py index 0bcf97a..09b34af 100644 --- a/kirovy/views/base_views.py +++ b/kirovy/views/base_views.py @@ -12,6 +12,7 @@ from kirovy import permissions, typing as t from kirovy.request import KirovyRequest +from kirovy.response import KirovyResponse from kirovy.serializers import KirovySerializer @@ -63,7 +64,6 @@ def create(self, request: KirovyRequest, *args, **kwargs) -> Response: ) def list(self, request, *args, **kwargs): - queryset = self.filter_queryset(self.get_queryset()) page = self.paginate_queryset(queryset) @@ -99,6 +99,16 @@ class KirovyRetrieveUpdateView(_g.RetrieveUpdateAPIView): request: KirovyRequest # Added for type hinting. Populated by DRF ``.setup()`` permission_classes = [permissions.CanEdit | permissions.ReadOnly] + def retrieve(self, request, *args, **kwargs): + instance = self.get_object() + serializer = self.get_serializer(instance) + return KirovyResponse( + t.ResponseData( + result=serializer.data, + ), + status=status.HTTP_200_OK, + ) + def put(self, request: KirovyRequest, *args, **kwargs) -> Response: raise _e.MethodNotAllowed( "PUT", diff --git a/kirovy/views/cnc_map_views.py b/kirovy/views/cnc_map_views.py index dc520ac..9f5200e 100644 --- a/kirovy/views/cnc_map_views.py +++ b/kirovy/views/cnc_map_views.py @@ -3,13 +3,21 @@ import pathlib from django.conf import settings -from django.core.files.uploadedfile import UploadedFile +from django.core.files.uploadedfile import UploadedFile, InMemoryUploadedFile +from django.db.models import Q from rest_framework import status from rest_framework.parsers import MultiPartParser from rest_framework.views import APIView from kirovy import permissions, typing as t, exceptions, constants -from kirovy.models import MapCategory, cnc_map, CncGame, CncFileExtension +from kirovy.models import ( + MapCategory, + cnc_map, + CncGame, + CncFileExtension, + map_preview, + CncMap, +) from kirovy.request import KirovyRequest from kirovy.response import KirovyResponse from kirovy.serializers import cnc_map_serializers @@ -34,7 +42,43 @@ class MapListCreateView(base_views.KirovyListCreateView): class MapRetrieveUpdateView(base_views.KirovyRetrieveUpdateView): - ... + serializer_class = cnc_map_serializers.CncMapBaseSerializer + + def get_queryset(self): + """Get the queryset for map detail views. + + Who can view what: + + - Staff: can view and edit everything + - Anyone: Can view published, legacy, or temporary (cncnet client uploaded) maps. + Banned maps will be excluded. + - Registered Users: Can edit their own maps if the map isn't banned. + Can view their own maps even if the map banned. + The queryset will return a user's banned map, but :class:`kirovy.permissions.CanEdit` will block + any modification attempts. + + Editing permissions are controlled via :class:`kirovy.permissions.CanEdit`. + + View permissions are controlled via :class:`kirovy.permissions.ReadOnly`. + + :return: + """ + if self.request.user.is_staff: + # Staff users can see everything. + return CncMap.objects.filter() + + # Anyone can view legacy maps, temporary maps (for the cncnet client,) and published maps that aren't banned. + queryset = CncMap.objects.filter( + Q(Q(is_published=True) | Q(is_legacy=True) | Q(is_temporary=True)) + & Q(is_banned=False) + ) + + if self.request.user.is_authenticated: + # Users can view their own maps in addition to the normal set. + # User can view their own maps even if the map was banned. + return queryset | CncMap.objects.filter(cnc_user_id=self.request.user.id) + + return queryset class MapDeleteView(base_views.KirovyDestroyView): @@ -107,18 +151,23 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: uploaded_file.write(written_ini.read().encode("utf8")) # Add categories. + non_existing_categories: t.Set[str] = set() for game_mode in map_parser.ini.categories: category = MapCategory.objects.filter(name__iexact=game_mode).first() if not category: + non_existing_categories.add(game_mode) continue new_map.categories.add(category) - # TODO: Save the preview image and get the link for the return. - # TODO: Save file hashes. + if non_existing_categories: + _LOGGER.warning( + "User attempted to upload map with categories that don't exist: non_existing_categories=%s", + non_existing_categories, + ) + new_map_file = cnc_map.CncMapFile( width=map_parser.ini.get(CncGen2MapSections.HEADER, "Width"), height=map_parser.ini.get(CncGen2MapSections.HEADER, "Height"), - name=new_map.generate_versioned_name_for_file(), cnc_map=new_map, file=uploaded_file, file_extension=extension, @@ -126,6 +175,24 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: ) new_map_file.save() + extracted_image = map_parser.extract_preview() + extracted_image_url: str = "" + if extracted_image: + image_io = io.BytesIO() + image_extension = CncFileExtension.objects.get(extension="jpg") + extracted_image.save(image_io, format="JPEG", quality=95) + django_image = InMemoryUploadedFile( + image_io, None, "temp.jpg", "image/jpeg", image_io.tell(), None + ) + new_map_preview = map_preview.MapPreview( + is_extracted=True, + cnc_map_file=new_map_file, + file=django_image, + file_extension=image_extension, + ) + new_map_preview.save() + extracted_image_url = new_map_preview.file.url + # TODO: Actually serialize the return data and include the link to the preview. # TODO: Should probably convert this to DRF for that step. return KirovyResponse( @@ -133,8 +200,9 @@ def post(self, request: KirovyRequest, format=None) -> KirovyResponse: message="File uploaded successfully", result={ "cnc_map": new_map.map_name, - "cnc_map_file": new_map_file.file.name, + "cnc_map_file": new_map_file.file.url, "cnc_map_id": new_map.id, + "extracted_preview_file": extracted_image_url, }, ), status=status.HTTP_201_CREATED, diff --git a/requirements-dev.txt b/requirements-dev.txt index c8baba5..f628e4b 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,3 +5,4 @@ black==24.* pre-commit==3.* pytest-django==4.* markdown>=3.4.4, <=4.0 +setuptools diff --git a/tests/fixtures/common_fixtures.py b/tests/fixtures/common_fixtures.py index a3c27fc..478b53e 100644 --- a/tests/fixtures/common_fixtures.py +++ b/tests/fixtures/common_fixtures.py @@ -318,7 +318,7 @@ def client_anonymous(create_client) -> KirovyClient: @pytest.fixture def client_user(user, create_client) -> KirovyClient: - """Returns a client with an active admin user.""" + """Returns a client with an active and verified user.""" return create_client(user) diff --git a/tests/test_views/test_map_upload.py b/tests/test_views/test_map_upload.py index c3cf7ba..faf305f 100644 --- a/tests/test_views/test_map_upload.py +++ b/tests/test_views/test_map_upload.py @@ -1,8 +1,12 @@ import pathlib +import subprocess +from hashlib import md5 from django.core.files.uploadedfile import UploadedFile from rest_framework import status +from kirovy import settings +from kirovy.models import CncMap, CncMapFile, MapCategory from kirovy.services.cnc_gen_2_services import CncGen2MapParser _UPLOAD_URL = "/maps/upload/" @@ -11,7 +15,6 @@ def test_map_file_upload_happy_path( client_user, file_map_desert, game_yuri, extension_map, tmp_media_root ): - # TODO: Finish the tests. response = client_user.post( _UPLOAD_URL, {"file": file_map_desert, "game_id": str(game_yuri.id)}, @@ -20,10 +23,57 @@ def test_map_file_upload_happy_path( ) assert response.status_code == status.HTTP_201_CREATED - uploaded_file = ( - pathlib.Path(tmp_media_root) / response.data["result"]["cnc_map_file"] + + uploaded_file_url: str = response.data["result"]["cnc_map_file"] + uploaded_image_url: str = response.data["result"]["extracted_preview_file"] + strip_media_url = f"/{settings.MEDIA_URL}" + uploaded_file = pathlib.Path(tmp_media_root) / uploaded_file_url.lstrip( + strip_media_url + ) + uploaded_image = pathlib.Path(tmp_media_root) / uploaded_image_url.lstrip( + strip_media_url ) assert uploaded_file.exists() + assert uploaded_image.exists() parser = CncGen2MapParser(UploadedFile(open(uploaded_file, "rb"))) assert parser.ini.get("CnCNet", "ID") == str(response.data["result"]["cnc_map_id"]) + + map_object = CncMap.objects.get(id=response.data["result"]["cnc_map_id"]) + file_object = CncMapFile.objects.get(cnc_map_id=map_object.id) + + assert map_object + + # Note: These won't match an md5 from the commandline because we add the ID to the map file. + assert file_object.hash_md5 == md5(open(uploaded_file, "rb").read()).hexdigest() + file_map_desert.seek(0) + assert file_object.hash_md5 != md5(file_map_desert.read()).hexdigest() + + get_response = client_user.get(f"/maps/{map_object.id}/") + + assert get_response.status_code == status.HTTP_200_OK + response_map = get_response.data["result"] + + # A lot of these will break if you change the desert.map file. + assert response_map["cnc_user_id"] == str(client_user.kirovy_user.id) + assert ( + response_map["map_name"] == "desert" + ), "Should match the name in the map file." + assert response_map["cnc_game_id"] == str(game_yuri.id) + assert response_map["category_ids"] == [ + str(MapCategory.objects.get(name__iexact="standard").id), + ] + assert not response_map[ + "is_published" + ], "Newly uploaded, unrefined, maps should default to unpublished." + assert not response_map[ + "is_temporary" + ], "Maps uploaded via a signed in user shouldn't be marked as temporary." + assert not response_map["is_reviewed"], "Maps should not default to being reviewed." + assert not response_map[ + "is_banned" + ], "Happy path maps should not be banned on upload." + assert ( + response_map["legacy_upload_date"] is None + ), "Non legacy maps should never have this field." + assert response_map["id"] == str(map_object.id)