Let two devices name the same photograph's version the same way

A version's uuid is the identity a cross-device merge keys on, and it was
minted at random, per catalog, per image. Two devices indexing one Nextcloud
library therefore held two different uuids for the same photograph — so the
sidecar they shared collected a `default = 1` block each, `Version::merge` was
never handed a matching pair to reconcile, and an afternoon's culling on the
tablet did not exist as far as the laptop was concerned.

`crate::merge` has said so in a comment since it was written: version uuids do
not reconcile across devices, a uuid-keyed join unions nothing, so keywords are
landed on the local default version instead. It named the problem and worked
around it. `rating`'s own comment asserted the opposite — that generating the
uuid here was what made it a cross-device identity — and `library::amend`
repeated the claim. Uniqueness was never the difficulty; agreement was.

`derived_version_uuid` computes it from `oc:fileid` instead. The server assigns
that integer, every client pointed at the library sees the same one, and it
survives a server-side rename and move — the three properties that already made
`ASSIGN_BY_FILE_ID` prefer it to a content hash. The layout is a UUIDv8 (RFC
9562, an application-defined form) carrying all sixty-four bits verbatim across
the variable fields with a fixed tag in the node field, so the mapping is
injective by construction rather than by a hash's good behaviour, and a uuid in
a sidecar can be read back to the file it belongs to by eye.

A library with no server behind it has no shared identity to derive and keeps a
generated one. The split is still reachable there if the folder is synced by
something else; `Sidecar::fuse_default_versions` repairs that case rather than
preventing it.

