diff --git a/src/actor.rs b/src/actor.rs index a004b067..c9879eac 100644 --- a/src/actor.rs +++ b/src/actor.rs @@ -55,6 +55,23 @@ enum Action { #[debug("reply")] reply: oneshot::Sender>, }, + #[display("DefaultAuthor")] + DefaultAuthor { + #[debug("reply")] + reply: oneshot::Sender>>, + }, + #[display("InitializeDefaultAuthor")] + InitializeDefaultAuthor { + author: Author, + #[debug("reply")] + reply: oneshot::Sender>, + }, + #[display("SetDefaultAuthor")] + SetDefaultAuthor { + author: AuthorId, + #[debug("reply")] + reply: oneshot::Sender>, + }, #[display("NewReplica")] ImportNamespace { capability: Capability, @@ -555,6 +572,26 @@ impl SyncHandle { rx.await? } + pub(crate) async fn default_author(&self) -> Result> { + let (reply, rx) = oneshot::channel(); + self.send(Action::DefaultAuthor { reply }).await?; + rx.await? + } + + pub(crate) async fn initialize_default_author(&self, author: Author) -> Result { + let (reply, rx) = oneshot::channel(); + self.send(Action::InitializeDefaultAuthor { author, reply }) + .await?; + rx.await? + } + + pub(crate) async fn set_default_author(&self, author: AuthorId) -> Result<()> { + let (reply, rx) = oneshot::channel(); + self.send(Action::SetDefaultAuthor { author, reply }) + .await?; + rx.await? + } + pub async fn import_namespace(&self, capability: Capability) -> Result { let (reply, rx) = oneshot::channel(); self.send(Action::ImportNamespace { capability, reply }) @@ -734,6 +771,13 @@ impl Actor { Action::DeleteAuthor { author, reply } => { send_reply(reply, self.store.delete_author(author)) } + Action::DefaultAuthor { reply } => send_reply(reply, self.store.default_author()), + Action::InitializeDefaultAuthor { author, reply } => { + send_reply(reply, self.store.initialize_default_author(author)) + } + Action::SetDefaultAuthor { author, reply } => { + send_reply(reply, self.store.set_default_author(author)) + } Action::ImportNamespace { capability, reply } => send_reply_with(reply, self, |this| { let id = capability.id(); let outcome = this.store.import_namespace(capability.clone())?; diff --git a/src/api.rs b/src/api.rs index fa505959..d5efffd6 100644 --- a/src/api.rs +++ b/src/api.rs @@ -132,8 +132,7 @@ impl DocsApi { /// Returns the default document author of this node. /// - /// On persistent nodes, the author is created on first start and its public key is saved - /// in the data directory. + /// On persistent nodes, the author is created on first start and saved in the docs database. /// /// The default author can be set with [`Self::author_set_default`]. pub async fn author_default(&self) -> Result { @@ -145,8 +144,8 @@ impl DocsApi { /// /// If the author does not exist, an error is returned. /// - /// On a persistent node, the author id will be saved to a file in the data directory and - /// reloaded after a restart. + /// On a persistent node, the author id is saved in the docs database and reloaded after a + /// restart. pub async fn author_set_default(&self, author_id: AuthorId) -> Result<()> { self.inner .rpc(AuthorSetDefaultRequest { author_id }) diff --git a/src/engine.rs b/src/engine.rs index 88013f16..621ca630 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,7 +4,7 @@ use std::sync::{Arc, RwLock}; -use anyhow::{bail, Result}; +use anyhow::Result; use iroh::{Endpoint, EndpointAddr, PublicKey}; use iroh_blobs::{ api::{blobs::BlobStatus, downloader::Downloader, Store}, @@ -66,7 +66,6 @@ impl Engine { replica_store: crate::store::Store, bao_store: iroh_blobs::api::Store, downloader: Downloader, - default_author_storage: DefaultAuthorStorage, protect_cb: Option, ) -> anyhow::Result { let (live_actor_tx, to_live_actor_recv) = mpsc::channel(ACTOR_CHANNEL_CAP); @@ -132,7 +131,7 @@ impl Engine { .instrument(error_span!("sync", %me)), ); - let default_author = match DefaultAuthor::load(default_author_storage, &sync).await { + let default_author = match DefaultAuthor::load(&sync).await { Ok(author) => author, Err(err) => { // If loading the default author failed, make sure to shutdown the sync actor before @@ -338,116 +337,26 @@ impl LiveEvent { } } -/// Where to persist the default author. -/// -/// If set to `Mem`, a new author will be created in the docs store before spawning the sync -/// engine. Changing the default author will not be persisted. -/// -/// If set to `Persistent`, the default author will be loaded from and persisted to the specified -/// path (as hex encoded string of the author's public key). -#[derive(Debug)] -pub enum DefaultAuthorStorage { - /// Memory storage. - Mem, - /// File based persistent storage. - #[cfg(feature = "fs-store")] - Persistent(std::path::PathBuf), -} - -impl DefaultAuthorStorage { - /// Load the default author from the storage. - /// - /// Will create and save a new author if the storage is empty. - /// - /// Returns an error if the author can't be parsed or if the uathor does not exist in the docs - /// store. - pub async fn load(&self, docs_store: &SyncHandle) -> anyhow::Result { - match self { - Self::Mem => { - let author = Author::new(&mut rand::rng()); - let author_id = author.id(); - docs_store.import_author(author).await?; - Ok(author_id) - } - #[cfg(feature = "fs-store")] - Self::Persistent(ref path) => { - use std::str::FromStr; - - use anyhow::Context; - if path.exists() { - let data = tokio::fs::read_to_string(path).await.with_context(|| { - format!( - "Failed to read the default author file at `{}`", - path.to_string_lossy() - ) - })?; - let author_id = AuthorId::from_str(&data).with_context(|| { - format!( - "Failed to parse the default author from `{}`", - path.to_string_lossy() - ) - })?; - if docs_store.export_author(author_id).await?.is_none() { - bail!( - "The default author is missing from the docs store. To recover, delete the file `{}`. Then iroh will create a new default author.", - path.to_string_lossy() - ) - } - Ok(author_id) - } else { - let author = Author::new(&mut rand::rng()); - let author_id = author.id(); - docs_store.import_author(author).await?; - // Make sure to write the default author to the store - // *before* we write the default author ID file. - // Otherwise the default author ID file is effectively a dangling reference. - docs_store.flush_store().await?; - self.persist(author_id).await?; - Ok(author_id) - } - } - } - } - - /// Save a new default author. - pub async fn persist(&self, #[allow(unused)] author_id: AuthorId) -> anyhow::Result<()> { - match self { - Self::Mem => { - // persistence is not possible for the mem storage so this is a noop. - } - #[cfg(feature = "fs-store")] - Self::Persistent(ref path) => { - use anyhow::Context; - tokio::fs::write(path, author_id.to_string()) - .await - .with_context(|| { - format!( - "Failed to write the default author to `{}`", - path.to_string_lossy() - ) - })?; - } - } - Ok(()) - } -} - /// Persistent default author for a docs engine. #[derive(Debug)] pub struct DefaultAuthor { value: RwLock, - storage: DefaultAuthorStorage, } impl DefaultAuthor { /// Load the default author from storage. /// /// If the storage is empty creates a new author and persists it. - pub async fn load(storage: DefaultAuthorStorage, docs_store: &SyncHandle) -> Result { - let value = storage.load(docs_store).await?; + pub async fn load(docs_store: &SyncHandle) -> Result { + let value = match docs_store.default_author().await? { + Some(author) => author, + None => { + let author = Author::new(&mut rand::rng()); + docs_store.initialize_default_author(author).await? + } + }; Ok(Self { value: RwLock::new(value), - storage, }) } @@ -458,10 +367,7 @@ impl DefaultAuthor { /// Set the default author. pub async fn set(&self, author_id: AuthorId, docs_store: &SyncHandle) -> Result<()> { - if docs_store.export_author(author_id).await?.is_none() { - bail!("The author does not exist"); - } - self.storage.persist(author_id).await?; + docs_store.set_default_author(author_id).await?; *self.value.write().unwrap() = author_id; Ok(()) } diff --git a/src/protocol.rs b/src/protocol.rs index 08ebb8bf..97dc7ccc 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -9,7 +9,7 @@ use iroh_gossip::net::Gossip; use crate::{ api::DocsApi, - engine::{DefaultAuthorStorage, Engine, ProtectCallbackHandler}, + engine::{Engine, ProtectCallbackHandler}, store::Store, }; @@ -104,18 +104,11 @@ impl Builder { blobs: BlobsStore, gossip: Gossip, ) -> anyhow::Result { - let replica_store = match &self.storage { + let replica_store = match self.storage { Storage::Memory => Store::memory(), #[cfg(feature = "fs-store")] Storage::Persistent(path) => Store::persistent(path.join("docs.redb"))?, }; - let author_store = match &self.storage { - Storage::Memory => DefaultAuthorStorage::Mem, - #[cfg(feature = "fs-store")] - Storage::Persistent(path) => { - DefaultAuthorStorage::Persistent(path.join("default-author")) - } - }; let downloader = blobs.downloader(&endpoint); let engine = Engine::spawn( endpoint, @@ -123,7 +116,6 @@ impl Builder { replica_store, blobs, downloader, - author_store, self.protect_cb, ) .await?; diff --git a/src/store/fs.rs b/src/store/fs.rs index b9143f96..df6cf54e 100644 --- a/src/store/fs.rs +++ b/src/store/fs.rs @@ -8,7 +8,7 @@ use std::{ ops::Bound, }; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use iroh::{KeyParsingError, PublicKey}; use iroh_blobs::Hash; use n0_future::time::SystemTime; @@ -46,7 +46,7 @@ use self::{ ranges::RangeExt, tables::{ LatestPerAuthorKey, LatestPerAuthorValue, ReadOnlyTables, RecordsId, RecordsTable, - RecordsValue, Tables, TransactionAndTables, + RecordsValue, Tables, TransactionAndTables, DEFAULT_AUTHOR_KEY, }, }; @@ -99,6 +99,29 @@ fn open_database(path: &std::path::Path) -> Result { } } +#[cfg(feature = "fs-store")] +fn load_legacy_default_author(path: &std::path::Path) -> Result> { + use std::str::FromStr; + + use anyhow::Context; + + if !path.exists() { + return Ok(None); + } + let data = std::fs::read_to_string(path).with_context(|| { + format!( + "Failed to read the default author file at `{}`", + path.display() + ) + })?; + AuthorId::from_str(&data).map(Some).with_context(|| { + format!( + "Failed to parse the default author from `{}`", + path.display() + ) + }) +} + #[cfg(feature = "fs-store")] fn is_redb_v2_tuple_mismatch(err: &anyhow::Error) -> bool { err.chain().any(|e| { @@ -117,7 +140,7 @@ impl Store { fn memory_impl() -> Result { let db = Database::builder().create_with_backend(redb::backends::InMemoryBackend::new())?; - Self::new_impl(db) + Self::new_impl(db, || Ok(None)) } /// Create or open a store from a `path` to a database file. @@ -126,15 +149,18 @@ impl Store { #[cfg(feature = "fs-store")] pub fn persistent(path: impl AsRef) -> Result { let path = path.as_ref(); + let legacy_author_path = path.with_file_name("default-author"); let db = open_database(path)?; - match Self::new_impl(db) { + match Self::new_impl(db, || load_legacy_default_author(&legacy_author_path)) { Ok(store) => Ok(store), Err(err) if is_redb_v2_tuple_mismatch(&err) => { #[cfg(feature = "redb-v2-migration")] { info!("redb 2.x tuple format detected, running migration"); migrate_redb_v2_tuples::run(path)?; - Self::new_impl(open_database(path)?) + Self::new_impl(open_database(path)?, || { + load_legacy_default_author(&legacy_author_path) + }) } #[cfg(not(feature = "redb-v2-migration"))] { @@ -148,14 +174,17 @@ impl Store { } } - fn new_impl(db: redb::Database) -> Result { + fn new_impl( + db: redb::Database, + legacy_default_author: impl FnOnce() -> Result>, + ) -> Result { // Setup all tables let write_tx = db.begin_write()?; let _ = Tables::new(&write_tx)?; write_tx.commit()?; // Run database migrations - migrations::run_migrations(&db)?; + migrations::run_migrations(&db, legacy_default_author)?; Ok(Store { db, @@ -398,9 +427,58 @@ impl Store { }) } + pub(crate) fn default_author(&mut self) -> Result> { + let tables = self.tables()?; + let Some(value) = tables.config.get(DEFAULT_AUTHOR_KEY)? else { + return Ok(None); + }; + let author: AuthorId = (*value.value()).into(); + if tables.authors.get(author.as_bytes())?.is_none() { + bail!("The default author is missing from the docs store"); + } + Ok(Some(author)) + } + + pub(crate) fn initialize_default_author(&mut self, author: Author) -> Result { + let id = author.id(); + self.modify(|tables| { + if let Some(value) = tables.config.get(DEFAULT_AUTHOR_KEY)? { + let existing: AuthorId = (*value.value()).into(); + if tables.authors.get(existing.as_bytes())?.is_none() { + bail!("The default author is missing from the docs store"); + } + return Ok(existing); + } + tables.authors.insert(id.as_bytes(), &author.to_bytes())?; + tables.config.insert(DEFAULT_AUTHOR_KEY, id.as_bytes())?; + Ok(id) + }) + } + + pub(crate) fn set_default_author(&mut self, author: AuthorId) -> Result<()> { + self.modify(|tables| { + if tables.authors.get(author.as_bytes())?.is_none() { + bail!("The author does not exist"); + } + tables + .config + .insert(DEFAULT_AUTHOR_KEY, author.as_bytes())?; + Ok(()) + }) + } + /// Delete an author. + /// + /// Returns an error if the author is the current default author. Set a + /// different default author first. pub fn delete_author(&mut self, author: AuthorId) -> Result<()> { self.modify(|tables| { + if let Some(value) = tables.config.get(DEFAULT_AUTHOR_KEY)? { + let default: AuthorId = (*value.value()).into(); + if default == author { + bail!("The author is the default author and cannot be deleted"); + } + } tables.authors.remove(author.as_bytes())?; Ok(()) }) @@ -1287,6 +1365,21 @@ mod tests { Ok(()) } + #[test] + #[cfg(feature = "fs-store")] + fn test_default_author_in_database() -> Result<()> { + let dbfile = tempfile::NamedTempFile::new()?; + let mut store = Store::persistent(dbfile.path())?; + let author = Author::new(&mut rand::rng()); + let author_id = store.initialize_default_author(author)?; + store.flush()?; + drop(store); + + let mut store = Store::persistent(dbfile.path())?; + assert_eq!(store.default_author()?, Some(author_id)); + Ok(()) + } + #[test] #[cfg(feature = "fs-store")] fn test_no_migration_on_fresh_store() -> Result<()> { diff --git a/src/store/fs/migrations.rs b/src/store/fs/migrations.rs index e7cef40f..3dc2454e 100644 --- a/src/store/fs/migrations.rs +++ b/src/store/fs/migrations.rs @@ -5,23 +5,33 @@ use redb::{Database, ReadableTable, ReadableTableMetadata, TableHandle, WriteTra use tracing::{debug, info}; use super::tables::{ - LATEST_PER_AUTHOR_TABLE, NAMESPACES_TABLE, NAMESPACES_TABLE_V1, RECORDS_BY_KEY_TABLE, - RECORDS_TABLE, + AUTHORS_TABLE, CONFIG_TABLE, DEFAULT_AUTHOR_KEY, LATEST_PER_AUTHOR_TABLE, NAMESPACES_TABLE, + NAMESPACES_TABLE_V1, RECORDS_BY_KEY_TABLE, RECORDS_TABLE, }; -use crate::{Capability, NamespaceSecret}; +use crate::{AuthorId, Capability, NamespaceSecret}; /// Run all database migrations, if needed. -pub fn run_migrations(db: &Database) -> Result<()> { +/// +/// `legacy_default_author` is only invoked if the database does not already +/// contain a default author, so a stale legacy sidecar file cannot fail an +/// already-migrated database. +pub fn run_migrations( + db: &Database, + legacy_default_author: impl FnOnce() -> Result>, +) -> Result<()> { run_migration(db, migration_001_populate_latest_table)?; run_migration(db, migration_002_namespaces_populate_v2)?; run_migration(db, migration_003_namespaces_delete_v1)?; run_migration(db, migration_004_populate_by_key_index)?; + run_migration(db, |tx| { + migration_005_import_default_author(tx, legacy_default_author) + })?; Ok(()) } fn run_migration(db: &Database, f: F) -> Result<()> where - F: Fn(&WriteTransaction) -> Result, + F: FnOnce(&WriteTransaction) -> Result, { let name = std::any::type_name::(); let name = name.split("::").last().unwrap(); @@ -117,6 +127,26 @@ fn migration_003_namespaces_delete_v1(tx: &WriteTransaction) -> Result Result>, +) -> Result { + let mut config = tx.open_table(CONFIG_TABLE)?; + if config.get(DEFAULT_AUTHOR_KEY)?.is_some() { + return Ok(MigrateOutcome::Skip); + } + let Some(author) = legacy_default_author()? else { + return Ok(MigrateOutcome::Skip); + }; + let authors = tx.open_table(AUTHORS_TABLE)?; + anyhow::ensure!( + authors.get(author.as_bytes())?.is_some(), + "The default author is missing from the docs store" + ); + config.insert(DEFAULT_AUTHOR_KEY, author.as_bytes())?; + Ok(MigrateOutcome::Execute(1)) +} + /// migration 004: populate the by_key index table(which did not exist before) fn migration_004_populate_by_key_index(tx: &WriteTransaction) -> Result { let mut by_key_table = tx.open_table(RECORDS_BY_KEY_TABLE)?; diff --git a/src/store/fs/tables.rs b/src/store/fs/tables.rs index 34744162..e30f9bcf 100644 --- a/src/store/fs/tables.rs +++ b/src/store/fs/tables.rs @@ -15,6 +15,11 @@ use crate::PeerIdBytes; /// Value: `[u8; 32]` # Author pub const AUTHORS_TABLE: TableDefinition<&[u8; 32], &[u8; 32]> = TableDefinition::new("authors-1"); +pub const CONFIG_TABLE: TableDefinition<&str, &[u8; 32]> = TableDefinition::new("config-1"); + +/// Key in [`CONFIG_TABLE`] holding the id of the default author. +pub const DEFAULT_AUTHOR_KEY: &str = "default-author"; + /// Table: Namespaces v1 (replaced by Namespaces v2 in migration ) /// Key: `[u8; 32]` # NamespaceId /// Value: `[u8; 32]` # NamespaceSecret @@ -121,6 +126,7 @@ pub struct Tables<'tx> { pub namespace_peers: MultimapTable<'tx, &'static [u8; 32], (Nanos, &'static PeerIdBytes)>, pub download_policy: Table<'tx, &'static [u8; 32], &'static [u8]>, pub authors: Table<'tx, &'static [u8; 32], &'static [u8; 32]>, + pub config: Table<'tx, &'static str, &'static [u8; 32]>, } impl<'tx> Tables<'tx> { @@ -132,6 +138,7 @@ impl<'tx> Tables<'tx> { let namespace_peers = tx.open_multimap_table(NAMESPACE_PEERS_TABLE)?; let download_policy = tx.open_table(DOWNLOAD_POLICY_TABLE)?; let authors = tx.open_table(AUTHORS_TABLE)?; + let config = tx.open_table(CONFIG_TABLE)?; Ok(Self { records, records_by_key, @@ -140,6 +147,7 @@ impl<'tx> Tables<'tx> { namespace_peers, download_policy, authors, + config, }) } } @@ -154,6 +162,7 @@ pub struct ReadOnlyTables { pub namespace_peers: ReadOnlyMultimapTable<&'static [u8; 32], (Nanos, &'static PeerIdBytes)>, pub download_policy: ReadOnlyTable<&'static [u8; 32], &'static [u8]>, pub authors: ReadOnlyTable<&'static [u8; 32], &'static [u8; 32]>, + pub config: ReadOnlyTable<&'static str, &'static [u8; 32]>, tx: ReadTransaction, } @@ -166,6 +175,7 @@ impl ReadOnlyTables { let namespace_peers = tx.open_multimap_table(NAMESPACE_PEERS_TABLE)?; let download_policy = tx.open_table(DOWNLOAD_POLICY_TABLE)?; let authors = tx.open_table(AUTHORS_TABLE)?; + let config = tx.open_table(CONFIG_TABLE)?; Ok(Self { records, records_by_key, @@ -174,6 +184,7 @@ impl ReadOnlyTables { namespace_peers, download_policy, authors, + config, tx, }) } diff --git a/tests/client.rs b/tests/client.rs index 214d5597..7f6ba614 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -197,48 +197,6 @@ async fn test_default_author_persist() -> TestResult<()> { iroh.shutdown().await?; }; - // check that a new default author is created if the default author file is deleted - // manually. - let default_author = { - tokio::fs::remove_file(iroh_root.join("default-author")).await?; - let iroh = Node::persistent(iroh_root, empty_endpoint().await?) - .spawn() - .await?; - let author = iroh.docs().author_default().await?; - assert!(author != default_author); - assert!(iroh.docs().author_export(author).await?.is_some()); - assert!(iroh.docs().author_delete(author).await.is_err()); - iroh.shutdown().await?; - author - }; - - // check that the node fails to start if the default author is missing from the docs store. - { - let mut docs_store = iroh_docs::store::fs::Store::persistent(iroh_root.join("docs.redb"))?; - docs_store.delete_author(default_author)?; - docs_store.flush()?; - drop(docs_store); - let iroh = Node::persistent(iroh_root, empty_endpoint().await?) - .spawn() - .await; - assert!(iroh.is_err()); - - // somehow the blob store is not shutdown correctly (yet?) on macos. - // so we give it some time until we find a proper fix. - #[cfg(target_os = "macos")] - n0_future::time::sleep(std::time::Duration::from_secs(1)).await; - - tokio::fs::remove_file(iroh_root.join("default-author")).await?; - drop(iroh); - let iroh = Node::persistent(iroh_root, empty_endpoint().await?) - .spawn() - .await; - if let Err(cause) = iroh.as_ref() { - panic!("failed to start node: {cause:?}"); - } - iroh?.shutdown().await?; - } - // check that the default author can be set manually and is persisted. let default_author = { let iroh = Node::persistent(iroh_root, empty_endpoint().await?) @@ -260,3 +218,26 @@ async fn test_default_author_persist() -> TestResult<()> { Ok(()) } + +#[tokio::test] +#[cfg(feature = "fs-store")] +async fn test_default_author_sidecar_migration() -> TestResult<()> { + let root = tempfile::TempDir::new()?; + let database_path = root.path().join("docs.redb"); + let author = iroh_docs::Author::new(&mut rand::rng()); + let author_id = author.id(); + + let mut store = iroh_docs::store::fs::Store::persistent(&database_path)?; + store.import_author(author)?; + store.flush()?; + drop(store); + tokio::fs::write(root.path().join("default-author"), author_id.to_string()).await?; + + let iroh = Node::persistent(root.path(), empty_endpoint().await?) + .spawn() + .await?; + assert_eq!(iroh.docs().author_default().await?, author_id); + iroh.shutdown().await?; + + Ok(()) +}