dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Key collection membership on the identity that exists

Browse Source 
Collections synced their names and arrived empty on every device. The names are
keyed on a uuid and worked; the membership union was keyed on
`images.content_hash`, and the schema says plainly what that column is:
"computed only when something needs it (import dedup, reconnect-by-hash), never
in a scan". A library that has only ever been scanned has one for no image at
all, so the join matched nothing and `WHERE ri.content_hash IS NOT NULL`
discarded whatever survived. The union could never have moved a single row.

Measured on a real catalog: 23,174 images, content hashes for 0 of them,
`oc:fileid` for all 23,174, twelve collections, zero members.

So membership now resolves through the file id first, exactly as keyword
assignment already did — `ASSIGN_BY_FILE_ID` was added for this same reason and
its doc comment even notes that membership was still on the hash. It is
recorded for every image the moment a remote scan sees it, survives server-side
rename and move (FR-NC-5), is the same integer on every device pointed at one
Nextcloud, and is already what the thumbnail shards are keyed by.

The content-hash union is kept rather than replaced: a local-only library has no
`remote` rows, and where a hash has been computed it is a true identity that
survives a library moving between servers. Both statements run; `INSERT OR
IGNORE` against the primary key makes the overlap free.

This repairs the merge. It cannot invent membership that no device recorded —
where the rows were never written, collections stay empty until they are filled
in again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-22 21:19:13 +02:00
co-authored by Claude Opus 5
parent d04087af83
commit 67f15bffb7
1 changed files with 177 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-catalog/src/merge.rs
+175 -17
View File
@@ -383,29 +383,69 @@ fn merge_collections_within(tx: &Connection, report: &mut MergeReport) -> Result
// ---- membership -------------------------------------------------------
//
// Set union, keyed on (collection uuid, image content hash). The hash
// rather than the image id, for the same reason collections use a uuid:
// image ids are local. An image the remote has and we do not is skipped —
// it will join when a scan or sync catalogues it, and the next merge picks
// it up.
// Set union, keyed on (collection uuid, image identity). Not the image id,
// for the same reason collections use a uuid: image ids are local. An image
// the remote has and we do not is skipped — it will join when a scan or
// sync catalogues it, and the next merge picks it up.
//
// Both identities are tried, and the file id first. See
// [`MEMBERS_BY_FILE_ID`] for why keying on the content hash alone made this
// whole union a no-op on every ordinary library.
//
// Tombstoned collections are excluded, or a merge would repopulate a
// collection it had just deleted.
let added = tx.execute(
"INSERT OR IGNORE INTO main.collection_members(collection_id, image_id, position, added)
let added = tx.execute(MEMBERS_BY_FILE_ID, [])? + tx.execute(MEMBERS_BY_CONTENT_HASH, [])?;
report.members_added = added;
Ok(())
}
/// Take membership for images both devices know by the server's file id.
///
/// **This is the identity that exists.** Membership was keyed on
/// `images.content_hash` alone, and the schema is explicit that the column is
/// "computed only when something needs it (import dedup, reconnect-by-hash),
/// never in a scan" — so on an ordinary library it is NULL for every row, the
/// join matched nothing, and `WHERE ri.content_hash IS NOT NULL` discarded what
/// little was left. Collections synced their names, because those are keyed on
/// a uuid, and arrived empty on every device. A 23,000-image library had a
/// content hash for none of them and an `oc:fileid` for all of them.
///
/// `oc:fileid` is recorded for every image the moment a remote scan sees it, is
/// stable across server-side rename and move (FR-NC-5), and is the same integer
/// on every device pointed at the same Nextcloud — which is exactly the
/// situation where two devices share collections. It is already what the
/// thumbnail shards are keyed by, and what [`ASSIGN_BY_FILE_ID`] uses for
/// keywords; membership was the one thing left behind.
const MEMBERS_BY_FILE_ID: &str = "
INSERT OR IGNORE INTO main.collection_members(collection_id, image_id, position, added)
SELECT lc.id, li.id, rm.position, rm.added
FROM remote_cat.collection_members rm
JOIN remote_cat.collections rc ON rc.id = rm.collection_id
JOIN main.collections lc ON lc.uuid = rc.uuid AND lc.deleted = 0
JOIN remote_cat.remote rr ON rr.image_id = rm.image_id
JOIN main.remote lr ON lr.file_id = rr.file_id
JOIN main.images li ON li.id = lr.image_id";
/// The same union for a library with no server behind it.
///
/// A local-only library has no `remote` rows at all, so [`MEMBERS_BY_FILE_ID`]
/// matches nothing and the content hash is the only identity available. Kept
/// rather than replaced: where a hash *has* been computed — an imported card,
/// a reconnect — it is a true identity, and one that survives a library moving
/// between servers.
///
/// Both statements run. `INSERT OR IGNORE` against the
/// `(collection_id, image_id)` primary key makes the overlap free.
const MEMBERS_BY_CONTENT_HASH: &str = "
INSERT OR IGNORE INTO main.collection_members(collection_id, image_id, position, added)
SELECT lc.id, li.id, rm.position, rm.added
FROM remote_cat.collection_members rm
JOIN remote_cat.collections rc ON rc.id = rm.collection_id
JOIN main.collections lc ON lc.uuid = rc.uuid AND lc.deleted = 0
JOIN remote_cat.images ri ON ri.id = rm.image_id
JOIN main.images li ON li.content_hash = ri.content_hash
WHERE ri.content_hash IS NOT NULL",
[],
)?;
report.members_added = added;
Ok(())
}
WHERE ri.content_hash IS NOT NULL";
/// Schema name the downloaded remote catalog is attached under.
///
@@ -615,9 +655,9 @@ const ASSIGN_BY_FILE_ID: &str = "
/// The same union for a library with no server behind it.
///
/// A local-only library has no `remote` rows at all, so [`ASSIGN_BY_FILE_ID`]
/// matches nothing and this is the only identity available — and it is the one
/// collection membership already uses, so a library where membership merges
/// has keywords that merge too.
/// matches nothing and this is the only identity available — the same pair
/// collection membership uses, so a library where membership merges has
/// keywords that merge too.
const ASSIGN_BY_CONTENT_HASH: &str = "
INSERT OR IGNORE INTO main.keywords(version_id, keyword)
SELECT lv.id, rk.keyword
@@ -741,6 +781,43 @@ mod tests {
c
}
/// An image with no content hash — which is every image in a library that
/// has only ever been scanned.
fn add_image_without_hash(c: &Connection, db: &str, id: i64) {
c.execute(
&format!(
"INSERT INTO {db}.roots(id, kind, label) VALUES (1, 'remote', 'lib')
ON CONFLICT(id) DO NOTHING"
),
[],
)
.unwrap();
c.execute(
&format!(
"INSERT INTO {db}.images(id, root_id, source_ref, added_at)
VALUES (?1, 1, ?2, 0)"
),
rusqlite::params![id, format!("img{id}.CR3")],
)
.unwrap();
}
fn count(c: &Connection, sql: &str) -> i64 {
c.query_row(sql, [], |r| r.get(0)).unwrap()
}
/// Give an image the `oc:fileid` a remote scan records for it.
///
/// What every image in a real synced library has, and what none of them
/// has a content hash for.
fn add_remote_id(c: &Connection, db: &str, image_id: i64, file_id: i64) {
c.execute(
&format!("INSERT INTO {db}.remote(image_id, file_id) VALUES (?1, ?2)"),
rusqlite::params![image_id, file_id],
)
.unwrap();
}
fn add_image(c: &Connection, db: &str, id: i64, hash: &str) {
c.execute(
&format!(
@@ -910,6 +987,87 @@ mod tests {
assert_eq!(n, 2, "both devices' additions survive");
}
#[test]
fn a_library_that_never_computed_a_hash_still_merges_membership() {
// The bug this replaces. `content_hash` is computed only by import
// dedup or a reconnect — never by a scan — so a synced library has one
// for no image at all. Keying membership on it alone meant collections
// arrived with their names and none of their contents, on every device,
// for every user.
let c = two_catalogs();
add_collection(&c, "main", 1, "shared", "Trip", 1);
add_collection(&c, "remote_cat", 1, "shared", "Trip", 1);
// No hashes anywhere, and a file id for both — a real library.
add_image_without_hash(&c, "main", 77);
add_image_without_hash(&c, "remote_cat", 3);
add_remote_id(&c, "main", 77, 90210);
add_remote_id(&c, "remote_cat", 3, 90210);
c.execute(
"INSERT INTO remote_cat.collection_members(collection_id, image_id, added)
VALUES (1, 3, 0)",
[],
)
.unwrap();
let report = merge_collections(&c).unwrap();
assert_eq!(report.members_added, 1);
let img: i64 = c
.query_row("SELECT image_id FROM main.collection_members", [], |r| {
r.get(0)
})
.unwrap();
assert_eq!(img, 77, "resolved through the server's file id");
}
#[test]
fn a_different_photograph_on_the_server_is_not_adopted() {
// The file id is an identity, so two different ids must not join.
let c = two_catalogs();
add_collection(&c, "main", 1, "shared", "Trip", 1);
add_collection(&c, "remote_cat", 1, "shared", "Trip", 1);
add_image_without_hash(&c, "main", 77);
add_image_without_hash(&c, "remote_cat", 3);
add_remote_id(&c, "main", 77, 111);
add_remote_id(&c, "remote_cat", 3, 222);
c.execute(
"INSERT INTO remote_cat.collection_members(collection_id, image_id, added)
VALUES (1, 3, 0)",
[],
)
.unwrap();
let report = merge_collections(&c).unwrap();
assert_eq!(report.members_added, 0);
assert_eq!(count(&c, "SELECT COUNT(*) FROM main.collection_members"), 0);
}
#[test]
fn an_image_identified_both_ways_is_added_once() {
// Both statements run, and their overlap must be free rather than a
// constraint violation or a double count.
let c = two_catalogs();
add_collection(&c, "main", 1, "shared", "Trip", 1);
add_collection(&c, "remote_cat", 1, "shared", "Trip", 1);
add_image(&c, "main", 77, "same-photo");
add_image(&c, "remote_cat", 3, "same-photo");
add_remote_id(&c, "main", 77, 90210);
add_remote_id(&c, "remote_cat", 3, 90210);
c.execute(
"INSERT INTO remote_cat.collection_members(collection_id, image_id, added)
VALUES (1, 3, 0)",
[],
)
.unwrap();
merge_collections(&c).unwrap();
assert_eq!(count(&c, "SELECT COUNT(*) FROM main.collection_members"), 1);
}
#[test]
fn membership_maps_across_devices_by_content_hash() {
// The same photograph carries different integer ids on each device.