diff --git a/core/dr-catalog/src/collections.rs b/core/dr-catalog/src/collections.rs index 30d642a..0f4a5e0 100644 --- a/core/dr-catalog/src/collections.rs +++ b/core/dr-catalog/src/collections.rs @@ -682,7 +682,7 @@ fn require_exists(conn: &Connection, id: CollectionId) -> Result<(), CatalogErro /// same reasoning as the connector's date parsing. Version 4 layout, seeded /// from the OS via `getrandom` through `rusqlite`'s existing dependency-free /// path — see below. -fn new_uuid() -> String { +pub(crate) fn new_uuid() -> String { let b = random_bytes(); // Version 4, variant 1, per RFC 4122 §4.4. let v6 = (b[6] & 0x0F) | 0x40; diff --git a/core/dr-catalog/src/error.rs b/core/dr-catalog/src/error.rs index b6d6f08..0a8f0da 100644 --- a/core/dr-catalog/src/error.rs +++ b/core/dr-catalog/src/error.rs @@ -42,6 +42,15 @@ pub enum CatalogError { #[error("no such collection: {0}")] NoSuchCollection(u64), + /// A keyword the caller named is gone — deleted, or fused into another by a + /// merge while its id sat in a UI model. + /// + /// Its own variant rather than a silent no-op because the two are different + /// answers to the user: a rename that quietly did nothing looks exactly like + /// a rename that did not take. + #[error("no such keyword: {0}")] + NoSuchKeyword(u64), + /// Images were dropped onto a smart collection. /// /// A smart collection's membership *is* its selector, so member rows would diff --git a/core/dr-catalog/src/keywords.rs b/core/dr-catalog/src/keywords.rs new file mode 100644 index 0000000..c3676ba --- /dev/null +++ b/core/dr-catalog/src/keywords.rs @@ -0,0 +1,1220 @@ +//! TRACES: FR-CAT-5 | FR-CAT-6 | FR-CAT-13 | NFR-R5 +//! Keywords: the vocabulary, the assignments, and the edits the UI performs. +//! +//! The read half of this shipped with the catalog and the write half did not. +//! [`crate::query`] has compiled [`dr_types::Selector::Keyword`] against the +//! `keywords` table since v1, and [`dr_types::Selector::Text`] substring-matches +//! it, so the library could always be searched by keyword — but nothing could +//! ever *put* one there. A user could filter to a word they had no way to +//! apply. This module is the missing half. +//! +//! # Two tables, and which one is the fact +//! +//! **`keywords`** is the assignment: `(version_id, keyword)`, where the keyword +//! is the *word itself* as text. This is the durable fact. It is what +//! [`crate::query`] matches, what a sidecar carries, and what XMP `dc:subject` +//! interoperates on (FR-CAT-13) — every one of which speaks in strings. +//! +//! **`keyword_terms`** is the identity: uuid, revision, tombstone. It exists so +//! that a *rename* and a *deletion* have something a cross-device merge can key +//! on, and so that a keyword can exist in the vocabulary before any photograph +//! carries it. It is not referenced by the assignment rows; see the V6 +//! migration in [`crate::schema`] for why pointing at it would be a mistake. +//! +//! A rename therefore writes both: the term rows (so the edit merges) and every +//! assignment carrying the old text (so the search, the sidecar and the XMP all +//! agree). Those writes are one transaction, because a catalog holding half a +//! rename would show the keyword under one name and find it under the other. +//! [`rename`] explains why the old identity is retired rather than relabelled. +//! +//! # Which version an assignment lands on +//! +//! The schema hangs keywords off `versions`, not `images`, and that is right: +//! FR-NC-8 makes the sidecar a keyed set of versions, so everything durable +//! about a photograph is stored per version. But a *write* from the UI has to +//! land somewhere definite, and it lands on the **default** version — the same +//! choice [`crate::rating`] makes, for the same reason. +//! +//! Reading is deliberately asymmetric: [`crate::query`] matches a keyword on +//! *any* version of an image (`EXISTS ... WHERE kv.image_id = images.id`), and +//! [`for_images`] does the same. A keyword names what is in the frame, so a +//! word applied to a virtual copy must still find the photograph — but there +//! must be exactly one place a UI write goes, or two copies of one frame come +//! to disagree about their own subject. +//! +//! # Uniqueness is converged upon, not constrained +//! +//! `keyword_terms.name` carries no unique index. Two devices that each type +//! "Iceland" both create a term, with different uuids, and neither is wrong +//! until they meet. A constraint would abort the merge transaction at that +//! moment — which is the ordinary case, not a corner one. +//! +//! Instead: [`create`] resolves an existing name rather than adding a second +//! row, and [`fuse_duplicates`] collapses a cross-device pair onto the +//! lexicographically smaller uuid. That rule is deterministic and symmetric, so +//! both devices reach the same answer without a round trip. +//! +//! # Keywords are user text, and they only ever bind +//! +//! Every keyword here reaches SQLite as a bound parameter. `crate::query` has +//! the same rule and a test that asserts it; this module is the write side of +//! the same guarantee, and the reason it matters more here is that this is +//! where the text arrives from the user in the first place. + +use rusqlite::{Connection, OptionalExtension}; + +use dr_types::ImageId; + +use crate::error::CatalogError; + +/// A keyword's local row id in `keyword_terms`. +/// +/// Local to this catalog and meaningless on another device, exactly as +/// [`dr_types::CollectionId`] is — [`Keyword::uuid`] is what a merge keys on. +/// +/// Defined here rather than in `dr-types` because nothing outside the catalog +/// and the panel that draws it has any use for it: an id that names a row in a +/// rebuildable index is not a term of the shared vocabulary the way an +/// `ImageId` is. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct KeywordId(pub u64); + +/// One keyword, as the vocabulary list draws it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Keyword { + pub id: KeywordId, + /// Device-independent identity. The integer `id` is local and collides + /// across devices; this is what a merge keys on. + pub uuid: String, + pub name: String, + /// How many images in the whole library carry it. Shown beside the word so + /// the user can tell a keyword they use from one they typed once by + /// mistake. + pub image_count: usize, +} + +/// A keyword's standing across a *selection*, for the panel. +/// +/// The three-way distinction is the whole reason this type exists: applying a +/// keyword to forty photographs where thirty already carry it must not look +/// the same as applying it to forty that do not, and removing one that only +/// some of them carry must not silently claim to have removed it from all. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Coverage { + /// No image in the selection carries it. + None, + /// Some do and some do not — Lightroom's dash rather than a tick. + Some, + /// Every image in the selection carries it. + All, +} + +/// A keyword plus how much of the current selection it covers. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SelectionKeyword { + pub keyword: Keyword, + pub coverage: Coverage, + /// How many of the *selected* images carry it. + pub selected_count: usize, +} + +/// Longest keyword accepted. +/// +/// Not a storage limit — SQLite would take a megabyte — but a paste guard. The +/// field that feeds this is one line in a sheet, and a keyword the width of a +/// paragraph is a mis-paste rather than an intention. Truncating rather than +/// refusing, so the paste is recoverable by editing rather than lost. +pub const MAX_KEYWORD_LEN: usize = 128; + +/// Create a keyword, or return the one that already carries this name. +/// +/// **Resolving rather than refusing** is what separates this from +/// [`crate::collections::create`], which happily makes a second "Iceland". A +/// keyword *is* its text — that is what the assignment rows store and what XMP +/// carries — so two term rows with one name are two identities for one thing, +/// and the second would be a duplicate in the vocabulary list that no amount +/// of user care could distinguish from the first. +/// +/// The UUID is generated here rather than taken from the caller: it is the +/// merge identity, and a caller that reuses one fuses two keywords at the next +/// sync. +pub fn create(conn: &Connection, name: &str) -> Result { + let name = normalise(name)?; + + // A tombstoned row is deliberately *not* resurrected here: it carries a + // revision that says "deleted", and reusing it would make the new keyword + // inherit an argument it was never part of. A fresh uuid starts the + // conversation again, which is what the user asked for by typing the word. + if let Some(id) = live_id_for_name(conn, &name)? { + return Ok(id); + } + + let now = now_secs(); + conn.execute( + "INSERT INTO keyword_terms(uuid, name, created, revision, modified) + VALUES (?1, ?2, ?3, 1, ?3)", + rusqlite::params![new_uuid(), name, now], + )?; + Ok(KeywordId(conn.last_insert_rowid() as u64)) +} + +/// Rename a keyword, everywhere it appears. +/// +/// Returns the identity the word now has, which is **not** the one passed in. +/// +/// # A rename retires one word and raises another +/// +/// The obvious implementation — change `name` on the term row and keep its +/// uuid — is wrong here, and the reason is that assignments store *text*. Take +/// two keywords, "Icland" and "Iceland", and rename the first onto the second. +/// Something has to give, because two live rows may not both be "Iceland", and +/// every way of choosing between them by uuid or by revision loses the rename +/// on the device that chose the other way. The rename then silently fails to +/// propagate, which is the worst of the outcomes available. +/// +/// So a rename is modelled as what it actually is to a body of *text*: the old +/// word stops existing and the new one exists. The old term row is tombstoned +/// **keeping its old name**, the new word gets (or already has) a live row, and +/// every assignment is rewritten between them. That composes with the merge +/// rules already in place — the tombstone travels as a deletion and takes the +/// old word's assignments with it on every device, and the new word travels as +/// an ordinary keyword — rather than needing a rule of its own. +/// +/// One transaction over both tables. A catalog holding half a rename would show +/// a photograph under the new word and fail to find it under either. +/// +/// Renaming onto a name another keyword already holds therefore **fuses the +/// two**, which is almost always what was meant: the user is correcting +/// "Icland" to "Iceland" and there is already an "Iceland". Refusing would +/// leave them to do it by hand, image by image, with no bulk gesture to do it +/// with. +pub fn rename(conn: &Connection, id: KeywordId, name: &str) -> Result { + let name = normalise(name)?; + let old = name_of(conn, id)?; + if old == name { + // Not an error, and not an edit either: bumping the revision for a + // no-op rename would let an idle device win a merge against one that + // did real work. + return Ok(id); + } + + let tx = conn.unchecked_transaction()?; + + // Assignments first. If this fails, the term rows are untouched and the + // catalog is merely unchanged rather than internally inconsistent. + rewrite_assignments(&tx, &old, &name)?; + retire(&tx, id)?; + let now = create(&tx, &name)?; + + tx.commit()?; + Ok(now) +} + +/// Delete a keyword, leaving a tombstone. +/// +/// The term row survives with `deleted = 1` because a merge against a device +/// that still holds the keyword would otherwise resurrect it — the same rule +/// [`crate::collections::delete`] follows. +/// +/// The assignment rows go outright. They carry no independent identity: the +/// tombstone is what merges, and a photograph keyworded with a word that no +/// longer exists would be searchable by a term absent from every list. +/// +/// Returns how many assignments went. +pub fn delete(conn: &Connection, id: KeywordId) -> Result { + let name = name_of(conn, id)?; + + let tx = conn.unchecked_transaction()?; + let removed = tx.execute("DELETE FROM keywords WHERE keyword = ?1", [&name])?; + retire(&tx, id)?; + tx.commit()?; + Ok(removed) +} + +/// The whole vocabulary, most-used first and then alphabetical. +/// +/// Used-first because the list is a target for a thumb: the words a +/// photographer reaches for are the ones they already use, and burying them +/// under a one-off typo sorted to the top is what makes a keyword list stop +/// being used. Alphabetical *within* a count so the order is stable across +/// launches and across devices — row-id order would differ per device, which +/// is disorienting on the same library seen from two machines. +/// +/// One grouped aggregate rather than a count per row: the sheet redraws on +/// every assignment, and a query per keyword would be one statement per word +/// in the library. +pub fn list(conn: &Connection) -> Result, CatalogError> { + let mut stmt = conn.prepare( + // DISTINCT image, not row: a word on two versions of one frame is one + // photograph, and reporting two is the kind of small lie that makes a + // user stop trusting the counts. + "SELECT t.id, t.uuid, t.name, + (SELECT count(DISTINCT v.image_id) + FROM keywords k JOIN versions v ON v.id = k.version_id + WHERE k.keyword = t.name) + FROM keyword_terms t + WHERE t.deleted = 0 + ORDER BY 4 DESC, t.name COLLATE NOCASE, t.id", + )?; + let rows = stmt + .query_map([], |r| { + Ok(Keyword { + id: KeywordId(r.get::<_, i64>(0)? as u64), + uuid: r.get(1)?, + name: r.get(2)?, + image_count: r.get::<_, i64>(3)? as usize, + }) + })? + .collect::, _>>()?; + Ok(rows) +} + +/// Assign a keyword to images, creating the keyword if it is new. +/// +/// The bulk form is the *only* form, because keywording a selection is the +/// common case rather than the exception: a photographer picks out the frames +/// with the puffin in them and applies "puffin" once. One transaction, so a +/// crash partway through cannot leave half a selection keyworded — the same +/// reasoning as [`crate::rating::set_rating_many`]. +/// +/// Additive and idempotent. An image that already carries the word is left +/// alone rather than rewritten, because re-applying a keyword to a selection +/// that overlaps what is already tagged is a normal thing to do. +/// +/// Returns how many images genuinely gained it, which is what the UI reports — +/// "added to 3 of 12" is the honest message when nine already had it. +pub fn assign(conn: &Connection, images: &[ImageId], name: &str) -> Result { + let name = normalise(name)?; + let tx = conn.unchecked_transaction()?; + + // The term row first, so the word appears in the vocabulary even when the + // selection turns out to be empty — typing a keyword into the field and + // pressing return is a legitimate way to build a vocabulary ahead of the + // photographs it will be used on. + create(&tx, &name)?; + + let mut added = 0; + { + let mut stmt = tx.prepare( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, ?2) + ON CONFLICT(version_id, keyword) DO NOTHING", + )?; + for image in images { + // Creates the version if the image has none — a library scanned + // before versions existed, or a scan interrupted between the image + // insert and the commit. Failing the keyword because of either + // would be the wrong answer: the user typed a word and expects it + // to stick. + let version = crate::rating::default_version_id(&tx, *image)?; + added += stmt.execute(rusqlite::params![version, name])?; + } + } + + tx.commit()?; + Ok(added) +} + +/// Take a keyword off images. +/// +/// Removes the assignment only. The keyword itself survives in the vocabulary +/// and on every other photograph that carries it — that asymmetry is the point +/// of a join table, and it is why this is "remove from these images" and never +/// "delete the keyword". [`delete`] is the other gesture, and it says so. +/// +/// Removes it from **every** version of each image, not only the default. A +/// user who unticks a word is saying the photograph is not of that thing, and +/// leaving it on a virtual copy would keep the frame in the search results +/// with nothing in the panel to explain why. +pub fn unassign(conn: &Connection, images: &[ImageId], name: &str) -> Result { + let name = normalise(name)?; + if images.is_empty() { + return Ok(0); + } + + let tx = conn.unchecked_transaction()?; + let mut removed = 0; + { + let mut stmt = tx.prepare( + "DELETE FROM keywords + WHERE keyword = ?2 + AND version_id IN (SELECT id FROM versions WHERE image_id = ?1)", + )?; + for image in images { + if stmt.execute(rusqlite::params![image.0 as i64, name])? > 0 { + removed += 1; + } + } + } + tx.commit()?; + Ok(removed) +} + +/// Every keyword on one image, alphabetically. +/// +/// Across all its versions, and de-duplicated: the panel shows what the +/// photograph is of, and a word on two virtual copies is still one subject. +pub fn for_image(conn: &Connection, image: ImageId) -> Result, CatalogError> { + let mut stmt = conn.prepare( + "SELECT DISTINCT k.keyword + FROM keywords k JOIN versions v ON v.id = k.version_id + WHERE v.image_id = ?1 + ORDER BY k.keyword COLLATE NOCASE", + )?; + let rows = stmt + .query_map([image.0 as i64], |r| r.get(0))? + .collect::, _>>()?; + Ok(rows) +} + +/// The vocabulary, annotated with how much of `images` each keyword covers. +/// +/// This is what the keywording panel draws: one list, in which a word the whole +/// selection already carries, a word only some of it carries, and a word none +/// of it carries are three visibly different things. +/// +/// Two statements regardless of the selection size — the vocabulary, and one +/// grouped count over the selection. A count per keyword would be one query +/// per word on every redraw, which is the cost `crate::rating::judgements` +/// exists to avoid for stars. +/// +/// A word that is on an image but has somehow lost its term row still appears, +/// synthesised with no identity. That happens to a catalog rebuilt from +/// sidecars between the rebuild and the next [`adopt_orphan_terms`], and a +/// panel that omitted the keywords the photograph visibly has would read as the +/// keywords having been lost. +pub fn for_images( + conn: &Connection, + images: &[ImageId], +) -> Result, CatalogError> { + let vocabulary = list(conn)?; + if images.is_empty() { + return Ok(vocabulary + .into_iter() + .map(|keyword| SelectionKeyword { + keyword, + coverage: Coverage::None, + selected_count: 0, + }) + .collect()); + } + + // Placeholders are generated from the *count* of ids, never from any text + // that came from outside — the same rule `rating::judgements` follows, and + // the reason a keyword can never reach SQL as anything but a parameter. + let placeholders = std::iter::repeat_n("?", images.len()) + .collect::>() + .join(","); + let sql = format!( + "SELECT k.keyword, count(DISTINCT v.image_id) + FROM keywords k JOIN versions v ON v.id = k.version_id + WHERE v.image_id IN ({placeholders}) + GROUP BY k.keyword" + ); + let params: Vec = images + .iter() + .map(|i| rusqlite::types::Value::Integer(i.0 as i64)) + .collect(); + + let mut counts: std::collections::HashMap = std::collections::HashMap::new(); + { + let mut stmt = conn.prepare(&sql)?; + let rows = stmt.query_map(rusqlite::params_from_iter(params.iter()), |r| { + Ok((r.get::<_, String>(0)?, r.get::<_, i64>(1)?)) + })?; + for (word, n) in rows.flatten() { + counts.insert(word, n as usize); + } + } + + let mut out: Vec = Vec::with_capacity(vocabulary.len()); + let mut seen = std::collections::HashSet::new(); + for keyword in vocabulary { + let n = counts.get(&keyword.name).copied().unwrap_or(0); + seen.insert(keyword.name.clone()); + out.push(SelectionKeyword { + coverage: coverage_of(n, images.len()), + selected_count: n, + keyword, + }); + } + + // Words the selection carries that the vocabulary has no row for. Sorted + // and appended rather than interleaved, so the list the user has been + // looking at does not reshuffle around them. + let mut orphans: Vec<(String, usize)> = counts + .into_iter() + .filter(|(word, _)| !seen.contains(word)) + .collect(); + orphans.sort_by(|a, b| a.0.to_lowercase().cmp(&b.0.to_lowercase())); + for (name, n) in orphans { + out.push(SelectionKeyword { + keyword: Keyword { + // Zero, because there is no row. The panel treats it as a word + // it can assign and unassign but not rename — which is exactly + // true until the next backfill gives it an identity. + id: KeywordId(0), + uuid: String::new(), + image_count: n, + name, + }, + coverage: coverage_of(n, images.len()), + selected_count: n, + }); + } + + Ok(out) +} + +fn coverage_of(carrying: usize, selected: usize) -> Coverage { + if carrying == 0 { + Coverage::None + } else if carrying >= selected { + Coverage::All + } else { + Coverage::Some + } +} + +/// Give a term row to every word some image carries without one. +/// +/// Runs from [`crate::schema::backfill`] on every open. Cheap on the common +/// path: one anti-joined scan of an indexed column that inserts nothing once +/// the vocabulary is complete. +/// +/// Returns how many terms were adopted, so an import or a rebuild can log it +/// rather than silently writing thousands of rows. +pub fn adopt_orphan_terms(conn: &Connection) -> Result { + let orphans: Vec = { + let mut stmt = conn.prepare( + // A tombstone counts as "has a term row". A word the user deleted + // whose assignment somehow outlived the deletion must not be + // quietly readmitted to the vocabulary; it stays visible as an + // orphan in [`for_images`] instead, which is a state someone can + // see and act on rather than one that silently undoes a deletion. + "SELECT DISTINCT k.keyword FROM keywords k + WHERE NOT EXISTS (SELECT 1 FROM keyword_terms t + WHERE t.name = k.keyword)", + )?; + let found = stmt + .query_map([], |r| r.get(0))? + .collect::, _>>()?; + found + }; + if orphans.is_empty() { + return Ok(0); + } + + // One transaction for the batch. An import from Lightroom can bring in + // hundreds of keywords, and per-statement commits would be hundreds of + // fsyncs for what is conceptually one adoption. + let tx = conn.unchecked_transaction()?; + let now = now_secs(); + { + let mut stmt = tx.prepare( + "INSERT INTO keyword_terms(uuid, name, created, revision, modified) + VALUES (?1, ?2, ?3, 1, ?3)", + )?; + for name in &orphans { + stmt.execute(rusqlite::params![new_uuid(), name, now])?; + } + } + tx.commit()?; + Ok(orphans.len()) +} + +/// Collapse term rows that describe the same word onto one identity. +/// +/// The keyword *is* its text, so two live rows named "Iceland" are two +/// identities for one thing. That happens for one ordinary reason: two devices +/// each typed the word before they had ever synced, and each minted a uuid. +/// +/// The survivor is the **lexicographically smallest uuid**, and the losers are +/// deleted outright rather than tombstoned. Both halves of that matter: +/// +/// - *Smallest uuid* is a rule both devices apply to the same pair and reach +/// the same answer from, with no round trip and no ordering dependency. Any +/// rule that consulted a revision or a timestamp would let two devices pick +/// different survivors and never converge. +/// - *Deleted, not tombstoned*, because the word itself has not been deleted — +/// the redundant row is being retired, and a tombstone would propagate as +/// "the user removed this keyword" and take the assignments with it. +/// +/// Assignment rows need no repair: they store the text, which is the same on +/// both sides, which is precisely why fusing is possible at all. +/// +/// Returns how many rows were retired. +pub fn fuse_duplicates(conn: &Connection) -> Result { + let n = conn.execute( + "DELETE FROM keyword_terms + WHERE deleted = 0 + AND uuid > (SELECT min(o.uuid) FROM keyword_terms o + WHERE o.deleted = 0 AND o.name = keyword_terms.name)", + [], + )?; + Ok(n) +} + +/// Point every assignment at a new spelling of the same word. +/// +/// `INSERT OR IGNORE` then `DELETE` rather than a bare `UPDATE`, because a +/// version may already carry the destination word — renaming "Icland" to +/// "Iceland" on a frame that has both — and the primary key would reject the +/// update for that row and abort the rename for every other. +fn rewrite_assignments(conn: &Connection, from: &str, to: &str) -> Result<(), CatalogError> { + conn.execute( + "INSERT OR IGNORE INTO keywords(version_id, keyword) + SELECT version_id, ?2 FROM keywords WHERE keyword = ?1", + rusqlite::params![from, to], + )?; + conn.execute("DELETE FROM keywords WHERE keyword = ?1", [from])?; + Ok(()) +} + +/// The stored form of a keyword the user typed. +/// +/// Trimmed, inner whitespace collapsed, and length-capped. Trimming matters +/// more than it looks: `keywords.keyword` is compared with `=` by +/// [`crate::query`], so " puffin" and "puffin" would be two keywords that look +/// identical in every list and never match each other's searches. +/// +/// Case is deliberately **preserved**. "Iceland" is a place and "iceland" is a +/// typo of it, and a photographer who capitalises their proper nouns should +/// find them capitalised. The lists sort `COLLATE NOCASE` so the two still land +/// beside each other where both exist. +/// +/// Public because a caller that is about to *tell the user* what it did needs +/// the word as it will be stored, not as it was typed. Reporting `Added " +/// puffin "` for something the list then shows as `puffin` is a small +/// inconsistency, but it is the kind that makes a user wonder whether the +/// leading space mattered — and the only way to answer that from the outside +/// is to duplicate this rule, which is how the two come to disagree. +pub fn normalise(name: &str) -> Result { + let collapsed = name.split_whitespace().collect::>().join(" "); + if collapsed.is_empty() { + return Err(CatalogError::BadName( + "A keyword needs a word in it.".into(), + )); + } + // Truncated on a *character* boundary: `String::truncate` panics mid-code + // point, and a keyword is as likely to be "Þingvellir" as "puffin". + Ok(collapsed.chars().take(MAX_KEYWORD_LEN).collect()) +} + +/// The live term row holding this exact name, if there is one. +fn live_id_for_name(conn: &Connection, name: &str) -> Result, CatalogError> { + let id: Option = conn + .query_row( + "SELECT id FROM keyword_terms WHERE name = ?1 AND deleted = 0 ORDER BY uuid LIMIT 1", + [name], + |r| r.get(0), + ) + .optional()?; + Ok(id.map(|v| KeywordId(v as u64))) +} + +fn name_of(conn: &Connection, id: KeywordId) -> Result { + conn.query_row( + "SELECT name FROM keyword_terms WHERE id = ?1 AND deleted = 0", + [id.0 as i64], + |r| r.get(0), + ) + .optional()? + .ok_or(CatalogError::NoSuchKeyword(id.0)) +} + +/// Tombstone a term row, bumping its revision. +/// +/// **The name is deliberately left as it was.** A tombstone is read by the +/// merge as "the user removed *this word*", and the other device acts on it by +/// deleting the assignments carrying that text — so a tombstone renamed to the +/// word it was merged into would delete the assignments of the surviving +/// keyword instead of the retired one. See [`rename`]. +/// +/// The revision bump is not optional. [`crate::merge`] compares revisions, so a +/// deletion that updated `modified` alone is invisible to it — the other device +/// keeps its live row and the keyword comes back at the next sync. +fn retire(conn: &Connection, id: KeywordId) -> Result<(), CatalogError> { + conn.execute( + "UPDATE keyword_terms + SET deleted = 1, revision = revision + 1, modified = ?2 + WHERE id = ?1", + rusqlite::params![id.0 as i64, now_secs()], + )?; + Ok(()) +} + +/// A random UUID, formatted as the canonical 8-4-4-4-12. +/// +/// Shares [`crate::collections`]'s implementation rather than repeating it: a +/// second generator is a second thing to get wrong, and the merge identity is +/// the one place a weak one is unrecoverable. +fn new_uuid() -> String { + crate::collections::new_uuid() +} + +fn now_secs() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Catalog; + use dr_types::Selector; + + /// A catalog with six images, each with a default version. + fn seeded() -> Catalog { + let cat = Catalog::in_memory().unwrap(); + let c = cat.connection(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + for i in 1..=6i64 { + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (?1, 1, ?2, 0)", + rusqlite::params![i, format!("IMG_{i}.CR3")], + ) + .unwrap(); + } + crate::rating::ensure_default_versions(c).unwrap(); + cat + } + + fn img(n: u64) -> ImageId { + ImageId(n) + } + + fn names(cat: &Catalog) -> Vec { + list(cat.connection()) + .unwrap() + .into_iter() + .map(|k| k.name) + .collect() + } + + #[test] + fn a_keyword_can_be_applied_and_then_found() { + // The whole point: the search path existed and nothing could feed it. + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "puffin").unwrap(); + + let q = crate::Query { + filter: Selector::Keyword("puffin".into()), + ..Default::default() + }; + assert_eq!(cat.count(&q, 0).unwrap(), 2); + } + + #[test] + fn assigning_to_a_selection_is_one_gesture() { + let cat = seeded(); + let all: Vec = (1..=6).map(img).collect(); + assert_eq!(assign(cat.connection(), &all, "iceland").unwrap(), 6); + } + + #[test] + fn reapplying_to_an_overlapping_selection_reports_only_the_new_ones() { + // "Added to 1 of 3" is the honest message, and the UI shows it. + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "puffin").unwrap(); + let added = assign(cat.connection(), &[img(1), img(2), img(3)], "puffin").unwrap(); + assert_eq!(added, 1); + } + + #[test] + fn assignment_is_idempotent_and_stores_one_row() { + let cat = seeded(); + assign(cat.connection(), &[img(1)], "puffin").unwrap(); + assign(cat.connection(), &[img(1)], "puffin").unwrap(); + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), ["puffin"]); + } + + #[test] + fn unassigning_leaves_the_keyword_on_everything_else() { + // The join-table asymmetry: removing a word from one photograph is not + // deleting the word. + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "puffin").unwrap(); + assert_eq!(unassign(cat.connection(), &[img(1)], "puffin").unwrap(), 1); + + assert!(for_image(cat.connection(), img(1)).unwrap().is_empty()); + assert_eq!(for_image(cat.connection(), img(2)).unwrap(), ["puffin"]); + assert_eq!(names(&cat), ["puffin"], "the vocabulary still has it"); + } + + #[test] + fn a_word_on_a_virtual_copy_is_removed_with_the_frame() { + // Unticking says "this photograph is not of that", and a word left on + // a virtual copy would keep the frame in the results with nothing in + // the panel to explain it. + let cat = seeded(); + let c = cat.connection(); + c.execute( + "INSERT INTO versions(image_id, uuid, name, is_default) VALUES (1, 'copy', 'Crop', 0)", + [], + ) + .unwrap(); + let copy: i64 = c.last_insert_rowid(); + assign(c, &[img(1)], "puffin").unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, 'puffin')", + [copy], + ) + .unwrap(); + + unassign(c, &[img(1)], "puffin").unwrap(); + let n: i64 = c + .query_row("SELECT count(*) FROM keywords", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 0); + } + + #[test] + fn a_keyword_on_a_virtual_copy_still_finds_the_photograph() { + // The read side is deliberately asymmetric with the write side: a word + // names what is in the frame, whichever copy carries it. + let cat = seeded(); + let c = cat.connection(); + c.execute( + "INSERT INTO versions(image_id, uuid, name, is_default) VALUES (3, 'copy', 'Crop', 0)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, 'gannet')", + [c.last_insert_rowid()], + ) + .unwrap(); + + let q = crate::Query { + filter: Selector::Keyword("gannet".into()), + ..Default::default() + }; + assert_eq!(cat.count(&q, 0).unwrap(), 1); + } + + #[test] + fn creating_the_same_word_twice_is_one_keyword() { + // A keyword is its text. Two rows with one name would be a duplicate + // in the list that no user could tell apart. + let cat = seeded(); + let a = create(cat.connection(), "iceland").unwrap(); + let b = create(cat.connection(), "iceland").unwrap(); + assert_eq!(a, b); + assert_eq!(names(&cat), ["iceland"]); + } + + #[test] + fn a_keyword_can_exist_before_any_photograph_carries_it() { + // Building the vocabulary ahead of the shoot is a real workflow, and + // it is the reason the term table exists at all. + let cat = seeded(); + create(cat.connection(), "puffin").unwrap(); + assert_eq!(names(&cat), ["puffin"]); + assert_eq!(list(cat.connection()).unwrap()[0].image_count, 0); + } + + #[test] + fn whitespace_around_a_keyword_is_not_part_of_it() { + // `query` compares with `=`, so " puffin" would be a second keyword + // that looked identical in every list and matched nothing the first + // matched. + let cat = seeded(); + assign(cat.connection(), &[img(1)], " puffin ").unwrap(); + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), ["puffin"]); + } + + #[test] + fn inner_whitespace_collapses() { + let cat = seeded(); + assign(cat.connection(), &[img(1)], "black\tguillemot").unwrap(); + assert_eq!( + for_image(cat.connection(), img(1)).unwrap(), + ["black guillemot"] + ); + } + + #[test] + fn normalise_is_what_a_caller_can_show_the_user() { + // Public so a status line can quote the word as stored rather than as + // typed. If these two ever disagree, the UI is reporting a keyword the + // list will not show. + let cat = seeded(); + assign(cat.connection(), &[img(1)], " black \t guillemot ").unwrap(); + assert_eq!( + for_image(cat.connection(), img(1)).unwrap(), + [normalise(" black \t guillemot ").unwrap()] + ); + } + + #[test] + fn a_blank_keyword_is_refused_rather_than_stored() { + let cat = seeded(); + assert!(matches!( + assign(cat.connection(), &[img(1)], " "), + Err(CatalogError::BadName(_)) + )); + } + + #[test] + fn an_overlong_keyword_is_cut_on_a_character_boundary() { + // A mis-paste, not an intention. Truncating on a byte would panic on + // the multi-byte characters an Icelandic place name is full of. + let cat = seeded(); + let long = "Þingvellir".repeat(40); + assign(cat.connection(), &[img(1)], &long).unwrap(); + let stored = &for_image(cat.connection(), img(1)).unwrap()[0]; + assert_eq!(stored.chars().count(), MAX_KEYWORD_LEN); + } + + #[test] + fn case_is_preserved() { + // "Iceland" is a place; "iceland" is a typo of it. + let cat = seeded(); + assign(cat.connection(), &[img(1)], "Iceland").unwrap(); + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), ["Iceland"]); + } + + #[test] + fn renaming_moves_every_assignment_with_it() { + // The failure this guards against: the vocabulary shows the new word + // and the search only finds the old one. + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "Icland").unwrap(); + let id = list(cat.connection()).unwrap()[0].id; + + rename(cat.connection(), id, "Iceland").unwrap(); + + assert_eq!(names(&cat), ["Iceland"]); + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), ["Iceland"]); + let q = crate::Query { + filter: Selector::Keyword("Iceland".into()), + ..Default::default() + }; + assert_eq!(cat.count(&q, 0).unwrap(), 2); + } + + #[test] + fn renaming_onto_an_existing_keyword_fuses_the_two() { + // Correcting a typo when the correct spelling already exists. Refusing + // would leave the user to fix it photograph by photograph. + let cat = seeded(); + assign(cat.connection(), &[img(1)], "Icland").unwrap(); + assign(cat.connection(), &[img(2)], "Iceland").unwrap(); + let typo = list(cat.connection()) + .unwrap() + .into_iter() + .find(|k| k.name == "Icland") + .unwrap() + .id; + + rename(cat.connection(), typo, "Iceland").unwrap(); + + assert_eq!(names(&cat), ["Iceland"]); + assert_eq!(list(cat.connection()).unwrap()[0].image_count, 2); + } + + #[test] + fn renaming_a_frame_that_carries_both_spellings_does_not_abort() { + // The primary key would reject a bare UPDATE for that one row and take + // the whole rename down with it. + let cat = seeded(); + assign(cat.connection(), &[img(1)], "Icland").unwrap(); + assign(cat.connection(), &[img(1)], "Iceland").unwrap(); + let typo = list(cat.connection()) + .unwrap() + .into_iter() + .find(|k| k.name == "Icland") + .unwrap() + .id; + + rename(cat.connection(), typo, "Iceland").unwrap(); + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), ["Iceland"]); + } + + #[test] + fn a_no_op_rename_is_not_an_edit() { + // A revision bumped for nothing lets an idle device win a merge + // against one that did real work. + let cat = seeded(); + let id = create(cat.connection(), "puffin").unwrap(); + let before = revision_of(&cat, id); + + assert_eq!(rename(cat.connection(), id, " puffin ").unwrap(), id); + assert_eq!(revision_of(&cat, id), before); + assert_eq!(deleted_of(&cat, id), 0, "and it does not retire the word"); + } + + #[test] + fn a_rename_retires_the_old_word_under_its_old_name() { + // The tombstone is what carries the rename to the other device, and it + // is read as "delete the assignments spelt this way" — so it has to + // keep the *old* spelling or it would delete the new word's. + let cat = seeded(); + assign(cat.connection(), &[img(1)], "Icland").unwrap(); + let old = list(cat.connection()).unwrap()[0].id; + + let new = rename(cat.connection(), old, "Iceland").unwrap(); + assert_ne!(new, old); + assert_eq!(deleted_of(&cat, old), 1); + assert_eq!(name_row(&cat, old), "Icland"); + assert!( + revision_of(&cat, old) > 1, + "the tombstone must out-revision the live row the other device holds" + ); + } + + fn revision_of(cat: &Catalog, id: KeywordId) -> i64 { + scalar(cat, "revision", id) + } + + fn deleted_of(cat: &Catalog, id: KeywordId) -> i64 { + scalar(cat, "deleted", id) + } + + /// One integer column of a term row. The column name is a literal from this + /// file and never user text — the same rule the module follows. + fn scalar(cat: &Catalog, column: &str, id: KeywordId) -> i64 { + cat.connection() + .query_row( + &format!("SELECT {column} FROM keyword_terms WHERE id = ?1"), + [id.0 as i64], + |r| r.get(0), + ) + .unwrap() + } + + fn name_row(cat: &Catalog, id: KeywordId) -> String { + cat.connection() + .query_row( + "SELECT name FROM keyword_terms WHERE id = ?1", + [id.0 as i64], + |r| r.get(0), + ) + .unwrap() + } + + #[test] + fn deleting_a_keyword_leaves_a_tombstone_and_takes_its_assignments() { + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "blurry").unwrap(); + let id = list(cat.connection()).unwrap()[0].id; + + assert_eq!(delete(cat.connection(), id).unwrap(), 2); + assert!(names(&cat).is_empty()); + assert!(for_image(cat.connection(), img(1)).unwrap().is_empty()); + + // The row survives, or a merge with a device that still holds the + // keyword would bring it straight back. + let deleted: i64 = cat + .connection() + .query_row( + "SELECT deleted FROM keyword_terms WHERE id = ?1", + [id.0 as i64], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(deleted, 1); + } + + #[test] + fn retyping_a_deleted_keyword_starts_a_fresh_identity() { + // Reusing the tombstoned row would make the new keyword inherit a + // revision that says "deleted" and lose an argument it was never in. + let cat = seeded(); + let first = create(cat.connection(), "puffin").unwrap(); + delete(cat.connection(), first).unwrap(); + + let second = create(cat.connection(), "puffin").unwrap(); + assert_ne!(first, second); + assert_eq!(names(&cat), ["puffin"]); + } + + #[test] + fn coverage_distinguishes_all_from_some() { + // The dash rather than the tick. Applying a word to forty frames where + // thirty already have it must not look like applying it to forty that + // do not. + let cat = seeded(); + assign(cat.connection(), &[img(1), img(2)], "puffin").unwrap(); + assign(cat.connection(), &[img(1)], "nest").unwrap(); + + let rows = for_images(cat.connection(), &[img(1), img(2)]).unwrap(); + let puffin = rows.iter().find(|r| r.keyword.name == "puffin").unwrap(); + let nest = rows.iter().find(|r| r.keyword.name == "nest").unwrap(); + + assert_eq!(puffin.coverage, Coverage::All); + assert_eq!(nest.coverage, Coverage::Some); + assert_eq!(nest.selected_count, 1); + } + + #[test] + fn a_keyword_no_one_in_the_selection_has_still_appears() { + // It is a target, not a report: the list is what the user assigns + // from, so hiding the unused words would hide the point of it. + let cat = seeded(); + create(cat.connection(), "puffin").unwrap(); + let rows = for_images(cat.connection(), &[img(1)]).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].coverage, Coverage::None); + } + + #[test] + fn the_vocabulary_leads_with_the_words_actually_used() { + let cat = seeded(); + create(cat.connection(), "unused").unwrap(); + assign(cat.connection(), &[img(1), img(2)], "puffin").unwrap(); + assert_eq!(names(&cat), ["puffin", "unused"]); + } + + #[test] + fn a_word_on_two_versions_counts_as_one_photograph() { + // A count that double-counts virtual copies is the kind of small lie + // that makes a user stop trusting the numbers. + let cat = seeded(); + let c = cat.connection(); + assign(c, &[img(1)], "puffin").unwrap(); + c.execute( + "INSERT INTO versions(image_id, uuid, name, is_default) VALUES (1, 'copy', 'Crop', 0)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, 'puffin')", + [c.last_insert_rowid()], + ) + .unwrap(); + + assert_eq!(list(c).unwrap()[0].image_count, 1); + } + + #[test] + fn an_image_with_no_version_gains_one_rather_than_losing_the_keyword() { + // A library scanned before versions existed. The user typed a word and + // expects it to stick. + let cat = seeded(); + let c = cat.connection(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES (99, 1, 'IMG_99.CR3', 0)", + [], + ) + .unwrap(); + + assign(c, &[img(99)], "puffin").unwrap(); + assert_eq!(for_image(c, img(99)).unwrap(), ["puffin"]); + } + + #[test] + fn a_hostile_keyword_is_stored_rather_than_executed() { + // The write side of `query`'s injection guard. It goes in as a + // parameter, comes back out unchanged, and the table is still there. + let cat = seeded(); + let evil = "'; DROP TABLE images; --"; + assign(cat.connection(), &[img(1)], evil).unwrap(); + + assert_eq!(for_image(cat.connection(), img(1)).unwrap(), [evil]); + let n: i64 = cat + .connection() + .query_row("SELECT count(*) FROM images", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 6); + } + + #[test] + fn words_that_predate_the_vocabulary_are_adopted() { + // A catalog rebuilt from sidecars, or keyworded by an older build. + let cat = seeded(); + let c = cat.connection(); + let version = crate::rating::default_version_id(c, img(1)).unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, 'seabird')", + [version], + ) + .unwrap(); + + assert_eq!(adopt_orphan_terms(c).unwrap(), 1); + assert_eq!(names(&cat), ["seabird"]); + // It runs on every open, so a second pass must find nothing to do. + assert_eq!(adopt_orphan_terms(c).unwrap(), 0); + } + + #[test] + fn an_orphaned_word_is_still_shown_before_it_is_adopted() { + let cat = seeded(); + let c = cat.connection(); + let version = crate::rating::default_version_id(c, img(1)).unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (?1, 'seabird')", + [version], + ) + .unwrap(); + + let rows = for_images(c, &[img(1)]).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].keyword.name, "seabird"); + assert_eq!(rows[0].keyword.id, KeywordId(0), "no identity yet"); + } + + #[test] + fn duplicate_identities_for_one_word_fuse_onto_the_smaller_uuid() { + // Two devices each typed "Iceland" before they had ever synced. + let cat = seeded(); + let c = cat.connection(); + c.execute( + "INSERT INTO keyword_terms(uuid, name, created, revision, modified) + VALUES ('aaaa', 'Iceland', 0, 1, 1), ('zzzz', 'Iceland', 0, 9, 9)", + [], + ) + .unwrap(); + + assert_eq!(fuse_duplicates(c).unwrap(), 1); + let survivor: String = c + .query_row("SELECT uuid FROM keyword_terms", [], |r| r.get(0)) + .unwrap(); + assert_eq!( + survivor, "aaaa", + "the rule must not consult the revision, or two devices pick differently" + ); + } + + #[test] + fn fusing_leaves_a_tombstone_alone() { + // A tombstone is the user's deletion. Retiring it as a duplicate would + // quietly undo it. + let cat = seeded(); + let c = cat.connection(); + c.execute( + "INSERT INTO keyword_terms(uuid, name, created, revision, modified, deleted) + VALUES ('aaaa', 'Iceland', 0, 1, 1, 0), ('zzzz', 'Iceland', 0, 9, 9, 1)", + [], + ) + .unwrap(); + + assert_eq!(fuse_duplicates(c).unwrap(), 0); + } + + #[test] + fn an_empty_selection_still_creates_the_keyword() { + // Typing a word into the field with nothing selected builds the + // vocabulary, which is a legitimate thing to do ahead of a shoot. + let cat = seeded(); + assert_eq!(assign(cat.connection(), &[], "puffin").unwrap(), 0); + assert_eq!(names(&cat), ["puffin"]); + } + + #[test] + fn renaming_a_keyword_that_is_gone_says_so() { + let cat = seeded(); + assert!(matches!( + rename(cat.connection(), KeywordId(404), "x"), + Err(CatalogError::NoSuchKeyword(404)) + )); + } +} diff --git a/core/dr-catalog/src/lib.rs b/core/dr-catalog/src/lib.rs index aca35d4..8a0994a 100644 --- a/core/dr-catalog/src/lib.rs +++ b/core/dr-catalog/src/lib.rs @@ -14,9 +14,10 @@ //! - [`walk`] — those decisions driven against real storage, local or SAF //! - [`query`] — selectors compiled to indexed SQL, windowed for the grid //! - [`collections`] — the collection tree and membership the UI edits +//! - [`keywords`] — the keyword vocabulary and what it is assigned to //! - [`jobs`] — the durable background work queue //! - [`trash`] — soft delete to a folder, then permanent delete -//! - [`merge`] / [`sync`] — cross-device collection merging +//! - [`merge`] / [`sync`] — cross-device merging of collections and keywords //! //! # The one thing everything is designed around //! @@ -35,6 +36,7 @@ pub mod cache; pub mod collections; pub mod error; pub mod jobs; +pub mod keywords; pub mod merge; pub mod query; pub mod rating; @@ -48,6 +50,7 @@ pub use cache::{Budget, Cache, DEFAULT_BUDGET_BYTES}; pub use collections::{Collection, CollectionKind, TreeRow}; pub use error::CatalogError; pub use jobs::{Job, JobKind, Priority}; +pub use keywords::{Coverage, Keyword, KeywordId, SelectionKeyword}; pub use merge::MergeReport; pub use query::{Query, Sort}; pub use rating::{Judgement, MAX_RATING}; diff --git a/core/dr-catalog/src/schema.rs b/core/dr-catalog/src/schema.rs index da13480..271f772 100644 --- a/core/dr-catalog/src/schema.rs +++ b/core/dr-catalog/src/schema.rs @@ -15,7 +15,7 @@ use rusqlite::Connection; use crate::error::CatalogError; /// Schema version this build writes and understands. -pub const SCHEMA_VERSION: i64 = 5; +pub const SCHEMA_VERSION: i64 = 6; /// Apply migrations up to [`SCHEMA_VERSION`]. /// @@ -66,6 +66,12 @@ pub fn migrate(conn: &Connection) -> Result { tx.pragma_update(None, "user_version", 5)?; tx.commit()?; } + if from < 6 { + let tx = conn.unchecked_transaction()?; + tx.execute_batch(V6)?; + tx.pragma_update(None, "user_version", 6)?; + tx.commit()?; + } Ok(from) } @@ -103,6 +109,20 @@ pub fn backfill(conn: &Connection) -> Result, Catalog out.push(("default_versions", n)); } + // v6: a vocabulary row for every word some image already carries. + // + // Three ways a catalog arrives holding assignments with no term behind + // them, and all three are normal rather than exceptional: a library + // keyworded by a build that predates this table, a catalog rebuilt from + // sidecars (which carry the word and not the identity), and an import from + // Lightroom or darktable (FR-CAT-14). Without this the words are + // searchable but absent from the vocabulary list, which reads as the + // keywords having been lost. + let n = crate::keywords::adopt_orphan_terms(conn)?; + if n > 0 { + out.push(("keyword_terms", n)); + } + Ok(out) } @@ -134,15 +154,45 @@ pub fn configure(conn: &Connection) -> Result<(), CatalogError> { /// in [`V1`]: every `CREATE TABLE`/`CREATE INDEX` must name its object /// unqualified, which they do. pub fn v1_for_attached(schema_name: &str) -> String { - V1.replace("CREATE TABLE ", &format!("CREATE TABLE {schema_name}.")) + rewrite_for_attached(V1, schema_name) + // REFERENCES within an attached schema resolve to that schema already, + // so foreign keys need no rewriting — but the ON clause of an index + // does, and `CREATE INDEX x.name ON table` is the correct form. +} + +/// Every table this build knows about, rewritten to target an attached +/// database. +/// +/// [`v1_for_attached`] is kept alongside this rather than replaced by it: a +/// remote catalog written by an older build genuinely has only the v1 tables, +/// and the merge has to keep working against one (see +/// [`crate::merge::merge_keywords`]). Building that case in a test needs a way +/// to say "v1 and no more". +/// +/// Only the migrations that *create* objects appear here. V2 through V5 are +/// `ALTER TABLE ... ADD COLUMN`, and the columns they add are local index +/// state — shadowing, trashing, cache pinning — that a merge never reads +/// across the attachment. +pub fn for_attached(schema_name: &str) -> String { + format!( + "{}\n{}", + rewrite_for_attached(V1, schema_name), + rewrite_for_attached(V6, schema_name) + ) +} + +/// Qualify every object a `CREATE` statement names with `schema_name`. +/// +/// The rewrite is textual and therefore only as good as the naming discipline +/// in the batches it is given: every `CREATE TABLE`/`CREATE INDEX` must name +/// its object unqualified, which they do. +fn rewrite_for_attached(sql: &str, schema_name: &str) -> String { + sql.replace("CREATE TABLE ", &format!("CREATE TABLE {schema_name}.")) .replace("CREATE INDEX ", &format!("CREATE INDEX {schema_name}.")) .replace( "CREATE UNIQUE INDEX ", &format!("CREATE UNIQUE INDEX {schema_name}."), ) - // REFERENCES within an attached schema resolve to that schema already, - // so foreign keys need no rewriting — but the ON clause of an index - // does, and `CREATE INDEX x.name ON table` is the correct form. } /// Mark each JPEG that sits beside a RAW of the same name. @@ -226,6 +276,63 @@ fn stem_of(path: &str) -> &str { } } +const V6: &str = r#" +-- TRACES: FR-CAT-5 | FR-CAT-6 | FR-NC-9 +-- Keywords gain an identity, so that renaming and deleting one can cross +-- between devices. +-- +-- The v1 `keywords` table is the *assignment*: one row per (version, word), +-- and the word is stored as text. That stays exactly as it is, and this +-- migration adds nothing to it, for a reason that is easy to get backwards. +-- +-- # Why assignments keep the text rather than pointing at a row here +-- +-- The catalog is a rebuildable index (ARCH §6.12). What an image is keyworded +-- with is authoritative in the sidecar and in XMP `dc:subject` (FR-CAT-13), +-- and both of those carry a *string*. Rewriting the join to reference +-- `keyword_terms(id)` would mean a catalog rebuilt from sidecars had to invent +-- term rows before it could record a single assignment, and an integer that +-- means nothing on the other device would sit where the durable fact belongs. +-- It would also break `crate::query`, which matches `kw.keyword` directly and +-- must keep hitting `keywords_term` on a 50k library (FR-CAT-6). +-- +-- So the text is the fact and this table is the *identity*: it exists to give +-- a rename and a deletion something a merge can key on, and to let a keyword +-- exist in the vocabulary before any photograph carries it. +CREATE TABLE keyword_terms ( + id INTEGER PRIMARY KEY, + -- Device-independent identity, as `collections.uuid` is. The integer id is + -- local and collides across devices. + uuid TEXT NOT NULL UNIQUE, + -- The word itself, and the value written into every assignment row. + name TEXT NOT NULL, + created INTEGER NOT NULL, + -- Monotonic, bumped on every local edit. `crate::merge` compares these + -- rather than timestamps, so a clock-skewed device cannot silently win. + revision INTEGER NOT NULL DEFAULT 1, + modified INTEGER NOT NULL, + -- Tombstone, so a merge against a device that still holds the keyword does + -- not resurrect it. + deleted INTEGER NOT NULL DEFAULT 0 +); + +-- Deliberately **not** UNIQUE. +-- +-- Two devices that each type "Iceland" create two rows with two uuids, and +-- both are correct until they meet. A unique constraint would abort the merge +-- transaction at exactly that moment — the ordinary case, not a corner one. +-- Uniqueness is instead reached by convergence: `crate::keywords::create` +-- resolves an existing name locally, and `crate::keywords::fuse_duplicates` +-- collapses a cross-device pair onto the lexicographically smaller uuid, which +-- both devices compute identically without talking to each other. +-- +-- Partial on `deleted = 0` because every lookup here is a live one: the +-- vocabulary list, the resolve-by-name in `create`, and the fuse pass all +-- exclude tombstones, and including them would grow the index with every +-- keyword the library has ever had rather than with the ones it has. +CREATE INDEX keyword_terms_name ON keyword_terms(name) WHERE deleted = 0; +"#; + const V5: &str = r#" -- TRACES: FR-NC-6a | FR-CAT-9 | NFR-RES-4 -- Offline availability: what is kept, why it is kept, and where it lives. @@ -671,6 +778,77 @@ mod tests { assert_eq!(bytes, 100, "the existing row is untouched"); } + #[test] + fn a_v5_catalog_keeps_its_keywords_and_gains_their_identities() { + // TRACES: FR-CAT-5 + // The migration case that matters here: a library keyworded by an + // import or an older build already has assignment rows, and they must + // survive into the vocabulary rather than being left searchable but + // invisible. + let c = mem(); + for step in [V1, V2, V3, V4, V5] { + c.execute_batch(step).unwrap(); + } + c.pragma_update(None, "user_version", 5).unwrap(); + c.execute( + "INSERT INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO images(id, root_id, source_ref, added_at) VALUES (7, 1, 'IMG_7.CR3', 0)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO versions(id, image_id, uuid, name, is_default) + VALUES (1, 7, 'v-7', 'Default', 1)", + [], + ) + .unwrap(); + c.execute( + "INSERT INTO keywords(version_id, keyword) VALUES (1, 'puffin')", + [], + ) + .unwrap(); + + assert_eq!(migrate(&c).unwrap(), 5, "migrated from v5"); + assert_eq!(backfilled(&c, "keyword_terms"), 1); + + let name: String = c + .query_row("SELECT name FROM keyword_terms", [], |r| r.get(0)) + .unwrap(); + assert_eq!(name, "puffin"); + // The assignment is untouched — it is the durable fact, and the term + // row is only its identity. + let n: i64 = c + .query_row("SELECT count(*) FROM keywords", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 1); + + // It runs on every open, so a second pass must find nothing to do. + assert_eq!(backfilled(&c, "keyword_terms"), 0); + } + + #[test] + fn two_devices_may_both_hold_a_term_of_the_same_name() { + // Deliberately not a unique index. Two devices each typing "Iceland" + // is the ordinary case, and a constraint would abort the merge + // transaction at exactly the moment they first sync. + let c = mem(); + migrate(&c).unwrap(); + c.execute( + "INSERT INTO keyword_terms(uuid, name, created, revision, modified) + VALUES ('a', 'Iceland', 0, 1, 1), ('b', 'Iceland', 0, 1, 1)", + [], + ) + .unwrap(); + let n: i64 = c + .query_row("SELECT count(*) FROM keyword_terms", [], |r| r.get(0)) + .unwrap(); + assert_eq!(n, 2); + } + #[test] fn stems_ignore_directories_containing_dots() { assert_eq!(stem_of("2026.08/IMG_1.CR2"), "IMG_1");