mirror of
https://github.com/wahyd4/links.git
synced 2026-08-08 21:04:53 +10:00
fix: drop drag&drop upload zone (button only) + direct-first R2 backfill download
- Collection detail: remove the 'Drag & drop images here' zone per 大哥; Upload button opens the hidden file picker, progress shown via toast - migrate_image_storage: download via direct boto3 get_object first, presigned URL only as fallback (custom domain 403s for this account even with valid creds)
This commit is contained in:
@@ -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}')
|
||||
|
||||
|
||||
@@ -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 @@
|
||||
</div>
|
||||
|
||||
<!-- Upload zone -->
|
||||
<div class="upload-zone" id="uploadZone">
|
||||
<svg fill="none" stroke="currentColor" viewBox="0 0 24 24" stroke-width="1.5">
|
||||
<path stroke-linecap="round" stroke-linejoin="round" d="M3 16.5v2.25A2.25 2.25 0 005.25 21h13.5A2.25 2.25 0 0021 18.75V16.5m-13.5-9L12 3m0 0l4.5 4.5M12 3v13.5"/>
|
||||
</svg>
|
||||
<div class="upload-title">{% trans "Drag & drop images here" %}</div>
|
||||
<div class="upload-sub">{% trans "or click to browse · JPG, PNG, WebP, GIF, HEIC" %}</div>
|
||||
<div class="upload-progress"><div class="upload-progress-bar" id="uploadProgressBar"></div></div>
|
||||
<div class="upload-status" id="uploadStatus"></div>
|
||||
<input type="file" id="fileInput" multiple accept="image/*" class="hidden" style="display:none;">
|
||||
</div>
|
||||
<input type="file" id="fileInput" multiple accept="image/*" style="display:none;">
|
||||
|
||||
<!-- Images grid -->
|
||||
<div class="image-grid" id="imagesGrid">
|
||||
@@ -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);
|
||||
|
||||
+11
-12
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user