aleleba/lightcraft
1
0
Fork 0
mirror of https://github.com/storytold/lightcraft.git synced 2026-11-13 09:20:07 -06:00
Code Issues Packages Projects Releases Wiki Activity

Issue #2165: a Lightroom collection that can't nest goes under the nearest set instead of failing the import (#3145)

Browse Source 
* Issue #2165: place a Lightroom collection that can't nest where it is under the nearest set

Lightroom Classic keeps a saved print, book, slideshow or web gallery inside a plain collection, but a
LightCraft album nests only in a folder. The importer passed that collection as the album's parent,
AddAlbum rejected it with "parent must be an existing folder", and the atomic batch rolled back every
album and every catalog-only photo (#2165: photos but no collections; #2532: nothing at all).

An album whose Lightroom parent isn't a collection set (plain, smart, a saved creation, system-only or
missing) now joins the nearest set above it, or the top level, and the import report names it under
warnings. Before each AddAlbum the parent is checked against the catalog's rule (an existing folder,
or one added earlier in the batch), so no album can fail the batch. Builds on #2168 by @endthestart.

Fixes #2165, fixes #2532.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Issue #2165: a Lightroom collection never silently loses its album; warnings name the set as the library does

- Two collections never share an album. An album id the counter hands out that is already taken (the counter
  saturates, so a damaged one repeats) stops the import with an error naming the collection, as AddAlbum's
  "album exists" did before; it no longer drops the second collection and its photos while the report counts it.
  A collection listed twice imports once when both rows are the same and stops the import when they differ. An
  index that names one existing album for two collections gives the second an album of its own.
- A relocation warning names the destination set as this library calls it (a set renamed after an earlier import),
  and a set whose earlier album isn't a folder is described as such, not as "not a collection set".
