diff --git a/links/management/commands/migrate_image_storage.py b/links/management/commands/migrate_image_storage.py index 64e81ba..ecc5dde 100644 --- a/links/management/commands/migrate_image_storage.py +++ b/links/management/commands/migrate_image_storage.py @@ -3,10 +3,9 @@ One-time / idempotent migration helper. For every Image that still has a legacy ``file_key`` and no linked ``file`` (FileUpload), this command: - 1. Generates a short-lived R2 presigned URL (the same URL the app serves in - the UI — proven to work; direct boto3 API calls against the S3 endpoint - fail with SignatureDoesNotMatch for this account, so we go through the - custom domain like the browser does). + 1. Downloads the object from R2 — direct boto3 ``get_object`` first (works + whenever the credentials are valid), presigned custom-domain URL as a + fallback. 2. Downloads the bytes. 3. Writes them to FILE_UPLOADS_FOLDER using the exact same persistence logic as regular file uploads (temp file + fsync + byte-count verification). @@ -33,18 +32,24 @@ from links.models import Image from links.storage import R2Storage -def _download_via_presigned_url(storage, key): - """Download an R2 object using the presigned custom-domain URL. +def _download_r2_object(storage, key): + """Download an R2 object, preferring direct boto3 API access. - Direct boto3 ``get_object`` calls fail for this account - (SignatureDoesNotMatch on the S3 API endpoint), while the presigned URLs - the app serves via the R2 custom domain work fine — so reuse that path. + Direct ``get_object`` works whenever the R2 credentials are valid. Some + accounts/domains additionally break on presigned-URL fetches through the + custom domain (403 — domain binding/signature path issue), so fall back to + the presigned URL only when the direct call fails. """ - url = storage.get_url(key, expires_in=3600) - if not url: - raise ValueError(f'No signed URL for {key}') - with urllib.request.urlopen(url, timeout=60) as resp: - return resp.read() + try: + obj = storage.client.get_object(Bucket=storage.bucket, Key=key) + return obj['Body'].read() + except Exception as direct_err: # noqa: BLE001 + url = storage.get_url(key, expires_in=3600) + if not url: + raise ValueError(f'no signed URL after direct download failed: {direct_err}') from direct_err + req = urllib.request.Request(url, headers={'User-Agent': 'links-backfill'}) + with urllib.request.urlopen(req, timeout=60) as resp: + return resp.read() class Command(BaseCommand): @@ -80,8 +85,8 @@ class Command(BaseCommand): skipped += 1 continue try: - # 1–2. Download from R2 via the presigned custom-domain URL - raw = _download_via_presigned_url(storage, image.file_key) + # 1–2. Download from R2 (direct API first, presigned fallback) + raw = _download_r2_object(storage, image.file_key) if not raw: raise ValueError(f'Empty object for {image.file_key}') diff --git a/links/templates/links/collection_detail.html b/links/templates/links/collection_detail.html index beccfc5..84ab9e7 100644 --- a/links/templates/links/collection_detail.html +++ b/links/templates/links/collection_detail.html @@ -85,29 +85,6 @@ .apple-btn-danger:hover { background: #d63027; } .apple-btn svg { width: 1.05rem; height: 1.05rem; } - /* ---------- Upload zone ---------- */ - .upload-zone { - position: relative; display: flex; flex-direction: column; align-items: center; justify-content: center; - gap: 0.5rem; min-height: 150px; padding: 1.6rem 1.2rem; margin-bottom: 1.6rem; text-align: center; - background: #fff; border-radius: var(--apple-radius-lg); - border: 2px dashed rgba(0, 0, 0, 0.14); - cursor: pointer; overflow: hidden; - transition: border-color 0.3s var(--apple-ease), background 0.3s var(--apple-ease), box-shadow 0.3s var(--apple-ease); - } - .upload-zone:hover, .upload-zone.dragover { border-color: var(--apple-blue); background: rgba(0, 113, 227, 0.04); } - .upload-zone svg { width: 2.1rem; height: 2.1rem; color: #b0b0b6; transition: transform 0.4s var(--apple-ease); } - .upload-zone:hover svg { transform: translateY(-3px); } - .upload-title { font-size: 0.98rem; font-weight: 600; } - .upload-sub { font-size: 0.82rem; color: var(--apple-gray); } - .upload-progress { - position: absolute; left: 0; right: 0; bottom: 0; height: 4px; background: rgba(0, 0, 0, 0.06); - } - .upload-progress-bar { - height: 100%; width: 0; background: var(--apple-blue); border-radius: 0 2px 2px 0; - transition: width 0.25s ease; - } - .upload-status { font-size: 0.82rem; color: var(--apple-gray); min-height: 1.2em; } - /* ---------- Image grid ---------- */ .image-grid { display: grid; gap: 0.9rem; @@ -381,16 +358,7 @@ -
- - - -
{% trans "Drag & drop images here" %}
-
{% trans "or click to browse · JPG, PNG, WebP, GIF, HEIC" %}
-
-
- -
+
@@ -529,24 +497,10 @@ showToast._timer = setTimeout(() => t.classList.remove('show'), 2600); } - /* ── Upload (native drag & drop, no CDN dependency) ── */ - const zone = document.getElementById('uploadZone'); + /* ── Upload (via header Upload button → hidden file input) ── */ const fileInput = document.getElementById('fileInput'); - const statusEl = document.getElementById('uploadStatus'); - const progressBar = document.getElementById('uploadProgressBar'); - zone.addEventListener('click', () => fileInput.click()); fileInput.addEventListener('change', () => { if (fileInput.files.length) uploadFiles(fileInput.files); }); - ['dragenter', 'dragover'].forEach(ev => zone.addEventListener(ev, e => { - e.preventDefault(); e.stopPropagation(); zone.classList.add('dragover'); - })); - ['dragleave', 'drop'].forEach(ev => zone.addEventListener(ev, e => { - e.preventDefault(); e.stopPropagation(); zone.classList.remove('dragover'); - })); - zone.addEventListener('drop', e => { - const files = e.dataTransfer.files; - if (files.length) uploadFiles(files); - }); function uploadFiles(fileList) { const files = Array.from(fileList).filter(f => f.type.startsWith('image/') || /\.(jpe?g|png|webp|gif|heic|heif|avif)$/i.test(f.name)); @@ -556,8 +510,7 @@ } const total = files.length; let done = 0, failed = 0; - statusEl.textContent = '0 / ' + total + ' {% trans "uploading…" %}'; - progressBar.style.width = '0%'; + showToast('0 / ' + total + ' {% trans "uploading…" %}'); files.forEach(file => { const fd = new FormData(); @@ -565,31 +518,23 @@ const xhr = new XMLHttpRequest(); xhr.open('POST', '/api/collections/{{ collection.pk }}/upload_images'); xhr.setRequestHeader('X-CSRFToken', getCookie('csrftoken')); - xhr.upload.onprogress = (e) => { - if (e.lengthComputable) { - progressBar.style.width = Math.min(100, Math.round((done + e.loaded / e.total) / total * 100)) + '%'; - } - }; xhr.onload = () => { done++; if (xhr.status >= 200 && xhr.status < 300) { - statusEl.textContent = done + ' / ' + total + ' {% trans "uploaded" %}'; + showToast(done + ' / ' + total + ' {% trans "uploaded" %}'); } else { failed++; - statusEl.textContent = done + ' / ' + total + ' ({{ "failed"|escapejs }}: ' + failed + ')'; + showToast(done + ' / ' + total + ' (failed: ' + failed + ')'); } - progressBar.style.width = Math.min(100, Math.round(done / total * 100)) + '%'; if (done === total) { - statusEl.textContent = failed - ? (done - failed) + ' {% trans "uploaded" %}, ' + failed + ' {% trans "failed" %}' - : '{% trans "All images uploaded ✓" %}'; - setTimeout(() => { statusEl.textContent = ''; progressBar.style.width = '0%'; }, 1200); + if (!failed) showToast('{% trans "All images uploaded ✓" %}'); + else showToast(done - failed + ' {% trans "uploaded" %}, ' + failed + ' {% trans "failed" %}'); setTimeout(() => location.reload(), 900); } }; xhr.onerror = () => { done++; failed++; - statusEl.textContent = done + ' / ' + total + ' (failed: ' + failed + ')'; + showToast(done + ' / ' + total + ' (failed: ' + failed + ')'); if (done === total) setTimeout(() => location.reload(), 900); }; xhr.send(fd); diff --git a/tests/test_collections.py b/tests/test_collections.py index cd4d326..f52b32a 100644 --- a/tests/test_collections.py +++ b/tests/test_collections.py @@ -150,6 +150,14 @@ class TestMigrateImageStorageCommand: content_type="image/png", size=len(raw or b"data"), ) + @staticmethod + def _patch_download(raw=None, exc=None): + return mock.patch( + "links.management.commands.migrate_image_storage._download_r2_object", + return_value=raw if raw is not None else b"\x89PNGdata", + side_effect=exc, + ) + def test_dry_run_does_nothing(self, tmp_path): coll = ImageCollection.objects.create(name="C") self._legacy_image(coll, raw=b"\x89PNGdata") @@ -165,10 +173,7 @@ class TestMigrateImageStorageCommand: raw = raw.getvalue() self._legacy_image(coll, raw=raw) - with mock.patch( - "links.management.commands.migrate_image_storage._download_via_presigned_url", - return_value=raw, - ): + with self._patch_download(raw=raw): call_command("migrate_image_storage", "--commit") img = Image.objects.get() @@ -177,10 +182,7 @@ class TestMigrateImageStorageCommand: assert os.path.exists(img.file.file_path) assert img.file.size == len(raw) # Idempotent: second run has nothing to do - with mock.patch( - "links.management.commands.migrate_image_storage._download_via_presigned_url", - return_value=raw, - ): + with self._patch_download(raw=raw): call_command("migrate_image_storage", "--commit") assert Image.objects.get().file is not None @@ -188,10 +190,7 @@ class TestMigrateImageStorageCommand: coll = ImageCollection.objects.create(name="C") self._legacy_image(coll, raw=b"\x89PNGdata") - with mock.patch( - "links.management.commands.migrate_image_storage._download_via_presigned_url", - side_effect=Exception("network down"), - ): + with self._patch_download(exc=Exception("network down")): call_command("migrate_image_storage", "--commit") img = Image.objects.get()