Skip to content

Commit a972a5c

Browse files
rtibblesclaude
andcommitted
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 learningequality#5975. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzUP3UYP4cLyouvssXyekj
1 parent e8951f7 commit a972a5c

3 files changed

Lines changed: 115 additions & 16 deletions

File tree

contentcuration/contentcuration/tests/viewsets/test_file.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import uuid
2+
from unittest import mock
23

34
from django.urls import reverse
45
from le_utils.constants import content_kinds
@@ -12,6 +13,8 @@
1213
from contentcuration.tests.viewsets.base import generate_delete_event
1314
from contentcuration.tests.viewsets.base import generate_update_event
1415
from contentcuration.tests.viewsets.base import SyncTestMixin
16+
from contentcuration.viewsets.file import FileUploadURLSerializer
17+
from contentcuration.viewsets.file import MAX_NON_RESUMABLE_UPLOAD_SIZE
1518
from contentcuration.viewsets.sync.constants import CONTENTNODE
1619
from contentcuration.viewsets.sync.constants import FILE
1720

@@ -546,6 +549,9 @@ def test_mismatched_preset_upload(self):
546549

547550
def test_insufficient_storage(self):
548551
self.file["size"] = 100000000000000
552+
self.file[
553+
"resumable"
554+
] = True # resumable bypasses the >500MB guard so this still exercises the quota (412) path
549555

