diff --git a/Cargo.lock b/Cargo.lock index 260daee0..4c80925a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4170,6 +4170,7 @@ dependencies = [ "anyhow", "derive_more", "diesel", + "jsonlogic", "log", "serde", "serde_json", diff --git a/crates/cac_client/Cargo.toml b/crates/cac_client/Cargo.toml index 09ee5756..de6655e9 100644 --- a/crates/cac_client/Cargo.toml +++ b/crates/cac_client/Cargo.toml @@ -20,7 +20,7 @@ strum_macros = { workspace = true } strum = { workspace = true } tokio = {version = "1.29.1", features = ["full"]} service_utils = { path = "../service_utils" } -superposition_types = {path = "../superposition_types"} +superposition_types = {path = "../superposition_types" } anyhow = { workspace = true } superposition_macros = { path = "../superposition_macros" } [lib] diff --git a/crates/context_aware_config/Cargo.toml b/crates/context_aware_config/Cargo.toml index 6f016cef..add6798a 100644 --- a/crates/context_aware_config/Cargo.toml +++ b/crates/context_aware_config/Cargo.toml @@ -65,5 +65,9 @@ jsonlogic = { workspace = true } superposition_types = { path = "../superposition_types" } superposition_macros = { path = "../superposition_macros" } + +[features] +disable_db_data_validation = ["superposition_types/disable_db_data_validation"] + [lints] workspace = true diff --git a/crates/context_aware_config/src/api/config/handlers.rs b/crates/context_aware_config/src/api/config/handlers.rs index b71c8c0e..90fcea77 100644 --- a/crates/context_aware_config/src/api/config/handlers.rs +++ b/crates/context_aware_config/src/api/config/handlers.rs @@ -23,7 +23,7 @@ use diesel::{ }; use serde_json::{json, Map, Value}; use superposition_macros::{bad_argument, db_error, unexpected_error}; -use superposition_types::{result as superposition, User}; +use superposition_types::{result as superposition, Cac, Condition, Overrides, User}; use itertools::Itertools; use jsonschema::JSONSchema; @@ -313,23 +313,50 @@ fn get_contextids_from_overrideid( Ok(res) } -fn construct_new_payload(req_payload: &Map) -> web::Json { +fn construct_new_payload( + req_payload: &Map, +) -> superposition::Result> { let mut res = req_payload.clone(); res.remove("to_be_deleted"); res.remove("override_id"); res.remove("id"); - if let Some(Value::Object(res_context)) = res.get("context") { - if let Some(Value::Object(res_override)) = res.get("override") { - return web::Json(PutReq { - context: res_context.to_owned(), - r#override: res_override.to_owned(), - }); - } - } - web::Json(PutReq { - context: Map::new(), - r#override: Map::new(), - }) + + let context = res + .get("context") + .and_then(|val| val.as_object()) + .map_or_else( + || { + log::error!("construct new payload: Context not present"); + Err(bad_argument!("Context not present")) + }, + |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 + .get("override") + .and_then(|val| val.as_object()) + .map_or_else( + || { + log::error!("construct new payload Override not present"); + Err(bad_argument!("Override not present")) + }, + |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 { + context: context, + r#override: override_, + })); } #[allow(clippy::too_many_arguments)] @@ -337,7 +364,7 @@ async fn reduce_config_key( user: User, conn: &mut PooledConnection>, mut og_contexts: Vec, - mut og_overrides: Map, + mut og_overrides: HashMap, check_key: &str, dimension_schema_map: &HashMap, default_config: Map, @@ -352,17 +379,15 @@ async fn reduce_config_key( )))?; let mut contexts_overrides_values = Vec::new(); - for (override_id, override_value) in og_overrides.clone() { - if let Value::Object(mut override_obj) = override_value { - if let Some(value_of_check_key) = override_obj.remove(check_key) { - let context_arr = get_contextids_from_overrideid( - og_contexts.clone(), - override_obj, - value_of_check_key.clone(), - &override_id, - )?; - contexts_overrides_values.extend(context_arr); - } + for (override_id, mut override_value) in og_overrides.clone() { + if let Some(value_of_check_key) = override_value.remove(check_key) { + let context_arr = get_contextids_from_overrideid( + og_contexts.clone(), + override_value.into(), + value_of_check_key.clone(), + &override_id, + )?; + contexts_overrides_values.extend(context_arr); } } @@ -371,7 +396,7 @@ async fn reduce_config_key( for (index, ctx) in contexts_overrides_values.iter().enumerate() { let priority = validate_dimensions_and_calculate_priority( "context", - &(ctx.0).condition, + &json!((ctx.0).condition), dimension_schema_map, )?; priorities.push((index, priority)) @@ -404,6 +429,18 @@ async fn reduce_config_key( Some(Value::Bool(to_be_deleted)), Some(override_val), ) => { + let override_val = Cac::::try_from_db( + override_val.as_object().unwrap_or(&Map::new()).clone(), + ) + .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); @@ -412,11 +449,12 @@ async fn reduce_config_key( } else { if is_approve { let _ = delete_context_api(cid.clone(), user.clone(), conn); - let put_req = construct_new_payload(request_payload); - let _ = put(put_req, conn, false, &user); + if let Ok(put_req) = construct_new_payload(request_payload) { + let _ = put(put_req, conn, false, &user); + } } - let new_id = hash(override_val); + let new_id = hash(&json!(override_val)); og_overrides.insert(new_id.clone(), override_val.clone()); let mut ctx_index = 0; @@ -560,7 +598,7 @@ async fn get_resolved_config( .contexts .into_iter() .map(|val| cac_client::Context { - condition: val.condition, + condition: json!(val.condition), override_with_keys: val.override_with_keys, }) .collect::>(); @@ -572,12 +610,17 @@ async fn get_resolved_config( .and_then(|val| MergeStrategy::from_str(val).ok()) .unwrap_or_default(); + let mut override_map = Map::new(); + for (key, val) in config.overrides.iter() { + override_map.insert(key.to_owned(), json!(val)); + } + let response = if let Some(Value::String(_)) = query_params_map.get("show_reasoning") { eval_cac_with_reasoning( config.default_configs, &cac_client_contexts, - &config.overrides, + &override_map, &query_params_map, merge_strategy, ) @@ -589,7 +632,7 @@ async fn get_resolved_config( eval_cac( config.default_configs, &cac_client_contexts, - &config.overrides, + &override_map, &query_params_map, merge_strategy, ) diff --git a/crates/context_aware_config/src/api/config/helpers.rs b/crates/context_aware_config/src/api/config/helpers.rs index f7a44e17..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::result as superposition; +use superposition_types::{result as superposition, Cac, Overrides}; pub fn filter_context( contexts: &[Context], @@ -14,7 +12,10 @@ pub fn filter_context( contexts .iter() .filter_map(|context| { - match jsonlogic::partial_apply(&context.condition, &json!(dimension_data)) { + match jsonlogic::partial_apply( + &json!(context.condition), + &json!(dimension_data), + ) { Ok(jsonlogic::PartialApplyOutcome::Resolved(Value::Bool(true))) | Ok(jsonlogic::PartialApplyOutcome::Ambiguous) => Some(context.clone()), _ => None, @@ -27,7 +28,7 @@ pub fn filter_config_by_prefix( config: &Config, prefix_list: &Vec, ) -> superposition::Result { - let mut filtered_overrides: Map = Map::new(); + let mut filtered_overrides: HashMap = HashMap::new(); let filtered_default_config: Map = config .default_configs @@ -41,21 +42,22 @@ pub fn filter_config_by_prefix( .collect(); for (key, overrides) in &config.overrides { - let overrides_map = overrides - .as_object() - .ok_or_else(|| { - log::error!("failed to decode overrides."); - unexpected_error!("failed to decode overrides.") - })? - .clone(); - - let filtered_overrides_map: Map = overrides_map + let override_map: Map = overrides.to_owned().into(); + let filtered_overrides_map: Map = override_map .into_iter() .filter(|(key, _)| filtered_default_config.contains_key(key)) .collect(); if !filtered_overrides_map.is_empty() { - filtered_overrides.insert(key.clone(), Value::Object(filtered_overrides_map)); + filtered_overrides.insert( + key.clone(), + 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(), + ); } } @@ -80,7 +82,7 @@ pub fn filter_config_by_dimensions( dimension_data: &Map, ) -> superposition::Result { let filtered_context = filter_context(&config.contexts, dimension_data); - let filtered_overrides: Map = filtered_context + let filtered_overrides: HashMap = filtered_context .iter() .flat_map(|ele| { let override_with_key = &ele.override_with_keys[0]; diff --git a/crates/context_aware_config/src/api/config/types.rs b/crates/context_aware_config/src/api/config/types.rs index 4ae24fb8..33283aae 100644 --- a/crates/context_aware_config/src/api/config/types.rs +++ b/crates/context_aware_config/src/api/config/types.rs @@ -1,17 +1,20 @@ +use std::collections::HashMap; + use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; +use superposition_types::{Condition, Overrides}; #[derive(Serialize, Deserialize)] pub struct Config { pub contexts: Vec, - pub overrides: Map, + pub overrides: HashMap, pub default_configs: Map, } #[derive(Serialize, Clone, Deserialize)] pub struct Context { pub id: String, - pub condition: Value, + pub condition: Condition, pub priority: i32, pub override_with_keys: [String; 1], } diff --git a/crates/context_aware_config/src/api/context/handlers.rs b/crates/context_aware_config/src/api/context/handlers.rs index 0d0df3a6..1f7cdeba 100644 --- a/crates/context_aware_config/src/api/context/handlers.rs +++ b/crates/context_aware_config/src/api/context/handlers.rs @@ -193,17 +193,19 @@ fn create_ctx_from_put_req( conn: &mut DBConnection, user: &User, ) -> superposition::Result { - let ctx_condition = Value::Object(req.context.to_owned()); - let ctx_override: Value = req.r#override.to_owned().into(); - validate_override_with_default_configs(conn, &req.r#override)?; + let ctx_condition = req.context.to_owned().into_inner(); + let condition_val = json!(ctx_condition); + 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)?; let priority = validate_dimensions_and_calculate_priority( "context", - &ctx_condition, + &condition_val, &dimension_schema_map, )?; @@ -211,11 +213,11 @@ fn create_ctx_from_put_req( return Err(bad_argument!("No dimension found in context")); } - let context_id = hash(&ctx_condition); + let context_id = hash(&condition_val); let override_id = hash(&ctx_override); Ok(Context { id: context_id.clone(), - value: ctx_condition, + value: condition_val, priority, override_id: override_id.to_owned(), override_: ctx_override.to_owned(), @@ -399,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 3a9043ed..3705c8f1 100644 --- a/crates/context_aware_config/src/api/context/helpers.rs +++ b/crates/context_aware_config/src/api/context/helpers.rs @@ -3,7 +3,7 @@ use base64::prelude::*; use service_utils::helpers::extract_dimensions; use std::str; use superposition_macros::{unexpected_error, validation_error}; -use superposition_types::result as superposition; +use superposition_types::{result as superposition, Condition}; use crate::api::functions::helpers::get_published_functions_by_names; use crate::validation_functions::execute_fn; @@ -24,7 +24,7 @@ type DBConnection = PooledConnection>; pub fn validate_condition_with_functions( conn: &mut DBConnection, - context: &Value, + context: &Condition, ) -> superposition::Result<()> { use dimensions::dsl; let context = extract_dimensions(context)?; diff --git a/crates/context_aware_config/src/api/context/types.rs b/crates/context_aware_config/src/api/context/types.rs index 8663e0f6..3d103998 100644 --- a/crates/context_aware_config/src/api/context/types.rs +++ b/crates/context_aware_config/src/api/context/types.rs @@ -1,17 +1,18 @@ use serde::{Deserialize, Serialize}; -use serde_json::{Map, Value}; +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: Map, - pub r#override: Map, + 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)] @@ -66,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() { @@ -87,16 +89,20 @@ mod tests { }) .to_string(); - let mut expected_context = Map::new(); - expected_context.insert("foo".to_string(), json!("bar")); - expected_context.insert("bar".to_string(), json!({ "baz": "baz"})); + let mut expected_condition = Map::new(); + expected_condition.insert("foo".to_string(), json!("bar")); + expected_condition.insert("bar".to_string(), json!({ "baz": "baz"})); + 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_ = Cac::::try_from(expected_override) + .expect("Invalid context override"); let expected_action = ContextAction::Put(PutReq { - context: expected_context, - r#override: expected_override, + context: context, + r#override: override_, }); let action_deserialized = diff --git a/crates/context_aware_config/src/helpers.rs b/crates/context_aware_config/src/helpers.rs index c966e4f1..85dfe205 100644 --- a/crates/context_aware_config/src/helpers.rs +++ b/crates/context_aware_config/src/helpers.rs @@ -3,7 +3,9 @@ use crate::{ db::{ models::ConfigVersion, schema::{ - config_versions, contexts::dsl as ctxt, default_configs::dsl as def_conf, + config_versions, + contexts::dsl::{self as ctxt}, + default_configs::dsl as def_conf, }, }, }; @@ -23,8 +25,8 @@ use service_utils::{ service::types::AppState, }; -use superposition_macros::{db_error, validation_error}; -use superposition_types::result as superposition; +use superposition_macros::{db_error, unexpected_error, validation_error}; +use superposition_types::{result as superposition, Cac, Condition, Overrides}; use std::collections::HashMap; @@ -323,21 +325,36 @@ pub fn generate_cac( db_error!(err) })?; - let (contexts, overrides) = contexts_vec.into_iter().fold( - (Vec::new(), Map::new()), - |(mut ctxts, mut overrides), - (id, condition, priority_, override_id, override_)| { - let ctxt = Context { - id, - condition, - priority: priority_, - override_with_keys: [override_id.to_owned()], - }; - ctxts.push(ctxt); - overrides.insert(override_id, override_); - (ctxts, overrides) - }, - ); + let mut contexts = Vec::new(); + let mut overrides: HashMap = HashMap::new(); + + for (id, condition, priority_, override_id, override_) in contexts_vec.iter() { + let condition = Cac::::try_from_db( + condition.as_object().unwrap_or(&Map::new()).clone(), + ) + .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(), + ) + .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, + priority: priority_.to_owned(), + override_with_keys: [override_id.to_owned()], + }; + contexts.push(ctxt); + overrides.insert(override_id.to_owned(), override_); + } let default_config_vec = def_conf::default_configs .select((def_conf::key, def_conf::value)) diff --git a/crates/experimentation_platform/Cargo.toml b/crates/experimentation_platform/Cargo.toml index 3377661f..0bb08abd 100644 --- a/crates/experimentation_platform/Cargo.toml +++ b/crates/experimentation_platform/Cargo.toml @@ -35,5 +35,8 @@ reqwest = { workspace = true } anyhow = { workspace = true } superposition_macros = { path = "../superposition_macros" } +[features] +disable_db_data_validation = ["superposition_types/disable_db_data_validation"] + [lints] workspace = true diff --git a/crates/experimentation_platform/src/api/experiments/handlers.rs b/crates/experimentation_platform/src/api/experiments/handlers.rs index 2280e360..8fca5078 100644 --- a/crates/experimentation_platform/src/api/experiments/handlers.rs +++ b/crates/experimentation_platform/src/api/experiments/handlers.rs @@ -20,7 +20,9 @@ use service_utils::service::types::{ AppHeader, AppState, CustomHeaders, DbConnection, Tenant, }; use superposition_macros::{bad_argument, response_error, unexpected_error}; -use superposition_types::{result as superposition, SuperpositionUser, User}; +use superposition_types::{ + result as superposition, Condition, Exp, Overrides, SuperpositionUser, User, +}; use super::{ helpers::{ @@ -37,7 +39,6 @@ use super::{ }; use crate::{ - api::experiments::helpers::validate_context, db::models::{EventLog, Experiment, ExperimentStatusType}, db::schema::{event_log::dsl as event_log, experiments::dsl as experiments}, }; @@ -152,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())); @@ -164,13 +166,12 @@ async fn create( "Variant ids are expected to be unique. Provide unqiue variant IDs" )); } - validate_override_keys(&unique_override_keys)?; // Checking if all the variants are overriding the mentioned keys let variant_overrides = variants .iter() - .map(|variant| &variant.overrides) - .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 { @@ -182,12 +183,17 @@ async fn create( } // validating context - validate_context(&req.context)?; + 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; let (valid, reason) = - validate_experiment(&req.context, &unique_override_keys, None, flags, &mut conn)?; + validate_experiment(&exp_context, &unique_override_keys, None, flags, &mut conn)?; if !valid { return Err(bad_argument!(reason)); } @@ -204,7 +210,7 @@ async fn create( variant.id = variant_id.to_string(); let updated_cacccontext = - add_variant_dimension_to_ctx(&req.context, variant_id.to_string())?; + add_variant_dimension_to_ctx(&exp_context, variant_id.to_string())?; let payload = ContextPutReq { context: updated_cacccontext @@ -264,7 +270,7 @@ async fn create( override_keys: unique_override_keys.to_vec(), traffic_percentage: 0, status: ExperimentStatusType::CREATED, - context: 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, @@ -361,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); @@ -597,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 @@ -661,8 +668,8 @@ async fn update_overrides( let variant_overrides = new_variants .iter() - .map(|variant| &variant.overrides) - .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 { @@ -673,11 +680,25 @@ async fn update_overrides( ) )?; } + let experiment_condition = Exp::::try_from_db( + experiment + .context + .as_object() + .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( - &experiment.context, + &experiment_condition, &override_keys, Some(experiment_id), flags, @@ -705,11 +726,11 @@ async fn update_overrides( // adding operations to create new updated variant contexts for variant in &mut new_variants { let updated_cacccontext = - add_variant_dimension_to_ctx(&experiment.context, variant.id.to_string()) + add_variant_dimension_to_ctx(&experiment_condition, variant.id.to_string()) .map_err(|e| { - log::error!("failed to add `variantIds` dimension to context: {e}"); - unexpected_error!("Something went wrong, failed to update experiment") - })?; + log::error!("failed to add `variantIds` dimension to context: {e}"); + unexpected_error!("Something went wrong, failed to update experiment") + })?; let payload = ContextPutReq { context: updated_cacccontext diff --git a/crates/experimentation_platform/src/api/experiments/helpers.rs b/crates/experimentation_platform/src/api/experiments/helpers.rs index ab951a4f..42cf962c 100644 --- a/crates/experimentation_platform/src/api/experiments/helpers.rs +++ b/crates/experimentation_platform/src/api/experiments/helpers.rs @@ -6,8 +6,8 @@ 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::result as superposition; +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; @@ -37,16 +37,6 @@ pub fn check_variant_types(variants: &Vec) -> superposition::Result<()> Ok(()) } -pub fn validate_context(context: &Value) -> superposition::Result<()> { - let dimensions = extract_dimensions(context)?; - if dimensions.contains_key("variantIds") { - return Err(bad_argument!( - "experiment's context should not contain variantIds dimension" - )); - } - Ok(()) -} - pub fn validate_override_keys(override_keys: &Vec) -> superposition::Result<()> { let mut key_set: HashSet<&str> = HashSet::new(); for key in override_keys { @@ -61,8 +51,8 @@ pub fn validate_override_keys(override_keys: &Vec) -> superposition::Res } pub fn are_overlapping_contexts( - context_a: &Value, - context_b: &Value, + context_a: &Condition, + context_b: &Condition, ) -> superposition::Result { let dimensions_a = extract_dimensions(context_a)?; let dimensions_b = extract_dimensions(context_b)?; @@ -91,7 +81,7 @@ pub fn are_overlapping_contexts( } pub fn check_variant_override_coverage( - variant_override: &Map, + variant_override: &Overrides, override_keys: &Vec, ) -> bool { if variant_override.keys().len() != override_keys.len() { @@ -107,7 +97,7 @@ pub fn check_variant_override_coverage( } pub fn check_variants_override_coverage( - variant_overrides: &Vec<&Map>, + variant_overrides: &Vec, override_keys: &Vec, ) -> bool { for variant_override in variant_overrides { @@ -120,7 +110,7 @@ pub fn check_variants_override_coverage( } pub fn is_valid_experiment( - context: &Value, + context: &Condition, override_keys: &[String], flags: &ExperimentationFlags, active_experiments: &[Experiment], @@ -133,8 +123,19 @@ 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 = Exp::::try_from_db( + active_experiment + .context + .as_object() + .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_experiment.context) + are_overlapping_contexts(context, &active_exp_context) .map_err(|e| { log::info!("experiment validation failed with error: {e}"); bad_argument!( @@ -175,7 +176,7 @@ pub fn is_valid_experiment( } pub fn validate_experiment( - context: &Value, + context: &Condition, override_keys: &[String], experiment_id: Option, flags: &ExperimentationFlags, @@ -198,7 +199,7 @@ pub fn validate_experiment( } pub fn add_variant_dimension_to_ctx( - context_json: &Value, + context: &Condition, variant: String, ) -> superposition::Result { let variant_condition = serde_json::json!({ @@ -207,10 +208,7 @@ pub fn add_variant_dimension_to_ctx( { "var": "variantIds" } ] }); - - let context = context_json.as_object().ok_or(bad_argument!( - "Context not an object. Ensure the context provided obeys the rules of JSON logic" - ))?; + let context: Map = context.clone().into(); if context.is_empty() { Ok(variant_condition) @@ -222,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![context_json.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 f6db7c71..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: Value, + 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 1175e10f..4b317dd2 100644 --- a/crates/experimentation_platform/tests/experimentation_tests.rs +++ b/crates/experimentation_platform/tests/experimentation_tests.rs @@ -4,7 +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::AppError; +use superposition_types::{result as superposition, Cac, Condition, Exp, Overrides}; enum Dimensions { Os(String), @@ -12,41 +12,45 @@ enum Dimensions { VariantIds(String), } -fn single_dimension_ctx_gen(value: Dimensions) -> serde_json::Value { +fn single_dimension_ctx_gen(value: Dimensions) -> Map { + let mut map = Map::new(); match value { - Dimensions::Os(os) => serde_json::json!({ - "==": [ + Dimensions::Os(os) => map.insert( + "==".to_string(), + json!([ {"var": "os"}, os - ] - }), - Dimensions::Client(client_id) => serde_json::json!({ - "==": [ + ]), + ), + Dimensions::Client(client_id) => map.insert( + "==".to_string(), + json!([ client_id, {"var": "clientId"} - ] - }), - Dimensions::VariantIds(id) => serde_json::json!({ - "in": [ + ]), + ), + Dimensions::VariantIds(id) => map.insert( + "in".to_string(), + json!([ id, {"var": "variantIds"}, - ] - }), - } + ]), + ), + }; + map } -fn multiple_dimension_ctx_gen(values: Vec) -> serde_json::Value { - let mut conditions: Vec = vec![]; +fn multiple_dimension_ctx_gen(values: Vec) -> Map { + let mut conditions: Vec> = vec![]; for val in values { conditions.push(single_dimension_ctx_gen(val)); } - - serde_json::json!({ "and": conditions }) + Map::from_iter(vec![("and".to_string(), json!(conditions))]) } fn experiment_gen( override_keys: &[String], - context: &Value, + context: &Map, status: ExperimentStatusType, variants: &Value, ) -> Experiment { @@ -61,7 +65,7 @@ fn experiment_gen( override_keys: override_keys.to_vec(), status, - context: context.clone(), + context: json!(context.clone()), variants: variants.clone(), chosen_variant: None, } @@ -72,7 +76,7 @@ fn test_duplicate_override_key_entries() { let override_keys = vec!["key1".to_string(), "key2".to_string(), "key1".to_string()]; assert!(matches!( helpers::validate_override_keys(&override_keys), - Err(AppError::BadArgument(_)) + Err(superposition::AppError::BadArgument(_)) )); } @@ -86,13 +90,20 @@ fn test_unique_override_key_entries() { } #[test] -fn test_extract_dimensions() -> Result<(), AppError> { +fn test_extract_dimensions() -> Result<(), superposition::AppError> { let context_a = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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 = 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")), @@ -109,17 +120,31 @@ fn test_extract_dimensions() -> Result<(), AppError> { } #[test] -fn test_are_overlapping_contexts() -> Result<(), AppError> { +fn test_are_overlapping_contexts() -> Result<(), superposition::AppError> { let context_a = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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 = 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 = 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)?); @@ -135,24 +160,34 @@ fn test_are_overlapping_contexts() -> Result<(), AppError> { } #[test] -fn test_check_variants_override_coverage() { +fn test_check_variants_override_coverage() -> Result<(), superposition::AppError> { let override_keys = vec!["key1".to_string(), "key2".to_string()]; let overrides = [ - Map::from_iter(vec![ + Exp::::try_from(Map::from_iter(vec![ ("key1".to_string(), json!("value1")), ("key2".to_string(), json!("value2")), - ]), - // has one override key mi)ssing - Map::from_iter(vec![("key1".to_string(), json!("value1"))]), - // has an unknown override) key - Map::from_iter(vec![("key3".to_string(), json!("value3"))]), - // has an extra unknown ov)erride key - Map::from_iter(vec![ + ])), + // 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() + .map(|a| a.map(|b| b.into_inner())) + .collect::, String>>() + .map_err(superposition::AppError::BadArgument)?; assert!(helpers::check_variant_override_coverage( &overrides[0], @@ -170,17 +205,21 @@ fn test_check_variants_override_coverage() { &overrides[3], &override_keys )); + Ok(()) } /************************* No Restrictions *****************************************/ #[test] fn test_is_valid_experiment_no_restrictions_overlapping_experiment( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -210,11 +249,14 @@ fn test_is_valid_experiment_no_restrictions_overlapping_experiment( #[test] fn test_is_valid_experiment_no_restrictions_non_overlapping_experiment( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -249,11 +291,14 @@ fn test_is_valid_experiment_no_restrictions_non_overlapping_experiment( #[test] fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_experiment_same_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -283,11 +328,14 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_experiment_one_same_key( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -317,11 +365,14 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_experiment_diff_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -353,11 +404,14 @@ fn test_is_valid_experiment_restrict_same_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_experiment_same_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -387,11 +441,14 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_experiment_one_diff_key( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -421,11 +478,14 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_experiment_diff_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -457,11 +517,14 @@ fn test_is_valid_experiment_restrict_diff_keys_overlapping_ctx_overlapping_exper #[test] fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlapping_experiment_same_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -494,11 +557,14 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi #[test] fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlapping_experiment_one_diff_key( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -531,11 +597,14 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi #[test] fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlapping_experiment_diff_keys( -) -> Result<(), AppError> { +) -> Result<(), superposition::AppError> { let experiment_context = multiple_dimension_ctx_gen(vec![ Dimensions::Os("os1".to_string()), Dimensions::Client("testclient1".to_string()), ]); + 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, @@ -565,31 +634,3 @@ fn test_is_valid_experiment_restrict_same_keys_non_overlapping_ctx_non_overlappi Ok(()) } - -#[test] -fn test_fail_context_with_variant_ids_dimensions() { - let experiment_context = multiple_dimension_ctx_gen(vec![ - Dimensions::Os("os1".to_string()), - Dimensions::Client("testclient1".to_string()), - Dimensions::VariantIds("123456789-variant1".to_string()), - ]); - - let result = helpers::validate_context(&experiment_context); - assert!(result.is_err()); - - let error_msg = result.unwrap_err(); - match error_msg { - AppError::BadArgument(msg) => assert_eq!(msg, "experiment's context should not contain variantIds dimension"), - _ => panic!("Not a AppError::BadArgument('experiment's context should not contain variantIds dimension')") - } -} - -#[test] -fn test_pass_context_without_variant_ids_dimensions() { - let experiment_context = multiple_dimension_ctx_gen(vec![ - Dimensions::Os("os1".to_string()), - Dimensions::Client("testclient1".to_string()), - ]); - - assert!(helpers::validate_context(&experiment_context).is_ok()); -} diff --git a/crates/service_utils/src/helpers.rs b/crates/service_utils/src/helpers.rs index 217389a2..4e1966c0 100644 --- a/crates/service_utils/src/helpers.rs +++ b/crates/service_utils/src/helpers.rs @@ -12,7 +12,7 @@ use std::{ fmt::{self, Display}, str::FromStr, }; -use superposition_types::result; +use superposition_types::{result, Condition}; const CONFIG_TAG_REGEX: &str = "^[a-zA-Z0-9_-]{1,64}$"; @@ -129,20 +129,15 @@ pub fn get_pod_info() -> (String, String) { (pod_id, deployment_id) } -pub fn extract_dimensions(context_json: &Value) -> result::Result> { +pub fn extract_dimensions(context: &Condition) -> result::Result> { // Assuming max 2-level nesting in context json logic - let context = context_json - .as_object() - .ok_or( - result::AppError::BadArgument("Error extracting dimensions, context not a valid JSON object. Provide a valid JSON context".into()) - )?; - let conditions = match context.get("and") { + let conditions: Vec = match (*context).get("and") { Some(conditions_json) => conditions_json .as_array() .ok_or(result::AppError::BadArgument("Error extracting dimensions, failed parsing conditions as an array. Ensure the context provided obeys the rules of JSON logic".into()))? .clone(), - None => vec![context_json.clone()], + None => vec![Value::Object(context.to_owned().into())], }; let mut dimension_tuples = Vec::new(); diff --git a/crates/superposition_types/Cargo.toml b/crates/superposition_types/Cargo.toml index 819b4ddb..0619c09c 100644 --- a/crates/superposition_types/Cargo.toml +++ b/crates/superposition_types/Cargo.toml @@ -18,6 +18,12 @@ derive_more = { workspace = true } thiserror = { workspace = true } diesel = { workspace = true } anyhow = { workspace = true } +jsonlogic = { workspace = true } + +[features] +disable_db_data_validation = [] [lints] workspace = true + + diff --git a/crates/superposition_types/src/lib.rs b/crates/superposition_types/src/lib.rs index 089ea350..fa56cd9b 100644 --- a/crates/superposition_types/src/lib.rs +++ b/crates/superposition_types/src/lib.rs @@ -1,8 +1,11 @@ pub mod result; 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_json::json; +use serde::{Deserialize, Deserializer, Serialize}; +use serde_json::{json, Map, Value}; pub trait SuperpositionUser { fn get_email(&self) -> String; @@ -74,3 +77,312 @@ impl FromRequest for User { } } } + +#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +pub struct Cac(T); +impl Cac { + pub fn into_inner(self) -> T { + self.0 + } +} + +#[derive(Clone, Debug, PartialEq, Copy, Serialize)] +pub struct Exp(T); +impl Exp { + pub fn into_inner(self) -> T { + self.0 + } +} + +macro_rules! impl_try_from_map { + ($wrapper:ident, $type:ident, $validate:expr) => { + impl TryFrom> for $wrapper<$type> { + type Error = String; + + fn try_from(map: Map) -> Result { + Ok(Self($validate(map)?)) + } + } + + 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) + } + } + + 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, +)] +pub struct Overrides(Map); + +impl Overrides { + 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 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] + fn ok_test_deserialize_condition() -> superposition::Result<()> { + let db_request_condition_map: Map = Map::from_iter(vec![( + "and".to_string(), + json!([ + { + "==": [ + { + "var": "clientId" + }, + "meesho" + ] + } + ]), + )]); + + let default_request_condition_map: Map = Map::from_iter(vec![( + "and".to_string(), + json!([ + { + "==": [ + { + "var": "os" + }, + "ios" + ] + } + ]), + )]); + + let exp_request_condition_map: Map = Map::from_iter(vec![( + "and".to_string(), + json!([ + { + "==": [ + { + "var": "clientId" + }, + "meesho" + ] + } + ]), + )]); + + let db_condition = serde_json::from_str::( + &json!(db_request_condition_map).to_string(), + ) + .unwrap(); + let db_expected_condition = + 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::( + &json!(default_request_condition_map).to_string(), + ) + .unwrap(); + let default_expected_condition = + 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::( + &json!(exp_request_condition_map).to_string(), + ) + .unwrap(); + let exp_expected_condition = + Exp::::try_from(exp_request_condition_map) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); + assert_eq!(exp_condition, exp_expected_condition); + + Ok(()) + } + + #[test] + fn fail_test_deserialize_condition() -> superposition::Result<()> { + let request_condition_map: Map = Map::from_iter(vec![( + "and".to_string(), + json!([ + { + ".": [ + { + "var": "clientId" + }, + "meesho" + ] + } + ]), + )]); + + let exp_condition_map: Map = Map::from_iter(vec![( + "and".to_string(), + json!([ + { + "in": [ + "variant-id", + { + "var": "variantIds" + } + ] + } + ]), + )]); + + let fail_condition = serde_json::from_str::>( + &json!(request_condition_map).to_string(), + ) + .map_err(|_| "Invalid operation".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) + .to_string() + .contains("Invalid operation"), + true + ); + + assert_eq!( + json!(fail_exp_condition) + .to_string() + .contains("variantIds should not be present"), + true + ); + + let db_expected_condition = Exp::::try_from_db(exp_condition_map) + .map(|_| true) + .map_err(|_| "variantIds should not be present".to_string()); + + assert_eq!( + json!(db_expected_condition) + .to_string() + .contains("variantIds should not be present"), + true + ); + + Ok(()) + } + + #[test] + fn test_deserialize_override() -> superposition::Result<()> { + let override_map = Map::from_iter(vec![ + ("key1".to_string(), json!("val1")), + ("key2".to_string(), json!(5)), + ]); + + let empty_override_map = Map::new(); + + let deserialize_overrides = + serde_json::from_str::(&json!(override_map).to_string()).unwrap(); + 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 = Exp::::try_from(override_map.clone()) + .map_err(superposition::AppError::BadArgument)? + .into_inner(); + assert_eq!(deserialize_overrides, exp_expected_overrides); + + 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::>( + &json!(empty_override_map.clone()).to_string(), + ) + .map_err(|_| "override should not be empty".to_string()); + + assert_eq!( + json!(empty_overrides) + .to_string() + .contains("override should not be empty"), + true + ); + + Ok(()) + } +} diff --git a/makefile b/makefile index 24e641f8..7c77cd8e 100644 --- a/makefile +++ b/makefile @@ -96,6 +96,9 @@ get-password: superposition: cargo run --color always --bin superposition --no-default-features --features=ssr +superposition_legacy: + cargo run --color always --bin superposition --no-default-features --features='ssr superposition_types/disable_db_data_validation context_aware_config/disable_db_data_validation experimentation_platform/disable_db_data_validation' + superposition_dev: # export DB_PASSWORD=`./docker-compose/localstack/get_db_password.sh` cargo watch -x 'run --color always --bin superposition --no-default-features --features=ssr' @@ -126,6 +129,14 @@ run: kill build done make superposition -e DOCKER_DNS=$(DOCKER_DNS) + +run_legacy: kill build + while ! make validate-psql-connection validate-aws-connection; \ + do echo "waiting for postgres, localstack bootup"; \ + sleep 0.5; \ + done + make superposition_legacy -e DOCKER_DNS=$(DOCKER_DNS) + ci-test: ci-setup cargo test npm run test