Deriving it for new rows alone would have fixed nothing — every image in an
existing library already has a version, so every one of them would have carried
on writing to its own rival identity. `align_default_version_uuids` moves them,
and runs from `schema::backfill` on every catalog open. It selects on the tag
in SQL, so a catalog already realigned matches no rows and writes nothing, and
it declines rather than fails where a virtual copy already holds the target.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-05 16:13:58 +02:00
co-authored by Claude Opus 5
parent 601c984894
commit ca2a135e28
3 changed files with 432 additions and 34 deletions
+398 -12
View File
@@ -63,6 +63,59 @@ impl Judgement {
}
}
/// TRACES: FR-NC-8 | FR-NC-9
/// The default version's uuid for a photograph the server knows by `file_id`.
///
/// # Why this is derived and not generated
///
/// A version's uuid is the identity a cross-device merge keys on. It used to
/// be minted at random per row, and the comment above this function used to
/// say that made it unique — which it did, and that was precisely the bug.
/// Two devices indexing the same library minted *different* uuids for the same
/// photograph, so the sidecar they shared ended up with two `default = 1`
/// blocks, `Version::merge` never saw a matching pair to reconcile, and a
/// rating made on one device was invisible on the other. `crate::merge` has
/// documented the consequence for keywords for as long as it has existed: a
/// uuid-keyed join across two catalogs unions nothing at all.
///
/// `oc:fileid` is the identity that *is* shared. The server assigns it, every
/// client pointed at that library sees the same integer, and it survives a
/// server-side rename and move — the same three properties that made
/// `crate::merge::ASSIGN_BY_FILE_ID` prefer it to a content hash.
///
/// # The layout
///
/// A UUIDv8 (RFC 9562: an application-defined layout) carrying the file id
/// verbatim across the four variable fields, with a fixed tag in the node
/// field saying what minted it. Verbatim rather than hashed so the mapping is
/// injective by construction: two file ids cannot collide, which a truncated
/// hash could, and a uuid read out of a sidecar can be traced back to the file
/// it belongs to by eye.
///
/// Every device computes this identically from the same integer, which is the
/// whole point — there is no negotiation and no first-writer-wins.
pub fn derived_version_uuid(file_id: i64) -> String {
let id = file_id as u64;
format!(
// 32 + 16 + 12 + 4 = 64 bits of file id, then the tag.
"{:08x}-{:04x}-8{:03x}-{:04x}-{:012x}",
(id >> 32) as u32,
(id >> 16) as u16,
(id >> 4) as u16 & 0x0FFF,
// The two high bits are the RFC's variant field and must be `0b10`;
// the remaining fourteen carry the file id's last four bits.
0x8000u16 | ((id as u16 & 0x000F) << 10),
DERIVED_VERSION_TAG,
)
}
/// The node field of a [`derived_version_uuid`], identifying what minted it.
///
/// Fixed and arbitrary. Its only job is to keep a derived uuid from colliding
/// with a randomly minted one and to make it recognisable in a sidecar read by
/// eye — `…-d0c5ec0de001` is visibly not a v4.
const DERIVED_VERSION_TAG: u64 = 0xd0c5_ec0d_e001;
/// Give every image without one a default version.
///
/// Idempotent, and cheap on the common path: the `NOT EXISTS` sub-select is
@@ -72,8 +125,9 @@ impl Judgement {
/// Returns how many were created, so a scan can log the backfill rather than
/// silently doing thousands of inserts.
///
/// The UUID is per row and generated here — it is the merge identity across
/// devices (FR-NC-8), so two images must never share one.
/// The uuid comes from [`derived_version_uuid`] where the server has named the
/// file, so every device computes the same one; only a library with no server
/// behind it falls back to a generated id.
pub fn ensure_default_versions(conn: &Connection) -> Result<usize, CatalogError> {
// One transaction for the batch. A backfill over a 24k-image library is
// 24k inserts, and per-statement commits would make it minutes rather
@@ -93,13 +147,17 @@ pub fn ensure_default_versions(conn: &Connection) -> Result<usize, CatalogError>
/// transaction within a transaction". The same split, for the same reason, as
/// `collections::add_within`.
pub fn ensure_default_versions_within(conn: &Connection) -> Result<usize, CatalogError> {
let ids: Vec<i64> = {
// The remote id travels with the image so the uuid can be derived from it.
// A `LEFT JOIN`, because a library on a folder or a card has no `remote`
// row at all and still needs its versions.
let ids: Vec<(i64, Option<i64>)> = {
let mut stmt = conn.prepare(
"SELECT i.id FROM images i
"SELECT i.id, r.file_id FROM images i
LEFT JOIN remote r ON r.image_id = i.id
WHERE NOT EXISTS (SELECT 1 FROM versions v WHERE v.image_id = i.id)",
)?;
let found = stmt
.query_map([], |r| r.get(0))?
.query_map([], |r| Ok((r.get(0)?, r.get(1)?)))?
.collect::<Result<Vec<_>, _>>()?;
found
};
@@ -112,14 +170,103 @@ pub fn ensure_default_versions_within(conn: &Connection) -> Result<usize, Catalo
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (?1, ?2, ?3, 1, 0, 0)",
)?;
for id in &ids {
insert.execute(rusqlite::params![id, new_uuid(), DEFAULT_VERSION_NAME])?;
for (id, file_id) in &ids {
insert.execute(rusqlite::params![
id,
version_uuid(*file_id),
DEFAULT_VERSION_NAME
])?;
}
}
Ok(ids.len())
}
/// The uuid to mint for a new default version.
///
/// Derived from the server's file id where there is one, so two devices agree
/// (FR-NC-8); generated where there is not.
///
/// # What the fallback costs
///
/// A library with no server behind it — a folder, a card — has no identity two
/// devices could both compute, so the split this derivation prevents is still
/// reachable there if that folder is synced by something else. That case is
/// repaired rather than prevented: `Sidecar::fuse_default_versions` folds the
/// rival defaults together the next time either device reads the file.
fn version_uuid(file_id: Option<i64>) -> String {
match file_id {
Some(id) => derived_version_uuid(id),
None => new_uuid(),
}
}
/// TRACES: FR-NC-8 | FR-NC-9
/// Move default versions minted before [`derived_version_uuid`] onto it.
///
/// Every catalog written by an earlier build holds a randomly minted uuid per
/// image, and its peers hold different ones for the same photographs. Deriving
/// the uuid only for *new* rows would leave every image already indexed —
/// which is all of them, on a library anybody has used — writing to the same
/// rival identity it always did.
///
/// Safe to run repeatedly: it selects only rows whose uuid is not already the
/// derived one, so a realigned catalog matches nothing and writes nothing.
///
/// # Why this cannot collide
///
/// `versions.uuid` is `UNIQUE`, and `remote.file_id` has a unique index of its
/// own, so two images cannot derive the same uuid. The one row that could
/// stand in the way is a *virtual copy* (FR-CAT-12) that already holds the
/// target — impossible to mint but not impossible to receive from a merge — so
/// the update is skipped where the target is taken rather than failing the
/// backfill and, with it, the catalog open.
///
/// # The sidecar side is not this function's business
///
/// Moving the catalog's uuid alone would leave the file's default under the
/// old one and the next write would add a rival rather than amend it. What
/// stops that is `library::amend` fusing onto the write's uuid before it looks
/// anything up, which renames the file's default to match. This end and that
/// one have to land together, and they do.
///
/// Returns how many rows moved.
pub fn align_default_version_uuids(conn: &Connection) -> Result<usize, CatalogError> {
// Filtered in SQL rather than in the loop: every derived uuid ends in the
// tag, so a catalog that has already been realigned selects no rows at all
// and this costs one indexed pass instead of twenty-four thousand reads.
let already = format!("%-{DERIVED_VERSION_TAG:012x}");
let stale: Vec<(i64, i64)> = {
let mut stmt = conn.prepare(
"SELECT v.id, r.file_id
FROM versions v
JOIN remote r ON r.image_id = v.image_id
WHERE v.is_default = 1 AND v.uuid NOT LIKE ?1",
)?;
let found = stmt
.query_map([&already], |r| Ok((r.get(0)?, r.get(1)?)))?
.collect::<Result<Vec<_>, _>>()?;
found
};
if stale.is_empty() {
return Ok(0);
}
let tx = conn.unchecked_transaction()?;
let mut moved = 0usize;
{
// `OR IGNORE` covers the taken-target case described above: the row
// keeps the uuid it has, which is the state this build has always
// coped with, rather than aborting the transaction.
let mut update = tx.prepare("UPDATE OR IGNORE versions SET uuid = ?2 WHERE id = ?1")?;
for (row, file_id) in &stale {
moved += update.execute(rusqlite::params![row, derived_version_uuid(*file_id)])?;
}
}
tx.commit()?;
Ok(moved)
}
/// The default version's row id for an image, creating one if it has none.
///
/// Every write path goes through this rather than assuming a version exists.
@@ -144,10 +291,21 @@ pub fn default_version_id(conn: &Connection, image: ImageId) -> Result<i64, Cata
return Ok(id);
}
// Derived from the server's file id where there is one, so the version
// this mints is the same one the photographer's other device will mint
// (FR-NC-8). A miss here is a library with no server behind it.
let file_id: Option<i64> = conn
.query_row(
"SELECT file_id FROM remote WHERE image_id = ?1",
[image.0 as i64],
|r| r.get(0),
)
.optional()?;
conn.execute(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (?1, ?2, ?3, 1, 0, 0)",
rusqlite::params![image.0 as i64, new_uuid(), DEFAULT_VERSION_NAME],
rusqlite::params![image.0 as i64, version_uuid(file_id), DEFAULT_VERSION_NAME],
)?;
Ok(conn.last_insert_rowid())
}
@@ -356,15 +514,22 @@ fn flag_from_code(v: i64) -> FlagState {
}
}
/// A version UUID.
/// A generated version UUID, for a photograph no server has named.
///
/// Hand-rolled rather than pulling in the `uuid` crate for one function — the
/// same reasoning as the date maths in `library_ui`. This needs to be unique
/// across devices, not cryptographically unguessable: it keys a merge, and an
/// attacker who can write to the sidecar has already won.
/// same reasoning as the date maths in `library_ui`. It needs to be unique,
/// not cryptographically unguessable: it keys a merge, and an attacker who can
/// write to the sidecar has already won.
///
/// Seeded from the system clock and a per-process counter, so two versions
/// created inside the same nanosecond tick still differ.
///
/// **Unique is not the same as agreed**, which is the distinction that cost a
/// photographer a day of culling. Two devices calling this for the same
/// photograph get two different answers, and a merge keyed on the result then
/// has no pair to reconcile. Anything with a `file_id` behind it must use
/// [`derived_version_uuid`]; this is the fallback for libraries that have no
/// server to supply one.
fn new_uuid() -> String {
use std::sync::atomic::{AtomicU64, Ordering};
static COUNTER: AtomicU64 = AtomicU64::new(0);
@@ -731,3 +896,224 @@ mod tests {
assert_eq!(cat.count(&unrated, 0).unwrap(), 4);
}
}
/// TRACES: FR-NC-8 | FR-NC-9
/// The identity two devices have to agree on without talking to each other.
#[cfg(test)]
mod derived_identity {
use super::*;
use crate::Catalog;
/// A catalog whose images the server has named, as a remote scan leaves it.
fn with_remote_images(file_ids: &[i64]) -> Catalog {
let cat = Catalog::in_memory().unwrap();
let c = cat.connection();
c.execute(
"INSERT INTO roots(id, kind, label) VALUES (1, 'remote', 'lib')",
[],
)
.unwrap();
for (i, file_id) in file_ids.iter().enumerate() {
c.execute(
"INSERT INTO images(root_id, source_ref, added_at) VALUES (1, ?1, 0)",
[format!("img{i:03}.CR3")],
)
.unwrap();
let image = c.last_insert_rowid();
c.execute(
"INSERT INTO remote(image_id, file_id) VALUES (?1, ?2)",
rusqlite::params![image, file_id],
)
.unwrap();
}
cat
}
fn default_uuids(cat: &Catalog) -> Vec<String> {
let mut stmt = cat
.connection()
.prepare("SELECT uuid FROM versions WHERE is_default = 1 ORDER BY image_id")
.unwrap();
stmt.query_map([], |r| r.get(0))
.unwrap()
.map(Result::unwrap)
.collect()
}
/// The whole point: the same photograph, indexed independently on two
/// devices, gets one identity. This used to be two.
#[test]
fn two_devices_derive_the_same_uuid_for_one_photograph() {
let laptop = with_remote_images(&[4_812]);
let tablet = with_remote_images(&[4_812]);
ensure_default_versions(laptop.connection()).unwrap();
ensure_default_versions(tablet.connection()).unwrap();
assert_eq!(default_uuids(&laptop), default_uuids(&tablet));
}
/// And different photographs must still be told apart — the property the
/// random uuid did have, which this must not give up to gain agreement.
#[test]
fn different_photographs_keep_different_uuids() {
let cat = with_remote_images(&[1, 2, 3, 0x7FFF_FFFF_FFFF_FFFF]);
ensure_default_versions(cat.connection()).unwrap();
let mut uuids = default_uuids(&cat);
let before = uuids.len();
uuids.sort();
uuids.dedup();
assert_eq!(uuids.len(), before, "two photographs share an identity");
}
/// The file id has to survive the layout intact, or two ids that differ
/// only in the bits it drops would collide.
#[test]
fn the_whole_file_id_is_carried() {
// A pair differing only in the low four bits, and a pair differing
// only in the high thirty-two — the two places a sloppy layout loses
// information.
assert_ne!(derived_version_uuid(0x10), derived_version_uuid(0x1F));
assert_ne!(
derived_version_uuid(0x0000_0001_0000_0000),
derived_version_uuid(0x0000_0002_0000_0000)
);
assert_ne!(derived_version_uuid(0), derived_version_uuid(-1));
}
/// Well-formed, and recognisably not a generated one.
#[test]
fn a_derived_uuid_is_a_well_formed_v8() {
let uuid = derived_version_uuid(4_812);
let fields: Vec<&str> = uuid.split('-').collect();
assert_eq!(fields.len(), 5);
assert_eq!(
fields.iter().map(|f| f.len()).collect::<Vec<_>>(),
vec![8, 4, 4, 4, 12]
);
assert!(fields[2].starts_with('8'), "version nibble: {uuid}");
// The RFC's variant field is the two high bits of the fourth group,
// and must read `0b10` — so the first hex digit is 8, 9, a or b.
assert!(
matches!(fields[3].as_bytes()[0], b'8' | b'9' | b'a' | b'b'),
"variant: {uuid}"
);
assert!(uuid.ends_with("d0c5ec0de001"), "tag: {uuid}");
}
/// A library with no server behind it has no shared identity to derive,
/// and must still get a version rather than failing.
#[test]
fn a_library_with_no_server_still_gets_its_versions() {
let cat = Catalog::in_memory().unwrap();
let c = cat.connection();
c.execute(
"INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')",
[],
)
.unwrap();
c.execute(
"INSERT INTO images(root_id, source_ref, added_at) VALUES (1, 'a.CR3', 0)",
[],
)
.unwrap();
assert_eq!(ensure_default_versions(c).unwrap(), 1);
assert_eq!(default_uuids(&cat).len(), 1);
}
/// The repair. A catalog written by an earlier build holds randomly minted
/// uuids, and leaving them there would mean every image already indexed —
/// which is all of them — kept writing to its own rival identity.
#[test]
fn a_catalog_from_an_earlier_build_is_realigned() {
let cat = with_remote_images(&[4_812, 4_813]);
let c = cat.connection();
// As the old code left it.
for (i, image) in [1i64, 2].iter().enumerate() {
c.execute(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (?1, ?2, 'Default', 1, ?3, 0)",
rusqlite::params![image, format!("random-{i}"), (i + 1) as i64],
)
.unwrap();
}
assert_eq!(align_default_version_uuids(c).unwrap(), 2);
assert_eq!(
default_uuids(&cat),
vec![derived_version_uuid(4_812), derived_version_uuid(4_813)]
);
// The judgement travels with the row — a realignment that dropped the
// ratings would be a worse bug than the one it fixes.
let ratings: Vec<i64> = {
let mut stmt = c
.prepare("SELECT rating FROM versions ORDER BY image_id")
.unwrap();
let v = stmt
.query_map([], |r| r.get(0))
.unwrap()
.map(Result::unwrap)
.collect();
v
};
assert_eq!(ratings, vec![1, 2]);
}
/// Runs on every catalog open, so a second pass must select nothing and
/// write nothing.
#[test]
fn realigning_twice_changes_nothing_the_second_time() {
let cat = with_remote_images(&[4_812]);
ensure_default_versions(cat.connection()).unwrap();
assert_eq!(
align_default_version_uuids(cat.connection()).unwrap(),
0,
"a freshly derived catalog must match nothing"
);
let before = default_uuids(&cat);
align_default_version_uuids(cat.connection()).unwrap();
assert_eq!(default_uuids(&cat), before);
}
/// A virtual copy (FR-CAT-12) that already holds the target uuid must not
/// take the backfill — and with it the catalog open — down with it.
#[test]
fn a_taken_target_leaves_the_row_where_it_is() {
let cat = with_remote_images(&[4_812]);
let c = cat.connection();
c.execute(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (1, 'random', 'Default', 1, 0, 0)",
[],
)
.unwrap();
c.execute(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (1, ?1, 'For print', 0, 0, 0)",
[derived_version_uuid(4_812)],
)
.unwrap();
assert_eq!(align_default_version_uuids(c).unwrap(), 0);
assert_eq!(default_uuids(&cat), vec!["random".to_string()]);
}
/// The backfill is what actually runs this, so it has to be wired in.
#[test]
fn opening_a_catalog_realigns_it() {
let cat = with_remote_images(&[4_812]);
cat.connection()
.execute(
"INSERT INTO versions(image_id, uuid, name, is_default, rating, flag)
VALUES (1, 'random', 'Default', 1, 3, 0)",
[],
)
.unwrap();
crate::schema::backfill(cat.connection()).unwrap();
assert_eq!(default_uuids(&cat), vec![derived_version_uuid(4_812)]);
}
}
+16 -4
View File
@@ -139,15 +139,27 @@ pub fn backfill(conn: &Connection) -> Result<Vec<(&'static str, usize)>, Catalog
// v3: every image needs a default version to carry its rating and flag.
// Libraries scanned before ratings existed have images and no versions at
// all, so there was nowhere for a judgement to go — see
// [`crate::rating`]. Backfilled rather than migrated in SQL because the
// UUID per row is the cross-device merge identity and must be generated,
// not derived.
// all, so there was nowhere for a judgement to go — see [`crate::rating`].
let n = crate::rating::ensure_default_versions(conn)?;
if n > 0 {
out.push(("default_versions", n));
}
// TRACES: FR-NC-8 | FR-NC-9
// The uuid on those rows is the cross-device merge identity, and it used
// to be generated rather than derived. This comment said so, and said it
// as though generating it were the point — it was the bug. Two devices
// minted different uuids for one photograph, so the sidecar they shared
// grew a `default = 1` block each and neither ever saw the other's work.
//
// Runs after the pass above so a row created a moment ago is already
// derived and matches nothing here. Ordering the other way would be
// correct too, just wasteful.
let n = crate::rating::align_default_version_uuids(conn)?;
if n > 0 {
out.push(("derived_version_uuids", n));
}
// v6: a vocabulary row for every word some image already carries.
//
// Three ways a catalog arrives holding assignments with no term behind