550556
self.client.force_authenticate(user=self.user)
551557
response = self.client.post(
@@ -590,6 +596,26 @@ def test_duration_zero(self):
590596

591597
self.assertEqual(response.status_code, 400)
592598

599+
def test_fractional_size_rejected(self):
600+
s = FileUploadURLSerializer(data={**self.file, "size": 1000.5})
601+
assert not s.is_valid()
602+
assert "size" in s.errors
603+
604+
def test_large_non_resumable_rejected(self):
605+
self.file["size"] = MAX_NON_RESUMABLE_UPLOAD_SIZE + 1
606+
self.client.force_authenticate(user=self.user)
607+
resp = self.client.post(reverse("file-upload-url"), self.file, format="json")
608+
assert resp.status_code == 400
609+
610+
def test_large_resumable_allowed(self):
611+
self.user.disk_space = 10 * 1024 * 1024 * 1024
612+
self.user.save()
613+
self.file["size"] = MAX_NON_RESUMABLE_UPLOAD_SIZE + 1
614+
self.file["resumable"] = True
615+
self.client.force_authenticate(user=self.user)
616+
resp = self.client.post(reverse("file-upload-url"), self.file, format="json")
617+
assert resp.status_code == 200
618+
593619

594620
class ContentIDTestCase(SyncTestMixin, StudioAPITestCase):
595621
def setUp(self):
@@ -763,3 +789,60 @@ def test_content_id__thumbnails_dont_update_content_id(self):
763789
self.assertEqual(
764790
copied_node_content_id_before_upload, copied_node_content_id_after_upload
765791
)
792+
793+
794+
class ResumableUploadURLTestCase(StudioAPITestCase):
795+
def setUp(self):
796+
super(ResumableUploadURLTestCase, self).setUp()
797+
self.user = testdata.user()
798+
# Give user enough quota to handle resumable uploads
799+
self.user.disk_space = 10 * 1024 * 1024 * 1024
800+
self.user.save()
801+
self.file = {
802+
"size": 1000,
803+
"checksum": uuid.uuid4().hex,
804+
"name": "le_studio",
805+
"file_format": file_formats.MP3,
806+
"preset": format_presets.AUDIO,
807+
"duration": 10.123,
808+
"resumable": True,
809+
}
810+
811+
@mock.patch("contentcuration.viewsets.file.default_storage")
812+
def test_resumable_returns_session_when_not_stored(self, mock_storage):
813+
mock_storage.get_stored_object_md5.return_value = None
814+
mock_storage.create_resumable_upload_session.return_value = (
815+
"https://session.url"
816+
)
817+
self.client.force_authenticate(user=self.user)
818+
resp = self.client.post(reverse("file-upload-url"), self.file, format="json")
819+
assert resp.status_code == 200
820+
data = resp.json()
821+
assert data["resumable"] is True
822+
assert data["uploadURL"] == "https://session.url"
823+
assert data["alreadyUploaded"] is False
824+
assert "file" in data
825+
assert data["file"]["id"]
826+
mock_storage.create_resumable_upload_session.assert_called_once()
827+
828+
@mock.patch("contentcuration.viewsets.file.default_storage")
829+
def test_resumable_skips_when_already_stored(self, mock_storage):
830+
mock_storage.get_stored_object_md5.return_value = self.file["checksum"]
831+
self.client.force_authenticate(user=self.user)
832+
resp = self.client.post(reverse("file-upload-url"), self.file, format="json")
833+
data = resp.json()
834+
assert data["resumable"] is True and data["alreadyUploaded"] is True
835+
assert data["uploadURL"] is None
836+
assert "file" in data
837+
assert data["file"]["id"]
838+
mock_storage.create_resumable_upload_session.assert_not_called()
839+
840+
def test_resumable_falls_back_to_single_put_on_s3(self):
841+
# default_storage is S3 in the test env → no resumable support
842+
self.client.force_authenticate(user=self.user)
843+
resp = self.client.post(reverse("file-upload-url"), self.file, format="json")
844+
data = resp.json()
845+
assert data["resumable"] is False
846+
assert "uploadURL" in data
847+
assert "file" in data
848+
assert data["file"]["id"]

contentcuration/contentcuration/utils/files.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import base64
2+
import codecs
23
import copy
34
import mimetypes
45
import os

contentcuration/contentcuration/viewsets/file.py

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import codecs
21
import math
32

43
from django.core.exceptions import PermissionDenied
4+
from django.core.files.storage import default_storage
55
from django.http import HttpResponseBadRequest
66
from le_utils.constants import file_formats
77
from le_utils.constants import format_presets
@@ -34,6 +34,8 @@
3434

3535
PRESET_LOOKUP = {p.id: p for p in format_presets.PRESETLIST}
3636

37+
MAX_NON_RESUMABLE_UPLOAD_SIZE = 500 * 1024 * 1024
38+
3739

3840
class StrictFloatField(serializers.FloatField):
3941
def to_internal_value(self, data):
@@ -48,7 +50,7 @@ class FileUploadURLSerializer(serializers.Serializer):
4850
"""
4951
Serializer to validate inputs for the upload_url endpoint.
5052
Required:
51-
- size: a float value
53+
- size: an integer value (bytes)
5254
- checksum: a 32-digit hex string
5355
- name: a string (note: mapped from request.data['name'])
5456
- file_format: a valid file format choice from file_formats.choices
@@ -57,12 +59,13 @@ class FileUploadURLSerializer(serializers.Serializer):
5759
- duration: a number that will be floored to an integer and must be > 0
5860
"""
5961

60-
size = serializers.FloatField(required=True)
62+
size = serializers.IntegerField(required=True)
6163
checksum = serializers.RegexField(regex=r"^[0-9a-f]{32}$", required=True)
6264
name = serializers.CharField(required=True)
6365
file_format = serializers.ChoiceField(choices=file_formats.choices, required=True)
6466
preset = serializers.ChoiceField(choices=format_presets.choices, required=True)
6567
duration = StrictFloatField(required=False, allow_null=True)
68+
resumable = serializers.BooleanField(required=False, default=False)
6669

6770
def validate_duration(self, value):
6871
if value is None:
@@ -89,6 +92,10 @@ def validate(self, attrs):
8992
raise serializers.ValidationError(
9093
f"File format {attrs['file_format']} is not an allowed format for this preset {attrs['preset']}"
9194
)
95+
if not attrs["resumable"] and attrs["size"] > MAX_NON_RESUMABLE_UPLOAD_SIZE:
96+
raise serializers.ValidationError(
97+
"Files larger than 500 MB must use a resumable upload."
98+
)
9299
return attrs
93100

94101

@@ -235,26 +242,37 @@ def upload_url(self, request):
235242
file_format = validated_data["file_format"]
236243
preset = validated_data["preset"]
237244
duration = validated_data.get("duration")
245+
resumable = validated_data["resumable"]
238246

239247
try:
240-
request.user.check_space(float(size), checksum)
248+
request.user.check_space(size, checksum)
241249
except PermissionDenied:
242250
return HttpResponseBadRequest(
243251
reason="Not enough space. Check your storage under Settings page.",
244252
status=412,
245253
)
246254

247-
might_skip = File.objects.filter(checksum=checksum).exists()
248-
249255
filepath = generate_object_storage_name(
250256
checksum, filename, default_ext=file_format
251257
)
252-
checksum_base64 = codecs.encode(
253-
codecs.decode(checksum, "hex"), "base64"
254-
).decode()
255-
retval = get_presigned_upload_url(
256-
filepath, checksum_base64, 600, content_length=size
257-
)
258+
if resumable and hasattr(default_storage, "create_resumable_upload_session"):
259+
# Resumable response omits mimetype/might_skip.
260+
stored = default_storage.get_stored_object_md5(filepath) == checksum
261+
retval = {
262+
"resumable": True,
263+
"uploadURL": None
264+
if stored
265+
else default_storage.create_resumable_upload_session(
266+
filepath, checksum, size
267+
),
268+
"alreadyUploaded": stored,
269+
}
270+
else:
271+
retval = get_presigned_upload_url(
272+
filepath, checksum, 600, content_length=size
273+
)
274+
retval["resumable"] = False
275+
retval["might_skip"] = File.objects.filter(checksum=checksum).exists()
258276

259277
file = File(
260278
file_size=size,
@@ -270,8 +288,5 @@ def upload_url(self, request):
270288
# Avoid using our file_on_disk attribute for checks
271289
file.save(set_by_file_on_disk=False)
272290

273-
retval.update(
274-
{"might_skip": might_skip, "file": self.serialize_object(id=file.id)}
275-
)
276-
291+
retval["file"] = self.serialize_object(id=file.id)
277292
return Response(retval)

0 commit comments

Comments
 (0)