From e8951f753a013774da43b008cd99971bdc0de319 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 24 Jun 2026 22:51:28 -0700 Subject: [PATCH 1/2] feat: add GCS resumable upload storage helpers - supports_resumable flag on the GCS storage backends - get_stored_object_md5: dedup lookup against an object's GCS-computed md5 - create_resumable_upload_session: pins md5 + declared-size metadata - hex_to_base64 checksum helper Part of #5975. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UzUP3UYP4cLyouvssXyekj --- .../tests/test_storage_common.py | 39 +++++++++++++------ .../contentcuration/utils/files.py | 31 +++++++++++++++ .../contentcuration/utils/gcs_storage.py | 28 +++++++++++-- .../contentcuration/utils/storage_common.py | 35 ++++------------- 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_storage_common.py b/contentcuration/contentcuration/tests/test_storage_common.py index f89534c194..82feef09ea 100644 --- a/contentcuration/contentcuration/tests/test_storage_common.py +++ b/contentcuration/contentcuration/tests/test_storage_common.py @@ -1,4 +1,3 @@ -import codecs import hashlib from datetime import timedelta from io import BytesIO @@ -12,6 +11,7 @@ from .base import StudioTestCase from contentcuration.models import generate_object_storage_name +from contentcuration.utils.gcs_storage import GoogleCloudStorage from contentcuration.utils.storage_common import _get_gcs_presigned_put_url from contentcuration.utils.storage_common import determine_content_type from contentcuration.utils.storage_common import get_presigned_upload_url @@ -79,7 +79,7 @@ def test_raises_error(self): with pytest.raises(UnknownStorageBackendError): get_presigned_upload_url( "nice", - "err", + "d41d8cd98f00b204e9800998ecf8427e", 5, 0, storage=self.STORAGE, @@ -157,6 +157,23 @@ def test_generate_signed_url_called_with_required_arguments(self): content_type=mimetype, ) + def test_create_resumable_session_pins_md5_size_and_returns_url(self): + storage = GoogleCloudStorage(self.client, "bucket") + blob = self.client.get_bucket.return_value.blob.return_value + blob.create_resumable_upload_session.return_value = "https://session.url" + + url = storage.create_resumable_upload_session( + "storage/a/b/abc.jpg", + "d41d8cd98f00b204e9800998ecf8427e", + 2048, + ) + + assert url == "https://session.url" + assert blob.md5_hash == "1B2M2Y8AsgTpgAmY7PhCfg==" # hex checksum, b64-encoded + assert blob.content_type == "image/jpeg" + assert blob.metadata == {"declared-size": "2048"} + blob.create_resumable_upload_session.assert_called_once() + class S3StoragePresignedURLUnitTestCase(StudioTestCase): """ @@ -177,7 +194,12 @@ def test_returns_string_if_inputs_are_valid(self): # use a real connection here as a sanity check ret = get_presigned_upload_url( - "a/b/abc.jpg", "aBc", 10, 1, storage=self.STORAGE, client=None + "a/b/abc.jpg", + "d41d8cd98f00b204e9800998ecf8427e", + 10, + 1, + storage=self.STORAGE, + client=None, ) url = ret["uploadURL"] @@ -189,19 +211,12 @@ def test_can_upload_file_to_presigned_url(self): """ file_contents = b"blahfilecontents" file = BytesIO(file_contents) - # S3 expects a base64-encoded MD5 checksum - md5 = hashlib.md5(file_contents) - md5_checksum = md5.hexdigest() - md5_checksum_base64 = codecs.encode( - codecs.decode(md5_checksum, "hex"), "base64" - ).decode() + md5_checksum = hashlib.md5(file_contents).hexdigest() filename = "blahfile.jpg" filepath = generate_object_storage_name(md5_checksum, filename) - ret = get_presigned_upload_url( - filepath, md5_checksum_base64, 1000, len(file_contents) - ) + ret = get_presigned_upload_url(filepath, md5_checksum, 1000, len(file_contents)) url = ret["uploadURL"] content_type = ret["mimetype"] diff --git a/contentcuration/contentcuration/utils/files.py b/contentcuration/contentcuration/utils/files.py index 0cb447a601..785faba80b 100644 --- a/contentcuration/contentcuration/utils/files.py +++ b/contentcuration/contentcuration/utils/files.py @@ -1,5 +1,6 @@ import base64 import copy +import mimetypes import os import re import tempfile @@ -17,6 +18,12 @@ from contentcuration.models import File from contentcuration.models import generate_object_storage_name + +# Do this to ensure that we infer mimetypes for files properly, specifically +# zip file and epub files. +# to add additional files add them to the mime.types file +mimetypes.init([os.path.join(os.path.dirname(__file__), "mime.types")]) + ImageFile.LOAD_TRUNCATED_IMAGES = True THUMBNAIL_WIDTH = 400 @@ -196,3 +203,27 @@ def create_thumbnail_from_base64( ) finally: os.close(fd) + + +def determine_content_type(filename): + """ + Guesses the content type of a filename. Returns the mimetype of a file. + + Returns "application/octet-stream" if the type can't be guessed. + Raises an AssertionError if filename is not a string. + """ + + typ, _ = mimetypes.guess_type(filename) + + if not typ: + return "application/octet-stream" + return typ + + +def hex_to_base64(hexdigest): + """Convert a hex-encoded digest (e.g. an MD5 checksum) to base64.""" + return codecs.encode(codecs.decode(hexdigest, "hex"), "base64").decode().strip() + + +def base64_to_hex(b64): + return codecs.encode(codecs.decode(b64.encode(), "base64"), "hex").decode().strip() diff --git a/contentcuration/contentcuration/utils/gcs_storage.py b/contentcuration/contentcuration/utils/gcs_storage.py index 5c4a425aec..75987e443c 100644 --- a/contentcuration/contentcuration/utils/gcs_storage.py +++ b/contentcuration/contentcuration/utils/gcs_storage.py @@ -11,6 +11,10 @@ from google.cloud.storage import Client from google.cloud.storage.blob import Blob +from .files import determine_content_type +from contentcuration.utils.files import base64_to_hex +from contentcuration.utils.files import hex_to_base64 + OLD_STUDIO_STORAGE_PREFIX = "/contentworkshop_content/" CONTENT_DATABASES_MAX_AGE = 5 # seconds @@ -120,10 +124,6 @@ def save(self, name, fobj, max_length=None, blob_object=None): blob.content_encoding = "gzip" fobj = buffer - # determine the current file's mimetype based on the name - # import determine_content_type lazily in here, so we don't get into an infinite loop with circular dependencies - from contentcuration.utils.storage_common import determine_content_type - content_type = determine_content_type(name) # force the current file to be at file location 0, to @@ -215,6 +215,18 @@ def _is_file_empty(fobj): fobj.seek(current_location) return len(byt) == 0 + def create_resumable_upload_session(self, name, md5, size): + blob = self.bucket.blob(name) + md5_b64 = hex_to_base64(md5) + blob.md5_hash = md5_b64.strip() + blob.content_type = determine_content_type(name) + blob.metadata = {"declared-size": str(size)} + return blob.create_resumable_upload_session(client=self.client) + + def get_stored_object_md5(self, name): + blob = self.bucket.get_blob(name) + return base64_to_hex(blob.md5_hash) if blob is not None else None + class CompositeGCS(Storage): def __init__(self): @@ -283,3 +295,11 @@ def get_created_time(self, name): def get_modified_time(self, name): return self._get_readable_backend(name).get_modified_time(name) + + def create_resumable_upload_session(self, name, md5_b64, size): + return self._get_writeable_backend().create_resumable_upload_session( + name, md5_b64, size + ) + + def get_stored_object_md5(self, name): + return self._get_readable_backend(name).get_stored_object_md5(name) diff --git a/contentcuration/contentcuration/utils/storage_common.py b/contentcuration/contentcuration/utils/storage_common.py index 10d79bd5c5..6f347b27ca 100644 --- a/contentcuration/contentcuration/utils/storage_common.py +++ b/contentcuration/contentcuration/utils/storage_common.py @@ -1,43 +1,22 @@ -import mimetypes -import os from datetime import timedelta from django.conf import settings from django.core.files.storage import default_storage from django_s3_storage.storage import S3Storage +from .files import determine_content_type +from .files import hex_to_base64 from .gcs_storage import CompositeGCS from .gcs_storage import GoogleCloudStorage -# Do this to ensure that we infer mimetypes for files properly, specifically -# zip file and epub files. -# to add additional files add them to the mime.types file -mimetypes.init([os.path.join(os.path.dirname(__file__), "mime.types")]) - - class UnknownStorageBackendError(Exception): pass -def determine_content_type(filename): - """ - Guesses the content type of a filename. Returns the mimetype of a file. - - Returns "application/octet-stream" if the type can't be guessed. - Raises an AssertionError if filename is not a string. - """ - - typ, _ = mimetypes.guess_type(filename) - - if not typ: - return "application/octet-stream" - return typ - - def get_presigned_upload_url( filepath, - md5sum_b64, + md5_hex, lifetime_sec, content_length, storage=default_storage, @@ -48,9 +27,10 @@ def get_presigned_upload_url( contents with the contents of your PUT request. :param: filepath: the file path inside the bucket, to the file. - :param: md5sum_b64: the base64 encoded md5 hash of the file. The holder of the URL will - have to set a Content-MD5 HTTP header matching this md5sum once it - initiates the download. + :param: md5_hex: the hex-encoded md5 hash of the file. The base64 encoding + that GCS requires for the Content-MD5 header is handled internally; the + holder of the URL must set a Content-MD5 HTTP header matching that + base64-encoded value once it initiates the upload. :param: lifetime_sec: the lifetime of the generated upload url, in seconds. :param: content_length: the size of the content, in bytes. :param: client: the storage client that will be used to gennerate the presigned URL. @@ -67,6 +47,7 @@ def get_presigned_upload_url( # both storage types are having difficulties enforcing it. mimetype = determine_content_type(filepath) + md5sum_b64 = hex_to_base64(md5_hex) if isinstance(storage, (GoogleCloudStorage, CompositeGCS)): client = client or storage.get_client() bucket = settings.AWS_S3_BUCKET_NAME From a972a5c2db3f9528c69bad9ca68b3573f88b7bf2 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 24 Jun 2026 22:51:36 -0700 Subject: [PATCH 2/2] feat: add opt-in resumable scheme to the upload_url endpoint - accept a `resumable` flag (defaults off) - GCS: skip when the stored md5 matches the checksum, else return a server-initiated resumable session URI - non-GCS backends fall back to single-PUT - reject non-resumable uploads over 500 MB - make `size` an IntegerField, dropping the redundant float casts Part of #5975. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UzUP3UYP4cLyouvssXyekj --- .../tests/viewsets/test_file.py | 83 +++++++++++++++++++ .../contentcuration/utils/files.py | 1 + .../contentcuration/viewsets/file.py | 47 +++++++---- 3 files changed, 115 insertions(+), 16 deletions(-) diff --git a/contentcuration/contentcuration/tests/viewsets/test_file.py b/contentcuration/contentcuration/tests/viewsets/test_file.py index 9737c7f4bd..b775827040 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_file.py +++ b/contentcuration/contentcuration/tests/viewsets/test_file.py @@ -1,4 +1,5 @@ import uuid +from unittest import mock from django.urls import reverse from le_utils.constants import content_kinds @@ -12,6 +13,8 @@ from contentcuration.tests.viewsets.base import generate_delete_event from contentcuration.tests.viewsets.base import generate_update_event from contentcuration.tests.viewsets.base import SyncTestMixin +from contentcuration.viewsets.file import FileUploadURLSerializer +from contentcuration.viewsets.file import MAX_NON_RESUMABLE_UPLOAD_SIZE from contentcuration.viewsets.sync.constants import CONTENTNODE from contentcuration.viewsets.sync.constants import FILE @@ -546,6 +549,9 @@ def test_mismatched_preset_upload(self): def test_insufficient_storage(self): self.file["size"] = 100000000000000 + self.file[ + "resumable" + ] = True # resumable bypasses the >500MB guard so this still exercises the quota (412) path self.client.force_authenticate(user=self.user) response = self.client.post( @@ -590,6 +596,26 @@ def test_duration_zero(self): self.assertEqual(response.status_code, 400) + def test_fractional_size_rejected(self): + s = FileUploadURLSerializer(data={**self.file, "size": 1000.5}) + assert not s.is_valid() + assert "size" in s.errors + + def test_large_non_resumable_rejected(self): + self.file["size"] = MAX_NON_RESUMABLE_UPLOAD_SIZE + 1 + self.client.force_authenticate(user=self.user) + resp = self.client.post(reverse("file-upload-url"), self.file, format="json") + assert resp.status_code == 400 + + def test_large_resumable_allowed(self): + self.user.disk_space = 10 * 1024 * 1024 * 1024 + self.user.save() + self.file["size"] = MAX_NON_RESUMABLE_UPLOAD_SIZE + 1 + self.file["resumable"] = True + self.client.force_authenticate(user=self.user) + resp = self.client.post(reverse("file-upload-url"), self.file, format="json") + assert resp.status_code == 200 + class ContentIDTestCase(SyncTestMixin, StudioAPITestCase): def setUp(self): @@ -763,3 +789,60 @@ def test_content_id__thumbnails_dont_update_content_id(self): self.assertEqual( copied_node_content_id_before_upload, copied_node_content_id_after_upload ) + + +class ResumableUploadURLTestCase(StudioAPITestCase): + def setUp(self): + super(ResumableUploadURLTestCase, self).setUp() + self.user = testdata.user() + # Give user enough quota to handle resumable uploads + self.user.disk_space = 10 * 1024 * 1024 * 1024 + self.user.save() + self.file = { + "size": 1000, + "checksum": uuid.uuid4().hex, + "name": "le_studio", + "file_format": file_formats.MP3, + "preset": format_presets.AUDIO, + "duration": 10.123, + "resumable": True, + } + + @mock.patch("contentcuration.viewsets.file.default_storage") + def test_resumable_returns_session_when_not_stored(self, mock_storage): + mock_storage.get_stored_object_md5.return_value = None + mock_storage.create_resumable_upload_session.return_value = ( + "https://session.url" + ) + self.client.force_authenticate(user=self.user) + resp = self.client.post(reverse("file-upload-url"), self.file, format="json") + assert resp.status_code == 200 + data = resp.json() + assert data["resumable"] is True + assert data["uploadURL"] == "https://session.url" + assert data["alreadyUploaded"] is False + assert "file" in data + assert data["file"]["id"] + mock_storage.create_resumable_upload_session.assert_called_once() + + @mock.patch("contentcuration.viewsets.file.default_storage") + def test_resumable_skips_when_already_stored(self, mock_storage): + mock_storage.get_stored_object_md5.return_value = self.file["checksum"] + self.client.force_authenticate(user=self.user) + resp = self.client.post(reverse("file-upload-url"), self.file, format="json") + data = resp.json() + assert data["resumable"] is True and data["alreadyUploaded"] is True + assert data["uploadURL"] is None + assert "file" in data + assert data["file"]["id"] + mock_storage.create_resumable_upload_session.assert_not_called() + + def test_resumable_falls_back_to_single_put_on_s3(self): + # default_storage is S3 in the test env → no resumable support + self.client.force_authenticate(user=self.user) + resp = self.client.post(reverse("file-upload-url"), self.file, format="json") + data = resp.json() + assert data["resumable"] is False + assert "uploadURL" in data + assert "file" in data + assert data["file"]["id"] diff --git a/contentcuration/contentcuration/utils/files.py b/contentcuration/contentcuration/utils/files.py index 785faba80b..dca7502262 100644 --- a/contentcuration/contentcuration/utils/files.py +++ b/contentcuration/contentcuration/utils/files.py @@ -1,4 +1,5 @@ import base64 +import codecs import copy import mimetypes import os diff --git a/contentcuration/contentcuration/viewsets/file.py b/contentcuration/contentcuration/viewsets/file.py index afadbff0cb..577eb827e1 100644 --- a/contentcuration/contentcuration/viewsets/file.py +++ b/contentcuration/contentcuration/viewsets/file.py @@ -1,7 +1,7 @@ -import codecs import math from django.core.exceptions import PermissionDenied +from django.core.files.storage import default_storage from django.http import HttpResponseBadRequest from le_utils.constants import file_formats from le_utils.constants import format_presets @@ -34,6 +34,8 @@ PRESET_LOOKUP = {p.id: p for p in format_presets.PRESETLIST} +MAX_NON_RESUMABLE_UPLOAD_SIZE = 500 * 1024 * 1024 + class StrictFloatField(serializers.FloatField): def to_internal_value(self, data): @@ -48,7 +50,7 @@ class FileUploadURLSerializer(serializers.Serializer): """ Serializer to validate inputs for the upload_url endpoint. Required: - - size: a float value + - size: an integer value (bytes) - checksum: a 32-digit hex string - name: a string (note: mapped from request.data['name']) - file_format: a valid file format choice from file_formats.choices @@ -57,12 +59,13 @@ class FileUploadURLSerializer(serializers.Serializer): - duration: a number that will be floored to an integer and must be > 0 """ - size = serializers.FloatField(required=True) + size = serializers.IntegerField(required=True) checksum = serializers.RegexField(regex=r"^[0-9a-f]{32}$", required=True) name = serializers.CharField(required=True) file_format = serializers.ChoiceField(choices=file_formats.choices, required=True) preset = serializers.ChoiceField(choices=format_presets.choices, required=True) duration = StrictFloatField(required=False, allow_null=True) + resumable = serializers.BooleanField(required=False, default=False) def validate_duration(self, value): if value is None: @@ -89,6 +92,10 @@ def validate(self, attrs): raise serializers.ValidationError( f"File format {attrs['file_format']} is not an allowed format for this preset {attrs['preset']}" ) + if not attrs["resumable"] and attrs["size"] > MAX_NON_RESUMABLE_UPLOAD_SIZE: + raise serializers.ValidationError( + "Files larger than 500 MB must use a resumable upload." + ) return attrs @@ -235,26 +242,37 @@ def upload_url(self, request): file_format = validated_data["file_format"] preset = validated_data["preset"] duration = validated_data.get("duration") + resumable = validated_data["resumable"] try: - request.user.check_space(float(size), checksum) + request.user.check_space(size, checksum) except PermissionDenied: return HttpResponseBadRequest( reason="Not enough space. Check your storage under Settings page.", status=412, ) - might_skip = File.objects.filter(checksum=checksum).exists() - filepath = generate_object_storage_name( checksum, filename, default_ext=file_format ) - checksum_base64 = codecs.encode( - codecs.decode(checksum, "hex"), "base64" - ).decode() - retval = get_presigned_upload_url( - filepath, checksum_base64, 600, content_length=size - ) + if resumable and hasattr(default_storage, "create_resumable_upload_session"): + # Resumable response omits mimetype/might_skip. + stored = default_storage.get_stored_object_md5(filepath) == checksum + retval = { + "resumable": True, + "uploadURL": None + if stored + else default_storage.create_resumable_upload_session( + filepath, checksum, size + ), + "alreadyUploaded": stored, + } + else: + retval = get_presigned_upload_url( + filepath, checksum, 600, content_length=size + ) + retval["resumable"] = False + retval["might_skip"] = File.objects.filter(checksum=checksum).exists() file = File( file_size=size, @@ -270,8 +288,5 @@ def upload_url(self, request): # Avoid using our file_on_disk attribute for checks file.save(set_by_file_on_disk=False) - retval.update( - {"might_skip": might_skip, "file": self.serialize_object(id=file.id)} - ) - + retval["file"] = self.serialize_object(id=file.id) return Response(retval)