diff --git a/Cargo.lock b/Cargo.lock index dfb58ff89..afbb6eb0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -937,6 +937,12 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "80115a2dfde04491e181c2440a39e4be26e52d9ca4e92bed213f65b94e0b8db1" +[[package]] +name = "difference" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" + [[package]] name = "digest" version = "0.8.1" @@ -961,6 +967,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "212d0f5754cb6769937f4501cc0e67f4f4483c8d2c3e1e922ee9edbe4ab4c7c0" +[[package]] +name = "downcast" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bb454f0228b18c7f4c3b0ebbee346ed9c52e7443b0999cd543ff3571205701d" + [[package]] name = "either" version = "1.6.1" @@ -1066,6 +1078,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1267f4ac4f343772758f7b1bdcbe767c218bbab93bb432acbf5162bbf85a6c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fnv" version = "1.0.7" @@ -1082,6 +1103,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fragile" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69a039c3498dc930fe810151a34ba0c1c70b02b8625035592e74432f678591f2" + [[package]] name = "fs_extra" version = "1.2.0" @@ -1822,6 +1849,7 @@ dependencies = [ "memmap", "milli", "mime", + "mockall", "once_cell", "oxidized-json-checker", "parking_lot", @@ -2023,6 +2051,33 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "mockall" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18d614ad23f9bb59119b8b5670a85c7ba92c5e9adf4385c81ea00c51c8be33d5" +dependencies = [ + "cfg-if 1.0.0", + "downcast", + "fragile", + "lazy_static", + "mockall_derive", + "predicates", + "predicates-tree", +] + +[[package]] +name = "mockall_derive" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5dd4234635bca06fc96c7368d038061e0aae1b00a764dc817e900dc974e3deea" +dependencies = [ + "cfg-if 1.0.0", + "proc-macro2 1.0.24", + "quote 1.0.9", + "syn 1.0.64", +] + [[package]] name = "net2" version = "0.2.37" @@ -2046,6 +2101,12 @@ dependencies = [ "libc", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "ntapi" version = "0.3.6" @@ -2329,6 +2390,35 @@ version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" +[[package]] +name = "predicates" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eeb433456c1a57cc93554dea3ce40b4c19c4057e41c55d4a0f3d84ea71c325aa" +dependencies = [ + "difference", + "float-cmp", + "normalize-line-endings", + "predicates-core", + "regex", +] + +[[package]] +name = "predicates-core" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57e35a3326b75e49aa85f5dc6ec15b41108cf5aee58eabb1f274dd18b73c2451" + +[[package]] +name = "predicates-tree" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15f553275e5721409451eb85e15fd9a860a6e5ab4496eb215987502b5f5391f2" +dependencies = [ + "predicates-core", + "treeline", +] + [[package]] name = "proc-macro-error" version = "1.0.4" @@ -3438,6 +3528,12 @@ dependencies = [ "tracing", ] +[[package]] +name = "treeline" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7f741b240f1a48843f9b8e0444fb55fb2a4ff67293b50a9179dfd5ea67f8d41" + [[package]] name = "trust-dns-proto" version = "0.19.7" diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml index 4b8ecd13d..93776269f 100644 --- a/meilisearch-http/Cargo.toml +++ b/meilisearch-http/Cargo.toml @@ -80,10 +80,11 @@ version = "0.18.1" [dev-dependencies] +actix-rt = "2.1.0" +assert-json-diff = { branch = "master", git = "https://github.com/qdequele/assert-json-diff" } +mockall = "0.9.1" serde_url_params = "0.2.0" tempdir = "0.3.7" -assert-json-diff = { branch = "master", git = "https://github.com/qdequele/assert-json-diff" } -actix-rt = "2.1.0" urlencoding = "1.1.1" [features] diff --git a/meilisearch-http/src/index_controller/index_actor/mod.rs b/meilisearch-http/src/index_controller/index_actor/mod.rs index dac7ef06c..e3e48add7 100644 --- a/meilisearch-http/src/index_controller/index_actor/mod.rs +++ b/meilisearch-http/src/index_controller/index_actor/mod.rs @@ -23,6 +23,9 @@ use actor::IndexActor; pub use handle_impl::IndexActorHandleImpl; +#[cfg(test)] +use mockall::automock; + pub type Result = std::result::Result; type UpdateResult = std::result::Result, Failed>; @@ -68,7 +71,8 @@ pub enum IndexError { #[async_trait::async_trait] -pub trait IndexActorHandle: Sync + Send + Clone { +#[cfg_attr(test, automock)] +pub trait IndexActorHandle { async fn create_index(&self, uuid: Uuid, primary_key: Option) -> Result; async fn update( &self, diff --git a/meilisearch-http/src/index_controller/snapshot.rs b/meilisearch-http/src/index_controller/snapshot.rs index afdcdaf23..998821d9f 100644 --- a/meilisearch-http/src/index_controller/snapshot.rs +++ b/meilisearch-http/src/index_controller/snapshot.rs @@ -7,9 +7,9 @@ use tokio::fs; use tokio::task::spawn_blocking; use tokio::time::sleep; -use crate::helpers::compression; use super::update_actor::UpdateActorHandle; use super::uuid_resolver::UuidResolverHandle; +use crate::helpers::compression; #[allow(dead_code)] pub struct SnapshotService { @@ -22,7 +22,7 @@ pub struct SnapshotService { impl SnapshotService where U: UpdateActorHandle, - R: UuidResolverHandle + R: UuidResolverHandle, { pub fn new( uuid_resolver_handle: R, @@ -39,7 +39,6 @@ where } pub async fn run(self) { - loop { sleep(self.snapshot_period).await; if let Err(e) = self.perform_snapshot().await { @@ -49,7 +48,7 @@ where } async fn perform_snapshot(&self) -> anyhow::Result<()> { - if self.snapshot_path.file_name().is_none() { + if !self.snapshot_path.is_file() { bail!("invalid snapshot file path"); } @@ -58,15 +57,21 @@ where fs::create_dir_all(&temp_snapshot_path).await?; - let uuids = self.uuid_resolver_handle.snapshot(temp_snapshot_path.clone()).await?; + let uuids = self + .uuid_resolver_handle + .snapshot(temp_snapshot_path.clone()) + .await?; if uuids.is_empty() { - return Ok(()) + return Ok(()); } let tasks = uuids .iter() - .map(|&uuid| self.update_handle.snapshot(uuid, temp_snapshot_path.clone())) + .map(|&uuid| { + self.update_handle + .snapshot(uuid, temp_snapshot_path.clone()) + }) .collect::>(); futures::future::try_join_all(tasks).await?; @@ -75,7 +80,10 @@ where let temp_snapshot_file_clone = temp_snapshot_file.clone(); let temp_snapshot_path_clone = temp_snapshot_path.clone(); - spawn_blocking(move || compression::to_tar_gz(temp_snapshot_path_clone, temp_snapshot_file_clone)).await??; + spawn_blocking(move || { + compression::to_tar_gz(temp_snapshot_path_clone, temp_snapshot_file_clone) + }) + .await??; fs::rename(temp_snapshot_file, &self.snapshot_path).await?; @@ -84,3 +92,137 @@ where Ok(()) } } + +#[cfg(test)] +mod test { + use futures::future::{ok, err}; + use rand::Rng; + use tokio::time::timeout; + use uuid::Uuid; + + use super::*; + use crate::index_controller::update_actor::{MockUpdateActorHandle, UpdateError}; + use crate::index_controller::uuid_resolver::{MockUuidResolverHandle, UuidError}; + + #[actix_rt::test] + async fn test_normal() { + let mut rng = rand::thread_rng(); + let uuids_num = rng.gen_range(5, 10); + let uuids = (0..uuids_num).map(|_| Uuid::new_v4()).collect::>(); + + let mut uuid_resolver = MockUuidResolverHandle::new(); + let uuids_clone = uuids.clone(); + uuid_resolver + .expect_snapshot() + .times(1) + .returning(move |_| Box::pin(ok(uuids_clone.clone()))); + + let mut update_handle = MockUpdateActorHandle::new(); + let uuids_clone = uuids.clone(); + update_handle + .expect_snapshot() + .withf(move |uuid, _path| uuids_clone.contains(uuid)) + .times(uuids_num) + .returning(move |_, _| Box::pin(ok(()))); + + let snapshot_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let snapshot_service = SnapshotService::new( + uuid_resolver, + update_handle, + Duration::from_millis(100), + snapshot_path.path().to_owned(), + ); + + snapshot_service.perform_snapshot().await.unwrap(); + } + + #[actix_rt::test] + async fn bad_file_name() { + let uuid_resolver = MockUuidResolverHandle::new(); + let update_handle = MockUpdateActorHandle::new(); + + let snapshot_service = SnapshotService::new( + uuid_resolver, + update_handle, + Duration::from_millis(100), + "directory/".into(), + ); + + assert!(snapshot_service.perform_snapshot().await.is_err()); + } + + #[actix_rt::test] + async fn error_performing_uuid_snapshot() { + let mut uuid_resolver = MockUuidResolverHandle::new(); + uuid_resolver + .expect_snapshot() + .times(1) + // abitrary error + .returning(|_| Box::pin(err(UuidError::NameAlreadyExist))); + + let update_handle = MockUpdateActorHandle::new(); + + let snapshot_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let snapshot_service = SnapshotService::new( + uuid_resolver, + update_handle, + Duration::from_millis(100), + snapshot_path.path().to_owned(), + ); + + assert!(snapshot_service.perform_snapshot().await.is_err()); + // Nothing was written to the file + assert_eq!(snapshot_path.as_file().metadata().unwrap().len(), 0); + } + + #[actix_rt::test] + async fn error_performing_index_snapshot() { + let uuid = Uuid::new_v4(); + let mut uuid_resolver = MockUuidResolverHandle::new(); + uuid_resolver + .expect_snapshot() + .times(1) + .returning(move |_| Box::pin(ok(vec![uuid]))); + + let mut update_handle = MockUpdateActorHandle::new(); + update_handle + .expect_snapshot() + // abitrary error + .returning(|_, _| Box::pin(err(UpdateError::UnexistingUpdate(0)))); + + let snapshot_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let snapshot_service = SnapshotService::new( + uuid_resolver, + update_handle, + Duration::from_millis(100), + snapshot_path.path().to_owned(), + ); + + assert!(snapshot_service.perform_snapshot().await.is_err()); + // Nothing was written to the file + assert_eq!(snapshot_path.as_file().metadata().unwrap().len(), 0); + } + + #[actix_rt::test] + async fn test_loop() { + let mut uuid_resolver = MockUuidResolverHandle::new(); + uuid_resolver + .expect_snapshot() + // we expect the funtion to be called between 2 and 3 time in the given interval. + .times(2..4) + // abitrary error, to short-circuit the function + .returning(move |_| Box::pin(err(UuidError::NameAlreadyExist))); + + let update_handle = MockUpdateActorHandle::new(); + + let snapshot_path = tempfile::NamedTempFile::new_in(".").unwrap(); + let snapshot_service = SnapshotService::new( + uuid_resolver, + update_handle, + Duration::from_millis(100), + snapshot_path.path().to_owned(), + ); + + let _ = timeout(Duration::from_millis(300), snapshot_service.run()).await; + } +} diff --git a/meilisearch-http/src/index_controller/update_actor/actor.rs b/meilisearch-http/src/index_controller/update_actor/actor.rs index 629a87ba4..df949279e 100644 --- a/meilisearch-http/src/index_controller/update_actor/actor.rs +++ b/meilisearch-http/src/index_controller/update_actor/actor.rs @@ -23,7 +23,7 @@ impl UpdateActor where D: AsRef<[u8]> + Sized + 'static, S: UpdateStoreStore, - I: IndexActorHandle + 'static, + I: IndexActorHandle + Clone + Send + Sync + 'static, { pub fn new( store: S, diff --git a/meilisearch-http/src/index_controller/update_actor/handle_impl.rs b/meilisearch-http/src/index_controller/update_actor/handle_impl.rs index 43b2ff8c2..59f67fbe0 100644 --- a/meilisearch-http/src/index_controller/update_actor/handle_impl.rs +++ b/meilisearch-http/src/index_controller/update_actor/handle_impl.rs @@ -24,7 +24,7 @@ where update_store_size: usize, ) -> anyhow::Result where - I: IndexActorHandle + 'static, + I: IndexActorHandle + Clone + Send + Sync + 'static, { let path = path.as_ref().to_owned().join("updates"); let (sender, receiver) = mpsc::channel(100); diff --git a/meilisearch-http/src/index_controller/update_actor/mod.rs b/meilisearch-http/src/index_controller/update_actor/mod.rs index 740323cdc..1fb53a2b3 100644 --- a/meilisearch-http/src/index_controller/update_actor/mod.rs +++ b/meilisearch-http/src/index_controller/update_actor/mod.rs @@ -23,6 +23,9 @@ pub type Result = std::result::Result; type UpdateStore = update_store::UpdateStore; type PayloadData = std::result::Result>; +#[cfg(test)] +use mockall::automock; + #[derive(Debug, Error)] pub enum UpdateError { #[error("error with update: {0}")] @@ -34,6 +37,7 @@ pub enum UpdateError { } #[async_trait::async_trait] +#[cfg_attr(test, automock(type Data=Vec;))] pub trait UpdateActorHandle { type Data: AsRef<[u8]> + Sized + 'static + Sync + Send; diff --git a/meilisearch-http/src/index_controller/update_actor/store.rs b/meilisearch-http/src/index_controller/update_actor/store.rs index 9320c488f..676182a62 100644 --- a/meilisearch-http/src/index_controller/update_actor/store.rs +++ b/meilisearch-http/src/index_controller/update_actor/store.rs @@ -1,14 +1,14 @@ -use std::collections::HashMap; use std::collections::hash_map::Entry; +use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; -use uuid::Uuid; -use tokio::sync::RwLock; use tokio::fs; +use tokio::sync::RwLock; +use uuid::Uuid; +use super::{Result, UpdateError, UpdateStore}; use crate::index_controller::IndexActorHandle; -use super::{UpdateStore, UpdateError, Result}; #[async_trait::async_trait] pub trait UpdateStoreStore { @@ -25,11 +25,7 @@ pub struct MapUpdateStoreStore { } impl MapUpdateStoreStore { - pub fn new( - index_handle: I, - path: impl AsRef, - update_store_size: usize, - ) -> Self { + pub fn new(index_handle: I, path: impl AsRef, update_store_size: usize) -> Self { let db = Arc::new(RwLock::new(HashMap::new())); let path = path.as_ref().to_owned(); Self { @@ -42,7 +38,10 @@ impl MapUpdateStoreStore { } #[async_trait::async_trait] -impl UpdateStoreStore for MapUpdateStoreStore { +impl UpdateStoreStore for MapUpdateStoreStore +where + I: IndexActorHandle + Clone + Send + Sync + 'static, +{ async fn get_or_create(&self, uuid: Uuid) -> Result> { match self.db.write().await.entry(uuid) { Entry::Vacant(e) => { diff --git a/meilisearch-http/src/index_controller/uuid_resolver/mod.rs b/meilisearch-http/src/index_controller/uuid_resolver/mod.rs index 08cbe70e0..2361fc08b 100644 --- a/meilisearch-http/src/index_controller/uuid_resolver/mod.rs +++ b/meilisearch-http/src/index_controller/uuid_resolver/mod.rs @@ -12,6 +12,9 @@ use actor::UuidResolverActor; use message::UuidResolveMsg; use store::{HeedUuidStore, UuidStore}; +#[cfg(test)] +use mockall::automock; + pub use handle_impl::UuidResolverHandleImpl; const UUID_STORE_SIZE: usize = 1_073_741_824; //1GiB @@ -19,6 +22,7 @@ const UUID_STORE_SIZE: usize = 1_073_741_824; //1GiB pub type Result = std::result::Result; #[async_trait::async_trait] +#[cfg_attr(test, automock)] pub trait UuidResolverHandle { async fn resolve(&self, name: String) -> anyhow::Result; async fn get_or_create(&self, name: String) -> Result;