From 0c40c69f9b1a954f19fca387e674bce43d898c50 Mon Sep 17 00:00:00 2001 From: DioCrafts Date: Mon, 22 Jun 2026 00:22:49 +0200 Subject: [PATCH] perf(photos): ETag/304 conditional revalidation on the timeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/photos sent only X-Next-Cursor — no ETag — so every gallery re-mount rebuilt up to 500 PhotoDtos, serde-serialized the whole vector, and shipped the full body even when nothing changed. The handler now emits a lightweight content-derived ETag (hash of before + limit + max(modified_at) + row count) and honours If-None-Match, with Cache-Control: private, no-cache so the SPA's default fetch cache mode always revalidates. An unchanged "navigate away and back" becomes an empty 304 instead of a full rebuild + reserialize + transfer. The DB query still runs (the cheap part); the win is skipping the DTO build, serialization, and body bytes. Proven end-to-end (throwaway Postgres + server, 7 images): 1st GET (no If-None-Match) -> 200 4586 bytes + ETag 2nd GET (If-None-Match matches) -> 304 0 bytes 3rd GET (If-None-Match stale) -> 200 4586 bytes (correctly invalidated) ~655 B/photo, so a full 500-row first page saves ~320 KB + a 500-DTO build/serialize per unchanged revalidation. Unlike a cold load this is the common gallery-navigation path, so it hits real user-facing latency. Regression test: tests/api/photos_etag.hurl (added to the api-test suite). Methodology in benches/PHOTOS-ETAG.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- benches/PHOTOS-ETAG.md | 38 ++++++++ src/interfaces/api/handlers/photos_handler.rs | 49 ++++++++-- tests/api/photos_etag.hurl | 90 +++++++++++++++++++ tests/api/run.sh | 1 + 4 files changed, 171 insertions(+), 7 deletions(-) create mode 100644 benches/PHOTOS-ETAG.md create mode 100644 tests/api/photos_etag.hurl diff --git a/benches/PHOTOS-ETAG.md b/benches/PHOTOS-ETAG.md new file mode 100644 index 00000000..ba3b78e5 --- /dev/null +++ b/benches/PHOTOS-ETAG.md @@ -0,0 +1,38 @@ +# Photos timeline — ETag / 304 conditional revalidation + +`GET /api/photos` returned only `X-Next-Cursor` — no ETag. So every time the +gallery re-mounts (navigate away and back), the server rebuilds up to 500 +`PhotoDto`s (each a `FileDto::from` with several allocations), serde-serializes +the whole vector, and ships the full JSON body — even when nothing changed. + +The handler now emits a lightweight, content-derived ETag and honours +`If-None-Match`, with `Cache-Control: private, no-cache` so the browser always +revalidates. An unchanged gallery re-mount becomes an empty **304** instead of a +full rebuild + reserialize + re-transfer. + +- **ETag** = hash of `(before, limit, max(modified_at), row count)` over the page + — page identity plus a freshness signal, mirroring the file-list endpoint. + Any upload/edit/delete that changes the page bumps `max(modified_at)` or the + count, so the ETag changes and the client gets the fresh body. +- The browser revalidates because the SPA's `apiFetch` uses the default fetch + cache mode and `no-cache` forces a conditional request. +- The DB query still runs (it's the cheap part); the win is skipping the DTO + build + serialization + body bytes. + +## Reproduce / proof + +`tests/api/photos_etag.hurl` (run by the api-test suite) asserts: first GET → 200 ++ ETag + `Cache-Control: no-cache`; conditional GET with the matching ETag → +empty 304; a stale `If-None-Match` → full 200. End-to-end measurement against a +throwaway Postgres + server (7 uploaded images): + +``` +1st GET (no If-None-Match) : 200 4586 bytes + ETag +2nd GET (If-None-Match matches) : 304 0 bytes ← the win +3rd GET (If-None-Match stale) : 200 4586 bytes ← correctly invalidated +``` + +~655 B/photo on the wire. At a full 500-row first page that's **~320 KB + +500-DTO build/serialize saved per unchanged "navigate away and back"**, plus the +response bytes. Steady-state gallery navigation is the common case, so this hits +real user-facing latency (unlike a one-time cold load). diff --git a/src/interfaces/api/handlers/photos_handler.rs b/src/interfaces/api/handlers/photos_handler.rs index c6243e82..00001b3b 100644 --- a/src/interfaces/api/handlers/photos_handler.rs +++ b/src/interfaces/api/handlers/photos_handler.rs @@ -1,7 +1,8 @@ use axum::{ Json, + body::Body, extract::{Query, State}, - http::StatusCode, + http::{HeaderMap, Response, StatusCode, header}, response::IntoResponse, }; use serde::{Deserialize, Serialize}; @@ -59,6 +60,7 @@ struct PhotoDto { pub async fn list_photos( State(state): State>, auth_user: AuthUser, + headers: HeaderMap, Query(params): Query, ) -> impl IntoResponse { let user_id = auth_user.id; @@ -71,7 +73,35 @@ pub async fn list_photos( .await { Ok((files, sort_dates, dims)) => { - info!("Photos: returned {} media files for user", files.len()); + // Lightweight revalidation ETag: page identity (cursor + limit) plus a + // freshness signal (max modified_at + row count over the page), + // mirroring the file-list endpoint. With `Cache-Control: no-cache` the + // browser always revalidates with If-None-Match, so a "navigate away + // and back" to an unchanged gallery returns an empty 304 instead of + // rebuilding 500 DTOs + reserializing + reshipping the whole body. + let max_mod = files.iter().map(|f| f.modified_at()).max().unwrap_or(0); + let count = files.len(); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + std::hash::Hash::hash(¶ms.before, &mut hasher); + std::hash::Hash::hash(&limit, &mut hasher); + std::hash::Hash::hash(&max_mod, &mut hasher); + std::hash::Hash::hash(&count, &mut hasher); + let etag = format!("\"{:x}\"", std::hash::Hasher::finish(&hasher)); + + if let Some(inm) = headers.get(header::IF_NONE_MATCH) + && let Ok(client_etag) = inm.to_str() + && client_etag == etag + { + return Response::builder() + .status(StatusCode::NOT_MODIFIED) + .header(header::ETAG, &etag) + .header(header::CACHE_CONTROL, "private, no-cache") + .body(Body::empty()) + .unwrap() + .into_response(); + } + + info!("Photos: returned {} media files for user", count); // Convert to DTOs with sort_date + pixel dimensions populated. let dtos: Vec = files @@ -89,12 +119,17 @@ pub async fn list_photos( }) .collect(); - // Set cursor header for next page let mut response = Json(&dtos).into_response(); - if let Some(&last_sd) = sort_dates.last() { - response - .headers_mut() - .insert("X-Next-Cursor", last_sd.to_string().parse().unwrap()); + { + let h = response.headers_mut(); + h.insert(header::ETAG, header::HeaderValue::from_str(&etag).unwrap()); + h.insert( + header::CACHE_CONTROL, + header::HeaderValue::from_static("private, no-cache"), + ); + if let Some(&last_sd) = sort_dates.last() { + h.insert("X-Next-Cursor", last_sd.to_string().parse().unwrap()); + } } response diff --git a/tests/api/photos_etag.hurl b/tests/api/photos_etag.hurl new file mode 100644 index 00000000..abd20882 --- /dev/null +++ b/tests/api/photos_etag.hurl @@ -0,0 +1,90 @@ +# ============================================================= +# OxiCloud – Photos timeline ETag / 304 conditional revalidation +# ============================================================= +# Proves GET /api/photos returns a stable, content-derived ETag and +# honours If-None-Match with an EMPTY 304 — so "navigate away and back" +# to an unchanged gallery skips rebuilding the DTOs + reserializing + +# reshipping the whole page body. Run: +# hurl --variables-file tests/api/test.env --file-root tests \ +# --test tests/api/setup.hurl tests/api/photos_etag.hurl +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 – Login +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "{{username}}", + "password": "{{password}}" +} + +HTTP 200 +[Captures] +token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 2 – Home folder id (upload target) +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +home_folder_id: jsonpath "$[0].id" + + +# ───────────────────────────────────────────────────────────── +# Step 3 – Upload an image so the photos timeline is non-empty +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{token}} +[MultipartFormData] +folder_id: {{home_folder_id}} +file: file,fixtures/dedup-test.jpg; image/jpeg + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step 4 – First GET: 200 with a body and an ETag (capture it). +# Cache-Control: no-cache makes the browser revalidate. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/photos?limit=200 +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +photos_etag: header "ETag" +[Asserts] +header "ETag" exists +header "Cache-Control" contains "no-cache" +jsonpath "$" isCollection +jsonpath "$" count >= 1 + + +# ───────────────────────────────────────────────────────────── +# Step 5 – Conditional GET with the same ETag: empty 304 (the win). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/photos?limit=200 +Authorization: Bearer {{token}} +If-None-Match: {{photos_etag}} + +HTTP 304 +[Asserts] +header "ETag" == "{{photos_etag}}" +bytes count == 0 + + +# ───────────────────────────────────────────────────────────── +# Step 6 – A stale/mismatched ETag still gets the full 200 body. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/photos?limit=200 +Authorization: Bearer {{token}} +If-None-Match: "stale-etag-does-not-match" + +HTTP 200 +[Asserts] +jsonpath "$" count >= 1 diff --git a/tests/api/run.sh b/tests/api/run.sh index de0e717d..ebe7450f 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -136,6 +136,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/nc_ocs_user_info.hurl" \ "$API_DIR/nc_avatar_preview.hurl" \ "$API_DIR/files-folders.hurl" \ + "$API_DIR/photos_etag.hurl" \ "$API_DIR/favorites.hurl" \ "$API_DIR/trash.hurl" \ "$API_DIR/trash_resources.hurl" \