Skip to content

Commit

Permalink
fix(core)/missing strategy variants no longer impacts other strategies (
Browse files Browse the repository at this point in the history
  • Loading branch information
sighphyre authored Dec 6, 2023
1 parent 969d5f0 commit 1f58217
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 7 deletions.
102 changes: 98 additions & 4 deletions unleash-yggdrasil/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ fn compile_variant_rule(
)
.iter()
.map(|(rule_string, strategy_variants, stickiness, group_id)| {
if strategy_variants.is_empty() {
return None;
};
let compiled_rule: Option<RuleFragment> = compile_rule(rule_string).ok();
compiled_rule.map(|rule| {
(
Expand Down Expand Up @@ -496,7 +493,11 @@ impl EngineState {
});

let variant = if let Some(strategy_variants) = strategy_variants {
self.resolve_variant(strategy_variants.0, strategy_variants.1, context)
if strategy_variants.0.is_empty() {
self.resolve_variant(&toggle.variants, &toggle.name, context)
} else {
self.resolve_variant(strategy_variants.0, strategy_variants.1, context)
}
} else {
self.resolve_variant(&toggle.variants, &toggle.name, context)
};
Expand Down Expand Up @@ -1666,4 +1667,97 @@ mod test {
assert_eq!(metrics.toggles.get("some-toggle").unwrap().no, 1);
assert!(metrics.toggles.get("parent-flag").is_none());
}

#[test]
pub fn strategy_variants_are_selected_over_base_variants_if_present_and_also_when_previous_failing_strategy_has_none(
) {
let raw_state = r#"
{
"version": 2,
"features": [
{
"name": "toggle1",
"type": "release",
"enabled": true,
"project": "TestProject20",
"stale": false,
"strategies": [
{
"name": "flexibleRollout",
"constraints": [
{
"contextName": "userId",
"operator": "IN",
"values": [
"17"
],
"inverted": false,
"caseInsensitive": false
}
],
"parameters": {
"groupId": "toggle1",
"rollout": "100",
"stickiness": "default"
},
"variants": []
},
{
"name": "flexibleRollout",
"constraints": [],
"parameters": {
"groupId": "toggle1",
"rollout": "100",
"stickiness": "default"
},
"variants": [
{
"name": "theselectedone",
"weight": 1000,
"overrides": [],
"stickiness": "default",
"weightType": "variable"
}
]
}
],
"variants": [
{
"name": "notselected",
"weight": 1000,
"overrides": [],
"stickiness": "default",
"weightType": "variable"
}
],
"description": null,
"impressionData": false
}
],
"query": {
"environment": "development",
"inlineSegmentConstraints": true
},
"meta": {
"revisionId": 12137,
"etag": "\"76d8bb0e:12137\"",
"queryHash": "76d8bb0e"
}
}
"#;

let feature_set: ClientFeatures = serde_json::from_str(raw_state).unwrap();
let mut engine = EngineState::default();
let context = Context {
..Context::default()
};

engine.take_state(feature_set).unwrap();

let results = engine.resolve_all(&context, &None);
let targeted_toggle = results.unwrap().get("toggle1").unwrap().clone();

assert!(targeted_toggle.enabled);
assert_eq!(targeted_toggle.variant.name, "theselectedone");
}
}
1 change: 0 additions & 1 deletion unleash-yggdrasil/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ impl EnrichedContext {
}
}


#[derive(Debug)]
pub enum SdkError {
StrategyEvaluationError,
Expand Down
5 changes: 3 additions & 2 deletions yggdrasilffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ mod tests {
};
use unleash_yggdrasil::{EngineState, ExtendedVariantDef};

use crate::{check_enabled, new_engine, Response, ResponseCode, check_variant};
use crate::{check_enabled, check_variant, new_engine, Response, ResponseCode};

#[test]
fn when_requesting_a_toggle_that_does_not_exist_then_a_response_with_no_error_and_not_found_is_returned(
Expand Down Expand Up @@ -563,7 +563,8 @@ mod tests {
let string_response =
check_variant(engine_ptr, toggle_name_ptr, context_ptr, results_ptr);
let response = CStr::from_ptr(string_response).to_str().unwrap();
let variant_response: Response<ExtendedVariantDef> = serde_json::from_str(response).unwrap();
let variant_response: Response<ExtendedVariantDef> =
serde_json::from_str(response).unwrap();

assert!(variant_response.status_code == ResponseCode::Ok);
let variant_response = variant_response.value.expect("Expected variant response");
Expand Down

0 comments on commit 1f58217

Please sign in to comment.