Merge master into tablet-selection
Two real conflicts, both from work that landed either side of the same lines rather than against them. `lib.rs`: the settings controller was hoisted above the People screen's wiring, and Android's thumbnail-tier eviction registered itself at the same point. Independent, so both stay. `library.rs`: manual collection ordering and burst folding each added a clause to the same two queries. The scoped range read now carries both — the folding matters there for one step further on than it does in the grid, because a collapsed burst is one cell, so an ordinal counted over a list still holding every frame names a photograph several places away from the one the user pointed at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+192
-11
@@ -80,7 +80,20 @@ pub enum ScanMessage {
|
||||
/// amount of string matching on the far side can reliably recover it.
|
||||
/// Without the flag a dead connection and a bad password produce the same
|
||||
/// banner, which sends the user to re-enter a credential that was fine.
|
||||
Failed { message: String, offline: bool },
|
||||
///
|
||||
/// `lost_root` is the same idea one step further out, and it is carried
|
||||
/// separately from `offline` rather than folded into it because the two
|
||||
/// end differently. An offline library comes back when the network does,
|
||||
/// with nothing asked of anyone; a library whose root cannot be opened
|
||||
/// comes back only when someone restores access to it — a share put back
|
||||
/// on the server, a drive plugged in, and in time a document tree granted
|
||||
/// again once one can be (FR-PLAT-AND-2). Both show the same grid of what
|
||||
/// is stored locally, and they must not offer the same explanation.
|
||||
Failed {
|
||||
message: String,
|
||||
offline: bool,
|
||||
lost_root: bool,
|
||||
},
|
||||
}
|
||||
|
||||
/// One decoded thumbnail, ready for the grid.
|
||||
@@ -186,6 +199,26 @@ const VISIBLE_UNALIASED: &str = "shadowed_by IS NULL AND trashed_at IS NULL";
|
||||
/// restore the same frame twice.
|
||||
const TRASHED: &str = "i.shadowed_by IS NULL AND i.trashed_at IS NOT NULL";
|
||||
|
||||
/// TRACES: FR-CULL-5
|
||||
/// The clause that hides the frames a collapsed burst is standing in for.
|
||||
///
|
||||
/// Subject to exactly the discipline [`VISIBLE`] is under, and for the same
|
||||
/// reason: the header's count, the scrollbar's size, the run a shift-click
|
||||
/// resolves and the ordinal a scrub lands on are four answers about one list.
|
||||
/// A burst folded away in the cells but still counted in the total would leave
|
||||
/// the grid ending in rows that draw nothing, with no clue why.
|
||||
///
|
||||
/// The predicate itself is `dr_catalog::bursts`'s, not this file's, so the
|
||||
/// interface and the pass that writes the table cannot come to disagree about
|
||||
/// what collapsed means.
|
||||
///
|
||||
/// A function rather than a constant because it has to name the image table,
|
||||
/// and the grid aliases it as `i` where the timeline's queries do not. `image`
|
||||
/// is a table name from this file and never anything a user supplied.
|
||||
fn uncollapsed(image: &str) -> String {
|
||||
format!(" AND {}", dr_catalog::bursts::not_collapsed_away(image))
|
||||
}
|
||||
|
||||
/// TRACES: FR-CAT-4
|
||||
/// The order the grid lists photographs in: when they were taken.
|
||||
///
|
||||
@@ -1146,6 +1179,7 @@ pub fn spawn_scan(
|
||||
let _ = tx.send(ScanMessage::Failed {
|
||||
message: e.message,
|
||||
offline: e.offline,
|
||||
lost_root: e.lost_root,
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -1160,6 +1194,7 @@ pub fn spawn_scan(
|
||||
struct ScanFailure {
|
||||
message: String,
|
||||
offline: bool,
|
||||
lost_root: bool,
|
||||
}
|
||||
|
||||
impl ScanFailure {
|
||||
@@ -1169,6 +1204,7 @@ impl ScanFailure {
|
||||
Self {
|
||||
message: message.to_string(),
|
||||
offline: false,
|
||||
lost_root: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1177,11 +1213,48 @@ impl From<dr_sync::RemoteError> for ScanFailure {
|
||||
fn from(e: dr_sync::RemoteError) -> Self {
|
||||
Self {
|
||||
offline: e.indicates_offline(),
|
||||
lost_root: e.indicates_lost_root(),
|
||||
message: e.to_string(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-PLAT-AND-2 | FR-CAT-9
|
||||
/// Record that a library can no longer be opened, without losing it.
|
||||
///
|
||||
/// Called on the worker, before the failure crosses the channel, because this
|
||||
/// is where the catalog handle is — and because the marking must be durable
|
||||
/// whether or not anyone is left to draw a banner. A process killed between
|
||||
/// the failure and the next launch must still come back knowing what it could
|
||||
/// not reach.
|
||||
///
|
||||
/// Nothing is deleted. Every rating, every edit and every row stays exactly
|
||||
/// where it was; what changes is that the images now say they are offline, so
|
||||
/// the grid can show them as held-not-here rather than as ordinary
|
||||
/// photographs whose thumbnails happen to be failing one at a time.
|
||||
///
|
||||
/// A root with no row yet is the first scan of a library that has never
|
||||
/// succeeded, and there is nothing to mark — the failure alone is the whole
|
||||
/// story, and the launch screen is where it is told.
|
||||
fn mark_library_offline(catalog: &Catalog, root: &str) {
|
||||
let conn = catalog.connection();
|
||||
let root_id: Option<i64> = conn
|
||||
.query_row(
|
||||
"SELECT id FROM roots WHERE label = ?1 AND kind = 'remote'",
|
||||
[root],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.ok();
|
||||
let Some(root_id) = root_id else {
|
||||
log::info!("library {root} has no catalog root yet; nothing to mark offline");
|
||||
return;
|
||||
};
|
||||
match dr_catalog::mark_root_offline(conn, dr_types::RootId(root_id as u64)) {
|
||||
Ok(()) => log::warn!("library {root} is unreachable; its images are marked offline"),
|
||||
Err(e) => log::error!("could not mark {root} offline: {e}"),
|
||||
}
|
||||
}
|
||||
|
||||
fn run_scan(
|
||||
tx: &Sender<ScanMessage>,
|
||||
conn: Connection,
|
||||
@@ -1199,21 +1272,50 @@ fn run_scan(
|
||||
let rt = crate::net_runtime::build().map_err(ScanFailure::local)?;
|
||||
|
||||
rt.block_on(async {
|
||||
let backend = crate::remote::connect(&conn).map_err(ScanFailure::local)?;
|
||||
// TRACES: FR-PLAT-AND-2 | FR-CAT-9
|
||||
// Classified rather than flattened to a local failure, because the
|
||||
// removed-card case never gets as far as a request: the folder
|
||||
// connector checks its root when it is constructed, so a library on an
|
||||
// ejected card fails here and not in the walk. Reported as an ordinary
|
||||
// error it left the grid showing a healthy library of images that
|
||||
// could no longer be opened, one silent thumbnail failure at a time.
|
||||
let backend = match crate::remote::connect(&conn) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
if e.indicates_lost_root() {
|
||||
mark_library_offline(&catalog, &root);
|
||||
}
|
||||
return Err(e.into());
|
||||
}
|
||||
};
|
||||
|
||||
// Stored folder ETags, so an unchanged subtree is skipped whole. On a
|
||||
// first run this is empty and the walk is complete; on every run after
|
||||
// it is what keeps cost proportional to what changed (ARCH §8.4).
|
||||
let known = load_folder_etags(&catalog, &root);
|
||||
|
||||
let result = dr_sync::scan(&*backend, &RemotePath::new(&root), &filter, &known, |p| {
|
||||
let scanned = dr_sync::scan(&*backend, &RemotePath::new(&root), &filter, &known, |p| {
|
||||
let _ = tx.send(ScanMessage::Progress {
|
||||
directories: p.directories_listed,
|
||||
pruned: p.directories_pruned,
|
||||
images: p.images_found,
|
||||
});
|
||||
})
|
||||
.await?;
|
||||
.await;
|
||||
|
||||
// TRACES: FR-PLAT-AND-2 | FR-CAT-9
|
||||
// Written before the failure is reported, not after: the banner is a
|
||||
// consequence of the catalog state and not the other way round, and a
|
||||
// process that dies between the two must come back knowing.
|
||||
let result = match scanned {
|
||||
Ok(r) => r,
|
||||
Err(e) => {
|
||||
if e.indicates_lost_root() {
|
||||
mark_library_offline(&catalog, &root);
|
||||
}
|
||||
return Err(e.into());
|
||||
}
|
||||
};
|
||||
|
||||
persist(&catalog, &root, &result).map_err(ScanFailure::local)?;
|
||||
|
||||
@@ -1306,13 +1408,27 @@ fn persist(
|
||||
.ok()
|
||||
});
|
||||
|
||||
// TRACES: FR-PLAT-AND-2 | FR-CAT-9
|
||||
// The `availability` arm is what ends an offline library, and it does
|
||||
// it one photograph at a time. 3 is `Availability::Offline` and 0 is
|
||||
// `MetadataOnly`, the same code this statement inserts new rows with —
|
||||
// so a row that was marked offline when the root became unreachable is
|
||||
// returned to exactly the state a fresh scan would have given it, and
|
||||
// a row that was never marked is not touched at all.
|
||||
//
|
||||
// Conditional rather than a blanket reset for the same reason
|
||||
// `dr_catalog::walk` restores per file rather than per root: the only
|
||||
// thing that may clear "I could not reach this" is having reached it,
|
||||
// and this statement runs precisely once per file the scan listed.
|
||||
tx.execute(
|
||||
"INSERT INTO images(root_id, folder_id, source_ref, format, file_size,
|
||||
availability, metadata_state, added_at)
|
||||
VALUES (?1, ?2, ?3, ?4, ?5, 0, 1, ?6)
|
||||
ON CONFLICT(root_id, source_ref) DO UPDATE SET
|
||||
file_size = excluded.file_size,
|
||||
folder_id = excluded.folder_id",
|
||||
folder_id = excluded.folder_id,
|
||||
availability = CASE WHEN images.availability = 3
|
||||
THEN 0 ELSE images.availability END",
|
||||
rusqlite::params![
|
||||
root_id,
|
||||
folder_id,
|
||||
@@ -3816,10 +3932,11 @@ pub fn read_cells_scoped(
|
||||
.join(",");
|
||||
let rated = filter.sql();
|
||||
let (order, order_params) = grid_order_for(catalog, Some(scope));
|
||||
let folded = uncollapsed("i");
|
||||
let sql = format!(
|
||||
"SELECT {CELL_COLUMNS}
|
||||
FROM images i
|
||||
WHERE {VISIBLE}{rated}
|
||||
WHERE {VISIBLE}{rated}{folded}
|
||||
AND i.id IN (SELECT image_id FROM collection_members
|
||||
WHERE collection_id IN ({placeholders}))
|
||||
{order}
|
||||
@@ -3854,11 +3971,12 @@ fn read_cells_all(
|
||||
limit: usize,
|
||||
) -> Result<Vec<LibraryCell>, dr_catalog::CatalogError> {
|
||||
let rated = filter.sql();
|
||||
let folded = uncollapsed("i");
|
||||
let mut rows = {
|
||||
let mut stmt = catalog.connection().prepare(&format!(
|
||||
"SELECT {CELL_COLUMNS}
|
||||
FROM images i
|
||||
WHERE {VISIBLE}{rated}
|
||||
WHERE {VISIBLE}{rated}{folded}
|
||||
{GRID_ORDER}
|
||||
LIMIT ?1 OFFSET ?2"
|
||||
))?;
|
||||
@@ -3964,10 +4082,15 @@ pub fn read_ids_span(
|
||||
// different ORDER BY names a different photograph.
|
||||
let (order, order_params) = grid_order_for(catalog, scope);
|
||||
params.extend(order_params);
|
||||
// And the same folding, for the same reason one step further on: a
|
||||
// collapsed burst is one cell in the grid, so an ordinal counted over
|
||||
// a list that still held every frame of it would name a photograph
|
||||
// several places away from the one the user pointed at.
|
||||
let folded = uncollapsed("i");
|
||||
(
|
||||
format!(
|
||||
"SELECT i.id FROM images i
|
||||
WHERE {VISIBLE}{rated}{clause}
|
||||
WHERE {VISIBLE}{rated}{folded}{clause}
|
||||
{order}
|
||||
LIMIT ? OFFSET ?"
|
||||
),
|
||||
@@ -4096,9 +4219,10 @@ pub fn total_images_scoped(
|
||||
// Counted through `images` rather than over `collection_members` alone, so
|
||||
// `VISIBLE` applies — a trashed photograph is still a member row, and
|
||||
// counting it made the header claim images the grid would not draw.
|
||||
let folded = uncollapsed("i");
|
||||
let sql = format!(
|
||||
"SELECT count(DISTINCT i.id) FROM images i
|
||||
WHERE {VISIBLE}{rated}
|
||||
WHERE {VISIBLE}{rated}{folded}
|
||||
AND i.id IN (SELECT image_id FROM collection_members
|
||||
WHERE collection_id IN ({placeholders}))"
|
||||
);
|
||||
@@ -4409,8 +4533,9 @@ fn total_images_filtered(
|
||||
filter: &RatingFilter,
|
||||
) -> Result<usize, dr_catalog::CatalogError> {
|
||||
let rated = filter.sql();
|
||||
let folded = uncollapsed("i");
|
||||
let n: i64 = catalog.connection().query_row(
|
||||
&format!("SELECT count(*) FROM images i WHERE {VISIBLE}{rated}"),
|
||||
&format!("SELECT count(*) FROM images i WHERE {VISIBLE}{rated}{folded}"),
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)?;
|
||||
@@ -4956,12 +5081,16 @@ mod tests {
|
||||
#[test]
|
||||
fn the_window_read_walks_the_ordering_index() {
|
||||
let catalog = with_images(20);
|
||||
// Including the burst clause, because the grid includes it: a
|
||||
// predicate that quietly cost the ordering index would put the sort
|
||||
// back and this is the only place that would notice.
|
||||
let folded = uncollapsed("i");
|
||||
let plan: Vec<String> = catalog
|
||||
.connection()
|
||||
.prepare(&format!(
|
||||
"EXPLAIN QUERY PLAN
|
||||
SELECT {CELL_COLUMNS} FROM images i
|
||||
WHERE {VISIBLE}
|
||||
WHERE {VISIBLE}{folded}
|
||||
{GRID_ORDER}
|
||||
LIMIT 10 OFFSET 5"
|
||||
))
|
||||
@@ -5014,6 +5143,58 @@ mod tests {
|
||||
catalog
|
||||
}
|
||||
|
||||
/// TRACES: FR-CULL-5
|
||||
/// A folded burst takes rows out of the cells, the count and the range a
|
||||
/// shift-click resolves — all three, together.
|
||||
///
|
||||
/// This is the test that would fail if the clause were added to four of
|
||||
/// the five queries that need it. That failure has no other symptom: the
|
||||
/// header claims images the grid will not draw, the scrollbar sizes itself
|
||||
/// for rows that are not there, and neither number looks wrong on its own.
|
||||
#[test]
|
||||
fn folding_a_burst_takes_the_same_rows_out_of_every_answer() {
|
||||
use dr_catalog::bursts::{self, Rules, Signature};
|
||||
|
||||
let catalog = with_images(4);
|
||||
// Three of the four are one burst: a second apart, one signature.
|
||||
let ids = image_ids(&catalog);
|
||||
for (n, id) in ids.iter().enumerate() {
|
||||
let hash = if n < 3 { 0xFF00 } else { 0x00FF };
|
||||
catalog
|
||||
.connection()
|
||||
.execute(
|
||||
"UPDATE images SET captured_at = ?2, camera = 'Canon EOS R5',
|
||||
perceptual_hash = ?3
|
||||
WHERE id = ?1",
|
||||
rusqlite::params![id.0 as i64, 1_000 + n as i64, Signature(hash).to_stored()],
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
bursts::regroup(catalog.connection(), Rules::default()).unwrap();
|
||||
|
||||
let filter = RatingFilter::default();
|
||||
// Open, as a new burst is: nothing has been taken away yet.
|
||||
assert_eq!(read_cells(&catalog, 0, 50).unwrap().len(), 4);
|
||||
assert_eq!(total_images_filtered(&catalog, &filter).unwrap(), 4);
|
||||
|
||||
bursts::set_expanded(catalog.connection(), ids[0], false).unwrap();
|
||||
|
||||
let cells = read_cells(&catalog, 0, 50).unwrap();
|
||||
assert_eq!(cells.len(), 2, "the folded frames are still in the cells");
|
||||
assert_eq!(
|
||||
total_images_filtered(&catalog, &filter).unwrap(),
|
||||
cells.len(),
|
||||
"the header's count and the cells disagree"
|
||||
);
|
||||
assert_eq!(
|
||||
read_ids_span(&catalog, None, &filter, false, 0, 49)
|
||||
.unwrap()
|
||||
.len(),
|
||||
cells.len(),
|
||||
"a shift-click over the whole grid would select frames it cannot show"
|
||||
);
|
||||
}
|
||||
|
||||
fn image_ids(catalog: &Catalog) -> Vec<dr_types::ImageId> {
|
||||
let mut stmt = catalog
|
||||
.connection()
|
||||
|
||||
Reference in New Issue
Block a user