mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-14 19:27:01 -04:00
Never crash: Repair damaged catalog snapshots on open
Fix ids, stacks, album folders and counters that don't agree, and stop album trees at 64 deep.
This commit is contained in:
1 parent
b3d392b499
commit
271bc41842
9 files changed
+311
-18
No files matched your search
@@ -113,6 +113,8 @@ pub struct LoadReport {
|
||||
/// The library was in an older catalog format (`0` = before format versions: a log without a
|
||||
/// snapshot) and was rewritten in this version's format ([`VERSION`]).
|
||||
pub upgraded_from: Option<u32>,
|
||||
/// What [`Catalog::repair`] changed in a damaged snapshot (empty for a sound one).
|
||||
pub repaired: Vec<String>,
|
||||
}
|
||||
|
||||
/// Where persistence time goes (reported by `library.info` → `persistence`; printed to stderr
|
||||
@@ -328,7 +330,12 @@ impl Journal {
|
||||
return Err(newer(format!("{SNAPSHOT} is catalog format v{}", h.version)));
|
||||
}
|
||||
let s: SnapFile = serde_json::from_slice(&bytes).map_err(|e| CatalogError::Corrupt(format!("{SNAPSHOT}: {e}")))?;
|
||||
(s.catalog, s.seq, Some(h.version))
|
||||
let mut catalog = s.catalog;
|
||||
report.repaired = catalog.repair();
|
||||
for r in &report.repaired {
|
||||
log::warn!("{SNAPSHOT}: {r}");
|
||||
}
|
||||
(catalog, s.seq, Some(h.version))
|
||||
}
|
||||
None => (Catalog::new(), 0, None),
|
||||
};
|
||||
|
||||
+102
-9
@@ -290,19 +290,21 @@ impl Catalog {
|
||||
|
||||
// ---- ids
|
||||
|
||||
// (saturating: a counter a damaged snapshot left at the top hands out its last id again, which
|
||||
// the op that adds it then refuses as taken, rather than overflowing)
|
||||
pub fn alloc_photo_id(&mut self) -> PhotoId {
|
||||
let id = PhotoId(self.next_photo.max(1));
|
||||
self.next_photo = id.0 + 1;
|
||||
self.next_photo = id.0.saturating_add(1);
|
||||
id
|
||||
}
|
||||
pub fn alloc_album_id(&mut self) -> AlbumId {
|
||||
let id = AlbumId(self.next_album.max(1));
|
||||
self.next_album = id.0 + 1;
|
||||
self.next_album = id.0.saturating_add(1);
|
||||
id
|
||||
}
|
||||
pub fn alloc_stack_id(&mut self) -> StackId {
|
||||
let id = StackId(self.next_stack.max(1));
|
||||
self.next_stack = id.0 + 1;
|
||||
self.next_stack = id.0.saturating_add(1);
|
||||
id
|
||||
}
|
||||
|
||||
@@ -497,7 +499,7 @@ impl Catalog {
|
||||
return Err(CatalogError::Invalid(format!("photo {:?} exists", photo.id)));
|
||||
}
|
||||
let id = photo.id;
|
||||
self.next_photo = self.next_photo.max(id.0 + 1);
|
||||
self.next_photo = self.next_photo.max(id.0.saturating_add(1));
|
||||
self.photos.insert(id, Arc::new(*photo));
|
||||
Op::RemovePhoto { id }
|
||||
}
|
||||
@@ -587,7 +589,7 @@ impl Catalog {
|
||||
self.validate_rules(rules)?;
|
||||
}
|
||||
let id = album.id;
|
||||
self.next_album = self.next_album.max(id.0 + 1);
|
||||
self.next_album = self.next_album.max(id.0.saturating_add(1));
|
||||
self.albums.insert(id, album);
|
||||
Op::RemoveAlbum { id }
|
||||
}
|
||||
@@ -607,13 +609,15 @@ impl Catalog {
|
||||
if p == id || !self.albums.get(&p).is_some_and(|a| a.folder) {
|
||||
return Err(CatalogError::Invalid("parent must be another folder".into()));
|
||||
}
|
||||
// no cycles
|
||||
// no cycles (bounded: a loop elsewhere in a damaged catalog must not hang this)
|
||||
let mut cur = Some(p);
|
||||
let mut steps = 0usize;
|
||||
while let Some(c) = cur {
|
||||
if c == id {
|
||||
if c == id || steps > self.albums.len() {
|
||||
return Err(CatalogError::Invalid("cannot move a folder into itself".into()));
|
||||
}
|
||||
cur = self.albums.get(&c).and_then(|a| a.parent);
|
||||
steps += 1;
|
||||
}
|
||||
}
|
||||
let a = self.album_mut(id)?;
|
||||
@@ -659,7 +663,7 @@ impl Catalog {
|
||||
}
|
||||
self.validate_stack(stack.id, &stack.photos)?;
|
||||
let id = stack.id;
|
||||
self.next_stack = self.next_stack.max(id.0 + 1);
|
||||
self.next_stack = self.next_stack.max(id.0.saturating_add(1));
|
||||
self.stacks.insert(id, stack);
|
||||
Op::RemoveStack { id }
|
||||
}
|
||||
@@ -823,7 +827,94 @@ impl Catalog {
|
||||
}
|
||||
|
||||
pub fn from_snapshot(s: &str) -> Result<Catalog> {
|
||||
serde_json::from_str(s).map_err(|e| CatalogError::Corrupt(e.to_string()))
|
||||
let mut c: Catalog = serde_json::from_str(s).map_err(|e| CatalogError::Corrupt(e.to_string()))?;
|
||||
c.repair();
|
||||
Ok(c)
|
||||
}
|
||||
|
||||
/// Make a catalog read from a snapshot hold together where a damaged or hand-edited file left
|
||||
/// it inconsistent, so nothing that relies on it can fail (code indexes stacks by the ids it
|
||||
/// finds in them, and walks folders up to the top): every photo, album and stack carries the
|
||||
/// id it is filed under; the id counters lie past every id in use; a stack keeps only photos
|
||||
/// in the catalog, once each, and goes when fewer than two are left; an album whose parent is
|
||||
/// missing, not a folder or inside itself moves to the top level. Returns what was changed
|
||||
/// (nothing, for a catalog this build wrote).
|
||||
pub fn repair(&mut self) -> Vec<String> {
|
||||
let mut changed = Vec::new();
|
||||
for (id, p) in self.photos.iter_mut() {
|
||||
if p.id != *id {
|
||||
changed.push(format!("photo {} was filed as {}", p.id.0, id.0));
|
||||
Arc::make_mut(p).id = *id;
|
||||
}
|
||||
}
|
||||
for (id, a) in self.albums.iter_mut() {
|
||||
if a.id != *id {
|
||||
changed.push(format!("album {} was filed as {}", a.id.0, id.0));
|
||||
a.id = *id;
|
||||
}
|
||||
}
|
||||
let photos = &self.photos;
|
||||
self.stacks.retain(|id, st| {
|
||||
if st.id != *id {
|
||||
changed.push(format!("stack {} was filed as {}", st.id.0, id.0));
|
||||
st.id = *id;
|
||||
}
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
let before = st.photos.len();
|
||||
st.photos.retain(|p| photos.contains_key(p) && seen.insert(*p));
|
||||
if st.photos.len() != before {
|
||||
changed.push(format!("stack {}: {} missing or repeated photo(s) left out", id.0, before - st.photos.len()));
|
||||
}
|
||||
let keep = st.photos.len() >= 2;
|
||||
if !keep {
|
||||
changed.push(format!("stack {} had fewer than two photos: removed", id.0));
|
||||
}
|
||||
keep
|
||||
});
|
||||
// a photo in two stacks stays in the first
|
||||
let mut stacked = std::collections::HashSet::new();
|
||||
self.stacks.retain(|id, st| {
|
||||
let before = st.photos.len();
|
||||
st.photos.retain(|p| stacked.insert(*p));
|
||||
if st.photos.len() != before {
|
||||
changed.push(format!("stack {}: {} photo(s) already in another stack left out", id.0, before - st.photos.len()));
|
||||
}
|
||||
st.photos.len() >= 2
|
||||
});
|
||||
let parents: Vec<(AlbumId, Option<AlbumId>)> = self.albums.values().map(|a| (a.id, a.parent)).collect();
|
||||
for (id, parent) in parents {
|
||||
let Some(p) = parent else { continue };
|
||||
let mut cur = Some(p);
|
||||
let mut steps = 0usize;
|
||||
let ok = loop {
|
||||
match cur {
|
||||
None => break true,
|
||||
Some(c) if c == id || steps > self.albums.len() => break false,
|
||||
Some(c) => match self.albums.get(&c) {
|
||||
Some(a) if a.folder => cur = a.parent,
|
||||
_ => break false,
|
||||
},
|
||||
}
|
||||
steps += 1;
|
||||
};
|
||||
if !ok && let Some(a) = self.albums.get_mut(&id) {
|
||||
changed.push(format!("album {}: its folder {} is missing, not a folder or inside it: moved to the top level", id.0, p.0));
|
||||
a.parent = None;
|
||||
}
|
||||
}
|
||||
let past = |last: Option<u64>| last.map_or(1, |v| v.saturating_add(1));
|
||||
let (photo, album, stack) = (
|
||||
past(self.photos.keys().last().map(|k| k.0)),
|
||||
past(self.albums.keys().last().map(|k| k.0)),
|
||||
past(self.stacks.keys().last().map(|k| k.0)),
|
||||
);
|
||||
if self.next_photo < photo || self.next_album < album || self.next_stack < stack {
|
||||
changed.push("id counters behind the ids in use: moved past them".into());
|
||||
self.next_photo = self.next_photo.max(photo);
|
||||
self.next_album = self.next_album.max(album);
|
||||
self.next_stack = self.next_stack.max(stack);
|
||||
}
|
||||
changed
|
||||
}
|
||||
|
||||
/// Replay an op log (JSON lines) on top of `self`. A torn final line (crash mid-write) is ignored.
|
||||
@@ -878,4 +969,6 @@ mod tests_local;
|
||||
#[cfg(test)]
|
||||
mod tests_lock;
|
||||
#[cfg(test)]
|
||||
mod tests_repair;
|
||||
#[cfg(test)]
|
||||
mod tests_torn_append;
|
||||
@@ -25,7 +25,7 @@ impl Catalog {
|
||||
}
|
||||
/// Photo → (stack, position in the stack), for every stacked photo.
|
||||
pub fn stack_index(&self) -> HashMap<PhotoId, (StackId, usize)> {
|
||||
self.stacks.values().flat_map(|s| s.photos.iter().enumerate().map(move |(i, p)| (*p, (s.id, i)))).collect()
|
||||
self.stacks.iter().flat_map(|(id, s)| s.photos.iter().enumerate().map(move |(i, p)| (*p, (*id, i)))).collect()
|
||||
}
|
||||
|
||||
pub(crate) fn validate_stack(&self, id: StackId, photos: &[PhotoId]) -> Result<()> {
|
||||
@@ -70,7 +70,7 @@ impl Catalog {
|
||||
}
|
||||
}
|
||||
for sid in &touched {
|
||||
for p in self.stacks[sid].photos.clone() {
|
||||
for p in self.stacks.get(sid).map(|s| s.photos.clone()).unwrap_or_default() {
|
||||
push(&mut order, p);
|
||||
}
|
||||
}
|
||||
@@ -186,7 +186,10 @@ impl Catalog {
|
||||
if !done.insert(*sid) {
|
||||
continue;
|
||||
}
|
||||
let s = &self.stacks[sid];
|
||||
let Some(s) = self.stacks.get(sid) else {
|
||||
out.push(*id);
|
||||
continue;
|
||||
};
|
||||
let members = s.photos.iter().copied().filter(|p| present.contains(p));
|
||||
if s.collapsed {
|
||||
out.extend(members.take(1));
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
//! A damaged or hand-edited snapshot that still parses is made to hold together on load
|
||||
//! ([`Catalog::repair`]): found by fuzzing snapshots of a real library.
|
||||
|
||||
use crate::journal::SNAPSHOT;
|
||||
use crate::*;
|
||||
|
||||
fn album(id: u64, parent: Option<u64>, folder: bool, photos: Vec<PhotoId>) -> Album {
|
||||
Album {
|
||||
id: AlbumId(id),
|
||||
name: format!("A{id}"),
|
||||
parent: parent.map(AlbumId),
|
||||
folder,
|
||||
photos,
|
||||
cover: None,
|
||||
smart: None,
|
||||
quick: false,
|
||||
order: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// A catalog of four photos, a stack of two, a folder with an album in it.
|
||||
fn sound() -> (Catalog, Vec<PhotoId>) {
|
||||
let mut c = Catalog::new();
|
||||
let mut ids = Vec::new();
|
||||
for i in 0..4 {
|
||||
let id = c.alloc_photo_id();
|
||||
c.apply(Op::AddPhoto {
|
||||
photo: Box::new(Photo::new(id, Source::File { path: format!("/p/{i}.jpg") }, &format!("{i}.jpg"), "JPEG", 60, 40, "2026-01-01")),
|
||||
})
|
||||
.unwrap();
|
||||
ids.push(id);
|
||||
}
|
||||
let sid = c.alloc_stack_id();
|
||||
c.apply(Op::AddStack { stack: Stack { id: sid, photos: vec![ids[0], ids[1]], collapsed: true } }).unwrap();
|
||||
let (f, a) = (c.alloc_album_id(), c.alloc_album_id());
|
||||
c.apply(Op::AddAlbum { album: album(f.0, None, true, vec![]) }).unwrap();
|
||||
c.apply(Op::AddAlbum { album: album(a.0, Some(f.0), false, vec![ids[2]]) }).unwrap();
|
||||
(c, ids)
|
||||
}
|
||||
|
||||
/// The snapshot file of `c` with `edit` applied to its JSON, loaded through the journal.
|
||||
fn load_edited(c: &Catalog, edit: impl FnOnce(&mut serde_json::Value)) -> (Catalog, LoadReport) {
|
||||
let m = MemStore::new();
|
||||
let (mut j, _, _) = Journal::open(Box::new(m.clone())).unwrap();
|
||||
j.snapshot(c).unwrap();
|
||||
let mut v: serde_json::Value = serde_json::from_slice(&m.get(SNAPSHOT).unwrap()).unwrap();
|
||||
edit(&mut v["catalog"]);
|
||||
m.set(SNAPSHOT, serde_json::to_vec(&v).unwrap());
|
||||
drop(j);
|
||||
let (_, c, report) = Journal::open(Box::new(m)).unwrap();
|
||||
(c, report)
|
||||
}
|
||||
|
||||
/// A stack whose `id` differs from the key it is filed under made `arrange_stacks` index the map
|
||||
/// with an id it doesn't hold, which panics: the grid of any library with such a stack.
|
||||
#[test]
|
||||
fn a_stack_filed_under_another_id_is_arranged() {
|
||||
let (c, ids) = sound();
|
||||
let (c, report) = load_edited(&c, |v| {
|
||||
let key = v["stacks"].as_object().unwrap().keys().next().unwrap().clone();
|
||||
v["stacks"][&key]["id"] = 77.into();
|
||||
});
|
||||
assert!(report.repaired.iter().any(|r| r.contains("stack 77 was filed as")), "{:?}", report.repaired);
|
||||
assert_eq!(c.arrange_stacks(&ids), vec![ids[0], ids[2], ids[3]], "the collapsed stack shows its top");
|
||||
assert!(c.stacks().all(|s| c.stack_of(s.photos[0]).is_some_and(|t| t.id == s.id)));
|
||||
}
|
||||
|
||||
/// Stacks of missing, repeated or too few photos (or one photo in two stacks) are put right: a
|
||||
/// stack of none had no top (`photos[0]`).
|
||||
#[test]
|
||||
fn stacks_keep_only_photos_in_the_catalog() {
|
||||
let (c, ids) = sound();
|
||||
let (c, report) = load_edited(&c, |v| {
|
||||
let key = v["stacks"].as_object().unwrap().keys().next().unwrap().clone();
|
||||
v["stacks"][&key]["photos"] = serde_json::json!([ids[0].0, 999, ids[0].0, ids[1].0]);
|
||||
v["stacks"]["8"] = serde_json::json!({"id": 8, "photos": []});
|
||||
v["stacks"]["9"] = serde_json::json!({"id": 9, "photos": [ids[1].0, ids[3].0, ids[2].0]});
|
||||
});
|
||||
let stacks: Vec<(StackId, Vec<PhotoId>)> = c.stacks().map(|s| (s.id, s.photos.clone())).collect();
|
||||
assert_eq!(stacks.len(), 2, "{stacks:?} {:?}", report.repaired);
|
||||
assert_eq!(stacks[0].1, vec![ids[0], ids[1]], "missing and repeated photos left out");
|
||||
assert_eq!(stacks[1].1, vec![ids[3], ids[2]], "a photo already in a stack stays in the first");
|
||||
assert!(!c.arrange_stacks(&ids).is_empty());
|
||||
}
|
||||
|
||||
/// Folders inside each other (a loop), or under an album or a missing folder, move to the top
|
||||
/// level: walking up from them never ends otherwise, and moving any folder there hung.
|
||||
#[test]
|
||||
fn folder_loops_and_missing_parents_move_to_the_top() {
|
||||
let (c, _) = sound();
|
||||
let (mut c, report) = load_edited(&c, |v| {
|
||||
v["albums"]["10"] = serde_json::to_value(album(10, Some(11), true, vec![])).unwrap();
|
||||
v["albums"]["11"] = serde_json::to_value(album(11, Some(10), true, vec![])).unwrap();
|
||||
v["albums"]["12"] = serde_json::to_value(album(12, Some(12), true, vec![])).unwrap();
|
||||
v["albums"]["13"] = serde_json::to_value(album(13, Some(404), false, vec![])).unwrap();
|
||||
});
|
||||
assert!(!report.repaired.is_empty());
|
||||
for id in [10, 11, 12, 13] {
|
||||
let a = c.album(AlbumId(id)).unwrap();
|
||||
assert!(
|
||||
a.parent.is_none() || a.parent.is_some_and(|p| c.album(p).is_some_and(|f| f.folder && f.parent.is_none())),
|
||||
"album {id}: {:?}",
|
||||
a.parent
|
||||
);
|
||||
}
|
||||
// moving a folder around terminates either way
|
||||
let _ = c.apply(Op::MoveAlbum { id: AlbumId(10), parent: Some(AlbumId(11)) });
|
||||
let _ = c.apply(Op::MoveAlbum { id: AlbumId(11), parent: Some(AlbumId(10)) });
|
||||
}
|
||||
|
||||
/// Id counters left at the top (or behind the ids in use) by a damaged file: allocating no longer
|
||||
/// overflows (a panic in debug builds) and never hands out an id already in use.
|
||||
#[test]
|
||||
fn id_counters_are_moved_past_the_ids_in_use() {
|
||||
let (c, ids) = sound();
|
||||
let (mut c, report) = load_edited(&c, |v| {
|
||||
v["next_photo"] = 1.into();
|
||||
v["next_album"] = 1.into();
|
||||
v["next_stack"] = 1.into();
|
||||
});
|
||||
assert!(report.repaired.iter().any(|r| r.contains("id counters")), "{:?}", report.repaired);
|
||||
assert!(c.alloc_photo_id().0 > ids[3].0);
|
||||
let (mut c, _) = load_edited(&c, |v| v["next_photo"] = u64::MAX.into());
|
||||
assert_eq!((c.alloc_photo_id().0, c.alloc_photo_id().0), (u64::MAX, u64::MAX), "saturates");
|
||||
let p = Photo::new(PhotoId(u64::MAX), Source::Demo { scene: 1 }, "a", "JPEG", 1, 1, "t");
|
||||
c.apply(Op::AddPhoto { photo: Box::new(p.clone()) }).unwrap();
|
||||
assert!(c.apply(Op::AddPhoto { photo: Box::new(p) }).is_err(), "the id is taken: refused, not overwritten");
|
||||
}
|
||||
|
||||
/// A sound catalog loads unchanged.
|
||||
#[test]
|
||||
fn a_sound_snapshot_needs_no_repair() {
|
||||
let (c, _) = sound();
|
||||
let (loaded, report) = load_edited(&c, |_| {});
|
||||
assert!(report.repaired.is_empty(), "{:?}", report.repaired);
|
||||
assert_eq!(loaded.to_snapshot(), c.to_snapshot());
|
||||
}
|
||||
|
||||
/// Found by fuzzing: an album filed under one id but naming another (here: the folder's) put
|
||||
/// itself inside itself, and listing the folder tree overflowed the stack (an abort, which no
|
||||
/// panic guard catches, every time the library opened).
|
||||
#[test]
|
||||
fn an_album_filed_under_another_id_gets_its_own() {
|
||||
let (c, _) = sound();
|
||||
let folder = c.albums().find(|a| a.folder).unwrap().id;
|
||||
let inner = c.albums().find(|a| !a.folder).unwrap().id;
|
||||
let (c, report) = load_edited(&c, |v| v["albums"][inner.0.to_string()]["id"] = folder.0.into());
|
||||
assert!(report.repaired.iter().any(|r| r.contains(&format!("album {} was filed as {}", folder.0, inner.0))), "{:?}", report.repaired);
|
||||
let a = c.album(inner).unwrap();
|
||||
assert_eq!((a.id, a.parent), (inner, Some(folder)));
|
||||
assert_eq!(c.album_children(Some(folder)).iter().map(|a| a.id).collect::<Vec<_>>(), vec![inner]);
|
||||
}
|
||||
@@ -31,14 +31,24 @@ pub fn photo_summary(p: &Photo) -> Value {
|
||||
})
|
||||
}
|
||||
|
||||
fn album_json(a: &Album, all: &[Album], cat: &lightcraft_catalog::Catalog) -> Value {
|
||||
/// How deep [`album_json`] lists folders inside folders: a catalog that loops (an album inside
|
||||
/// itself, which [`lightcraft_catalog::Catalog::repair`] puts right on load) lists no further
|
||||
/// instead of overflowing the stack.
|
||||
const MAX_ALBUM_DEPTH: usize = 64;
|
||||
|
||||
fn album_json(a: &Album, all: &[Album], cat: &lightcraft_catalog::Catalog, depth: usize) -> Value {
|
||||
let children: Vec<Value> = if depth < MAX_ALBUM_DEPTH {
|
||||
all.iter().filter(|c| c.parent == Some(a.id)).map(|c| album_json(c, all, cat, depth + 1)).collect()
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let mut v = json!({
|
||||
"id": a.id.0,
|
||||
"name": a.name,
|
||||
"folder": a.folder,
|
||||
"count": cat.album_count(a.id),
|
||||
"cover": a.cover.map(|c| c.0),
|
||||
"children": all.iter().filter(|c| c.parent == Some(a.id)).map(|c| album_json(c, all, cat)).collect::<Vec<_>>(),
|
||||
"children": children,
|
||||
});
|
||||
if let Some(rules) = &a.smart {
|
||||
v["smart"] = json!(true);
|
||||
@@ -139,7 +149,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
}),
|
||||
cmd!(query "albums.list", "List Albums", [], None, "{}", always, |s, _| {
|
||||
let all: Vec<Album> = s.catalog.albums().cloned().collect();
|
||||
Ok(Value::Array(all.iter().filter(|a| a.parent.is_none()).map(|a| album_json(a, &all, &s.catalog)).collect()))
|
||||
Ok(Value::Array(all.iter().filter(|a| a.parent.is_none()).map(|a| album_json(a, &all, &s.catalog, 0)).collect()))
|
||||
}),
|
||||
cmd!(query "photo.inspect", "Inspect Photo", [], None, "{id?}", always, |s, p| {
|
||||
let id = photo_arg(s, p, "photo.inspect")?;
|
||||
|
||||
@@ -1178,3 +1178,20 @@ fn hdr_edits_render_with_their_hdr_histogram() {
|
||||
assert!(r.histogram.above_sdr() > 0.0);
|
||||
assert!(r.deep.is_none(), "the preview itself stays the 8-bit SDR rendition");
|
||||
}
|
||||
|
||||
/// Found by fuzzing catalog snapshots: an album filed under one id but naming its own folder's
|
||||
/// listed itself inside itself, and `albums.list` recursed until the stack overflowed (an abort:
|
||||
/// no guard catches it). Loading repairs such a catalog (`Catalog::repair`); one that skipped
|
||||
/// that still lists, the loop cut off.
|
||||
#[test]
|
||||
fn albums_list_survives_a_catalog_that_loops() {
|
||||
let mut s = Session::with_demo();
|
||||
let f = s.execute("album.create", &json!({"name": "F", "folder": true})).unwrap()["id"].as_u64().unwrap();
|
||||
let a = s.execute("album.create", &json!({"name": "A", "parent": f})).unwrap()["id"].as_u64().unwrap();
|
||||
let mut v: serde_json::Value = serde_json::from_str(&s.catalog.to_snapshot()).unwrap();
|
||||
v["albums"][a.to_string()]["id"] = f.into();
|
||||
// as read, without the repair a load does
|
||||
s.catalog = serde_json::from_value(v).unwrap();
|
||||
let r = s.execute("albums.list", &json!({})).unwrap();
|
||||
assert!(r.to_string().contains("\"F\""), "{r}");
|
||||
}
|
||||
@@ -1150,6 +1150,10 @@ type AlbumKids = std::collections::HashMap<Option<AlbumId>, Vec<Album>>;
|
||||
|
||||
/// `open_to`: folders forced open this frame (the way down to an album that was just made).
|
||||
fn albums_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, all: &AlbumKids, parent: Option<AlbumId>, indent: f32, open_to: &[AlbumId]) {
|
||||
// 64 folders deep at most (16 points a level): a catalog that loops must not overflow the stack
|
||||
if indent > 64.0 * 16.0 {
|
||||
return;
|
||||
}
|
||||
let Some(kids) = all.get(&parent) else { return };
|
||||
for a in kids {
|
||||
if a.folder {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Lightroom parity checklist
|
||||
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (LR-BEHAV-AUTOSAVE: damaged snapshots are repaired on open; LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
|
||||
|
||||
The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we
|
||||
have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good
|
||||
@@ -450,7 +450,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
|
||||
| Id | Feature | Tier | Status | Evidence | Missing / notes |
|
||||
|---|---|---|---|---|---|
|
||||
| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved; a write that fails part-way is cut back to the last whole record so the retry never reads back as damage (logs from older builds with such a fragment load in full); quitting saves queued changes in the closing snapshot even when the log can't be appended to; versioned catalog format (`journal::VERSION`): older libraries are upgraded on open, a library from a newer LightCraft is refused untouched ("written by a newer version") |
|
||||
| LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved; a write that fails part-way is cut back to the last whole record so the retry never reads back as damage (logs from older builds with such a fragment load in full); quitting saves queued changes in the closing snapshot even when the log can't be appended to; versioned catalog format (`journal::VERSION`): older libraries are upgraded on open, a library from a newer LightCraft is refused untouched ("written by a newer version"); a damaged or hand-edited snapshot that still reads is made consistent on open (`Catalog::repair`: records filed under another id, stacks of missing, repeated or too few photos, folders inside themselves, id counters behind the ids in use) instead of panicking or hanging later |
|
||||
| LR-BEHAV-UNDO | Global undo | P0 | ✅ | `cmd:edit.undo`, `crates/engine/src/tests.rs` (`rating_flag_undo_redo`), `crates/engine/src/tests_filesystem_history.rs` | covers ratings, albums, deletes, edits; partial file/XMP moves keep unfinished repairs across retries and discarded history; merged renames move directly to their final catalog path |
|
||||
| LR-BEHAV-MULTISELECT | Multi-selection | P0 | ✅ | `cmd:library.select` (`replace`/`add`/`toggle`/`range`), `cmd:library.selectAll` | |
|
||||
| LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | |
|
||||
@@ -806,6 +806,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
|
||||
| 2026-10-12 | minor | LR-IMP-FORMATS: the Hasselblad Lunar (a Sony NEX-7 ARW under Make HASSELBLAD) decodes from sensor data instead of opening as its preview; an uncompressed Leaf MOS imports as a preview-only raw instead of a greyscale image of its mosaic (#1275); status unchanged (🟡) |
|
||||
| 2026-10-12 | minor | LR-HDR-EXPORT, LR-EXP-HDR: HLG AVIF export (`hdrTransfer`); LR-VIEW-HDR-DISPLAY: tested SDR/HDR display decision |
|
||||
| 2026-10-12 | minor | LR-EXP-PSD, MENU-FILE-PHOTOSHOP: Photo ▸ Edit In ▸ Open Original in Other Editor (RAW, no copy) hands the original files (the raws themselves) to the other editor in one start; statuses unchanged |
|
||||
| 2026-10-13 | patch | LR-BEHAV-AUTOSAVE: a damaged snapshot that still parses is repaired on open (ids, stacks, folder loops, id counters) instead of panicking or hanging; status unchanged |
|
||||
| 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references |
|
||||
| 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) |
|
||||
| 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged |
|
||||
|
||||
@@ -2,6 +2,12 @@
|
||||
|
||||
## 2026-10-13
|
||||
|
||||
### Reliability
|
||||
- A library whose catalog file was damaged (or edited by hand) in a way that still reads no longer crashes
|
||||
LightCraft when it shows a stack, or hangs it when a folder is moved: on opening, records are put back under
|
||||
their own ids, stacks keep only photos that exist (at least two), folders inside themselves move to the top,
|
||||
and new photos never reuse an id in use. What was repaired is written to the log.
|
||||
|
||||
### Slider drags on large raws
|
||||
- Dragging a slider no longer re-renders the edited photo's filmstrip thumbnail on every frame; it follows once you
|
||||
let go. On a 64 MP raw the first of those renders decoded the whole file again, which held up the loupe's
|
||||
|
||||
Reference in new issue
Block a user