From 73ad36bc15b32fe47abebf30417696685272ed95 Mon Sep 17 00:00:00 2001 From: Ellie Huxtable Date: Mon, 3 Jul 2023 08:42:34 +0100 Subject: [PATCH] Add tests, all passing --- Cargo.lock | 5 + Cargo.toml | 1 + atuin-client/Cargo.toml | 1 + atuin-client/src/record/sync.rs | 247 +++++++++++++++++++++++++++++++- atuin-common/Cargo.toml | 6 + atuin-common/src/record.rs | 6 +- 6 files changed, 259 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ffa0b0b..43f91ef4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -151,7 +151,12 @@ dependencies = [ "memchr", "minspan", "parse_duration", +<<<<<<< HEAD "rand 0.8.5", +======= + "pretty_assertions", + "rand", +>>>>>>> f788279b (Add tests, all passing) "regex", "reqwest", "rmp", diff --git a/Cargo.toml b/Cargo.toml index f2a77c62..017aa7e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ tokio = { version = "1", features = ["full"] } uuid = { version = "1.3", features = ["v4", "serde"] } whoami = "1.1.2" typed-builder = "0.14.0" +pretty_assertions = "1.3.0" [workspace.dependencies.reqwest] version = "0.11" diff --git a/atuin-client/Cargo.toml b/atuin-client/Cargo.toml index 6da679f8..e31f9dda 100644 --- a/atuin-client/Cargo.toml +++ b/atuin-client/Cargo.toml @@ -69,3 +69,4 @@ generic-array = { version = "0.14", optional = true, features = ["serde"] } [dev-dependencies] tokio = { version = "1", features = ["full"] } +pretty_assertions = { workspace = true } diff --git a/atuin-client/src/record/sync.rs b/atuin-client/src/record/sync.rs index 3eec7522..5b03b6de 100644 --- a/atuin-client/src/record/sync.rs +++ b/atuin-client/src/record/sync.rs @@ -1,16 +1,20 @@ -use atuin_common::record::RecordIndex; // do a sync :O use eyre::Result; use uuid::Uuid; +use super::store::Store; use crate::{api_client::Client, settings::Settings}; -use super::store::Store; +use atuin_common::record::{Diff, RecordIndex}; -pub async fn diff( - settings: &Settings, - store: &mut impl Store, -) -> Result> { +#[derive(Debug, Eq, PartialEq)] +pub enum Operation { + // Either upload or download until the tail matches the below + Upload { tail: Uuid, host: Uuid, tag: String }, + Download { tail: Uuid, host: Uuid, tag: String }, +} + +pub async fn diff(settings: &Settings, store: &mut impl Store) -> Result { let client = Client::new(&settings.sync_address, &settings.session_token)?; // First, build our own index @@ -23,3 +27,234 @@ pub async fn diff( Ok(diff) } + +// Take a diff, along with a local store, and resolve it into a set of operations. +// With the store as context, we can determine if a tail exists locally or not and therefore if it needs uploading or download. +// In theory this could be done as a part of the diffing stage, but it's easier to reason +// about and test this way +pub async fn operations(diff: Diff, store: &impl Store) -> Result> { + let mut operations = Vec::with_capacity(diff.len()); + + for i in diff { + let (host, tag, tail) = i; + + // First, try to fetch the tail + // If it exists locally, then that means we need to update the remote + // host until it has the same tail. Ie, upload. + // If it does not exist locally, that means remote is ahead of us. + // Therefore, we need to download until our local tail matches + let record = store.get(tail).await; + + let op = if let Ok(_) = record { + // if local has the ID, then we should find the actual tail of this + // store, so we know what we need to update the remote to. + let tail = store + .last(host, tag.as_str()) + .await? + .expect("failed to fetch last record, expected tag/host to exist"); + + // TODO(ellie) update the diffing so that it stores the context of the current tail + // that way, we can determine how much we need to upload. + // For now just keep uploading until tails match + + Operation::Upload { + tail: tail.id, + host, + tag, + } + } else { + Operation::Download { tail, host, tag } + }; + + operations.push(op); + } + + // sort them - purely so we have a stable testing order, and can rely on + // same input = same output + // We can sort by ID so long as we continue to use UUIDv7 or something + // with the same properties + + operations.sort_by_key(|op| match op { + Operation::Upload { tail, host, .. } => ("upload", host.clone(), tail.clone()), + Operation::Download { tail, host, .. } => ("download", host.clone(), tail.clone()), + }); + + Ok(operations) +} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use atuin_common::record::{Diff, Record, RecordIndex}; + use pretty_assertions::{assert_eq, assert_ne}; + use uuid::Uuid; + + use crate::record::{ + sqlite_store::SqliteStore, + store::Store, + sync::{self, Operation}, + }; + + fn test_record() -> Record { + Record::builder() + .host(atuin_common::utils::uuid_v7()) + .version("v1".into()) + .tag(atuin_common::utils::uuid_v7().simple().to_string()) + .data(vec![0, 1, 2, 3]) + .build() + } + + // Take a list of local records, and a list of remote records. + // Return the local database, and a diff of local/remote, ready to build + // ops + async fn build_test_diff( + local_records: Vec, + remote_records: Vec, + ) -> (SqliteStore, Diff) { + let local_store = SqliteStore::new(":memory:") + .await + .expect("failed to open in memory sqlite"); + let remote_store = SqliteStore::new(":memory:") + .await + .expect("failed to open in memory sqlite"); // "remote" + + for i in local_records { + local_store.push(&i).await.unwrap(); + } + + for i in remote_records { + remote_store.push(&i).await.unwrap(); + } + + let local_tails = local_store.tail_records().await.unwrap(); + let local_index = RecordIndex::from(local_tails); + + let remote_tails = remote_store.tail_records().await.unwrap(); + let remote_index = RecordIndex::from(remote_tails); + + let diff = local_index.diff(&remote_index); + + (local_store, diff) + } + + #[tokio::test] + async fn test_basic_diff() { + // a diff where local is ahead of remote. nothing else. + + let record = test_record(); + let (store, diff) = build_test_diff(vec![record.clone()], vec![]).await; + + assert_eq!(diff.len(), 1); + + let operations = sync::operations(diff, &store).await.unwrap(); + + assert_eq!(operations.len(), 1); + + assert_eq!( + operations[0], + Operation::Upload { + host: record.host, + tag: record.tag, + tail: record.id + } + ); + } + + #[tokio::test] + async fn build_two_way_diff() { + // a diff where local is ahead of remote for one, and remote for + // another. One upload, one download + + let shared_record = test_record(); + + let remote_ahead = test_record(); + let local_ahead = shared_record.new_child(vec![1, 2, 3]); + + let local = vec![shared_record.clone(), local_ahead.clone()]; // local knows about the already synced, and something newer in the same store + let remote = vec![shared_record.clone(), remote_ahead.clone()]; // remote knows about the already-synced, and one new record in a new store + + let (store, diff) = build_test_diff(local, remote).await; + let operations = sync::operations(diff, &store).await.unwrap(); + + assert_eq!(operations.len(), 2); + + assert_eq!( + operations, + vec![ + Operation::Download { + tail: remote_ahead.id, + host: remote_ahead.host, + tag: remote_ahead.tag, + }, + Operation::Upload { + tail: local_ahead.id, + host: local_ahead.host, + tag: local_ahead.tag, + }, + ] + ); + } + + #[tokio::test] + async fn build_complex_diff() { + // One shared, ahead but known only by remote + // One known only by local + // One known only by remote + + let shared_record = test_record(); + + let remote_known = test_record(); + let local_known = test_record(); + + let second_shared = test_record(); + let second_shared_remote_ahead = second_shared.new_child(vec![1, 2, 3]); + + let local_ahead = shared_record.new_child(vec![1, 2, 3]); + + let local = vec![ + shared_record.clone(), + second_shared.clone(), + local_known.clone(), + local_ahead.clone(), + ]; + + let remote = vec![ + shared_record.clone(), + second_shared.clone(), + second_shared_remote_ahead.clone(), + remote_known.clone(), + ]; // remote knows about the already-synced, and one new record in a new store + + let (store, diff) = build_test_diff(local, remote).await; + let operations = sync::operations(diff, &store).await.unwrap(); + + assert_eq!(operations.len(), 4); + + assert_eq!( + operations, + vec![ + Operation::Download { + tail: remote_known.id, + host: remote_known.host, + tag: remote_known.tag, + }, + Operation::Download { + tail: second_shared_remote_ahead.id, + host: second_shared.host, + tag: second_shared.tag, + }, + Operation::Upload { + tail: local_ahead.id, + host: local_ahead.host, + tag: local_ahead.tag, + }, + Operation::Upload { + tail: local_known.id, + host: local_known.host, + tag: local_known.tag, + }, + ] + ); + } +} diff --git a/atuin-common/Cargo.toml b/atuin-common/Cargo.toml index ead3df84..0f7676c4 100644 --- a/atuin-common/Cargo.toml +++ b/atuin-common/Cargo.toml @@ -17,7 +17,13 @@ serde = { workspace = true } uuid = { workspace = true } rand = { workspace = true } typed-builder = { workspace = true } +<<<<<<< HEAD eyre = { workspace = true } [dev-dependencies] pretty_assertions = "1.3.0" +======= + +[dev-dependencies] +pretty_assertions = { workspace = true } +>>>>>>> f788279b (Add tests, all passing) diff --git a/atuin-common/src/record.rs b/atuin-common/src/record.rs index a9959d13..34103ae6 100644 --- a/atuin-common/src/record.rs +++ b/atuin-common/src/record.rs @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize}; use typed_builder::TypedBuilder; use uuid::Uuid; +<<<<<<< HEAD #[derive(Clone, Debug, PartialEq)] pub struct DecryptedData(pub Vec); @@ -13,6 +14,9 @@ pub struct EncryptedData { pub data: String, pub content_encryption_key: String, } +======= +pub type Diff = Vec<(Uuid, String, Uuid)>; +>>>>>>> f788279b (Add tests, all passing) /// A single record stored inside of our local database #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)] @@ -125,7 +129,7 @@ impl RecordIndex { /// other machine has a different tail, it will be the differing tail. This is useful to /// check if the other index is ahead of us, or behind. /// If the other index does not have the (host, tag) pair, then the other value will be None. - pub fn diff(&self, other: &Self) -> Vec<(Uuid, String, Uuid)> { + pub fn diff(&self, other: &Self) -> Diff { let mut ret = Vec::new(); // First, we check if other has everything that self has