From f76e4fc28a4cc82639826d4b1c4deced9e9ddf0d Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 11:04:48 +0200 Subject: [PATCH 01/36] Draft uploading files locally and on s3 --- config/django/base.py | 4 ++ config/settings/aws.py | 22 +++++++ requirements/base.txt | 4 +- styleguide_example/api/urls.py | 1 + styleguide_example/files/__init__.py | 0 styleguide_example/files/admin.py | 10 +++ styleguide_example/files/apis.py | 41 ++++++++++++ styleguide_example/files/apps.py | 5 ++ .../files/migrations/0001_initial.py | 34 ++++++++++ .../files/migrations/__init__.py | 0 styleguide_example/files/models.py | 23 +++++++ styleguide_example/files/services.py | 51 ++++++++++++++ styleguide_example/files/urls.py | 11 ++++ styleguide_example/files/utils.py | 13 ++++ styleguide_example/integrations/__init__.py | 0 styleguide_example/integrations/apps.py | 5 ++ styleguide_example/integrations/aws/client.py | 66 +++++++++++++++++++ 17 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 config/settings/aws.py create mode 100644 styleguide_example/files/__init__.py create mode 100644 styleguide_example/files/admin.py create mode 100644 styleguide_example/files/apis.py create mode 100644 styleguide_example/files/apps.py create mode 100644 styleguide_example/files/migrations/0001_initial.py create mode 100644 styleguide_example/files/migrations/__init__.py create mode 100644 styleguide_example/files/models.py create mode 100644 styleguide_example/files/services.py create mode 100644 styleguide_example/files/urls.py create mode 100644 styleguide_example/files/utils.py create mode 100644 styleguide_example/integrations/__init__.py create mode 100644 styleguide_example/integrations/apps.py create mode 100644 styleguide_example/integrations/aws/client.py diff --git a/config/django/base.py b/config/django/base.py index b93df957..8f371584 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -41,6 +41,8 @@ 'styleguide_example.users.apps.UsersConfig', 'styleguide_example.errors.apps.ErrorsConfig', 'styleguide_example.testing_examples.apps.TestingExamplesConfig', + 'styleguide_example.integrations.apps.IntegrationsConfig', + 'styleguide_example.files.apps.FilesConfig', ] THIRD_PARTY_APPS = [ @@ -171,6 +173,8 @@ 'DEFAULT_AUTHENTICATION_CLASSES': [] } +SERVER_HOST_DOMAIN = env("SERVER_HOST_DOMAIN", default="http://localhost:8000") + from config.settings.cors import * # noqa from config.settings.jwt import * # noqa from config.settings.sessions import * # noqa diff --git a/config/settings/aws.py b/config/settings/aws.py new file mode 100644 index 00000000..c61089c4 --- /dev/null +++ b/config/settings/aws.py @@ -0,0 +1,22 @@ +from config.env import env + +DEFAULT_FILE_STORAGE = env( + "DEFAULT_FILE_STORAGE", + default="django.core.files.storage.FileSystemStorage", +) + +USE_S3_UPLOAD = env("USE_S3_UPLOAD", default=False) + +if USE_S3_UPLOAD: + AWS_ACCESS_KEY_ID = env("AWS_ACCESS_KEY_ID") + AWS_SECRET_ACCESS_KEY = env("AWS_SECRET_ACCESS_KEY") + AWS_STORAGE_BUCKET_NAME = env("AWS_STORAGE_BUCKET_NAME") + + AWS_FILES_EXPIRY = 60 * 60 # 1 hour. Change this configuration if needed + + AWS_S3_REGION_NAME = env("AWS_S3_REGION_NAME") + AWS_S3_CUSTOM_DOMAIN = env("AWS_S3_CUSTOM_DOMAIN") + AWS_S3_DOMAIN = ( + AWS_S3_CUSTOM_DOMAIN or f"{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com" + ) + MEDIA_URL = f"https://{AWS_S3_DOMAIN}/media/" diff --git a/requirements/base.txt b/requirements/base.txt index 87b5fce3..9ba30455 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -10,7 +10,9 @@ django-celery-beat==2.2.1 whitenoise==6.0.0 django-filter==21.1 -django-cors-headers==3.11.0 django-extensions==3.1.5 +django-cors-headers==3.10.0 drf-jwt==1.19.2 + +boto3==1.20.20 diff --git a/styleguide_example/api/urls.py b/styleguide_example/api/urls.py index 0f659849..f960f19a 100644 --- a/styleguide_example/api/urls.py +++ b/styleguide_example/api/urls.py @@ -6,4 +6,5 @@ ), path('users/', include(('styleguide_example.users.urls', 'users'))), path('errors/', include(('styleguide_example.errors.urls', 'errors'))), + path('files/', include(('styleguide_example.files.urls', 'files'))), ] diff --git a/styleguide_example/files/__init__.py b/styleguide_example/files/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/styleguide_example/files/admin.py b/styleguide_example/files/admin.py new file mode 100644 index 00000000..cca33435 --- /dev/null +++ b/styleguide_example/files/admin.py @@ -0,0 +1,10 @@ +from django.contrib import admin + +from styleguide_example.files.models import File + + +@admin.register(File) +class FileAdmin(admin.ModelAdmin): + list_display = ["id", "file_name"] + + ordering = ["-created_at"] diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py new file mode 100644 index 00000000..f7644530 --- /dev/null +++ b/styleguide_example/files/apis.py @@ -0,0 +1,41 @@ +from django.conf import settings +from django.shortcuts import get_object_or_404 +from django.core.exceptions import PermissionDenied + +from rest_framework import serializers, status +from rest_framework.response import Response +from rest_framework.views import APIView + +from styleguide_example.files.models import File +from styleguide_example.files.services import file_generate_private_presigned_post_data + +from styleguide_example.api.mixins import ApiAuthMixin + + +class FileGeneratePrivatePresignedPostApi(ApiAuthMixin, APIView): + class InputSerializer(serializers.Serializer): + file_name = serializers.CharField() + file_type = serializers.CharField() + + def post(self, request, *args, **kwargs): + serializer = self.InputSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + presigned_data = file_generate_private_presigned_post_data( + user=request.user, **serializer.validated_data + ) + + return Response(data=presigned_data) + + +class FileLocalUploadAPI(ApiAuthMixin, APIView): + def post(self, request, file_id): + if settings.USE_S3_UPLOAD: + raise PermissionDenied('USE_S3_UPLOAD is enabled. Access to this API is forbidden.') + + file = get_object_or_404(File, id=file_id) + + file.file = request.FILES["file"] + file.save() + + return Response(status=status.HTTP_201_CREATED) diff --git a/styleguide_example/files/apps.py b/styleguide_example/files/apps.py new file mode 100644 index 00000000..63f97e04 --- /dev/null +++ b/styleguide_example/files/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class FilesConfig(AppConfig): + name = 'styleguide_example.files' diff --git a/styleguide_example/files/migrations/0001_initial.py b/styleguide_example/files/migrations/0001_initial.py new file mode 100644 index 00000000..e2454e09 --- /dev/null +++ b/styleguide_example/files/migrations/0001_initial.py @@ -0,0 +1,34 @@ +# Generated by Django 3.2.9 on 2021-12-06 09:03 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import styleguide_example.files.utils + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='File', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(db_index=True, default=django.utils.timezone.now)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('file', models.FileField(upload_to=styleguide_example.files.utils.file_generate_upload_path)), + ('file_name', models.CharField(max_length=255)), + ('file_type', models.CharField(max_length=255)), + ('uploaded_by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/styleguide_example/files/migrations/__init__.py b/styleguide_example/files/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py new file mode 100644 index 00000000..26af7203 --- /dev/null +++ b/styleguide_example/files/models.py @@ -0,0 +1,23 @@ +from django.db import models +from django.conf import settings + +from styleguide_example.common.models import BaseModel + +from styleguide_example.users.models import BaseUser + +from styleguide_example.files.utils import file_generate_upload_path + + +class File(BaseModel): + file = models.FileField(upload_to=file_generate_upload_path) + file_name = models.CharField(max_length=255) + file_type = models.CharField(max_length=255) + + uploaded_by = models.ForeignKey(BaseUser, on_delete=models.CASCADE) + + @property + def url(self): + if settings.USE_S3_UPLOAD: + return self.file.url + + return f"{settings.SERVER_HOST_DOMAIN}{self.file.url}" diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py new file mode 100644 index 00000000..82e3cc2e --- /dev/null +++ b/styleguide_example/files/services.py @@ -0,0 +1,51 @@ +from django.conf import settings +from django.db import transaction + +from styleguide_example.files.models import File +from styleguide_example.files.utils import ( + file_generate_upload_path, + file_generate_local_upload_url +) + +from styleguide_example.integrations.aws.client import s3_generate_private_presigned_post + +from styleguide_example.users.models import BaseUser + + +def file_create_for_upload(*, user: BaseUser, file_name: str, file_type: str) -> File: + image = File( + file_name=file_name, + file_type=file_type, + uploaded_by=user, + file=None + ) + image.full_clean() + image.save() + + return image + + +@transaction.atomic +def file_generate_private_presigned_post_data(*, user: BaseUser, file_name: str, file_type: str): + file = file_create_for_upload(user=user, file_name=file_name, file_type=file_type) + + if settings.USE_S3_UPLOAD: + upload_path = file_generate_upload_path(file, file.file_name) + + presigned_data = s3_generate_private_presigned_post( + file_path=upload_path, file_type=file.file_type + ) + + """ + Setting the file.file path to be the s3 upload path without uploading the file. + The actual file upload will be done by the FE. + """ + file.file = file.file.field.attr_class(file, file.file.field, upload_path) + file.save() + else: + presigned_data = { + "url": file_generate_local_upload_url(file_id=file.id), + "params": {"headers": {"Authorization": f"Token {user.auth_token}"}}, + } + + return {"identifier": file.id, **presigned_data} diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py new file mode 100644 index 00000000..64f4bb23 --- /dev/null +++ b/styleguide_example/files/urls.py @@ -0,0 +1,11 @@ +from django.urls import path + +from styleguide_example.files.apis import FileGeneratePrivatePresignedPostApi, FileLocalUploadAPI + +urlpatterns = [ + path("files/private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), + path( + "files//local-upload/", + FileLocalUploadAPI.as_view(), + ), +] diff --git a/styleguide_example/files/utils.py b/styleguide_example/files/utils.py new file mode 100644 index 00000000..3b5d15a1 --- /dev/null +++ b/styleguide_example/files/utils.py @@ -0,0 +1,13 @@ +import pathlib + +from django.conf import settings + + +def file_generate_upload_path(instance, filename): + extension = pathlib.Path(filename).suffix + + return f"files/{instance.id}{extension}" + + +def file_generate_local_upload_url(*, file_id: str): + return f"{settings.SERVER_HOST_DOMAIN}/api/files/images/{file_id}/local-upload/" diff --git a/styleguide_example/integrations/__init__.py b/styleguide_example/integrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/styleguide_example/integrations/apps.py b/styleguide_example/integrations/apps.py new file mode 100644 index 00000000..2dddf743 --- /dev/null +++ b/styleguide_example/integrations/apps.py @@ -0,0 +1,5 @@ +from django.apps import AppConfig + + +class IntegrationsConfig(AppConfig): + name = 'styleguide_example.integrations' diff --git a/styleguide_example/integrations/aws/client.py b/styleguide_example/integrations/aws/client.py new file mode 100644 index 00000000..1f5e4edb --- /dev/null +++ b/styleguide_example/integrations/aws/client.py @@ -0,0 +1,66 @@ +import logging + +import boto3 +from botocore.exceptions import ClientError + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + +from typing import Optional + + +def get_s3_client(): + required_config = [ + settings.AWS_ACCESS_KEY_ID, + settings.AWS_SECRET_ACCESS_KEY, + settings.AWS_STORAGE_BUCKET_NAME, + settings.AWS_FILES_EXPIRY + ] + + for config in required_config: + if not config: + raise ImproperlyConfigured(f'AWS not configured. Missing {config}.') + + return boto3.client( + service_name="s3", + aws_access_key_id=settings.AWS_ACCESS_KEY_ID, + aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY, + ) + + +def s3_generate_private_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: + s3_client = get_s3_client() + + try: + url = s3_client.generate_presigned_post( + settings.AWS_STORAGE_BUCKET_NAME, + file_path, + Fields={"acl": "private", "Content-Type": file_type}, + Conditions=[{"acl": "private"}, {"Content-Type": file_type}], + ExpiresIn=settings.AWS_FILES_EXPIRY, + ) + + except ClientError as e: + logging.error(e) + return None + + return url + + +def s3_generate_public_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: + s3_client = get_s3_client() + + try: + url = s3_client.generate_presigned_post( + settings.AWS_STORAGE_BUCKET_NAME, + file_path, + Fields={"acl": "public", "Content-Type": file_type}, + Conditions=[{"acl": "public"}, {"Content-Type": file_type}], + ExpiresIn=settings.AWS_FILES_EXPIRY, + ) + + except ClientError as e: + logging.error(e) + return None + + return url From c4149f9a9fe1001869764a6702bb4ccdeb0623a4 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 11:52:18 +0200 Subject: [PATCH 02/36] Drop duplication of "/files" in files urls --- styleguide_example/files/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index 64f4bb23..e0acc518 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -3,9 +3,9 @@ from styleguide_example.files.apis import FileGeneratePrivatePresignedPostApi, FileLocalUploadAPI urlpatterns = [ - path("files/private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), + path("private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), path( - "files//local-upload/", + "/local-upload/", FileLocalUploadAPI.as_view(), ), ] From 421cb5d43f8cb6c9592edbac6819e5a021379579 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 12:09:32 +0200 Subject: [PATCH 03/36] Add file.uploaded_at field --- styleguide_example/files/migrations/0001_initial.py | 3 ++- styleguide_example/files/models.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/styleguide_example/files/migrations/0001_initial.py b/styleguide_example/files/migrations/0001_initial.py index e2454e09..ea4a5092 100644 --- a/styleguide_example/files/migrations/0001_initial.py +++ b/styleguide_example/files/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.9 on 2021-12-06 09:03 +# Generated by Django 3.2.9 on 2021-12-06 10:09 from django.conf import settings from django.db import migrations, models @@ -25,6 +25,7 @@ class Migration(migrations.Migration): ('file', models.FileField(upload_to=styleguide_example.files.utils.file_generate_upload_path)), ('file_name', models.CharField(max_length=255)), ('file_type', models.CharField(max_length=255)), + ('uploaded_at', models.DateTimeField(blank=True, null=True)), ('uploaded_by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), ], options={ diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index 26af7203..470c1701 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -13,6 +13,7 @@ class File(BaseModel): file_name = models.CharField(max_length=255) file_type = models.CharField(max_length=255) + uploaded_at = models.DateTimeField(null=True, blank=True) uploaded_by = models.ForeignKey(BaseUser, on_delete=models.CASCADE) @property From 7fe1d22ff368f7fb4c0bbaad9cfa886347cc6312 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 12:12:54 +0200 Subject: [PATCH 04/36] Add an api for verifying a file upload --- styleguide_example/files/apis.py | 16 ++++++++++++++++ styleguide_example/files/urls.py | 10 +++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index f7644530..8560354d 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -1,3 +1,5 @@ +from datetime import timezone + from django.conf import settings from django.shortcuts import get_object_or_404 from django.core.exceptions import PermissionDenied @@ -36,6 +38,20 @@ def post(self, request, file_id): file = get_object_or_404(File, id=file_id) file.file = request.FILES["file"] + + file.full_clean() + file.save() + + return Response(status=status.HTTP_201_CREATED) + + +class FileVerifyUploadAPI(ApiAuthMixin, APIView): + def post(self, request, file_id): + file = get_object_or_404(File, id=file_id) + + file.uploaded_at = timezone.now() + + file.full_clean() file.save() return Response(status=status.HTTP_201_CREATED) diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index e0acc518..af9576e3 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -1,6 +1,10 @@ from django.urls import path -from styleguide_example.files.apis import FileGeneratePrivatePresignedPostApi, FileLocalUploadAPI +from styleguide_example.files.apis import ( + FileGeneratePrivatePresignedPostApi, + FileLocalUploadAPI, + FileVerifyUploadAPI +) urlpatterns = [ path("private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), @@ -8,4 +12,8 @@ "/local-upload/", FileLocalUploadAPI.as_view(), ), + path( + "/verify-upload/", + FileVerifyUploadAPI.as_view(), + ), ] From a13a9b3bfb2f97fd0ff4f300a57cedbab5c2c7ab Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:03:10 +0200 Subject: [PATCH 05/36] Add media folder to .gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index b6e47617..395352a1 100644 --- a/.gitignore +++ b/.gitignore @@ -127,3 +127,7 @@ dmypy.json # Pyre type checker .pyre/ + + +# media files +/media \ No newline at end of file From 29b870e53fc2f282781af9d09c673fcc14b75575 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:03:25 +0200 Subject: [PATCH 06/36] Setup media root and media dir. Make sure django sees aws settings --- config/django/base.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/django/base.py b/config/django/base.py index 8f371584..55fb7d37 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -175,8 +175,14 @@ SERVER_HOST_DOMAIN = env("SERVER_HOST_DOMAIN", default="http://localhost:8000") +# # Media +MEDIA_ROOT = os.path.join(BASE_DIR, "media") +MEDIA_URL = "/media/" + + from config.settings.cors import * # noqa from config.settings.jwt import * # noqa from config.settings.sessions import * # noqa from config.settings.celery import * # noqa from config.settings.sentry import * # noqa +from config.settings.aws import * # noqa From 4d39c2eac0c3f3a5f09e9da62273e9a4649c81a5 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:03:56 +0200 Subject: [PATCH 07/36] Import timezone from django.utils --- styleguide_example/files/apis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 8560354d..36ca79b7 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -1,4 +1,4 @@ -from datetime import timezone +from django.utils import timezone from django.conf import settings from django.shortcuts import get_object_or_404 From 7068073c24291f2407796003757ac64897e3b34e Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:04:16 +0200 Subject: [PATCH 08/36] Send session key as auth headers when uploading files locally --- styleguide_example/files/apis.py | 2 +- styleguide_example/files/services.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 36ca79b7..9c26526d 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -24,7 +24,7 @@ def post(self, request, *args, **kwargs): serializer.is_valid(raise_exception=True) presigned_data = file_generate_private_presigned_post_data( - user=request.user, **serializer.validated_data + request=request, **serializer.validated_data ) return Response(data=presigned_data) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 82e3cc2e..255f5940 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -26,7 +26,9 @@ def file_create_for_upload(*, user: BaseUser, file_name: str, file_type: str) -> @transaction.atomic -def file_generate_private_presigned_post_data(*, user: BaseUser, file_name: str, file_type: str): +def file_generate_private_presigned_post_data(*, request, file_name: str, file_type: str): + user = request.user + file = file_create_for_upload(user=user, file_name=file_name, file_type=file_type) if settings.USE_S3_UPLOAD: @@ -43,9 +45,12 @@ def file_generate_private_presigned_post_data(*, user: BaseUser, file_name: str, file.file = file.file.field.attr_class(file, file.file.field, upload_path) file.save() else: + """ + Use "Token {user.auth_token} if you're using Token Authentication + """ presigned_data = { "url": file_generate_local_upload_url(file_id=file.id), - "params": {"headers": {"Authorization": f"Token {user.auth_token}"}}, + "params": {"headers": {"Authorization": f"Session {request.session.session_key}"}}, } return {"identifier": file.id, **presigned_data} From 3a5436917dd39473614421004c5e71b0662b5298 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:04:57 +0200 Subject: [PATCH 09/36] MAke file field optional --- styleguide_example/files/migrations/0001_initial.py | 4 ++-- styleguide_example/files/models.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/styleguide_example/files/migrations/0001_initial.py b/styleguide_example/files/migrations/0001_initial.py index ea4a5092..5ce878d0 100644 --- a/styleguide_example/files/migrations/0001_initial.py +++ b/styleguide_example/files/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.9 on 2021-12-06 10:09 +# Generated by Django 3.2.9 on 2021-12-06 15:55 from django.conf import settings from django.db import migrations, models @@ -22,7 +22,7 @@ class Migration(migrations.Migration): ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('created_at', models.DateTimeField(db_index=True, default=django.utils.timezone.now)), ('updated_at', models.DateTimeField(auto_now=True)), - ('file', models.FileField(upload_to=styleguide_example.files.utils.file_generate_upload_path)), + ('file', models.FileField(blank=True, null=True, upload_to=styleguide_example.files.utils.file_generate_upload_path)), ('file_name', models.CharField(max_length=255)), ('file_type', models.CharField(max_length=255)), ('uploaded_at', models.DateTimeField(blank=True, null=True)), diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index 470c1701..09f12db5 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -9,7 +9,7 @@ class File(BaseModel): - file = models.FileField(upload_to=file_generate_upload_path) + file = models.FileField(upload_to=file_generate_upload_path, null=True, blank=True) file_name = models.CharField(max_length=255) file_type = models.CharField(max_length=255) From d3a234954af49dd8f1819082cd612e376ad76bed Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:05:08 +0200 Subject: [PATCH 10/36] Accept integers in url params instead of uuids - All models shuuld have uuids as ids by default It is a matter of time we fix this in the styleguide --- styleguide_example/files/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index af9576e3..60ac4879 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -9,11 +9,11 @@ urlpatterns = [ path("private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), path( - "/local-upload/", + "/local-upload/", FileLocalUploadAPI.as_view(), ), path( - "/verify-upload/", + "/verify-upload/", FileVerifyUploadAPI.as_view(), ), ] From 31b9ea2c01023d58fef8b569dadbba1861ffd75a Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:06:03 +0200 Subject: [PATCH 11/36] Fix generating an upload url --- styleguide_example/files/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/styleguide_example/files/utils.py b/styleguide_example/files/utils.py index 3b5d15a1..28be4d60 100644 --- a/styleguide_example/files/utils.py +++ b/styleguide_example/files/utils.py @@ -10,4 +10,4 @@ def file_generate_upload_path(instance, filename): def file_generate_local_upload_url(*, file_id: str): - return f"{settings.SERVER_HOST_DOMAIN}/api/files/images/{file_id}/local-upload/" + return f"{settings.SERVER_HOST_DOMAIN}/api/files/{file_id}/local-upload/" From c42ef6e454d9281a3ad014a0ed38827b3b195cc0 Mon Sep 17 00:00:00 2001 From: kbadova Date: Mon, 6 Dec 2021 18:09:44 +0200 Subject: [PATCH 12/36] Make sure uploaded files can be served by the BE --- config/urls.py | 4 +++- styleguide_example/files/admin.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config/urls.py b/config/urls.py index 105b2bd1..a72499ea 100644 --- a/config/urls.py +++ b/config/urls.py @@ -14,9 +14,11 @@ 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) """ from django.contrib import admin +from django.conf import settings from django.urls import path, include +from django.conf.urls.static import static urlpatterns = [ path('admin/', admin.site.urls), path('api/', include(('styleguide_example.api.urls', 'api'))), -] +] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/styleguide_example/files/admin.py b/styleguide_example/files/admin.py index cca33435..a4d4f481 100644 --- a/styleguide_example/files/admin.py +++ b/styleguide_example/files/admin.py @@ -5,6 +5,6 @@ @admin.register(File) class FileAdmin(admin.ModelAdmin): - list_display = ["id", "file_name"] + list_display = ["id", "file_name", "url"] ordering = ["-created_at"] From a15ac6c225393d7d9cee4651234657b9b0f9005a Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 3 Apr 2022 17:13:00 +0300 Subject: [PATCH 13/36] Iteration 1: Local file upload via Django admin --- styleguide_example/files/admin.py | 64 +++++++++++++++++- .../files/migrations/0001_initial.py | 9 +-- styleguide_example/files/models.py | 29 ++++++-- styleguide_example/files/services.py | 66 +++++++++++++++++++ styleguide_example/files/utils.py | 12 +++- 5 files changed, 167 insertions(+), 13 deletions(-) diff --git a/styleguide_example/files/admin.py b/styleguide_example/files/admin.py index a4d4f481..c7b516c9 100644 --- a/styleguide_example/files/admin.py +++ b/styleguide_example/files/admin.py @@ -1,10 +1,70 @@ -from django.contrib import admin +from django import forms + +from django.contrib import admin, messages +from django.core.exceptions import ValidationError from styleguide_example.files.models import File +from styleguide_example.files.services import ( + file_create_for_direct_upload, + file_update_for_direct_upload +) + + +class FileForm(forms.ModelForm): + class Meta: + model = File + fields = ["file", "uploaded_by"] @admin.register(File) class FileAdmin(admin.ModelAdmin): - list_display = ["id", "file_name", "url"] + list_display = [ + "id", + "original_file_name", + "file_name", + "file_type", + "url", + "uploaded_by", + "created_at", + "upload_finished_at", + "is_valid", + ] + list_select_related = ["uploaded_by"] ordering = ["-created_at"] + + def get_form(self, request, obj=None, **kwargs): + # That's a bit of a hack + # Dynamically change self.form, before delegating to the actual ModelAdmin.get_form + # Proper kwargs are form, fields, exclude, formfield_callback + if obj is None: + self.form = FileForm + + return super().get_form(request, obj, **kwargs) + + readonly_fields = ( + "original_file_name", + "file_name", + "file_type", + "created_at", + "updated_at", + "upload_finished_at" + ) + + def save_model(self, request, obj, form, change): + try: + cleaned_data = form.cleaned_data + + if change: + file_update_for_direct_upload( + file=obj, + file_object=cleaned_data["file"], + user=cleaned_data["uploaded_by"] + ) + else: + file_create_for_direct_upload( + file_object=cleaned_data["file"], + user=cleaned_data["uploaded_by"] + ) + except ValidationError as exc: + self.message_user(request, str(exc), messages.ERROR) diff --git a/styleguide_example/files/migrations/0001_initial.py b/styleguide_example/files/migrations/0001_initial.py index 5ce878d0..c4a54155 100644 --- a/styleguide_example/files/migrations/0001_initial.py +++ b/styleguide_example/files/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.9 on 2021-12-06 15:55 +# Generated by Django 3.2.12 on 2022-04-03 13:31 from django.conf import settings from django.db import migrations, models @@ -23,10 +23,11 @@ class Migration(migrations.Migration): ('created_at', models.DateTimeField(db_index=True, default=django.utils.timezone.now)), ('updated_at', models.DateTimeField(auto_now=True)), ('file', models.FileField(blank=True, null=True, upload_to=styleguide_example.files.utils.file_generate_upload_path)), - ('file_name', models.CharField(max_length=255)), + ('original_file_name', models.TextField()), + ('file_name', models.CharField(max_length=255, unique=True)), ('file_type', models.CharField(max_length=255)), - ('uploaded_at', models.DateTimeField(blank=True, null=True)), - ('uploaded_by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ('upload_finished_at', models.DateTimeField(blank=True, null=True)), + ('uploaded_by', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), ], options={ 'abstract': False, diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index 09f12db5..a82ae6ff 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -1,3 +1,4 @@ + from django.db import models from django.conf import settings @@ -5,16 +6,36 @@ from styleguide_example.users.models import BaseUser -from styleguide_example.files.utils import file_generate_upload_path +from styleguide_example.files.utils import ( + file_generate_upload_path +) class File(BaseModel): file = models.FileField(upload_to=file_generate_upload_path, null=True, blank=True) - file_name = models.CharField(max_length=255) + + original_file_name = models.TextField() + + file_name = models.CharField(max_length=255, unique=True) file_type = models.CharField(max_length=255) - uploaded_at = models.DateTimeField(null=True, blank=True) - uploaded_by = models.ForeignKey(BaseUser, on_delete=models.CASCADE) + # As a specific behavior, + # We might want to preserve files after the uploader has been deleted. + # In case you want to delete the files too, use models.CASCADE & drop the null=True + uploaded_by = models.ForeignKey( + BaseUser, + null=True, + on_delete=models.SET_NULL + ) + + upload_finished_at = models.DateTimeField(blank=True, null=True) + + @property + def is_valid(self): + """ + We consider a file "valid" if the the datetime flag has value. + """ + return bool(self.upload_finished_at) @property def url(self): diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 255f5940..246a06ee 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -1,5 +1,8 @@ +import mimetypes + from django.conf import settings from django.db import transaction +from django.utils import timezone from styleguide_example.files.models import File from styleguide_example.files.utils import ( @@ -11,6 +14,69 @@ from styleguide_example.users.models import BaseUser +from styleguide_example.files.utils import file_generate_name + + +def file_create_for_direct_upload( + *, + user: BaseUser, + file_object, + file_name: str = "", + file_type: str = "", +) -> File: + if not file_name: + file_name = file_object.name + + if not file_type: + file_type, encoding = mimetypes.guess_type(file_name) + + if file_type is None: + file_type = "" + + obj = File( + file=file_object, + original_file_name=file_name, + file_name=file_generate_name(file_name), + file_type=file_type, + uploaded_by=user, + upload_finished_at=timezone.now() + ) + + obj.full_clean() + obj.save() + + return obj + + +def file_update_for_direct_upload( + *, + file: File, + user: BaseUser, + file_object, + file_name: str = "", + file_type: str = "", +) -> File: + if not file_name: + file_name = file_object.name + + if not file_type: + file_type, encoding = mimetypes.guess_type(file_name) + + if file_type is None: + file_type = "" + + file.file = file_object + file.original_file_name = file_name + file.file_name = file_generate_name(file_name) + file.file_type = file_type + file.uploaded_by = user + file.upload_finished_at = timezone.now() + + file.full_clean() + file.save() + + return file + def file_create_for_upload(*, user: BaseUser, file_name: str, file_type: str) -> File: image = File( diff --git a/styleguide_example/files/utils.py b/styleguide_example/files/utils.py index 28be4d60..c7ff9aa9 100644 --- a/styleguide_example/files/utils.py +++ b/styleguide_example/files/utils.py @@ -1,12 +1,18 @@ import pathlib +from uuid import uuid4 + from django.conf import settings -def file_generate_upload_path(instance, filename): - extension = pathlib.Path(filename).suffix +def file_generate_name(original_file_name): + extension = pathlib.Path(original_file_name).suffix + + return f"{uuid4().hex}{extension}" - return f"files/{instance.id}{extension}" + +def file_generate_upload_path(instance, filename): + return f"files/{instance.file_name}" def file_generate_local_upload_url(*, file_id: str): From 94e72c966aefd4eb02dc6f185cb3e5068bd73f15 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Sun, 3 Apr 2022 18:27:49 +0300 Subject: [PATCH 14/36] Iteration 2: S3 file upload via Django admin - Using `django-storages` --- config/django/base.py | 10 ++++------ config/settings/files_and_storages.py | 27 +++++++++++++++++++++++++++ requirements/base.txt | 1 + styleguide_example/files/models.py | 2 +- 4 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 config/settings/files_and_storages.py diff --git a/config/django/base.py b/config/django/base.py index 55fb7d37..c9025511 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -173,16 +173,14 @@ 'DEFAULT_AUTHENTICATION_CLASSES': [] } +# TODO: Think of a better name? SERVER_HOST_DOMAIN = env("SERVER_HOST_DOMAIN", default="http://localhost:8000") -# # Media -MEDIA_ROOT = os.path.join(BASE_DIR, "media") -MEDIA_URL = "/media/" - - from config.settings.cors import * # noqa from config.settings.jwt import * # noqa from config.settings.sessions import * # noqa from config.settings.celery import * # noqa from config.settings.sentry import * # noqa -from config.settings.aws import * # noqa + +# from config.settings.aws import * # noqa +from config.settings.files_and_storages import * # noqa diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py new file mode 100644 index 00000000..44ba3923 --- /dev/null +++ b/config/settings/files_and_storages.py @@ -0,0 +1,27 @@ +import os + +from config.env import env, environ + +# TODO: Dedup +BASE_DIR = environ.Path(__file__) - 3 + +# direct | pass-thru +FILE_UPLOAD_STRATEGY = env("FILE_UPLOAD_STRATEGY", default="direct") +# local | s3 +FILE_UPLOAD_STORAGE = env("FILE_UPLOAD_STORAGE", default="local") + +if FILE_UPLOAD_STORAGE == "local": + MEDIA_ROOT_NAME = "media" + MEDIA_ROOT = os.path.join(BASE_DIR, MEDIA_ROOT_NAME) + MEDIA_URL = f"/{MEDIA_ROOT_NAME}/" + +if FILE_UPLOAD_STORAGE == "s3": + # Using django-storages + # https://django-storages.readthedocs.io/en/latest/backends/amazon-S3.html + DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' + + AWS_S3_ACCESS_KEY_ID = env("AWS_S3_ACCESS_KEY_ID") + AWS_S3_SECRET_ACCESS_KEY = env("AWS_S3_SECRET_ACCESS_KEY") + AWS_STORAGE_BUCKET_NAME = env("AWS_STORAGE_BUCKET_NAME") + AWS_S3_REGION_NAME = env("AWS_S3_REGION_NAME") + AWS_S3_SIGNATURE_VERSION = env("AWS_S3_SIGNATURE_VERSION", default="s3v4") diff --git a/requirements/base.txt b/requirements/base.txt index 9ba30455..15e86ed9 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -12,6 +12,7 @@ whitenoise==6.0.0 django-filter==21.1 django-extensions==3.1.5 django-cors-headers==3.10.0 +django-storages==1.12.3 drf-jwt==1.19.2 diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index a82ae6ff..61ea3fbd 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -39,7 +39,7 @@ def is_valid(self): @property def url(self): - if settings.USE_S3_UPLOAD: + if settings.FILE_UPLOAD_STORAGE == "s3": return self.file.url return f"{settings.SERVER_HOST_DOMAIN}{self.file.url}" From 356537ec6edf10ec605d8a3d362d2c9509ad4df4 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 12:05:03 +0300 Subject: [PATCH 15/36] Iteration 3: API for direct upload --- styleguide_example/files/apis.py | 34 +++++++++++++++----------------- styleguide_example/files/urls.py | 10 ++++------ 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 9c26526d..77fe846f 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -1,19 +1,32 @@ from django.utils import timezone -from django.conf import settings from django.shortcuts import get_object_or_404 -from django.core.exceptions import PermissionDenied from rest_framework import serializers, status from rest_framework.response import Response from rest_framework.views import APIView from styleguide_example.files.models import File -from styleguide_example.files.services import file_generate_private_presigned_post_data +from styleguide_example.files.services import ( + file_create_for_direct_upload, + file_generate_private_presigned_post_data +) from styleguide_example.api.mixins import ApiAuthMixin +class FileDirectUploadApi(ApiAuthMixin, APIView): + def post(self, request): + file_object = request.FILES["file"] + + file = file_create_for_direct_upload( + file_object=file_object, + user=request.user + ) + + return Response(data={"id": file.id}, status=status.HTTP_201_CREATED) + + class FileGeneratePrivatePresignedPostApi(ApiAuthMixin, APIView): class InputSerializer(serializers.Serializer): file_name = serializers.CharField() @@ -30,21 +43,6 @@ def post(self, request, *args, **kwargs): return Response(data=presigned_data) -class FileLocalUploadAPI(ApiAuthMixin, APIView): - def post(self, request, file_id): - if settings.USE_S3_UPLOAD: - raise PermissionDenied('USE_S3_UPLOAD is enabled. Access to this API is forbidden.') - - file = get_object_or_404(File, id=file_id) - - file.file = request.FILES["file"] - - file.full_clean() - file.save() - - return Response(status=status.HTTP_201_CREATED) - - class FileVerifyUploadAPI(ApiAuthMixin, APIView): def post(self, request, file_id): file = get_object_or_404(File, id=file_id) diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index 60ac4879..51b15cf4 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -1,17 +1,15 @@ from django.urls import path from styleguide_example.files.apis import ( + FileDirectUploadApi, + FileGeneratePrivatePresignedPostApi, - FileLocalUploadAPI, - FileVerifyUploadAPI + FileVerifyUploadAPI, ) urlpatterns = [ + path("upload/direct/", FileDirectUploadApi.as_view()), path("private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), - path( - "/local-upload/", - FileLocalUploadAPI.as_view(), - ), path( "/verify-upload/", FileVerifyUploadAPI.as_view(), From d315a7b1dafadde1998201227775c4227de76f59 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 14:59:58 +0300 Subject: [PATCH 16/36] Iteration 4: API for pass-thru upload + s3 --- config/django/base.py | 1 - config/settings/aws.py | 22 ------ config/settings/files_and_storages.py | 5 ++ styleguide_example/files/apis.py | 34 ++++++---- styleguide_example/files/services.py | 60 ++++++++++++++++- styleguide_example/files/urls.py | 12 ++-- styleguide_example/integrations/aws/client.py | 67 +++++++------------ 7 files changed, 114 insertions(+), 87 deletions(-) delete mode 100644 config/settings/aws.py diff --git a/config/django/base.py b/config/django/base.py index c9025511..47772f45 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -182,5 +182,4 @@ from config.settings.celery import * # noqa from config.settings.sentry import * # noqa -# from config.settings.aws import * # noqa from config.settings.files_and_storages import * # noqa diff --git a/config/settings/aws.py b/config/settings/aws.py deleted file mode 100644 index c61089c4..00000000 --- a/config/settings/aws.py +++ /dev/null @@ -1,22 +0,0 @@ -from config.env import env - -DEFAULT_FILE_STORAGE = env( - "DEFAULT_FILE_STORAGE", - default="django.core.files.storage.FileSystemStorage", -) - -USE_S3_UPLOAD = env("USE_S3_UPLOAD", default=False) - -if USE_S3_UPLOAD: - AWS_ACCESS_KEY_ID = env("AWS_ACCESS_KEY_ID") - AWS_SECRET_ACCESS_KEY = env("AWS_SECRET_ACCESS_KEY") - AWS_STORAGE_BUCKET_NAME = env("AWS_STORAGE_BUCKET_NAME") - - AWS_FILES_EXPIRY = 60 * 60 # 1 hour. Change this configuration if needed - - AWS_S3_REGION_NAME = env("AWS_S3_REGION_NAME") - AWS_S3_CUSTOM_DOMAIN = env("AWS_S3_CUSTOM_DOMAIN") - AWS_S3_DOMAIN = ( - AWS_S3_CUSTOM_DOMAIN or f"{AWS_STORAGE_BUCKET_NAME}.s3.amazonaws.com" - ) - MEDIA_URL = f"https://{AWS_S3_DOMAIN}/media/" diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py index 44ba3923..220f56c4 100644 --- a/config/settings/files_and_storages.py +++ b/config/settings/files_and_storages.py @@ -25,3 +25,8 @@ AWS_STORAGE_BUCKET_NAME = env("AWS_STORAGE_BUCKET_NAME") AWS_S3_REGION_NAME = env("AWS_S3_REGION_NAME") AWS_S3_SIGNATURE_VERSION = env("AWS_S3_SIGNATURE_VERSION", default="s3v4") + + # https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html#canned-acl + AWS_DEFAULT_ACL = env("AWS_DEFAULT_ACL", default="private") + + AWS_PRESIGNED_EXPIRY = env.int("AWS_PRESIGNED_EXPIRY", default=10) # seconds diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 77fe846f..d4177c40 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -1,5 +1,3 @@ -from django.utils import timezone - from django.shortcuts import get_object_or_404 from rest_framework import serializers, status @@ -9,7 +7,8 @@ from styleguide_example.files.models import File from styleguide_example.files.services import ( file_create_for_direct_upload, - file_generate_private_presigned_post_data + file_pass_thru_upload_start, + file_pass_thru_upload_finish, ) from styleguide_example.api.mixins import ApiAuthMixin @@ -27,7 +26,7 @@ def post(self, request): return Response(data={"id": file.id}, status=status.HTTP_201_CREATED) -class FileGeneratePrivatePresignedPostApi(ApiAuthMixin, APIView): +class FilePassThruUploadStartApi(ApiAuthMixin, APIView): class InputSerializer(serializers.Serializer): file_name = serializers.CharField() file_type = serializers.CharField() @@ -36,20 +35,29 @@ def post(self, request, *args, **kwargs): serializer = self.InputSerializer(data=request.data) serializer.is_valid(raise_exception=True) - presigned_data = file_generate_private_presigned_post_data( - request=request, **serializer.validated_data + presigned_data = file_pass_thru_upload_start( + user=request.user, + **serializer.validated_data ) return Response(data=presigned_data) -class FileVerifyUploadAPI(ApiAuthMixin, APIView): - def post(self, request, file_id): - file = get_object_or_404(File, id=file_id) +class FilePassThruUploadFinishApi(ApiAuthMixin, APIView): + class InputSerializer(serializers.Serializer): + file_id = serializers.CharField() + + def post(self, request): + serializer = self.InputSerializer(data=request.data) + serializer.is_valid(raise_exception=True) - file.uploaded_at = timezone.now() + file_id = serializer.validated_data["file_id"] - file.full_clean() - file.save() + file = get_object_or_404(File, id=file_id) + + file = file_pass_thru_upload_finish( + file=file, + user=request.user + ) - return Response(status=status.HTTP_201_CREATED) + return Response({"id": file.id}) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 246a06ee..32905abb 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -10,7 +10,7 @@ file_generate_local_upload_url ) -from styleguide_example.integrations.aws.client import s3_generate_private_presigned_post +from styleguide_example.integrations.aws.client import s3_generate_presigned_post from styleguide_example.users.models import BaseUser @@ -92,7 +92,61 @@ def file_create_for_upload(*, user: BaseUser, file_name: str, file_type: str) -> @transaction.atomic -def file_generate_private_presigned_post_data(*, request, file_name: str, file_type: str): +def file_pass_thru_upload_start( + *, + user: BaseUser, + file_name: str, + file_type: str +): + file = File( + original_file_name=file_name, + file_name=file_generate_name(file_name), + file_type=file_type, + uploaded_by=user, + file=None + ) + file.full_clean() + file.save() + + if settings.FILE_UPLOAD_STORAGE == "s3": + upload_path = file_generate_upload_path(file, file.file_name) + + presigned_data = s3_generate_presigned_post( + file_path=upload_path, file_type=file.file_type + ) + + """ + TODO: Why are we doing this? + + Setting the file.file path to be the s3 upload path without uploading the file. + The actual file upload will be done by the FE. + """ + file.file = file.file.field.attr_class(file, file.file.field, upload_path) + file.save() + else: + # direct + pass + + return {"id": file.id, **presigned_data} + + +@transaction.atomic +def file_pass_thru_upload_finish( + *, + user: BaseUser, + file: File +) -> File: + # Potentially, check against user + + file.upload_finished_at = timezone.now() + file.full_clean() + file.save() + + return file + + +@transaction.atomic +def file_generate_presigned_post_data(*, request, file_name: str, file_type: str): user = request.user file = file_create_for_upload(user=user, file_name=file_name, file_type=file_type) @@ -100,7 +154,7 @@ def file_generate_private_presigned_post_data(*, request, file_name: str, file_t if settings.USE_S3_UPLOAD: upload_path = file_generate_upload_path(file, file.file_name) - presigned_data = s3_generate_private_presigned_post( + presigned_data = s3_generate_presigned_post( file_path=upload_path, file_type=file.file_type ) diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index 51b15cf4..ff4500fc 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -3,15 +3,13 @@ from styleguide_example.files.apis import ( FileDirectUploadApi, - FileGeneratePrivatePresignedPostApi, - FileVerifyUploadAPI, + FilePassThruUploadStartApi, + FilePassThruUploadFinishApi, ) + urlpatterns = [ path("upload/direct/", FileDirectUploadApi.as_view()), - path("private-presigned-post/", FileGeneratePrivatePresignedPostApi.as_view()), - path( - "/verify-upload/", - FileVerifyUploadAPI.as_view(), - ), + path("upload/pass-thru/start/", FilePassThruUploadStartApi.as_view()), + path("upload/pass-thru/finish/", FilePassThruUploadFinishApi.as_view()), ] diff --git a/styleguide_example/integrations/aws/client.py b/styleguide_example/integrations/aws/client.py index 1f5e4edb..4c020192 100644 --- a/styleguide_example/integrations/aws/client.py +++ b/styleguide_example/integrations/aws/client.py @@ -1,7 +1,4 @@ -import logging - import boto3 -from botocore.exceptions import ClientError from django.conf import settings from django.core.exceptions import ImproperlyConfigured @@ -9,12 +6,14 @@ from typing import Optional -def get_s3_client(): +def s3_get_client(): required_config = [ - settings.AWS_ACCESS_KEY_ID, - settings.AWS_SECRET_ACCESS_KEY, + settings.AWS_S3_ACCESS_KEY_ID, + settings.AWS_S3_SECRET_ACCESS_KEY, + settings.AWS_S3_REGION_NAME, settings.AWS_STORAGE_BUCKET_NAME, - settings.AWS_FILES_EXPIRY + settings.AWS_DEFAULT_ACL, + settings.AWS_PRESIGNED_EXPIRY ] for config in required_config: @@ -23,44 +22,30 @@ def get_s3_client(): return boto3.client( service_name="s3", - aws_access_key_id=settings.AWS_ACCESS_KEY_ID, - aws_secret_access_key=settings.AWS_SECRET_ACCESS_KEY, + aws_access_key_id=settings.AWS_S3_ACCESS_KEY_ID, + aws_secret_access_key=settings.AWS_S3_SECRET_ACCESS_KEY, + region_name=settings.AWS_S3_REGION_NAME ) -def s3_generate_private_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: - s3_client = get_s3_client() - - try: - url = s3_client.generate_presigned_post( - settings.AWS_STORAGE_BUCKET_NAME, - file_path, - Fields={"acl": "private", "Content-Type": file_type}, - Conditions=[{"acl": "private"}, {"Content-Type": file_type}], - ExpiresIn=settings.AWS_FILES_EXPIRY, - ) - - except ClientError as e: - logging.error(e) - return None - - return url +def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: + s3_client = s3_get_client() + acl = settings.AWS_DEFAULT_ACL + expires_in = settings.AWS_PRESIGNED_EXPIRY -def s3_generate_public_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: - s3_client = get_s3_client() - - try: - url = s3_client.generate_presigned_post( - settings.AWS_STORAGE_BUCKET_NAME, - file_path, - Fields={"acl": "public", "Content-Type": file_type}, - Conditions=[{"acl": "public"}, {"Content-Type": file_type}], - ExpiresIn=settings.AWS_FILES_EXPIRY, - ) - - except ClientError as e: - logging.error(e) - return None + url = s3_client.generate_presigned_post( + settings.AWS_STORAGE_BUCKET_NAME, + file_path, + Fields={ + "acl": acl, + "Content-Type": file_type + }, + Conditions=[ + {"acl": acl}, + {"Content-Type": file_type} + ], + ExpiresIn=expires_in, + ) return url From 0b2b3924cec517d48e218a1f1f047e78e166ade2 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 15:02:27 +0300 Subject: [PATCH 17/36] Update comment on why are we doing this --- styleguide_example/files/services.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 32905abb..48421cee 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -116,10 +116,7 @@ def file_pass_thru_upload_start( ) """ - TODO: Why are we doing this? - - Setting the file.file path to be the s3 upload path without uploading the file. - The actual file upload will be done by the FE. + We are doing this in order to have an associated file for the field. """ file.file = file.file.field.attr_class(file, file.file.field, upload_path) file.save() From f55883731a27b21f42a1f0f997d3731375c17f38 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 15:22:15 +0300 Subject: [PATCH 18/36] Iteration 5: API + Pass-thru + local upload --- styleguide_example/files/apis.py | 16 ++++++++++++ styleguide_example/files/services.py | 38 +++++++++++++++++++++------- styleguide_example/files/urls.py | 36 +++++++++++++++++++++++--- styleguide_example/files/utils.py | 8 +++++- 4 files changed, 84 insertions(+), 14 deletions(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index d4177c40..9cb9f1a1 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -8,6 +8,7 @@ from styleguide_example.files.services import ( file_create_for_direct_upload, file_pass_thru_upload_start, + file_pass_thru_upload_local, file_pass_thru_upload_finish, ) @@ -43,6 +44,21 @@ def post(self, request, *args, **kwargs): return Response(data=presigned_data) +class FilePassThruUploadLocalApi(ApiAuthMixin, APIView): + def post(self, request, file_id): + file = get_object_or_404(File, id=file_id) + + file_object = request.FILES["file"] + + file = file_pass_thru_upload_local( + user=request.user, + file=file, + file_object=file_object + ) + + return Response({"id": file.id}) + + class FilePassThruUploadFinishApi(ApiAuthMixin, APIView): class InputSerializer(serializers.Serializer): file_id = serializers.CharField() diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 48421cee..58199e45 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -108,25 +108,45 @@ def file_pass_thru_upload_start( file.full_clean() file.save() - if settings.FILE_UPLOAD_STORAGE == "s3": - upload_path = file_generate_upload_path(file, file.file_name) + upload_path = file_generate_upload_path(file, file.file_name) + + """ + We are doing this in order to have an associated file for the field. + """ + file.file = file.file.field.attr_class(file, file.file.field, upload_path) + file.save() + presigned_data = {} + + if settings.FILE_UPLOAD_STORAGE == "s3": presigned_data = s3_generate_presigned_post( file_path=upload_path, file_type=file.file_type ) - """ - We are doing this in order to have an associated file for the field. - """ - file.file = file.file.field.attr_class(file, file.file.field, upload_path) - file.save() else: - # direct - pass + presigned_data = { + "url": file_generate_local_upload_url(file_id=file.id), + # "params": {"headers": {"Authorization": f"Session {request.session.session_key}"}}, + } return {"id": file.id, **presigned_data} +@transaction.atomic +def file_pass_thru_upload_local( + *, + user: BaseUser, + file: File, + file_object +) -> File: + # Potentially, check against user + file.file = file_object + file.full_clean() + file.save() + + return file + + @transaction.atomic def file_pass_thru_upload_finish( *, diff --git a/styleguide_example/files/urls.py b/styleguide_example/files/urls.py index ff4500fc..05fa699a 100644 --- a/styleguide_example/files/urls.py +++ b/styleguide_example/files/urls.py @@ -1,15 +1,43 @@ -from django.urls import path +from django.urls import path, include from styleguide_example.files.apis import ( FileDirectUploadApi, FilePassThruUploadStartApi, FilePassThruUploadFinishApi, + FilePassThruUploadLocalApi, ) urlpatterns = [ - path("upload/direct/", FileDirectUploadApi.as_view()), - path("upload/pass-thru/start/", FilePassThruUploadStartApi.as_view()), - path("upload/pass-thru/finish/", FilePassThruUploadFinishApi.as_view()), + path( + "upload/", + include(([ + path( + "direct/", + FileDirectUploadApi.as_view(), + name="direct" + ), + path( + "pass-thru/", + include(([ + path( + "start/", + FilePassThruUploadStartApi.as_view(), + name="start" + ), + path( + "finish/", + FilePassThruUploadFinishApi.as_view(), + name="finish" + ), + path( + "local//", + FilePassThruUploadLocalApi.as_view(), + name="local" + ) + ], "pass-thru")) + ) + ], "upload")) + ) ] diff --git a/styleguide_example/files/utils.py b/styleguide_example/files/utils.py index c7ff9aa9..9feaab5d 100644 --- a/styleguide_example/files/utils.py +++ b/styleguide_example/files/utils.py @@ -2,6 +2,7 @@ from uuid import uuid4 +from django.urls import reverse from django.conf import settings @@ -16,4 +17,9 @@ def file_generate_upload_path(instance, filename): def file_generate_local_upload_url(*, file_id: str): - return f"{settings.SERVER_HOST_DOMAIN}/api/files/{file_id}/local-upload/" + url = reverse( + "api:files:upload:pass-thru:local", + kwargs={"file_id": file_id} + ) + + return f"{settings.SERVER_HOST_DOMAIN}{url}" From 11e525ffed02a0368532e701b44ddc7c170e358e Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 15:23:18 +0300 Subject: [PATCH 19/36] fixup! Iteration 5: API + Pass-thru + local upload --- styleguide_example/files/services.py | 1 - 1 file changed, 1 deletion(-) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 58199e45..1ac5e730 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -126,7 +126,6 @@ def file_pass_thru_upload_start( else: presigned_data = { "url": file_generate_local_upload_url(file_id=file.id), - # "params": {"headers": {"Authorization": f"Session {request.session.session_key}"}}, } return {"id": file.id, **presigned_data} From e8a9bff21c60c259679eb16e20ed452536eb4faf Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 16:35:29 +0300 Subject: [PATCH 20/36] fixup! fixup! Iteration 5: API + Pass-thru + local upload --- styleguide_example/files/services.py | 31 ---------------------------- 1 file changed, 31 deletions(-) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 1ac5e730..d2730197 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -159,34 +159,3 @@ def file_pass_thru_upload_finish( file.save() return file - - -@transaction.atomic -def file_generate_presigned_post_data(*, request, file_name: str, file_type: str): - user = request.user - - file = file_create_for_upload(user=user, file_name=file_name, file_type=file_type) - - if settings.USE_S3_UPLOAD: - upload_path = file_generate_upload_path(file, file.file_name) - - presigned_data = s3_generate_presigned_post( - file_path=upload_path, file_type=file.file_type - ) - - """ - Setting the file.file path to be the s3 upload path without uploading the file. - The actual file upload will be done by the FE. - """ - file.file = file.file.field.attr_class(file, file.file.field, upload_path) - file.save() - else: - """ - Use "Token {user.auth_token} if you're using Token Authentication - """ - presigned_data = { - "url": file_generate_local_upload_url(file_id=file.id), - "params": {"headers": {"Authorization": f"Session {request.session.session_key}"}}, - } - - return {"identifier": file.id, **presigned_data} From 05f0bc8f610beffc30ffe3c856d98d419e7eb555 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 16:51:06 +0300 Subject: [PATCH 21/36] Introduce `FileDirectUploadService` to serve as an example --- styleguide_example/files/admin.py | 51 +++++++------ styleguide_example/files/apis.py | 11 ++- styleguide_example/files/services.py | 105 ++++++++++++--------------- 3 files changed, 81 insertions(+), 86 deletions(-) diff --git a/styleguide_example/files/admin.py b/styleguide_example/files/admin.py index c7b516c9..a77e46bc 100644 --- a/styleguide_example/files/admin.py +++ b/styleguide_example/files/admin.py @@ -5,8 +5,7 @@ from styleguide_example.files.models import File from styleguide_example.files.services import ( - file_create_for_direct_upload, - file_update_for_direct_upload + FileDirectUploadService ) @@ -34,37 +33,45 @@ class FileAdmin(admin.ModelAdmin): ordering = ["-created_at"] def get_form(self, request, obj=None, **kwargs): - # That's a bit of a hack - # Dynamically change self.form, before delegating to the actual ModelAdmin.get_form - # Proper kwargs are form, fields, exclude, formfield_callback + """ + That's a bit of a hack + Dynamically change self.form, before delegating to the actual ModelAdmin.get_form + Proper kwargs are form, fields, exclude, formfield_callback + """ if obj is None: self.form = FileForm return super().get_form(request, obj, **kwargs) - readonly_fields = ( - "original_file_name", - "file_name", - "file_type", - "created_at", - "updated_at", - "upload_finished_at" - ) + def get_readonly_fields(self, request, obj=None): + """ + We want to show those fields only when we have an existing object. + """ + + if obj is not None: + return [ + "original_file_name", + "file_name", + "file_type", + "created_at", + "updated_at", + "upload_finished_at" + ] + + return [] def save_model(self, request, obj, form, change): try: cleaned_data = form.cleaned_data + service = FileDirectUploadService( + file_obj=cleaned_data["file"], + user=cleaned_data["uploaded_by"] + ) + if change: - file_update_for_direct_upload( - file=obj, - file_object=cleaned_data["file"], - user=cleaned_data["uploaded_by"] - ) + service.update(file=obj) else: - file_create_for_direct_upload( - file_object=cleaned_data["file"], - user=cleaned_data["uploaded_by"] - ) + service.create() except ValidationError as exc: self.message_user(request, str(exc), messages.ERROR) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 9cb9f1a1..51202e32 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -6,7 +6,7 @@ from styleguide_example.files.models import File from styleguide_example.files.services import ( - file_create_for_direct_upload, + FileDirectUploadService, file_pass_thru_upload_start, file_pass_thru_upload_local, file_pass_thru_upload_finish, @@ -17,12 +17,11 @@ class FileDirectUploadApi(ApiAuthMixin, APIView): def post(self, request): - file_object = request.FILES["file"] - - file = file_create_for_direct_upload( - file_object=file_object, - user=request.user + service = FileDirectUploadService( + user=request.user, + file_obj=request.FILES["file"] ) + file = service.create() return Response(data={"id": file.id}, status=status.HTTP_201_CREATED) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index d2730197..952aa568 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -1,5 +1,7 @@ import mimetypes +from typing import Tuple + from django.conf import settings from django.db import transaction from django.utils import timezone @@ -17,78 +19,65 @@ from styleguide_example.files.utils import file_generate_name -def file_create_for_direct_upload( - *, - user: BaseUser, - file_object, - file_name: str = "", - file_type: str = "", -) -> File: - if not file_name: - file_name = file_object.name - - if not file_type: - file_type, encoding = mimetypes.guess_type(file_name) +class FileDirectUploadService: + """ + This also serves as an example of a service class, + which encapsulates 2 different behaviors (create & update) under a namespace. - if file_type is None: - file_type = "" + Meaning, we use the class here for: - obj = File( - file=file_object, - original_file_name=file_name, - file_name=file_generate_name(file_name), - file_type=file_type, - uploaded_by=user, - upload_finished_at=timezone.now() - ) + 1. The namespace + 2. The ability to reuse `_infer_file_name_and_type` (which can also be an util) + """ + def __init__(self, user: BaseUser, file_obj): + self.user = user + self.file_obj = file_obj - obj.full_clean() - obj.save() + def _infer_file_name_and_type(self, file_name: str = "", file_type: str = "") -> Tuple[str, str]: + if not file_name: + file_name = self.file_obj.name - return obj + if not file_type: + file_type, encoding = mimetypes.guess_type(file_name) + if file_type is None: + file_type = "" -def file_update_for_direct_upload( - *, - file: File, - user: BaseUser, - file_object, - file_name: str = "", - file_type: str = "", -) -> File: - if not file_name: - file_name = file_object.name + return file_name, file_type - if not file_type: - file_type, encoding = mimetypes.guess_type(file_name) + @transaction.atomic + def create(self, file_name: str = "", file_type: str = "") -> File: + file_name, file_type = self._infer_file_name_and_type(file_name, file_type) - if file_type is None: - file_type = "" + obj = File( + file=self.file_obj, + original_file_name=file_name, + file_name=file_generate_name(file_name), + file_type=file_type, + uploaded_by=self.user, + upload_finished_at=timezone.now() + ) - file.file = file_object - file.original_file_name = file_name - file.file_name = file_generate_name(file_name) - file.file_type = file_type - file.uploaded_by = user - file.upload_finished_at = timezone.now() + obj.full_clean() + obj.save() - file.full_clean() - file.save() + return obj - return file + @transaction.atomic + def update(self, file: File, file_name: str = "", file_type: str = "") -> File: + file_name, file_type = self._infer_file_name_and_type(file_name, file_type) + file.file = self.file_obj + file.original_file_name = file_name + file.file_name = file_generate_name(file_name) + file.file_type = file_type + file.uploaded_by = self.user + file.upload_finished_at = timezone.now() -def file_create_for_upload(*, user: BaseUser, file_name: str, file_type: str) -> File: - image = File( - file_name=file_name, - file_type=file_type, - uploaded_by=user, - file=None - ) - image.full_clean() - image.save() + file.full_clean() + file.save() - return image + return file @transaction.atomic From d328ad04151d8b3903cb2dde434275126645cc71 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 17:23:31 +0300 Subject: [PATCH 22/36] Partially fix `mypy` --- requirements/base.txt | 1 + styleguide_example/files/services.py | 15 ++++++++------- styleguide_example/integrations/aws/__init__.py | 0 styleguide_example/integrations/aws/client.py | 10 +++++----- 4 files changed, 14 insertions(+), 12 deletions(-) create mode 100644 styleguide_example/integrations/aws/__init__.py diff --git a/requirements/base.txt b/requirements/base.txt index 15e86ed9..4cde76b5 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -17,3 +17,4 @@ django-storages==1.12.3 drf-jwt==1.19.2 boto3==1.20.20 +boto3-stubs==1.21.32 diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 952aa568..6d7d794e 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -1,6 +1,6 @@ import mimetypes -from typing import Tuple +from typing import Tuple, Dict, Any from django.conf import settings from django.db import transaction @@ -38,10 +38,12 @@ def _infer_file_name_and_type(self, file_name: str = "", file_type: str = "") -> file_name = self.file_obj.name if not file_type: - file_type, encoding = mimetypes.guess_type(file_name) + guessed_file_type, encoding = mimetypes.guess_type(file_name) - if file_type is None: + if guessed_file_type is None: file_type = "" + else: + file_type = guessed_file_type return file_name, file_type @@ -86,7 +88,7 @@ def file_pass_thru_upload_start( user: BaseUser, file_name: str, file_type: str -): +) -> Dict[str, Any]: file = File( original_file_name=file_name, file_name=file_generate_name(file_name), @@ -105,7 +107,7 @@ def file_pass_thru_upload_start( file.file = file.file.field.attr_class(file, file.file.field, upload_path) file.save() - presigned_data = {} + presigned_data: Dict[str, Any] = {} if settings.FILE_UPLOAD_STORAGE == "s3": presigned_data = s3_generate_presigned_post( @@ -114,7 +116,7 @@ def file_pass_thru_upload_start( else: presigned_data = { - "url": file_generate_local_upload_url(file_id=file.id), + "url": file_generate_local_upload_url(file_id=str(file.id)), } return {"id": file.id, **presigned_data} @@ -142,7 +144,6 @@ def file_pass_thru_upload_finish( file: File ) -> File: # Potentially, check against user - file.upload_finished_at = timezone.now() file.full_clean() file.save() diff --git a/styleguide_example/integrations/aws/__init__.py b/styleguide_example/integrations/aws/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/styleguide_example/integrations/aws/client.py b/styleguide_example/integrations/aws/client.py index 4c020192..6cbf8e81 100644 --- a/styleguide_example/integrations/aws/client.py +++ b/styleguide_example/integrations/aws/client.py @@ -1,10 +1,10 @@ +from typing import Dict, Any + import boto3 from django.conf import settings from django.core.exceptions import ImproperlyConfigured -from typing import Optional - def s3_get_client(): required_config = [ @@ -28,13 +28,13 @@ def s3_get_client(): ) -def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Optional[str]: +def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Dict[str, Any]: s3_client = s3_get_client() acl = settings.AWS_DEFAULT_ACL expires_in = settings.AWS_PRESIGNED_EXPIRY - url = s3_client.generate_presigned_post( + presigned_data = s3_client.generate_presigned_post( settings.AWS_STORAGE_BUCKET_NAME, file_path, Fields={ @@ -48,4 +48,4 @@ def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Optional[st ExpiresIn=expires_in, ) - return url + return presigned_data From 50cd1a0d0c8aa0f1c52290a4d9def3c4bd1f3967 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 17:28:30 +0300 Subject: [PATCH 23/36] Move stubs to local file --- requirements/base.txt | 1 - requirements/local.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/base.txt b/requirements/base.txt index 4cde76b5..15e86ed9 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -17,4 +17,3 @@ django-storages==1.12.3 drf-jwt==1.19.2 boto3==1.20.20 -boto3-stubs==1.21.32 diff --git a/requirements/local.txt b/requirements/local.txt index f9a04974..4986bc3f 100644 --- a/requirements/local.txt +++ b/requirements/local.txt @@ -14,3 +14,4 @@ ipython==8.2.0 mypy==0.942 django-stubs==1.9.0 djangorestframework-stubs==1.4.0 +boto3-stubs==1.21.32 From a337a922d37944942c0271d0b3dc4580f080cd13 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 17:44:47 +0300 Subject: [PATCH 24/36] Add `mypy.ini` --- mypy.ini | 35 +++++++++++++++++++++++++++++++++++ setup.cfg | 36 ------------------------------------ 2 files changed, 35 insertions(+), 36 deletions(-) create mode 100644 mypy.ini diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 00000000..759f4e23 --- /dev/null +++ b/mypy.ini @@ -0,0 +1,35 @@ +[mypy] +plugins = + mypy_django_plugin.main, + mypy_drf_plugin.main + +[mypy.plugins.django-stubs] +django_settings_module = "config.django.base" + +[mypy-config.*] +# Ignore everything related to Django config +ignore_errors = true + +[mypy-styleguide_example.*.migrations.*] +# Ignore Django migrations +ignore_errors = true + +[mypy-celery.*] +# Remove this when celery stubs are present +ignore_missing_imports = True + +[mypy-django_celery_beat.*] +# Remove this when django_celery_beat stubs are present +ignore_missing_imports = True + +[mypy-django_filters.*] +# Remove this when django_filters stubs are present +ignore_missing_imports = True + +[mypy-factory.*] +# Remove this when factory stubs are present +ignore_missing_imports = True + +[mypy-rest_framework_jwt.*] +# Remove this when rest_framework_jwt stubs are present +ignore_missing_imports = True diff --git a/setup.cfg b/setup.cfg index 230cafbd..3f7883e1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,39 +4,3 @@ exclude = .git, __pycache__, */migrations/* - -[mypy] -plugins = - mypy_django_plugin.main, - mypy_drf_plugin.main - -[mypy.plugins.django-stubs] -django_settings_module = "config.django.base" - -[mypy-config.*] -# Ignore everything related to Django config -ignore_errors = true - -[mypy-styleguide_example.*.migrations.*] -# Ignore Django migrations -ignore_errors = true - -[mypy-celery.*] -# Remove this when celery stubs are present -ignore_missing_imports = True - -[mypy-django_celery_beat.*] -# Remove this when django_celery_beat stubs are present -ignore_missing_imports = True - -[mypy-django_filters.*] -# Remove this when django_filters stubs are present -ignore_missing_imports = True - -[mypy-factory.*] -# Remove this when factory stubs are present -ignore_missing_imports = True - -[mypy-rest_framework_jwt.*] -# Remove this when rest_framework_jwt stubs are present -ignore_missing_imports = True From e0383bdd1785dfa7cb9ced045f0dabc2a53197a3 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 17:51:45 +0300 Subject: [PATCH 25/36] Explicitly specify mypy config --- .github/workflows/django.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/django.yml b/.github/workflows/django.yml index 56d1b3c0..3ec3d2ed 100644 --- a/.github/workflows/django.yml +++ b/.github/workflows/django.yml @@ -8,7 +8,7 @@ jobs: - name: Build docker run: docker-compose build - name: Type check - run: docker-compose run django mypy styleguide_example/ + run: docker-compose run django mypy --config mypy.ini styleguide_example/ - name: Run migrations run: docker-compose run django python manage.py migrate - name: Run tests @@ -38,7 +38,7 @@ jobs: python -m pip install --upgrade pip pip install -r requirements/local.txt - name: Type check - run: mypy styleguide_example/ + run: mypy --config mypy.ini styleguide_example/ - name: Run migrations run: python manage.py migrate - name: Run tests From e652bb72e6904c18fdac84158cfb26e8cf1011e5 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Mon, 4 Apr 2022 17:54:40 +0300 Subject: [PATCH 26/36] Check mypy version --- .github/workflows/django.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/django.yml b/.github/workflows/django.yml index 3ec3d2ed..5ad021b2 100644 --- a/.github/workflows/django.yml +++ b/.github/workflows/django.yml @@ -38,7 +38,9 @@ jobs: python -m pip install --upgrade pip pip install -r requirements/local.txt - name: Type check - run: mypy --config mypy.ini styleguide_example/ + run: | + mypy --version + mypy --config mypy.ini styleguide_example/ - name: Run migrations run: python manage.py migrate - name: Run tests From 315db5f4a23a8ac0e6c387dea16a8b6a8f58fe6c Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 09:43:41 +0300 Subject: [PATCH 27/36] Introduce `S3Credentials` --- requirements/base.txt | 1 + styleguide_example/common/utils.py | 31 ++++++++- styleguide_example/integrations/aws/client.py | 67 +++++++++++++------ 3 files changed, 77 insertions(+), 22 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 15e86ed9..b374e384 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -17,3 +17,4 @@ django-storages==1.12.3 drf-jwt==1.19.2 boto3==1.20.20 +attrs==21.4.0 diff --git a/styleguide_example/common/utils.py b/styleguide_example/common/utils.py index 89849a77..88ce46b3 100644 --- a/styleguide_example/common/utils.py +++ b/styleguide_example/common/utils.py @@ -1,7 +1,9 @@ -from rest_framework import serializers - +from django.conf import settings from django.shortcuts import get_object_or_404 from django.http import Http404 +from django.core.exceptions import ImproperlyConfigured + +from rest_framework import serializers def make_mock_object(**kwargs): @@ -30,3 +32,28 @@ def inline_serializer(*, fields, data=None, **kwargs): return serializer_class(data=data, **kwargs) return serializer_class(**kwargs) + + +def assert_settings(required_settings, error_message_prefix=""): + """ + Checks if each item from `required_settings` is present in Django settings + """ + not_present = [] + values = {} + + for required_setting in required_settings: + if not hasattr(settings, required_setting): + not_present.append(required_setting) + continue + + values[required_setting] = getattr(settings, required_setting) + + if not_present: + if not error_message_prefix: + error_message_prefix = "Required settings not found." + + stringified_not_present = ", ".join(not_present) + + raise ImproperlyConfigured(f"{error_message_prefix} Could not find: {stringified_not_present}") + + return values diff --git a/styleguide_example/integrations/aws/client.py b/styleguide_example/integrations/aws/client.py index 6cbf8e81..f3663b99 100644 --- a/styleguide_example/integrations/aws/client.py +++ b/styleguide_example/integrations/aws/client.py @@ -1,41 +1,68 @@ from typing import Dict, Any +from functools import lru_cache + +from attrs import define + import boto3 -from django.conf import settings -from django.core.exceptions import ImproperlyConfigured +from styleguide_example.common.utils import assert_settings + + +@define +class S3Credentials: + access_key_id: str + secret_access_key: str + region_name: str + bucket_name: str + default_acl: str + presigned_expiry: int + + +@lru_cache +def s3_get_credentials() -> S3Credentials: + required_config = assert_settings( + [ + "AWS_S3_ACCESS_KEY_ID", + "AWS_S3_SECRET_ACCESS_KEY", + "AWS_S3_REGION_NAME", + "AWS_STORAGE_BUCKET_NAME", + "AWS_DEFAULT_ACL", + "AWS_PRESIGNED_EXPIRY" + ], + "S3 credentials not found." + ) + + return S3Credentials( + access_key_id=required_config["AWS_S3_ACCESS_KEY_ID"], + secret_access_key=required_config["AWS_S3_SECRET_ACCESS_KEY"], + region_name=required_config["AWS_S3_REGION_NAME"], + bucket_name=required_config["AWS_STORAGE_BUCKET_NAME"], + default_acl=required_config["AWS_DEFAULT_ACL"], + presigned_expiry=required_config["AWS_PRESIGNED_EXPIRY"] + ) def s3_get_client(): - required_config = [ - settings.AWS_S3_ACCESS_KEY_ID, - settings.AWS_S3_SECRET_ACCESS_KEY, - settings.AWS_S3_REGION_NAME, - settings.AWS_STORAGE_BUCKET_NAME, - settings.AWS_DEFAULT_ACL, - settings.AWS_PRESIGNED_EXPIRY - ] - - for config in required_config: - if not config: - raise ImproperlyConfigured(f'AWS not configured. Missing {config}.') + credentials = s3_get_credentials() return boto3.client( service_name="s3", - aws_access_key_id=settings.AWS_S3_ACCESS_KEY_ID, - aws_secret_access_key=settings.AWS_S3_SECRET_ACCESS_KEY, - region_name=settings.AWS_S3_REGION_NAME + aws_access_key_id=credentials.access_key_id, + aws_secret_access_key=credentials.secret_access_key, + region_name=credentials.region_name ) def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Dict[str, Any]: + credentials = s3_get_credentials() s3_client = s3_get_client() - acl = settings.AWS_DEFAULT_ACL - expires_in = settings.AWS_PRESIGNED_EXPIRY + acl = credentials.default_acl + expires_in = credentials.presigned_expiry presigned_data = s3_client.generate_presigned_post( - settings.AWS_STORAGE_BUCKET_NAME, + credentials.bucket_name, file_path, Fields={ "acl": acl, From 079561eeaf610f296ac940e733fb021ab27f424a Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 09:57:22 +0300 Subject: [PATCH 28/36] Introduce `FilePassThruUploadService` --- styleguide_example/files/apis.py | 23 ++--- styleguide_example/files/services.py | 125 +++++++++++++-------------- 2 files changed, 67 insertions(+), 81 deletions(-) diff --git a/styleguide_example/files/apis.py b/styleguide_example/files/apis.py index 51202e32..75311423 100644 --- a/styleguide_example/files/apis.py +++ b/styleguide_example/files/apis.py @@ -7,9 +7,7 @@ from styleguide_example.files.models import File from styleguide_example.files.services import ( FileDirectUploadService, - file_pass_thru_upload_start, - file_pass_thru_upload_local, - file_pass_thru_upload_finish, + FilePassThruUploadService ) from styleguide_example.api.mixins import ApiAuthMixin @@ -35,10 +33,8 @@ def post(self, request, *args, **kwargs): serializer = self.InputSerializer(data=request.data) serializer.is_valid(raise_exception=True) - presigned_data = file_pass_thru_upload_start( - user=request.user, - **serializer.validated_data - ) + service = FilePassThruUploadService(request.user) + presigned_data = service.start(**serializer.validated_data) return Response(data=presigned_data) @@ -49,11 +45,8 @@ def post(self, request, file_id): file_object = request.FILES["file"] - file = file_pass_thru_upload_local( - user=request.user, - file=file, - file_object=file_object - ) + service = FilePassThruUploadService(request.user) + file = service.upload_local(file=file, file_object=file_object) return Response({"id": file.id}) @@ -70,9 +63,7 @@ def post(self, request): file = get_object_or_404(File, id=file_id) - file = file_pass_thru_upload_finish( - file=file, - user=request.user - ) + service = FilePassThruUploadService(request.user) + service.finish(file=file) return Response({"id": file.id}) diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index 6d7d794e..b429037a 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -9,15 +9,14 @@ from styleguide_example.files.models import File from styleguide_example.files.utils import ( file_generate_upload_path, - file_generate_local_upload_url + file_generate_local_upload_url, + file_generate_name ) from styleguide_example.integrations.aws.client import s3_generate_presigned_post from styleguide_example.users.models import BaseUser -from styleguide_example.files.utils import file_generate_name - class FileDirectUploadService: """ @@ -82,70 +81,66 @@ def update(self, file: File, file_name: str = "", file_type: str = "") -> File: return file -@transaction.atomic -def file_pass_thru_upload_start( - *, - user: BaseUser, - file_name: str, - file_type: str -) -> Dict[str, Any]: - file = File( - original_file_name=file_name, - file_name=file_generate_name(file_name), - file_type=file_type, - uploaded_by=user, - file=None - ) - file.full_clean() - file.save() - - upload_path = file_generate_upload_path(file, file.file_name) - - """ - We are doing this in order to have an associated file for the field. +class FilePassThruUploadService: """ - file.file = file.file.field.attr_class(file, file.file.field, upload_path) - file.save() + This also serves as an example of a service class, + which encapsulates a flow (start & finish) + one-off action (upload_local) into a namespace. - presigned_data: Dict[str, Any] = {} + Meaning, we use the class here for: + + 1. The namespace + """ + def __init__(self, user: BaseUser): + self.user = user - if settings.FILE_UPLOAD_STORAGE == "s3": - presigned_data = s3_generate_presigned_post( - file_path=upload_path, file_type=file.file_type + @transaction.atomic + def start(self, *, file_name: str, file_type: str) -> Dict[str, Any]: + file = File( + original_file_name=file_name, + file_name=file_generate_name(file_name), + file_type=file_type, + uploaded_by=self.user, + file=None ) + file.full_clean() + file.save() - else: - presigned_data = { - "url": file_generate_local_upload_url(file_id=str(file.id)), - } - - return {"id": file.id, **presigned_data} - - -@transaction.atomic -def file_pass_thru_upload_local( - *, - user: BaseUser, - file: File, - file_object -) -> File: - # Potentially, check against user - file.file = file_object - file.full_clean() - file.save() - - return file - - -@transaction.atomic -def file_pass_thru_upload_finish( - *, - user: BaseUser, - file: File -) -> File: - # Potentially, check against user - file.upload_finished_at = timezone.now() - file.full_clean() - file.save() - - return file + upload_path = file_generate_upload_path(file, file.file_name) + + """ + We are doing this in order to have an associated file for the field. + """ + file.file = file.file.field.attr_class(file, file.file.field, upload_path) + file.save() + + presigned_data: Dict[str, Any] = {} + + if settings.FILE_UPLOAD_STORAGE == "s3": + presigned_data = s3_generate_presigned_post( + file_path=upload_path, file_type=file.file_type + ) + + else: + presigned_data = { + "url": file_generate_local_upload_url(file_id=str(file.id)), + } + + return {"id": file.id, **presigned_data} + + @transaction.atomic + def finish(self, *, file: File) -> File: + # Potentially, check against user + file.upload_finished_at = timezone.now() + file.full_clean() + file.save() + + return file + + @transaction.atomic + def upload_local(self, *, file: File, file_object) -> File: + # Potentially, check against user + file.file = file_object + file.full_clean() + file.save() + + return file From f00ebbe7aebf5e7f5ac2f01c6417a64932f9ec57 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 10:45:45 +0300 Subject: [PATCH 29/36] Introduce enums for settings --- config/env.py | 10 ++++++++++ config/settings/files_and_storages.py | 14 +++++++++++--- styleguide_example/files/enums.py | 11 +++++++++++ styleguide_example/files/models.py | 3 ++- styleguide_example/files/services.py | 3 ++- 5 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 styleguide_example/files/enums.py diff --git a/config/env.py b/config/env.py index 463cfb87..fa4ea710 100644 --- a/config/env.py +++ b/config/env.py @@ -1,3 +1,13 @@ +from django.core.exceptions import ImproperlyConfigured + import environ env = environ.Env() + + +def env_to_enum(enum_cls, value): + for x in enum_cls: + if x.value == value: + return x + + raise ImproperlyConfigured(f"Env value {repr(value)} could not be found in {repr(enum_cls)}") diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py index 220f56c4..824640df 100644 --- a/config/settings/files_and_storages.py +++ b/config/settings/files_and_storages.py @@ -1,14 +1,22 @@ import os -from config.env import env, environ +from config.env import env, environ, env_to_enum + +from styleguide_example.files.enums import FileUploadStrategy, FileUploadStorage # TODO: Dedup BASE_DIR = environ.Path(__file__) - 3 # direct | pass-thru -FILE_UPLOAD_STRATEGY = env("FILE_UPLOAD_STRATEGY", default="direct") +FILE_UPLOAD_STRATEGY = env_to_enum( + FileUploadStrategy, + env("FILE_UPLOAD_STRATEGY", default="direct") +) # local | s3 -FILE_UPLOAD_STORAGE = env("FILE_UPLOAD_STORAGE", default="local") +FILE_UPLOAD_STORAGE = env_to_enum( + FileUploadStorage, + env("FILE_UPLOAD_STORAGE", default="local") +) if FILE_UPLOAD_STORAGE == "local": MEDIA_ROOT_NAME = "media" diff --git a/styleguide_example/files/enums.py b/styleguide_example/files/enums.py new file mode 100644 index 00000000..a123137a --- /dev/null +++ b/styleguide_example/files/enums.py @@ -0,0 +1,11 @@ +from enum import Enum + + +class FileUploadStrategy(Enum): + DIRECT = "direct" + PASS_THRU = "pass-thru" + + +class FileUploadStorage(Enum): + LOCAL = "local" + S3 = "s3" diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index 61ea3fbd..122e3741 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -9,6 +9,7 @@ from styleguide_example.files.utils import ( file_generate_upload_path ) +from styleguide_example.files.enums import FileUploadStorage class File(BaseModel): @@ -39,7 +40,7 @@ def is_valid(self): @property def url(self): - if settings.FILE_UPLOAD_STORAGE == "s3": + if settings.FILE_UPLOAD_STORAGE == FileUploadStorage.S3: return self.file.url return f"{settings.SERVER_HOST_DOMAIN}{self.file.url}" diff --git a/styleguide_example/files/services.py b/styleguide_example/files/services.py index b429037a..e055f485 100644 --- a/styleguide_example/files/services.py +++ b/styleguide_example/files/services.py @@ -12,6 +12,7 @@ file_generate_local_upload_url, file_generate_name ) +from styleguide_example.files.enums import FileUploadStorage from styleguide_example.integrations.aws.client import s3_generate_presigned_post @@ -115,7 +116,7 @@ def start(self, *, file_name: str, file_type: str) -> Dict[str, Any]: presigned_data: Dict[str, Any] = {} - if settings.FILE_UPLOAD_STORAGE == "s3": + if settings.FILE_UPLOAD_STORAGE == FileUploadStorage.S3: presigned_data = s3_generate_presigned_post( file_path=upload_path, file_type=file.file_type ) From c5bcd1e3af087807ff3a1faabc12598601bb4e0d Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 10:47:11 +0300 Subject: [PATCH 30/36] fixup! Introduce enums for settings --- config/settings/files_and_storages.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py index 824640df..84373b0c 100644 --- a/config/settings/files_and_storages.py +++ b/config/settings/files_and_storages.py @@ -7,12 +7,10 @@ # TODO: Dedup BASE_DIR = environ.Path(__file__) - 3 -# direct | pass-thru FILE_UPLOAD_STRATEGY = env_to_enum( FileUploadStrategy, env("FILE_UPLOAD_STRATEGY", default="direct") ) -# local | s3 FILE_UPLOAD_STORAGE = env_to_enum( FileUploadStorage, env("FILE_UPLOAD_STORAGE", default="local") From f5427b95c51315b99450fa0a8084c11fd215d803 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:03:43 +0300 Subject: [PATCH 31/36] Move `BASE_DIR` to `config.env` --- config/django/base.py | 5 +---- config/env.py | 2 ++ config/settings/files_and_storages.py | 4 +--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/config/django/base.py b/config/django/base.py index 47772f45..e5abbd65 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -12,10 +12,7 @@ import os -from config.env import env, environ - -# Build paths inside the project like this: os.path.join(BASE_DIR, ...) -BASE_DIR = environ.Path(__file__) - 3 +from config.env import env, BASE_DIR env.read_env(os.path.join(BASE_DIR, ".env")) diff --git a/config/env.py b/config/env.py index fa4ea710..a68f0123 100644 --- a/config/env.py +++ b/config/env.py @@ -4,6 +4,8 @@ env = environ.Env() +BASE_DIR = environ.Path(__file__) - 3 + def env_to_enum(enum_cls, value): for x in enum_cls: diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py index 84373b0c..2f480268 100644 --- a/config/settings/files_and_storages.py +++ b/config/settings/files_and_storages.py @@ -1,11 +1,9 @@ import os -from config.env import env, environ, env_to_enum +from config.env import BASE_DIR, env, env_to_enum from styleguide_example.files.enums import FileUploadStrategy, FileUploadStorage -# TODO: Dedup -BASE_DIR = environ.Path(__file__) - 3 FILE_UPLOAD_STRATEGY = env_to_enum( FileUploadStrategy, From cf7817df2f6831377651c022eb1b1495d74fc507 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:06:43 +0300 Subject: [PATCH 32/36] Rename `SERVER_HOST_DOMAIN` to `APP_DOMAIN` --- config/django/base.py | 3 +-- styleguide_example/files/models.py | 2 +- styleguide_example/files/utils.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/config/django/base.py b/config/django/base.py index e5abbd65..4d037d40 100644 --- a/config/django/base.py +++ b/config/django/base.py @@ -170,8 +170,7 @@ 'DEFAULT_AUTHENTICATION_CLASSES': [] } -# TODO: Think of a better name? -SERVER_HOST_DOMAIN = env("SERVER_HOST_DOMAIN", default="http://localhost:8000") +APP_DOMAIN = env("APP_DOMAIN", default="http://localhost:8000") from config.settings.cors import * # noqa from config.settings.jwt import * # noqa diff --git a/styleguide_example/files/models.py b/styleguide_example/files/models.py index 122e3741..dd11b896 100644 --- a/styleguide_example/files/models.py +++ b/styleguide_example/files/models.py @@ -43,4 +43,4 @@ def url(self): if settings.FILE_UPLOAD_STORAGE == FileUploadStorage.S3: return self.file.url - return f"{settings.SERVER_HOST_DOMAIN}{self.file.url}" + return f"{settings.APP_DOMAIN}{self.file.url}" diff --git a/styleguide_example/files/utils.py b/styleguide_example/files/utils.py index 9feaab5d..992bf515 100644 --- a/styleguide_example/files/utils.py +++ b/styleguide_example/files/utils.py @@ -22,4 +22,4 @@ def file_generate_local_upload_url(*, file_id: str): kwargs={"file_id": file_id} ) - return f"{settings.SERVER_HOST_DOMAIN}{url}" + return f"{settings.APP_DOMAIN}{url}" From 52cc46b99551baa40952f9759e10eecbcd6740ae Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:22:20 +0300 Subject: [PATCH 33/36] Properly calculate `BASE_DIR` --- config/env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/env.py b/config/env.py index a68f0123..ac9a08b1 100644 --- a/config/env.py +++ b/config/env.py @@ -4,7 +4,7 @@ env = environ.Env() -BASE_DIR = environ.Path(__file__) - 3 +BASE_DIR = environ.Path(__file__) - 2 def env_to_enum(enum_cls, value): From 1bf7f2f8e3d6c8ecd3fecfc2399e20c429e83033 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:29:18 +0300 Subject: [PATCH 34/36] Fix enum/string checks --- config/settings/files_and_storages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/settings/files_and_storages.py b/config/settings/files_and_storages.py index 2f480268..f9fe62b6 100644 --- a/config/settings/files_and_storages.py +++ b/config/settings/files_and_storages.py @@ -14,12 +14,12 @@ env("FILE_UPLOAD_STORAGE", default="local") ) -if FILE_UPLOAD_STORAGE == "local": +if FILE_UPLOAD_STORAGE == FileUploadStorage.LOCAL: MEDIA_ROOT_NAME = "media" MEDIA_ROOT = os.path.join(BASE_DIR, MEDIA_ROOT_NAME) MEDIA_URL = f"/{MEDIA_ROOT_NAME}/" -if FILE_UPLOAD_STORAGE == "s3": +if FILE_UPLOAD_STORAGE == FileUploadStorage.S3: # Using django-storages # https://django-storages.readthedocs.io/en/latest/backends/amazon-S3.html DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' From a9fa38d53fcf2704e490c815a9604f3afccd0c66 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:41:25 +0300 Subject: [PATCH 35/36] Add more settings to `.env.example` --- .env.example | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.env.example b/.env.example index cb64cc06..1ac077c3 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,10 @@ PYTHONBREAKPOINT=ipdb.set_trace SENTRY_DSN="" + +FILE_UPLOAD_STRATEGY="direct" # pass-thru +FILE_UPLOAD_STORAGE="local" # s3 + +AWS_S3_ACCESS_KEY_ID="" +AWS_S3_SECRET_ACCESS_KEY="" +AWS_STORAGE_BUCKET_NAME="django-styleguide-example" +AWS_S3_REGION_NAME="eu-central-1" From bc83158a63fb42899ed550f281b39852e53ab4f0 Mon Sep 17 00:00:00 2001 From: Radoslav Georgiev Date: Tue, 5 Apr 2022 14:59:35 +0300 Subject: [PATCH 36/36] Add the shape of the presigned data --- styleguide_example/integrations/aws/client.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/styleguide_example/integrations/aws/client.py b/styleguide_example/integrations/aws/client.py index f3663b99..e12c7c02 100644 --- a/styleguide_example/integrations/aws/client.py +++ b/styleguide_example/integrations/aws/client.py @@ -61,6 +61,24 @@ def s3_generate_presigned_post(*, file_path: str, file_type: str) -> Dict[str, A acl = credentials.default_acl expires_in = credentials.presigned_expiry + """ + TODO: Create a type for the presigned_data + It looks like this: + + { + 'fields': { + 'Content-Type': 'image/png', + 'acl': 'private', + 'key': 'files/bafdccb665a447468e237781154883b5.png', + 'policy': 'some-long-base64-string', + 'x-amz-algorithm': 'AWS4-HMAC-SHA256', + 'x-amz-credential': 'AKIASOZLZI5FJDJ6XTSZ/20220405/eu-central-1/s3/aws4_request', + 'x-amz-date': '20220405T114912Z', + 'x-amz-signature': '7d8be89aabec12b781d44b5b3f099d07be319b9a41d9a9c804bd1075e1ef5735' + }, + 'url': 'https://django-styleguide-example.s3.amazonaws.com/' + } + """ presigned_data = s3_client.generate_presigned_post( credentials.bucket_name, file_path,