From be4dd5b47ff432509d9e5680c55544e2ae544dc9 Mon Sep 17 00:00:00 2001 From: Michilis Date: Thu, 6 Aug 2026 20:14:31 +0000 Subject: [PATCH] Add mobile save sheet with same-origin photo downloads. Phones cannot land downloads in the photo library, so the gallery opens a share sheet for a cacheable preview while streaming via a dedicated download endpoint and recording preview sizes. Co-authored-by: Cursor --- .../(public)/photos/[slug]/GalleryClient.tsx | 44 ++- frontend/src/components/Lightbox.tsx | 6 +- frontend/src/components/gallery/PhotoTile.tsx | 8 +- frontend/src/components/gallery/SaveSheet.tsx | 370 ++++++++++++++++++ frontend/src/i18n/locales/en.json | 15 + frontend/src/i18n/locales/es.json | 15 + frontend/src/lib/api/photos.ts | 11 + frontend/tailwind.config.js | 12 + photo-api/internal/httpapi/api_test.go | 174 ++++++++ photo-api/internal/httpapi/dto.go | 33 +- photo-api/internal/httpapi/files.go | 183 ++++++++- photo-api/internal/httpapi/galleries.go | 1 + photo-api/internal/httpapi/public.go | 1 + photo-api/internal/httpapi/server.go | 3 + photo-api/internal/httpapi/sizes.go | 64 +++ photo-api/internal/photosync/run_test.go | 2 +- photo-api/internal/store/photos.go | 43 +- photo-api/internal/worker/worker.go | 21 +- photo-api/migrations/0003_preview_size.pg.sql | 6 + .../migrations/0003_preview_size.sqlite.sql | 6 + 20 files changed, 962 insertions(+), 56 deletions(-) create mode 100644 frontend/src/components/gallery/SaveSheet.tsx create mode 100644 photo-api/internal/httpapi/sizes.go create mode 100644 photo-api/migrations/0003_preview_size.pg.sql create mode 100644 photo-api/migrations/0003_preview_size.sqlite.sql diff --git a/frontend/src/app/(public)/photos/[slug]/GalleryClient.tsx b/frontend/src/app/(public)/photos/[slug]/GalleryClient.tsx index ea39a09..ae8cbe1 100644 --- a/frontend/src/app/(public)/photos/[slug]/GalleryClient.tsx +++ b/frontend/src/app/(public)/photos/[slug]/GalleryClient.tsx @@ -15,6 +15,7 @@ import { MasonryGrid, } from '@/components/gallery/GalleryLayout'; import { useDownloads } from '@/components/gallery/useDownloads'; +import SaveSheet, { SavePhoto, useMobileSave } from '@/components/gallery/SaveSheet'; import Lightbox from '@/components/Lightbox'; import LoginModal from '@/components/LoginModal'; import { @@ -53,12 +54,30 @@ export default function GalleryClient({ slug, eventSlug, initial }: GalleryClien download: es ? 'Descargar' : 'Download', downloading: es ? 'Descargando…' : 'Downloading…', }; - const downloadPhoto = (photo: Photo) => + + // On a phone a download cannot reach the photo library, so the button + // opens a sheet offering the shareable preview instead (SaveSheet). + // Desktop keeps downloading the original straight away. + const mobileSave = useMobileSave(); + const [savePhoto, setSavePhoto] = useState(null); + + const downloadPhoto = (photo: Photo) => { + if (mobileSave) { + setSavePhoto({ + id: photo.id, + previewUrl: photo.urls.download || photo.urls.preview || photo.urls.original, + originalUrl: photo.urls.downloadOriginal || photo.urls.original, + previewSize: photo.previewSizeBytes, + originalSize: photo.sizeBytes, + }); + return; + } downloads.start({ id: photo.id, url: photo.urls.original, filename: photo.originalFilename, }); + }; // Server-rendered public galleries need no client fetch. Everything else // (link/ticket/private) is fetched here with the share token and/or the @@ -235,10 +254,12 @@ export default function GalleryClient({ slug, eventSlug, initial }: GalleryClien } const readyPhotos = photos.filter((p) => p.status === 'ready' && p.urls.thumb); + // previewUrl is the download endpoint's preview: rendering and saving from + // the same URL is what lets "Save photo" be answered by the HTTP cache. const lightboxItems = readyPhotos.map((p) => ({ id: p.id, - previewUrl: p.urls.preview || p.urls.original, - downloadUrl: p.urls.original, + previewUrl: p.urls.download || p.urls.preview || p.urls.original, + downloadUrl: p.urls.downloadOriginal || p.urls.original, filename: p.originalFilename, thumbUrl: p.urls.thumb, })); @@ -263,7 +284,11 @@ export default function GalleryClient({ slug, eventSlug, initial }: GalleryClien // first photo. Falls back to a navy gradient with no image. const coverPhoto = readyPhotos.find((p) => p.id === gallery.coverPhotoId) || readyPhotos[0] || null; - const heroUrl = coverPhoto ? coverPhoto.urls.preview || coverPhoto.urls.thumb : null; + // Same preview URL the lightbox uses, so the cover photo's bytes are + // fetched once for both. + const heroUrl = coverPhoto + ? coverPhoto.urls.download || coverPhoto.urls.preview || coverPhoto.urls.thumb + : null; return (
@@ -338,15 +363,20 @@ export default function GalleryClient({ slug, eventSlug, initial }: GalleryClien index={lightboxIndex} onClose={() => setLightboxIndex(null)} onNavigate={setLightboxIndex} - onDownload={(it) => - downloads.start({ id: it.id, url: it.downloadUrl, filename: it.filename }) - } + onDownload={(it) => { + const photo = readyPhotos.find((p) => p.id === it.id); + if (photo) downloadPhoto(photo); + }} downloadingId={ lightboxItems.find((it) => downloads.isPending(it.id))?.id ?? null } downloadLabels={downloadLabels} /> )} + + {/* Shared by both entry points: the tile button and the lightbox's. + Fetches nothing until it is open. */} + setSavePhoto(null)} /> {/* Call to action: send attendees to their dashboard, everyone else to diff --git a/frontend/src/components/Lightbox.tsx b/frontend/src/components/Lightbox.tsx index d9aa42f..444d72f 100644 --- a/frontend/src/components/Lightbox.tsx +++ b/frontend/src/components/Lightbox.tsx @@ -179,10 +179,14 @@ export default function Lightbox({ )} {/* eslint-disable-next-line @next/next/no-img-element */} + {/* Nothing is layered over the image (the navigation buttons sit at + the edges), and neither -webkit-touch-callout nor -webkit-user- + select is suppressed here, so iOS long-press → "Save to Photos" + still works as a backup to the save sheet. Don't add select-none. */} e.stopPropagation()} draggable={false} /> diff --git a/frontend/src/components/gallery/PhotoTile.tsx b/frontend/src/components/gallery/PhotoTile.tsx index 0dad9b6..be10d28 100644 --- a/frontend/src/components/gallery/PhotoTile.tsx +++ b/frontend/src/components/gallery/PhotoTile.tsx @@ -87,13 +87,15 @@ export default function PhotoTile({ aria-busy={downloading} aria-label={downloading ? labels.downloading : labels.download} className={clsx( - 'absolute bottom-2 right-2 hidden md:flex p-2 rounded-full bg-black/50 text-white transition-opacity hover:bg-black/80 focus:outline-none focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-primary-yellow', + 'absolute bottom-2 right-2 flex p-2.5 md:p-2 rounded-full bg-black/50 text-white transition-opacity hover:bg-black/80 focus:outline-none focus-visible:opacity-100 focus-visible:ring-2 focus-visible:ring-primary-yellow', + // Touch devices have no hover to reveal it, and this is where the + // button matters most — it is the entry point to the save sheet. downloading ? 'opacity-100 cursor-wait hover:bg-black/50' - : 'opacity-0 group-hover:opacity-100' + : 'opacity-100 md:opacity-0 md:group-hover:opacity-100' )} > - {downloading ? : } + {downloading ? : }
); diff --git a/frontend/src/components/gallery/SaveSheet.tsx b/frontend/src/components/gallery/SaveSheet.tsx new file mode 100644 index 0000000..30c0018 --- /dev/null +++ b/frontend/src/components/gallery/SaveSheet.tsx @@ -0,0 +1,370 @@ +'use client'; + +import { useCallback, useEffect, useRef, useState } from 'react'; +import { ArrowDownTrayIcon, PhotoIcon } from '@heroicons/react/24/outline'; +import { useLanguage } from '@/context/LanguageContext'; +import Spinner from '@/components/ui/Spinner'; + +// The mobile save flow. On a phone `` is a dead end for someone +// who wants the photo in their camera roll: iOS is WebKit everywhere, so it +// files the download under Files > Downloads and nothing else, and Android +// drops it in Downloads where Google Photos may or may not pick it up. The +// only route into the device gallery on either platform is the Web Share +// API, which hands the file to the native share sheet ("Save Image" on iOS, +// "Photos" on Android). +// +// So on mobile the download button opens this sheet instead of downloading, +// and the default row shares the 2048px preview — a file a phone screen +// cannot tell from the original, at a fraction of the bytes. The original +// stays one tap away for anyone who actually wants it. + +export interface SavePhoto { + id: string; + /** Preview download URL — the same URL the lightbox renders, so this is + * usually answered from the HTTP cache rather than the network. */ + previewUrl: string; + originalUrl: string; + /** Byte sizes from the gallery response, used to label the rows. */ + previewSize?: number; + originalSize?: number; +} + +type RowKey = 'preview' | 'original'; +type RowStatus = 'idle' | 'loading' | 'ready' | 'error'; + +interface RowState { + status: RowStatus; + /** 0..1, only tracked for the original's determinate progress bar. */ + progress: number; +} + +const IDLE: RowState = { status: 'idle', progress: 0 }; + +/** + * True on finger-first devices, i.e. where a plain download lands in + * Downloads instead of the photo library and this sheet is worth showing. + * Feature detection only — user agent strings lie, and iPad has been + * claiming to be a Mac for years. + * + * Deliberately NOT gated on `navigator.share`. Web Share only exists in a + * secure context, so requiring it here made the sheet disappear entirely on + * any plain-HTTP origin (a phone hitting the dev server over the LAN sees + * `navigator.share === undefined`) and on Firefox for Android, which has no + * file sharing. Both of those should still get the sheet — their rows just + * fall back to a download. Whether a given row shares or downloads is + * decided per file, at tap time, by canShareFile() below. + */ +export function useMobileSave(): boolean { + const [mobile, setMobile] = useState(false); + + useEffect(() => { + // Comma = OR. `hover: none` catches touch devices that report a fine + // pointer because a stylus is paired. + const mq = window.matchMedia('(pointer: coarse), (hover: none)'); + const sync = () => setMobile(mq.matches); + sync(); + mq.addEventListener('change', sync); + return () => mq.removeEventListener('change', sync); + }, []); + + return mobile; +} + +function canShareFile(file: File): boolean { + return typeof navigator.canShare === 'function' && navigator.canShare({ files: [file] }); +} + +/** Reads the server's Content-Disposition name (spanglish--.jpg). */ +function filenameFrom(header: string | null, fallback: string): string { + const match = header?.match(/filename\*?=(?:UTF-8'')?"?([^";]+)"?/i); + return match ? decodeURIComponent(match[1]) : fallback; +} + +function formatBytes(bytes: number | undefined, locale: string): string | null { + if (!bytes || bytes <= 0) return null; + const mb = bytes / 1_000_000; + if (mb < 1) { + return `${Math.round(bytes / 1000).toLocaleString(locale)} kB`; + } + return `${mb.toLocaleString(locale, { maximumFractionDigits: 1 })} MB`; +} + +/** + * Fetches a URL into a File. onProgress is only wired up when the caller + * wants a determinate bar — reading the body in chunks costs an extra copy, + * which is not worth it for the preview that is normally already cached. + */ +async function fetchAsFile( + url: string, + signal: AbortSignal, + onProgress?: (fraction: number) => void +): Promise { + // No `cache` option: the default is what lets the lightbox's already + // rendered preview be reused instead of refetched. + const res = await fetch(url, { credentials: 'same-origin', signal }); + if (!res.ok) throw new Error(`Download failed (${res.status})`); + + const type = res.headers.get('Content-Type') || 'image/jpeg'; + const name = filenameFrom(res.headers.get('Content-Disposition'), 'spanglish-photo.jpg'); + const total = Number(res.headers.get('Content-Length') || 0); + + let blob: Blob; + if (onProgress && total > 0 && res.body) { + const reader = res.body.getReader(); + const chunks: Uint8Array[] = []; + let received = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + chunks.push(value); + received += value.length; + onProgress(Math.min(1, received / total)); + } + blob = new Blob(chunks as BlobPart[], { type }); + } else { + blob = await res.blob(); + } + return new File([blob], name, { type: blob.type || type }); +} + +/** Fallback for browsers without file sharing: a plain download. */ +function saveViaAnchor(file: File) { + const objectUrl = URL.createObjectURL(file); + const a = document.createElement('a'); + a.href = objectUrl; + a.download = file.name; + document.body.appendChild(a); + a.click(); + a.remove(); + setTimeout(() => URL.revokeObjectURL(objectUrl), 10_000); +} + +interface SaveSheetProps { + /** The photo to save; null keeps the sheet closed. */ + photo: SavePhoto | null; + onClose: () => void; +} + +export default function SaveSheet({ photo, onClose }: SaveSheetProps) { + if (!photo) return null; + // Keyed by photo so a sheet opened for a different photo starts clean + // instead of inheriting the previous one's fetched file. + return ; +} + +function SaveSheetPanel({ photo, onClose }: { photo: SavePhoto; onClose: () => void }) { + const { t, locale } = useLanguage(); + const [rows, setRows] = useState>({ + preview: IDLE, + original: IDLE, + }); + + // Resolved files live in a ref as well as state: the tap handler has to + // read them *synchronously*. iOS rejects navigator.share() with + // NotAllowedError if anything is awaited between the tap and the call, + // so there is no chance to read them out of a promise first. + const files = useRef>>({}); + const aborter = useRef(new AbortController()); + const closed = useRef(false); + + const setRow = useCallback((key: RowKey, patch: Partial) => { + if (closed.current) return; + setRows((prev) => ({ ...prev, [key]: { ...prev[key], ...patch } })); + }, []); + + const prepare = useCallback( + async (key: RowKey): Promise => { + setRow(key, { status: 'loading', progress: 0 }); + try { + const file = await fetchAsFile( + key === 'preview' ? photo.previewUrl : photo.originalUrl, + aborter.current.signal, + // Only the original gets a determinate bar; the preview is + // normally cached and resolves before a spinner would even paint. + key === 'original' ? (progress) => setRow(key, { progress }) : undefined + ); + files.current[key] = file; + setRow(key, { status: 'ready', progress: 1 }); + return file; + } catch (err) { + // Aborting is how the sheet closes and how the user dismisses the + // native share sheet — neither is an error worth reporting. + if ((err as Error)?.name === 'AbortError') return null; + setRow(key, { status: 'error', progress: 0 }); + return null; + } + }, + [photo.previewUrl, photo.originalUrl, setRow] + ); + + // Nothing is fetched until the sheet is open — not on lightbox open, not + // on swipe, not on scroll. Browsing the whole gallery without tapping + // save costs exactly the thumbs and previews already on screen. + useEffect(() => { + // The controller is created here, not at render: closing the sheet + // aborts it, so a re-run of this effect (React StrictMode double-invokes + // it in development) has to start from a fresh, unaborted one. + closed.current = false; + const controller = new AbortController(); + aborter.current = controller; + void prepare('preview'); + return () => { + closed.current = true; + controller.abort(); + }; + }, [prepare]); + + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }, [onClose]); + + const deliver = (file: File) => { + if (canShareFile(file)) { + // Called with nothing awaited in front of it — see the ref above. + navigator + .share({ files: [file] }) + .then(() => onClose()) + .catch((err: Error) => { + // AbortError just means the share sheet was dismissed. + if (err?.name !== 'AbortError') saveViaAnchor(file); + }); + return; + } + saveViaAnchor(file); + onClose(); + }; + + const onRowTap = (key: RowKey) => { + const ready = files.current[key]; + if (ready) { + deliver(ready); + return; + } + if (rows[key].status === 'loading') return; + // Not fetched yet (or it failed): fetch now and stop. The row goes back + // to tappable when it resolves, so the share happens on a fresh + // gesture rather than a stale one iOS would refuse. + void prepare(key); + }; + + const sizes = { + preview: formatBytes(photo.previewSize, locale), + original: formatBytes(photo.originalSize, locale), + }; + + return ( +
+ +
+ + ); +} + +function SaveRow({ + icon, + label, + hint, + size, + state, + statusLabels, + showProgress, + onTap, +}: { + icon: React.ReactNode; + label: string; + hint: string; + size: string | null; + state: RowState; + statusLabels: (key: string) => string; + showProgress?: boolean; + onTap: () => void; +}) { + const loading = state.status === 'loading'; + // The subtitle carries the row's state, so a slow fetch explains itself + // without the row ever becoming untappable for good. + const subtitle = + state.status === 'loading' + ? statusLabels('gallery.save.preparing') + : state.status === 'error' + ? statusLabels('gallery.save.failed') + : [hint, size].filter(Boolean).join(' · '); + + return ( + + ); +} diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 572d9c1..916d6f2 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -367,5 +367,20 @@ "title": "TikTok", "subtitle": "Videos & fun content" } + }, + "gallery": { + "save": { + "sheetTitle": "Save photo", + "photo": "Save photo", + "photoHint": "Best for phone", + "original": "Download original", + "originalHint": "Full quality", + "cancel": "Cancel", + "preparing": "Preparing…", + "ready": "Ready — tap to save", + "failed": "Could not prepare it. Tap to retry.", + "download": "Download", + "downloading": "Downloading…" + } } } diff --git a/frontend/src/i18n/locales/es.json b/frontend/src/i18n/locales/es.json index 00efd3c..fed3414 100644 --- a/frontend/src/i18n/locales/es.json +++ b/frontend/src/i18n/locales/es.json @@ -367,5 +367,20 @@ "title": "TikTok", "subtitle": "Videos y contenido divertido" } + }, + "gallery": { + "save": { + "sheetTitle": "Guardar foto", + "photo": "Guardar foto", + "photoHint": "Ideal para el teléfono", + "original": "Descargar original", + "originalHint": "Calidad completa", + "cancel": "Cancelar", + "preparing": "Preparando…", + "ready": "Lista: toca para guardar", + "failed": "No se pudo preparar. Toca para reintentar.", + "download": "Descargar", + "downloading": "Descargando…" + } } } diff --git a/frontend/src/lib/api/photos.ts b/frontend/src/lib/api/photos.ts index e2e9a39..0409ad1 100644 --- a/frontend/src/lib/api/photos.ts +++ b/frontend/src/lib/api/photos.ts @@ -41,6 +41,8 @@ export interface Photo { originalFilename?: string; contentType: string; sizeBytes: number; + /** Size of the preview variant; absent until the server has measured it. */ + previewSizeBytes?: number; width?: number; height?: number; takenAt?: string; @@ -51,6 +53,15 @@ export interface Photo { thumb?: string; preview?: string; original: string; + /** + * Download endpoints. Unlike `preview`/`original` these always stream + * same-origin instead of redirecting to S3, so fetch() can read them + * into a Blob (needed by the mobile share sheet) without CORS. + * `download` serves the preview and is what the lightbox renders, so + * saving it hits the HTTP cache instead of the network. + */ + download?: string; + downloadOriginal: string; }; // Upload responses only: these bytes were already in the gallery, so nothing // was stored and the rest of this object describes the existing photo. diff --git a/frontend/tailwind.config.js b/frontend/tailwind.config.js index 811cc61..4656f94 100644 --- a/frontend/tailwind.config.js +++ b/frontend/tailwind.config.js @@ -37,6 +37,18 @@ module.exports = { 'card': '0 4px 6px -1px rgba(0, 0, 0, 0.1), 0 2px 4px -1px rgba(0, 0, 0, 0.06)', 'card-hover': '0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05)', }, + // Entrance for the mobile save sheet (components/gallery/SaveSheet). + keyframes: { + 'sheet-fade': { from: { opacity: '0' }, to: { opacity: '1' } }, + 'sheet-rise': { + from: { transform: 'translateY(100%)' }, + to: { transform: 'translateY(0)' }, + }, + }, + animation: { + 'sheet-fade': 'sheet-fade 150ms ease-out', + 'sheet-rise': 'sheet-rise 200ms ease-out', + }, }, }, plugins: [], diff --git a/photo-api/internal/httpapi/api_test.go b/photo-api/internal/httpapi/api_test.go index 60aa08e..752d556 100644 --- a/photo-api/internal/httpapi/api_test.go +++ b/photo-api/internal/httpapi/api_test.go @@ -750,3 +750,177 @@ func TestSlugCollision(t *testing.T) { t.Fatalf("default visibility: %s", a.Visibility) } } + +// TestDownloadEndpoint pins the guarantees the mobile save sheet is built +// on: the bytes arrive same-origin (never a redirect), typed as an image, +// named after the event, sized up front and cacheable forever. +func TestDownloadEndpoint(t *testing.T) { + e := setup(t) + admin := e.makeToken(t, uAdmin) + g := createGallery(t, e, admin, map[string]any{ + "title": "Fotos Del Evento", "visibility": "public", "eventId": evPaid, + }) + p := uploadPhoto(t, e, admin, g.ID, testJPEG(t)) + if processQueue(t, e, p.ID).Status != "ready" { + t.Fatal("processing failed") + } + + base := "/api/photos/files/" + p.ID + "/download" + for _, tc := range []struct { + name, path, wantDisposition, wantFilename string + }{ + // No ?size at all must behave exactly like ?size=preview. + {"default", base, "inline", "spanglish-fiesta-1.jpg"}, + {"preview", base + "?size=preview", "inline", "spanglish-fiesta-1.jpg"}, + {"original", base + "?size=original", "attachment", "spanglish-fiesta-1.jpg"}, + } { + t.Run(tc.name, func(t *testing.T) { + w := e.request(t, "GET", tc.path, "", nil) + if w.Code != 200 { + t.Fatalf("status %d %s", w.Code, w.Body.String()) + } + if ct := w.Header().Get("Content-Type"); ct != "image/jpeg" { + t.Fatalf("content type %q: iOS only offers Save Image for image/*", ct) + } + if n := w.Header().Get("Content-Length"); n != fmt.Sprint(w.Body.Len()) || n == "0" { + t.Fatalf("content length %q, body %d bytes", n, w.Body.Len()) + } + cd := w.Header().Get("Content-Disposition") + if !strings.HasPrefix(cd, tc.wantDisposition) || !strings.Contains(cd, tc.wantFilename) { + t.Fatalf("disposition %q, want %s with %s", cd, tc.wantDisposition, tc.wantFilename) + } + if cc := w.Header().Get("Cache-Control"); cc != "public, max-age=31536000, immutable" { + t.Fatalf("cache-control %q", cc) + } + }) + } + + // Each size serves its own variant byte-for-byte. (No size comparison + // between the two here: the fixture is a synthetic 800x600 image, so its + // 2048px preview re-encode is larger than the "original" — which says + // nothing about the multi-megapixel photos the sheet exists for.) + prev := e.request(t, "GET", base, "", nil) + orig := e.request(t, "GET", base+"?size=original", "", nil) + if !bytes.Equal(prev.Body.Bytes(), e.request(t, "GET", "/api/photos/files/"+p.ID+"/preview", "", nil).Body.Bytes()) { + t.Fatal("download?size=preview must serve the same bytes as the preview variant") + } + if !bytes.Equal(orig.Body.Bytes(), e.request(t, "GET", "/api/photos/files/"+p.ID+"/original", "", nil).Body.Bytes()) { + t.Fatal("download?size=original must serve the same bytes as the original variant") + } + + // An unknown size is not a silent fallback. + if w := e.request(t, "GET", base+"?size=thumb", "", nil); w.Code != 404 { + t.Fatalf("unknown size: want 404, got %d", w.Code) + } + + // The gallery response labels both rows without any extra request. + detail := decode[galleryResp](t, e.request(t, "GET", "/api/photos/public/galleries/"+g.Slug, "", nil)) + got := detail.Photos[0] + if got.PreviewSizeBytes != int64(prev.Body.Len()) { + t.Fatalf("previewSizeBytes %d, served %d", got.PreviewSizeBytes, prev.Body.Len()) + } + if got.SizeBytes != int64(orig.Body.Len()) { + t.Fatalf("sizeBytes %d, served %d", got.SizeBytes, orig.Body.Len()) + } + if got.URLs.Download != base { + t.Fatalf("download url %q, want %q", got.URLs.Download, base) + } + if got.URLs.DownloadOriginal != base+"?size=original" { + t.Fatalf("downloadOriginal url %q", got.URLs.DownloadOriginal) + } +} + +// A restricted gallery's download must obey the same token check as every +// other byte-serving route, and must not become shared-cacheable. +func TestDownloadRespectsAccess(t *testing.T) { + e := setup(t) + admin := e.makeToken(t, uAdmin) + g := createGallery(t, e, admin, map[string]any{"title": "Privada Descarga", "visibility": "private"}) + p := uploadPhoto(t, e, admin, g.ID, testJPEG(t)) + if processQueue(t, e, p.ID).Status != "ready" { + t.Fatal("processing failed") + } + + detail := decode[galleryResp](t, e.request(t, "GET", "/api/photos/galleries/"+g.ID, admin, nil)) + url := detail.Photos[0].URLs.Download + if !strings.Contains(url, "?token=v1.") { + t.Fatalf("private download URL should carry a view token: %s", url) + } + // ?size and ?token have to coexist on the original's URL. + if o := detail.Photos[0].URLs.DownloadOriginal; !strings.Contains(o, "?size=original&token=v1.") { + t.Fatalf("private original download URL: %s", o) + } + + w := e.request(t, "GET", url, "", nil) + if w.Code != 200 { + t.Fatalf("view token fetch: want 200, got %d", w.Code) + } + // Restricted galleries stay out of shared caches; the browser's own + // cache (which is what the save sheet reuses) still applies. + if cc := w.Header().Get("Cache-Control"); cc != "private, max-age=31536000, immutable" { + t.Fatalf("cache-control %q must not be public for a private gallery", cc) + } + bare := strings.SplitN(url, "?", 2)[0] + if w := e.request(t, "GET", bare, "", nil); w.Code != 403 { + t.Fatalf("anon download without token: want 403, got %d", w.Code) + } + if w := e.request(t, "GET", bare+"?size=original", "", nil); w.Code != 403 { + t.Fatalf("anon original download without token: want 403, got %d", w.Code) + } +} + +// Photos processed before preview_size_bytes existed have no stored size. +// The first gallery view measures them and writes the result back, so no +// separate backfill command is needed for an existing library. +func TestPreviewSizeBackfilledOnRead(t *testing.T) { + e := setup(t) + ctx := context.Background() + admin := e.makeToken(t, uAdmin) + g := createGallery(t, e, admin, map[string]any{"title": "Antigua", "visibility": "public"}) + p := uploadPhoto(t, e, admin, g.ID, testJPEG(t)) + if processQueue(t, e, p.ID).Status != "ready" { + t.Fatal("processing failed") + } + + // Rewind to what an pre-migration row looks like. + if _, err := e.db.ExecContext(ctx, e.db.Rebind( + "UPDATE photos_photos SET preview_size_bytes = NULL WHERE id = ?"), p.ID); err != nil { + t.Fatal(err) + } + if before, err := e.db.GetPhoto(ctx, p.ID); err != nil { + t.Fatal(err) + } else if before.PreviewSizeBytes != 0 { + t.Fatalf("setup: want an unmeasured row, got %d", before.PreviewSizeBytes) + } + + served := e.request(t, "GET", "/api/photos/files/"+p.ID+"/download", "", nil).Body.Len() + got := decode[galleryResp](t, e.request(t, "GET", "/api/photos/public/galleries/"+g.Slug, "", nil)) + if got.Photos[0].PreviewSizeBytes != int64(served) { + t.Fatalf("response size %d, served %d", got.Photos[0].PreviewSizeBytes, served) + } + // …and it was persisted, so the next view costs no Stat. + after, err := e.db.GetPhoto(ctx, p.ID) + if err != nil { + t.Fatal(err) + } + if after.PreviewSizeBytes != int64(served) { + t.Fatalf("stored size %d, served %d", after.PreviewSizeBytes, served) + } +} + +// Originals uploaded as HEIC or PNG must keep their real type and extension, +// and a row with a missing/generic type must still go out as an image. +func TestDownloadContentTypeNeverOctetStream(t *testing.T) { + for _, tc := range []struct{ ct, name, want string }{ + {"image/heic", "IMG_1.HEIC", "image/heic"}, + {"image/png", "shot.png", "image/png"}, + {"application/octet-stream", "IMG_2.heic", "image/heic"}, + {"application/octet-stream", "scan.PNG", "image/png"}, + {"", "photo.jpeg", "image/jpeg"}, + {"", "", "image/jpeg"}, + } { + if got := imageContentType(tc.ct, tc.name); got != tc.want { + t.Errorf("imageContentType(%q, %q) = %q, want %q", tc.ct, tc.name, got, tc.want) + } + } +} diff --git a/photo-api/internal/httpapi/dto.go b/photo-api/internal/httpapi/dto.go index b64e482..ddc6002 100644 --- a/photo-api/internal/httpapi/dto.go +++ b/photo-api/internal/httpapi/dto.go @@ -42,6 +42,13 @@ type photoURLs struct { Thumb string `json:"thumb,omitempty"` Preview string `json:"preview,omitempty"` Original string `json:"original"` + // Download / DownloadOriginal hit the download endpoint (files.go), which + // always streams the bytes same-origin instead of redirecting to S3, so a + // fetch() can read them into a Blob without CORS. Download serves the + // preview variant and doubles as the lightbox's source, so the save + // fetch is answered from the HTTP cache rather than the network. + Download string `json:"download,omitempty"` + DownloadOriginal string `json:"downloadOriginal"` } type photoJSON struct { @@ -51,6 +58,9 @@ type photoJSON struct { OriginalFilename string `json:"originalFilename,omitempty"` ContentType string `json:"contentType"` SizeBytes int64 `json:"sizeBytes"` + // PreviewSizeBytes lets the mobile save sheet label both of its rows with + // a real file size without fetching anything. Omitted while unknown. + PreviewSizeBytes int64 `json:"previewSizeBytes,omitempty"` Width int `json:"width,omitempty"` Height int `json:"height,omitempty"` TakenAt string `json:"takenAt,omitempty"` @@ -91,6 +101,22 @@ func fileURL(photoID, variant, token string) string { return u } +// downloadURL builds the download endpoint's URL. size is left off for the +// preview, which the endpoint serves by default — a shorter, stabler string +// for the URL the lightbox also renders. +func downloadURL(photoID, size, token string) string { + u := "/api/photos/files/" + photoID + "/download" + sep := "?" + if size != "" && size != downloadSizePreview { + u += "?size=" + size + sep = "&" + } + if token != "" { + u += sep + "token=" + token + } + return u +} + func (s *Server) photoToJSON(p store.Photo, token string, admin bool) photoJSON { out := photoJSON{ ID: p.ID, @@ -101,16 +127,21 @@ func (s *Server) photoToJSON(p store.Photo, token string, admin bool) photoJSON SizeBytes: p.SizeBytes, Width: p.Width, Height: p.Height, + PreviewSizeBytes: p.PreviewSizeBytes, TakenAt: isoTime(p.TakenAt), Status: p.Status, CreatedAt: isoTime(p.CreatedAt), - URLs: photoURLs{Original: fileURL(p.ID, "original", token)}, + URLs: photoURLs{ + Original: fileURL(p.ID, "original", token), + DownloadOriginal: downloadURL(p.ID, downloadSizeOriginal, token), + }, } if p.ThumbKey != "" { out.URLs.Thumb = fileURL(p.ID, "thumb", token) } if p.PreviewKey != "" { out.URLs.Preview = fileURL(p.ID, "preview", token) + out.URLs.Download = downloadURL(p.ID, downloadSizePreview, token) } if admin { out.LastError = p.LastError diff --git a/photo-api/internal/httpapi/files.go b/photo-api/internal/httpapi/files.go index 2ee44ea..d081d26 100644 --- a/photo-api/internal/httpapi/files.go +++ b/photo-api/internal/httpapi/files.go @@ -7,13 +7,24 @@ import ( "log" "net/http" "os" + "strings" "time" "git.azzamo.net/Michilis/Spanglish/photo-api/internal/storage" + "git.azzamo.net/Michilis/Spanglish/photo-api/internal/store" ) const presignExpiry = 15 * time.Minute +// Sizes the download endpoint offers. Preview is the default because it is +// what a phone can actually save to its photo library: iOS and Android only +// reach the gallery through the share sheet, and the share sheet is worth +// handing a 2048px JPEG rather than a multi-megabyte original. +const ( + downloadSizePreview = "preview" + downloadSizeOriginal = "original" +) + // serveFile delivers photo bytes after re-running the gallery access check. // S3: 302 to a short-lived presigned URL; local: streamed directly. func (s *Server) serveFile(w http.ResponseWriter, r *http.Request) { @@ -22,23 +33,10 @@ func (s *Server) serveFile(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "Not Found") return } - p, err := s.db.GetPhoto(r.Context(), r.PathValue("photoId")) - if err != nil { - writeStoreError(w, err, "Photo not found") + p, _, ok := s.authorizedPhoto(w, r) + if !ok { return } - g, err := s.db.GetGallery(r.Context(), p.GalleryID) - if err != nil { - writeStoreError(w, err, "Photo not found") - return - } - user := s.optionalUser(r) - token := r.URL.Query().Get("token") - if denial := s.authorize(r, g, user, token); denial != nil { - writeError(w, denial.status, denial.msg) - return - } - isAdmin := user != nil && user.IsAdmin() var key, contentType, downloadName string switch variant { @@ -58,12 +56,6 @@ func (s *Server) serveFile(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusNotFound, "Not ready") return } - // Non-ready originals stay admin-only so uploads that fail processing - // never leak to viewers. - if p.Status != "ready" && !isAdmin { - writeError(w, http.StatusNotFound, "Not ready") - return - } if url, err := s.storage.PresignGet(r.Context(), key, downloadName, contentType, presignExpiry); err == nil { http.Redirect(w, r, url, http.StatusFound) @@ -100,6 +92,155 @@ func (s *Server) serveFile(w http.ResponseWriter, r *http.Request) { } } +// serveDownload backs the gallery's save/download actions. It differs from +// serveFile in three ways that the mobile save flow depends on: +// +// - It always streams. serveFile 302s to S3 when presigning is available, +// and a cross-origin redirect is opaque to fetch() — the client could +// never read the bytes into a Blob to hand to navigator.share(). +// - It names the file (spanglish--.jpg) and always declares a +// real image/* type, because iOS only offers "Save Image" in the share +// sheet for something it recognises as an image. +// - It caches for a year as immutable. ?size=preview is also the URL the +// lightbox renders, so tapping save re-reads the bytes already on screen +// out of the HTTP cache instead of fetching them again. +func (s *Server) serveDownload(w http.ResponseWriter, r *http.Request) { + size := r.URL.Query().Get("size") + if size == "" { + size = downloadSizePreview + } + if size != downloadSizePreview && size != downloadSizeOriginal { + writeError(w, http.StatusNotFound, "Not Found") + return + } + p, g, ok := s.authorizedPhoto(w, r) + if !ok { + return + } + + key, contentType := p.PreviewKey, "image/jpeg" + if size == downloadSizeOriginal { + key, contentType = p.OriginalKey, imageContentType(p.ContentType, p.OriginalFilename) + } + if key == "" { + writeError(w, http.StatusNotFound, "Not ready") + return + } + + reader, n, err := s.storage.Open(r.Context(), key) + if err != nil { + if os.IsNotExist(err) || errors.Is(err, storage.ErrNotExist) { + writeError(w, http.StatusNotFound, "Not Found") + return + } + log.Printf("Error: open %s: %v", key, err) + writeError(w, http.StatusInternalServerError, "Internal Server Error") + return + } + defer reader.Close() + + w.Header().Set("Content-Type", contentType) + w.Header().Set("Content-Length", fmt.Sprintf("%d", n)) + w.Header().Set("X-Content-Type-Options", "nosniff") + // The preview is rendered as an as well as saved, so it is marked + // inline; only the original is a pure download. Either way the filename + // travels with it, which is what a bare fallback and the + // Android share sheet display. + disposition := "inline" + if size == downloadSizeOriginal { + disposition = "attachment" + } + w.Header().Set("Content-Disposition", fmt.Sprintf("%s; filename=%q", + disposition, s.downloadFilename(r, g, p, size))) + // Objects are immutable (a new upload writes a new key), so the response + // can be cached for as long as the browser will keep it. Restricted + // galleries stay out of shared caches: access is re-checked per request + // here, and only the browser that passed the check may reuse the bytes. + scope := "private" + if g.Visibility == store.VisibilityPublic { + scope = "public" + } + w.Header().Set("Cache-Control", scope+", max-age=31536000, immutable") + + if _, err := io.Copy(w, reader); err != nil { + // Routine on mobile: the client aborts the fetch when the save sheet + // is cancelled, which lands here as a broken pipe. + log.Printf("stream %s: %v", key, err) + } +} + +// authorizedPhoto loads the photo and its gallery and re-runs the access +// check every byte-serving handler owes (PLAN.md §7). It writes the error +// response itself; ok=false means the caller is done. +func (s *Server) authorizedPhoto(w http.ResponseWriter, r *http.Request) (store.Photo, store.Gallery, bool) { + p, err := s.db.GetPhoto(r.Context(), r.PathValue("photoId")) + if err != nil { + writeStoreError(w, err, "Photo not found") + return store.Photo{}, store.Gallery{}, false + } + g, err := s.db.GetGallery(r.Context(), p.GalleryID) + if err != nil { + writeStoreError(w, err, "Photo not found") + return store.Photo{}, store.Gallery{}, false + } + user := s.optionalUser(r) + if denial := s.authorize(r, g, user, r.URL.Query().Get("token")); denial != nil { + writeError(w, denial.status, denial.msg) + return store.Photo{}, store.Gallery{}, false + } + // Non-ready originals stay admin-only so uploads that fail processing + // never leak to viewers. + if p.Status != "ready" && !(user != nil && user.IsAdmin()) { + writeError(w, http.StatusNotFound, "Not ready") + return store.Photo{}, store.Gallery{}, false + } + return p, g, true +} + +// downloadFilename names the saved file after the event it came from rather +// than the photographer's IMG_1234.JPG: spanglish--., +// numbered from 1 in gallery order. Standalone galleries use their own slug. +func (s *Server) downloadFilename(r *http.Request, g store.Gallery, p store.Photo, size string) string { + slug := g.Slug + if g.EventID != "" { + if ev, err := s.db.GetEventSummary(r.Context(), g.EventID); err == nil && ev.Slug != "" { + slug = ev.Slug + } + } + ext := ".jpg" + if size == downloadSizeOriginal { + if e := extForContentType(p.ContentType); e != "" { + ext = e + } + } + return fmt.Sprintf("spanglish-%s-%d%s", slug, p.Position+1, ext) +} + +// imageContentType keeps the download endpoint's Content-Type in the image/* +// family. An original stored with a missing or generic type would otherwise +// go out as application/octet-stream, and iOS drops "Save Image" from the +// share sheet for anything it cannot see as an image. +func imageContentType(ct, filename string) string { + if strings.HasPrefix(ct, "image/") { + return ct + } + if i := strings.LastIndex(filename, "."); i >= 0 { + switch strings.ToLower(filename[i:]) { + case ".jpg", ".jpeg": + return "image/jpeg" + case ".png": + return "image/png" + case ".gif": + return "image/gif" + case ".webp": + return "image/webp" + case ".heic", ".heif": + return "image/heic" + } + } + return "image/jpeg" +} + func extForContentType(ct string) string { switch ct { case "image/jpeg": diff --git a/photo-api/internal/httpapi/galleries.go b/photo-api/internal/httpapi/galleries.go index 2158adf..00fba47 100644 --- a/photo-api/internal/httpapi/galleries.go +++ b/photo-api/internal/httpapi/galleries.go @@ -106,6 +106,7 @@ func (s *Server) getGallery(w http.ResponseWriter, r *http.Request, _ auth.User) writeStoreError(w, err, "") return } + s.fillPreviewSizes(r.Context(), photos) urlToken := s.viewTokenFor(g) out := make([]photoJSON, 0, len(photos)) for _, p := range photos { diff --git a/photo-api/internal/httpapi/public.go b/photo-api/internal/httpapi/public.go index 56156a1..83f9263 100644 --- a/photo-api/internal/httpapi/public.go +++ b/photo-api/internal/httpapi/public.go @@ -66,6 +66,7 @@ func (s *Server) respondGalleryView(w http.ResponseWriter, r *http.Request, g st writeStoreError(w, err, "") return } + s.fillPreviewSizes(r.Context(), photos) out := make([]photoJSON, 0, len(photos)) for _, p := range photos { out = append(out, s.photoToJSON(p, urlToken, false)) diff --git a/photo-api/internal/httpapi/server.go b/photo-api/internal/httpapi/server.go index 065f186..8161017 100644 --- a/photo-api/internal/httpapi/server.go +++ b/photo-api/internal/httpapi/server.go @@ -52,6 +52,9 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("GET /api/photos/public/galleries", s.listPublicGalleries) mux.HandleFunc("GET /api/photos/public/galleries/{slug}", s.getPublicGallery) mux.HandleFunc("GET /api/photos/public/events/{eventSlug}/gallery", s.getEventGallery) + // The literal "download" segment takes precedence over {variant} under + // ServeMux's specificity rules, so the two can share the prefix. + mux.HandleFunc("GET /api/photos/files/{photoId}/download", s.serveDownload) mux.HandleFunc("GET /api/photos/files/{photoId}/{variant}", s.serveFile) return s.withCommon(mux) diff --git a/photo-api/internal/httpapi/sizes.go b/photo-api/internal/httpapi/sizes.go new file mode 100644 index 0000000..45c7b75 --- /dev/null +++ b/photo-api/internal/httpapi/sizes.go @@ -0,0 +1,64 @@ +package httpapi + +import ( + "context" + "log" + "sync" + + "git.azzamo.net/Michilis/Spanglish/photo-api/internal/store" +) + +// statConcurrency bounds the one-off Stat burst below; a gallery of ~70 +// photos then costs a handful of round-trips against S3, once ever. +const statConcurrency = 8 + +// fillPreviewSizes measures the preview objects of photos whose +// preview_size_bytes is still unset and writes the result back, updating the +// slice in place. +// +// The worker records the size when it generates the preview, so this only +// ever fires for rows that predate the column (or arrived via `photo-api +// sync`). It is a self-healing cache fill rather than a migration step: the +// first gallery view after deploy pays for one Stat per photo, every view +// after that reads the stored value. Errors are logged and swallowed — a +// missing size costs the save sheet its row label, which is not worth +// failing a gallery response over. +func (s *Server) fillPreviewSizes(ctx context.Context, photos []store.Photo) { + var ( + wg sync.WaitGroup + sem = make(chan struct{}, statConcurrency) + mu sync.Mutex + size = make(map[string]int64, len(photos)) + ) + for _, p := range photos { + if p.PreviewKey == "" || p.PreviewSizeBytes > 0 { + continue + } + wg.Add(1) + go func(p store.Photo) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + n, err := s.storage.Stat(ctx, p.PreviewKey) + if err != nil { + log.Printf("preview size %s: %v", p.PreviewKey, err) + return + } + mu.Lock() + size[p.ID] = n + mu.Unlock() + }(p) + } + wg.Wait() + if len(size) == 0 { + return + } + for i := range photos { + if n, ok := size[photos[i].ID]; ok { + photos[i].PreviewSizeBytes = n + if err := s.db.SetPreviewSize(ctx, photos[i].ID, n); err != nil { + log.Printf("store preview size %s: %v", photos[i].ID, err) + } + } + } +} diff --git a/photo-api/internal/photosync/run_test.go b/photo-api/internal/photosync/run_test.go index 73aca30..f09f2ae 100644 --- a/photo-api/internal/photosync/run_test.go +++ b/photo-api/internal/photosync/run_test.go @@ -161,7 +161,7 @@ func syncEnv(t *testing.T) (config.Config, *store.DB, *stubS3, []string) { }); err != nil { t.Fatal(err) } - if err := db.MarkPhotoReady(ctx, photoID, thumbKey, previewKey, 100, 80, now); err != nil { + if err := db.MarkPhotoReady(ctx, photoID, thumbKey, previewKey, 7, 100, 80, now); err != nil { t.Fatal(err) } return cfg, db, stub, []string{origKey, thumbKey, previewKey} diff --git a/photo-api/internal/store/photos.go b/photo-api/internal/store/photos.go index b563aed..dd461b6 100644 --- a/photo-api/internal/store/photos.go +++ b/photo-api/internal/store/photos.go @@ -22,6 +22,9 @@ type Photo struct { Height int ThumbKey string PreviewKey string + // PreviewSizeBytes is the size of the preview object, 0 when it has not + // been measured yet (rows written before the column existed). + PreviewSizeBytes int64 TakenAt time.Time Status string Attempts int @@ -35,11 +38,11 @@ type Photo struct { } const photoColumns = `id, gallery_id, position, original_key, original_filename, content_type, - size_bytes, width, height, thumb_key, preview_key, taken_at, status, attempts, next_attempt_at, - last_error, created_at, updated_at, checksum` + size_bytes, width, height, thumb_key, preview_key, preview_size_bytes, taken_at, status, + attempts, next_attempt_at, last_error, created_at, updated_at, checksum` func scanPhoto(s scanner) (Photo, error) { - var v [19]any + var v [20]any dest := make([]any, len(v)) for i := range v { dest[i] = &v[i] @@ -59,14 +62,15 @@ func scanPhoto(s scanner) (Photo, error) { Height: int(asInt(v[8])), ThumbKey: asString(v[9]), PreviewKey: asString(v[10]), - TakenAt: asTime(v[11]), - Status: asString(v[12]), - Attempts: int(asInt(v[13])), - NextAttemptAt: asTime(v[14]), - LastError: asString(v[15]), - CreatedAt: asTime(v[16]), - UpdatedAt: asTime(v[17]), - Checksum: asString(v[18]), + PreviewSizeBytes: asInt(v[11]), + TakenAt: asTime(v[12]), + Status: asString(v[13]), + Attempts: int(asInt(v[14])), + NextAttemptAt: asTime(v[15]), + LastError: asString(v[16]), + CreatedAt: asTime(v[17]), + UpdatedAt: asTime(v[18]), + Checksum: asString(v[19]), }, nil } @@ -323,17 +327,26 @@ func (db *DB) ClaimNextPhoto(ctx context.Context) (Photo, error) { return db.GetPhoto(ctx, id) } -func (db *DB) MarkPhotoReady(ctx context.Context, id, thumbKey, previewKey string, width, height int, takenAt time.Time) error { +func (db *DB) MarkPhotoReady(ctx context.Context, id, thumbKey, previewKey string, previewSize int64, width, height int, takenAt time.Time) error { var takenArg any if !takenAt.IsZero() { takenArg = db.TimeArg(takenAt) } _, err := db.ExecContext(ctx, db.Rebind(` UPDATE photos_photos - SET status = 'ready', thumb_key = ?, preview_key = ?, width = ?, height = ?, taken_at = ?, - last_error = NULL, updated_at = ? + SET status = 'ready', thumb_key = ?, preview_key = ?, preview_size_bytes = ?, width = ?, + height = ?, taken_at = ?, last_error = NULL, updated_at = ? WHERE id = ?`), - thumbKey, previewKey, width, height, takenArg, db.TimeArg(time.Now()), id) + thumbKey, previewKey, previewSize, width, height, takenArg, db.TimeArg(time.Now()), id) + return err +} + +// SetPreviewSize records the measured size of an already-generated preview. +// Used to fill in rows processed before the column existed; failure to write +// is not worth failing a read over, so callers may ignore the error. +func (db *DB) SetPreviewSize(ctx context.Context, id string, size int64) error { + _, err := db.ExecContext(ctx, db.Rebind( + "UPDATE photos_photos SET preview_size_bytes = ? WHERE id = ?"), size, id) return err } diff --git a/photo-api/internal/worker/worker.go b/photo-api/internal/worker/worker.go index be7e455..2097525 100644 --- a/photo-api/internal/worker/worker.go +++ b/photo-api/internal/worker/worker.go @@ -146,14 +146,17 @@ func (w *Worker) process(ctx context.Context, p store.Photo) error { prefix := fmt.Sprintf("galleries/%s", p.GalleryID) thumbKey := fmt.Sprintf("%s/thumb/%s.jpg", prefix, base) previewKey := fmt.Sprintf("%s/preview/%s.jpg", prefix, base) - if err := w.upload(ctx, thumbKey, thumbPath); err != nil { + if _, err := w.upload(ctx, thumbKey, thumbPath); err != nil { return fmt.Errorf("store thumb: %w", err) } - if err := w.upload(ctx, previewKey, previewPath); err != nil { + // The preview's size is recorded because the gallery response labels the + // mobile save sheet with it; measuring it here costs nothing. + previewSize, err := w.upload(ctx, previewKey, previewPath) + if err != nil { return fmt.Errorf("store preview: %w", err) } - return w.db.MarkPhotoReady(ctx, p.ID, thumbKey, previewKey, res.Width, res.Height, res.TakenAt) + return w.db.MarkPhotoReady(ctx, p.ID, thumbKey, previewKey, previewSize, res.Width, res.Height, res.TakenAt) } func (w *Worker) download(ctx context.Context, key, dst string) error { @@ -171,15 +174,19 @@ func (w *Worker) download(ctx context.Context, key, dst string) error { return err } -func (w *Worker) upload(ctx context.Context, key, src string) error { +// upload stores a generated variant and reports how many bytes it holds. +func (w *Worker) upload(ctx context.Context, key, src string) (int64, error) { f, err := os.Open(src) if err != nil { - return err + return 0, err } defer f.Close() info, err := f.Stat() if err != nil { - return err + return 0, err } - return w.storage.Put(ctx, key, f, info.Size(), "image/jpeg") + if err := w.storage.Put(ctx, key, f, info.Size(), "image/jpeg"); err != nil { + return 0, err + } + return info.Size(), nil } diff --git a/photo-api/migrations/0003_preview_size.pg.sql b/photo-api/migrations/0003_preview_size.pg.sql new file mode 100644 index 0000000..a6fb1a4 --- /dev/null +++ b/photo-api/migrations/0003_preview_size.pg.sql @@ -0,0 +1,6 @@ +-- Byte size of the generated preview variant, so the gallery response can +-- label the mobile save sheet's rows without the client HEADing every file. +-- NULL means "not measured yet" — rows predating this migration, until the +-- read path stats them once (see fillPreviewSizes in httpapi/sizes.go). +-- (No semicolons in these comments: the migration runner splits on them.) +ALTER TABLE photos_photos ADD COLUMN IF NOT EXISTS preview_size_bytes bigint; diff --git a/photo-api/migrations/0003_preview_size.sqlite.sql b/photo-api/migrations/0003_preview_size.sqlite.sql new file mode 100644 index 0000000..d8e24df --- /dev/null +++ b/photo-api/migrations/0003_preview_size.sqlite.sql @@ -0,0 +1,6 @@ +-- Byte size of the generated preview variant, so the gallery response can +-- label the mobile save sheet's rows without the client HEADing every file. +-- NULL means "not measured yet" — rows predating this migration, until the +-- read path stats them once (see fillPreviewSizes in httpapi/sizes.go). +-- (No semicolons in these comments: the migration runner splits on them.) +ALTER TABLE photos_photos ADD COLUMN preview_size_bytes integer; -- 2.54.0