diff --git a/editor/src/dispatcher.rs b/editor/src/dispatcher.rs index f2c665648d..4941768b2a 100644 --- a/editor/src/dispatcher.rs +++ b/editor/src/dispatcher.rs @@ -96,8 +96,9 @@ impl Dispatcher { s } + #[cfg(test)] pub fn with_executor(executor: crate::node_graph_executor::NodeGraphExecutor) -> Self { - let mut s = Self::default(); + let mut s = Self::new(Arc::new(graph_craft::application_io::resource::HashMapResourceStorage::new()), None); s.message_handlers.portfolio_message_handler = PortfolioMessageHandler::with_executor(executor); s } diff --git a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs index ce1bf94b37..a5df618a69 100644 --- a/editor/src/messages/portfolio/document/resource/resource_message_handler.rs +++ b/editor/src/messages/portfolio/document/resource/resource_message_handler.rs @@ -3,7 +3,7 @@ use crate::messages::portfolio::{document::resource::utility_types::EmbeddedReso use crate::messages::prelude::*; use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; -use graph_craft::application_io::resource::{DataSource, LoadResource, Resource, ResourceHash, ResourceId, ResourceRegistry}; +use graph_craft::application_io::resource::{DataSource, LoadResource, Resource, ResourceHash, ResourceId, ResourceRegistry, ResourceStorage}; use graphene_std::text::Font; use std::sync::Arc; use url::Url; @@ -49,15 +49,8 @@ impl MessageHandler> for ResourceMes responses.add(ResourceMessage::Resolve { resource_id }); } ResourceMessage::ResolveAll => { - // A resource keeps its hash when storage evicts its data, so only a fetchable source can repair it - let refetchable = self - .registry - .resolved() - .filter(|info| info.hash.is_some_and(|hash| !resource_storage.contains(hash))) - .filter(|info| info.sources.iter().any(|source| matches!(source, DataSource::Url(_) | DataSource::Font { .. }))) - .map(|info| info.id); - let ids: Vec = self.registry.unresolved().map(|info| info.id).chain(refetchable).collect(); - + let storage = resource_storage.resources_mut(); + let ids: Vec = self.registry.ids().filter(|id| !self.registry.hash(id).is_some_and(|hash| storage.contains(&hash))).collect(); for id in ids { if self.pending_resolves.contains(&id) { continue; @@ -74,9 +67,7 @@ impl MessageHandler> for ResourceMes log::error!("Resolve for {resource_id}: no registry entry"); return; }; - // This hash names the very data that is missing, so it cannot stand in for fetching that data - let data_missing = info.hash.is_some_and(|hash| !resource_storage.contains(hash)); - if info.hash.is_some() && !data_missing { + if info.hash.is_some_and(|hash| resource_storage.resources_mut().contains(hash)) { log::warn!("Resource {resource_id} already resolved"); return; } @@ -89,7 +80,7 @@ impl MessageHandler> for ResourceMes .sources .iter() .map(|source| match source { - DataSource::Font { family, style } if !data_missing => { + DataSource::Font { family, style } => { let font = match style { Some(style) => Font::new(family.clone(), style.clone()), None => Font::new_with_default_style(family.clone()), @@ -225,20 +216,16 @@ impl ResourceMessageHandler { .resolved() .filter(|info| info.sources.contains(&DataSource::Embedded)) .filter_map(|info| { - let (id, hash) = (info.id, *info.hash?); - let resource = resources_load_handle.load(hash); - Some(async move { (id, hash, resource.await) }) + if let Some(hash) = info.hash { + let resource = resources_load_handle.load(*hash); + Some(async move { resource.await.map(|resource| (*hash, resource)) }) + } else { + None + } }) .collect::>(); - let loaded = futures::future::join_all(embedded).await; - - // Saving without these bytes writes a document whose registry claims to carry them - for (id, hash, _) in loaded.iter().filter(|(_, _, resource)| resource.is_none()) { - log::error!("Resource {id} ({hash}) is marked as embedded but its data is missing from storage, so the saved document will not contain it"); - } - - self.embedded = EmbeddedResources::from_iter(loaded.into_iter().filter_map(|(_, hash, resource)| resource.map(|resource| (hash, resource)))); + self.embedded = EmbeddedResources::from_iter(futures::future::join_all(embedded).await.into_iter().flatten()); } pub fn collect_garbage(&mut self, used: &[ResourceId]) { @@ -318,45 +305,28 @@ mod tests { use super::*; use graph_craft::application_io::resource::ResourceStorage; - /// Storage can lose a resource's data while the document keeps the hash naming it, which leaves the graph - /// pointing at bytes that are gone. Only sources that can be fetched again are worth re-resolving. #[test] - fn resolve_all_refetches_resources_whose_data_is_missing() { + fn resolve_all_refetches_resources_whose_bytes_are_missing() { let mut handler = ResourceMessageHandler::default(); let storage = ResourceStorageMessageHandler::default(); + let fonts = FontsMessageHandler::default(); - // Present: its bytes are in storage, so it is already usable - let present = ResourceId::new(); - let present_hash = storage.resources_mut().store(b"stored font bytes"); - handler.registry.resolve(&present, present_hash); - handler.registry.push_source_back(&present, DataSource::Embedded); - handler.registry.push_source_back( - &present, - DataSource::Font { - family: "Lato".into(), - style: Some("Regular (400)".into()), - }, - ); + let font = |style: &str| DataSource::Font { + family: "Lato".into(), + style: Some(style.into()), + }; - // Recoverable: its bytes are gone, but the font it came from can be downloaded again - let recoverable = ResourceId::new(); - handler.registry.resolve(&recoverable, ResourceHash::from(b"evicted font bytes".as_slice())); - handler.registry.push_source_back(&recoverable, DataSource::Embedded); - handler.registry.push_source_back( - &recoverable, - DataSource::Font { - family: "Lato".into(), - style: Some("Black (900)".into()), - }, - ); + let cached = ResourceId::from(1); + let cached_hash = storage.resources_mut().store(b"stored font bytes"); + handler.registry.resolve(&cached, cached_hash); + handler.registry.push_source_back(&cached, font("Regular (400)")); - // Unrecoverable: its bytes are gone and nothing records where to fetch them from - let unrecoverable = ResourceId::new(); - handler.registry.resolve(&unrecoverable, ResourceHash::from(b"evicted image bytes".as_slice())); - handler.registry.push_source_back(&unrecoverable, DataSource::Embedded); + let evicted = ResourceId::from(2); + let evicted_hash = ResourceHash::from(b"evicted font bytes".as_slice()); + handler.registry.resolve(&evicted, evicted_hash); + handler.registry.push_source_back(&evicted, font("Black (900)")); let mut responses = VecDeque::new(); - let fonts = FontsMessageHandler::default(); handler.process_message( ResourceMessage::ResolveAll, &mut responses, @@ -368,9 +338,8 @@ mod tests { ); let resolve_requested = |id: ResourceId| responses.contains(&Message::from(ResourceMessage::Resolve { resource_id: id })); - - assert!(resolve_requested(recoverable), "a missing resource with a font source should be fetched again"); - assert!(!resolve_requested(present), "a resource whose data is in storage should be left alone"); - assert!(!resolve_requested(unrecoverable), "a missing resource with no fetchable source has nowhere to fetch from"); + assert!(resolve_requested(evicted), "a resource whose bytes are gone should be resolved again"); + assert!(!resolve_requested(cached), "a resource whose bytes are in storage should be left alone"); + assert_eq!(handler.registry.hash(&evicted), Some(evicted_hash), "the hash keeps naming the content"); } } diff --git a/editor/src/messages/resource_storage/resource_storage_message_handler.rs b/editor/src/messages/resource_storage/resource_storage_message_handler.rs index 0f5dc35516..26001fb948 100644 --- a/editor/src/messages/resource_storage/resource_storage_message_handler.rs +++ b/editor/src/messages/resource_storage/resource_storage_message_handler.rs @@ -48,11 +48,6 @@ impl ResourceStorageMessageHandler { inner: self.storage.clone().expect("Resource storage not initialized"), } } - - /// Whether the resource's data is held in storage, assuming it is until storage is initialized. - pub fn contains(&self, hash: &ResourceHash) -> bool { - self.storage.as_ref().is_none_or(|storage| storage.contains(hash)) - } } impl std::fmt::Debug for ResourceStorageMessageHandler { diff --git a/node-graph/graph-craft/src/application_io/resource/opfs.rs b/node-graph/graph-craft/src/application_io/resource/opfs.rs index e023db78b5..261a172b64 100644 --- a/node-graph/graph-craft/src/application_io/resource/opfs.rs +++ b/node-graph/graph-craft/src/application_io/resource/opfs.rs @@ -153,9 +153,6 @@ async fn drain_queue(inner: Arc>) { Mutation::Write { hash, bytes } => { if let Err(error) = write_file(&directory, &hash, &bytes).await { log::error!("OPFS write for {hash} failed: {error:?}"); - - // Nothing reached disk, so leaving the hash listed would claim a file that later sessions cannot read - inner.lock().unwrap().on_disk.remove(&hash); } } Mutation::Delete { hash } => { diff --git a/node-graph/nodes/gstd/src/platform_application_io.rs b/node-graph/nodes/gstd/src/platform_application_io.rs index 13ad491d0e..5ed9eb366a 100644 --- a/node-graph/nodes/gstd/src/platform_application_io.rs +++ b/node-graph/nodes/gstd/src/platform_application_io.rs @@ -276,7 +276,6 @@ pub async fn resource<'a: 'n>( return placeholder(); }; - // Stored bytes go missing when the browser evicts its storage or a write is interrupted let Some(resource) = application_io.load_resource(hash).await else { log::error!("Resource {hash} was not found in storage"); return placeholder();