M10.7d: download attachment bytes into a content-addressed store (task 2107)
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 5s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Build & push image (push) Successful in 35s
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 1m29s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 1m47s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 5s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Build & push image (push) Successful in 35s
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 1m29s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 1m47s
The client half of task 1942's server work. Metadata already rides the delta feed; this fetches the payload so a synced image exists on the device. Blobs are filed under their own sha256, so the same image attached to five notes is stored once and re-downloading it is free — the dedupe the task asks for falls out of content addressing rather than needing bookkeeping. The hash is also the integrity check, applied on the way IN. Bytes that don't hash to what the server advertised are refused rather than filed under a name that lies about them — and because the blob then still counts as missing, the next sync simply tries again. SECURITY: the hash arrives in a server response and becomes a FILENAME, so it is validated as 64 hex characters before touching the filesystem. Without that, a hostile or buggy server could send "../../..." and steer a write outside the blob directory. Tested. A failed attachment never fails the sync. Notes are the primary data and have already landed; aborting here would let one unreachable file block every future sync. Counted, logged, surfaced in the UI as "they'll retry on the next sync", and retried because the blob is still absent. sha2 is pure Rust, so the Windows cross-compile lane pays nothing for it — the constraint recorded in ci-requirements.md. SPLIT, deliberately: this stores the bytes but does NOT yet render them in the webview. That half needs a custom URI scheme or the asset protocol, whose URL form differs by platform (Windows uses http://scheme.localhost/, others scheme://localhost/) — and CI cannot verify webview rendering at all, being headless with no webview. Guessing at it here would ship an unverifiable change on the most fragile lane. Follow-up filed; synced images will show as broken until it lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SreJkbxB4gx8pPsu8QbLPi
This commit is contained in:
@@ -10,6 +10,7 @@ use chrono::{SecondsFormat, Utc};
|
||||
use rusqlite::{params, Connection, OptionalExtension};
|
||||
use serde::Serialize;
|
||||
|
||||
use super::blobs::BlobStore;
|
||||
use super::client;
|
||||
use super::state;
|
||||
use super::wire;
|
||||
@@ -33,6 +34,10 @@ pub struct PullSummary {
|
||||
/// top. Should be 0 in the normal cycle, because push runs first; anything higher
|
||||
/// means local work was overwritten, which is worth saying out loud.
|
||||
pub clobbered_dirty: usize,
|
||||
pub blobs_downloaded: usize,
|
||||
/// Attachments whose bytes couldn't be fetched or failed verification. Counted
|
||||
/// rather than fatal — see `download_missing_blobs`.
|
||||
pub blobs_failed: usize,
|
||||
}
|
||||
|
||||
impl PullSummary {
|
||||
@@ -43,10 +48,66 @@ impl PullSummary {
|
||||
self.labels_applied += other.labels_applied;
|
||||
self.labels_deleted += other.labels_deleted;
|
||||
self.clobbered_dirty += other.clobbered_dirty;
|
||||
self.blobs_downloaded += other.blobs_downloaded;
|
||||
self.blobs_failed += other.blobs_failed;
|
||||
self.cursor = other.cursor;
|
||||
}
|
||||
}
|
||||
|
||||
/// `(note_id, attachment_id, sha256)` for every attachment that advertises a hash.
|
||||
/// The caller filters against the blob store — which blobs we hold isn't a SQL
|
||||
/// question.
|
||||
pub fn hashed_attachments(conn: &Connection) -> rusqlite::Result<Vec<(String, String, String)>> {
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT note_id, id, sha256 FROM attachments
|
||||
WHERE sha256 IS NOT NULL AND sha256 <> ''",
|
||||
)?;
|
||||
let rows = stmt.query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)))?;
|
||||
rows.collect()
|
||||
}
|
||||
|
||||
/// Fetch the bytes for any attachment we have metadata for but no blob.
|
||||
///
|
||||
/// A failed attachment NEVER fails the sync. Notes are the primary data and they've
|
||||
/// already landed; an image that didn't arrive is retried on the next cycle simply
|
||||
/// because its blob still counts as missing. Aborting here would mean one unreachable
|
||||
/// file could block every future sync.
|
||||
async fn download_missing_blobs(
|
||||
db: &Db,
|
||||
blobs: &BlobStore,
|
||||
base_url: &str,
|
||||
token: &str,
|
||||
) -> Result<(usize, usize), String> {
|
||||
let wanted = {
|
||||
let conn = db.0.lock().map_err(|e| e.to_string())?;
|
||||
hashed_attachments(&conn).map_err(|e| e.to_string())?
|
||||
};
|
||||
|
||||
let mut downloaded = 0;
|
||||
let mut failed = 0;
|
||||
for (note_id, attachment_id, sha256) in wanted {
|
||||
// Content-addressed, so this skips blobs we already hold — including the same
|
||||
// image attached to a different note.
|
||||
if blobs.has(&sha256) {
|
||||
continue;
|
||||
}
|
||||
match client::fetch_attachment(base_url, token, ¬e_id, &attachment_id).await {
|
||||
Ok(bytes) => match blobs.store(&sha256, &bytes) {
|
||||
Ok(_) => downloaded += 1,
|
||||
Err(e) => {
|
||||
log::warn!("attachment {attachment_id}: {e}");
|
||||
failed += 1;
|
||||
}
|
||||
},
|
||||
Err(e) => {
|
||||
log::warn!("attachment {attachment_id}: {e}");
|
||||
failed += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok((downloaded, failed))
|
||||
}
|
||||
|
||||
fn now() -> String {
|
||||
Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true)
|
||||
}
|
||||
@@ -339,7 +400,12 @@ fn position_of(explicit: i64, index: usize) -> i64 {
|
||||
/// NOTE ON ORDERING: the full cycle is push-then-pull (docs/sync.md). Running this
|
||||
/// against a store with unpushed edits lets the server's version land on top of them
|
||||
/// — counted as `clobbered_dirty` and logged, rather than hidden.
|
||||
pub async fn run(db: &Db, base_url: &str, token: &str) -> Result<PullSummary, String> {
|
||||
pub async fn run(
|
||||
db: &Db,
|
||||
blobs: &BlobStore,
|
||||
base_url: &str,
|
||||
token: &str,
|
||||
) -> Result<PullSummary, String> {
|
||||
let mut total = PullSummary::default();
|
||||
|
||||
loop {
|
||||
@@ -377,12 +443,24 @@ pub async fn run(db: &Db, base_url: &str, token: &str) -> Result<PullSummary, St
|
||||
}
|
||||
}
|
||||
|
||||
// Notes first, bytes after: the metadata is what makes the attachments knowable,
|
||||
// and knowing one is missing is what lets the next cycle retry it.
|
||||
let (downloaded, failed) = download_missing_blobs(db, blobs, base_url, token).await?;
|
||||
total.blobs_downloaded = downloaded;
|
||||
total.blobs_failed = failed;
|
||||
|
||||
if total.clobbered_dirty > 0 {
|
||||
log::warn!(
|
||||
"pull overwrote {} note(s) that still had unpushed local edits",
|
||||
total.clobbered_dirty
|
||||
);
|
||||
}
|
||||
if total.blobs_failed > 0 {
|
||||
log::warn!(
|
||||
"pull: {} attachment(s) couldn't be downloaded; will retry next sync",
|
||||
total.blobs_failed
|
||||
);
|
||||
}
|
||||
log::info!(
|
||||
"pull complete: {} page(s), {} note(s) applied, {} deleted, {} label(s) applied, cursor {}",
|
||||
total.pages,
|
||||
|
||||
Reference in New Issue
Block a user