daniel543x/designcraft
1
0 Fork 0
mirror of https://github.com/storytold/designcraft.git synced 2026-11-14 14:22:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

DES-10 review: link status after open, preflight without reading originals

Browse Source 
- Open queues every linked file's lookup (stat, then hash) for the background
  check instead of stating candidates on the UI thread.
- links.list, links.update and relinkFolder use the open's status only while
  its check is still running; afterwards the file is looked at as it is now.
- Preflight tells Modified from size, time and the remembered hash checks
  (the worker keeps them between passes) and the vector test from the mime
  type; it never loads a linked original.
- A modification time of 0 never vouches for a file's bytes.
- Migration keeps an unsettled time as UNVERIFIED_MTIME; recovery snapshots
  keep images not yet saved as links embedded.
- Links panel test fixture has real files, so its Embedded assertions hold.
This commit is contained in:
aiongg committed 2026-11-14 12:46:23 +00:00
1 parent 18e1ba14b3
commit b8be148b4c
10 files changed
+290 -112
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/engine/src/asset_io.rs
+22 -1
View File
@@ -68,6 +68,12 @@ pub fn fingerprint_from(meta: &crate::io::Metadata) -> Fingerprint {
Fingerprint { size: meta.len(), mtime }
}
/// Whether `f` has a modification time to tell the file by: [`fingerprint_from`] gives 0 where
/// the file system reports none, and two such files of one size would look the same.
pub fn has_time(f: &Fingerprint) -> bool {
f.mtime != 0
}
/// The files that may hold `source`'s bytes, in the order they are tried: the absolute path,
/// then the relative one resolved against the document. Paths [`crate::io::document_path`]
/// refuses are left out.
@@ -127,7 +133,7 @@ fn read_candidate(path: &Path, source: &LinkedSource, trusted: Option<Fingerprin
if found.size != source.len {
return Err(Miss::Modified);
}
let known = Some(found) == source.fingerprint || Some(found) == trusted;
let known = has_time(&found) && (Some(found) == source.fingerprint || Some(found) == trusted);
if !known && source.hash.is_none() && source.fingerprint.is_some() {
// Nothing to tell the bytes by.
return Err(Miss::Modified);
@@ -246,6 +252,21 @@ mod tests {
assert_eq!(read_linked(&source, None).ok().map(|l| l.bytes), Some(b"original".to_vec()));
}
/// A time of 0 is what a file system without times gives: it can't tell two files apart.
#[test]
fn a_file_without_a_time_is_told_by_its_hash() {
let dir = Dir::new("no-time");
let path = dir.0.join("a.png");
let at_epoch = |p: &Path| std::fs::OpenOptions::new().write(true).open(p).unwrap().set_modified(std::time::UNIX_EPOCH).unwrap();
std::fs::write(&path, b"original").unwrap();
at_epoch(&path);
let source = source_of(&path);
assert_eq!(source.fingerprint.map(|f| f.mtime), Some(0));
std::fs::write(&path, b"changed!").unwrap();
at_epoch(&path);
assert_eq!(read_linked(&source, None).err(), Some(Unavailable::Modified));
}
/// Through the installed loader and the store: a file changed on disk is never adopted.
#[test]
fn asset_data_loads_through_the_installed_loader() {
crates/engine/src/cmd/links.rs
+17 -4
View File
@@ -314,6 +314,13 @@ fn linked_status(a: &Asset, document: Option<&Path>, checks: &mut ImageChecks) -
}
}
/// Whether a linked image's files are there and none holds its bytes ([`linked_status`]
/// "modified"): Preflight's check, which reads a file only as the Links panel does, once per new
/// size and time (`checks`).
pub(crate) fn link_modified(a: &Asset, document: Option<&Path>, checks: &mut LinkListChecks) -> bool {
a.storage == Storage::Linked && linked_status(a, document, &mut checks.images).0 == "modified"
}
/// Is the linked file still the one the asset's bytes were loaded from, going by the size and
/// time kept then ([`designcraft_doc::Asset::link_fingerprint`], [`loaded_fingerprint`])? Nothing
/// is read when it is, also on the first check after a document is opened, which matters on a
@@ -343,8 +350,10 @@ fn embedded_copy(a: &Asset) -> bool {
/// [`status_cached`], and the file's size and time when a read found the asset's bytes in it
/// although the fingerprint kept with the asset says nothing or something else: what
/// [`remember_link_files`] keeps, so the file isn't read again in another session. What the open
/// of the document found (`open`) answers first, while the asset's bytes are the ones it checked.
/// [`remember_link_files`] keeps, so the file isn't read again in another session. While the
/// background check the open of the document started (`open`) still has the asset's file to look
/// at, it is `checking`; after that the file is looked at as it is now, which may have changed
/// since.
fn image_status(
a: &designcraft_doc::Asset,
document: Option<&Path>,
@@ -355,8 +364,8 @@ fn image_status(
return ("embedded", None);
}
if a.storage == Storage::Linked {
if let Some(status) = open.and_then(|o| o.status(a)) {
return (status.as_str(), None);
if open.is_some_and(|o| o.checking() && o.status(a) == Some(super::native_links::LinkStatus::Checking)) {
return ("checking", None);
}
return linked_status(a, document, checks);
}
@@ -419,9 +428,13 @@ pub(crate) fn checkable_size(len: usize) -> bool {
/// while the file was changed in the last two: another change in that second keeps the time. A
/// time in the future (clock skew, a file copied from another machine) counts as settled, or such
/// a file would be read on every poll; a later save gives it a time unlike the remembered one.
/// A file without a time ([`crate::asset_io::has_time`]) never is.
pub(crate) fn settled(f: &Fingerprint) -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
if !crate::asset_io::has_time(f) {
return false;
}
let now = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| i64::try_from(d.as_secs()).unwrap_or(i64::MAX));
f.mtime > now || now.saturating_sub(f.mtime) >= 2
}
crates/engine/src/cmd/native_links.rs
+103 -73
View File
@@ -42,7 +42,8 @@ pub fn specs() -> Vec<CommandSpec> {
pub enum LinkStatus {
/// A file holds the recorded bytes.
Ok,
/// A file of the recorded size is being hashed.
/// The background check is looking for the file, and hashes one of the recorded size whose
/// time is new.
Checking,
/// Files are there, none with the recorded bytes.
Modified,
@@ -101,11 +102,13 @@ impl OpenLinks {
/// One file check for the background thread.
#[cfg_attr(target_arch = "wasm32", allow(dead_code))]
enum Work {
/// Hash `candidates` (files of the recorded size) until one holds `hash`.
Verify { asset: AssetId, key: SourceKey, hash: String, len: u64, candidates: Vec<PathBuf> },
/// Compare the file at `path` with the embedded `bytes` of a document saved before linked
/// storage.
Migrate { asset: AssetId, key: SourceKey, bytes: Arc<Vec<u8>>, path: PathBuf, page: u32 },
/// Find the file that holds `source`'s bytes: one at its absolute or relative path with the
/// recorded size and time, else one of the recorded size whose content hash is the recorded
/// one.
Find { asset: AssetId, key: SourceKey, source: LinkedSource },
/// Compare the file at `source`'s path (the first of its candidates that has the size of the
/// embedded `bytes`) with the embedded bytes of a document saved before linked storage.
Migrate { asset: AssetId, key: SourceKey, bytes: Arc<Vec<u8>>, source: LinkedSource, page: u32 },
}
/// What a [`Work`] found. `key` is the asset's bytes' key when the check started: a result for
@@ -115,8 +118,10 @@ enum Outcome {
/// The file at `path` holds the bytes; `fingerprint` is its size and time, when they can
/// vouch for the bytes later ([`super::links::settled`]).
Holds { asset: AssetId, key: SourceKey, path: PathBuf, fingerprint: Option<Fingerprint> },
/// No candidate holds the bytes.
/// Files are there; none holds the bytes.
Differs { asset: AssetId, key: SourceKey },
/// No file at either path, or one that may not be followed.
Absent { asset: AssetId, key: SourceKey },
/// The embedded bytes are the file's.
Confirmed {
asset: AssetId,
@@ -170,12 +175,12 @@ pub(crate) struct Opened {
work: Vec<Work>,
}
/// Point each linked asset of `d`, opened from the file `document`, at its file, and find which
/// file holds its bytes without reading any (a background check hashes the ones that need it,
/// [`start`]). A file found at the relative path becomes the link. With `migrate` (a document
/// opened from its file, neither from bytes nor recovered), embedded assets of a document saved
/// before linked storage whose linked file has the embedded bytes' size are queued for a byte
/// comparison.
/// Point each linked asset of `d`, opened from the file `document`, at its file, and queue the
/// look for the file that holds its bytes for the background check ([`start`]): nothing is read
/// or asked for its size here, so a share that doesn't answer can't stall the open. A file found
/// at the relative path becomes the link. With `migrate` (a document opened from its file,
/// neither from bytes nor recovered), embedded assets of a document saved before linked storage
/// are queued for a byte comparison with their linked files.
pub(crate) fn resolve(d: &mut Document, document: Option<&Path>, migrate: bool) -> Opened {
let mut opened = Opened::default();
let ids: Vec<AssetId> = d.assets.keys().copied().collect();
@@ -188,8 +193,15 @@ pub(crate) fn resolve(d: &mut Document, document: Option<&Path>, migrate: bool)
};
let a = Arc::make_mut(asset);
a.data = a.data.with_source(source.clone());
let status = locate(a, &source, document, &mut opened.work);
opened.status.insert(id, (a.data.key(), status));
let key = a.data.key();
// No files on the web: the link record is kept as it is.
let status = if cfg!(target_arch = "wasm32") {
LinkStatus::Missing
} else {
opened.work.push(Work::Find { asset: id, key, source });
LinkStatus::Checking
};
opened.status.insert(id, (key, status));
} else if migrate
&& let Some(document) = document
&& let Some(work) = migration(asset, document)
@@ -200,39 +212,6 @@ pub(crate) fn resolve(d: &mut Document, document: Option<&Path>, migrate: bool)
opened
}
/// [`resolve`] for one linked asset → its status; work for the background check goes to `work`.
fn locate(a: &mut Asset, source: &LinkedSource, document: Option<&Path>, work: &mut Vec<Work>) -> LinkStatus {
#[cfg(not(target_arch = "wasm32"))]
{
let found: Vec<(PathBuf, Fingerprint)> = crate::asset_io::candidates(source)
.into_iter()
.filter_map(|p| {
let meta = crate::io::file_metadata(&p)?;
Some((p, crate::asset_io::fingerprint_from(&meta)))
})
.collect();
if let Some((path, _)) = found.iter().find(|(_, f)| Some(*f) == source.fingerprint) {
adopt_path(a, path, document);
return LinkStatus::Ok;
}
let same_size: Vec<PathBuf> = found.iter().filter(|(_, f)| f.size == source.len).map(|(p, _)| p.clone()).collect();
match &source.hash {
Some(hash) if !same_size.is_empty() => {
work.push(Work::Verify { asset: a.id, key: a.data.key(), hash: hash.clone(), len: source.len, candidates: same_size });
LinkStatus::Checking
}
_ if found.is_empty() => LinkStatus::Missing,
_ => LinkStatus::Modified,
}
}
#[cfg(target_arch = "wasm32")]
{
// No files on the web: the link record is kept as it is.
let _ = (a, source, document, work);
LinkStatus::Missing
}
}
/// `path` (a file found to hold `a`'s bytes) as `a`'s link, with its path relative to the
/// document and its bytes read from there.
fn adopt_path(a: &mut Asset, path: &Path, document: Option<&Path>) {
@@ -267,8 +246,7 @@ fn legacy(a: &Asset) -> bool {
a.storage == Storage::Embedded && a.link.is_some() && a.link_hash.is_none()
}
/// The byte comparison that would let a [`legacy`] asset be saved as a link: its linked file
/// exists with the size of the embedded bytes.
/// The byte comparison that would let a [`legacy`] asset be saved as a link.
fn migration(a: &Asset, document: &Path) -> Option<Work> {
#[cfg(not(target_arch = "wasm32"))]
{
@@ -287,8 +265,7 @@ fn migration(a: &Asset, document: &Path) -> Option<Work> {
fingerprint: None,
hash: None,
};
let path = crate::asset_io::candidates(&source).into_iter().find(|p| crate::io::file_metadata(p).is_some_and(|m| m.len() == source.len))?;
Some(Work::Migrate { asset: a.id, key: a.data.key(), bytes, path, page: a.page })
Some(Work::Migrate { asset: a.id, key: a.data.key(), bytes, source, page: a.page })
}
#[cfg(target_arch = "wasm32")]
{
@@ -356,21 +333,34 @@ const CHUNK: usize = 1 << 20;
#[cfg(not(target_arch = "wasm32"))]
fn check(work: Work, cancel: &AtomicBool) -> Option<Outcome> {
match work {
Work::Verify { asset, key, hash, len, candidates } => {
for path in candidates {
let Some(before) = fingerprint(&path) else { continue };
match read_file(&path, len, cancel, |_, _| true)? {
Some(found) if found == hash && fingerprint(&path) == Some(before) => {
let fingerprint = super::links::settled(&before).then_some(before);
return Some(Outcome::Holds { asset, key, path, fingerprint });
Work::Find { asset, key, source } => {
let found: Vec<(PathBuf, Fingerprint)> =
crate::asset_io::candidates(&source).into_iter().filter_map(|p| fingerprint(&p).map(|f| (p, f))).collect();
if let Some((path, _)) = found.iter().find(|(_, f)| super::links::settled(f) && Some(*f) == source.fingerprint) {
return Some(Outcome::Holds { asset, key, path: path.clone(), fingerprint: None });
}
if found.is_empty() {
return Some(Outcome::Absent { asset, key });
}
let Some(hash) = &source.hash else { return Some(Outcome::Differs { asset, key }) };
for (path, before) in found.iter().filter(|(_, f)| f.size == source.len) {
match read_file(path, source.len, cancel, |_, _| true)? {
Some(found) if found == *hash && fingerprint(path) == Some(*before) => {
let fingerprint = super::links::settled(before).then_some(*before);
return Some(Outcome::Holds { asset, key, path: path.clone(), fingerprint });
}
_ => {}
}
}
Some(Outcome::Differs { asset, key })
}
Work::Migrate { asset, key, bytes, path, page } => {
let Some(before) = fingerprint(&path) else { return Some(Outcome::Unconfirmed) };
Work::Migrate { asset, key, bytes, source, page } => {
let len = bytes.len() as u64;
let Some((path, before)) =
crate::asset_io::candidates(&source).into_iter().find_map(|p| fingerprint(&p).filter(|f| f.size == len).map(|f| (p, f)))
else {
return Some(Outcome::Unconfirmed);
};
let same = |at: usize, chunk: &[u8]| at.checked_add(chunk.len()).and_then(|end| bytes.get(at..end)) == Some(chunk);
let read = read_file(&path, bytes.len() as u64, cancel, same)?;
// Changed while it was read: not the bytes it had.
@@ -379,7 +369,10 @@ fn check(work: Work, cancel: &AtomicBool) -> Option<Outcome> {
}
let hash = crate::asset_io::content_hash(&bytes);
let proxy = designcraft_render::proxy::make_proxy(&bytes, page);
Some(Outcome::Confirmed { asset, key, path, fingerprint: before, hash, proxy })
// A time that can't vouch for the bytes (changed in the last seconds, or none) is
// kept as one no file has: saves then go by the hash, not by size and time.
let fingerprint = if super::links::settled(&before) { before } else { Fingerprint { size: len, mtime: super::links::UNVERIFIED_MTIME } };
Some(Outcome::Confirmed { asset, key, path, fingerprint, hash, proxy })
}
}
}
@@ -488,6 +481,13 @@ fn current(st: &mut DocState, id: AssetId, key: SourceKey) -> Option<&mut Asset>
fn apply_one(st: &mut DocState, outcome: Outcome, document: Option<&Path>) -> bool {
match outcome {
Outcome::Holds { asset, key, path, fingerprint } => {
let unchanged = fingerprint.is_none()
&& st.doc.assets.get(&asset).is_some_and(|a| a.data.key() == key && a.link.as_deref() == Some(path.to_string_lossy().as_ref()));
if unchanged {
// The file at the recorded path, with the recorded size and time: nothing to change.
st.links.status.insert(asset, (key, LinkStatus::Ok));
return false;
}
let Some(a) = current(st, asset, key) else { return false };
if let Some(f) = fingerprint {
a.link_fingerprint = Some(f);
@@ -501,9 +501,11 @@ fn apply_one(st: &mut DocState, outcome: Outcome, document: Option<&Path>) -> bo
true
}
Outcome::Differs { asset, key } => {
if st.doc.assets.get(&asset).is_some_and(|a| a.data.key() == key) {
st.links.status.insert(asset, (key, LinkStatus::Modified));
}
found(st, asset, key, LinkStatus::Modified);
false
}
Outcome::Absent { asset, key } => {
found(st, asset, key, LinkStatus::Missing);
false
}
Outcome::Confirmed { asset, key, path, fingerprint, hash, proxy } => {
@@ -527,6 +529,13 @@ fn apply_one(st: &mut DocState, outcome: Outcome, document: Option<&Path>) -> bo
}
}
/// Keep `status` for asset `id`, while its bytes are still `key`'s.
fn found(st: &mut DocState, id: AssetId, key: SourceKey, status: LinkStatus) {
if st.doc.assets.get(&id).is_some_and(|a| a.data.key() == key) {
st.links.status.insert(id, (key, status));
}
}
/// The status line's message once `n` embedded images were found identical to their files.
fn notice(n: usize) -> String {
if n == 1 {
@@ -543,22 +552,33 @@ fn notice(n: usize) -> String {
/// linked storage, [`resolve`]) whose file has changed since the check is embedded in this save
/// after all.
pub(crate) fn for_destination(d: &Document, from: Option<&Path>, destination: &Path) -> Option<Document> {
let mut changes: Vec<(AssetId, Option<String>, Option<String>, Storage)> = Vec::new();
rewrite(d, from, destination, false)
}
/// [`for_destination`] for a recovery snapshot of `d`, whose file is `document`. Embedded images
/// to be saved as links stay as the document's file has them (embedded, to be compared with their
/// files again), so a recovered document never depends on a file the user hasn't saved a link to.
pub(crate) fn for_recovery(d: &Document, document: &Path) -> Option<Document> {
rewrite(d, Some(document), document, true)
}
fn rewrite(d: &Document, from: Option<&Path>, destination: &Path, recovery: bool) -> Option<Document> {
let mut changes: Vec<(AssetId, Option<String>, Option<String>, bool)> = Vec::new();
for a in d.assets.values() {
let Some(link) = a.link.as_deref() else { continue };
let absolute = crate::io::document_path(link, from).ok();
let new_link = absolute.as_ref().map(|p| p.to_string_lossy().into_owned()).filter(|p| p != link);
let relative = absolute.as_deref().and_then(|p| relative_to(destination, p)).filter(|r| a.link_relative.as_ref() != Some(r));
let storage = if a.storage == Storage::Linked && !a.data.is_linked() && !still_there(a, from) { Storage::Embedded } else { a.storage };
if new_link.is_some() || relative.is_some() || storage != a.storage {
changes.push((a.id, new_link, relative, storage));
let embed = a.storage == Storage::Linked && !a.data.is_linked() && (recovery || !still_there(a, from));
if new_link.is_some() || relative.is_some() || embed {
changes.push((a.id, new_link, relative, embed));
}
}
if changes.is_empty() {
return None;
}
let mut saved = d.clone();
for (id, link, relative, storage) in changes {
for (id, link, relative, embed) in changes {
let Some(a) = saved.assets.get_mut(&id).map(Arc::make_mut) else { continue };
if let Some(link) = link {
a.link = Some(link);
@@ -566,17 +586,27 @@ pub(crate) fn for_destination(d: &Document, from: Option<&Path>, destination: &P
if let Some(relative) = relative {
a.link_relative = Some(relative);
}
a.storage = storage;
if embed {
a.storage = Storage::Embedded;
if recovery {
// As the document's file has it: the next open from that file compares it again.
a.link_hash = None;
a.proxy = None;
}
}
}
Some(saved)
}
/// Whether a file still has the size and time `a`'s link recorded when its bytes were found in it.
/// Whether a file still has the size and time `a`'s link recorded when its bytes were found in it
/// (a time that can vouch for them, [`super::links::settled`]).
fn still_there(a: &Asset, document: Option<&Path>) -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
let Some(source) = LinkedSource::of(a, document) else { return false };
crate::asset_io::candidates(&source).iter().any(|p| fingerprint(p) == source.fingerprint)
crate::asset_io::candidates(&source)
.iter()
.any(|p| fingerprint(p).is_some_and(|f| super::links::settled(&f) && Some(f) == source.fingerprint))
}
#[cfg(target_arch = "wasm32")]
{
crates/engine/src/cmd/native_links/tests.rs
+97 -5
View File
@@ -67,12 +67,20 @@ fn saved_linked(dir: &Dir) -> (PathBuf, AssetId) {
(doc, id)
}
fn open(path: &Path) -> Session {
/// Open the file at `path`, with its background check of linked files still to come.
fn opened(path: &Path) -> Session {
let mut s = Session::new();
s.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap();
s
}
/// Open the file at `path` and wait for its background check of linked files.
fn open(path: &Path) -> Session {
let mut s = opened(path);
wait(&mut s);
s
}
fn asset(s: &Session, id: AssetId) -> Arc<Asset> {
Arc::clone(&s.doc().unwrap().doc.assets[&id])
}
@@ -109,7 +117,7 @@ fn a_linked_save_writes_no_original_and_records_the_relative_path() {
let s = open(&doc);
let a = asset(&s, id);
assert_eq!((a.storage, a.link_relative.as_deref()), (Storage::Linked, Some("links/art.png")));
assert!(a.data.is_linked() && a.data.resident().is_none(), "nothing is read on open");
assert!(a.data.is_linked() && a.data.resident().is_none(), "nothing is read on open, nor by its check");
assert_eq!(status(&s, id), Some(LinkStatus::Ok), "the recorded size and time: no check needed");
assert!(!s.doc().unwrap().links.checking());
assert_eq!(a.data.load().unwrap().as_slice(), png(30));
@@ -146,7 +154,7 @@ fn the_absolute_file_wins_and_otherwise_the_one_with_the_bytes() {
// The original changes (same size): the copy holds the bytes, found by its hash.
flip_a_byte(&dir.0.join("links/art.png"));
age(&dir.0.join("links/art.png"), 10);
let mut s = open(&copy.0.join("doc.designcraft"));
let mut s = opened(&copy.0.join("doc.designcraft"));
assert_eq!(status(&s, id), Some(LinkStatus::Checking));
wait(&mut s);
let a = asset(&s, id);
@@ -162,7 +170,7 @@ fn a_touched_file_with_the_same_bytes_is_ok_and_keeps_its_new_time() {
let (doc, id) = saved_linked(&dir);
let image = dir.0.join("links/art.png");
age(&image, 1000);
let mut s = open(&doc);
let mut s = opened(&doc);
let key = asset(&s, id).data.key();
assert_eq!(status(&s, id), Some(LinkStatus::Checking));
wait(&mut s);
@@ -201,6 +209,7 @@ fn migration_links_confirmed_images_and_drops_the_copy_only_after_confirmation()
let (same, changed) = (dir.0.join("links/same.png"), dir.0.join("links/changed.png"));
std::fs::write(&same, png(30)).unwrap();
std::fs::write(&changed, png(31)).unwrap();
age(&same, 100);
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
let (a, b) = (place(&mut s, &png(30)), place(&mut s, &png(31)));
@@ -212,7 +221,7 @@ fn migration_links_confirmed_images_and_drops_the_copy_only_after_confirmation()
// One file changes since (same size, other bytes).
flip_a_byte(&changed);
let mut s = open(&doc);
let mut s = opened(&doc);
assert!(asset(&s, a).data.embedded_bytes().is_some() && asset(&s, a).storage == Storage::Embedded, "nothing changes before the comparison");
wait(&mut s);
let (left, kept) = (asset(&s, a), asset(&s, b));
@@ -264,6 +273,7 @@ fn byte_opened_and_recovered_documents_are_not_migrated_and_recovery_holds_no_or
drop(s);
let mut r = Session::new();
r.execute("file.recovery.open", &json!({"dir": recovery.to_string_lossy()})).unwrap();
wait(&mut r);
let a = asset(&r, id);
assert_eq!((a.storage, a.link_relative.as_deref()), (Storage::Linked, Some("links/art.png")));
assert_eq!(a.data.linked_source().and_then(|s| s.document.clone()), Some(doc.clone()), "resolved against the document's file");
@@ -293,3 +303,85 @@ fn save_a_copy_with_embedded_links_is_portable() {
assert!(stored_assets(&copy).is_empty());
assert!(s.execute("file.saveACopy", &json!({"path": copy.to_string_lossy(), "embedLinks": 1})).is_err());
}
#[test]
fn the_open_looks_for_files_in_the_background() {
let dir = Dir::new("background");
let (doc, id) = saved_linked(&dir);
let mut s = opened(&doc);
assert_eq!(status(&s, id), Some(LinkStatus::Checking), "nothing is asked of the file system on open");
assert!(s.doc().unwrap().links.checking());
wait(&mut s);
assert_eq!(status(&s, id), Some(LinkStatus::Ok));
assert!(!s.doc().unwrap().is_dirty());
}
/// The open's statuses don't outlive a change to the file: Update reads a file edited since,
/// and a file missing at the open counts once it is back.
#[test]
fn a_file_changed_or_restored_after_the_open_is_seen() {
let dir = Dir::new("after-open");
let (doc, id) = saved_linked(&dir);
let image = dir.0.join("links/art.png");
let mut s = open(&doc);
assert_eq!(status(&s, id), Some(LinkStatus::Ok));
std::fs::write(&image, png(77)).unwrap();
age(&image, 10);
let r = s.execute("links.update", &json!({"asset": id.0})).unwrap();
assert_eq!(r["updated"], 1, "{r}");
assert_eq!(asset(&s, id).data.load().unwrap().as_slice(), png(77));
let restored = Dir::new("restored");
let (doc, id) = saved_linked(&restored);
let image = restored.0.join("links/art.png");
let original = std::fs::read(&image).unwrap();
std::fs::remove_file(&image).unwrap();
let mut s = open(&doc);
assert_eq!(status(&s, id), Some(LinkStatus::Missing));
std::fs::write(&image, &original).unwrap();
age(&image, 10);
let rows = s.execute("links.list", &json!({})).unwrap();
let row = rows.as_array().unwrap().iter().find(|r| r["asset"] == id.0).cloned().unwrap();
assert_eq!(row["status"], "ok", "{row}");
}
#[test]
fn preflight_reads_no_linked_original() {
let dir = Dir::new("preflight");
let (doc, id) = saved_linked(&dir);
let mut s = open(&doc);
let issues = s.execute("preflight.run", &json!({})).unwrap();
assert!(!issues.to_string().contains("modifiedLink"), "{issues}");
assert!(asset(&s, id).data.resident().is_none(), "the original isn't read");
flip_a_byte(&dir.0.join("links/art.png"));
age(&dir.0.join("links/art.png"), 10);
let issues = s.execute("preflight.run", &json!({})).unwrap();
assert!(issues.to_string().contains("modifiedLink"), "{issues}");
}
#[test]
fn recovery_keeps_images_not_yet_saved_as_links() {
let dir = Dir::new("recover-migrated");
let image = dir.0.join("links/art.png");
std::fs::write(&image, png(30)).unwrap();
age(&image, 100);
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
let id = place(&mut s, &png(30));
link_to(&mut s, id, &image, false);
let doc = dir.0.join("doc.designcraft");
s.execute("file.save", &json!({"path": doc.to_string_lossy()})).unwrap();
let mut s = open(&doc);
assert_eq!(asset(&s, id).storage, Storage::Linked, "found identical to its file, to be saved as a link");
s.execute("frame.create", &json!({"rect": [36, 36, 300, 200]})).unwrap();
let recovery = dir.0.join("recovery");
s.execute("file.recovery.save", &json!({"dir": recovery.to_string_lossy()})).unwrap();
let entry = recovery.join(format!("{}.designcraft", s.doc().unwrap().uid));
assert_eq!(stored_assets(&entry), [format!("assets/{}.png", id.0)], "the bytes stay in the snapshot");
drop(s);
let mut r = Session::new();
r.execute("file.recovery.open", &json!({"dir": recovery.to_string_lossy()})).unwrap();
wait(&mut r);
let a = asset(&r, id);
assert_eq!((a.storage, a.data.embedded_bytes().map(|b| b.to_vec())), (Storage::Embedded, Some(png(30))));
}
crates/engine/src/cmd/preflight.rs
+20 -12
View File
@@ -9,7 +9,7 @@ use designcraft_geom::Affine;
use serde::Serialize;
use serde_json::{Value, json};
use super::{CommandSpec, cmd, has_doc};
use super::{CommandSpec, LinkListChecks, cmd, has_doc};
use crate::{Result, Session};
mod white_overprint;
@@ -260,14 +260,23 @@ pub fn overset_warnings(s: &Session, pages: Option<&[usize]>) -> Vec<String> {
out
}
pub fn check(s: &Session, min_ppi: f64) -> Vec<Issue> {
let Some(st) = s.active() else { return vec![] };
check_document(&st.doc, &s.cache, st.path.as_deref().map(std::path::Path::new), min_ppi)
/// The active document's issues. Linked files are looked at as `links.list` looks at them, with
/// what it remembers of them.
pub fn check(s: &mut Session, min_ppi: f64) -> Vec<Issue> {
let mut links = std::mem::take(&mut s.link_checks);
let issues = match s.active() {
Some(st) => check_document(&st.doc, &s.cache, st.path.as_deref().map(std::path::Path::new), min_ppi, &mut links),
None => vec![],
};
s.link_checks = links;
issues
}
/// [`check`] of document `d` saved at `document` (relative links resolve against it), with the
/// compositions in `cache`. It needs no session, so it can run on another thread.
pub fn check_document(d: &Document, cache: &Cache, document: Option<&std::path::Path>, min_ppi: f64) -> Vec<Issue> {
/// compositions in `cache`. It needs no session, so it can run on another thread. `links` is what
/// the caller remembers of linked files between checks: a file is read only when its size is the
/// recorded one and its time is new.
pub fn check_document(d: &Document, cache: &Cache, document: Option<&std::path::Path>, min_ppi: f64, links: &mut LinkListChecks) -> Vec<Issue> {
let mut out = Vec::new();
// Stories: overset and fonts.
let mut missing_fonts: Vec<String> = Vec::new();
@@ -437,8 +446,7 @@ pub fn check_document(d: &Document, cache: &Cache, document: Option<&std::path::
if let Content::Graphic(g) = &it.content
&& let Some(a) = d.assets.get(&g.asset)
&& a.data.is_linked()
&& !super::links::link_missing(a, document)
&& a.data.load().err() == Some(designcraft_doc::Unavailable::Modified)
&& super::links::link_modified(a, document, links)
{
out.push(Issue {
severity: "warning",
@@ -463,7 +471,7 @@ pub fn check_document(d: &Document, cache: &Cache, document: Option<&std::path::
}),
Some(a) => {
// Placed PDF and SVG pages export as vectors; `pixels` is only the preview's.
let vector = a.output_bytes().is_ok_and(|o| designcraft_images::is_pdf(&o.bytes) || designcraft_images::is_svg(&o.bytes));
let vector = a.mime == "application/pdf" || a.mime == "image/svg+xml";
if let Some((pw, ph)) = a.pixels.filter(|_| !vector) {
// Measure each image axis in points, not its rotated bounding box or
// cropped frame. Either axis can fall below the minimum resolution.
@@ -589,10 +597,10 @@ mod arabic_tests {
let r = s.execute("frame.create", &json!({"rect": [0, 0, 300, 200], "content": "text", "text": text})).unwrap();
s.execute("text.select", &json!({"story": r["story"], "anchor": 0, "focus": text.len()})).unwrap();
s.execute("type.char", &json!({"attrs": {"diacriticPosition": "openType", "diacriticXOffset": 150}})).unwrap();
assert!(!check(&s, 150.0).iter().any(|i| i.kind == "unsupportedTypography"));
assert!(!check(&mut s, 150.0).iter().any(|i| i.kind == "unsupportedTypography"));
s.execute("type.para", &json!({"attrs": {"arabicJustification": "NaskhJustification", "paragraphKashidaWidth": 2}})).unwrap();
s.execute("type.char", &json!({"attrs": {"diacriticPosition": "tight"}})).unwrap();
let issues = check(&s, 150.0);
let issues = check(&mut s, 150.0);
let typography: Vec<_> = issues.iter().filter(|i| i.kind == "unsupportedTypography").collect();
assert_eq!(typography.len(), 3, "{typography:?}");
assert!(typography.iter().all(|i| i.severity == "warning"));
@@ -683,7 +691,7 @@ mod mojikumi_nested_tests {
pf.para.mojikumi = Some("MojikumiTable/Unrecognized".into());
cell.insert_note(0, "Note", pf);
d.story_mut(sid).unwrap().insert_table(0, table);
let issues = check(&s, 150.0);
let issues = check(&mut s, 150.0);
assert_eq!(issues.iter().filter(|i| i.kind == "unsupportedTypography" && i.message.contains("Unrecognized")).count(), 1);
}
}
crates/engine/src/recovery.rs
+2 -1
View File
@@ -751,8 +751,9 @@ impl Snapshot {
let doc = d.preview_stash.clone().unwrap_or_else(|| d.doc.clone());
// Linked files are recorded as a save to the document's own file records them, never
// copied in; relative paths are relative to that file, which recovery resolves them against.
// Embedded images not yet saved as links keep their bytes.
let doc = match d.path.as_deref().map(Path::new) {
Some(path) => crate::cmd::native_links::for_destination(&doc, Some(path), path).map_or(doc, std::sync::Arc::new),
Some(path) => crate::cmd::native_links::for_recovery(&doc, path).map_or(doc, std::sync::Arc::new),
None => doc,
};
Snapshot {
crates/ui-egui/src/panels/properties/tests.rs
+15 -6
View File
@@ -20,6 +20,9 @@ fn language(app: &mut DesignApp) -> Value {
text_attrs(app).unwrap()["chars"]["language"].clone()
}
/// The graphic [`linked_graphics`] places.
const SVG: &[u8] = br##"<svg xmlns="http://www.w3.org/2000/svg" width="32" height="32"><rect width="32" height="32" fill="#4488cc"/></svg>"##;
fn linked_graphics(paths: &[Option<&str>]) -> (DesignApp, Vec<designcraft_doc::ItemId>, Vec<designcraft_doc::AssetId>) {
use designcraft_doc::{Asset, AssetId, Content, Graphic, ItemId};
use std::sync::Arc;
@@ -39,10 +42,7 @@ fn linked_graphics(paths: &[Option<&str>]) -> (DesignApp, Vec<designcraft_doc::I
name: "photo.svg".into(),
mime: "image/svg+xml".into(),
link: path.map(str::to_owned),
data: designcraft_doc::AssetData::embedded(
br##"<svg xmlns="http://www.w3.org/2000/svg" width="32" height="32"><rect width="32" height="32" fill="#4488cc"/></svg>"##
.to_vec(),
),
data: designcraft_doc::AssetData::embedded(SVG.to_vec()),
pixels: Some((32, 32)),
..Default::default()
}),
@@ -167,8 +167,14 @@ fn edit_page_shows_the_page_and_sets_its_size_alone() {
#[test]
fn links_panel_distinguishes_same_named_sources_and_keeps_link_actions() {
let first = "/project/assets/studio-a/photo.svg";
let second = "/project/assets/studio-b/photo.svg";
// Files holding the graphics' bytes, so their links are OK and can be embedded.
let dir = std::env::temp_dir().join(format!("dc-links-panel-same-name-{}", std::process::id()));
let (first, second) = (dir.join("studio-a/photo.svg"), dir.join("studio-b/photo.svg"));
for file in [&first, &second] {
std::fs::create_dir_all(file.parent().unwrap()).unwrap();
std::fs::write(file, SVG).unwrap();
}
let (first, second) = (first.to_str().unwrap(), second.to_str().unwrap());
let (app, items, assets) = linked_graphics(&[Some(first), Some(second), None]);
let mut h = links_harness(app, 320.0);
assert_eq!(h.get_all_by_label("photo.svg").count(), 3);
@@ -184,12 +190,15 @@ fn links_panel_distinguishes_same_named_sources_and_keeps_link_actions() {
// Embedding keeps the link for Unembed.
let doc = &h.state().session.active().unwrap().doc;
assert_eq!(doc.assets[&assets[1]].link.as_deref(), Some(second));
assert_eq!(doc.assets[&assets[0]].link.as_deref(), Some(first));
assert_eq!(h.get_all_by_label("Embedded").count(), 2);
assert_eq!(h.get_all_by_label("photo.svg").count(), 3);
h.get_by_label(first).click();
h.run_steps(3);
h.get_by_label("Go To").click();
h.run_steps(3);
assert_eq!(h.state().session.active().unwrap().selection.items, vec![items[0]]);
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
crates/ui-egui/src/preflight_worker.rs
+8 -4
View File
@@ -13,6 +13,7 @@ use std::time::{Duration, Instant};
use designcraft_compose::Cache;
use designcraft_doc::Document;
use designcraft_engine::cmd::LinkListChecks;
use designcraft_engine::cmd::preflight::{Issue, check_document};
use crate::DesignApp;
@@ -177,7 +178,7 @@ fn issues_at(app: &mut DesignApp, ctx: &egui::Context, now: f64) -> Option<Arc<V
return state.found.iter().find(|f| f.of.uid == of.uid).map(|f| Arc::clone(&f.issues));
}
let _ = (ctx, now);
let issues = Arc::new(check_document(&doc, &cache, of.path.as_deref().map(std::path::Path::new), MIN_PPI));
let issues = Arc::new(check_document(&doc, &cache, of.path.as_deref().map(std::path::Path::new), MIN_PPI, &mut LinkListChecks::default()));
#[cfg(test)]
{
state.inline_checks += 1;
@@ -373,16 +374,19 @@ fn spawn(ctx: egui::Context, live: &Arc<std::sync::atomic::AtomicUsize>) -> Opti
.spawn(move || {
let _alive = alive;
let mut own = None;
// The linked files read and what they held, so a file is read once per new time.
let mut links = LinkListChecks::default();
loop {
let job = match jobs.recv_timeout(FORGET_AFTER) {
Ok(job) => job,
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
own = None;
links = LinkListChecks::default();
continue;
}
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => break,
};
if done_tx.send(check(&mut own, job)).is_err() {
if done_tx.send(check(&mut own, &mut links, job)).is_err() {
break;
}
ctx.request_repaint();
@@ -394,9 +398,9 @@ fn spawn(ctx: egui::Context, live: &Arc<std::sync::atomic::AtomicUsize>) -> Opti
/// Checks `job` with the worker's own compositions, `own`.
#[cfg(not(target_arch = "wasm32"))]
fn check(own: &mut Option<(u64, Cache)>, job: Job) -> Found {
fn check(own: &mut Option<(u64, Cache)>, links: &mut LinkListChecks, job: Job) -> Found {
let started = Instant::now();
let issues = check_document(&job.doc, reader(own, &job), job.of.path.as_deref().map(std::path::Path::new), MIN_PPI);
let issues = check_document(&job.doc, reader(own, &job), job.of.path.as_deref().map(std::path::Path::new), MIN_PPI, links);
// The checked state isn't kept alive by its check once the result is sent.
drop(job.doc);
Found { of: job.of, issues: Arc::new(issues), took: started.elapsed(), asked_at: 0.0 }
crates/ui-egui/src/preflight_worker/tests.rs
+3 -3
View File
@@ -235,17 +235,17 @@ fn a_check_of_an_older_state_keeps_the_current_compositions() {
let cache = Arc::clone(&app.session.cache);
let current = cache.get(&doc, sid, None);
let mut own = None;
let found = super::check(&mut own, super::Job { of: older, doc: Arc::clone(&older_doc), cache: Arc::clone(&cache) });
let found = super::check(&mut own, &mut Default::default(), super::Job { of: older, doc: Arc::clone(&older_doc), cache: Arc::clone(&cache) });
assert_eq!(found.issues.iter().filter(|i| i.severity == "error").count(), 1);
assert!(Arc::ptr_eq(&cache.get(&doc, sid, None), &current), "the current state's composition is kept");
// The worker keeps its compositions for the next check of the document over the same cache.
let earlier = own.as_ref().unwrap().1.get(&older_doc, sid, None);
let (of, doc) = Checked::new(&app).unwrap();
super::check(&mut own, super::Job { of, doc, cache: Arc::clone(&cache) });
super::check(&mut own, &mut Default::default(), super::Job { of, doc, cache: Arc::clone(&cache) });
assert!(Arc::ptr_eq(&own.as_ref().unwrap().1.get(&older_doc, sid, None), &earlier), "kept for the next check");
let reloaded = Arc::new(designcraft_compose::Cache::new());
let (of, doc) = Checked::new(&app).unwrap();
super::check(&mut own, super::Job { of, doc, cache: Arc::clone(&reloaded) });
super::check(&mut own, &mut Default::default(), super::Job { of, doc, cache: Arc::clone(&reloaded) });
assert!(own.as_ref().unwrap().1.base().is_some_and(|b| Arc::ptr_eq(b, &reloaded)), "a new cache after a font reload");
}
docs/linked-artwork.md
+3 -3
View File
@@ -27,11 +27,11 @@ The proxy is drawn while the original loads, and in place of the original when t
| Status | Meaning |
|---|---|
| OK | A file at the link's path holds the recorded bytes. |
| Checking | A file of the recorded size is being hashed in the background. |
| Checking | Just after the document opens: the file is being looked for in the background, and hashed when it has the recorded size and another time. |
| Modified | A file exists, but its bytes differ from the recorded ones. |
| Missing | No file at either path, or a path that is not followed (a path on another computer, a device). |
Status comes from the content hash. A file with the recorded size and modification time is OK without a read. A file with the recorded size and another time is hashed on a background thread: a matching hash is OK and the new time is remembered, a different hash is Modified. Modification time alone is never trusted to decide Modified, because it is unreliable on network shares.
Status comes from the content hash. A file with the recorded size and modification time is OK without a read. A file with the recorded size and another time is hashed on a background thread: a matching hash is OK and the new time is remembered, a different hash is Modified. Modification time alone is never trusted to decide Modified, because it is unreliable on network shares. A file whose file system reports no modification time is always told by its hash.
A Modified file's bytes are never adopted silently. The proxy stays on screen until you update or relink.
@@ -59,7 +59,7 @@ A document with any linked asset is written as format version 5; otherwise the v
## Opening
Opening reads no original. Each link is looked for at its absolute path, then at its path relative to the document. A match on size and time is OK at once; other candidates are hashed in the background. When both paths exist, the file whose hash matches wins.
Opening reads no original and asks nothing of the file system: a background check looks for each link at its absolute path, then at its path relative to the document. A match on size and time is OK without a read; other candidates of the recorded size are hashed. When both paths exist, the file whose hash matches wins.
## Old files with embedded images