diff --git a/src/appview/drafts.rs b/src/appview/drafts.rs index e43875d..ea1b7b3 100644 --- a/src/appview/drafts.rs +++ b/src/appview/drafts.rs @@ -716,6 +716,9 @@ mod tests { .unwrap(); assert_eq!(published.models.len(), 1); let requests = create_records.lock().await; + // Assert exactly 3 calls in part→model→thing order BEFORE any + // dereference, so a missing or reordered createRecord cannot satisfy + // the chain. assert_eq!(requests.len(), 3); assert_eq!( requests @@ -731,6 +734,21 @@ mod tests { for request in requests.iter() { assert_eq!(request["repo"], DID_A); } + // Derive expected CIDs independently from request bytes, matching the + // fixture's per-request CID derivation. + let part_cid = + crate::appview::test_support::test_blob(&serde_json::to_vec(&requests[0]).unwrap()).0; + let model_cid = + crate::appview::test_support::test_blob(&serde_json::to_vec(&requests[1]).unwrap()).0; + let thing_cid = + crate::appview::test_support::test_blob(&serde_json::to_vec(&requests[2]).unwrap()).0; + // CIDs must be pairwise distinct so a misbound or partial publish + // cannot satisfy the chain. + let mut distinct = std::collections::HashSet::new(); + assert!(distinct.insert(&part_cid), "part CID distinct"); + assert!(distinct.insert(&model_cid), "model CID distinct"); + assert!(distinct.insert(&thing_cid), "thing CID distinct"); + // Assert each request's own identity. assert_eq!(requests[0]["rkey"], serde_json::Value::Null); assert_eq!( requests[0]["record"]["$type"], @@ -745,14 +763,12 @@ mod tests { "space.polymodel.library.model" ); assert_eq!(requests[1]["record"]["name"], "Model"); + // model.parts[0] must equal the captured PART response (uri+cid). assert_eq!( requests[1]["record"]["parts"][0]["uri"], format!("at://{DID_A}/space.polymodel.library.part/record-0") ); - assert_eq!( - requests[1]["record"]["parts"][0]["cid"], - "bafkreih3ryqpylsmh4siyygdtplff46bgrzjro4xpofu2widxbifkyqgam" - ); + assert_eq!(requests[1]["record"]["parts"][0]["cid"], part_cid); assert_eq!(requests[2]["rkey"], serde_json::Value::Null); assert_eq!( requests[2]["record"]["$type"], @@ -760,14 +776,30 @@ mod tests { ); assert_eq!(requests[2]["record"]["name"], "After"); assert_eq!(requests[2]["record"]["license"], "CC-BY-4.0"); + // thing.models[0] must equal the captured MODEL response. assert_eq!( requests[2]["record"]["models"][0]["uri"], format!("at://{DID_A}/space.polymodel.library.model/record-1") ); + assert_eq!(requests[2]["record"]["models"][0]["cid"], model_cid); + // Assert the publish output's strong refs match the captured responses. + assert_eq!(published.models.len(), 1); + assert_eq!(published.models[0].parts.len(), 1); + assert_eq!( + published.models[0].parts[0].uri.as_str(), + format!("at://{DID_A}/space.polymodel.library.part/record-0") + ); + assert_eq!(published.models[0].parts[0].cid.as_str(), part_cid); + assert_eq!( + published.models[0].model.uri.as_str(), + format!("at://{DID_A}/space.polymodel.library.model/record-1") + ); + assert_eq!(published.models[0].model.cid.as_str(), model_cid); assert_eq!( - requests[2]["record"]["models"][0]["cid"], - "bafkreih3ryqpylsmh4siyygdtplff46bgrzjro4xpofu2widxbifkyqgam" + published.thing.uri.as_str(), + format!("at://{DID_A}/space.polymodel.library.thing/record-2") ); + assert_eq!(published.thing.cid.as_str(), thing_cid); drop(requests); let deleted_after_publish = app .oneshot( diff --git a/src/appview/test_support.rs b/src/appview/test_support.rs index badf775..a099559 100644 --- a/src/appview/test_support.rs +++ b/src/appview/test_support.rs @@ -379,9 +379,15 @@ pub(crate) async fn loopback_pds_for_publish( .as_str() .map(ToOwned::to_owned) .unwrap_or_else(|| format!("record-{ordinal}")); + // Derive a distinct CID per createRecord call from the + // request body (repo + collection + record) so that + // strong-ref assertions cannot be satisfied by a + // misbound or partial publish. + let request_bytes = serde_json::to_vec(&request).unwrap(); + let (cid, _) = test_blob(&request_bytes); axum::Json(json!({ "uri": format!("at://{DID_A}/{collection}/{rkey}"), - "cid": "bafkreih3ryqpylsmh4siyygdtplff46bgrzjro4xpofu2widxbifkyqgam" + "cid": cid })) .into_response() } diff --git a/src/appview/writes.rs b/src/appview/writes.rs index 687a6a2..c412928 100644 --- a/src/appview/writes.rs +++ b/src/appview/writes.rs @@ -1763,7 +1763,9 @@ mod tests { use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; use tower::ServiceExt; - use crate::appview::test_support::{seed_identity, seed_oauth_session_at, seed_profile}; + use crate::appview::test_support::{ + seed_identity, seed_oauth_session_at, seed_profile, test_blob, + }; use super::*; @@ -1949,8 +1951,19 @@ mod tests { ); assert_eq!(output.file.chunks.len(), 1, "{mime}"); let output_chunk = &output.file.chunks[0]; - assert_eq!(output_chunk.offset, 0, "{mime}"); - assert_eq!(output_chunk.size, bytes.len() as i64, "{mime}"); + // Independently derive the expected chunk blob CID from the + // request bytes — not from the response — so a bad upload + // response cannot echo a wrong CID. + let expected_cid = test_blob(bytes).0; + let expected_offset: i64 = 0; + let expected_size: i64 = bytes.len() as i64; + assert_eq!(output_chunk.offset, expected_offset, "{mime}"); + assert_eq!(output_chunk.size, expected_size, "{mime}"); + assert_eq!( + output_chunk.blob.blob().r#ref.as_str(), + expected_cid, + "{mime}" + ); assert_eq!(output_chunk.blob.blob().mime_type.as_str(), mime, "{mime}"); assert_eq!(output_chunk.blob.blob().size, bytes.len(), "{mime}"); let upload_id = output.upload_id.to_string(); @@ -1964,19 +1977,20 @@ mod tests { .unwrap(); assert_eq!(row.0, DID_A, "{mime}"); assert_eq!(row.1, filename, "{mime}"); - assert_eq!(row.2, bytes.len() as i64, "{mime}"); + assert_eq!(row.2, expected_size, "{mime}"); assert_eq!(row.3, digest, "{mime}"); let stored: File = serde_json::from_str(&row.4).unwrap(); assert_eq!(stored.mime_type.as_str(), mime, "{mime}"); - assert_eq!(stored.size, bytes.len() as i64, "{mime}"); + assert_eq!(stored.size, expected_size, "{mime}"); assert_eq!(stored.digest.as_deref(), Some(digest.as_slice()), "{mime}"); assert_eq!(stored.chunks.len(), 1, "{mime}"); let stored_chunk = &stored.chunks[0]; - assert_eq!(stored_chunk.offset, output_chunk.offset, "{mime}"); - assert_eq!(stored_chunk.size, output_chunk.size, "{mime}"); + // Assert stored manifest CID equals the independently-derived CID. + assert_eq!(stored_chunk.offset, expected_offset, "{mime}"); + assert_eq!(stored_chunk.size, expected_size, "{mime}"); assert_eq!( stored_chunk.blob.blob().r#ref.as_str(), - output_chunk.blob.blob().r#ref.as_str(), + expected_cid, "{mime}" ); assert_eq!(stored_chunk.blob.blob().mime_type.as_str(), mime, "{mime}"); diff --git a/src/publish.rs b/src/publish.rs index 3006ce3..44e1ef7 100644 --- a/src/publish.rs +++ b/src/publish.rs @@ -194,6 +194,10 @@ struct Flow { /// Current owner of the shared file-upload status. A stale operation may /// retire or replace the status only while its token remains current. upload_operation: Signal>, + /// Current owner of the image-upload status. Separate from + /// `upload_operation` so the two signals never couple; a stale image + /// completion must not overwrite a newer image upload's status. + image_upload_operation: Signal>, /// Monotonic token source for asynchronous operations in this flow instance. operation_counter: Signal, } @@ -241,6 +245,7 @@ fn PublishFlow(resume: String) -> Element { let name_element: Signal>> = use_signal(|| None); let preview_generations = use_signal(HashMap::new); let upload_operation = use_signal(|| None); + let image_upload_operation = use_signal(|| None); let operation_counter = use_signal(|| 0_u64); let flow = Flow { @@ -265,6 +270,7 @@ fn PublishFlow(resume: String) -> Element { name_element, preview_generations, upload_operation, + image_upload_operation, operation_counter, }; @@ -285,6 +291,8 @@ fn PublishFlow(resume: String) -> Element { let multi = loaded.models.len() > 1; normalize_loaded_draft(&mut loaded); flow.preview_generations.write().clear(); + flow.upload_operation.set(None); + flow.image_upload_operation.set(None); { let mut composition = flow.composition; composition.set(loaded); @@ -465,6 +473,8 @@ impl Flow { let multi = loaded.models.len() > 1; normalize_loaded_draft(&mut loaded); self.preview_generations.write().clear(); + self.upload_operation.set(None); + self.image_upload_operation.set(None); { let mut composition = self.composition; composition.set(loaded); @@ -490,6 +500,8 @@ impl Flow { Ok(_) => { if self.draft_id.read().as_deref() == Some(id.as_str()) { self.preview_generations.write().clear(); + self.upload_operation.set(None); + self.image_upload_operation.set(None); let mut composition = self.composition; let mut draft_id = self.draft_id; composition.set(DraftThingInput::default()); @@ -2071,6 +2083,24 @@ impl Flow { true } + fn begin_image_upload(&self, label: String) -> u64 { + let token = self.next_operation_token(); + let mut operation = self.image_upload_operation; + operation.set(Some(token)); + let mut upload = self.image_upload; + upload.set(UploadStatus::Uploading(label)); + token + } + + fn set_image_upload_if_current(&self, token: u64, status: UploadStatus) -> bool { + if self.image_upload_operation.read().as_ref() != Some(&token) { + return false; + } + let mut upload = self.image_upload; + upload.set(status); + true + } + /// Stage every dropped/picked file through one batch: ensure the default /// model + auto name, stage each file (stop at first failure), then save once. async fn handle_files_upload(self, files: Vec) { @@ -2485,14 +2515,14 @@ impl Flow { /// not race the file-upload status. async fn handle_image_upload(self, target: FieldTarget, is_cover: bool, file: FileData) { let filename = file.name(); - let mut status = self.image_upload; - status.set(UploadStatus::Uploading(filename.clone())); + let image_token = self.begin_image_upload(filename.clone()); let bytes = match file.read_bytes().await { Ok(bytes) => bytes.to_vec(), Err(_) => { - status.set(UploadStatus::Error( - "Could not read the selected image.".into(), - )); + self.set_image_upload_if_current( + image_token, + UploadStatus::Error("Could not read the selected image.".into()), + ); return; } }; @@ -2514,11 +2544,14 @@ impl Flow { list.push(image); } } - status.set(UploadStatus::Idle); + self.set_image_upload_if_current(image_token, UploadStatus::Idle); self.mark_dirty_and_save(); } Err(error) => { - status.set(UploadStatus::Error(format!("Image upload failed: {error}"))); + self.set_image_upload_if_current( + image_token, + UploadStatus::Error(format!("Image upload failed: {error}")), + ); } } } @@ -3824,8 +3857,13 @@ mod tests { } #[test] - fn removed_model_cannot_receive_stale_preview_after_vector_compaction() { - let generated = test_image("stale generated"); + fn mismatched_primary_cannot_commit_preview() { + // Unique to the sync path: directly invoke commit_generated_preview + // with a mismatched primary/identity (the identity targets + // resource-a/model-a but the composition's remaining model has a + // different primary resource). The commit must be rejected without + // mutating the composition — no preview, no parts, no cover. + let generated = test_image("mismatched generated"); let resource = DraftStagedResource { resource_id: "resource-a".into(), upload_id: "upload-a".into(), @@ -3836,27 +3874,14 @@ mod tests { format: Some("stl".into()), }; let mut composition = DraftThingInput { - models: vec![ - DraftModelInput { - model_id: Some("model-a".into()), - primary_resource_id: Some("resource-a".into()), - staged_resources: vec![resource.clone()], - parts: vec![DraftPartInput { - resource_id: Some("resource-a".into()), - ..Default::default() - }], - ..Default::default() - }, - DraftModelInput { - model_id: Some("model-b".into()), - primary_resource_id: Some("resource-b".into()), - ..Default::default() - }, - ], + models: vec![DraftModelInput { + model_id: Some("model-b".into()), + primary_resource_id: Some("resource-b".into()), + ..Default::default() + }], ..Default::default() }; let identity = preview_identity("draft", "model-a", "resource-a", &[resource]).unwrap(); - composition.models.remove(0); assert!(!commit_generated_preview( &mut composition, @@ -3870,6 +3895,7 @@ mod tests { assert_eq!(composition.models[0].model_id.as_deref(), Some("model-b")); assert!(composition.models[0].generated_preview.is_none()); assert!(composition.models[0].previews.is_none()); + assert!(composition.models[0].parts.is_empty()); assert!(composition.previews.is_none()); }