From 6d0319f40158628ef8e3426e4247fdf19ea5bd19 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 29 Oct 2021 14:58:36 +0300 Subject: [PATCH 01/10] get_all_spenders now uses pagination and AllSpendersResponse --- validator_worker/src/sentry_interface.rs | 47 ++++++++++++++++++------ 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index e36591b19..9af1c7523 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, time::Duration}; use chrono::{DateTime, Utc}; -use futures::future::{join_all, TryFutureExt}; +use futures::future::{join_all, try_join_all, TryFutureExt}; use reqwest::{Client, Method}; use slog::Logger; @@ -9,8 +9,8 @@ use primitives::{ adapter::Adapter, balances::{CheckedState, UncheckedState}, sentry::{ - AccountingResponse, EventAggregateResponse, LastApprovedResponse, SuccessResponse, - ValidatorMessageResponse, + AccountingResponse, AllSpendersResponse, EventAggregateResponse, LastApprovedResponse, + SuccessResponse, ValidatorMessageResponse, }, spender::Spender, util::ApiUrl, @@ -161,26 +161,51 @@ impl SentryApi { .map_err(Error::Request) } - // TODO: Pagination & use of `AllSpendersResponse` - pub async fn get_all_spenders( + pub async fn get_spenders_page( &self, - channel: ChannelId, - ) -> Result, Error> { + channel: &ChannelId, + page: u64, + ) -> Result { let url = self .whoami .url - .join(&format!("v5/channel/{}/spender/all", channel)) + .join(&format!("v5/channel/{}/spender/all?page={}", channel, page)) .expect("Should not error when creating endpoint"); - self.client + let first_page: AllSpendersResponse = self + .client .get(url) .bearer_auth(&self.whoami.token) .send() .await? - // TODO: Should be `AllSpendersResponse` and should have pagination! .json() .map_err(Error::Request) - .await + .await?; + + Ok(first_page) + } + + pub async fn get_all_spenders( + &self, + channel: ChannelId, + ) -> Result, Error> { + let first_page = self.get_spenders_page(&channel, 0).await?; + + if first_page.pagination.total_pages < 2 { + Ok(first_page.spenders) + } else { + let all: Vec = try_join_all( + (1..first_page.pagination.total_pages).map(|i| self.get_spenders_page(&channel, i)), + ) + .await?; + + let result_all: HashMap = std::iter::once(first_page) + .chain(all.into_iter()) + .flat_map(|p| p.spenders) + .collect(); + + Ok(result_all) + } } /// Get the accounting from Sentry From f972c118bad9fdc6e0dd17849cd951229704d510 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Mon, 1 Nov 2021 15:15:31 +0200 Subject: [PATCH 02/10] get_all_spenders test progress --- Cargo.lock | 1 + validator_worker/Cargo.toml | 3 + validator_worker/src/sentry_interface.rs | 143 +++++++++++++++++++++-- 3 files changed, 137 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00867c83b..72fe47128 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4035,6 +4035,7 @@ dependencies = [ "thiserror", "tokio", "toml", + "wiremock", ] [[package]] diff --git a/validator_worker/Cargo.toml b/validator_worker/Cargo.toml index 116e9ea9e..efb7e2a72 100644 --- a/validator_worker/Cargo.toml +++ b/validator_worker/Cargo.toml @@ -39,3 +39,6 @@ serde_urlencoded = "0.7" toml = "0.5" # CLI clap = "^2.33" + +[dev-dependencies] +wiremock = "0.5" diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index 9af1c7523..b2c89244f 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -9,8 +9,8 @@ use primitives::{ adapter::Adapter, balances::{CheckedState, UncheckedState}, sentry::{ - AccountingResponse, AllSpendersResponse, EventAggregateResponse, LastApprovedResponse, - SuccessResponse, ValidatorMessageResponse, + AccountingResponse, AllSpendersResponse, EventAggregateResponse, LastApprovedResponse, SuccessResponse, + ValidatorMessageResponse, }, spender::Spender, util::ApiUrl, @@ -161,19 +161,14 @@ impl SentryApi { .map_err(Error::Request) } - pub async fn get_spenders_page( - &self, - channel: &ChannelId, - page: u64, - ) -> Result { + pub async fn get_spenders_page(&self, channel: &ChannelId, page: u64) -> Result { let url = self .whoami .url .join(&format!("v5/channel/{}/spender/all?page={}", channel, page)) .expect("Should not error when creating endpoint"); - let first_page: AllSpendersResponse = self - .client + let first_page: AllSpendersResponse = self.client .get(url) .bearer_auth(&self.whoami.token) .send() @@ -195,7 +190,8 @@ impl SentryApi { Ok(first_page.spenders) } else { let all: Vec = try_join_all( - (1..first_page.pagination.total_pages).map(|i| self.get_spenders_page(&channel, i)), + (1..first_page.pagination.total_pages) + .map(|i| self.get_spenders_page(&channel, i)), ) .await?; @@ -402,3 +398,130 @@ pub mod campaigns { client.get(endpoint).send().await?.json().await } } + +#[cfg(test)] +mod test { + use super::*; + use wiremock::{ + matchers::{method, path}, + Mock, MockServer, ResponseTemplate, + }; + use adapter::DummyAdapter; + use primitives::{ + adapter::DummyAdapterOptions, + config::configuration, + spender::Spendable, + util::tests::{ + discard_logger, + prep_db::{ADDRESSES, DUMMY_VALIDATOR_LEADER, DUMMY_VALIDATOR_FOLLOWER, DUMMY_CAMPAIGN, IDS} + }, + Deposit, UnifiedNum + }; + use serde_json::json; + use std::str::FromStr; + + #[tokio::test] + async fn test_get_all_spenders() { + let server = MockServer::start().await; + + let mut validators = Validators::new(); + validators.insert(DUMMY_VALIDATOR_LEADER.id, Validator { + url: ApiUrl::from_str(&DUMMY_VALIDATOR_LEADER.url).expect("Should parse"), + token: AuthToken::default(), + }); + validators.insert(DUMMY_VALIDATOR_FOLLOWER.id, Validator { + url: ApiUrl::from_str(&DUMMY_VALIDATOR_FOLLOWER.url).expect("Should parse"), + token: AuthToken::default(), + }); + + let mut config = configuration("development", None).expect("Should get Config"); + config.spendable_find_limit = 2; + + let adapter = DummyAdapter::init( + DummyAdapterOptions { + dummy_identity: IDS["leader"], + dummy_auth: Default::default(), + dummy_auth_tokens: Default::default(), + }, + &config, + ); + let logger = discard_logger(); + + let sentry = SentryApi::init(adapter, logger, config, validators).expect("Should build sentry"); + + let mut first_page_response = HashMap::new(); + first_page_response.insert(ADDRESSES["user"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + first_page_response.insert(ADDRESSES["publisher"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + + let mut second_page_response = HashMap::new(); + second_page_response.insert(ADDRESSES["publisher2"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + second_page_response.insert(ADDRESSES["creator"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + + let mut third_page_response = HashMap::new(); + third_page_response.insert(ADDRESSES["tester"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + + let first_page_response = json!(first_page_response); + let second_page_response = json!(second_page_response); + let third_page_response = json!(third_page_response); + + Mock::given(method("GET")) + .and(path(format!("/v5/channel/{}/spender/all?page=0", DUMMY_CAMPAIGN.channel.id()))) + .respond_with(ResponseTemplate::new(200).set_body_json(&first_page_response)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path(format!("/v5/channel/{}/spender/all?page=1", DUMMY_CAMPAIGN.channel.id()))) + .respond_with(ResponseTemplate::new(200).set_body_json(&second_page_response)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path(format!("/v5/channel/{}/spender/all?page=2", DUMMY_CAMPAIGN.channel.id()))) + .respond_with(ResponseTemplate::new(200).set_body_json(&third_page_response)) + .mount(&server) + .await; + + // let expected_response = json!(vec![spendable_user, spendable_publisher, spendable_publisher2, spendable_creator, spendable_tester]); + let mut expected_response = HashMap::new(); + expected_response.insert(ADDRESSES["user"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + expected_response.insert(ADDRESSES["publisher"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + expected_response.insert(ADDRESSES["publisher2"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + expected_response.insert(ADDRESSES["creator"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + expected_response.insert(ADDRESSES["tester"], Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }); + + let res = sentry.get_all_spenders(DUMMY_CAMPAIGN.channel.id()).await.expect("should get response"); + + assert!(expected_response.len() == res.len() && expected_response.keys().all(|k| res.contains_key(k))) + } +} \ No newline at end of file From a63c4ddab9ce6ddda6898cf53633763b6178a9c9 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Thu, 4 Nov 2021 20:17:10 +0200 Subject: [PATCH 03/10] fixed tests + formatting --- validator_worker/src/sentry_interface.rs | 234 +++++++++++++++-------- 1 file changed, 149 insertions(+), 85 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index b2c89244f..caed9498d 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -9,8 +9,8 @@ use primitives::{ adapter::Adapter, balances::{CheckedState, UncheckedState}, sentry::{ - AccountingResponse, AllSpendersResponse, EventAggregateResponse, LastApprovedResponse, SuccessResponse, - ValidatorMessageResponse, + AccountingResponse, AllSpendersResponse, EventAggregateResponse, LastApprovedResponse, + SuccessResponse, ValidatorMessageResponse, }, spender::Spender, util::ApiUrl, @@ -161,14 +161,19 @@ impl SentryApi { .map_err(Error::Request) } - pub async fn get_spenders_page(&self, channel: &ChannelId, page: u64) -> Result { + pub async fn get_spenders_page( + &self, + channel: &ChannelId, + page: u64, + ) -> Result { let url = self .whoami .url .join(&format!("v5/channel/{}/spender/all?page={}", channel, page)) .expect("Should not error when creating endpoint"); - let first_page: AllSpendersResponse = self.client + let first_page: AllSpendersResponse = self + .client .get(url) .bearer_auth(&self.whoami.token) .send() @@ -190,8 +195,7 @@ impl SentryApi { Ok(first_page.spenders) } else { let all: Vec = try_join_all( - (1..first_page.pagination.total_pages) - .map(|i| self.get_spenders_page(&channel, i)), + (1..first_page.pagination.total_pages).map(|i| self.get_spenders_page(&channel, i)), ) .await?; @@ -402,10 +406,6 @@ pub mod campaigns { #[cfg(test)] mod test { use super::*; - use wiremock::{ - matchers::{method, path}, - Mock, MockServer, ResponseTemplate, - }; use adapter::DummyAdapter; use primitives::{ adapter::DummyAdapterOptions, @@ -413,115 +413,179 @@ mod test { spender::Spendable, util::tests::{ discard_logger, - prep_db::{ADDRESSES, DUMMY_VALIDATOR_LEADER, DUMMY_VALIDATOR_FOLLOWER, DUMMY_CAMPAIGN, IDS} + prep_db::{ + ADDRESSES, DUMMY_CAMPAIGN, DUMMY_VALIDATOR_FOLLOWER, DUMMY_VALIDATOR_LEADER, IDS, + }, }, - Deposit, UnifiedNum + Deposit, UnifiedNum, }; use serde_json::json; use std::str::FromStr; + use wiremock::{ + matchers::{method, path}, + Mock, MockServer, ResponseTemplate, + }; #[tokio::test] async fn test_get_all_spenders() { - let server = MockServer::start().await; + let listener = std::net::TcpListener::bind("localhost:8005").unwrap(); + let expected_server_address = listener + .local_addr() + .expect("Failed to get server address."); + let server = MockServer::builder().listener(listener).start().await; - let mut validators = Validators::new(); - validators.insert(DUMMY_VALIDATOR_LEADER.id, Validator { - url: ApiUrl::from_str(&DUMMY_VALIDATOR_LEADER.url).expect("Should parse"), - token: AuthToken::default(), - }); - validators.insert(DUMMY_VALIDATOR_FOLLOWER.id, Validator { - url: ApiUrl::from_str(&DUMMY_VALIDATOR_FOLLOWER.url).expect("Should parse"), - token: AuthToken::default(), - }); + // Verifying our server is running on the correct port + assert_eq!(&expected_server_address, server.address()); - let mut config = configuration("development", None).expect("Should get Config"); - config.spendable_find_limit = 2; - - let adapter = DummyAdapter::init( - DummyAdapterOptions { - dummy_identity: IDS["leader"], - dummy_auth: Default::default(), - dummy_auth_tokens: Default::default(), + let mut first_page_response = HashMap::new(); + first_page_response.insert( + ADDRESSES["user"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + first_page_response.insert( + ADDRESSES["publisher"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, }, - &config, ); - let logger = discard_logger(); - - let sentry = SentryApi::init(adapter, logger, config, validators).expect("Should build sentry"); - - let mut first_page_response = HashMap::new(); - first_page_response.insert(ADDRESSES["user"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - first_page_response.insert(ADDRESSES["publisher"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); let mut second_page_response = HashMap::new(); - second_page_response.insert(ADDRESSES["publisher2"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - second_page_response.insert(ADDRESSES["creator"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); + second_page_response.insert( + ADDRESSES["publisher2"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + second_page_response.insert( + ADDRESSES["creator"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); let mut third_page_response = HashMap::new(); - third_page_response.insert(ADDRESSES["tester"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); + third_page_response.insert( + ADDRESSES["tester"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); let first_page_response = json!(first_page_response); let second_page_response = json!(second_page_response); let third_page_response = json!(third_page_response); Mock::given(method("GET")) - .and(path(format!("/v5/channel/{}/spender/all?page=0", DUMMY_CAMPAIGN.channel.id()))) + .and(path(format!( + "/v5/channel/{}/spender/all?page=0", + DUMMY_CAMPAIGN.channel.id() + ))) .respond_with(ResponseTemplate::new(200).set_body_json(&first_page_response)) .mount(&server) .await; Mock::given(method("GET")) - .and(path(format!("/v5/channel/{}/spender/all?page=1", DUMMY_CAMPAIGN.channel.id()))) + .and(path(format!( + "/v5/channel/{}/spender/all?page=1", + DUMMY_CAMPAIGN.channel.id() + ))) .respond_with(ResponseTemplate::new(200).set_body_json(&second_page_response)) .mount(&server) .await; Mock::given(method("GET")) - .and(path(format!("/v5/channel/{}/spender/all?page=2", DUMMY_CAMPAIGN.channel.id()))) + .and(path(format!( + "/v5/channel/{}/spender/all?page=2", + DUMMY_CAMPAIGN.channel.id() + ))) .respond_with(ResponseTemplate::new(200).set_body_json(&third_page_response)) .mount(&server) .await; - // let expected_response = json!(vec![spendable_user, spendable_publisher, spendable_publisher2, spendable_creator, spendable_tester]); + let mut validators = Validators::new(); + validators.insert( + DUMMY_VALIDATOR_LEADER.id, + Validator { + url: ApiUrl::from_str(&DUMMY_VALIDATOR_LEADER.url).expect("Should parse"), + token: AuthToken::default(), + }, + ); + validators.insert( + DUMMY_VALIDATOR_FOLLOWER.id, + Validator { + url: ApiUrl::from_str(&DUMMY_VALIDATOR_FOLLOWER.url).expect("Should parse"), + token: AuthToken::default(), + }, + ); + + let mut config = configuration("development", None).expect("Should get Config"); + config.spendable_find_limit = 2; + + let adapter = DummyAdapter::init( + DummyAdapterOptions { + dummy_identity: IDS["leader"], + dummy_auth: Default::default(), + dummy_auth_tokens: Default::default(), + }, + &config, + ); + let logger = discard_logger(); + + let sentry = + SentryApi::init(adapter, logger, config, validators).expect("Should build sentry"); + let mut expected_response = HashMap::new(); - expected_response.insert(ADDRESSES["user"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - expected_response.insert(ADDRESSES["publisher"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - expected_response.insert(ADDRESSES["publisher2"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - expected_response.insert(ADDRESSES["creator"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - expected_response.insert(ADDRESSES["tester"], Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }); - - let res = sentry.get_all_spenders(DUMMY_CAMPAIGN.channel.id()).await.expect("should get response"); - - assert!(expected_response.len() == res.len() && expected_response.keys().all(|k| res.contains_key(k))) + expected_response.insert( + ADDRESSES["user"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + expected_response.insert( + ADDRESSES["publisher"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + expected_response.insert( + ADDRESSES["publisher2"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + expected_response.insert( + ADDRESSES["creator"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + expected_response.insert( + ADDRESSES["tester"], + Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }, + ); + + let res = sentry + .get_all_spenders(DUMMY_CAMPAIGN.channel.id()) + .await + .expect("should get response"); + + assert!( + expected_response.len() == res.len() + && expected_response.keys().all(|k| res.contains_key(k)) + ) } -} \ No newline at end of file +} From e7607ab0202853054bc7842732a548fdac87f790 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 5 Nov 2021 16:02:13 +0200 Subject: [PATCH 04/10] text is now running --- validator_worker/src/sentry_interface.rs | 61 +++++++++++++++--------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index caed9498d..d96c9a793 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -410,42 +410,48 @@ mod test { use primitives::{ adapter::DummyAdapterOptions, config::configuration, - spender::Spendable, + sentry::Pagination, util::tests::{ discard_logger, prep_db::{ ADDRESSES, DUMMY_CAMPAIGN, DUMMY_VALIDATOR_FOLLOWER, DUMMY_VALIDATOR_LEADER, IDS, }, }, - Deposit, UnifiedNum, + UnifiedNum, }; - use serde_json::json; use std::str::FromStr; use wiremock::{ - matchers::{method, path}, + matchers::{method, path, query_param}, Mock, MockServer, ResponseTemplate, }; #[tokio::test] async fn test_get_all_spenders() { + // We need to use a custom port as the request will fail on any other port let listener = std::net::TcpListener::bind("localhost:8005").unwrap(); let expected_server_address = listener .local_addr() .expect("Failed to get server address."); let server = MockServer::builder().listener(listener).start().await; - // Verifying our server is running on the correct port + // Verifying our server is running on the correct address and port assert_eq!(&expected_server_address, server.address()); - let mut first_page_response = HashMap::new(); - first_page_response.insert( + let mut first_page_response = AllSpendersResponse { + spenders: HashMap::new(), + pagination: Pagination { + page: 0, + total_pages: 3, + }, + }; + first_page_response.spenders.insert( ADDRESSES["user"], Spender { total_deposited: UnifiedNum::from(100_000_000), spender_leaf: None, }, ); - first_page_response.insert( + first_page_response.spenders.insert( ADDRESSES["publisher"], Spender { total_deposited: UnifiedNum::from(100_000_000), @@ -453,15 +459,21 @@ mod test { }, ); - let mut second_page_response = HashMap::new(); - second_page_response.insert( + let mut second_page_response = AllSpendersResponse { + spenders: HashMap::new(), + pagination: Pagination { + page: 1, + total_pages: 3, + }, + }; + second_page_response.spenders.insert( ADDRESSES["publisher2"], Spender { total_deposited: UnifiedNum::from(100_000_000), spender_leaf: None, }, ); - second_page_response.insert( + second_page_response.spenders.insert( ADDRESSES["creator"], Spender { total_deposited: UnifiedNum::from(100_000_000), @@ -469,8 +481,14 @@ mod test { }, ); - let mut third_page_response = HashMap::new(); - third_page_response.insert( + let mut third_page_response = AllSpendersResponse { + spenders: HashMap::new(), + pagination: Pagination { + page: 2, + total_pages: 3, + }, + }; + third_page_response.spenders.insert( ADDRESSES["tester"], Spender { total_deposited: UnifiedNum::from(100_000_000), @@ -478,33 +496,32 @@ mod test { }, ); - let first_page_response = json!(first_page_response); - let second_page_response = json!(second_page_response); - let third_page_response = json!(third_page_response); - Mock::given(method("GET")) .and(path(format!( - "/v5/channel/{}/spender/all?page=0", + "/v5/channel/{}/spender/all", DUMMY_CAMPAIGN.channel.id() ))) + .and(query_param("page", "0")) .respond_with(ResponseTemplate::new(200).set_body_json(&first_page_response)) .mount(&server) .await; Mock::given(method("GET")) .and(path(format!( - "/v5/channel/{}/spender/all?page=1", + "/v5/channel/{}/spender/all", DUMMY_CAMPAIGN.channel.id() ))) + .and(query_param("page", "1")) .respond_with(ResponseTemplate::new(200).set_body_json(&second_page_response)) .mount(&server) .await; Mock::given(method("GET")) .and(path(format!( - "/v5/channel/{}/spender/all?page=2", + "/v5/channel/{}/spender/all", DUMMY_CAMPAIGN.channel.id() ))) + .and(query_param("page", "2")) .respond_with(ResponseTemplate::new(200).set_body_json(&third_page_response)) .mount(&server) .await; @@ -584,8 +601,8 @@ mod test { .expect("should get response"); assert!( - expected_response.len() == res.len() + expected_response.keys().len() == res.keys().len() && expected_response.keys().all(|k| res.contains_key(k)) - ) + ); } } From c7279dcf5625bc4cc644aed5a9a2e2d85b5666d3 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 5 Nov 2021 18:00:52 +0200 Subject: [PATCH 05/10] Fixed CI checks --- validator_worker/src/sentry_interface.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index de10cb952..bd98bc620 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -410,7 +410,7 @@ mod test { use adapter::DummyAdapter; use primitives::{ adapter::DummyAdapterOptions, - config::configuration, + config::{configuration, Environment}, sentry::Pagination, util::tests::{ discard_logger, @@ -543,7 +543,7 @@ mod test { }, ); - let mut config = configuration("development", None).expect("Should get Config"); + let mut config = configuration(Environment::Development, None).expect("Should get Config"); config.spendable_find_limit = 2; let adapter = DummyAdapter::init( From c0eea9e348ad7731e1c633881f12d22f25d77734 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 5 Nov 2021 18:32:36 +0200 Subject: [PATCH 06/10] cargo fmt --- sentry/src/db.rs | 14 ++++++++++---- test_harness/src/lib.rs | 4 +++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry/src/db.rs b/sentry/src/db.rs index 9b64fcac3..7126d81a4 100644 --- a/sentry/src/db.rs +++ b/sentry/src/db.rs @@ -334,13 +334,19 @@ pub mod tests_postgres { // set the database in the configuration of the inside Pool (used for tests) config.dbname(&database.name); - let manager = - deadpool_postgres::Manager::from_config(config, NoTls, self.manager_config.clone()); - + let manager = deadpool_postgres::Manager::from_config( + config, + NoTls, + self.manager_config.clone(), + ); + deadpool_postgres::Pool::new(manager, 15) }; - let result = database.pool.get().await? + let result = database + .pool + .get() + .await? .simple_query(queries) .await .map_err(PoolError::Backend) diff --git a/test_harness/src/lib.rs b/test_harness/src/lib.rs index af12e22ec..cc1290332 100644 --- a/test_harness/src/lib.rs +++ b/test_harness/src/lib.rs @@ -730,7 +730,9 @@ pub mod run { let postgres = postgres_connection(42, postgres_config).await; let mut redis = redis_connection(app_config.redis_url).await?; - Manager::flush_db(&mut redis).await.expect("Should flush redis database"); + Manager::flush_db(&mut redis) + .await + .expect("Should flush redis database"); let campaign_remaining = CampaignRemaining::new(redis.clone()); From ae6d240049bd576097dba56e7057fb6972495067 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Mon, 8 Nov 2021 17:23:55 +0200 Subject: [PATCH 07/10] improvements to the test --- validator_worker/src/sentry_interface.rs | 85 +++++++----------------- 1 file changed, 24 insertions(+), 61 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index bd98bc620..b6fda0499 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -426,17 +426,15 @@ mod test { Mock, MockServer, ResponseTemplate, }; + #[tokio::test] async fn test_get_all_spenders() { - // We need to use a custom port as the request will fail on any other port - let listener = std::net::TcpListener::bind("localhost:8005").unwrap(); - let expected_server_address = listener - .local_addr() - .expect("Failed to get server address."); - let server = MockServer::builder().listener(listener).start().await; + let server = MockServer::start().await; - // Verifying our server is running on the correct address and port - assert_eq!(&expected_server_address, server.address()); + let test_spender = Spender { + total_deposited: UnifiedNum::from(100_000_000), + spender_leaf: None, + }; let mut first_page_response = AllSpendersResponse { spenders: HashMap::new(), @@ -447,17 +445,11 @@ mod test { }; first_page_response.spenders.insert( ADDRESSES["user"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); first_page_response.spenders.insert( ADDRESSES["publisher"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); let mut second_page_response = AllSpendersResponse { @@ -469,17 +461,11 @@ mod test { }; second_page_response.spenders.insert( ADDRESSES["publisher2"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); second_page_response.spenders.insert( ADDRESSES["creator"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); let mut third_page_response = AllSpendersResponse { @@ -491,10 +477,7 @@ mod test { }; third_page_response.spenders.insert( ADDRESSES["tester"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); Mock::given(method("GET")) @@ -531,18 +514,10 @@ mod test { validators.insert( DUMMY_VALIDATOR_LEADER.id, Validator { - url: ApiUrl::from_str(&DUMMY_VALIDATOR_LEADER.url).expect("Should parse"), + url: ApiUrl::from_str(&server.uri()).expect("Should parse"), token: AuthToken::default(), }, ); - validators.insert( - DUMMY_VALIDATOR_FOLLOWER.id, - Validator { - url: ApiUrl::from_str(&DUMMY_VALIDATOR_FOLLOWER.url).expect("Should parse"), - token: AuthToken::default(), - }, - ); - let mut config = configuration(Environment::Development, None).expect("Should get Config"); config.spendable_find_limit = 2; @@ -562,38 +537,23 @@ mod test { let mut expected_response = HashMap::new(); expected_response.insert( ADDRESSES["user"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); expected_response.insert( ADDRESSES["publisher"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); expected_response.insert( ADDRESSES["publisher2"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); expected_response.insert( ADDRESSES["creator"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); expected_response.insert( ADDRESSES["tester"], - Spender { - total_deposited: UnifiedNum::from(100_000_000), - spender_leaf: None, - }, + test_spender.clone(), ); let res = sentry @@ -601,9 +561,12 @@ mod test { .await .expect("should get response"); - assert!( - expected_response.keys().len() == res.keys().len() - && expected_response.keys().all(|k| res.contains_key(k)) - ); + assert!(expected_response.keys().len() == res.keys().len()); + // Is page 1 included + assert!(res.contains_key(&ADDRESSES["user"]) && res.contains_key(&ADDRESSES["publisher"])); + // Is page 2 included + assert!(res.contains_key(&ADDRESSES["publisher2"]) && res.contains_key(&ADDRESSES["creator"])); + // Is page 3 included + assert!(res.contains_key(&ADDRESSES["tester"])); } } From ea9333aeb8726fb4ad5d157d0df282c31e419f5e Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Mon, 8 Nov 2021 17:24:43 +0200 Subject: [PATCH 08/10] cargo fmt --- validator_worker/src/sentry_interface.rs | 65 +++++++++--------------- 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index b6fda0499..dd187863b 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -426,7 +426,6 @@ mod test { Mock, MockServer, ResponseTemplate, }; - #[tokio::test] async fn test_get_all_spenders() { let server = MockServer::start().await; @@ -443,14 +442,12 @@ mod test { total_pages: 3, }, }; - first_page_response.spenders.insert( - ADDRESSES["user"], - test_spender.clone(), - ); - first_page_response.spenders.insert( - ADDRESSES["publisher"], - test_spender.clone(), - ); + first_page_response + .spenders + .insert(ADDRESSES["user"], test_spender.clone()); + first_page_response + .spenders + .insert(ADDRESSES["publisher"], test_spender.clone()); let mut second_page_response = AllSpendersResponse { spenders: HashMap::new(), @@ -459,14 +456,12 @@ mod test { total_pages: 3, }, }; - second_page_response.spenders.insert( - ADDRESSES["publisher2"], - test_spender.clone(), - ); - second_page_response.spenders.insert( - ADDRESSES["creator"], - test_spender.clone(), - ); + second_page_response + .spenders + .insert(ADDRESSES["publisher2"], test_spender.clone()); + second_page_response + .spenders + .insert(ADDRESSES["creator"], test_spender.clone()); let mut third_page_response = AllSpendersResponse { spenders: HashMap::new(), @@ -475,10 +470,9 @@ mod test { total_pages: 3, }, }; - third_page_response.spenders.insert( - ADDRESSES["tester"], - test_spender.clone(), - ); + third_page_response + .spenders + .insert(ADDRESSES["tester"], test_spender.clone()); Mock::given(method("GET")) .and(path(format!( @@ -535,26 +529,11 @@ mod test { SentryApi::init(adapter, logger, config, validators).expect("Should build sentry"); let mut expected_response = HashMap::new(); - expected_response.insert( - ADDRESSES["user"], - test_spender.clone(), - ); - expected_response.insert( - ADDRESSES["publisher"], - test_spender.clone(), - ); - expected_response.insert( - ADDRESSES["publisher2"], - test_spender.clone(), - ); - expected_response.insert( - ADDRESSES["creator"], - test_spender.clone(), - ); - expected_response.insert( - ADDRESSES["tester"], - test_spender.clone(), - ); + expected_response.insert(ADDRESSES["user"], test_spender.clone()); + expected_response.insert(ADDRESSES["publisher"], test_spender.clone()); + expected_response.insert(ADDRESSES["publisher2"], test_spender.clone()); + expected_response.insert(ADDRESSES["creator"], test_spender.clone()); + expected_response.insert(ADDRESSES["tester"], test_spender.clone()); let res = sentry .get_all_spenders(DUMMY_CAMPAIGN.channel.id()) @@ -565,7 +544,9 @@ mod test { // Is page 1 included assert!(res.contains_key(&ADDRESSES["user"]) && res.contains_key(&ADDRESSES["publisher"])); // Is page 2 included - assert!(res.contains_key(&ADDRESSES["publisher2"]) && res.contains_key(&ADDRESSES["creator"])); + assert!( + res.contains_key(&ADDRESSES["publisher2"]) && res.contains_key(&ADDRESSES["creator"]) + ); // Is page 3 included assert!(res.contains_key(&ADDRESSES["tester"])); } From 9bdb2f3057855d02bf16c32af91555be6b4918a5 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 9 Nov 2021 17:42:26 +0200 Subject: [PATCH 09/10] better asserts in tests, renamed some variables --- primitives/src/spender.rs | 4 +- validator_worker/src/sentry_interface.rs | 93 ++++++++++++------------ 2 files changed, 50 insertions(+), 47 deletions(-) diff --git a/primitives/src/spender.rs b/primitives/src/spender.rs index 7aba539e3..474b50837 100644 --- a/primitives/src/spender.rs +++ b/primitives/src/spender.rs @@ -1,14 +1,14 @@ use crate::{Address, Channel, Deposit, UnifiedNum}; use serde::{Deserialize, Serialize}; -#[derive(Serialize, Deserialize, Debug, Clone)] +#[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)] #[serde(rename_all = "camelCase")] pub struct SpenderLeaf { pub total_spent: UnifiedNum, // merkle_proof: [u8; 32], // TODO } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] pub struct Spender { pub total_deposited: UnifiedNum, pub spender_leaf: Option, diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index dd187863b..c048d5349 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -161,6 +161,7 @@ impl SentryApi { .map_err(Error::Request) } + /// page always starts from 0 pub async fn get_spenders_page( &self, channel: &ChannelId, @@ -172,7 +173,7 @@ impl SentryApi { .join(&format!("v5/channel/{}/spender/all?page={}", channel, page)) .expect("Should not error when creating endpoint"); - let first_page: AllSpendersResponse = self + let page = self .client .get(url) .bearer_auth(&self.whoami.token) @@ -181,8 +182,7 @@ impl SentryApi { .json() .map_err(Error::Request) .await?; - - Ok(first_page) + Ok(page) } pub async fn get_all_spenders( @@ -429,50 +429,48 @@ mod test { #[tokio::test] async fn test_get_all_spenders() { let server = MockServer::start().await; - let test_spender = Spender { total_deposited: UnifiedNum::from(100_000_000), spender_leaf: None, }; - - let mut first_page_response = AllSpendersResponse { - spenders: HashMap::new(), + let mut all_spenders = HashMap::new(); + all_spenders.insert(ADDRESSES["user"], test_spender.clone()); + all_spenders.insert(ADDRESSES["publisher"], test_spender.clone()); + all_spenders.insert(ADDRESSES["publisher2"], test_spender.clone()); + all_spenders.insert(ADDRESSES["creator"], test_spender.clone()); + all_spenders.insert(ADDRESSES["tester"], test_spender.clone()); + + let first_page_response = AllSpendersResponse { + spenders: vec![ + (ADDRESSES["user"], all_spenders.get(&ADDRESSES["user"]).unwrap().to_owned()), + (ADDRESSES["publisher"], all_spenders.get(&ADDRESSES["publisher"]).unwrap().to_owned()), + ] + .into_iter().collect(), pagination: Pagination { page: 0, total_pages: 3, }, }; - first_page_response - .spenders - .insert(ADDRESSES["user"], test_spender.clone()); - first_page_response - .spenders - .insert(ADDRESSES["publisher"], test_spender.clone()); - - let mut second_page_response = AllSpendersResponse { - spenders: HashMap::new(), + + let second_page_response = AllSpendersResponse { + spenders: vec![ + (ADDRESSES["publisher2"], all_spenders.get(&ADDRESSES["publisher2"]).unwrap().to_owned()), + (ADDRESSES["creator"], all_spenders.get(&ADDRESSES["creator"]).unwrap().to_owned()), + ] + .into_iter().collect(), pagination: Pagination { page: 1, total_pages: 3, }, }; - second_page_response - .spenders - .insert(ADDRESSES["publisher2"], test_spender.clone()); - second_page_response - .spenders - .insert(ADDRESSES["creator"], test_spender.clone()); - - let mut third_page_response = AllSpendersResponse { - spenders: HashMap::new(), + + let third_page_response = AllSpendersResponse { + spenders: vec![(ADDRESSES["tester"], all_spenders.get(&ADDRESSES["tester"]).unwrap().to_owned())].into_iter().collect(), pagination: Pagination { page: 2, total_pages: 3, }, }; - third_page_response - .spenders - .insert(ADDRESSES["tester"], test_spender.clone()); Mock::given(method("GET")) .and(path(format!( @@ -528,26 +526,31 @@ mod test { let sentry = SentryApi::init(adapter, logger, config, validators).expect("Should build sentry"); - let mut expected_response = HashMap::new(); - expected_response.insert(ADDRESSES["user"], test_spender.clone()); - expected_response.insert(ADDRESSES["publisher"], test_spender.clone()); - expected_response.insert(ADDRESSES["publisher2"], test_spender.clone()); - expected_response.insert(ADDRESSES["creator"], test_spender.clone()); - expected_response.insert(ADDRESSES["tester"], test_spender.clone()); - - let res = sentry + let mut res = sentry .get_all_spenders(DUMMY_CAMPAIGN.channel.id()) .await .expect("should get response"); - assert!(expected_response.keys().len() == res.keys().len()); - // Is page 1 included - assert!(res.contains_key(&ADDRESSES["user"]) && res.contains_key(&ADDRESSES["publisher"])); - // Is page 2 included - assert!( - res.contains_key(&ADDRESSES["publisher2"]) && res.contains_key(&ADDRESSES["creator"]) - ); - // Is page 3 included - assert!(res.contains_key(&ADDRESSES["tester"])); + // Checks for page 1 + let res_user = res.remove(&ADDRESSES["user"]); + let res_publisher = res.remove(&ADDRESSES["publisher"]); + assert!(res_user.is_some() && res_publisher.is_some()); + assert_eq!(res_user.unwrap(), test_spender); + assert_eq!(res_publisher.unwrap(), test_spender); + + // Checks for page 2 + let res_publisher2 = res.remove(&ADDRESSES["publisher2"]); + let res_creator = res.remove(&ADDRESSES["creator"]); + assert!(res_publisher2.is_some() && res_creator.is_some()); + assert_eq!(res_publisher2.unwrap(), test_spender); + assert_eq!(res_creator.unwrap(), test_spender); + + // Checks for page 3 + let res_tester = res.remove(&ADDRESSES["tester"]); + assert!(res_tester.is_some()); + assert_eq!(res_tester.unwrap(), test_spender); + + // There should be no remaining elements + assert_eq!(res.len(), 0) } } From 850d8112f5f7743dd8e77a9f578e785580ceaa76 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Tue, 9 Nov 2021 17:49:02 +0200 Subject: [PATCH 10/10] cargo fmt --- validator_worker/src/sentry_interface.rs | 47 ++++++++++++++++++------ 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/validator_worker/src/sentry_interface.rs b/validator_worker/src/sentry_interface.rs index c048d5349..6f551a310 100644 --- a/validator_worker/src/sentry_interface.rs +++ b/validator_worker/src/sentry_interface.rs @@ -173,16 +173,14 @@ impl SentryApi { .join(&format!("v5/channel/{}/spender/all?page={}", channel, page)) .expect("Should not error when creating endpoint"); - let page = self - .client + self.client .get(url) .bearer_auth(&self.whoami.token) .send() .await? .json() .map_err(Error::Request) - .await?; - Ok(page) + .await } pub async fn get_all_spenders( @@ -415,7 +413,7 @@ mod test { util::tests::{ discard_logger, prep_db::{ - ADDRESSES, DUMMY_CAMPAIGN, DUMMY_VALIDATOR_FOLLOWER, DUMMY_VALIDATOR_LEADER, IDS, + ADDRESSES, DUMMY_CAMPAIGN, DUMMY_VALIDATOR_LEADER, IDS, }, }, UnifiedNum, @@ -442,10 +440,20 @@ mod test { let first_page_response = AllSpendersResponse { spenders: vec![ - (ADDRESSES["user"], all_spenders.get(&ADDRESSES["user"]).unwrap().to_owned()), - (ADDRESSES["publisher"], all_spenders.get(&ADDRESSES["publisher"]).unwrap().to_owned()), + ( + ADDRESSES["user"], + all_spenders.get(&ADDRESSES["user"]).unwrap().to_owned(), + ), + ( + ADDRESSES["publisher"], + all_spenders + .get(&ADDRESSES["publisher"]) + .unwrap() + .to_owned(), + ), ] - .into_iter().collect(), + .into_iter() + .collect(), pagination: Pagination { page: 0, total_pages: 3, @@ -454,10 +462,20 @@ mod test { let second_page_response = AllSpendersResponse { spenders: vec![ - (ADDRESSES["publisher2"], all_spenders.get(&ADDRESSES["publisher2"]).unwrap().to_owned()), - (ADDRESSES["creator"], all_spenders.get(&ADDRESSES["creator"]).unwrap().to_owned()), + ( + ADDRESSES["publisher2"], + all_spenders + .get(&ADDRESSES["publisher2"]) + .unwrap() + .to_owned(), + ), + ( + ADDRESSES["creator"], + all_spenders.get(&ADDRESSES["creator"]).unwrap().to_owned(), + ), ] - .into_iter().collect(), + .into_iter() + .collect(), pagination: Pagination { page: 1, total_pages: 3, @@ -465,7 +483,12 @@ mod test { }; let third_page_response = AllSpendersResponse { - spenders: vec![(ADDRESSES["tester"], all_spenders.get(&ADDRESSES["tester"]).unwrap().to_owned())].into_iter().collect(), + spenders: vec![( + ADDRESSES["tester"], + all_spenders.get(&ADDRESSES["tester"]).unwrap().to_owned(), + )] + .into_iter() + .collect(), pagination: Pagination { page: 2, total_pages: 3,