diff --git a/.config/supply-chain/audits.toml b/.config/supply-chain/audits.toml index f0bcc9c5..43d29cef 100644 --- a/.config/supply-chain/audits.toml +++ b/.config/supply-chain/audits.toml @@ -11,6 +11,11 @@ who = "Jean Mertz " criteria = "safe-to-deploy" delta = "0.49.0 -> 0.50.0" +[[audits.datetime_literal]] +who = "Jean Mertz " +criteria = "safe-to-deploy" +version = "0.1.3" + [[audits.fancy-regex]] who = "Jean Mertz " criteria = "safe-to-deploy" diff --git a/.config/supply-chain/imports.lock b/.config/supply-chain/imports.lock index dac9c620..cb835ae0 100644 --- a/.config/supply-chain/imports.lock +++ b/.config/supply-chain/imports.lock @@ -3640,3 +3640,10 @@ who = "Jack Grigg " criteria = "safe-to-deploy" delta = "0.1.1 -> 0.1.3" aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml" + +[[audits.zcash.audits.windows-link]] +who = "Jack Grigg " +criteria = "safe-to-deploy" +delta = "0.2.0 -> 0.2.1" +notes = "No code changes at all." +aggregated-from = "https://raw.githubusercontent.com/zcash/librustzcash/main/supply-chain/audits.toml" diff --git a/Cargo.lock b/Cargo.lock index 1324dd74..27afb7c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -737,6 +737,15 @@ dependencies = [ "syn", ] +[[package]] +name = "datetime_literal" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcd4412a63631453298d1c8d9935baca734b58663adb3114bc872c6d13c0b5a6" +dependencies = [ + "chrono", +] + [[package]] name = "derive_builder" version = "0.20.2" @@ -2216,6 +2225,7 @@ dependencies = [ "camino", "camino-tempfile", "chrono", + "datetime_literal", "directories", "jp_config", "jp_conversation", diff --git a/Cargo.toml b/Cargo.toml index 6c036f3b..80b9530c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ comrak = { version = "0.50", default-features = false } convert_case = { version = "0.11", default-features = false } crossbeam-channel = { version = "0.5", default-features = false } crossterm = { version = "0.29", default-features = false } +datetime_literal = { version = "0.1", default-features = false } dhat = { version = "0.3", default-features = false } directories = { version = "6", default-features = false } duct = { version = "1", default-features = false } diff --git a/crates/jp_workspace/Cargo.toml b/crates/jp_workspace/Cargo.toml index 8bc0abcb..097148db 100644 --- a/crates/jp_workspace/Cargo.toml +++ b/crates/jp_workspace/Cargo.toml @@ -27,6 +27,7 @@ thiserror = { workspace = true } tracing = { workspace = true } [dev-dependencies] +datetime_literal = { workspace = true } test-log = { workspace = true } [lints] diff --git a/crates/jp_workspace/src/lib.rs b/crates/jp_workspace/src/lib.rs index b9d9276b..c8d87d61 100644 --- a/crates/jp_workspace/src/lib.rs +++ b/crates/jp_workspace/src/lib.rs @@ -168,26 +168,38 @@ impl Workspace { ); let conversation_ids = storage.load_all_conversation_ids(); - let active_conversation = match storage - .load_conversation_metadata(&metadata.active_conversation_id) - { - Ok(conversation) => conversation, - // If the active conversation cannot be found on disk, we try to - // load the last known conversation on disk, and if that fails, we - // return an error. - Err(error) if error.kind().is_missing() => { - let last_conversation_id = conversation_ids.last().copied(); - warn!( - %error, - missing_id = %metadata.active_conversation_id, - new_id = %last_conversation_id.as_ref().map(ToString::to_string).unwrap_or_default(), - "Failed to load active conversation, falling back to last stored conversation." - ); - - metadata.active_conversation_id = last_conversation_id.ok_or(error)?; - storage.load_conversation_metadata(&metadata.active_conversation_id)? + + let mut all_ids = std::iter::once(metadata.active_conversation_id) + .chain(conversation_ids.iter().rev().copied()) + .peekable(); + let mut active_conversation = None; + while let Some(id) = all_ids.next() { + metadata.active_conversation_id = id; + + match storage.load_conversation_metadata(&metadata.active_conversation_id) { + Ok(conversation) => { + active_conversation = Some(conversation); + break; + } + + // If the active conversation cannot be found on disk, we try to + // load the last known conversation on disk, and if that fails, + // we return an error. + Err(error) if error.kind().is_missing() => { + warn!( + %error, + missing_id = %metadata.active_conversation_id, + new_id = %all_ids.peek().map(ToString::to_string).unwrap_or_default(), + "Failed to load active conversation, trying to fall back to last stored conversation." + ); + } + + Err(error) => return Err(error.into()), } - Err(error) => return Err(error.into()), + } + + let Some(active_conversation) = active_conversation else { + return Err(Error::NotFound("Conversation", String::new())); }; let conversations = conversation_ids @@ -673,8 +685,12 @@ mod tests { use std::{collections::HashMap, fs, time::Duration}; use camino_tempfile::tempdir; - use chrono::TimeZone as _; - use jp_storage::{CONVERSATIONS_DIR, METADATA_FILE, value::read_json}; + use datetime_literal::datetime; + use jp_conversation::ConversationsMetadata; + use jp_storage::{ + CONVERSATIONS_DIR, METADATA_FILE, + value::{read_json, write_json}, + }; use test_log::test; use super::*; @@ -901,6 +917,110 @@ mod tests { ); } + #[test] + fn test_load_falls_back_when_active_conversation_missing() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("root"); + let storage = root.join("storage"); + + let id1 = ConversationId::try_from(datetime!(2024-01-01 00:00:00 Z)).unwrap(); + let id2 = ConversationId::try_from(datetime!(2024-01-02 00:00:00 Z)).unwrap(); + let id3 = ConversationId::try_from(datetime!(2024-01-03 00:00:00 Z)).unwrap(); + + fs::create_dir_all(&storage).unwrap(); + write_conversation_to_disk(&storage, &id2, &Conversation::default()); + write_conversation_to_disk(&storage, &id3, &Conversation::default()); + + // Point metadata at a conversation that doesn't exist on disk. + write_conversations_metadata_to_disk(&storage, &id1); + + let mut workspace = Workspace::new(&root).persisted_at(&storage).unwrap(); + workspace.disable_persistence(); + workspace.load().unwrap(); + + // Should fall back to the last conversation. + assert_eq!(workspace.active_conversation_id(), id3); + } + + #[test] + fn test_load_falls_back_when_active_is_also_last_conversation() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("root"); + let storage = root.join("storage"); + + let id1 = ConversationId::try_from(datetime!(2024-01-01 00:00:00 Z)).unwrap(); + let id2 = ConversationId::try_from(datetime!(2024-01-02 00:00:00 Z)).unwrap(); + let id3 = ConversationId::try_from(datetime!(2024-01-03 00:00:00 Z)).unwrap(); + + fs::create_dir_all(&storage).unwrap(); + write_conversation_to_disk(&storage, &id1, &Conversation::default()); + write_conversation_to_disk(&storage, &id2, &Conversation::default()); + + // id3 has a directory on disk (so it shows up in conversation_ids) but + // it contains no metadata.json, so loading it will fail. + let id3_dir = storage.join(CONVERSATIONS_DIR).join(id3.to_dirname(None)); + fs::create_dir_all(&id3_dir).unwrap(); + + // Point metadata at id3, which exists as a directory but has no + // metadata.json, so loading it will fail. + write_conversations_metadata_to_disk(&storage, &id3); + + let mut workspace = Workspace::new(&root).persisted_at(&storage).unwrap(); + workspace.disable_persistence(); + workspace.load().unwrap(); + + // Should skip id3 (missing metadata), then try id2 (valid). + assert_eq!(workspace.active_conversation_id(), id2); + } + + #[test] + fn test_load_fails_when_no_conversations_exist() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("root"); + let storage = root.join("storage"); + + let missing_id = ConversationId::try_from(datetime!(2024-06-01 00:00:00 Z)).unwrap(); + + fs::create_dir_all(&storage).unwrap(); + write_conversations_metadata_to_disk(&storage, &missing_id); + + let mut workspace = Workspace::new(&root).persisted_at(&storage).unwrap(); + workspace.disable_persistence(); + + let err = workspace.load().unwrap_err(); + assert_eq!(err, Error::NotFound("Conversation", String::new())); + } + + #[test] + fn test_load_skips_multiple_missing_conversations() { + let tmp = tempdir().unwrap(); + let root = tmp.path().join("root"); + let storage = root.join("storage"); + + let id1 = ConversationId::try_from(datetime!(2024-01-01 00:00:00 Z)).unwrap(); + let id2 = ConversationId::try_from(datetime!(2024-01-02 00:00:00 Z)).unwrap(); + let id3 = ConversationId::try_from(datetime!(2024-01-03 00:00:00 Z)).unwrap(); + let id4 = ConversationId::try_from(datetime!(2024-01-04 00:00:00 Z)).unwrap(); + + fs::create_dir_all(&storage).unwrap(); + // Only id1 is valid; id2, id3, id4 are directories without metadata. + write_conversation_to_disk(&storage, &id1, &Conversation::default()); + for id in [&id2, &id3, &id4] { + let dir = storage.join(CONVERSATIONS_DIR).join(id.to_dirname(None)); + fs::create_dir_all(&dir).unwrap(); + } + + // Point at id4 (missing metadata), reverse iteration: id4, id3, id2 + // should all be skipped, landing on id1. + write_conversations_metadata_to_disk(&storage, &id4); + + let mut workspace = Workspace::new(&root).persisted_at(&storage).unwrap(); + workspace.disable_persistence(); + workspace.load().unwrap(); + + assert_eq!(workspace.active_conversation_id(), id1); + } + #[test] fn test_workspace_persist_active_conversation() { let tmp = tempdir().unwrap(); @@ -910,10 +1030,8 @@ mod tests { let mut workspace = Workspace::new(&root).persisted_at(&storage).unwrap(); let config = Arc::new(AppConfig::new_test()); - let id1 = - ConversationId::try_from(Utc.with_ymd_and_hms(2023, 1, 1, 0, 0, 0).unwrap()).unwrap(); - let id2 = - ConversationId::try_from(Utc.with_ymd_and_hms(2023, 1, 2, 0, 0, 0).unwrap()).unwrap(); + let id1 = ConversationId::try_from(datetime!(2024-01-01 00:00:00 Z)).unwrap(); + let id2 = ConversationId::try_from(datetime!(2024-01-02 00:00:00 Z)).unwrap(); workspace.create_conversation_with_id(id1, Conversation::default(), config.clone()); workspace.create_conversation_with_id(id2, Conversation::default(), config.clone()); @@ -937,4 +1055,29 @@ mod tests { assert!(id1_metadata_file.is_file()); assert!(!id2_metadata_file.is_file()); } + + /// Helper to write a conversation to disk in the expected storage layout. + /// + /// Creates `{storage}/conversations/{id}/metadata.json` and + /// `{storage}/conversations/{id}/events.json`. + fn write_conversation_to_disk( + storage: &Utf8Path, + id: &ConversationId, + conversation: &Conversation, + ) { + let conv_dir = storage.join(CONVERSATIONS_DIR).join(id.to_dirname(None)); + fs::create_dir_all(&conv_dir).unwrap(); + write_json(&conv_dir.join(METADATA_FILE), conversation).unwrap(); + + let stream = ConversationStream::new_test(); + write_json(&conv_dir.join("events.json"), &stream).unwrap(); + } + + /// Write a `conversations/metadata.json` pointing to the given active ID. + fn write_conversations_metadata_to_disk(storage: &Utf8Path, active_id: &ConversationId) { + let meta_path = storage.join(CONVERSATIONS_DIR).join(METADATA_FILE); + let meta = ConversationsMetadata::new(*active_id); + + write_json(&meta_path, &meta).unwrap(); + } }