fix(core,fuse,gui,cli): read only the photos a refresh has not read

A timeline refresh resolved every photo's node to learn its name, media
type, favourite tag, content hash and photo relation — 63 round-trips for
a 12,445-photo library, minutes of work, repeated every time the timeline
went stale sixty seconds later. None of that metadata changes on its own,
and when it does change the event feed names the node it changed.

`photos.resolved_at` (schema v31) records that a photo's node was read.
The refresh resolves the photos without that mark: the ones it has never
read, and the ones a remote event marked stale. An existing timeline is
backfilled as resolved, so upgrading does not re-read the library once.

Remote changes now name what they changed instead of asking for
everything. `classify_photo_events` turns a `NodeUpdated` into
`photos_mark_unresolved(uid)`, so a favourite toggled on a phone costs one
node read rather than the whole library; a local toggle is claimed as our
own echo, because the feed replays it and a `NodeUpdated` carries nothing
that says what changed. A continuity loss and `refresh photos --full` drop
every mark, which is the way back to a whole-library read.

Along the way: `photos_replace` never kept a photo's name, so a photo the
SDK could not decrypt was renamed to nothing on every refresh — and the
tab split and the RAW+JPEG grouping are both derived from the name. It is
kept now, and a row that *was* resolved is authoritative instead, so a
photo unlinked from its main elsewhere finally loses its `main_uid`.

`TIMELINE_ENRICH_CHUNK` is 150, matching the SDK's own `MAX_BATCH_COUNT`:
a chunk of 200 was split there into a full request and a 50-uid tail.
This commit is contained in:
nvrl committed 2026-09-19 17:39:49 +02:00
1 parent 376f3edb28
commit 6cc8f72bab
12 files changed
+634 -69

No files matched your search

