From 0f728beedc35cc62a048ed044a543d2065c8b8a8 Mon Sep 17 00:00:00 2001 From: ssebo Date: Thu, 30 Mar 2023 17:21:13 +0800 Subject: [PATCH 1/4] feat: prerequisite --- Cargo.lock | 2 +- Cargo.toml | 2 +- resources/fixtures/repo.json | 146 +++++++++++++++++ resources/fixtures/spec | 2 +- src/config.rs | 3 + src/evaluate.rs | 300 ++++++++++++++++++++++++----------- src/feature_probe.rs | 49 +++--- src/lib.rs | 10 ++ src/sync.rs | 6 +- 9 files changed, 401 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6700084..0b42af3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -793,7 +793,7 @@ dependencies = [ [[package]] name = "feature-probe-server-sdk" -version = "2.0.1" +version = "2.1.0" dependencies = [ "anyhow", "approx", diff --git a/Cargo.toml b/Cargo.toml index 78b6b74..e9a9cdd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] edition = "2021" name = "feature-probe-server-sdk" -version = "2.0.1" +version = "2.1.0" license = "Apache-2.0" authors = ["maintain@featureprobe.com"] description = "FeatureProbe Server Side SDK for Rust" diff --git a/resources/fixtures/repo.json b/resources/fixtures/repo.json index 1c88179..7741fa0 100644 --- a/resources/fixtures/repo.json +++ b/resources/fixtures/repo.json @@ -372,6 +372,152 @@ "not_in": true } ] + }, + "prerequisite_toggle": { + "key": "prerequisite_toggle", + "enabled": true, + "forClient": false, + "version": 1, + "disabledServe": { + "select": 0 + }, + "defaultServe": { + "select": 1 + }, + "rules": [ + { + "serve": { + "select": 2 + }, + "conditions": [ + { + "type": "segment", + "subject": "user", + "predicate": "is in", + "objects": [ + "some_segment1-fjoaefjaam" + ] + } + ] + } + ], + "prerequisites": [ + { + "key": "bool_toggle", + "value": false + }, + { + "key": "string_toggle", + "value": "2" + }, + { + "key": "number_toggle", + "value": 2 + } + ], + "variations": [ + { + "0": "0" + }, + { + "1": "1" + }, + { + "2": "2" + } + ] + }, + "prerequisite_toggle_not_exist": { + "key": "prerequisite_toggle_not_exist", + "enabled": true, + "forClient": false, + "version": 1, + "disabledServe": { + "select": 0 + }, + "defaultServe": { + "select": 1 + }, + "rules": [ + { + "serve": { + "select": 2 + }, + "conditions": [ + { + "type": "segment", + "subject": "user", + "predicate": "is in", + "objects": [ + "some_segment1-fjoaefjaam" + ] + } + ] + } + ], + "prerequisites": [ + { + "key": "bool_toggle_not_exist", + "value": true + } + ], + "variations": [ + { + "0": "0" + }, + { + "1": "1" + }, + { + "2": "2" + } + ] + }, + "prerequisite_toggle_not_match": { + "key": "prerequisite_toggle_not_match", + "enabled": true, + "forClient": false, + "version": 1, + "disabledServe": { + "select": 0 + }, + "defaultServe": { + "select": 1 + }, + "rules": [ + { + "serve": { + "select": 2 + }, + "conditions": [ + { + "type": "segment", + "subject": "user", + "predicate": "is in", + "objects": [ + "some_segment1-fjoaefjaam" + ] + } + ] + } + ], + "prerequisites": [ + { + "key": "bool_toggle", + "value": true + } + ], + "variations": [ + { + "0": "0" + }, + { + "1": "1" + }, + { + "2": "2" + } + ] } } } diff --git a/resources/fixtures/spec b/resources/fixtures/spec index 065c758..b055706 160000 --- a/resources/fixtures/spec +++ b/resources/fixtures/spec @@ -1 +1 @@ -Subproject commit 065c758e62b057e8f0664f9d2561fa1d35200306 +Subproject commit b055706fdd54247648493f434aecdf8d598b562c diff --git a/src/config.rs b/src/config.rs index 7f09996..2dd5cd3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -36,6 +36,7 @@ pub(crate) struct Config { pub realtime_url: Url, #[cfg(all(feature = "use_tokio", feature = "realtime"))] pub realtime_path: String, + pub max_prerequisites_deep: u8, } impl Default for FPConfig { @@ -74,6 +75,7 @@ impl Default for Config { realtime_url: Url::parse("https://featureprobe.io/server/realtime").unwrap(), #[cfg(all(feature = "use_tokio", feature = "realtime"))] realtime_path: "/server/realtime".to_owned(), + max_prerequisites_deep: 20, } } } @@ -122,6 +124,7 @@ impl FPConfig { realtime_url, #[cfg(all(feature = "use_tokio", feature = "realtime"))] realtime_path, + ..Default::default() } } } diff --git a/src/evaluate.rs b/src/evaluate.rs index 4033ed2..9103424 100644 --- a/src/evaluate.rs +++ b/src/evaluate.rs @@ -1,6 +1,6 @@ -use crate::unix_timestamp; use crate::user::FPUser; use crate::FPError; +use crate::{unix_timestamp, PrerequisiteError}; use byteorder::{BigEndian, ReadBytesExt}; use regex::Regex; use semver::Version; @@ -45,11 +45,6 @@ impl Serve { }), } } - - pub fn select_variation_value(&self, eval_param: &EvalParams) -> Result { - let v = self.select_variation(eval_param)?; - Ok(v.value) - } } #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] @@ -128,6 +123,7 @@ pub struct EvalParams<'a> { user: &'a FPUser, variations: &'a [Value], segment_repo: &'a HashMap, + toggle_repo: &'a HashMap, } #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Default)] @@ -142,6 +138,13 @@ pub struct EvalDetail { pub reason: String, } +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] +#[serde(rename_all = "camelCase")] +pub struct Prerequisites { + pub key: String, + pub value: Value, +} + #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] #[serde(rename_all = "camelCase")] pub struct Toggle { @@ -155,6 +158,7 @@ pub struct Toggle { default_serve: Serve, rules: Vec, variations: Vec, + prerequisites: Option>, } impl Toggle { @@ -162,103 +166,123 @@ impl Toggle { &self, user: &FPUser, segment_repo: &HashMap, - ) -> Result { + toggle_repo: &HashMap, + is_detail: bool, + deep: u8, + ) -> EvalDetail { let eval_param = EvalParams { user, segment_repo, + toggle_repo, key: &self.key, - is_detail: false, + is_detail, variations: &self.variations, }; + match self.do_eval(&eval_param, deep) { + Ok(eval) => eval, + Err(e) => self.default_variation(&eval_param, Some(e.to_string())), + } + } + + fn do_eval( + &self, + eval_param: &EvalParams, + max_deep: u8, + ) -> Result, PrerequisiteError> { if !self.enabled { - return self.disabled_serve.select_variation_value(&eval_param); + let v = self.disabled_serve.select_variation(eval_param).ok(); + return Ok(self.serve_variation(v, "disabled".to_owned(), None)); } - for rule in &self.rules { - if let Some(v) = rule.serve_variation(&eval_param)? { - return Ok(v.value); + self.check_prerequisites(eval_param, max_deep)?; + + for (i, rule) in self.rules.iter().enumerate() { + match rule.serve_variation(eval_param) { + Ok(v) => { + if v.is_some() { + return Ok(self.serve_variation(v, format!("rule {i}"), Some(i))); + } + } + Err(e) => { + return Ok(self.serve_variation(None, format!("{e:?}"), Some(i))); + } } } - self.default_serve.select_variation_value(&eval_param) + Ok(self.default_variation(eval_param, None)) } - pub fn track_access_events(&self) -> bool { - self.track_access_events.unwrap_or(false) + fn check_prerequisites( + &self, + eval_param: &EvalParams, + deep: u8, + ) -> Result<(), PrerequisiteError> { + if deep == 0 { + return Err(PrerequisiteError::DeepOverflow); + } + + if let Some(ref prerequisites) = self.prerequisites { + for pre in prerequisites { + let eval = match eval_param.toggle_repo.get(&pre.key) { + None => { + return Err(PrerequisiteError::NotExist(pre.key.to_string())); + } + Some(t) => t.do_eval( + &EvalParams { + key: &t.key, + variations: &t.variations, + is_detail: eval_param.is_detail, + user: eval_param.user, + segment_repo: eval_param.segment_repo, + toggle_repo: eval_param.toggle_repo, + }, + deep - 1, + )?, + }; + + match eval.value { + Some(v) if v == pre.value => continue, + _ => return Err(PrerequisiteError::NotMatch(pre.key.to_string())), + } + } + } + + Ok(()) } - pub fn eval_detail( + fn serve_variation( &self, - user: &FPUser, - segment_repo: &HashMap, + v: Option, + reason: String, + rule_index: Option, ) -> EvalDetail { - let eval_param = EvalParams { - user, - segment_repo, - key: &self.key, - is_detail: true, - variations: &self.variations, - }; - if !self.enabled { - let v = self.disabled_serve.select_variation(&eval_param).ok(); - return EvalDetail { - variation_index: v.as_ref().map(|v| v.index), - value: v.map(|v| v.value), - version: Some(self.version), - track_access_events: self.track_access_events, - last_modified: self.last_modified, - reason: "disabled".to_owned(), - ..Default::default() - }; + EvalDetail { + variation_index: v.as_ref().map(|v| v.index), + value: v.map(|v| v.value), + version: Some(self.version), + track_access_events: self.track_access_events, + last_modified: self.last_modified, + rule_index, + reason, } - for (i, rule) in self.rules.iter().enumerate() { - match rule.serve_variation(&eval_param) { - Ok(opt_value) => { - if let Some(v) = opt_value { - return EvalDetail { - value: Some(v.value), - variation_index: Some(v.index), - rule_index: Some(i), - track_access_events: self.track_access_events, - last_modified: self.last_modified, - version: Some(self.version), - reason: format!("rule {i}"), - }; - } - } + } - Err(e) => { - return EvalDetail { - rule_index: Some(i), - version: Some(self.version), - reason: format!("{e:?}"), - track_access_events: self.track_access_events, - last_modified: self.last_modified, - ..Default::default() - }; - } + fn default_variation( + &self, + eval_param: &EvalParams, + reason: Option, + ) -> EvalDetail { + match self.default_serve.select_variation(eval_param) { + Ok(v) => { + self.serve_variation(Some(v), concat_reason("default".to_owned(), reason), None) } + Err(e) => self.serve_variation(None, concat_reason(format!("{e:?}"), reason), None), } + } - match self.default_serve.select_variation(&eval_param) { - Ok(v) => EvalDetail { - value: Some(v.value), - variation_index: Some(v.index), - version: Some(self.version), - track_access_events: self.track_access_events, - last_modified: self.last_modified, - reason: "default.".to_owned(), - ..Default::default() - }, - Err(e) => EvalDetail { - track_access_events: self.track_access_events, - last_modified: self.last_modified, - version: Some(self.version), - reason: format!("{e:?}"), - ..Default::default() - }, - } + pub fn track_access_events(&self) -> bool { + self.track_access_events.unwrap_or(false) } #[cfg(feature = "internal")] @@ -291,6 +315,7 @@ impl Toggle { version: 0, for_client: false, rules: vec![], + prerequisites: None, } } } @@ -539,6 +564,13 @@ pub fn load_json(json_str: &str) -> Result { repo } +fn concat_reason(reason1: String, reason2: Option) -> String { + if let Some(reason2) = reason2 { + return format!("{reason1}. {reason2}."); + } + format!("{reason1}.") +} + #[cfg(test)] mod tests { use super::*; @@ -546,6 +578,8 @@ mod tests { use std::fs; use std::path::PathBuf; + const MAX_DEEP: u8 = 20; + #[test] fn test_load() { let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); @@ -579,8 +613,8 @@ mod tests { let user = FPUser::new().with("city", "4"); let toggle = repo.toggles.get("json_toggle").unwrap(); - let r = toggle.eval(&user, &repo.segments); - let r = r.unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + let r = r.value.unwrap(); let r = r.as_object().unwrap(); assert!(r.get("variation_1").is_some()); } @@ -596,8 +630,8 @@ mod tests { let user = FPUser::new().with("city", "100"); let toggle = repo.toggles.get("not_in_segment").unwrap(); - let r = toggle.eval(&user, &repo.segments); - let r = r.unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + let r = r.value.unwrap(); let r = r.as_object().unwrap(); assert!(r.get("not_in").is_some()); } @@ -613,19 +647,19 @@ mod tests { let user = FPUser::new().with("city", "1").with("os", "linux"); let toggle = repo.toggles.get("multi_condition_toggle").unwrap(); - let r = toggle.eval(&user, &repo.segments); - let r = r.unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + let r = r.value.unwrap(); let r = r.as_object().unwrap(); assert!(r.get("variation_0").is_some()); let user = FPUser::new().with("os", "linux"); let toggle = repo.toggles.get("multi_condition_toggle").unwrap(); - let r = toggle.eval_detail(&user, &repo.segments); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); assert!(r.reason.starts_with("default")); let user = FPUser::new().with("city", "1"); let toggle = repo.toggles.get("multi_condition_toggle").unwrap(); - let r = toggle.eval_detail(&user, &repo.segments); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); assert!(r.reason.starts_with("default")); } @@ -645,8 +679,8 @@ mod tests { let mut variation_1 = 0; let mut variation_2 = 0; for user in &users { - let r = toggle.eval(user, &repo.segments); - let r = r.unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + let r = r.value.unwrap(); let r = r.as_object().unwrap(); if r.get("variation_0").is_some() { variation_0 += 1; @@ -676,8 +710,9 @@ mod tests { let user = FPUser::new().with("city", "100"); let toggle = repo.toggles.get("disabled_toggle").unwrap(); - let r = toggle.eval(&user, &repo.segments); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); assert!(r + .value .unwrap() .as_object() .unwrap() @@ -685,6 +720,77 @@ mod tests { .is_some()); } + #[test] + fn test_prerequisite_toggle() { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("resources/fixtures/repo.json"); + let json_str = fs::read_to_string(path).unwrap(); + let repo = load_json(&json_str); + assert!(repo.is_ok()); + let repo = repo.unwrap(); + + let user = FPUser::new().with("city", "4"); + + let toggle = repo.toggles.get("prerequisite_toggle").unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + + assert!(r.value.unwrap().as_object().unwrap().get("2").is_some()); + } + + #[test] + fn test_prerequisite_not_exist() { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("resources/fixtures/repo.json"); + let json_str = fs::read_to_string(path).unwrap(); + let repo = load_json(&json_str); + assert!(repo.is_ok()); + let repo = repo.unwrap(); + + let user = FPUser::new().with("city", "4"); + + let toggle = repo.toggles.get("prerequisite_toggle_not_exist").unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + + assert!(r.value.unwrap().as_object().unwrap().get("1").is_some()); + assert!(r.reason.contains("not exist")); + } + + #[test] + fn test_prerequisite_not_match() { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("resources/fixtures/repo.json"); + let json_str = fs::read_to_string(path).unwrap(); + let repo = load_json(&json_str); + assert!(repo.is_ok()); + let repo = repo.unwrap(); + + let user = FPUser::new().with("city", "4"); + + let toggle = repo.toggles.get("prerequisite_toggle_not_match").unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + + assert!(r.value.unwrap().as_object().unwrap().get("1").is_some()); + assert!(r.reason.contains("not match")); + } + + #[test] + fn test_prerequisite_max_deep() { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("resources/fixtures/repo.json"); + let json_str = fs::read_to_string(path).unwrap(); + let repo = load_json(&json_str); + assert!(repo.is_ok()); + let repo = repo.unwrap(); + + let user = FPUser::new().with("city", "4"); + + let toggle = repo.toggles.get("prerequisite_toggle").unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, 1); + + assert!(r.value.unwrap().as_object().unwrap().get("1").is_some()); + assert!(r.reason.contains("deep overflow")); + } + fn gen_users(num: usize, random: bool) -> Vec { let mut users = Vec::with_capacity(num); for i in 0..num { @@ -722,6 +828,7 @@ mod distribution_tests { user: &user_bucket_by_name, variations: &[], segment_repo: &Default::default(), + toggle_repo: &Default::default(), }; let result = distribution.find_index(¶ms); @@ -747,6 +854,7 @@ mod distribution_tests { user: &user_bucket_by_name, variations: &[], segment_repo: &Default::default(), + toggle_repo: &Default::default(), }; let result = distribution.find_index(¶ms); @@ -758,6 +866,7 @@ mod distribution_tests { user: &user_bucket_by_name, variations: &[], segment_repo: &Default::default(), + toggle_repo: &Default::default(), }; let result = distribution.find_index(¶ms_no_detail); assert!(result.is_err()); @@ -786,6 +895,7 @@ mod distribution_tests { Value::String("b".to_string()), ], segment_repo: &Default::default(), + toggle_repo: &Default::default(), }; let result = serve.select_variation(¶ms).expect_err("e"); @@ -800,8 +910,10 @@ mod condition_tests { use std::fs; use std::path::PathBuf; + const MAX_DEEP: u8 = 20; + #[test] - fn test_unkown_condition() { + fn test_unknown_condition() { let json_str = r#" { "type": "new_type", @@ -1134,14 +1246,14 @@ mod condition_tests { let user = FPUser::new().with("city", "1"); let toggle = repo.toggles.get("json_toggle").unwrap(); - let r = toggle.eval(&user, &repo.segments); - let r = r.unwrap(); + let r = toggle.eval(&user, &repo.segments, &repo.toggles, false, MAX_DEEP); + let r = r.value.unwrap(); let r = r.as_object().unwrap(); assert!(r.get("variation_0").is_some()); } #[test] - fn test_segment_deserilize() { + fn test_segment_deserialize() { let json_str = r#" { "type":"segment", diff --git a/src/feature_probe.rs b/src/feature_probe.rs index 97ccc8f..5e909ad 100644 --- a/src/feature_probe.rs +++ b/src/feature_probe.rs @@ -84,38 +84,42 @@ impl FeatureProbe { } pub fn bool_value(&self, toggle: &str, user: &FPUser, default: bool) -> bool { - self.generic_detail(toggle, user, default, |v| v.as_bool()) + self.generic_eval(toggle, user, default, false, |v| v.as_bool()) .value } pub fn string_value(&self, toggle: &str, user: &FPUser, default: String) -> String { - self.generic_detail(toggle, user, default, |v| v.as_str().map(|s| s.to_owned())) - .value + self.generic_eval(toggle, user, default, false, |v| { + v.as_str().map(|s| s.to_owned()) + }) + .value } pub fn number_value(&self, toggle: &str, user: &FPUser, default: f64) -> f64 { - self.generic_detail(toggle, user, default, |v| v.as_f64()) + self.generic_eval(toggle, user, default, false, |v| v.as_f64()) .value } pub fn json_value(&self, toggle: &str, user: &FPUser, default: Value) -> Value { - self.generic_detail(toggle, user, default, Some).value + self.generic_eval(toggle, user, default, false, Some).value } pub fn bool_detail(&self, toggle: &str, user: &FPUser, default: bool) -> FPDetail { - self.generic_detail(toggle, user, default, |v| v.as_bool()) + self.generic_eval(toggle, user, default, true, |v| v.as_bool()) } pub fn string_detail(&self, toggle: &str, user: &FPUser, default: String) -> FPDetail { - self.generic_detail(toggle, user, default, |v| v.as_str().map(|x| x.to_owned())) + self.generic_eval(toggle, user, default, true, |v| { + v.as_str().map(|x| x.to_owned()) + }) } pub fn number_detail(&self, toggle: &str, user: &FPUser, default: f64) -> FPDetail { - self.generic_detail(toggle, user, default, |v| v.as_f64()) + self.generic_eval(toggle, user, default, true, |v| v.as_f64()) } pub fn json_detail(&self, toggle: &str, user: &FPUser, default: Value) -> FPDetail { - self.generic_detail(toggle, user, default, Some) + self.generic_eval(toggle, user, default, true, Some) } pub fn track(&self, event_name: &str, user: &FPUser, value: Option) { @@ -175,14 +179,15 @@ impl FeatureProbe { } } - fn generic_detail( + fn generic_eval( &self, toggle: &str, user: &FPUser, default: T, + is_detail: bool, transform: fn(Value) -> Option, ) -> FPDetail { - let (value, reason, detail) = match self.eval_detail(toggle, user) { + let (value, reason, detail) = match self.eval(toggle, user, is_detail) { None => ( default, Some(format!("Toggle:[{toggle}] not exist")), @@ -206,12 +211,17 @@ impl FeatureProbe { } } - fn eval_detail(&self, toggle: &str, user: &FPUser) -> Option> { + fn eval(&self, toggle: &str, user: &FPUser, is_detail: bool) -> Option> { let repo = self.repo.read(); - let detail = repo - .toggles - .get(toggle) - .map(|toggle| toggle.eval_detail(user, &repo.segments)); + let detail = repo.toggles.get(toggle).map(|toggle| { + toggle.eval( + user, + &repo.segments, + &repo.toggles, + is_detail, + self.config.max_prerequisites_deep, + ) + }); let track_access_events = match repo.toggles.get(toggle) { Some(toggle) => toggle.track_access_events(), None => false, @@ -293,7 +303,7 @@ impl FeatureProbe { tokio::spawn(async move { let url = slf.config.realtime_url; let server_sdk_key = slf.config.server_sdk_key.clone(); - tracing::trace!("connect_socket {}", url); + trace!("connect_socket {}", url); let client = socketio_rs::ClientBuilder::new(url.clone()) .namespace(&nsp) .on(socketio_rs::Event::Connect, move |_, socket, _| { @@ -339,7 +349,7 @@ impl FeatureProbe { if let Some(syncer) = &slf.syncer { let _ = syncer.sync_now(SyncType::Realtime); } else { - tracing::warn!("socket receive update event, but no synchronizer"); + warn!("socket receive update event, but no synchronizer"); } } .boxed() @@ -543,6 +553,7 @@ mod server_sdk_contract_tests { for scenario in root.unwrap().tests { println!("scenario: {}", scenario.scenario); assert!(!scenario.cases.is_empty()); + let fp = FeatureProbe::new_with("secret key".to_string(), scenario.fixture); for case in scenario.cases { @@ -631,7 +642,7 @@ mod server_sdk_contract_tests { } } - fn assert_detail(case: &Case, ret: FPDetail) { + fn assert_detail(case: &Case, ret: FPDetail) { match &case.expect_result.reason { None => (), Some(r) => { diff --git a/src/lib.rs b/src/lib.rs index 2cd7f41..bf6aeb2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,6 +50,16 @@ pub enum FPError { InternalError(String), } +#[derive(Debug, Error)] +enum PrerequisiteError { + #[error("prerequisite deep overflow")] + DeepOverflow, + #[error("prerequisite not exist: {0}")] + NotExist(String), + #[error("prerequisite not match: {0}")] + NotMatch(String), +} + #[derive(Debug, Deserialize)] pub struct SdkAuthorization(pub String); diff --git a/src/sync.rs b/src/sync.rs index 09c4f96..2dc11b7 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -358,7 +358,7 @@ mod tests { let is_timeout_fn: Option bool + Send>> = Some(Box::new(|| false)); let is_send = true; let r = Synchronizer::should_send( - Err(FPError::InternalError("unkown".to_owned())), + Err(FPError::InternalError("unknown".to_owned())), &is_timeout_fn, is_send, ); @@ -366,7 +366,7 @@ mod tests { let is_timeout_fn: Option bool + Send>> = Some(Box::new(|| true)); let r = Synchronizer::should_send( - Err(FPError::InternalError("unkown".to_owned())), + Err(FPError::InternalError("unknown".to_owned())), &is_timeout_fn, is_send, ); @@ -375,7 +375,7 @@ mod tests { let is_send = false; let is_timeout_fn: Option bool + Send>> = Some(Box::new(|| true)); let r = Synchronizer::should_send( - Err(FPError::InternalError("unkown".to_owned())), + Err(FPError::InternalError("unknown".to_owned())), &is_timeout_fn, is_send, ); From 82c427b664c9397e902d1ee95998c16244281cee Mon Sep 17 00:00:00 2001 From: ssebo Date: Thu, 30 Mar 2023 18:04:06 +0800 Subject: [PATCH 2/4] chore: remove simple blocking http version --- Cargo.lock | 59 ---------------------------- Cargo.toml | 18 +++------ Makefile | 3 +- src/config.rs | 29 ++++++-------- src/feature_probe.rs | 33 ++++++---------- src/sync.rs | 92 ++------------------------------------------ 6 files changed, 32 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0b42af3..77799ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,12 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "adler" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" - [[package]] name = "adler32" version = "1.2.0" @@ -340,12 +334,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "chunked_transfer" -version = "1.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fff857943da45f546682664a79488be82e69e43c1a7a2307679ab9afb3a66d2e" - [[package]] name = "ci_info" version = "0.10.2" @@ -484,15 +472,6 @@ dependencies = [ "libc", ] -[[package]] -name = "crc32fast" -version = "1.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b540bd8bc810d3885c6ea91e2018302f68baba2129ab3e88f32389ee9370880d" -dependencies = [ - "cfg-if", -] - [[package]] name = "criterion" version = "0.4.0" @@ -728,7 +707,6 @@ dependencies = [ "thiserror", "tokio", "tracing", - "ureq", "url", ] @@ -824,20 +802,9 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", - "ureq", "url", ] -[[package]] -name = "flate2" -version = "1.0.24" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f82b0f4c27ad9f8bfd1f3208d882da2b09c301bc1c828fd3a00d0216d2fbbff6" -dependencies = [ - "crc32fast", - "miniz_oxide", -] - [[package]] name = "fnv" version = "1.0.7" @@ -1380,15 +1347,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" -[[package]] -name = "miniz_oxide" -version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96590ba8f175222643a85693f33d26e9c8a015f599c216509b1a6894af675d34" -dependencies = [ - "adler", -] - [[package]] name = "minstant" version = "0.1.2" @@ -2582,23 +2540,6 @@ version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" -[[package]] -name = "ureq" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b97acb4c28a254fd7a4aeec976c46a7fa404eac4d7c134b30c75144846d7cb8f" -dependencies = [ - "base64", - "chunked_transfer", - "flate2", - "log", - "once_cell", - "rustls", - "url", - "webpki", - "webpki-roots", -] - [[package]] name = "url" version = "2.3.1" diff --git a/Cargo.toml b/Cargo.toml index e9a9cdd..e22acd3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,12 +17,8 @@ harness = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -default = ["use_std", "event"] +default = ["realtime"] internal = [] -use_std = ["ureq"] -event = ["feature-probe-event-std"] -use_tokio = ["reqwest", "tokio", "futures-util"] -event_tokio = ["feature-probe-event-tokio"] realtime = ["socketio-rs"] [dependencies] @@ -44,20 +40,18 @@ thiserror = "1.0" tracing = "0.1" url = "2" -feature-probe-event-std = { optional = true, version = "1.1.3", package = "feature-probe-event" } -feature-probe-event-tokio = { optional = true, version = "1.1.3", features = [ +feature-probe-event = { version = "1.1.3", features = [ "use_tokio", -], default-features = false, package = "feature-probe-event" } +], default-features = false} -reqwest = { optional = true, version = "0.11", default-features = false, features = [ +reqwest = { version = "0.11", default-features = false, features = [ "rustls-tls", "json", ] } -tokio = { optional = true, version = "1", features = ["full"] } -ureq = { optional = true, version = "2.4" } +tokio = { version = "1", features = ["full"] } socketio-rs = { optional = true, version = "0.1.7", default-features = false, features = ["client"] } -futures-util = { optional = true, version = "0.3", default-features = false, features = [ +futures-util = { version = "0.3", default-features = false, features = [ "sink", ] } diff --git a/Makefile b/Makefile index 4ed5c5d..e19188e 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,5 @@ release-test: cargo test --release --verbose --features async --no-default-features test: cargo test --verbose && \ - cargo test --verbose --features use_tokio --features internal --features event_tokio --no-default-features && \ - cargo test --verbose --features use_tokio --features internal --features event_tokio --features realtime --no-default-features + cargo test --verbose --features internal --features realtime --no-default-features diff --git a/src/config.rs b/src/config.rs index 2dd5cd3..35d7a2d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,5 @@ use std::time::Duration; -#[cfg(feature = "use_tokio")] use reqwest::Client; use tracing::info; use url::Url; @@ -12,13 +11,12 @@ pub struct FPConfig { pub events_url: Option, pub server_sdk_key: String, pub refresh_interval: Duration, - #[cfg(feature = "use_tokio")] pub http_client: Option, pub start_wait: Option, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] pub realtime_url: Option, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] pub realtime_path: Option, } @@ -28,13 +26,11 @@ pub(crate) struct Config { pub events_url: Url, pub server_sdk_key: String, pub refresh_interval: Duration, - #[cfg(feature = "use_tokio")] pub http_client: Option, pub start_wait: Option, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] pub realtime_url: Url, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] pub realtime_path: String, pub max_prerequisites_deep: u8, } @@ -48,12 +44,11 @@ impl Default for FPConfig { events_url: None, refresh_interval: Duration::from_secs(5), start_wait: None, - #[cfg(feature = "use_tokio")] http_client: None, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_url: None, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_path: None, } } @@ -68,12 +63,11 @@ impl Default for Config { events_url: Url::parse("https://featureprobe.io/server/api/events").unwrap(), refresh_interval: Duration::from_secs(60), start_wait: None, - #[cfg(feature = "use_tokio")] http_client: None, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_url: Url::parse("https://featureprobe.io/server/realtime").unwrap(), - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_path: "/server/realtime".to_owned(), max_prerequisites_deep: 20, } @@ -89,13 +83,13 @@ impl FPConfig { false => remote_url + "/", }; - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] let realtime_url = match &self.realtime_url { None => Url::parse(&(remote_url.clone() + "realtime")).expect("invalid realtime url"), Some(url) => url.to_owned(), }; - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] let realtime_path = match &self.realtime_path { Some(p) => p.to_owned(), None => realtime_url.path().to_owned(), @@ -118,11 +112,10 @@ impl FPConfig { server_sdk_key: self.server_sdk_key.clone(), refresh_interval: self.refresh_interval, start_wait: self.start_wait, - #[cfg(feature = "use_tokio")] http_client: self.http_client.clone(), - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_url, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] realtime_path, ..Default::default() } diff --git a/src/feature_probe.rs b/src/feature_probe.rs index 5e909ad..23425dd 100644 --- a/src/feature_probe.rs +++ b/src/feature_probe.rs @@ -11,33 +11,29 @@ use event::event::CustomEvent; use event::event::Event; use event::recorder::unix_timestamp; use event::recorder::EventRecorder; -#[cfg(feature = "event")] -use feature_probe_event_std as event; -#[cfg(feature = "event_tokio")] -use feature_probe_event_tokio as event; -#[cfg(all(feature = "use_tokio", feature = "realtime"))] +use feature_probe_event as event; +#[cfg(feature = "realtime")] use futures_util::FutureExt; use parking_lot::RwLock; use serde_json::Value; -#[cfg(all(feature = "use_tokio", feature = "realtime"))] +#[cfg(feature = "realtime")] use socketio_rs::Client; use std::collections::HashMap; use std::fmt::Debug; use std::sync::Arc; use tracing::{trace, warn}; -#[cfg(all(feature = "use_tokio", feature = "realtime"))] +#[cfg(feature = "realtime")] type SocketCallback = std::pin::Pin + Send>>; #[derive(Default, Clone)] pub struct FeatureProbe { repo: Arc>, syncer: Option, - #[cfg(any(feature = "event", feature = "event_tokio"))] event_recorder: Option, config: Config, should_stop: Arc>, - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] socket: Option, } @@ -148,17 +144,15 @@ impl FeatureProbe { }, repo: Arc::new(RwLock::new(repo)), syncer: None, - #[cfg(any(feature = "event", feature = "event_tokio"))] event_recorder: None, should_stop: Arc::new(RwLock::new(false)), - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] socket: None, } } pub fn close(&self) { trace!("closing featureprobe client"); - #[cfg(any(feature = "event", feature = "event_tokio"))] if let Some(recorder) = &self.event_recorder { recorder.flush(); } @@ -226,12 +220,10 @@ impl FeatureProbe { Some(toggle) => toggle.track_access_events(), None => false, }; - #[cfg(any(feature = "event", feature = "event_tokio"))] self.record_detail(toggle, user, track_access_events, &detail); detail } - #[cfg(any(feature = "event", feature = "event_tokio"))] fn record_detail( &self, toggle: &str, @@ -261,10 +253,9 @@ impl FeatureProbe { fn start(&mut self) { self.sync(); - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] self.connect_socket(); - #[cfg(any(feature = "event", feature = "event_tokio"))] self.flush_events(); } @@ -278,7 +269,6 @@ impl FeatureProbe { toggles_url, refresh_interval, auth, - #[cfg(feature = "use_tokio")] self.config.http_client.clone().unwrap_or_default(), repo, ); @@ -295,7 +285,7 @@ impl FeatureProbe { syncer.sync_now(t); } - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] fn connect_socket(&mut self) { let mut slf = self.clone(); let slf2 = self.clone(); @@ -327,7 +317,7 @@ impl FeatureProbe { }); } - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] fn socket_on_connect(socket: socketio_rs::Socket, server_sdk_key: String) -> SocketCallback { let sdk_key = server_sdk_key; trace!("socket_on_connect: {:?}", sdk_key); @@ -342,12 +332,12 @@ impl FeatureProbe { .boxed() } - #[cfg(all(feature = "use_tokio", feature = "realtime"))] + #[cfg(feature = "realtime")] fn socket_on_update(slf: Self, payload: Option) -> SocketCallback { trace!("socket_on_update: {:?}", payload); async move { if let Some(syncer) = &slf.syncer { - let _ = syncer.sync_now(SyncType::Realtime); + syncer.sync_now(SyncType::Realtime); } else { warn!("socket receive update event, but no synchronizer"); } @@ -355,7 +345,6 @@ impl FeatureProbe { .boxed() } - #[cfg(any(feature = "event", feature = "event_tokio"))] fn flush_events(&mut self) { trace!("flush_events"); let events_url = self.config.events_url.clone(); diff --git a/src/sync.rs b/src/sync.rs index 2dc11b7..ca6eb53 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -2,7 +2,6 @@ use crate::FPError; use crate::Repository; use headers::HeaderValue; use parking_lot::{Mutex, RwLock}; -#[cfg(feature = "use_tokio")] use reqwest::{header::AUTHORIZATION, Client, Method}; use std::{sync::mpsc::sync_channel, time::Instant}; use std::{sync::Arc, time::Duration}; @@ -27,7 +26,6 @@ struct Inner { toggles_url: Url, refresh_interval: Duration, auth: HeaderValue, - #[cfg(feature = "use_tokio")] client: Client, repo: Arc>, is_init: Arc>, @@ -51,7 +49,7 @@ impl Synchronizer { toggles_url: Url, refresh_interval: Duration, auth: HeaderValue, - #[cfg(feature = "use_tokio")] client: Client, + client: Client, repo: Arc>, ) -> Self { Self { @@ -59,7 +57,6 @@ impl Synchronizer { toggles_url, refresh_interval, auth, - #[cfg(feature = "use_tokio")] client, repo, is_init: Default::default(), @@ -73,35 +70,6 @@ impl Synchronizer { *lock } - #[cfg(feature = "use_std")] - pub fn start_sync(&self, start_wait: Option, should_stop: Arc>) { - let inner = self.inner.clone(); - let (tx, rx) = sync_channel(1); - let start = Instant::now(); - let mut is_send = false; - let interval_duration = inner.refresh_interval; - - let is_timeout = Self::init_timeout_fn(start_wait, interval_duration, start); - std::thread::spawn(move || loop { - if let Some(r) = - Self::should_send(inner.sync_now(SyncType::Polling), &is_timeout, is_send) - { - is_send = true; - let _ = tx.try_send(r); - } - - if *should_stop.read() { - break; - } - std::thread::sleep(inner.refresh_interval); - }); - - if start_wait.is_some() { - let _ = rx.recv(); - } - } - - #[cfg(feature = "use_tokio")] pub fn start_sync(&self, start_wait: Option, should_stop: Arc>) { let inner = self.inner.clone(); let (tx, rx) = sync_channel(1); @@ -180,19 +148,12 @@ impl Synchronizer { } pub fn sync_now(&self, t: SyncType) { - #[cfg(feature = "use_tokio")] - { - let slf = self.clone(); - tokio::spawn(async move { slf.inner.sync_now(t).await }); - } - - #[cfg(feature = "use_std")] - let _ = self.inner.sync_now(t); + let slf = self.clone(); + tokio::spawn(async move { slf.inner.sync_now(t).await }); } } impl Inner { - #[cfg(feature = "use_tokio")] pub async fn sync_now(&self, t: SyncType) -> Result<(), FPError> { use http::header::USER_AGENT; @@ -238,52 +199,6 @@ impl Inner { } } - #[cfg(feature = "use_std")] - pub fn sync_now(&self, t: SyncType) -> Result<(), FPError> { - trace!("sync_now {:?}, {:?}", self.auth, t); - //TODO: report failure - let mut request = ureq::get(self.toggles_url.as_str()) - .set( - "Authorization", - self.auth.to_str().expect("already valid header value"), - ) - .set("User-Agent", &crate::USER_AGENT) - .timeout(self.refresh_interval); - - { - let repo = self.repo.read(); - if let Some(version) = &repo.version { - request = request.query("version", &version.to_string()) - } - } // drop repo lock - - match request.call() { - Err(e) => Err(FPError::HttpError(e.to_string())), - Ok(r) => match r.into_string() { - Err(e) => Err(FPError::HttpError(e.to_string())), - Ok(body) => { - match serde_json::from_str::(&body) { - Err(e) => Err(FPError::JsonError(body, e)), - Ok(r) => { - // TODO: validate repo - debug!("sync success {:?}", r); - let mut repo = self.repo.write(); - if r.version > repo.version { - let old = (*repo).clone(); - let new = r.clone(); - *repo = r; - self.notify_update(old, new, t); - } - let mut is_init = self.is_init.write(); - *is_init = true; - Ok(()) - } - } - } - }, - } - } - fn notify_update(&self, old_repo: Repository, new_repo: Repository, t: SyncType) { let lock = self.update_callback.lock(); if let Some(cb) = &*lock { @@ -410,7 +325,6 @@ mod tests { toggles_url, refresh_interval, auth, - #[cfg(feature = "use_tokio")] client: Default::default(), repo: Default::default(), is_init: Default::default(), From c44cdd490501e3ffbbf3a56d82cecb23660f3a46 Mon Sep 17 00:00:00 2001 From: ssebo Date: Thu, 30 Mar 2023 18:24:28 +0800 Subject: [PATCH 3/4] chore: revert contract test case --- resources/fixtures/spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/fixtures/spec b/resources/fixtures/spec index b055706..065c758 160000 --- a/resources/fixtures/spec +++ b/resources/fixtures/spec @@ -1 +1 @@ -Subproject commit b055706fdd54247648493f434aecdf8d598b562c +Subproject commit 065c758e62b057e8f0664f9d2561fa1d35200306 From d8817631589d29ab625a17698338077473305024 Mon Sep 17 00:00:00 2001 From: ssebo Date: Thu, 30 Mar 2023 18:33:35 +0800 Subject: [PATCH 4/4] fix: github action --- .github/workflows/build-with-coverage.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build-with-coverage.yml b/.github/workflows/build-with-coverage.yml index 1e1af85..00b2005 100644 --- a/.github/workflows/build-with-coverage.yml +++ b/.github/workflows/build-with-coverage.yml @@ -29,6 +29,8 @@ jobs: steps: - name: checkout repository uses: actions/checkout@v2 + with: + submodules: recursive - name: generate code coverage run: |