From 1363650ee69bbeb693b99204c22c8419a716f240 Mon Sep 17 00:00:00 2001 From: Vika Date: Tue, 10 May 2022 07:25:07 +0300 Subject: FileStorage: fixes and regression tests for read_feed_with_limit Now I will know if something breaks horribly again. --- Cargo.lock | 25 +++++-- Cargo.toml | 3 +- src/database/file/mod.rs | 8 ++- src/database/mod.rs | 174 ++++++++++++++++++++++++++++++++++++----------- 4 files changed, 161 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40a3401..5d955ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -818,6 +818,12 @@ dependencies = [ "syn", ] +[[package]] +name = "deunicode" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2c9736e15e7df1638a7f6eee92a6511615c738246a052af5ba86f039b65aede" + [[package]] name = "diff" version = "0.1.12" @@ -964,6 +970,17 @@ version = "2.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77f3309417938f28bf8228fcff79a4a37103981e3e186d2ccd19c74b38f4eb71" +[[package]] +name = "faker_rand" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300d2ddbf2245b5b5e723995e0961033121b4fc2be9045fb661af82bd739ffb6" +dependencies = [ + "deunicode", + "lazy_static", + "rand 0.8.5", +] + [[package]] name = "fastrand" version = "1.7.0" @@ -1572,6 +1589,7 @@ dependencies = [ "easy-scraper", "either", "env_logger 0.8.4", + "faker_rand", "futures", "futures-util", "http-types", @@ -1588,7 +1606,6 @@ dependencies = [ "mediatype", "mockito", "newbase60", - "paste", "prometheus", "rand 0.8.5", "redis", @@ -2097,12 +2114,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "paste" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c520e05135d6e763148b6426a837e239041653ba7becd2e538c076c738025fc" - [[package]] name = "percent-encoding" version = "2.1.0" diff --git a/Cargo.toml b/Cargo.toml index eaaac7b..90532eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,9 +39,10 @@ path = "./templates" [dev-dependencies] mockito = "^0.30.0" # HTTP mocking for Rust. tempdir = "^0.3.7" # A library for managing a temporary directory and deleting all contents when it's dropped -paste = "^1.0.5" # Macros for all your token pasting needs test-logger = "^0.1.0" # Simple helper to initialize env_logger before unit and integration tests httpmock = "^0.6" # HTTP mocking library that allows you to simulate responses from HTTP based services +faker_rand = "^0.1.1" # Seedable, rand-compatible generators of fake data +rand = "^0.8.5" # Utilities for random number generation [dependencies] async-trait = "^0.1.50" # Type erasure for async trait methods diff --git a/src/database/file/mod.rs b/src/database/file/mod.rs index 4a40f38..f9588f5 100644 --- a/src/database/file/mod.rs +++ b/src/database/file/mod.rs @@ -483,16 +483,20 @@ impl Storage for FileStorage { if let Some(feed) = self.get_post(url).await? { if let Some(mut feed) = filter_post(feed, user) { if feed["children"].is_array() { + // This code contains several clones. It looks + // like the borrow checker thinks it is preventing + // me from doing something incredibly stupid. The + // borrow checker may or may not be right. let children = feed["children"].as_array().unwrap().clone(); let mut posts_iter = children .into_iter() .map(|s: serde_json::Value| s.as_str().unwrap().to_string()); // Note: we can't actually use skip_while here because we end up emitting `after`. // This imperative snippet consumes after instead of emitting it, allowing the - // stream of posts to return only those items that truly come *after*. + // stream of posts to return only those items that truly come *after* if let Some(after) = after { for s in posts_iter.by_ref() { - if &s != after { + if &s == after { break } } diff --git a/src/database/mod.rs b/src/database/mod.rs index 5a1dd3f..b9a8652 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -257,10 +257,9 @@ pub trait Storage: std::fmt::Debug + Clone + Send + Sync { #[cfg(test)] mod tests { use super::{MicropubChannel, Storage}; - use paste::paste; use serde_json::json; - async fn test_backend_basic_operations(backend: Backend) { + async fn test_basic_operations(backend: Backend) { let post: serde_json::Value = json!({ "type": ["h-entry"], "properties": { @@ -324,7 +323,7 @@ mod tests { } /// Note: this is merely a smoke check and is in no way comprehensive. - async fn test_backend_update(backend: Backend) { + async fn test_update(backend: Backend) { let post: serde_json::Value = json!({ "type": ["h-entry"], "properties": { @@ -358,30 +357,33 @@ mod tests { .await .unwrap(); - if let Some(returned_post) = backend.get_post(&key).await.unwrap() { - assert!(returned_post.is_object()); - assert_eq!( - returned_post["type"].as_array().unwrap().len(), - post["type"].as_array().unwrap().len() - ); - assert_eq!( - returned_post["type"].as_array().unwrap(), - post["type"].as_array().unwrap() - ); - assert_eq!( - returned_post["properties"]["content"][0].as_str().unwrap(), - "Different test content" - ); - assert_eq!( - returned_post["properties"]["category"].as_array().unwrap(), - &vec![json!("testing")] - ); - } else { - panic!("For some reason the backend did not return the post.") + match backend.get_post(&key).await { + Ok(Some(returned_post)) => { + assert!(returned_post.is_object()); + assert_eq!( + returned_post["type"].as_array().unwrap().len(), + post["type"].as_array().unwrap().len() + ); + assert_eq!( + returned_post["type"].as_array().unwrap(), + post["type"].as_array().unwrap() + ); + assert_eq!( + returned_post["properties"]["content"][0].as_str().unwrap(), + "Different test content" + ); + assert_eq!( + returned_post["properties"]["category"].as_array().unwrap(), + &vec![json!("testing")] + ); + }, + something_else => { + something_else.expect("Shouldn't error").expect("Should have the post"); + } } } - async fn test_backend_get_channel_list(backend: Backend) { + async fn test_get_channel_list(backend: Backend) { let feed = json!({ "type": ["h-feed"], "properties": { @@ -406,7 +408,7 @@ mod tests { ); } - async fn test_backend_settings(backend: Backend) { + async fn test_settings(backend: Backend) { backend .set_setting(crate::database::Settings::SiteName, "https://fireburn.ru/", "Vika's Hideout") .await @@ -420,23 +422,117 @@ mod tests { ); } + fn gen_random_post(domain: &str) -> serde_json::Value { + use faker_rand::lorem::{Paragraphs, Word}; + + let uid = format!( + "https://{domain}/posts/{}-{}-{}", + rand::random::(), rand::random::(), rand::random::() + ); + + let post = json!({ + "type": ["h-entry"], + "properties": { + "content": [rand::random::().to_string()], + "uid": [&uid], + "url": [&uid] + } + }); + + post + } + + async fn test_feed_pagination(backend: Backend) { + let posts = std::iter::from_fn(|| Some(gen_random_post("fireburn.ru"))) + .take(20) + .collect::>(); + + let feed = json!({ + "type": ["h-feed"], + "properties": { + "name": ["Main Page"], + "author": ["https://fireburn.ru/"], + "uid": ["https://fireburn.ru/feeds/main"] + }, + "children": posts.iter() + .filter_map(|json| json["properties"]["uid"][0].as_str()) + .collect::>() + }); + let key = feed["properties"]["uid"][0].as_str().unwrap(); + + backend + .put_post(&feed, "https://fireburn.ru/") + .await + .unwrap(); + println!("---"); + for (i, post) in posts.iter().enumerate() { + backend.put_post(post, "https://fireburn.ru/").await.unwrap(); + println!("posts[{}] = {}", i, post["properties"]["uid"][0]); + } + println!("---"); + let limit: usize = 10; + let result = backend.read_feed_with_limit(key, &None, limit, &None) + .await + .unwrap() + .unwrap(); + for (i, post) in result["children"].as_array().unwrap().iter().enumerate() { + println!("feed[0][{}] = {}", i, post["properties"]["uid"][0]); + } + println!("---"); + assert_eq!(result["children"].as_array().unwrap()[0..10], posts[0..10]); + + let result2 = backend.read_feed_with_limit( + key, + &result["children"] + .as_array() + .unwrap() + .last() + .unwrap() + ["properties"]["uid"][0] + .as_str() + .map(|i| i.to_owned()), + limit, &None + ).await.unwrap().unwrap(); + for (i, post) in result2["children"].as_array().unwrap().iter().enumerate() { + println!("feed[1][{}] = {}", i, post["properties"]["uid"][0]); + } + println!("---"); + assert_eq!(result2["children"].as_array().unwrap()[0..10], posts[10..20]); + + // Regression test for #4 + let nonsense_after = Some("1010101010".to_owned()); + let result3 = tokio::time::timeout(tokio::time::Duration::from_secs(10), async move { + backend.read_feed_with_limit( + key, &nonsense_after, limit, &None + ).await.unwrap().unwrap() + }).await.expect("Operation should not hang: see https://gitlab.com/kittybox/kittybox/-/issues/4"); + assert!(result3["children"].as_array().unwrap().is_empty()); + } + + /// Automatically generates a test suite for + macro_rules! test_all { + ($func_name:ident, $mod_name:ident) => { + mod $mod_name { + $func_name!(test_basic_operations); + $func_name!(test_get_channel_list); + $func_name!(test_settings); + $func_name!(test_update); + $func_name!(test_feed_pagination); + } + } + } macro_rules! file_test { - ($func_name:expr) => { - paste! { - #[tokio::test] - async fn [] () { - test_logger::ensure_env_logger_initialized(); - let tempdir = tempdir::TempDir::new("file").expect("Failed to create tempdir"); - let backend = super::FileStorage::new(tempdir.into_path()).await.unwrap(); - $func_name(backend).await - } + ($func_name:ident) => { + #[tokio::test] + async fn $func_name () { + test_logger::ensure_env_logger_initialized(); + let tempdir = tempdir::TempDir::new("file").expect("Failed to create tempdir"); + let backend = super::super::FileStorage::new(tempdir.into_path()).await.unwrap(); + super::$func_name(backend).await } }; } - - file_test!(test_backend_basic_operations); - file_test!(test_backend_get_channel_list); - file_test!(test_backend_settings); - file_test!(test_backend_update); + + test_all!(file_test, file); } -- cgit 1.4.1