lib: a blob on another node is None, and Notes says so in Pictures' words
load_image_bytes, load_pdf_bytes and load_comic_page_bytes returned BlobStoreError::NotFound when the document was here and its bytes were elsewhere, and five callers matched that arm out to mean "ask a peer". They now return Option, with None for exactly that (load_blob_if_local), and the callers lose the match. ComicsFfi::get_cover_bytes is Option too, so Android stops logging an absent cover as a failure. The one caller that showed the error was Notes' embedded images, whose placeholder read "blob not found: blake3:…". It now gets ImageState::Absent with copy::picture_absent's headline for the image's availability, drawn as a plain placeholder rather than an error — absence is not an error. (M3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
3767af658b
commit
94b52ac31a
@@ -41,9 +41,9 @@ use gpui::{
|
||||
use lru::LruCache;
|
||||
use synchronicity::agents::AgentId;
|
||||
use synchronicity::agents::comics::{
|
||||
ComicError, ComicShelf, ComicTreeItem, PREFETCH_AHEAD, ReadingDirection,
|
||||
fetch_comic_page_bytes_remote, import_comic_tree, list_comic_pages, load_comic_cover_bytes,
|
||||
load_comic_page_bytes, pages_needing_prefetch, save_latest_read_child, save_reading_direction,
|
||||
ComicShelf, ComicTreeItem, PREFETCH_AHEAD, ReadingDirection, fetch_comic_page_bytes_remote,
|
||||
import_comic_tree, list_comic_pages, load_comic_cover_bytes, load_comic_page_bytes,
|
||||
pages_needing_prefetch, save_latest_read_child, save_reading_direction,
|
||||
};
|
||||
use synchronicity::agents::reading::{reading_page, save_reading_page};
|
||||
use synchronicity::documents::DocumentID;
|
||||
@@ -51,7 +51,6 @@ use synchronicity::documents::import::{COMIC_FILE_TYPES, FileTypeFilter, ImportT
|
||||
use synchronicity::jobs::runner::JobRunner;
|
||||
use synchronicity::node::NodePubkey;
|
||||
use synchronicity::stores::Store;
|
||||
use synchronicity::stores::blob_store::BlobStoreError;
|
||||
use tokio::runtime::Handle;
|
||||
|
||||
use crate::agent_sidebar::{AgentSidebar, View};
|
||||
@@ -536,7 +535,7 @@ impl ComicsView {
|
||||
&store.artifact_store,
|
||||
&comic_id,
|
||||
)
|
||||
.ok()?;
|
||||
.ok()??;
|
||||
decode_thumbnail(&bytes)
|
||||
})
|
||||
.await;
|
||||
@@ -1230,11 +1229,8 @@ async fn load_page_bytes(
|
||||
) -> Result<Vec<u8>, String> {
|
||||
let local = executor
|
||||
.spawn(async move {
|
||||
match load_comic_page_bytes(&store.blob_store, &store.artifact_store, &page_id) {
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
Err(ComicError::BlobStore(BlobStoreError::NotFound(_))) => Ok(None),
|
||||
Err(e) => Err(format!("{e}")),
|
||||
}
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, &page_id)
|
||||
.map_err(|e| format!("{e}"))
|
||||
})
|
||||
.await?;
|
||||
match local {
|
||||
|
||||
@@ -30,7 +30,7 @@ use synchronicity::agents::notebook::{
|
||||
use synchronicity::agents::pictures;
|
||||
use synchronicity::copy::action;
|
||||
use synchronicity::copy::settings as copy_settings;
|
||||
use synchronicity::copy::{incoming_change, note_body_absent};
|
||||
use synchronicity::copy::{incoming_change, note_body_absent, picture_absent};
|
||||
use synchronicity::documents::{
|
||||
self as documents, ArtifactKind, DocumentAvailability, DocumentID, TextFormat, Timestamp,
|
||||
};
|
||||
@@ -2178,25 +2178,34 @@ impl NotesView {
|
||||
let id_for_fetch = id.clone();
|
||||
let result = cx
|
||||
.background_spawn(async move {
|
||||
let bytes = pictures::load_image_bytes(
|
||||
let Some(bytes) = pictures::load_image_bytes(
|
||||
&store.blob_store,
|
||||
&store.artifact_store,
|
||||
&id_for_fetch,
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
.map_err(|e| e.to_string())?
|
||||
else {
|
||||
// The bytes are on another node: say so in the
|
||||
// words Pictures uses for the same state.
|
||||
let availability = documents::document_availability(
|
||||
&store.artifact_store,
|
||||
&store.blob_store,
|
||||
&store.node_content_status_store,
|
||||
&id_for_fetch,
|
||||
)
|
||||
.unwrap_or(DocumentAvailability::Remote);
|
||||
let (headline, _) = picture_absent::copy(availability);
|
||||
return Ok(ImageState::Absent(headline.into()));
|
||||
};
|
||||
let rgba = image::load_from_memory(&bytes)
|
||||
.map_err(|e| e.to_string())?
|
||||
.into_rgba8();
|
||||
Ok::<_, String>(to_render_image(rgba))
|
||||
let (image, width_px, height_px) = to_render_image(rgba);
|
||||
Ok::<_, String>(ImageState::Loaded { image, width_px, height_px })
|
||||
})
|
||||
.await;
|
||||
this.update(cx, |this, cx| {
|
||||
let state = match result {
|
||||
Ok((image, width_px, height_px)) => {
|
||||
ImageState::Loaded { image, width_px, height_px }
|
||||
}
|
||||
Err(e) => ImageState::Error(e.into()),
|
||||
};
|
||||
let state = result.unwrap_or_else(|e| ImageState::Error(e.into()));
|
||||
this.image_cache.borrow_mut().insert(id, state);
|
||||
cx.notify();
|
||||
})
|
||||
|
||||
@@ -28,7 +28,7 @@ use gpui::{
|
||||
};
|
||||
use mupdf::{Colorspace, Matrix, Outline};
|
||||
use synchronicity::agents::pdf::{
|
||||
PdfError, PdfListItem, PdfMetadata, PdfOutlineItem, build_pdf_nodes, fetch_pdf_bytes_remote,
|
||||
PdfListItem, PdfMetadata, PdfOutlineItem, build_pdf_nodes, fetch_pdf_bytes_remote,
|
||||
load_pdf_bytes, read_metadata,
|
||||
};
|
||||
use synchronicity::agents::reading::{
|
||||
@@ -40,7 +40,6 @@ use synchronicity::documents::{DocumentAvailability, DocumentID};
|
||||
use synchronicity::jobs::runner::JobRunner;
|
||||
use synchronicity::node::NodePubkey;
|
||||
use synchronicity::stores::Store;
|
||||
use synchronicity::stores::blob_store::BlobStoreError;
|
||||
use tokio::runtime::Handle;
|
||||
|
||||
use crate::agent_sidebar::{AgentSidebar, View};
|
||||
@@ -476,11 +475,8 @@ impl RenderDispatch {
|
||||
let local = self
|
||||
.executor
|
||||
.spawn(async move {
|
||||
match load_pdf_bytes(&store.blob_store, &store.artifact_store, &document_id) {
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
Err(PdfError::BlobStore(BlobStoreError::NotFound(_))) => Ok(None),
|
||||
Err(e) => Err(format!("{e}")),
|
||||
}
|
||||
load_pdf_bytes(&store.blob_store, &store.artifact_store, &document_id)
|
||||
.map_err(|e| format!("{e}"))
|
||||
})
|
||||
.await?;
|
||||
match local {
|
||||
|
||||
@@ -29,7 +29,6 @@ use synchronicity::documents::{DocumentAvailability, DocumentID};
|
||||
use synchronicity::jobs::runner::JobRunner;
|
||||
use synchronicity::node::NodePubkey;
|
||||
use synchronicity::stores::Store;
|
||||
use synchronicity::stores::blob_store::BlobStoreError;
|
||||
use tokio::runtime::Handle;
|
||||
|
||||
use crate::agent_sidebar::{AgentSidebar, View};
|
||||
@@ -288,16 +287,8 @@ impl PicturesView {
|
||||
cx.spawn(async move |this, cx| {
|
||||
let local = cx
|
||||
.background_spawn(async move {
|
||||
match load_image_bytes(&store.blob_store, &store.artifact_store, &id_for_local)
|
||||
{
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
// The one error that means "ask a peer": the image
|
||||
// document and its artifact are here, only the blob
|
||||
// isn't. Anything else is a real failure that streaming
|
||||
// would not fix.
|
||||
Err(PictureError::BlobStore(BlobStoreError::NotFound(_))) => Ok(None),
|
||||
Err(e) => Err(format!("Failed to load image: {e}")),
|
||||
}
|
||||
load_image_bytes(&store.blob_store, &store.artifact_store, &id_for_local)
|
||||
.map_err(|e| format!("Failed to load image: {e}"))
|
||||
})
|
||||
.await;
|
||||
|
||||
|
||||
@@ -111,7 +111,14 @@ pub struct LinkContext {
|
||||
#[derive(Clone)]
|
||||
pub enum ImageState {
|
||||
Loading,
|
||||
Loaded { image: Arc<RenderImage>, width_px: u32, height_px: u32 },
|
||||
Loaded {
|
||||
image: Arc<RenderImage>,
|
||||
width_px: u32,
|
||||
height_px: u32,
|
||||
},
|
||||
/// The image document is here and its bytes are on another node — a state
|
||||
/// to say, in these words, not a failure.
|
||||
Absent(SharedString),
|
||||
Error(SharedString),
|
||||
}
|
||||
|
||||
@@ -619,6 +626,9 @@ fn render_image_block(
|
||||
render_loaded_image(theme, image_ctx.measure_px, image, width_px, height_px, alt)
|
||||
.into_any_element()
|
||||
}
|
||||
Some(ImageState::Absent(message)) => {
|
||||
render_image_placeholder(theme, message.to_string(), false, alt).into_any_element()
|
||||
}
|
||||
Some(ImageState::Error(message)) => {
|
||||
render_image_placeholder(theme, message.to_string(), true, alt).into_any_element()
|
||||
}
|
||||
|
||||
@@ -88,3 +88,18 @@ pub(crate) fn load_blob(
|
||||
) -> Result<Vec<u8>, BlobStoreError> {
|
||||
blob_store.load(&blob_ref.blob_hash, &blob_ref.plaintext_hash, blob_ref.vault_id)
|
||||
}
|
||||
|
||||
/// [`load_blob`], with the blob being on another node as `None` rather than an
|
||||
/// error. A blob-backed document whose bytes live elsewhere is the normal
|
||||
/// case, not a failure (ADR 0016): the document and its artifact are here, and
|
||||
/// the bytes are a fetch away. Every other error is still one.
|
||||
pub(crate) fn load_blob_if_local(
|
||||
blob_store: &BlobStore,
|
||||
blob_ref: &BlobRef,
|
||||
) -> Result<Option<Vec<u8>>, BlobStoreError> {
|
||||
match load_blob(blob_store, blob_ref) {
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
Err(BlobStoreError::NotFound(_)) => Ok(None),
|
||||
Err(e) => Err(e),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@ use zip::ZipArchive;
|
||||
use chrono::{Duration, Utc};
|
||||
|
||||
use crate::agents::RECENT_WINDOW_DAYS;
|
||||
use crate::agents::blob_document::{import_blob_document, load_blob, primary_blob_ref};
|
||||
use crate::agents::blob_document::{import_blob_document, load_blob_if_local, primary_blob_ref};
|
||||
use crate::agents::library_tree::library_tree;
|
||||
use crate::agents::pictures::write_image_properties;
|
||||
use crate::agents::reading::reading_page;
|
||||
@@ -1252,14 +1252,15 @@ pub fn comic_page_blob_ref(
|
||||
primary_blob_ref(artifact_store, page_id)?.ok_or(ComicError::NoPageBlob)
|
||||
}
|
||||
|
||||
/// Load the raw (still-encoded) bytes of one page's image blob. Decoding is left to
|
||||
/// the caller, which knows the rendering target's pixel format.
|
||||
/// Load the raw (still-encoded) bytes of one page's image blob, or `None` when
|
||||
/// they are on another node — see [`fetch_comic_page_bytes_remote`]. Decoding is
|
||||
/// left to the caller, which knows the rendering target's pixel format.
|
||||
pub fn load_comic_page_bytes(
|
||||
blob_store: &BlobStore,
|
||||
artifact_store: &ArtifactStore,
|
||||
page_id: &DocumentID,
|
||||
) -> Result<Vec<u8>, ComicError> {
|
||||
Ok(load_blob(blob_store, &comic_page_blob_ref(artifact_store, page_id)?)?)
|
||||
) -> Result<Option<Vec<u8>>, ComicError> {
|
||||
Ok(load_blob_if_local(blob_store, &comic_page_blob_ref(artifact_store, page_id)?)?)
|
||||
}
|
||||
|
||||
/// Fetch one comic page's image blob from whichever reachable ensemble peer
|
||||
@@ -1337,7 +1338,7 @@ pub fn load_comic_cover_bytes(
|
||||
doc_store: &DocumentStore,
|
||||
artifact_store: &ArtifactStore,
|
||||
comic_id: &DocumentID,
|
||||
) -> Result<Vec<u8>, ComicError> {
|
||||
) -> Result<Option<Vec<u8>>, ComicError> {
|
||||
let pages = list_comic_pages(doc_store, comic_id)?;
|
||||
let first = pages.first().ok_or(ComicError::PageOutOfRange(0))?;
|
||||
load_comic_page_bytes(blob_store, artifact_store, first)
|
||||
@@ -1794,7 +1795,9 @@ mod tests {
|
||||
for (id, expected) in pages.iter().zip([b"one" as &[u8], b"two", b"three"]) {
|
||||
assert_eq!(get_document(&store.doc_store, id).unwrap().doc_type, DocumentType::Image);
|
||||
assert_eq!(
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, id).unwrap(),
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, id)
|
||||
.unwrap()
|
||||
.unwrap(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
@@ -1817,8 +1820,9 @@ mod tests {
|
||||
&store.artifact_store,
|
||||
comic_id
|
||||
)
|
||||
.unwrap(),
|
||||
b"one"
|
||||
.unwrap()
|
||||
.as_deref(),
|
||||
Some(&b"one"[..])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1969,11 +1973,15 @@ mod tests {
|
||||
let pages = list_comic_pages(&store.doc_store, &comic_id).unwrap();
|
||||
assert_eq!(pages.len(), 2);
|
||||
assert_eq!(
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, &pages[0]).unwrap(),
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, &pages[0])
|
||||
.unwrap()
|
||||
.unwrap(),
|
||||
b"one"
|
||||
);
|
||||
assert_eq!(
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, &pages[1]).unwrap(),
|
||||
load_comic_page_bytes(&store.blob_store, &store.artifact_store, &pages[1])
|
||||
.unwrap()
|
||||
.unwrap(),
|
||||
b"two"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ use log::warn;
|
||||
use rusqlite::Transaction;
|
||||
|
||||
use crate::agents::blob_document::{
|
||||
ImportError, import_blob_document, load_blob, primary_blob_ref,
|
||||
ImportError, import_blob_document, load_blob_if_local, primary_blob_ref,
|
||||
};
|
||||
use crate::content_fetch::{self, ContentFetchError};
|
||||
use crate::copy;
|
||||
@@ -219,13 +219,14 @@ pub fn pdf_blob_ref(
|
||||
}
|
||||
|
||||
/// Load the raw bytes of a PDF document's primary content blob, for parsing by a
|
||||
/// renderer. Mirrors [`crate::agents::pictures::load_image_bytes`].
|
||||
/// renderer, or `None` when they are on another node — see
|
||||
/// [`fetch_pdf_bytes_remote`]. Mirrors [`crate::agents::pictures::load_image_bytes`].
|
||||
pub fn load_pdf_bytes(
|
||||
blob_store: &BlobStore,
|
||||
artifact_store: &ArtifactStore,
|
||||
doc_id: &DocumentID,
|
||||
) -> Result<Vec<u8>, PdfError> {
|
||||
Ok(load_blob(blob_store, &pdf_blob_ref(artifact_store, doc_id)?)?)
|
||||
) -> Result<Option<Vec<u8>>, PdfError> {
|
||||
Ok(load_blob_if_local(blob_store, &pdf_blob_ref(artifact_store, doc_id)?)?)
|
||||
}
|
||||
|
||||
/// Fetch a PDF document's whole content blob from whichever reachable
|
||||
|
||||
@@ -6,7 +6,7 @@ use rusqlite::Transaction;
|
||||
|
||||
use crate::agents::add_titled;
|
||||
use crate::agents::blob_document::{
|
||||
ImportError, import_blob_document, load_blob, primary_blob_ref,
|
||||
ImportError, import_blob_document, load_blob_if_local, primary_blob_ref,
|
||||
};
|
||||
use crate::agents::library_tree::{LibraryTreeItem, library_tree, sort_for_display};
|
||||
use crate::content_fetch::{self, ContentFetchError};
|
||||
@@ -44,15 +44,16 @@ pub fn build_picture_nodes(store: &Store) -> Result<Vec<LibraryTreeItem>, Docume
|
||||
Ok(nodes.into_iter().map(|n| n.into_item(String::new())).collect())
|
||||
}
|
||||
|
||||
/// Load the raw (still-encoded) bytes of an image document's primary blob.
|
||||
/// Load the raw (still-encoded) bytes of an image document's primary blob, or
|
||||
/// `None` when they are on another node — see [`fetch_image_bytes_remote`].
|
||||
/// Decoding is left to the caller, which knows the rendering target's pixel
|
||||
/// format.
|
||||
pub fn load_image_bytes(
|
||||
blob_store: &BlobStore,
|
||||
artifact_store: &ArtifactStore,
|
||||
doc_id: &DocumentID,
|
||||
) -> Result<Vec<u8>, PictureError> {
|
||||
Ok(load_blob(blob_store, &image_blob_ref(artifact_store, doc_id)?)?)
|
||||
) -> Result<Option<Vec<u8>>, PictureError> {
|
||||
Ok(load_blob_if_local(blob_store, &image_blob_ref(artifact_store, doc_id)?)?)
|
||||
}
|
||||
|
||||
/// The [`BlobRef`] of an image document's primary content. Mirrors
|
||||
@@ -70,7 +71,7 @@ pub fn image_blob_ref(
|
||||
/// a [`ContentStream`](crate::jobs::JobKind::ContentStream) job.
|
||||
///
|
||||
/// The counterpart of [`load_image_bytes`] for the case that one returns
|
||||
/// [`BlobStoreError::NotFound`] for: the image document and its artifact are
|
||||
/// `None` for: the image document and its artifact are
|
||||
/// here and only the bytes are elsewhere. Nothing is written to the local blob
|
||||
/// store — a picture the user opened once must not turn a `Stream`-policy node
|
||||
/// into a node that holds every picture it has ever displayed (ADR 0010), which
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::agents::comics::{self, ComicError};
|
||||
use crate::agents::comics;
|
||||
use crate::agents::reading;
|
||||
use crate::documents::DocumentID;
|
||||
use crate::jobs::runner::JobRunner;
|
||||
use crate::stores::blob_store::BlobStoreError;
|
||||
|
||||
use super::error::SyncError;
|
||||
use super::node::SyncNodeCore;
|
||||
@@ -35,13 +34,10 @@ impl ComicsFfi {
|
||||
job_runner: Option<&JobRunner>,
|
||||
) -> Result<Vec<u8>, SyncError> {
|
||||
let store = self.core.store();
|
||||
match comics::load_comic_page_bytes(&store.blob_store, &store.artifact_store, page_id) {
|
||||
Ok(bytes) => return Ok(bytes),
|
||||
// The one error that means "ask a peer": the page document and its
|
||||
// artifact are here, only the blob isn't. Anything else is a real
|
||||
// failure and streaming would not fix it.
|
||||
Err(ComicError::BlobStore(BlobStoreError::NotFound(_))) => {}
|
||||
Err(e) => return Err(e.into()),
|
||||
if let Some(bytes) =
|
||||
comics::load_comic_page_bytes(&store.blob_store, &store.artifact_store, page_id)?
|
||||
{
|
||||
return Ok(bytes);
|
||||
}
|
||||
// P2P supplies the endpoint and a runtime to wait on; the store stays the one
|
||||
// the local read just used, so both halves resolve the page against the same
|
||||
@@ -86,8 +82,9 @@ impl ComicsFfi {
|
||||
}
|
||||
|
||||
/// The raw (still-encoded) image bytes of the comic's first page, for use
|
||||
/// as a library cover thumbnail. Errors if no page has synced in yet.
|
||||
pub fn get_cover_bytes(&self, comic_id: DocumentID) -> Result<Vec<u8>, SyncError> {
|
||||
/// as a library cover thumbnail, or `None` while they are on another node.
|
||||
/// Errors if no page has synced in yet.
|
||||
pub fn get_cover_bytes(&self, comic_id: DocumentID) -> Result<Option<Vec<u8>>, SyncError> {
|
||||
let store = self.core.store();
|
||||
Ok(comics::load_comic_cover_bytes(
|
||||
&store.blob_store,
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::agents::LibraryTreeItem;
|
||||
use crate::agents::pictures::{self, PictureError};
|
||||
use crate::agents::pictures;
|
||||
use crate::documents::DocumentID;
|
||||
use crate::stores::blob_store::BlobStoreError;
|
||||
|
||||
use super::error::SyncError;
|
||||
use super::node::SyncNodeCore;
|
||||
@@ -44,21 +43,18 @@ impl PicturesFfi {
|
||||
/// here, streamed from a peer that holds them if they aren't. Decoding is
|
||||
/// the caller's, which knows its own target pixel format.
|
||||
///
|
||||
/// Mirrors [`ComicsFfi::get_page_bytes`](super::ComicsFfi::get_page_bytes)
|
||||
/// down to the error it treats as "ask a peer", and for the same reason:
|
||||
/// Mirrors [`ComicsFfi::get_page_bytes`](super::ComicsFfi::get_page_bytes),
|
||||
/// and for the same reason:
|
||||
/// every blob-backed type opens what it doesn't hold (ADR 0016), and an
|
||||
/// image is no exception. A `SyncError::Offline` back from here means the
|
||||
/// bytes exist somewhere but nothing reachable is serving them — a state to
|
||||
/// render, not a failure to report as one.
|
||||
pub fn get_image_bytes(&self, image_id: DocumentID) -> Result<Vec<u8>, SyncError> {
|
||||
let store = self.core.store();
|
||||
match pictures::load_image_bytes(&store.blob_store, &store.artifact_store, &image_id) {
|
||||
Ok(bytes) => return Ok(bytes),
|
||||
// The one error that means "ask a peer": the image document and its
|
||||
// artifact are here, only the blob isn't. Anything else is a real
|
||||
// failure and streaming would not fix it.
|
||||
Err(PictureError::BlobStore(BlobStoreError::NotFound(_))) => {}
|
||||
Err(e) => return Err(e.into()),
|
||||
if let Some(bytes) =
|
||||
pictures::load_image_bytes(&store.blob_store, &store.artifact_store, &image_id)?
|
||||
{
|
||||
return Ok(bytes);
|
||||
}
|
||||
// P2P supplies the endpoint and a runtime to wait on; the store stays the one
|
||||
// the local read just used, so both halves resolve the image against the same
|
||||
|
||||
Reference in New Issue
Block a user