Skip to content

Commit 0c40c69

Browse files
DioCraftsclaude
DioCrafts
and
claude
committed
perf(photos): ETag/304 conditional revalidation on the timeline
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) <noreply@anthropic.com>
1 parent f68972e commit 0c40c69

4 files changed

Lines changed: 171 additions & 7 deletions

File tree

‎benches/PHOTOS-ETAG.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Photos timeline — ETag / 304 conditional revalidation
2+
3+
`GET /api/photos` returned only `X-Next-Cursor` — no ETag. So every time the
4+
gallery re-mounts (navigate away and back), the server rebuilds up to 500
5+
`PhotoDto`s (each a `FileDto::from` with several allocations), serde-serializes
6+
the whole vector, and ships the full JSON body — even when nothing changed.
7+
8+
The handler now emits a lightweight, content-derived ETag and honours
9+
`If-None-Match`, with `Cache-Control: private, no-cache` so the browser always
10+
revalidates. An unchanged gallery re-mount becomes an empty **304** instead of a
11+
full rebuild + reserialize + re-transfer.
12+
13+
- **ETag** = hash of `(before, limit, max(modified_at), row count)` over the page
14+
— page identity plus a freshness signal, mirroring the file-list endpoint.
15+
Any upload/edit/delete that changes the page bumps `max(modified_at)` or the
16+
count, so the ETag changes and the client gets the fresh body.
17+
- The browser revalidates because the SPA's `apiFetch` uses the default fetch
18+
cache mode and `no-cache` forces a conditional request.
19+
- The DB query still runs (it's the cheap part); the win is skipping the DTO
20+
build + serialization + body bytes.
21+
22+
## Reproduce / proof
23+
24+
`tests/api/photos_etag.hurl` (run by the api-test suite) asserts: first GET → 200
25+
+ ETag + `Cache-Control: no-cache`; conditional GET with the matching ETag →
26+
empty 304; a stale `If-None-Match` → full 200. End-to-end measurement against a
27+
throwaway Postgres + server (7 uploaded images):
28+
29+
```
30+
1st GET (no If-None-Match) : 200 4586 bytes + ETag
31+
2nd GET (If-None-Match matches) : 304 0 bytes ← the win
32+
3rd GET (If-None-Match stale) : 200 4586 bytes ← correctly invalidated
33+
```
34+
35+
~655 B/photo on the wire. At a full 500-row first page that's **~320 KB +
36+
500-DTO build/serialize saved per unchanged "navigate away and back"**, plus the
37+
response bytes. Steady-state gallery navigation is the common case, so this hits
38+
real user-facing latency (unlike a one-time cold load).

‎src/interfaces/api/handlers/photos_handler.rs‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use axum::{
22
Json,
3+
body::Body,
34
extract::{Query, State},
4-
http::StatusCode,
5+
http::{HeaderMap, Response, StatusCode, header},
56
response::IntoResponse,
67
};
78
use serde::{Deserialize, Serialize};
@@ -59,6 +60,7 @@ struct PhotoDto {
5960
pub async fn list_photos(
6061
State(state): State<Arc<AppState>>,
6162
auth_user: AuthUser,
63+
headers: HeaderMap,
6264
Query(params): Query<PhotosQueryParams>,
6365
) -> impl IntoResponse {
6466
let user_id = auth_user.id;
@@ -71,7 +73,35 @@ pub async fn list_photos(
7173
.await
7274
{
7375
Ok((files, sort_dates, dims)) => {
74-
info!("Photos: returned {} media files for user", files.len());
76+
// Lightweight revalidation ETag: page identity (cursor + limit) plus a
77+
// freshness signal (max modified_at + row count over the page),
78+
// mirroring the file-list endpoint. With `Cache-Control: no-cache` the
79+
// browser always revalidates with If-None-Match, so a "navigate away
80+
// and back" to an unchanged gallery returns an empty 304 instead of
81+
// rebuilding 500 DTOs + reserializing + reshipping the whole body.
82+
let max_mod = files.iter().map(|f| f.modified_at()).max().unwrap_or(0);
83+
let count = files.len();
84+
let mut hasher = std::collections::hash_map::DefaultHasher::new();
85+
std::hash::Hash::hash(&params.before, &mut hasher);
86+
std::hash::Hash::hash(&limit, &mut hasher);
87+
std::hash::Hash::hash(&max_mod, &mut hasher);
88+
std::hash::Hash::hash(&count, &mut hasher);
89+
let etag = format!("\"{:x}\"", std::hash::Hasher::finish(&hasher));
90+
91+
if let Some(inm) = headers.get(header::IF_NONE_MATCH)
92+
&& let Ok(client_etag) = inm.to_str()
93+
&& client_etag == etag
94+
{
95+
return Response::builder()
96+
.status(StatusCode::NOT_MODIFIED)
97+
.header(header::ETAG, &etag)
98+
.header(header::CACHE_CONTROL, "private, no-cache")
99+
.body(Body::empty())
100+
.unwrap()
101+
.into_response();
102+
}
103+
104+
info!("Photos: returned {} media files for user", count);
75105

76106
// Convert to DTOs with sort_date + pixel dimensions populated.
77107
let dtos: Vec<PhotoDto> = files
@@ -89,12 +119,17 @@ pub async fn list_photos(
89119
})
90120
.collect();
91121

92-
// Set cursor header for next page
93122
let mut response = Json(&dtos).into_response();
94-
if let Some(&last_sd) = sort_dates.last() {
95-
response
96-
.headers_mut()
97-
.insert("X-Next-Cursor", last_sd.to_string().parse().unwrap());
123+
{
124+
let h = response.headers_mut();
125+
h.insert(header::ETAG, header::HeaderValue::from_str(&etag).unwrap());
126+
h.insert(
127+
header::CACHE_CONTROL,
128+
header::HeaderValue::from_static("private, no-cache"),
129+
);
130+
if let Some(&last_sd) = sort_dates.last() {
131+
h.insert("X-Next-Cursor", last_sd.to_string().parse().unwrap());
132+
}
98133
}
99134

100135
response

‎tests/api/photos_etag.hurl‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# =============================================================
2+
# OxiCloud – Photos timeline ETag / 304 conditional revalidation
3+
# =============================================================
4+
# Proves GET /api/photos returns a stable, content-derived ETag and
5+
# honours If-None-Match with an EMPTY 304 — so "navigate away and back"
6+
# to an unchanged gallery skips rebuilding the DTOs + reserializing +
7+
# reshipping the whole page body. Run:
8+
# hurl --variables-file tests/api/test.env --file-root tests \
9+
# --test tests/api/setup.hurl tests/api/photos_etag.hurl
10+
# =============================================================
11+
12+
13+
# ─────────────────────────────────────────────────────────────
14+
# Step 1 – Login
15+
# ─────────────────────────────────────────────────────────────
16+
POST {{base_url}}/api/auth/login
17+
Content-Type: application/json
18+
{
19+
"username": "{{username}}",
20+
"password": "{{password}}"
21+
}
22+
23+
HTTP 200
24+
[Captures]
25+
token: jsonpath "$.access_token"
26+
27+
28+
# ─────────────────────────────────────────────────────────────
29+
# Step 2 – Home folder id (upload target)
30+
# ─────────────────────────────────────────────────────────────
31+
GET {{base_url}}/api/folders
32+
Authorization: Bearer {{token}}
33+
34+
HTTP 200
35+
[Captures]
36+
home_folder_id: jsonpath "$[0].id"
37+
38+
39+
# ─────────────────────────────────────────────────────────────
40+
# Step 3 – Upload an image so the photos timeline is non-empty
41+
# ─────────────────────────────────────────────────────────────
42+
POST {{base_url}}/api/files/upload
43+
Authorization: Bearer {{token}}
44+
[MultipartFormData]
45+
folder_id: {{home_folder_id}}
46+
file: file,fixtures/dedup-test.jpg; image/jpeg
47+
48+
HTTP 201
49+
50+
51+
# ─────────────────────────────────────────────────────────────
52+
# Step 4 – First GET: 200 with a body and an ETag (capture it).
53+
# Cache-Control: no-cache makes the browser revalidate.
54+
# ─────────────────────────────────────────────────────────────
55+
GET {{base_url}}/api/photos?limit=200
56+
Authorization: Bearer {{token}}
57+
58+
HTTP 200
59+
[Captures]
60+
photos_etag: header "ETag"
61+
[Asserts]
62+
header "ETag" exists
63+
header "Cache-Control" contains "no-cache"
64+
jsonpath "$" isCollection
65+
jsonpath "$" count >= 1
66+
67+
68+
# ─────────────────────────────────────────────────────────────
69+
# Step 5 – Conditional GET with the same ETag: empty 304 (the win).
70+
# ─────────────────────────────────────────────────────────────
71+
GET {{base_url}}/api/photos?limit=200
72+
Authorization: Bearer {{token}}
73+
If-None-Match: {{photos_etag}}
74+
75+
HTTP 304
76+
[Asserts]
77+
header "ETag" == "{{photos_etag}}"
78+
bytes count == 0
79+
80+
81+
# ─────────────────────────────────────────────────────────────
82+
# Step 6 – A stale/mismatched ETag still gets the full 200 body.
83+
# ─────────────────────────────────────────────────────────────
84+
GET {{base_url}}/api/photos?limit=200
85+
Authorization: Bearer {{token}}
86+
If-None-Match: "stale-etag-does-not-match"
87+
88+
HTTP 200
89+
[Asserts]
90+
jsonpath "$" count >= 1

‎tests/api/run.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test
136136
"$API_DIR/nc_ocs_user_info.hurl" \
137137
"$API_DIR/nc_avatar_preview.hurl" \
138138
"$API_DIR/files-folders.hurl" \
139+
"$API_DIR/photos_etag.hurl" \
139140
"$API_DIR/favorites.hurl" \
140141
"$API_DIR/trash.hurl" \
141142
"$API_DIR/trash_resources.hurl" \

0 commit comments

Comments
 (0)