diff --git a/crates/context_aware_config/src/api/config/handlers.rs b/crates/context_aware_config/src/api/config/handlers.rs index 773b3756..06e97f3f 100644 --- a/crates/context_aware_config/src/api/config/handlers.rs +++ b/crates/context_aware_config/src/api/config/handlers.rs @@ -23,10 +23,7 @@ use diesel::{ }; use serde_json::{json, Map, Value}; use superposition_macros::{bad_argument, db_error, unexpected_error}; -use superposition_types::{ - get_db_cac_validation_type, result as superposition, Condition, Overrides, User, - ValidationType, -}; +use superposition_types::{result as superposition, Cac, Condition, Overrides, User}; use itertools::Itertools; use jsonschema::JSONSchema; @@ -335,7 +332,12 @@ fn construct_new_payload( log::error!("construct new payload Context not present"); Err(bad_argument!("Context not present")) }, - |val| Condition::new(val.to_owned(), ValidationType::CAC), + |val| { + Cac::::try_from(val.to_owned()).map_err(|err| { + log::error!("failed to decode condition with error : {}", err); + bad_argument!(err) + }) + }, )?; let override_ = res @@ -346,7 +348,12 @@ fn construct_new_payload( log::error!("construct new payload Override not present"); Err(bad_argument!("Override not present")) }, - |val| Overrides::new(val.to_owned(), ValidationType::CAC), + |val| { + Cac::::try_from(val.to_owned()).map_err(|err| { + log::error!("failed to decode override with error : {}", err); + bad_argument!(err) + }) + }, )?; return Ok(web::Json(PutReq { @@ -425,10 +432,18 @@ async fn reduce_config_key( Some(Value::Bool(to_be_deleted)), Some(override_val), ) => { - let override_val = Overrides::new( + let override_val = Cac::::try_from_db( override_val.as_object().unwrap_or(&Map::new()).clone(), - get_db_cac_validation_type(), - )?; + ) + .map_err(|err| { + log::error!( + "reduce_config_key: failed to decode overrides from db {}", + err + ); + unexpected_error!(err) + })? + .into_inner(); + if *to_be_deleted { if is_approve { let _ = delete_context_api(cid.clone(), user.clone(), conn); diff --git a/crates/context_aware_config/src/api/config/helpers.rs b/crates/context_aware_config/src/api/config/helpers.rs index e59bb076..c9010264 100644 --- a/crates/context_aware_config/src/api/config/helpers.rs +++ b/crates/context_aware_config/src/api/config/helpers.rs @@ -1,11 +1,9 @@ -use std::collections::HashMap; use super::types::{Config, Context}; use actix_web::web::Query; use serde_json::{json, Map, Value}; +use std::collections::HashMap; use superposition_macros::{bad_argument, unexpected_error}; -use superposition_types::{ - get_db_cac_validation_type, result as superposition, Overrides, -}; +use superposition_types::{result as superposition, Cac, Overrides}; pub fn filter_context( contexts: &[Context], @@ -53,7 +51,12 @@ pub fn filter_config_by_prefix( if !filtered_overrides_map.is_empty() { filtered_overrides.insert( key.clone(), - Overrides::new(filtered_overrides_map, get_db_cac_validation_type())?, + Cac::::try_from_db(filtered_overrides_map) + .map_err(|err| { + log::error!("filter_config_by_prefix : failed to decode overrides from db with error {}", err); + unexpected_error!(err) + })? + .into_inner(), ); } } diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index cbdeefbc..1f7cdeba 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -193,12 +193,13 @@ fn create_ctx_from_put_req( conn: &mut DBConnection, user: &User, ) -> superposition::Result { - let ctx_condition = req.context.to_owned(); + let ctx_condition = req.context.to_owned().into_inner(); let condition_val = json!(ctx_condition); - let ctx_override = json!(req.r#override.to_owned()); - validate_override_with_default_configs(conn, &req.r#override)?; + let r_override = req.r#override.clone().into_inner(); + let ctx_override = json!(r_override.to_owned()); + validate_override_with_default_configs(conn, &r_override)?; validate_condition_with_functions(conn, &ctx_condition)?; - validate_override_with_functions(conn, &req.r#override)?; + validate_override_with_functions(conn, &r_override)?; let dimension_schema_map = get_all_dimension_schema_map(conn)?; @@ -400,7 +401,7 @@ fn r#move( ) -> superposition::Result { use contexts::dsl; let req = req.into_inner(); - let ctx_condition = Value::Object(req.context); + let ctx_condition = Value::Object(req.context.into_inner().into()); let new_ctx_id = hash(&ctx_condition); let dimension_schema_map = get_all_dimension_schema_map(conn)?; let priority = validate_dimensions_and_calculate_priority( diff --git a/crates/context_aware_config/src/api/context/helpers.rs b/crates/context_aware_config/src/api/context/helpers.rs index 6c1c81a9..3705c8f1 100644 --- a/crates/context_aware_config/src/api/context/helpers.rs +++ b/crates/context_aware_config/src/api/context/helpers.rs @@ -24,10 +24,10 @@ type DBConnection = PooledConnection>; pub fn validate_condition_with_functions( conn: &mut DBConnection, - condition: &Condition, + context: &Condition, ) -> superposition::Result<()> { use dimensions::dsl; - let context = extract_dimensions(&condition)?; + let context = extract_dimensions(context)?; let dimensions_list: Vec = context.keys().cloned().collect(); let keys_function_array: Vec<(String, Option)> = dsl::dimensions .filter(dsl::dimension.eq_any(dimensions_list)) diff --git a/crates/context_aware_config/src/api/context/types.rs b/crates/context_aware_config/src/api/context/types.rs index 35042237..3d103998 100644 --- a/crates/context_aware_config/src/api/context/types.rs +++ b/crates/context_aware_config/src/api/context/types.rs @@ -1,18 +1,18 @@ use serde::{Deserialize, Serialize}; -use serde_json::{Map, Value}; -use superposition_types::{Condition, Overrides, ValidationType}; +use serde_json::Value; +use superposition_types::{Cac, Condition, Overrides}; #[cfg_attr(test, derive(Debug, PartialEq))] // Derive traits only when running tests #[derive(Deserialize, Clone)] pub struct PutReq { - pub context: Condition, - pub r#override: Overrides, + pub context: Cac, + pub r#override: Cac, } #[cfg_attr(test, derive(Debug, PartialEq))] // Derive traits only when running tests #[derive(Deserialize, Clone)] pub struct MoveReq { - pub context: Map, + pub context: Cac, } #[derive(Deserialize, Clone)] @@ -67,7 +67,8 @@ pub struct PriorityRecomputeResponse { #[cfg(test)] mod tests { use super::*; - use serde_json::json; + use serde_json::{json, Map}; + use superposition_types::Cac; #[test] fn test_deserialize_context_action() { @@ -91,16 +92,16 @@ mod tests { let mut expected_condition = Map::new(); expected_condition.insert("foo".to_string(), json!("bar")); expected_condition.insert("bar".to_string(), json!({ "baz": "baz"})); - let condition = Condition::new(expected_condition, ValidationType::CAC) + let context = Cac::::try_from(expected_condition) .expect("Invalid context condition"); let mut expected_override = Map::new(); expected_override.insert("foo".to_string(), json!("baz")); - let override_ = Overrides::new(expected_override, ValidationType::CAC) + let override_ = Cac::::try_from(expected_override) .expect("Invalid context override"); let expected_action = ContextAction::Put(PutReq { - context: condition, + context: context, r#override: override_, }); diff --git a/crates/context_aware_config/src/helpers.rs b/crates/context_aware_config/src/helpers.rs index a699be0a..85dfe205 100644 --- a/crates/context_aware_config/src/helpers.rs +++ b/crates/context_aware_config/src/helpers.rs @@ -25,10 +25,8 @@ use service_utils::{ service::types::AppState, }; -use superposition_macros::{db_error, validation_error}; -use superposition_types::{ - get_db_cac_validation_type, result as superposition, Condition, Overrides, -}; +use superposition_macros::{db_error, unexpected_error, validation_error}; +use superposition_types::{result as superposition, Cac, Condition, Overrides}; use std::collections::HashMap; @@ -331,14 +329,23 @@ pub fn generate_cac( let mut overrides: HashMap = HashMap::new(); for (id, condition, priority_, override_id, override_) in contexts_vec.iter() { - let condition = Condition::new( + let condition = Cac::::try_from_db( condition.as_object().unwrap_or(&Map::new()).clone(), - get_db_cac_validation_type(), - )?; - let override_ = Overrides::new( + ) + .map_err(|err| { + log::error!("generate_cac : failed to decode context from db {}", err); + unexpected_error!(err) + })? + .into_inner(); + + let override_ = Cac::::try_from_db( override_.as_object().unwrap_or(&Map::new()).clone(), - get_db_cac_validation_type(), - )?; + ) + .map_err(|err| { + log::error!("generate_cac : failed to decode overrides from db {}", err); + unexpected_error!(err) + })? + .into_inner(); let ctxt = Context { id: id.to_owned(), condition, diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 740d3628..8fca5078 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -21,8 +21,7 @@ use service_utils::service::types::{ }; use superposition_macros::{bad_argument, response_error, unexpected_error}; use superposition_types::{ - get_db_experiment_validation_type, result as superposition, Condition, Overrides, - SuperpositionUser, User, ValidationType, + result as superposition, Condition, Exp, Overrides, SuperpositionUser, User, }; use super::{ @@ -154,9 +153,10 @@ async fn create( // Checking if experiment has exactly 1 control variant, and // atleast 1 experimental variant check_variant_types(&variants)?; - let unique_override_keys: Vec = extract_override_keys(&variants[0].overrides) - .into_iter() - .collect(); + let unique_override_keys: Vec = + extract_override_keys(&variants[0].overrides.clone().into_inner()) + .into_iter() + .collect(); let unique_ids_of_variants_from_req: HashSet<&str> = HashSet::from_iter(variants.iter().map(|v| v.id.as_str())); @@ -170,10 +170,8 @@ async fn create( // Checking if all the variants are overriding the mentioned keys let variant_overrides = variants .iter() - .map(|variant| { - Overrides::new(variant.overrides.clone(), ValidationType::EXPERIMENTAL) - }) - .collect::>>()?; + .map(|variant| variant.overrides.clone().into_inner()) + .collect::>(); let are_valid_variants = check_variants_override_coverage(&variant_overrides, &unique_override_keys); if !are_valid_variants { @@ -185,7 +183,12 @@ async fn create( } // validating context - let exp_context = Condition::new(req.context.clone(), ValidationType::EXPERIMENTAL)?; + let exp_context = Exp::::try_from(req.context.clone()) + .map_err(|err| { + log::error!("failed to decode condition with error {}", err); + bad_argument!(err) + })? + .into_inner(); // validating experiment against other active experiments based on permission flags let flags = &state.experimentation_flags; @@ -267,7 +270,7 @@ async fn create( override_keys: unique_override_keys.to_vec(), traffic_percentage: 0, status: ExperimentStatusType::CREATED, - context: json!(req.context.clone()), + context: Value::Object(req.context.clone().into_inner().into()), variants: serde_json::to_value(variants).unwrap(), last_modified_by: user.get_email(), chosen_variant: None, @@ -364,7 +367,7 @@ pub async fn conclude( }; operations.push(ContextAction::MOVE((context_id, context_move_req))); } else { - for (key, val) in variant.overrides { + for (key, val) in variant.overrides.into_inner() { let create_req = HashMap::from([("value", val)]); let url = format!("{}/default-config/{}", state.cac_host, key); @@ -600,9 +603,10 @@ async fn update_overrides( let first_variant = variants.first().ok_or(bad_argument!( "Variant not found in request. Provide at least one entry in variant's list", ))?; - let override_keys = extract_override_keys(&first_variant.overrides) - .into_iter() - .collect(); + let override_keys = + extract_override_keys(&first_variant.overrides.clone().into_inner()) + .into_iter() + .collect(); // fetch the current variants of the experiment let experiment = experiments::experiments @@ -664,10 +668,8 @@ async fn update_overrides( let variant_overrides = new_variants .iter() - .map(|variant| { - Overrides::new(variant.overrides.clone(), ValidationType::EXPERIMENTAL) - }) - .collect::>>()?; + .map(|variant| variant.overrides.clone().into_inner()) + .collect::>(); let are_valid_variants = check_variants_override_coverage(&variant_overrides, &override_keys); if !are_valid_variants { @@ -678,14 +680,21 @@ async fn update_overrides( ) )?; } - let experiment_condition = Condition::new( + let experiment_condition = Exp::::try_from_db( experiment .context .as_object() - .unwrap_or(&Map::new()) - .clone(), - get_db_experiment_validation_type(), - )?; + .map_or_else(|| Map::new(), |ctx| ctx.clone()), + ) + .map_err(|err| { + log::error!( + "update_overrides : failed to decode condition from db with error {}", + err + ); + unexpected_error!(err) + })? + .into_inner(); + // validating experiment against other active experiments based on permission flags let flags = &state.experimentation_flags; let (valid, reason) = validate_experiment( diff --git a/crates/experimentation_platform/src/api/experiments/helpers.rs b/crates/experimentation_platform/src/api/experiments/helpers.rs index a6fca7ae..42cf962c 100644 --- a/crates/experimentation_platform/src/api/experiments/helpers.rs +++ b/crates/experimentation_platform/src/api/experiments/helpers.rs @@ -2,14 +2,12 @@ use super::types::{Variant, VariantType}; use crate::db::models::{Experiment, ExperimentStatusType}; use diesel::pg::PgConnection; use diesel::{BoolExpressionMethods, ExpressionMethods, QueryDsl, RunQueryDsl}; -use serde_json::{json, Map, Value}; +use serde_json::{Map, Value}; use service_utils::helpers::extract_dimensions; use service_utils::service::types::ExperimentationFlags; use std::collections::HashSet; -use superposition_macros::bad_argument; -use superposition_types::{ - get_db_experiment_validation_type, result as superposition, Condition, Overrides, -}; +use superposition_macros::{bad_argument, unexpected_error}; +use superposition_types::{result as superposition, Condition, Exp, Overrides}; pub fn check_variant_types(variants: &Vec) -> superposition::Result<()> { let mut experimental_variant_cnt = 0; @@ -125,14 +123,17 @@ pub fn is_valid_experiment( { let override_keys_set = HashSet::<&String>::from_iter(override_keys); for active_experiment in active_experiments { - let active_exp_context = Condition::new( + let active_exp_context = Exp::::try_from_db( active_experiment .context .as_object() - .unwrap_or(&Map::new()) - .to_owned(), - get_db_experiment_validation_type(), - )?; + .map_or_else(|| Map::new(), |ctx| ctx.clone()) + ) + .map_err(|err| { + log::error!("is_valid_experiment : failed to decode overrides from db with error {}", err); + unexpected_error!(err) + })? + .into_inner(); let are_overlapping = are_overlapping_contexts(context, &active_exp_context) .map_err(|e| { @@ -198,7 +199,7 @@ pub fn validate_experiment( } pub fn add_variant_dimension_to_ctx( - context_json: &Condition, + context: &Condition, variant: String, ) -> superposition::Result { let variant_condition = serde_json::json!({ @@ -207,7 +208,7 @@ pub fn add_variant_dimension_to_ctx( { "var": "variantIds" } ] }); - let context: Map = context_json.to_owned().into(); + let context: Map = context.clone().into(); if context.is_empty() { Ok(variant_condition) @@ -219,7 +220,7 @@ pub fn add_variant_dimension_to_ctx( "Failed parsing conditions as an array. Ensure the context provided obeys the rules of JSON logic" ))? .clone(), - None => vec![json!(context.clone())], + None => vec![Value::Object(context.clone())], }; conditions.push(variant_condition); diff --git a/crates/experimentation_platform/src/api/experiments/types.rs b/crates/experimentation_platform/src/api/experiments/types.rs index aea7af41..4896213a 100644 --- a/crates/experimentation_platform/src/api/experiments/types.rs +++ b/crates/experimentation_platform/src/api/experiments/types.rs @@ -2,6 +2,7 @@ use chrono::{DateTime, NaiveDateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use service_utils::helpers::deserialize_stringified_list; +use superposition_types::{Condition, Exp, Overrides}; use crate::db::models::{self, ExperimentStatusType}; @@ -17,7 +18,7 @@ pub struct Variant { pub variant_type: VariantType, pub context_id: Option, pub override_id: Option, - pub overrides: Map, + pub overrides: Exp, } /********** Experiment Create Req Types ************/ @@ -25,7 +26,7 @@ pub struct Variant { #[derive(Deserialize)] pub struct ExperimentCreateRequest { pub name: String, - pub context: Map, + pub context: Exp, pub variants: Vec, } @@ -154,7 +155,7 @@ pub struct RampRequest { #[derive(Deserialize, Debug)] pub struct VariantUpdateRequest { pub id: String, - pub overrides: Map, + pub overrides: Exp, } #[derive(Deserialize, Debug)] diff --git a/crates/experimentation_platform/tests/experimentation_tests.rs b/crates/experimentation_platform/tests/experimentation_tests.rs index a1643c86..4b317dd2 100644 --- a/crates/experimentation_platform/tests/experimentation_tests.rs +++ b/crates/experimentation_platform/tests/experimentation_tests.rs @@ -4,9 +4,7 @@ use experimentation_platform::db::models::{Experiment, ExperimentStatusType}; use serde_json::{json, Map, Value}; use service_utils::helpers::extract_dimensions; use service_utils::service::types::ExperimentationFlags; -use superposition_types::{ - result as superposition, Condition, Overrides, ValidationType, -}; +use superposition_types::{result as superposition, Cac, Condition, Exp, Overrides}; enum Dimensions { Os(String), @@ -97,11 +95,15 @@ fn test_extract_dimensions() -> Result<(), superposition::AppError> { Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let context_a = Condition::new(context_a.clone(), ValidationType::CAC)?; + let context_a = Cac::::try_from(context_a.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let context_b = single_dimension_ctx_gen(Dimensions::Client("testclient1".to_string())); - let context_b = Condition::new(context_b.clone(), ValidationType::CAC)?; + let context_b = Cac::::try_from(context_b.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let expected_dimensions_1 = serde_json::Map::from_iter(vec![ ("os".to_string(), json!("os1")), @@ -123,18 +125,26 @@ fn test_are_overlapping_contexts() -> Result<(), superposition::AppError> { Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let context_a = Condition::new(context_a.clone(), ValidationType::EXPERIMENTAL)?; + let context_a = Exp::::try_from(context_a.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let context_b = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient2".to_string()), ]); - let context_b = Condition::new(context_b.clone(), ValidationType::EXPERIMENTAL)?; + let context_b = Exp::::try_from(context_b.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let context_c = single_dimension_ctx_gen(Dimensions::Os("os1".to_string())); let context_d = single_dimension_ctx_gen(Dimensions::Os("os2".to_string())); - let context_c = Condition::new(context_c.clone(), ValidationType::EXPERIMENTAL)?; - let context_d = Condition::new(context_d.clone(), ValidationType::EXPERIMENTAL)?; + let context_c = Exp::::try_from(context_c.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); + let context_d = Exp::::try_from(context_d.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); // both contexts with same dimensions assert!(helpers::are_overlapping_contexts(&context_a, &context_a)?); @@ -153,35 +163,31 @@ fn test_are_overlapping_contexts() -> Result<(), superposition::AppError> { fn test_check_variants_override_coverage() -> Result<(), superposition::AppError> { let override_keys = vec!["key1".to_string(), "key2".to_string()]; let overrides = [ - Overrides::new( - Map::from_iter(vec![ - ("key1".to_string(), json!("value1")), - ("key2".to_string(), json!("value2")), - ]), - ValidationType::EXPERIMENTAL, - ), - // has one override key mi)ssing - Overrides::new( - Map::from_iter(vec![("key1".to_string(), json!("value1"))]), - ValidationType::EXPERIMENTAL, - ), - // has an unknown override) key - Overrides::new( - Map::from_iter(vec![("key3".to_string(), json!("value3"))]), - ValidationType::EXPERIMENTAL, - ), - // has an extra unknown ov)erride key - Overrides::new( - Map::from_iter(vec![ - ("key1".to_string(), json!("value1")), - ("key2".to_string(), json!("value2")), - ("key3".to_string(), json!("value3")), - ]), - ValidationType::EXPERIMENTAL, - ), + Exp::::try_from(Map::from_iter(vec![ + ("key1".to_string(), json!("value1")), + ("key2".to_string(), json!("value2")), + ])), + // has one override key missing + Exp::::try_from(Map::from_iter(vec![( + "key1".to_string(), + json!("value1"), + )])), + // has an unknown override key + Exp::::try_from(Map::from_iter(vec![( + "key3".to_string(), + json!("value3"), + )])), + // has an extra unknown override key + Exp::::try_from(Map::from_iter(vec![ + ("key1".to_string(), json!("value1")), + ("key2".to_string(), json!("value2")), + ("key3".to_string(), json!("value3")), + ])), ] .into_iter() - .collect::>>()?; + .map(|a| a.map(|b| b.into_inner())) + .collect::, String>>() + .map_err(superposition::AppError::BadArgument)?; assert!(helpers::check_variant_override_coverage( &overrides[0], @@ -211,8 +217,9 @@ fn test_is_valid_experiment_no_restrictions_overlapping_experiment( Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -247,8 +254,9 @@ fn test_is_valid_experiment_no_restrictions_non_overlapping_experiment( Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -288,8 +296,9 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: false, @@ -324,8 +333,9 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: false, @@ -360,8 +370,9 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: false, @@ -398,8 +409,9 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -434,8 +446,9 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -470,8 +483,9 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -508,8 +522,9 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -547,8 +562,9 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, @@ -586,8 +602,9 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); - let experiment_context = - Condition::new(experiment_context, ValidationType::EXPERIMENTAL)?; + let experiment_context = Exp::::try_from(experiment_context.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); let experiment_override_keys = vec!["key1".to_string(), "key2".to_string()]; let flags = ExperimentationFlags { allow_same_keys_overlapping_ctx: true, diff --git a/crates/service_utils/src/helpers.rs b/crates/service_utils/src/helpers.rs index 49cfe0df..4e1966c0 100644 --- a/crates/service_utils/src/helpers.rs +++ b/crates/service_utils/src/helpers.rs @@ -6,7 +6,7 @@ use log::info; use regex::Regex; use reqwest::header::{HeaderMap, HeaderName, HeaderValue}; use serde::de::{self, IntoDeserializer}; -use serde_json::{json, Map, Value}; +use serde_json::{Map, Value}; use std::{ env::VarError, fmt::{self, Display}, @@ -137,7 +137,7 @@ pub fn extract_dimensions(context: &Condition) -> result::Result vec![json!(*context)], + None => vec![Value::Object(context.to_owned().into())], }; let mut dimension_tuples = Vec::new(); diff --git a/crates/superposition_types/src/lib.rs b/crates/superposition_types/src/lib.rs index 57a8aeae..2d5b938e 100644 --- a/crates/superposition_types/src/lib.rs +++ b/crates/superposition_types/src/lib.rs @@ -1,10 +1,10 @@ pub mod result; -use crate::result as superposition; use actix::fut::{ready, Ready}; use actix_web::{dev::Payload, error, FromRequest, HttpMessage, HttpRequest}; +use anyhow::anyhow; use derive_more::{AsRef, Deref, DerefMut, Into}; use log::error; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use serde_json::{json, Map, Value}; pub trait SuperpositionUser { @@ -78,124 +78,127 @@ impl FromRequest for User { } } -#[derive(Debug, Clone, PartialEq)] -pub enum ValidationType { - CAC, - EXPERIMENTAL, - #[cfg(feature = "disable_db_data_validation")] - DB, -} - -pub fn get_db_experiment_validation_type() -> ValidationType { - #[cfg(feature = "disable_db_data_validation")] - return ValidationType::DB; - #[cfg(not(feature = "disable_db_data_validation"))] - return ValidationType::EXPERIMENTAL; +#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +pub struct Cac(T); +impl Cac { + pub fn into_inner(self) -> T { + self.0 + } } -pub fn get_db_cac_validation_type() -> ValidationType { - #[cfg(feature = "disable_db_data_validation")] - return ValidationType::DB; - #[cfg(not(feature = "disable_db_data_validation"))] - return ValidationType::CAC; +#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +pub struct Exp(T); +impl Exp { + pub fn into_inner(self) -> T { + self.0 + } } -#[derive(Deserialize, Clone, AsRef, Deref, Debug, Eq, PartialEq, Serialize, Into)] -#[serde(try_from = "Map")] -pub struct Condition(Map); +macro_rules! impl_try_from_map { + ($wrapper:ident, $type:ident, $validate:expr) => { + impl TryFrom> for $wrapper<$type> { + type Error = String; -impl Condition { - pub fn new( - condition_map: Map, - validation_type: ValidationType, - ) -> superposition::Result { - match validation_type { - ValidationType::CAC => { - if condition_map.is_empty() { - log::error!("Condition validation error: Context is empty"); - return Err(superposition::AppError::BadArgument( - "Context should not be empty".to_owned(), - )); - } - jsonlogic::expression::Expression::from_json(&json!(condition_map)) - .map_err(|msg| { - log::error!("Condition validation error: {}", msg); - superposition::AppError::BadArgument(msg) - })?; + fn try_from(map: Map) -> Result { + Ok(Self($validate(map)?)) } - ValidationType::EXPERIMENTAL => { - let condition_val = json!(condition_map); - let ast = jsonlogic::expression::Expression::from_json(&condition_val) - .map_err(|msg| { - log::error!("Condition validation error: {}", msg); - superposition::AppError::BadArgument(msg) - })?; - let dimensions = ast.get_variable_names().map_err(|msg| { - log::error!("Error while parsing variable names : {}", msg); - superposition::AppError::BadArgument(msg) - })?; - if dimensions.contains("variantIds") { - log::error!( - "experiment's context should not contain variantIds dimension" - ); - return Err(superposition::AppError::BadArgument( - "experiment's context should not contain variantIds dimension" - .to_string(), - )); - } + } + + impl<'de> Deserialize<'de> for $wrapper<$type> { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let map = Map::::deserialize(deserializer)?; + Self::try_from(map).map_err(serde::de::Error::custom) } - #[cfg(feature = "disable_db_data_validation")] - ValidationType::DB => (), - }; - Ok(Self(condition_map)) - } -} + } -impl TryFrom> for Condition { - type Error = String; - fn try_from(s: Map) -> Result { - Condition::new(s, ValidationType::CAC).map_err(|err| err.message()) - } + impl $wrapper<$type> { + pub fn try_from_db(map: Map) -> Result { + #[cfg(feature = "disable_db_data_validation")] + return Ok(Self($type(map))); + #[cfg(not(feature = "disable_db_data_validation"))] + return Self::try_from(map); + } + } + }; } #[derive( Deserialize, Serialize, Clone, AsRef, Deref, DerefMut, Debug, Eq, PartialEq, Into, )] -#[serde(try_from = "Map")] pub struct Overrides(Map); impl Overrides { - pub fn new( - override_map: Map, - validation_type: ValidationType, - ) -> superposition::Result { - match validation_type { - ValidationType::CAC | ValidationType::EXPERIMENTAL => { - if override_map.is_empty() { - log::error!("Override validation error: Override is empty"); - return Err(superposition::AppError::BadArgument( - "Override should not be empty".to_owned(), - )); - } - } - #[cfg(feature = "disable_db_data_validation")] - ValidationType::DB => (), - }; - + fn validate_data(override_map: Map) -> Result { + if override_map.is_empty() { + log::error!("Override validation error: Override is empty"); + return Err("Override should not be empty".to_owned()); + } Ok(Self(override_map)) } } -impl TryFrom> for Overrides { - type Error = String; - fn try_from(s: Map) -> Result { - Overrides::new(s, ValidationType::CAC).map_err(|err| err.message()) +impl IntoIterator for Overrides { + type Item = (String, Value); + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() } } +impl_try_from_map!(Cac, Overrides, Overrides::validate_data); +impl_try_from_map!(Exp, Overrides, Overrides::validate_data); + +#[derive(Deserialize, Serialize, Clone, AsRef, Deref, Debug, Eq, PartialEq, Into)] +pub struct Condition(Map); +impl Condition { + fn validate_data_for_cac(condition_map: Map) -> Result { + if condition_map.is_empty() { + log::error!("Condition validation error: Context is empty"); + return Err("Context should not be empty".to_owned()); + } + jsonlogic::expression::Expression::from_json(&json!(condition_map)).map_err( + |msg| { + log::error!("Condition validation error: {}", msg); + msg + }, + )?; + Ok(Self(condition_map)) + } + + fn validate_data_for_exp(condition_map: Map) -> Result { + let condition_val = json!(condition_map); + let ast = jsonlogic::expression::Expression::from_json(&condition_val).map_err( + |msg| { + log::error!("Condition validation error: {}", msg); + msg + }, + )?; + let dimensions = ast.get_variable_names().map_err(|msg| { + log::error!("Error while parsing variable names : {}", msg); + msg + })?; + if dimensions.contains("variantIds") { + log::error!("experiment's context should not contain variantIds dimension"); + return Err( + "experiment's context should not contain variantIds dimension" + .to_string(), + ); + } + Ok(Self(condition_map)) + } +} + +impl_try_from_map!(Cac, Condition, Condition::validate_data_for_cac); +impl_try_from_map!(Exp, Condition, Condition::validate_data_for_exp); + #[cfg(test)] mod tests { use super::*; + use result as superposition; use serde_json::json; #[test] @@ -247,7 +250,9 @@ mod tests { ) .unwrap(); let db_expected_condition = - Condition::new(db_request_condition_map, get_db_cac_validation_type())?; + Cac::::try_from_db(db_request_condition_map) + .map_err(|err| superposition::AppError::UnexpectedError(anyhow!(err)))? + .into_inner(); assert_eq!(db_condition, db_expected_condition); let default_condition = serde_json::from_str::( @@ -255,7 +260,9 @@ mod tests { ) .unwrap(); let default_expected_condition = - Condition::new(default_request_condition_map, ValidationType::CAC)?; + Cac::::try_from(default_request_condition_map) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); assert_eq!(default_condition, default_expected_condition); let exp_condition = serde_json::from_str::( @@ -263,7 +270,9 @@ mod tests { ) .unwrap(); let exp_expected_condition = - Condition::new(exp_request_condition_map, ValidationType::EXPERIMENTAL)?; + Exp::::try_from(exp_request_condition_map) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); assert_eq!(exp_condition, exp_expected_condition); Ok(()) @@ -299,13 +308,14 @@ mod tests { ]), )]); - let fail_condition = - serde_json::from_str::(&json!(request_condition_map).to_string()) - .map_err(|_| "Invalid operation".to_owned()); + let fail_condition = serde_json::from_str::>( + &json!(request_condition_map).to_string(), + ) + .map_err(|_| "Invalid operation".to_owned()); - let fail_exp_condition = - Condition::new(exp_condition_map, ValidationType::EXPERIMENTAL) - .map_err(|_| "variantIds should not be present".to_owned()); + let fail_exp_condition = Exp::::try_from(exp_condition_map.clone()) + .map(|a| a.into_inner()) + .map_err(|_| "variantIds should not be present".to_owned()); assert_eq!( json!(fail_condition) @@ -321,9 +331,10 @@ mod tests { true ); - let db_expected_condition = - Condition::new(request_condition_map.clone(), get_db_cac_validation_type()) - .map(|_| true)?; + let db_expected_condition = Exp::::try_from_db(exp_condition_map) + .map(|_| true) + .map_err(|err| superposition::AppError::UnexpectedError(anyhow!(err)))?; + assert_eq!(db_expected_condition, true); Ok(()) @@ -340,19 +351,22 @@ mod tests { let deserialize_overrides = serde_json::from_str::(&json!(override_map).to_string()).unwrap(); - let db_expected_overrides = - Overrides::new(override_map.clone(), get_db_cac_validation_type())?; + let db_expected_overrides = Cac::::try_from_db(override_map.clone()) + .map_err(|err| superposition::AppError::UnexpectedError(anyhow!(err)))? + .into_inner(); assert_eq!(deserialize_overrides, db_expected_overrides); - let exp_expected_overrides = - Overrides::new(override_map.clone(), ValidationType::EXPERIMENTAL)?; + let exp_expected_overrides = Exp::::try_from(override_map.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); assert_eq!(deserialize_overrides, exp_expected_overrides); - let default_expected_overrides = - Overrides::new(override_map.clone(), ValidationType::CAC)?; + let default_expected_overrides = Cac::::try_from(override_map.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); assert_eq!(deserialize_overrides, default_expected_overrides); - let empty_overrides = serde_json::from_str::( + let empty_overrides = serde_json::from_str::>( &json!(empty_override_map.clone()).to_string(), ) .map_err(|_| "override should not be empty".to_string());