From aefc4bc411b82cda2c81afb2929ec9b6a43823e0 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Fri, 30 Oct 2026 19:22:52 -0400 Subject: [PATCH] Scope canonical_rel_path and merge version-path annotations at link time (#194) Co-Authored-By: Claude Opus 5.5 --- src/database/mod.rs | 18 ++-- src/database/version_dao.rs | 167 ++++++++++++++++++++++++++++++++++-- src/tags.rs | 2 +- 3 files changed, 173 insertions(+), 14 deletions(-) diff --git a/src/database/mod.rs b/src/database/mod.rs index a2d1b7a..5332308 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -323,10 +323,11 @@ impl FavoriteDao for SqliteFavoriteDao { let mut connection = self.connection.lock().expect("Unable to get FavoriteDao"); // Favoriting a photo version favorites its original (#191). - let favorite_path = &version_dao::canonical_rel_path(connection.deref_mut(), favorite_path) - .ok() - .flatten() - .unwrap_or_else(|| favorite_path.to_string()); + let favorite_path = + &version_dao::canonical_rel_path(connection.deref_mut(), None, favorite_path) + .ok() + .flatten() + .unwrap_or_else(|| favorite_path.to_string()); if favorites .filter(userid.eq(user_id).and(rel_path.eq(&favorite_path))) @@ -349,10 +350,11 @@ impl FavoriteDao for SqliteFavoriteDao { use schema::favorites::dsl::*; let mut connection = self.connection.lock().unwrap(); - let favorite_path = version_dao::canonical_rel_path(connection.deref_mut(), &favorite_path) - .ok() - .flatten() - .unwrap_or(favorite_path); + let favorite_path = + version_dao::canonical_rel_path(connection.deref_mut(), None, &favorite_path) + .ok() + .flatten() + .unwrap_or(favorite_path); diesel::delete(favorites) .filter(userid.eq(user_id).and(rel_path.eq(favorite_path))) .execute(connection.deref_mut()) diff --git a/src/database/version_dao.rs b/src/database/version_dao.rs index 6a4ba8f..61e852b 100644 --- a/src/database/version_dao.rs +++ b/src/database/version_dao.rs @@ -282,8 +282,15 @@ fn placeholders(n: usize) -> String { /// If `rel_path` is a linked version whose original has a live row, the /// original's rel_path (same library when possible). Tag and favorite /// writes on a version path bind to the original through this. +/// +/// `library_id` scopes the version lookup to one library's row; `None` is +/// for the library-agnostic tag/favorite DAOs, which only know a rel_path. +/// Annotations are keyed by rel_path alone, so in either case a path that +/// *also* names a file in some library that is not a linked version stays +/// itself: redirecting it would hijack that photo's own tags and favorites. pub fn canonical_rel_path( conn: &mut SqliteConnection, + library_id: Option, rel_path: &str, ) -> QueryResult> { #[derive(QueryableByName)] @@ -295,15 +302,63 @@ pub fn canonical_rel_path( "SELECT o.rel_path AS rel_path FROM image_exif v \ JOIN photo_versions pv ON pv.version_hash = v.content_hash \ JOIN image_exif o ON o.content_hash = pv.original_hash \ - WHERE v.rel_path = ? \ + WHERE v.rel_path = ?1 AND (?2 IS NULL OR v.library_id = ?2) \ + AND NOT EXISTS (SELECT 1 FROM image_exif x WHERE x.rel_path = v.rel_path \ + AND NOT EXISTS (SELECT 1 FROM photo_versions p \ + WHERE p.version_hash = x.content_hash)) \ ORDER BY (o.library_id = v.library_id) DESC, o.id \ LIMIT 1", ) .bind::(rel_path) + .bind::, _>(library_id) .load::(conn)?; Ok(rows.into_iter().next().map(|r| r.rel_path)) } +/// Fold favorites and tags recorded on a version's own path into its +/// original's, now that reads and writes on the version path canonicalise +/// to the original and could never reach them again. Set-valued data is +/// unioned (duplicates dropped), then the version path's rows are removed. +/// A no-op unless the version has a live original (`canonical_rel_path`). +fn merge_annotations_into_original( + conn: &mut SqliteConnection, + library_id: i32, + version_rel_path: &str, +) -> QueryResult<()> { + let Some(original) = canonical_rel_path(conn, Some(library_id), version_rel_path)? else { + return Ok(()); + }; + if original == version_rel_path { + return Ok(()); + } + sql_query( + "INSERT INTO favorites (userid, rel_path) \ + SELECT f.userid, ?2 FROM favorites f WHERE f.rel_path = ?1 \ + AND NOT EXISTS (SELECT 1 FROM favorites g WHERE g.userid = f.userid AND g.rel_path = ?2)", + ) + .bind::(version_rel_path) + .bind::(&original) + .execute(conn)?; + sql_query("DELETE FROM favorites WHERE rel_path = ?") + .bind::(version_rel_path) + .execute(conn)?; + sql_query( + "INSERT INTO tagged_photo (rel_path, tag_id, created_time, content_hash) \ + SELECT ?2, t.tag_id, t.created_time, \ + (SELECT content_hash FROM image_exif WHERE rel_path = ?2 \ + AND content_hash IS NOT NULL LIMIT 1) \ + FROM tagged_photo t WHERE t.rel_path = ?1 \ + AND NOT EXISTS (SELECT 1 FROM tagged_photo u WHERE u.tag_id = t.tag_id AND u.rel_path = ?2)", + ) + .bind::(version_rel_path) + .bind::(&original) + .execute(conn)?; + sql_query("DELETE FROM tagged_photo WHERE rel_path = ?") + .bind::(version_rel_path) + .execute(conn)?; + Ok(()) +} + /// Root original of `hash`: follows `photo_versions` until a hash that is /// not itself a version. `None` on a cycle. fn root_original(conn: &mut SqliteConnection, hash: &str) -> QueryResult> { @@ -424,6 +479,8 @@ impl VersionDao for SqliteVersionDao { .bind::(reference) .execute(conn)?; } + + merge_annotations_into_original(conn, link.library_id, &link.rel_path)?; Ok(LinkOutcome::Linked) }) }) @@ -448,7 +505,7 @@ impl VersionDao for SqliteVersionDao { fn original_path_of(&mut self, rel_path: &str) -> Result, DbError> { trace_db_call("query", "original_path_of", |_span| { let mut conn = self.connection.lock().expect("Unable to get VersionDao"); - canonical_rel_path(&mut conn, rel_path) + canonical_rel_path(&mut conn, None, rel_path) .map_err(|e| anyhow::anyhow!("Query error: {}", e)) }) .map_err(|e| DbError::log(DbErrorKind::QueryError, e)) @@ -1014,20 +1071,120 @@ mod tests { .unwrap(); let mut conn = c.lock().unwrap(); assert_eq!( - canonical_rel_path(&mut conn, "aiedits/orig.jpg.restored.png").unwrap(), + canonical_rel_path(&mut conn, None, "aiedits/orig.jpg.restored.png").unwrap(), Some("orig.jpg".to_string()) ); - assert_eq!(canonical_rel_path(&mut conn, "orig.jpg").unwrap(), None); + assert_eq!( + canonical_rel_path(&mut conn, None, "orig.jpg").unwrap(), + None + ); drop(conn); // Original gone: nothing to bind to, the version is a normal photo. exec(&c, "DELETE FROM image_exif WHERE content_hash = 'o'"); let mut conn = c.lock().unwrap(); assert_eq!( - canonical_rel_path(&mut conn, "aiedits/orig.jpg.restored.png").unwrap(), + canonical_rel_path(&mut conn, None, "aiedits/orig.jpg.restored.png").unwrap(), None ); } + #[test] + fn canonical_path_is_scoped_to_the_versions_library() { + let (mut dao, c) = dao(); + insert_exif(&c, 1, "album_a/orig.jpg", "o"); + insert_exif(&c, 1, "album_a/aiedits/orig.jpg.edit.png", "v"); + dao.link_version(&link("v", "o", "album_a/aiedits/orig.jpg.edit.png")) + .unwrap(); + // Library 2 has an unrelated, unlinked photo at the same path. + insert_exif(&c, 2, "album_a/aiedits/orig.jpg.edit.png", "other"); + let mut conn = c.lock().unwrap(); + let p = "album_a/aiedits/orig.jpg.edit.png"; + assert_eq!( + canonical_rel_path(&mut conn, Some(2), p).unwrap(), + None, + "library 2's row is not a version" + ); + // Annotations are path-keyed, so a path that also names a + // standalone photo is never redirected away from it. + assert_eq!(canonical_rel_path(&mut conn, None, p).unwrap(), None); + assert_eq!( + canonical_rel_path(&mut conn, Some(1), p).unwrap(), + None, + "shared path with a standalone photo stays put" + ); + drop(conn); + exec(&c, "DELETE FROM image_exif WHERE content_hash = 'other'"); + let mut conn = c.lock().unwrap(); + assert_eq!( + canonical_rel_path(&mut conn, Some(1), p).unwrap(), + Some("album_a/orig.jpg".to_string()) + ); + assert_eq!(canonical_rel_path(&mut conn, Some(2), p).unwrap(), None); + } + + #[test] + fn linking_merges_annotations_from_the_version_path_into_the_original() { + let (mut dao, c) = dao(); + insert_exif(&c, 1, "album_a/orig.jpg", "o"); + insert_exif(&c, 1, "album_a/aiedits/orig.jpg.edit.png", "v"); + let v = "album_a/aiedits/orig.jpg.edit.png"; + let o = "album_a/orig.jpg"; + for t in ["t1", "t2"] { + exec( + &c, + &format!("INSERT INTO tags (name, created_time) VALUES ('{t}', 0)"), + ); + } + // Tag 1 on both paths (duplicate), tag 2 only on the version. + exec( + &c, + &format!( + "INSERT INTO tagged_photo (rel_path, tag_id, created_time) \ + SELECT '{o}', id, 1 FROM tags WHERE name = 't1'" + ), + ); + exec( + &c, + &format!( + "INSERT INTO tagged_photo (rel_path, tag_id, created_time) \ + SELECT '{v}', id, 2 FROM tags" + ), + ); + // User 1 favorited both; user 2 only the version. + exec( + &c, + &format!( + "INSERT INTO favorites (userid, rel_path) VALUES (1, '{o}'), (1, '{v}'), (2, '{v}')" + ), + ); + + dao.link_version(&link("v", "o", v)).unwrap(); + + let rows = |sql: &str| -> Vec<(String, i32)> { + #[derive(QueryableByName)] + struct R { + #[diesel(sql_type = Text)] + p: String, + #[diesel(sql_type = Integer)] + k: i32, + } + sql_query(sql) + .load::(&mut *c.lock().unwrap()) + .unwrap() + .into_iter() + .map(|r| (r.p, r.k)) + .collect() + }; + assert_eq!( + rows("SELECT rel_path AS p, userid AS k FROM favorites ORDER BY userid"), + vec![(o.to_string(), 1), (o.to_string(), 2)] + ); + assert_eq!( + rows("SELECT rel_path AS p, tag_id AS k FROM tagged_photo ORDER BY tag_id"), + vec![(o.to_string(), 1), (o.to_string(), 2)] + ); + } + #[test] fn sync_copies_date_gps_and_camera_from_a_converted_original() { let (mut dao, c) = dao(); diff --git a/src/tags.rs b/src/tags.rs index a517bcc..fd0165f 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -608,7 +608,7 @@ pub trait TagDao: Send + Sync { /// `path` is a linked photo version whose original is still indexed, else /// `path` itself. fn canonical_path(conn: &mut SqliteConnection, path: &str) -> String { - crate::database::version_dao::canonical_rel_path(conn, path) + crate::database::version_dao::canonical_rel_path(conn, None, path) .ok() .flatten() .unwrap_or_else(|| path.to_string())