Skip to content

Commit

Permalink
fix: cac,exp type wrapper
Browse files Browse the repository at this point in the history
  • Loading branch information
Pratik Mishra authored and Pratik Mishra committed Aug 1, 2024
1 parent 1778c05 commit 28574d7
Show file tree
Hide file tree
Showing 12 changed files with 322 additions and 253 deletions.
33 changes: 24 additions & 9 deletions crates/context_aware_config/src/api/config/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<Condition>::try_from(val.to_owned()).map_err(|err| {
log::error!("failed to decode condition with error : {}", err);
bad_argument!(err)
})
},
)?;

let override_ = res
Expand All @@ -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::<Overrides>::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 {
Expand Down Expand Up @@ -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::<Overrides>::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);
Expand Down
13 changes: 8 additions & 5 deletions crates/context_aware_config/src/api/config/helpers.rs
Original file line number Diff line number Diff line change
@@ -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],
Expand Down Expand Up @@ -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::<Overrides>::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(),
);
}
}
Expand Down
11 changes: 6 additions & 5 deletions crates/context_aware_config/src/api/context/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,13 @@ fn create_ctx_from_put_req(
conn: &mut DBConnection,
user: &User,
) -> superposition::Result<Context> {
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)?;

Expand Down Expand Up @@ -400,7 +401,7 @@ fn r#move(
) -> superposition::Result<PutResp> {
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(
Expand Down
4 changes: 2 additions & 2 deletions crates/context_aware_config/src/api/context/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ type DBConnection = PooledConnection<ConnectionManager<PgConnection>>;

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<String> = context.keys().cloned().collect();
let keys_function_array: Vec<(String, Option<String>)> = dsl::dimensions
.filter(dsl::dimension.eq_any(dimensions_list))
Expand Down
19 changes: 10 additions & 9 deletions crates/context_aware_config/src/api/context/types.rs
Original file line number Diff line number Diff line change
@@ -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<Condition>,
pub r#override: Cac<Overrides>,
}

#[cfg_attr(test, derive(Debug, PartialEq))] // Derive traits only when running tests
#[derive(Deserialize, Clone)]
pub struct MoveReq {
pub context: Map<String, Value>,
pub context: Cac<Condition>,
}

#[derive(Deserialize, Clone)]
Expand Down Expand Up @@ -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() {
Expand All @@ -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::<Condition>::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::<Overrides>::try_from(expected_override)
.expect("Invalid context override");

let expected_action = ContextAction::Put(PutReq {
context: condition,
context: context,
r#override: override_,
});

Expand Down
27 changes: 17 additions & 10 deletions crates/context_aware_config/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -331,14 +329,23 @@ pub fn generate_cac(
let mut overrides: HashMap<String, Overrides> = HashMap::new();

for (id, condition, priority_, override_id, override_) in contexts_vec.iter() {
let condition = Condition::new(
let condition = Cac::<Condition>::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::<Overrides>::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,
Expand Down
57 changes: 33 additions & 24 deletions crates/experimentation_platform/src/api/experiments/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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<String> = extract_override_keys(&variants[0].overrides)
.into_iter()
.collect();
let unique_override_keys: Vec<String> =
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()));
Expand All @@ -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::<superposition::Result<Vec<Overrides>>>()?;
.map(|variant| variant.overrides.clone().into_inner())
.collect::<Vec<Overrides>>();
let are_valid_variants =
check_variants_override_coverage(&variant_overrides, &unique_override_keys);
if !are_valid_variants {
Expand All @@ -185,7 +183,12 @@ async fn create(
}

// validating context
let exp_context = Condition::new(req.context.clone(), ValidationType::EXPERIMENTAL)?;
let exp_context = Exp::<Condition>::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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -664,10 +668,8 @@ async fn update_overrides(

let variant_overrides = new_variants
.iter()
.map(|variant| {
Overrides::new(variant.overrides.clone(), ValidationType::EXPERIMENTAL)
})
.collect::<superposition::Result<Vec<Overrides>>>()?;
.map(|variant| variant.overrides.clone().into_inner())
.collect::<Vec<Overrides>>();
let are_valid_variants =
check_variants_override_coverage(&variant_overrides, &override_keys);
if !are_valid_variants {
Expand All @@ -678,14 +680,21 @@ async fn update_overrides(
)
)?;
}
let experiment_condition = Condition::new(
let experiment_condition = Exp::<Condition>::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(
Expand Down
Loading

0 comments on commit 28574d7

Please sign in to comment.