+159 -15
View File
@@ -489,36 +489,73 @@ pub(super) async fn run_photos_event_sync(core: Core) {
}
}
/// Apply one batch of photos-volume events to the gallery's tables.
fn apply_photo_events(core: &Core, events: &[DriveEvent]) {
let mut gone: Vec<String> = Vec::new();
let mut refresh = false;
/// What one batch of photos-volume events asks the gallery's tables to do.
#[derive(Debug, Default, PartialEq)]
pub(super) struct PhotoEventPlan {
/// Photos that left the volume, by delete or by trash.
pub(super) gone: Vec<String>,
/// Photos whose node changed, so their stored metadata is no longer trusted.
pub(super) touched: Vec<String>,
/// Whether the whole timeline has to be read again.
pub(super) full: bool,
}
impl PhotoEventPlan {
fn is_empty(&self) -> bool {
self.gone.is_empty() && self.touched.is_empty() && !self.full
}
}
/// Read one batch of photos-volume events. Pure, so the classification is
/// testable without a volume behind it.
///
/// `is_echo` claims an event as this daemon's own write. The feed replays our
/// writes back at us and a `NodeUpdated` carries no revision id, so without this
/// every favourite the user toggles here would mark its own photo stale and buy
/// a resolve of a node we just wrote.
fn classify_photo_events(
events: &[DriveEvent],
mut is_echo: impl FnMut(&NodeUid) -> bool,
) -> PhotoEventPlan {
let mut plan = PhotoEventPlan::default();
for event in events {
match event {
DriveEvent::NodeDeleted { node_uid, .. } => gone.push(node_uid.to_string()),
DriveEvent::NodeDeleted { node_uid, .. } => plan.gone.push(node_uid.to_string()),
DriveEvent::NodeUpdated {
node_uid,
is_trashed,
..
} => {
if *is_trashed {
gone.push(node_uid.to_string());
} else {
// An untrash, a rename, a new upload from the phone: all of
// them need the timeline itself, which only a refresh has.
refresh = true;
plan.gone.push(node_uid.to_string());
} else if !is_echo(node_uid) {
// An untrash, a rename, a favourite toggled on the phone: the
// event says which node changed but never what about it, so
// the uid is marked stale and the next refresh reads it.
plan.touched.push(node_uid.to_string());
}
}
// Nothing here can be applied event by event, so the whole timeline
// is re-read instead.
// The daemon cannot know what it missed here, so everything it
// thinks it knows about every photo is dropped.
DriveEvent::ContinuityLost { .. } | DriveEvent::ScopeAccessLost { .. } => {
refresh = true;
plan.full = true;
}
DriveEvent::CursorAdvanced { .. } | DriveEvent::SharedWithMeUpdated { .. } => {}
}
}
plan
}
/// Apply one batch of photos-volume events to the gallery's tables.
fn apply_photo_events(core: &Core, events: &[DriveEvent]) {
let plan = classify_photo_events(events, |uid| core.take_self_change(uid));
let PhotoEventPlan {
gone,
touched,
full,
} = &plan;
if !gone.is_empty() {
match core.db.photos_delete(&gone) {
match core.db.photos_delete(gone) {
Ok(removed) if removed > 0 => {
debug!(removed, "photos removed by a remote event");
}
@@ -526,7 +563,22 @@ fn apply_photo_events(core: &Core, events: &[DriveEvent]) {
Err(error) => warn!(%error, "removing remotely deleted photos failed"),
}
}
if refresh || !gone.is_empty() {
// A stale mark is what makes the next refresh cheap: it reads these photos'
// nodes and carries every other photo's metadata as it stands.
if !touched.is_empty() {
match core.db.photos_mark_unresolved(touched) {
Ok(marked) if marked > 0 => debug!(marked, "photos changed by a remote event"),
Ok(_) => {}
Err(error) => warn!(%error, "marking remotely changed photos failed"),
}
}
if *full {
match core.db.photos_mark_all_unresolved() {
Ok(marked) => debug!(marked, "the photo event stream lost continuity"),
Err(error) => warn!(%error, "marking the whole timeline unresolved failed"),
}
}
if !plan.is_empty() {
core.invalidate_photos();
core.invalidate_albums();
}
@@ -901,4 +953,96 @@ mod tests {
assert!(acknowledge_applied_event(&db, &event, applied).is_err());
assert!(db.get_event_cursor().unwrap().is_none());
}
/// A photo trashed or deleted elsewhere leaves the gallery; one that only
/// changed is marked stale, so the next refresh reads that node and no other.
#[test]
fn a_photo_event_names_what_it_changed() {
let plan = classify_photo_events(
&[
DriveEvent::NodeDeleted {
id: DriveEventId::from("e1"),
node_uid: uid("deleted"),
parent_node_uid: None,
},
DriveEvent::NodeUpdated {
id: DriveEventId::from("e2"),
node_uid: uid("trashed"),
parent_node_uid: None,
is_trashed: true,
is_shared: false,
},
DriveEvent::NodeUpdated {
id: DriveEventId::from("e3"),
node_uid: uid("favourited"),
parent_node_uid: None,
is_trashed: false,
is_shared: false,
},
],
|_| false,
);
assert_eq!(
plan,
PhotoEventPlan {
gone: vec![uid("deleted").to_string(), uid("trashed").to_string()],
touched: vec![uid("favourited").to_string()],
full: false,
}
);
}
/// The feed replays this daemon's own writes. Applying one would cost a
/// resolve of a node whose changed field the daemon just wrote itself.
#[test]
fn our_own_photo_write_is_not_a_remote_change() {
let plan = classify_photo_events(
&[DriveEvent::NodeUpdated {
id: DriveEventId::from("e1"),
node_uid: uid("ours"),
parent_node_uid: None,
is_trashed: false,
is_shared: false,
}],
|claimed| *claimed == uid("ours"),
);
assert_eq!(plan, PhotoEventPlan::default());
}
/// A gap in the stream means the daemon cannot know what it missed, so it
/// keeps nothing.
#[test]
fn a_lost_photo_event_stream_asks_for_everything() {
for event in [
DriveEvent::ContinuityLost {
id: DriveEventId::from("e1"),
},
DriveEvent::ScopeAccessLost {
id: DriveEventId::from("e1"),
},
] {
let plan = classify_photo_events(std::slice::from_ref(&event), |_| false);
assert!(plan.full, "{event:?}");
assert!(plan.gone.is_empty());
assert!(plan.touched.is_empty());
}
}
/// Bookkeeping events change nothing about the gallery, and must not buy a
/// refresh of it.
#[test]
fn photo_bookkeeping_events_ask_for_nothing() {
let plan = classify_photo_events(
&[
DriveEvent::CursorAdvanced {
id: DriveEventId::from("e1"),
},
DriveEvent::SharedWithMeUpdated {
id: DriveEventId::from("e2"),
},
],
|_| false,
);
assert_eq!(plan, PhotoEventPlan::default());
}
}
+7 -2
View File
@@ -255,8 +255,13 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
core.invalidate_trash();
Ok(())
}
RefreshScope::Photos => {
RefreshScope::Photos { full } => {
core.invalidate_photos();
// A full refresh is "forget what you know", not a second code
// path: the ordinary refresh then finds every photo stale.
if let Some(Err(e)) = full.then(|| core.db.photos_mark_all_unresolved()) {
warn!(error = %e, "marking the whole timeline unresolved failed");
}
// Whatever changed the timeline can just as easily have
// changed an album; one scope covers the whole photos view.
core.invalidate_albums();
@@ -274,7 +279,7 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
match result {
Ok(()) => CtlResponse::Ok {
message: match scope {
RefreshScope::Photos => "refreshing".to_string(),
RefreshScope::Photos { .. } => "refreshing".to_string(),
_ => "refreshed".to_string(),
},
},
+4 -1
View File
@@ -213,7 +213,10 @@ const TIMELINE_TTL: Duration = Duration::from_secs(60);
/// Photos / Videos / Raw split). Batched so a large library is a handful of
/// round-trips rather than one request per photo, and bounded so a single call
/// never asks the server to decrypt the whole library at once.
const TIMELINE_ENRICH_CHUNK: usize = 200;
///
/// 150 because that is the SDK's own `MAX_BATCH_COUNT`: a larger chunk is split
/// there anyway, into one full request and a ragged tail.
const TIMELINE_ENRICH_CHUNK: usize = 150;
/// The same, for the persisted trash listing. Shorter, because the trash is the
/// one listing a user changes and then immediately looks at — though our own
/// mutations also invalidate it outright, so this only covers other clients.
+32 -7
View File
@@ -499,6 +499,11 @@ impl Core {
.map_err(|e| CoreError::from_api(&e, "update photo tags"))?;
pdfs_core::batch::into_unit(outcomes)
.map_err(|e| CoreError::from_api(&e, "update photo tags"))?;
// The photo event feed replays this write back at us as a bare
// `NodeUpdated`, which is indistinguishable from the same photo being
// favourited on a phone. Claim the echo, or the gallery pays for a
// resolve of a node whose one changed field is already stored below.
self.note_self_change(uid);
self.db
.photos_set_favorite(&uid.to_string(), favorite)
.map_err(CoreError::from)?;
@@ -1416,13 +1421,30 @@ impl Core {
.await
.map_err(|e| CoreError::from_api(&e, "timeline"))?;
// The timeline DTO carries only a uid and capture time, but the Photos
// page has to split into Photos / Videos / Raw — which needs each photo's
// name and media type. Resolve those in batches off the request path.
// Best-effort: a photo whose node we fail to resolve keeps whatever was
// learned before (or classifies from nothing, i.e. a still photo), so a
// partial resolve never blanks the timeline.
let uids: Vec<NodeUid> = items.iter().map(|it| it.uid.clone()).collect();
// The timeline DTO carries only a uid and capture time, but the gallery
// has to split into its tabs — which needs each photo's name and media
// type. Resolve those in batches off the request path. Best-effort: a
// photo whose node we fail to resolve keeps whatever was learned before
// (or classifies from nothing, i.e. a still photo), so a partial resolve
// never blanks the timeline.
//
// Only photos the daemon has never read, and ones a remote event marked
// stale, are read. Resolving all of them every time is what made a
// 12k-photo refresh take minutes (`docs/BUGS.md` B92): this metadata does
// not change on its own, and when it does change the event feed names the
// uid that changed (`Db::photos_mark_unresolved`).
let known: HashSet<String> = self
.db
.photos_resolved()
.map_err(CoreError::from)?
.into_iter()
.collect();
let uids: Vec<NodeUid> = items
.iter()
.filter(|it| !known.contains(&it.uid.to_string()))
.map(|it| it.uid.clone())
.collect();
let resolved_at = now_ms();
let mut meta: HashMap<String, PhotoMeta> = HashMap::new();
// A main photo names its related photos rather than the other way round,
// so the relation is recorded from whichever end resolves — the two ends
@@ -1482,6 +1504,7 @@ impl Core {
favorite: Some(m.favorite),
content_hash: m.content_hash,
main_uid,
resolved_at: Some(resolved_at),
..db::TimelineRow::new(key, it.capture_time)
},
None => db::TimelineRow {
@@ -1498,6 +1521,8 @@ impl Core {
// like it never happened, and the chunk count is what it scales with.
info!(
photos = rows.len(),
resolved = uids.len(),
skipped = rows.len().saturating_sub(uids.len()),
chunks = uids.len().div_ceil(TIMELINE_ENRICH_CHUNK),
elapsed_ms = started.elapsed().as_millis() as u64,
"photos timeline refreshed"
+7 -2
View File
@@ -222,8 +222,13 @@ impl Core {
tags_to_add: vec![PhotoTag::Favorite],
tags_to_remove: Vec::new(),
};
if let Err(e) = self.rt.block_on(photos.update_photos(&[update])) {
warn!(name = candidate.name, error = %e, "cannot re-favourite re-dated photo");
match self.rt.block_on(photos.update_photos(&[update])) {
// As in `set_photo_favorite`: claim our own echo, so the tag we
// just set does not come back as a remote change.
Ok(_) => self.note_self_change(&new_uid),
Err(e) => {
warn!(name = candidate.name, error = %e, "cannot re-favourite re-dated photo")
}
}
}
for album in &candidate.albums {