- The staged-album set is no longer called `added`, which #3142 uses for its photo count in this function.
- Tests: relocated re-import, a user-moved album on re-import, a renamed destination set, a set album that isn't a
  folder, sets nested under a plain collection, an exhausted album counter, duplicate collection rows and an index
  naming one album twice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(lightroom): name the Lightroom collection id in relocation warnings (#2165)

A collection imported somewhere other than where Lightroom keeps it now ends
its warning with "(Lightroom collection N)", so two collections that share a
name can be told apart, as #2165 asked for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(lightroom): a collection listed twice costs only the duplicate (#2165)

A different collection under the id of an earlier one stopped the whole
import. Like any bad Lightroom record it now costs only itself (#1588):
the first row of each id is kept, in catalog order, and a different one
is left out with a warning naming both; the photos listed under that id
go to the first. A row repeated as it is still drops quietly. The
warning reads as the one #3152 gives, so the import behaves the same
whichever of the two lands first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(lightroom): the nearest-set search ends on a parent loop or a deep chain (#2165)

`collection_parent` follows a collection's parents to the nearest set
with a seen-set and a 64-level cap. `validate` refuses loops before it
runs, so no import test reached that guard: a unit test now gives it
A in B in A, a collection in itself, a child of that loop (with and
without a set in it) and a 100-deep chain of plain collections. The
duplicate-collection test also checks that an exact repeat gives no
warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chad Elliott
2026-11-05 20:49:39 +01:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 1ff2211cd4
commit 5981ecadbb
4 changed files with 386 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/lightroom_catalog.rs
+122 -10
View File
@@ -937,6 +937,35 @@ pub(crate) struct ApplyPrepared<'a> {
pub(crate) undo0: usize,
}
/// The catalog's collections with each id once, in catalog order. A row repeated as it is drops quietly; a different
/// collection with an id an earlier one has is left out with a warning naming both, since two albums can't have one id
/// (#1588: one bad Lightroom record costs that record, not the import). The photos listed under the id go to the first.
fn first_of_each_collection<'a>(data: &'a CatalogImport, warnings: &mut Vec<String>) -> Vec<&'a Row> {
let mut first: HashMap<i64, &Row> = HashMap::new();
let mut rows = Vec::new();
for collection in &data.collections {
let id = number(collection, "id_local");
let Some(kept) = first.get(&id) else {
first.insert(id, collection);
rows.push(collection);
continue;
};
if *kept != collection {
let photos = if data.members.iter().any(|m| number(m, "collection") == id) {
format!("; the photos listed under that id go to “{}”", text(kept, "name"))
} else {
String::new()
};
warnings.push(format!(
"collection “{}”: not imported, it has the id of collection “{}”{photos}",
text(collection, "name"),
text(kept, "name")
));
}
}
rows
}
fn stage_collections(
s: &mut crate::Session,
data: &CatalogImport,
@@ -944,24 +973,42 @@ fn stage_collections(
ids: &HashMap<i64, PhotoId>,
ops: &mut Vec<Op>,
warnings: &mut Vec<String>,
) -> usize {
) -> crate::Result<usize> {
let rows = first_of_each_collection(data, warnings);
let mut albums = HashMap::new();
for collection in &data.collections {
// Two collections never share an album. An index that names one album for two gives the second a new one; the
// counter saturates, so a damaged one hands out an id that's taken, and that stops the import instead.
let mut assigned = HashSet::new();
for &collection in &rows {
if number(collection, "systemOnly") != 0 {
continue;
}
let collection_key = format!("{}:{}", key(&data.source), number(collection, "id_local"));
let id = index
let indexed = index
.collections
.get(&collection_key)
.copied()
.map(lightcraft_catalog::AlbumId)
.filter(|id| s.catalog.album(*id).is_some())
.unwrap_or_else(|| s.catalog.alloc_album_id());
.filter(|id| s.catalog.album(*id).is_some() && !assigned.contains(id));
let id = match indexed {
Some(id) => id,
None => {
let id = s.catalog.alloc_album_id();
if s.catalog.album(id).is_some() || assigned.contains(&id) {
return Err(error(format!(
"collection “{}”: the library has no unused album id left for it (its album numbering is exhausted or damaged)",
text(collection, "name")
)));
}
id
}
};
assigned.insert(id);
index.collections.insert(collection_key, id.0);
albums.insert(number(collection, "id_local"), id);
}
let parents: HashMap<_, _> = data.collections.iter().map(|r| (number(r, "id_local"), number(r, "parent"))).collect();
let parents: HashMap<_, _> = rows.iter().map(|r| (number(r, "id_local"), number(r, "parent"))).collect();
let collections: HashMap<_, _> = rows.iter().map(|r| (number(r, "id_local"), *r)).collect();
let mut grouped_members: HashMap<i64, Vec<&Row>> = HashMap::new();
for member in &data.members {
grouped_members.entry(number(member, "collection")).or_default().push(member);
@@ -969,7 +1016,7 @@ fn stage_collections(
for members in grouped_members.values_mut() {
members.sort_by(|a, b| text(a, "positionInCollection").cmp(text(b, "positionInCollection")));
}
let mut ordered: Vec<_> = data.collections.iter().collect();
let mut ordered = rows.clone();
ordered.sort_by_key(|r| {
let mut depth = 0;
let mut at = number(r, "parent");
@@ -979,10 +1026,19 @@ fn stage_collections(
}
depth
});
// Collections whose albums are folders: a collection set, or an earlier import's album when it is a folder.
let sets: HashSet<i64> = albums
.iter()
.filter(|(lr, id)| match s.catalog.album(**id) {
Some(album) => album.folder,
None => collections.get(*lr).is_some_and(|r| text(r, "creationId").contains("group")),
})
.map(|(lr, _)| *lr)
.collect();
let mut added_folders = HashSet::new();
for collection in ordered {
let Some(id) = albums.get(&number(collection, "id_local")).copied() else { continue };
let mut album = Album::new(id, text(collection, "name"));
album.parent = albums.get(&number(collection, "parent")).copied();
album.folder = text(collection, "creationId").contains("group");
if text(collection, "creationId").contains("smart") {
warnings.push(format!("{}: smart rules preserved in archive; imported current membership", album.name));
@@ -1008,10 +1064,49 @@ fn stage_collections(
ops.push(Op::SetAlbumPhotos { id, photos });
}
} else {
let (set, moved) = collection_parent(collection, &collections, &sets);
album.parent = set.and_then(|set| albums.get(&set).copied());
// `AddAlbum` rejects a parent that isn't a folder yet, and that would roll back the whole batch: every
// album and every catalog-only photo. `collection_parent` already prevents it; this keeps a bug there
// (or a cycle `validate` missed) from doing so.
if let Some(parent) = album.parent
&& !added_folders.contains(&parent)
&& !s.catalog.album(parent).is_some_and(|a| a.folder)
{
album.parent = None;
warnings.push(format!(
"{}: its collection set could not be created first; imported at the top level (Lightroom collection {})",
album.name,
number(collection, "id_local")
));
} else if moved {
let lr_parent = number(collection, "parent");
let why = match collections.get(&lr_parent) {
None => "its parent collection isn't in the catalog".to_string(),
Some(r) => match albums.get(&lr_parent).and_then(|id| s.catalog.album(*id)) {
// a set imported earlier whose album is no longer a folder
Some(a) if text(r, "creationId").contains("group") => {
format!("its collection set “{}” is the album “{}” in this library, which isn't a folder", text(r, "name"), a.name)
}
_ => format!("Lightroom keeps it inside “{}”, which isn't a collection set", text(r, "name")),
},
};
// the set's name in this library: the user may have renamed it since an earlier import
let to = match (album.parent.and_then(|id| s.catalog.album(id)), set.and_then(|set| collections.get(&set))) {
(Some(a), _) => format!("into “{}”", a.name),
(None, Some(r)) => format!("into “{}”", text(r, "name")),
(None, None) => "at the top level".to_string(),
};
// the Lightroom id tells apart collections that share a name (#2165)
warnings.push(format!("{}: {why}; imported {to} (Lightroom collection {})", album.name, number(collection, "id_local")));
}
if album.folder {
added_folders.insert(id);
}
ops.push(Op::AddAlbum { album });
}
}
albums.len()
Ok(albums.len())
}
/// Turn the read catalog into library records in one undo step and return the import report
@@ -1240,7 +1335,7 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co
}
by_path.entry(key(&src.path)).or_insert(photo_id);
}
let album_count = stage_collections(s, &data, index, &ids, &mut ops, &mut warnings);
let album_count = stage_collections(s, &data, index, &ids, &mut ops, &mut warnings)?;
if !ops.is_empty() {
s.commit("Import Lightroom Catalog", Op::Batch { ops })?;
}
@@ -1251,5 +1346,22 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co
})
}
/// The collection set a Lightroom collection's album goes into (`None`: the top level), and whether that isn't its
/// Lightroom parent. A LightCraft album nests only in a folder, but Lightroom also keeps a print, book, slideshow or
/// web gallery (or, in an odd catalog, a collection) inside a plain or smart collection, and a parent can be
/// system-only (not imported) or gone from the catalog. Such an album joins the nearest collection set above it.
fn collection_parent(collection: &Row, collections: &HashMap<i64, &Row>, sets: &HashSet<i64>) -> (Option<i64>, bool) {
let mut at = number(collection, "parent");
let mut seen = HashSet::from([number(collection, "id_local")]);
while at != 0 && !sets.contains(&at) {
// `validate` rejects cycles and deep chains; stop at the top level if one got here anyway.
if seen.len() > 64 || !seen.insert(at) {
return (None, true);
}
at = collections.get(&at).map_or(0, |r| number(r, "parent"));
}
((at != 0).then_some(at), at != number(collection, "parent"))
}
#[cfg(test)]
mod tests;
crates/engine/src/lightroom_catalog/tests.rs
+249
View File
@@ -405,6 +405,255 @@ fn a_reimport_counts_each_trashed_match_once() {
assert_eq!(r["imported"], 0, "{r}");
assert!(ids.iter().all(|id| s.catalog.photo(*id).unwrap().deleted));
}
/// The import report's warnings about `album`.
fn warnings_naming(r: &Value, album: &str) -> Vec<String> {
r["warnings"].as_array().unwrap().iter().filter_map(Value::as_str).filter(|w| w.starts_with(&format!("{album}:"))).map(String::from).collect()
}
fn album_named<'a>(s: &'a crate::Session, name: &str) -> &'a lightcraft_catalog::Album {
s.catalog.albums().find(|a| a.name == name).unwrap_or_else(|| panic!("no album {name}"))
}
/// A LightCraft album nests only in a folder. A collection Lightroom keeps inside a plain collection used to fail the
/// whole batch with "parent must be an existing folder", taking every album with it (#2165, #2532).
#[test]
fn a_collection_inside_a_plain_collection_joins_the_set_above_it() {
let mut s = crate::Session::new();
let mut data = sample();
data.collections.push(row(json!({"id_local":11,"parent":10,"name":"Picks","creationId":"com.adobe.ag.library.collection"})));
data.members.push(row(json!({"collection":11,"image":1,"positionInCollection":"a"})));
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["collections"], 3);
assert_eq!(s.catalog.photos().count(), 2);
let travel = album_named(&s, "Travel").id;
assert_eq!(album_named(&s, "Keepers").parent, Some(travel));
assert_eq!(album_named(&s, "Keepers").photos.len(), 2);
assert_eq!(album_named(&s, "Picks").parent, Some(travel));
assert_eq!(album_named(&s, "Picks").photos.len(), 1);
let w = warnings_naming(&r, "Picks");
assert_eq!(w.len(), 1, "{w:?}");
assert!(w[0].contains("Keepers") && w[0].contains("into “Travel”"), "{w:?}");
assert!(warnings_naming(&r, "Keepers").is_empty());
}
/// Lightroom Classic saves a print, book, slideshow or web gallery inside a collection; with no set above it, the
/// creation lands at the top level.
#[test]
fn a_creation_saved_inside_a_top_level_collection_lands_at_the_top_level() {
let mut s = crate::Session::new();
let mut data = sample();
data.collections.push(row(json!({"id_local":12,"name":"Loose","creationId":"com.adobe.ag.library.collection"})));
data.collections.push(row(json!({"id_local":13,"parent":12,"name":"Poster","creationId":"com.adobe.ag.print"})));
data.collections.push(row(json!({"id_local":14,"parent":10,"name":"Album Book","creationId":"com.adobe.ag.layout.book"})));
data.members.push(row(json!({"collection":13,"image":1,"positionInCollection":"a"})));
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["collections"], 5);
let poster = album_named(&s, "Poster");
assert_eq!((poster.parent, poster.folder, poster.photos.len()), (None, false, 1));
assert_eq!(album_named(&s, "Album Book").parent, Some(album_named(&s, "Travel").id));
let w = warnings_naming(&r, "Poster");
assert!(w.len() == 1 && w[0].contains("Loose") && w[0].contains("at the top level"), "{w:?}");
assert_eq!(warnings_naming(&r, "Album Book").len(), 1);
assert_eq!(album_named(&s, "Keepers").photos.len(), 2);
}
/// A parent that's gone from the catalog or system-only (Quick Collection, an unsaved print…) isn't imported; the
/// album lands at the top level, and the report says why it isn't where Lightroom had it.
#[test]
fn a_collection_whose_parent_is_missing_or_system_only_lands_at_the_top_level_with_a_warning() {
let mut s = crate::Session::new();
let mut data = sample();
data.collections.push(row(json!({"id_local":11,"parent":99,"name":"Orphan","creationId":"com.adobe.ag.library.collection"})));
data.collections.push(row(json!({"id_local":5,"name":"quick collection","creationId":"com.adobe.ag.library.collection","systemOnly":1})));
data.collections.push(row(json!({"id_local":12,"parent":5,"name":"Stray","creationId":"com.adobe.ag.print"})));
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["collections"], 4);
assert_eq!(album_named(&s, "Orphan").parent, None);
assert_eq!(album_named(&s, "Stray").parent, None);
assert!(s.catalog.albums().all(|a| a.name != "quick collection"));
let w = warnings_naming(&r, "Orphan");
assert!(w.len() == 1 && w[0].contains("isn't in the catalog"), "{w:?}");
let w = warnings_naming(&r, "Stray");
assert!(w.len() == 1 && w[0].contains("quick collection") && w[0].contains("at the top level"), "{w:?}");
assert_eq!(album_named(&s, "Keepers").parent, Some(album_named(&s, "Travel").id));
}
/// Imports `data` keeping `index` between imports, as a library on disk does.
fn import_indexed(s: &mut crate::Session, data: CatalogImport, index: &mut ImportIndex) -> crate::Result<Value> {
let undo0 = s.undo.len();
let missing_sources = HashSet::new();
let context =
ApplyPrepared { update_existing: false, files: Default::default(), index, archive_path: None, missing_sources: &missing_sources, undo0 };
apply_prepared(s, data, context).map(|applied| applied.report)
}
/// The sample with “Picks”, a collection Lightroom keeps inside the plain collection “Keepers”.
fn with_picks() -> CatalogImport {
let mut data = sample();
data.collections.push(row(json!({"id_local":11,"parent":10,"name":"Picks","creationId":"com.adobe.ag.library.collection"})));
data.members.push(row(json!({"collection":11,"image":1,"positionInCollection":"a"})));
data
}
/// Importing the catalog again finds the moved album where the first import put it, adds nothing twice and doesn't
/// warn again: the album is already in the library.
#[test]
fn a_relocated_collection_keeps_its_place_on_reimport_without_duplicates_or_a_second_warning() {
let mut s = crate::Session::new();
let mut index = ImportIndex::default();
let first = import_indexed(&mut s, with_picks(), &mut index).unwrap();
assert_eq!(warnings_naming(&first, "Picks").len(), 1);
let second = import_indexed(&mut s, with_picks(), &mut index).unwrap();
assert_eq!(second["collections"], 3);
assert_eq!(s.catalog.albums().count(), 3);
assert_eq!(album_named(&s, "Picks").parent, Some(album_named(&s, "Travel").id));
assert_eq!(album_named(&s, "Picks").photos.len(), 1);
assert_eq!(album_named(&s, "Keepers").photos.len(), 2);
assert!(warnings_naming(&second, "Picks").is_empty(), "{second}");
}
/// Albums the user moved after an import stay where the user put them when the catalog is imported again.
#[test]
fn an_album_the_user_moved_keeps_its_place_on_reimport() {
let mut s = crate::Session::new();
let mut index = ImportIndex::default();
import_indexed(&mut s, with_picks(), &mut index).unwrap();
for name in ["Picks", "Keepers"] {
let id = album_named(&s, name).id;
s.commit("Move Album", Op::MoveAlbum { id, parent: None }).unwrap();
}
let r = import_indexed(&mut s, with_picks(), &mut index).unwrap();
assert_eq!(s.catalog.albums().count(), 3);
assert_eq!(album_named(&s, "Picks").parent, None);
assert_eq!(album_named(&s, "Keepers").parent, None);
assert!(warnings_naming(&r, "Picks").is_empty() && warnings_naming(&r, "Keepers").is_empty(), "{r}");
}
/// A collection new since the last import goes into the set's album, and the warning names that album as the
/// library calls it now, not by its Lightroom name.
#[test]
fn a_relocation_warning_names_the_set_as_the_user_renamed_it() {
let mut s = crate::Session::new();
let mut index = ImportIndex::default();
import_indexed(&mut s, sample(), &mut index).unwrap();
let travel = album_named(&s, "Travel").id;
s.commit("Rename Album", Op::RenameAlbum { id: travel, name: "Trips".into() }).unwrap();
let r = import_indexed(&mut s, with_picks(), &mut index).unwrap();
assert_eq!(album_named(&s, "Picks").parent, Some(travel));
let w = warnings_naming(&r, "Picks");
assert!(w.len() == 1 && w[0].contains("inside “Keepers”") && w[0].ends_with("imported into “Trips” (Lightroom collection 11)"), "{w:?}");
}
/// The album an earlier import made for a Lightroom set may no longer be a folder (here: an index that names a plain
/// album). A collection in that set lands at the top level, and the warning says why without calling the set
/// something it isn't.
#[test]
fn a_set_whose_album_is_not_a_folder_puts_its_collections_at_the_top_level() {
let mut s = crate::Session::new();
let plain = s.catalog.alloc_album_id();
s.commit("New Album", Op::AddAlbum { album: Album::new(plain, "Travel picks") }).unwrap();
let mut index = ImportIndex::default();
index.collections.insert(format!("{}:9", key("test.lrcat")), plain.0);
let r = import_indexed(&mut s, sample(), &mut index).unwrap();
assert_eq!(s.catalog.albums().count(), 2);
assert!(!s.catalog.album(plain).unwrap().folder);
let keepers = album_named(&s, "Keepers");
assert_eq!((keepers.parent, keepers.photos.len()), (None, 2));
let w = warnings_naming(&r, "Keepers");
assert_eq!(w.len(), 1, "{w:?}");
assert!(w[0].contains("“Travel” is the album “Travel picks”") && w[0].contains("isn't a folder"), "{w:?}");
assert!(!w[0].contains("isn't a collection set") && w[0].ends_with("at the top level (Lightroom collection 10)"), "{w:?}");
}
/// Only the outermost set inside a plain collection moves; the sets and creations inside it keep their places.
#[test]
fn sets_nested_under_a_plain_collection_keep_their_own_nesting() {
let mut s = crate::Session::new();
let mut data = sample();
data.collections.push(row(json!({"id_local":12,"name":"Loose","creationId":"com.adobe.ag.library.collection"})));
data.collections.push(row(json!({"id_local":13,"parent":12,"name":"Outer","creationId":"com.adobe.ag.library.group"})));
data.collections.push(row(json!({"id_local":14,"parent":13,"name":"Inner","creationId":"com.adobe.ag.library.group"})));
data.collections.push(row(json!({"id_local":15,"parent":14,"name":"Poster","creationId":"com.adobe.ag.print"})));
data.members.push(row(json!({"collection":15,"image":1,"positionInCollection":"a"})));
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["collections"], 6);
let outer = album_named(&s, "Outer");
assert_eq!((outer.parent, outer.folder), (None, true));
let inner = album_named(&s, "Inner");
assert_eq!((inner.parent, inner.folder), (Some(outer.id), true));
assert_eq!(album_named(&s, "Poster").parent, Some(inner.id));
assert_eq!(album_named(&s, "Poster").photos.len(), 1);
let w = warnings_naming(&r, "Outer");
assert!(w.len() == 1 && w[0].contains("inside “Loose”") && w[0].ends_with("at the top level (Lightroom collection 13)"), "{w:?}");
assert!(warnings_naming(&r, "Inner").is_empty() && warnings_naming(&r, "Poster").is_empty(), "{r}");
}
/// The album counter saturates: one at its end hands every collection the same id. The second collection used to be
/// dropped without a word while the report counted it; the import now stops and names it.
#[test]
fn an_exhausted_album_counter_stops_the_import_naming_the_collection() {
let mut s = crate::Session::new();
s.commit("New Album", Op::AddAlbum { album: Album::new(lightcraft_catalog::AlbumId(u64::MAX - 1), "Last") }).unwrap();
let err = apply(&mut s, sample(), false).unwrap_err().to_string();
assert!(err.contains("“Keepers”") && err.contains("no unused album id"), "{err}");
assert_eq!(s.catalog.albums().count(), 1);
assert!(s.catalog.is_empty());
}
/// A collection listed twice imports once. A different collection under the same Lightroom id used to stop the import;
/// like any bad record it now costs only itself (#1588): it is left out with a warning naming both, and its photos go
/// to the first.
#[test]
fn a_collection_listed_twice_imports_once_and_a_conflicting_one_is_skipped_with_a_warning() {
let mut s = crate::Session::new();
let mut data = sample();
data.collections.push(data.collections[1].clone());
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["collections"], 2);
assert_eq!(s.catalog.albums().filter(|a| a.name == "Keepers").count(), 1);
assert_eq!(album_named(&s, "Keepers").photos.len(), 2);
assert_eq!(r["warnings"], json!([]), "an exact repeat drops quietly");
let mut s = crate::Session::new();
let mut data = sample();
// shallower than the first, so a depth-ordered pass would meet it first
data.collections.push(row(json!({"id_local":10,"parent":0,"name":"Rejects","creationId":"collection"})));
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["photos"], 2, "{r}");
assert_eq!(r["collections"], 2, "{r}");
assert!(s.catalog.albums().all(|a| a.name != "Rejects"));
let keepers = album_named(&s, "Keepers");
assert_eq!(keepers.photos.len(), 2);
assert!(keepers.parent.is_some(), "the first row's place is kept");
assert_eq!(
r["warnings"],
json!(["collection “Rejects”: not imported, it has the id of collection “Keepers”; the photos listed under that id go to “Keepers”"])
);
assert_eq!(s.undo.len(), 1);
}
/// The search for the nearest set ends on a parent loop that got past `validate` (A in B in A, a collection in
/// itself) and on a chain of plain collections deeper than 64: the album goes to the top level, flagged as moved.
#[test]
fn the_nearest_set_search_ends_on_a_parent_loop_or_a_deep_chain() {
let plain = |id: i64, parent: i64| row(json!({"id_local":id,"parent":parent,"name":format!("C{id}"),"creationId":"collection"}));
let (a, b, own) = (plain(1, 2), plain(2, 1), plain(3, 3));
let child = plain(4, 1);
let collections: HashMap<i64, &Row> = [(1, &a), (2, &b), (3, &own), (4, &child)].into_iter().collect();
let no_sets = HashSet::new();
for c in [&a, &b, &own, &child] {
assert_eq!(collection_parent(c, &collections, &no_sets), (None, true), "{c:?}");
}
// a set in the loop is still found
assert_eq!(collection_parent(&child, &collections, &HashSet::from([2])), (Some(2), true));
let chain: Vec<Row> = (100..200).map(|id| plain(id, if id == 100 { 0 } else { id - 1 })).collect();
let collections: HashMap<i64, &Row> = chain.iter().map(|r| (number(r, "id_local"), r)).collect();
assert_eq!(collection_parent(&chain[99], &collections, &no_sets), (None, true));
}
/// A damaged index that names one existing album for two collections gives the second an album of its own rather
/// than merging its photos into the first's.
#[test]
fn an_index_naming_one_album_for_two_collections_gives_the_second_its_own() {
let mut s = crate::Session::new();
let old = s.catalog.alloc_album_id();
s.commit("New Album", Op::AddAlbum { album: Album::new(old, "Old") }).unwrap();
let mut index = ImportIndex::default();
for lr in [10, 11] {
index.collections.insert(format!("{}:{lr}", key("test.lrcat")), old.0);
}
let r = import_indexed(&mut s, with_picks(), &mut index).unwrap();
assert_eq!(r["collections"], 3);
assert_eq!(s.catalog.album(old).unwrap().photos.len(), 2, "Keepers merges into the album the index names");
let picks = album_named(&s, "Picks");
assert_ne!(picks.id, old);
assert_eq!(picks.photos.len(), 1);
}
#[test]
fn preserves_existing_edits_and_rejects_invalid_structure_before_mutation() {
let mut s = crate::Session::new();
docs/lightroom-catalog-import.md
+8 -1
View File
@@ -110,7 +110,14 @@ fields are reported. Lightroom's `ProcessVersion` is not mapped: newly imported
LightCraft's latest rendering process, and photos already in the library keep theirs
([process-versions.md](process-versions.md)). When a recovery archive is saved, it retains source settings/history/snapshots
and original smart-collection rules. Smart collections become regular albums with current
membership. Archived history and snapshots are source data, not native LightCraft history yet.
membership, and so do saved prints, books, slideshows and web galleries (with the photos they hold). A LightCraft
album nests only inside a folder (a Lightroom collection set). A collection or creation Lightroom keeps elsewhere,
such as a print saved inside a plain collection, or one whose parent is system-only or gone from the catalog, joins the
nearest collection set above it, or the top level, and the result names it under warnings (issues #2165, #2532).
Quick Collection and unsaved creations are system-only and not imported; publish collections live in tables the
importer doesn't read. A collection the catalog lists twice imports once; a different collection with the id of an
earlier one is left out with a warning naming both, and the photos listed under that id go to the first. Archived
history and snapshots are source data, not native LightCraft history yet.
A Custom white balance on a raw LightCraft develops relative to its as-shot look (ARW, NEF, RW2,
RAF, CR3, CR2, PEF, SRW; Lightroom names them all `RAW`) can't be taken as Kelvin: the catalog keeps no
as-shot white for Custom photos, so the photo stays As Shot and `Temperature, Tint` are reported
docs/whats-new.md
+7
View File
@@ -51,6 +51,13 @@
## 2026-10-30
### Lightroom catalogs with a print inside a collection import their collections
- Lightroom Classic can keep a saved print, book, slideshow or web gallery inside a plain collection, but a
LightCraft album sits only inside a folder. Importing such a catalog stopped at the end with "parent must be an
existing folder" and brought over no collections, and photos whose originals were offline were lost with them
(issues #2165, #2532). Such a collection now joins the nearest collection set above it, or the top level, and the
import result names it under warnings; so does one whose parent is system-only or gone from the catalog.
### Lens distortion correction for the Sony A7 III
- Raws from the Sony A7 III (ILCE-7M3) now get the camera's own distortion correction, read from the file, like
the A7R IVA's: straight lines stay straight at wide angles (the FE 16–35 F4 ZA at 16 mm shifts detail near the