Skip to content

Commit d2ec4b0

Browse files
ferhatelmasferhatelmas
committed
fix(storage): cache control header and retry safety
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
1 parent 3c98900 commit d2ec4b0

5 files changed

Lines changed: 313 additions & 84 deletions

File tree

‎src/storage/src/storage3/_async/file_api.py‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from __future__ import annotations
22

3-
import base64
43
import json
54
import urllib.parse
65
from dataclasses import dataclass, field
@@ -157,19 +156,26 @@ async def upload_to_signed_url(
157156

158157
final_url = ["object", "upload", "sign", self.id, *path_parts]
159158

160-
options: UploadSignedUrlFileOptions = file_options or {}
161-
cache_control = options.get("cache-control")
162-
# cacheControl is also passed as form data
163-
# https://github.com/supabase/storage-js/blob/fa44be8156295ba6320ffeff96bdf91016536a46/src/packages/StorageFileApi.ts#L89
164-
_data = {}
165-
if cache_control:
166-
options["cache-control"] = f"max-age={cache_control}"
167-
_data = {"cacheControl": cache_control}
159+
options: dict[str, Any] = {
160+
**DEFAULT_FILE_OPTIONS,
161+
**(file_options or {}),
162+
}
163+
cache_control = options.pop("cache-control")
164+
content_type = options.pop("content-type")
165+
metadata = options.pop("metadata", None)
166+
file_opts_headers = options.pop("headers", None)
167+
168+
_data = {"cacheControl": cache_control}
169+
if metadata is not None:
170+
_data["metadata"] = json.dumps(metadata)
171+
168172
headers = {
169173
**self._client.headers,
170-
**DEFAULT_FILE_OPTIONS,
171174
**options,
172175
}
176+
if file_opts_headers:
177+
headers.update(file_opts_headers)
178+
173179
filename = path_parts[-1]
174180

175181
if (
@@ -178,14 +184,14 @@ async def upload_to_signed_url(
178184
or isinstance(file, FileIO)
179185
):
180186
# bytes or byte-stream-like object received
181-
_file = {"file": (filename, file, headers.pop("content-type"))}
187+
_file = {"file": (filename, file, content_type)}
182188
else:
183189
# str or pathlib.path received
184190
_file = {
185191
"file": (
186192
filename,
187193
open(file, "rb"),
188-
headers.pop("content-type"),
194+
content_type,
189195
)
190196
}
191197
response = await self._request(
@@ -512,56 +518,53 @@ async def _upload_or_update(
512518
file_options
513519
HTTP headers.
514520
"""
515-
if file_options is None:
516-
file_options = {}
517-
cache_control = file_options.pop("cache-control", None)
518-
_data = {}
521+
options: dict[str, Any] = {
522+
**DEFAULT_FILE_OPTIONS,
523+
**(file_options or {}),
524+
}
525+
cache_control = options.pop("cache-control")
526+
content_type = options.pop("content-type")
527+
_data = {"cacheControl": cache_control}
519528

520-
upsert = file_options.pop("upsert", None)
529+
upsert = options.pop("upsert", None)
521530
if upsert:
522-
file_options.update({"x-upsert": upsert})
531+
options["x-upsert"] = upsert
523532

524-
metadata = file_options.pop("metadata", None)
525-
file_opts_headers = file_options.pop("headers", None)
533+
metadata = options.pop("metadata", None)
534+
file_opts_headers = options.pop("headers", None)
526535

527536
headers = {
528537
**self._client.headers,
529-
**DEFAULT_FILE_OPTIONS,
530-
**file_options,
538+
**options,
531539
}
532540

533-
if metadata:
541+
if metadata is not None:
534542
metadata_str = json.dumps(metadata)
535-
headers["x-metadata"] = base64.b64encode(metadata_str.encode())
536-
_data.update({"metadata": metadata_str})
537-
538-
if file_opts_headers:
539-
headers.update({**file_opts_headers})
543+
_data["metadata"] = metadata_str
540544

541545
# Only include x-upsert on a POST method
542546
if method != "POST":
543-
del headers["x-upsert"]
547+
headers.pop("x-upsert", None)
544548

545-
filename = path[-1]
549+
if file_opts_headers:
550+
headers.update(file_opts_headers)
546551

547-
if cache_control:
548-
headers["cache-control"] = f"max-age={cache_control}"
549-
_data.update({"cacheControl": cache_control})
552+
filename = path[-1]
550553

551554
if (
552555
isinstance(file, BufferedReader)
553556
or isinstance(file, bytes)
554557
or isinstance(file, FileIO)
555558
):
556559
# bytes or byte-stream-like object received
557-
files = {"file": (filename, file, headers.pop("content-type"))}
560+
files = {"file": (filename, file, content_type)}
558561
else:
559562
# str or pathlib.path received
560563
files = {
561564
"file": (
562565
filename,
563566
open(file, "rb"),
564-
headers.pop("content-type"),
567+
content_type,
565568
)
566569
}
567570

‎src/storage/src/storage3/_sync/file_api.py‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from __future__ import annotations
22

3-
import base64
43
import json
54
import urllib.parse
65
from dataclasses import dataclass, field
@@ -157,19 +156,26 @@ def upload_to_signed_url(
157156

158157
final_url = ["object", "upload", "sign", self.id, *path_parts]
159158

160-
options: UploadSignedUrlFileOptions = file_options or {}
161-
cache_control = options.get("cache-control")
162-
# cacheControl is also passed as form data
163-
# https://github.com/supabase/storage-js/blob/fa44be8156295ba6320ffeff96bdf91016536a46/src/packages/StorageFileApi.ts#L89
164-
_data = {}
165-
if cache_control:
166-
options["cache-control"] = f"max-age={cache_control}"
167-
_data = {"cacheControl": cache_control}
159+
options: dict[str, Any] = {
160+
**DEFAULT_FILE_OPTIONS,
161+
**(file_options or {}),
162+
}
163+
cache_control = options.pop("cache-control")
164+
content_type = options.pop("content-type")
165+
metadata = options.pop("metadata", None)
166+
file_opts_headers = options.pop("headers", None)
167+
168+
_data = {"cacheControl": cache_control}
169+
if metadata is not None:
170+
_data["metadata"] = json.dumps(metadata)
171+
168172
headers = {
169173
**self._client.headers,
170-
**DEFAULT_FILE_OPTIONS,
171174
**options,
172175
}
176+
if file_opts_headers:
177+
headers.update(file_opts_headers)
178+
173179
filename = path_parts[-1]
174180

175181
if (
@@ -178,14 +184,14 @@ def upload_to_signed_url(
178184
or isinstance(file, FileIO)
179185
):
180186
# bytes or byte-stream-like object received
181-
_file = {"file": (filename, file, headers.pop("content-type"))}
187+
_file = {"file": (filename, file, content_type)}
182188
else:
183189
# str or pathlib.path received
184190
_file = {
185191
"file": (
186192
filename,
187193
open(file, "rb"),
188-
headers.pop("content-type"),
194+
content_type,
189195
)
190196
}
191197
response = self._request(
@@ -510,56 +516,53 @@ def _upload_or_update(
510516
file_options
511517
HTTP headers.
512518
"""
513-
if file_options is None:
514-
file_options = {}
515-
cache_control = file_options.pop("cache-control", None)
516-
_data = {}
519+
options: dict[str, Any] = {
520+
**DEFAULT_FILE_OPTIONS,
521+
**(file_options or {}),
522+
}
523+
cache_control = options.pop("cache-control")
524+
content_type = options.pop("content-type")
525+
_data = {"cacheControl": cache_control}
517526

518-
upsert = file_options.pop("upsert", None)
527+
upsert = options.pop("upsert", None)
519528
if upsert:
520-
file_options.update({"x-upsert": upsert})
529+
options["x-upsert"] = upsert
521530

522-
metadata = file_options.pop("metadata", None)
523-
file_opts_headers = file_options.pop("headers", None)
531+
metadata = options.pop("metadata", None)
532+
file_opts_headers = options.pop("headers", None)
524533

525534
headers = {
526535
**self._client.headers,
527-
**DEFAULT_FILE_OPTIONS,
528-
**file_options,
536+
**options,
529537
}
530538

531-
if metadata:
539+
if metadata is not None:
532540
metadata_str = json.dumps(metadata)
533-
headers["x-metadata"] = base64.b64encode(metadata_str.encode())
534-
_data.update({"metadata": metadata_str})
535-
536-
if file_opts_headers:
537-
headers.update({**file_opts_headers})
541+
_data["metadata"] = metadata_str
538542

539543
# Only include x-upsert on a POST method
540544
if method != "POST":
541-
del headers["x-upsert"]
545+
headers.pop("x-upsert", None)
542546

543-
filename = path[-1]
547+
if file_opts_headers:
548+
headers.update(file_opts_headers)
544549

545-
if cache_control:
546-
headers["cache-control"] = f"max-age={cache_control}"
547-
_data.update({"cacheControl": cache_control})
550+
filename = path[-1]
548551

549552
if (
550553
isinstance(file, BufferedReader)
551554
or isinstance(file, bytes)
552555
or isinstance(file, FileIO)
553556
):
554557
# bytes or byte-stream-like object received
555-
files = {"file": (filename, file, headers.pop("content-type"))}
558+
files = {"file": (filename, file, content_type)}
556559
else:
557560
# str or pathlib.path received
558561
files = {
559562
"file": (
560563
filename,
561564
open(file, "rb"),
562-
headers.pop("content-type"),
565+
content_type,
563566
)
564567
}
565568

‎src/storage/tests/_async/test_client.py‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,10 @@ async def test_client_upload(
282282
assert image_info is not None
283283
assert image_info.get("metadata", {}).get("mimetype") == file.mime_type
284284

285+
# Default cache-control is "3600" seconds and must be stored as max-age=3600
286+
info = await storage_file_client.info(file.bucket_path)
287+
assert info.get("cache_control") == "max-age=3600"
288+
285289

286290
async def test_client_upload_with_query(
287291
storage_file_client: AsyncBucketProxy, file: FileForTesting
@@ -336,7 +340,10 @@ async def test_client_update(
336340
await storage_file_client.update(
337341
two_files[0].bucket_path,
338342
two_files[1].local_path,
339-
{"content-type": two_files[1].mime_type},
343+
{
344+
"content-type": two_files[1].mime_type,
345+
"cache-control": "7200",
346+
},
340347
)
341348

342349
image = await storage_file_client.download(two_files[0].bucket_path)
@@ -348,6 +355,8 @@ async def test_client_update(
348355
assert image == two_files[1].file_content
349356
assert image_info is not None
350357
assert image_info.get("metadata", {}).get("mimetype") == two_files[1].mime_type
358+
info = await storage_file_client.info(two_files[0].bucket_path)
359+
assert info.get("cache_control") == "max-age=7200"
351360

352361

353362
@pytest.mark.parametrize(
@@ -371,7 +380,13 @@ async def test_client_upload_to_signed_url(
371380
# Test with content-type
372381
data = await storage_file_client.create_signed_upload_url(file.bucket_path)
373382
await storage_file_client.upload_to_signed_url(
374-
data["path"], data["token"], file.file_content, {"content-type": file.mime_type}
383+
data["path"],
384+
data["token"],
385+
file.file_content,
386+
{
387+
"content-type": file.mime_type,
388+
"metadata": {"source": "signed-upload"},
389+
},
375390
)
376391
image = await storage_file_client.download(file.bucket_path)
377392
files = await storage_file_client.list(file.bucket_folder)
@@ -380,8 +395,10 @@ async def test_client_upload_to_signed_url(
380395
assert image == file.file_content
381396
assert image_info is not None
382397
assert image_info.get("metadata", {}).get("mimetype") == file.mime_type
398+
info = await storage_file_client.info(file.bucket_path)
399+
assert info.get("metadata") == {"source": "signed-upload"}
383400

384-
# Test with file_options=None
401+
# Test with file_options=None — still applies default cache-control max-age=3600
385402
data = await storage_file_client.create_signed_upload_url(
386403
f"no_options_{file.bucket_path}"
387404
)
@@ -390,16 +407,18 @@ async def test_client_upload_to_signed_url(
390407
)
391408
image = await storage_file_client.download(f"no_options_{file.bucket_path}")
392409
assert image == file.file_content
410+
no_options_info = await storage_file_client.info(f"no_options_{file.bucket_path}")
411+
assert no_options_info.get("cache_control") == "max-age=3600"
393412

394-
# Test with cache-control
413+
# Test with explicit cache-control
395414
data = await storage_file_client.create_signed_upload_url(
396415
f"cached_{file.bucket_path}"
397416
)
398417
await storage_file_client.upload_to_signed_url(
399-
data["path"], data["token"], file.file_content, {"cache-control": "3600"}
418+
data["path"], data["token"], file.file_content, {"cache-control": "86400"}
400419
)
401420
cached_info = await storage_file_client.info(f"cached_{file.bucket_path}")
402-
assert cached_info.get("cache_control") == "max-age=3600"
421+
assert cached_info.get("cache_control") == "max-age=86400"
403422

404423

405424
async def test_client_create_signed_url(

0 commit comments

Comments
 (0)