From 8d2be3b495b615788d09b28380ca3f7933e054ed Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 29 Sep 2021 02:19:57 +0000 Subject: [PATCH] add get_version to server storage api --- sync-server/src/storage/inmemory.rs | 119 ++++++++++++++++++++++++++-- sync-server/src/storage/mod.rs | 7 ++ sync-server/src/storage/sqlite.rs | 80 +++++++++++++------ 3 files changed, 173 insertions(+), 33 deletions(-) diff --git a/sync-server/src/storage/inmemory.rs b/sync-server/src/storage/inmemory.rs index 143b2e760..7aef5f98c 100644 --- a/sync-server/src/storage/inmemory.rs +++ b/sync-server/src/storage/inmemory.rs @@ -6,8 +6,11 @@ struct Inner { /// Clients, indexed by client_key clients: HashMap, - /// Versions, indexed by (client_key, parent_version_id) + /// Versions, indexed by (client_key, version_id) versions: HashMap<(Uuid, Uuid), Version>, + + /// Child versions, indexed by (client_key, parent_version_id) + children: HashMap<(Uuid, Uuid), Uuid>, } pub struct InMemoryStorage(Mutex); @@ -18,6 +21,7 @@ impl InMemoryStorage { Self(Mutex::new(Inner { clients: HashMap::new(), versions: HashMap::new(), + children: HashMap::new(), })) } } @@ -66,11 +70,23 @@ impl<'a> StorageTxn for InnerTxn<'a> { client_key: Uuid, parent_version_id: Uuid, ) -> anyhow::Result> { - Ok(self - .0 - .versions - .get(&(client_key, parent_version_id)) - .cloned()) + if let Some(parent_version_id) = self.0.children.get(&(client_key, parent_version_id)) { + Ok(self + .0 + .versions + .get(&(client_key, *parent_version_id)) + .cloned()) + } else { + Ok(None) + } + } + + fn get_version( + &mut self, + client_key: Uuid, + version_id: Uuid, + ) -> anyhow::Result> { + Ok(self.0.versions.get(&(client_key, version_id)).cloned()) } fn add_version( @@ -86,9 +102,12 @@ impl<'a> StorageTxn for InnerTxn<'a> { parent_version_id, history_segment, }; + self.0 + .children + .insert((client_key, version.parent_version_id), version.version_id); self.0 .versions - .insert((client_key, version.parent_version_id), version); + .insert((client_key, version.version_id), version); Ok(()) } @@ -96,3 +115,89 @@ impl<'a> StorageTxn for InnerTxn<'a> { Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_emtpy_dir() -> anyhow::Result<()> { + let storage = InMemoryStorage::new(); + let mut txn = storage.txn()?; + let maybe_client = txn.get_client(Uuid::new_v4())?; + assert!(maybe_client.is_none()); + Ok(()) + } + + #[test] + fn test_get_client_empty() -> anyhow::Result<()> { + let storage = InMemoryStorage::new(); + let mut txn = storage.txn()?; + let maybe_client = txn.get_client(Uuid::new_v4())?; + assert!(maybe_client.is_none()); + Ok(()) + } + + #[test] + fn test_client_storage() -> anyhow::Result<()> { + let storage = InMemoryStorage::new(); + let mut txn = storage.txn()?; + + let client_key = Uuid::new_v4(); + let latest_version_id = Uuid::new_v4(); + txn.new_client(client_key, latest_version_id)?; + + let client = txn.get_client(client_key)?.unwrap(); + assert_eq!(client.latest_version_id, latest_version_id); + + let latest_version_id = Uuid::new_v4(); + txn.set_client_latest_version_id(client_key, latest_version_id)?; + + let client = txn.get_client(client_key)?.unwrap(); + assert_eq!(client.latest_version_id, latest_version_id); + + Ok(()) + } + + #[test] + fn test_gvbp_empty() -> anyhow::Result<()> { + let storage = InMemoryStorage::new(); + let mut txn = storage.txn()?; + let maybe_version = txn.get_version_by_parent(Uuid::new_v4(), Uuid::new_v4())?; + assert!(maybe_version.is_none()); + Ok(()) + } + + #[test] + fn test_add_version_and_get_version() -> anyhow::Result<()> { + let storage = InMemoryStorage::new(); + let mut txn = storage.txn()?; + + let client_key = Uuid::new_v4(); + let version_id = Uuid::new_v4(); + let parent_version_id = Uuid::new_v4(); + let history_segment = b"abc".to_vec(); + txn.add_version( + client_key, + version_id, + parent_version_id, + history_segment.clone(), + )?; + + let expected = Version { + version_id, + parent_version_id, + history_segment, + }; + + let version = txn + .get_version_by_parent(client_key, parent_version_id)? + .unwrap(); + assert_eq!(version, expected); + + let version = txn.get_version(client_key, version_id)?.unwrap(); + assert_eq!(version, expected); + + Ok(()) + } +} diff --git a/sync-server/src/storage/mod.rs b/sync-server/src/storage/mod.rs index 30b961fa1..121999892 100644 --- a/sync-server/src/storage/mod.rs +++ b/sync-server/src/storage/mod.rs @@ -43,6 +43,13 @@ pub trait StorageTxn { parent_version_id: Uuid, ) -> anyhow::Result>; + /// Get a version, indexed by its own version id + fn get_version( + &mut self, + client_key: Uuid, + version_id: Uuid, + ) -> anyhow::Result>; + /// Add a version (that must not already exist) fn add_version( &mut self, diff --git a/sync-server/src/storage/sqlite.rs b/sync-server/src/storage/sqlite.rs index 0d4f694b1..e1310a55c 100644 --- a/sync-server/src/storage/sqlite.rs +++ b/sync-server/src/storage/sqlite.rs @@ -71,6 +71,7 @@ impl SqliteStorage { let queries = vec![ "CREATE TABLE IF NOT EXISTS clients (client_key STRING PRIMARY KEY, latest_version_id STRING);", "CREATE TABLE IF NOT EXISTS versions (version_id STRING PRIMARY KEY, client_key STRING, parent_version_id STRING, history_segment BLOB);", + "CREATE INDEX IF NOT EXISTS versions_by_parent ON versions (parent_version_id);", ]; for q in queries { txn.execute(q, []).context("Creating table")?; @@ -100,6 +101,34 @@ impl Txn { .transaction() .map_err(|_e| SqliteError::CreateTransactionFailed) } + + /// Implementation for queries from the versions table + fn get_version_impl( + &mut self, + query: &'static str, + client_key: Uuid, + version_id_arg: Uuid, + ) -> anyhow::Result> { + let t = self.get_txn()?; + let r = t + .query_row( + query, + params![&StoredUuid(version_id_arg), &StoredUuid(client_key)], + |r| { + let version_id: StoredUuid = r.get("version_id")?; + let parent_version_id: StoredUuid = r.get("parent_version_id")?; + + Ok(Version { + version_id: version_id.0, + parent_version_id: parent_version_id.0, + history_segment: r.get("history_segment")?, + }) + }, + ) + .optional() + .context("Get version query")?; + Ok(r) + } } impl StorageTxn for Txn { @@ -148,24 +177,20 @@ impl StorageTxn for Txn { client_key: Uuid, parent_version_id: Uuid, ) -> anyhow::Result> { - let t = self.get_txn()?; - let r = t.query_row( + self.get_version_impl( "SELECT version_id, parent_version_id, history_segment FROM versions WHERE parent_version_id = ? AND client_key = ?", - params![&StoredUuid(parent_version_id), &StoredUuid(client_key)], - |r| { - let version_id: StoredUuid = r.get("version_id")?; - let parent_version_id: StoredUuid = r.get("parent_version_id")?; - - Ok(Version{ - version_id: version_id.0, - parent_version_id: parent_version_id.0, - history_segment: r.get("history_segment")?, - })} - ) - .optional() - .context("Get version query") - ?; - Ok(r) + client_key, + parent_version_id) + } + fn get_version( + &mut self, + client_key: Uuid, + version_id: Uuid, + ) -> anyhow::Result> { + self.get_version_impl( + "SELECT version_id, parent_version_id, history_segment FROM versions WHERE version_id = ? AND client_key = ?", + client_key, + version_id) } fn add_version( @@ -260,7 +285,7 @@ mod test { } #[test] - fn test_add_version_and_gvbp() -> anyhow::Result<()> { + fn test_add_version_and_get_version() -> anyhow::Result<()> { let tmp_dir = TempDir::new()?; let storage = SqliteStorage::new(&tmp_dir.path())?; let mut txn = storage.txn()?; @@ -275,18 +300,21 @@ mod test { parent_version_id, history_segment.clone(), )?; + + let expected = Version { + version_id, + parent_version_id, + history_segment, + }; + let version = txn .get_version_by_parent(client_key, parent_version_id)? .unwrap(); + assert_eq!(version, expected); + + let version = txn.get_version(client_key, version_id)?.unwrap(); + assert_eq!(version, expected); - assert_eq!( - version, - Version { - version_id, - parent_version_id, - history_segment, - } - ); Ok(()) } }