Merge pull request #1480 from StrajnarFilip/claude/fix-1078-linear-repair

M8: repair and library manifests check membership by set lookups (#1078)
This commit is contained in:
BFlat
2026-10-29 09:28:53 +07:00
committed by GitHub
3 changed files with 30 additions and 4 deletions
+20
View File
@@ -85,3 +85,23 @@ fn validate_is_linear_in_a_wide_parent() {
doc.get_mut(page).unwrap().children.retain(|c| *c != kid);
assert!(doc.validate().iter().any(|m| m.contains("not listed in parent")));
}
/// Repair lists every unlisted page once, by set lookups rather than a scan of the page list per
/// page (#1078: a file with many pages made repair quadratic).
#[test]
fn repair_relists_many_unlisted_pages_once_each() {
let mut doc = Document::new("d");
for i in 0..5_000 {
doc.add_page(&format!("P{i}"));
}
let all: Vec<_> = doc.pages.clone();
// Half the pages fall off the list.
doc.pages.retain(|p| p.0 % 2 == 0);
doc.repair();
let mut listed = doc.pages.clone();
listed.sort_unstable();
let mut want = all;
want.sort_unstable();
assert_eq!(listed, want, "every page is listed again, once");
assert!(doc.validate().is_empty());
}
+3 -1
View File
@@ -1143,8 +1143,10 @@ impl Document {
Arc::make_mut(&mut self.libraries).store = store;
}
self.pages.retain(|p| self.nodes.get(p).is_some_and(|n| n.is_page()) && Some(*p) != store);
// Unlisted pages join the list; a set keeps many pages from making this quadratic (#1078).
let mut listed: std::collections::BTreeSet<NodeId> = self.pages.iter().copied().collect();
for (k, n) in self.nodes.iter() {
if n.is_page() && !self.pages.contains(k) && Some(*k) != store {
if n.is_page() && Some(*k) != store && listed.insert(*k) {
self.pages.push(*k);
}
}
+7 -3
View File
@@ -90,9 +90,13 @@ pub(crate) fn read<R: Read + Seek>(
}
// Keep what the snapshot really holds; clip what people see.
manifest.components.retain(|a| doc.node(a.id).is_some_and(|n| matches!(n.kind, NodeKind::Component { .. } | NodeKind::ComponentSet { .. })));
manifest.styles.retain(|s| doc.styles.iter().any(|x| x.id == s.id));
manifest.collections.retain(|c| doc.variables.collections.iter().any(|x| x.id == c.id));
manifest.variables.retain(|v| doc.variables.variables.iter().any(|x| x.id == v.id && x.collection == v.collection));
// Membership by set lookups: a long manifest against a large snapshot stays linear (#1078).
let styles: BTreeSet<&str> = doc.styles.iter().map(|x| x.id.as_str()).collect();
let collections: BTreeSet<&str> = doc.variables.collections.iter().map(|x| x.id.as_str()).collect();
let variables: BTreeSet<(&str, &str)> = doc.variables.variables.iter().map(|x| (x.id.as_str(), x.collection.as_str())).collect();
manifest.styles.retain(|s| styles.contains(s.id.as_str()));
manifest.collections.retain(|c| collections.contains(c.id.as_str()));
manifest.variables.retain(|v| variables.contains(&(v.id.as_str(), v.collection.as_str())));
manifest.name = manifest.name.chars().take(MAX_NAME).collect();
manifest.description = manifest.description.chars().take(MAX_DESCRIPTION).collect();
Ok(Some(PublishedLibrary { manifest, doc: Arc::new(doc) }))