diff --git a/.gitmodules b/.gitmodules index c62bff8..e1bc1d5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine_tests/engine-test-data"] path = tests/engine_tests/engine-test-data url = https://github.com/flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.12.1 diff --git a/src/engine.rs b/src/engine.rs index 5d3782d..423d260 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,29 +1,37 @@ use crate::engine_eval::context::{EngineEvaluationContext, FeatureContext}; use crate::engine_eval::result::{EvaluationResult, FlagResult, SegmentResult}; -use crate::engine_eval::segment_evaluator::is_context_in_segment; +use crate::engine_eval::segment_evaluator::{ + is_context_in_segment_with_inputs, ConditionInputs, ContextJson, +}; use crate::utils::hashing; -use std::collections::HashMap; +use std::cell::{Cell, OnceCell, RefCell}; +use std::collections::{HashMap, HashSet}; -/// Holds a feature context with its associated segment name for priority comparison -struct FeatureContextWithSegment { - feature_context: FeatureContext, - segment_name: String, -} +/// Reason given to flags whose evaluation depends on themselves. +pub const CIRCULAR_DEPENDENCY_REASON: &str = "ERROR; code=CIRCULAR_DEPENDENCY"; /// Helper to get priority or default fn get_priority_or_default(priority: Option) -> f64 { priority.unwrap_or(f64::INFINITY) // Weakest possible priority } -/// Gets matching segments and their overrides -fn get_matching_segments_and_overrides( - ec: &EngineEvaluationContext, +/// Whether a segment override takes precedence over the one held so far; the first seen wins a tie +fn wins_over(candidate: &FeatureContext, incumbent: Option<&FeatureContext>) -> bool { + incumbent.is_none_or(|incumbent| { + get_priority_or_default(candidate.priority) < get_priority_or_default(incumbent.priority) + }) +} + +/// Gets matching segments, and the winning override, if any, of every feature segments override +fn get_matching_segments_and_overrides<'a>( + ec: &'a EngineEvaluationContext, + resolver: &DependencyResolver, ) -> ( Vec, - HashMap, + HashMap<&'a str, Option>>, ) { let mut segments = Vec::new(); - let mut segment_feature_contexts: HashMap = HashMap::new(); + let mut segment_feature_contexts: HashMap<&str, Option> = HashMap::new(); // Sort segment keys for deterministic ordering let mut segment_keys: Vec<_> = ec.segments.keys().collect(); @@ -33,7 +41,15 @@ fn get_matching_segments_and_overrides( for segment_key in segment_keys { let segment_context = &ec.segments[segment_key]; - if !is_context_in_segment(ec, segment_context) { + let cycle_hits = resolver.cycle_hits.get(); + let matches = is_context_in_segment_with_inputs(ec, segment_context, resolver); + // A verdict reached on a flag that could not be resolved is not sound either way + if !matches || resolver.cycle_hits.get() != cycle_hits { + for override_fc in &segment_context.overrides { + segment_feature_contexts + .entry(override_fc.name.as_str()) + .or_insert(None); + } continue; } @@ -45,24 +61,20 @@ fn get_matching_segments_and_overrides( // Process segment overrides for override_fc in &segment_context.overrides { - let feature_name = &override_fc.name; - - // Check if we should update the segment feature context - let should_update = if let Some(existing) = segment_feature_contexts.get(feature_name) { - let existing_priority = get_priority_or_default(existing.feature_context.priority); - let override_priority = get_priority_or_default(override_fc.priority); - override_priority < existing_priority - } else { - true - }; - - if should_update { + let feature_name = override_fc.name.as_str(); + + let incumbent = segment_feature_contexts + .get(feature_name) + .and_then(Option::as_ref) + .map(|existing| existing.feature_context); + + if wins_over(override_fc, incumbent) { segment_feature_contexts.insert( - feature_name.clone(), - FeatureContextWithSegment { - feature_context: override_fc.clone(), - segment_name: segment_context.name.clone(), - }, + feature_name, + Some(SegmentOverride { + feature_context: override_fc, + segment_name: &segment_context.name, + }), ); } } @@ -74,36 +86,27 @@ fn get_matching_segments_and_overrides( /// Gets flag results from feature contexts and segment overrides fn get_flag_results( ec: &EngineEvaluationContext, - segment_feature_contexts: &HashMap, + segment_feature_contexts: &HashMap<&str, Option>, + identity_key: Option<&String>, ) -> HashMap { let mut flags = HashMap::new(); - // Get identity key if identity exists - // If identity key is not provided, construct it from environment key and identifier - let identity_key: Option = ec.identity.as_ref().map(|i| { - if i.key.is_empty() { - format!("{}_{}", ec.environment.key, i.identifier) - } else { - i.key.clone() - } - }); - // Process all features for feature_context in ec.features.values() { + let segment_override = segment_feature_contexts.get(feature_context.name.as_str()); // Check if we have a segment override for this feature - if let Some(segment_fc) = segment_feature_contexts.get(&feature_context.name) { + if let Some(Some(segment_fc)) = segment_override { // Use segment override with multivariate evaluation - let fc = &segment_fc.feature_context; + let fc = segment_fc.feature_context; let reason = format!("TARGETING_MATCH; segment={}", segment_fc.segment_name); - let flag_result = - get_flag_result_from_feature_context(fc, identity_key.as_ref(), reason); + let flag_result = get_flag_result_from_feature_context(fc, identity_key, reason); flags.insert(feature_context.name.clone(), flag_result); } else { // Use default feature context let flag_result = get_flag_result_from_feature_context( feature_context, - identity_key.as_ref(), - "DEFAULT".to_string(), + identity_key, + get_fallback_reason(segment_override.is_some()), ); flags.insert(feature_context.name.clone(), flag_result); } @@ -113,15 +116,211 @@ fn get_flag_results( } pub fn get_evaluation_result(ec: &EngineEvaluationContext) -> EvaluationResult { + // Get identity key if identity exists + // If identity key is not provided, construct it from environment key and identifier + let identity_key: Option = ec.identity.as_ref().map(|i| { + if i.key.is_empty() { + format!("{}_{}", ec.environment.key, i.identifier) + } else { + i.key.clone() + } + }); + + let resolver = DependencyResolver::new(ec, identity_key.as_ref()); + // Process segments - let (segments, segment_feature_contexts) = get_matching_segments_and_overrides(ec); + let (segments, segment_feature_contexts) = get_matching_segments_and_overrides(ec, &resolver); // Get flag results - let flags = get_flag_results(ec, &segment_feature_contexts); + let mut flags = get_flag_results(ec, &segment_feature_contexts, identity_key.as_ref()); + + // Flags resolved for segment conditions take precedence, as only they are guarded against cycles + flags.extend(resolver.resolved.take()); + for feature_name in resolver.cyclic.take() { + if let Some(flag) = flags.get_mut(&feature_name) { + flag.reason = CIRCULAR_DEPENDENCY_REASON.to_string(); + } + } EvaluationResult { flags, segments } } +/// Resolves, and memoises, the flags read by segment conditions through `$.flags` paths +struct DependencyResolver<'a> { + ec: &'a EngineEvaluationContext, + identity_key: Option<&'a String>, + context_json: ContextJson<'a>, + /// Segment keys, in evaluation order, of the segments overriding each feature; built when a + /// condition first reads a flag + segment_keys_by_feature_name: OnceCell>>, + segment_matches: RefCell>, + /// Features being resolved, outermost first + resolving: RefCell>, + resolved: RefCell>, + /// Features found to be in a dependency cycle + cyclic: RefCell>, + /// Incremented whenever a cycle is cut, so that evaluations reaching one can be told apart + cycle_hits: Cell, +} + +impl<'a> DependencyResolver<'a> { + fn new(ec: &'a EngineEvaluationContext, identity_key: Option<&'a String>) -> Self { + Self { + ec, + identity_key, + context_json: ContextJson::new(ec), + segment_keys_by_feature_name: OnceCell::new(), + segment_matches: RefCell::new(HashMap::new()), + resolving: RefCell::new(Vec::new()), + resolved: RefCell::new(HashMap::new()), + cyclic: RefCell::new(HashSet::new()), + cycle_hits: Cell::new(0), + } + } + + fn resolve_feature(&self, feature_name: &str) { + let cycle_start = self + .resolving + .borrow() + .iter() + .position(|name| name == feature_name); + if let Some(cycle_start) = cycle_start { + let resolving = self.resolving.borrow(); + self.cyclic + .borrow_mut() + .extend(resolving[cycle_start..].iter().cloned()); + self.cycle_hits.set(self.cycle_hits.get() + 1); + return; + } + + let Some(feature_context) = self.ec.features.get(feature_name) else { + return; + }; + + self.resolving.borrow_mut().push(feature_name.to_string()); + let segment_override = self.get_segment_override(feature_name); + self.resolving.borrow_mut().pop(); + + if self.cyclic.borrow().contains(feature_name) { + // Not a result another condition may match on + return; + } + + let flag_result = match segment_override { + Some(segment_override) => get_flag_result_from_feature_context( + segment_override.feature_context, + self.identity_key, + format!("TARGETING_MATCH; segment={}", segment_override.segment_name), + ), + None => get_flag_result_from_feature_context( + feature_context, + self.identity_key, + get_fallback_reason( + self.get_segment_keys_by_feature_name() + .contains_key(feature_name), + ), + ), + }; + self.resolved + .borrow_mut() + .insert(feature_name.to_string(), flag_result); + } + + fn matches_segment(&self, segment_key: &'a str) -> bool { + if let Some(matches) = self.segment_matches.borrow().get(segment_key) { + return *matches; + } + + let cycle_hits = self.cycle_hits.get(); + let matches = + is_context_in_segment_with_inputs(self.ec, &self.ec.segments[segment_key], self); + if self.cycle_hits.get() != cycle_hits { + // Reached by cutting a cycle, so the verdict rests on a flag that could not be resolved + return false; + } + + self.segment_matches + .borrow_mut() + .insert(segment_key, matches); + matches + } + + fn get_segment_override(&self, feature_name: &str) -> Option> { + let mut segment_override: Option> = None; + + let segment_keys = self + .get_segment_keys_by_feature_name() + .get(feature_name) + .map(Vec::as_slice) + .unwrap_or_default(); + for segment_key in segment_keys { + if !self.matches_segment(segment_key) { + continue; + } + let segment_context = &self.ec.segments[segment_key.as_str()]; + for override_fc in &segment_context.overrides { + if override_fc.name == feature_name + && wins_over( + override_fc, + segment_override.as_ref().map(|o| o.feature_context), + ) + { + segment_override = Some(SegmentOverride { + feature_context: override_fc, + segment_name: &segment_context.name, + }); + } + } + } + + segment_override + } + + fn get_segment_keys_by_feature_name(&self) -> &HashMap<&'a str, Vec<&'a String>> { + self.segment_keys_by_feature_name.get_or_init(|| { + let mut segment_keys: Vec<_> = self.ec.segments.keys().collect(); + segment_keys.sort(); + + let mut segment_keys_by_feature_name: HashMap<&str, Vec<&String>> = HashMap::new(); + for segment_key in segment_keys { + for override_fc in &self.ec.segments[segment_key].overrides { + segment_keys_by_feature_name + .entry(override_fc.name.as_str()) + .or_default() + .push(segment_key); + } + } + segment_keys_by_feature_name + }) + } +} + +impl ConditionInputs for DependencyResolver<'_> { + fn get_context_json(&self) -> Option<&serde_json::Value> { + self.context_json.get() + } + + fn get_flag(&self, feature_name: &str) -> Option { + if let Some(flag) = self.resolved.borrow().get(feature_name) { + return Some(flag.clone()); + } + self.resolve_feature(feature_name); + self.resolved.borrow().get(feature_name).cloned() + } +} + +/// A segment override borrowed from the evaluation context +struct SegmentOverride<'a> { + feature_context: &'a FeatureContext, + segment_name: &'a str, +} + +/// Gets the reason for a flag evaluated to its environment default: +/// `DEFAULT` if the feature has segment overrides that did not match, `STATIC` if it has none. +fn get_fallback_reason(is_targeted: bool) -> String { + if is_targeted { "DEFAULT" } else { "STATIC" }.to_string() +} + /// Creates a FlagResult from a FeatureContext fn get_flag_result_from_feature_context( feature_context: &FeatureContext, @@ -130,6 +329,7 @@ fn get_flag_result_from_feature_context( ) -> FlagResult { let mut reason = default_reason; let mut value = feature_context.value.clone(); + let mut variant = None; // Handle multivariate features if !feature_context.variants.is_empty() @@ -148,13 +348,16 @@ fn get_flag_result_from_feature_context( let object_ids = vec![feature_context.key.as_str(), identity_key.unwrap().as_str()]; let hash_percentage = hashing::get_hashed_percentage_for_object_ids(object_ids, 1); + variant = Some("control".to_string()); + // Select variant based on weighted distribution let mut cumulative_weight = 0.0; - for variant in &sorted_variants { - cumulative_weight += variant.weight; + for feature_value in &sorted_variants { + cumulative_weight += feature_value.weight; if (hash_percentage as f64) <= cumulative_weight { - value = variant.value.clone(); - reason = format!("SPLIT; weight={}", variant.weight); + value = feature_value.value.clone(); + variant = feature_value.key.clone(); + reason = format!("SPLIT; weight={}", feature_value.weight); break; } } @@ -164,6 +367,7 @@ fn get_flag_result_from_feature_context( enabled: feature_context.enabled, name: feature_context.name.clone(), value, + variant, reason, metadata: feature_context.metadata.clone(), } diff --git a/src/engine_eval/context.rs b/src/engine_eval/context.rs index 6ec41b4..a56805c 100644 --- a/src/engine_eval/context.rs +++ b/src/engine_eval/context.rs @@ -29,6 +29,9 @@ fn default_feature_type() -> String { /// Represents a multivariate value for a feature flag. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct FeatureValue { + /// A stable identifier for the variant, returned as the flag result's variant when selected. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub key: Option, /// The value of the feature. pub value: FlagsmithValue, /// The weight of the feature value variant, as a percentage number (i.e. 100.0). diff --git a/src/engine_eval/mappers.rs b/src/engine_eval/mappers.rs index c075f57..166843d 100644 --- a/src/engine_eval/mappers.rs +++ b/src/engine_eval/mappers.rs @@ -91,6 +91,7 @@ fn map_multivariate_values_to_variants( mv_values .iter() .map(|mv| FeatureValue { + key: None, value: mv.multivariate_feature_option.value.clone(), weight: mv.percentage_allocation as f64, priority: None, diff --git a/src/engine_eval/result.rs b/src/engine_eval/result.rs index 8f65635..409b9a8 100644 --- a/src/engine_eval/result.rs +++ b/src/engine_eval/result.rs @@ -23,12 +23,16 @@ pub struct FlagResult { /// The name of the feature. pub name: String, - /// The reason for this evaluation result (e.g., "DEFAULT", "TARGETING_MATCH; segment=name", "SPLIT; weight=50"). + /// The reason for this evaluation result (e.g., "STATIC", "DEFAULT", "TARGETING_MATCH; segment=name", "SPLIT; weight=50"). pub reason: String, /// The value of the feature flag. pub value: FlagsmithValue, + /// The key of the multivariate variant selected, "control" for the control bucket, or `None` when no split applied. + #[serde(default)] + pub variant: Option, + /// Metadata about the feature. #[serde(default)] pub metadata: FeatureMetadata, diff --git a/src/engine_eval/segment_evaluator.rs b/src/engine_eval/segment_evaluator.rs index 4fe1d78..0ca81ab 100644 --- a/src/engine_eval/segment_evaluator.rs +++ b/src/engine_eval/segment_evaluator.rs @@ -2,21 +2,76 @@ use super::context::{ Condition, ConditionOperator, EngineEvaluationContext, SegmentContext, SegmentRule, SegmentRuleType, }; +use super::result::FlagResult; use crate::types::FlagsmithValue; use crate::utils::hashing; use regex::Regex; use semver::Version; use serde_json_path::JsonPath; +use std::cell::OnceCell; + +const FLAGS_PATH_PREFIX: &str = "$.flags"; + +/// Provides what segment conditions read through JSONPath properties. +pub(crate) trait ConditionInputs { + /// Gets the evaluation context as JSON, or `None` if it cannot be serialised. + fn get_context_json(&self) -> Option<&serde_json::Value>; + + /// Gets the evaluated flag for a feature, or `None` if it cannot be resolved. + fn get_flag(&self, feature_name: &str) -> Option; +} + +/// The evaluation context as JSON, serialised at most once however many conditions read it +pub(crate) struct ContextJson<'a> { + ec: &'a EngineEvaluationContext, + json: OnceCell>, +} + +impl<'a> ContextJson<'a> { + pub(crate) fn new(ec: &'a EngineEvaluationContext) -> Self { + Self { + ec, + json: OnceCell::new(), + } + } + + pub(crate) fn get(&self) -> Option<&serde_json::Value> { + self.json + .get_or_init(|| serde_json::to_value(self.ec).ok()) + .as_ref() + } +} + +/// Reads flags as unresolvable, for segments evaluated outside a whole evaluation +impl ConditionInputs for ContextJson<'_> { + fn get_context_json(&self) -> Option<&serde_json::Value> { + self.get() + } + + fn get_flag(&self, _feature_name: &str) -> Option { + None + } +} /// Determines if the given evaluation context matches the segment rules pub fn is_context_in_segment(ec: &EngineEvaluationContext, segment: &SegmentContext) -> bool { + is_context_in_segment_with_inputs(ec, segment, &ContextJson::new(ec)) +} + +/// Determines if the given evaluation context matches the segment rules, reading JSONPath +/// properties from `inputs` +pub(crate) fn is_context_in_segment_with_inputs( + ec: &EngineEvaluationContext, + segment: &SegmentContext, + inputs: &dyn ConditionInputs, +) -> bool { if segment.rules.is_empty() { return false; } // All top-level rules must match for rule in &segment.rules { - if !context_matches_segment_rule(ec, rule, &segment.key) { + if !context_matches_segment_rule(ec, rule, &segment.key, inputs) { return false; } } @@ -29,10 +84,17 @@ fn context_matches_segment_rule( ec: &EngineEvaluationContext, rule: &SegmentRule, segment_key: &str, + inputs: &dyn ConditionInputs, ) -> bool { // Check conditions if present if !rule.conditions.is_empty() - && !matches_conditions_by_rule_type(ec, &rule.conditions, &rule.rule_type, segment_key) + && !matches_conditions_by_rule_type( + ec, + &rule.conditions, + &rule.rule_type, + segment_key, + inputs, + ) { return false; } @@ -42,7 +104,7 @@ fn context_matches_segment_rule( return true; } - matches_sub_rules_by_rule_type(ec, &rule.rules, &rule.rule_type, segment_key) + matches_sub_rules_by_rule_type(ec, &rule.rules, &rule.rule_type, segment_key, inputs) } fn matches_sub_rules_by_rule_type( @@ -50,10 +112,11 @@ fn matches_sub_rules_by_rule_type( sub_rules: &[SegmentRule], rule_type: &SegmentRuleType, segment_key: &str, + inputs: &dyn ConditionInputs, ) -> bool { let mut results = sub_rules .iter() - .map(|sr| context_matches_segment_rule(ec, sr, segment_key)); + .map(|sr| context_matches_segment_rule(ec, sr, segment_key, inputs)); match rule_type { SegmentRuleType::All => results.all(|r| r), SegmentRuleType::Any => results.any(|r| r), @@ -67,9 +130,10 @@ fn matches_conditions_by_rule_type( conditions: &[Condition], rule_type: &SegmentRuleType, segment_key: &str, + inputs: &dyn ConditionInputs, ) -> bool { for condition in conditions { - let condition_matches = context_matches_condition(ec, condition, segment_key); + let condition_matches = context_matches_condition(ec, condition, segment_key, inputs); match rule_type { SegmentRuleType::All => { @@ -99,9 +163,10 @@ fn context_matches_condition( ec: &EngineEvaluationContext, condition: &Condition, segment_key: &str, + inputs: &dyn ConditionInputs, ) -> bool { let context_value = if !condition.property.is_empty() { - get_context_value(ec, &condition.property) + get_context_value(ec, &condition.property, inputs) } else { None }; @@ -124,10 +189,14 @@ fn context_matches_condition( } /// Gets a value from the context by property name or JSONPath -fn get_context_value(ec: &EngineEvaluationContext, property: &str) -> Option { +fn get_context_value( + ec: &EngineEvaluationContext, + property: &str, + inputs: &dyn ConditionInputs, +) -> Option { // If property starts with $., try to parse it as a JSONPath expression if property.starts_with("$.") { - if let Some(value) = get_value_from_jsonpath(ec, property) { + if let Some(value) = get_value_from_jsonpath(property, inputs) { return Some(value); } // If JSONPath parsing fails, fall through to treat it as a trait name @@ -144,30 +213,89 @@ fn get_context_value(ec: &EngineEvaluationContext, property: &str) -> Option Option { - // Parse the JSONPath expression - let json_path = match JsonPath::parse(path) { - Ok(p) => p, - Err(_) => return None, - }; +fn get_value_from_jsonpath(path: &str, inputs: &dyn ConditionInputs) -> Option { + let json_path = JsonPath::parse(path).ok()?; + + if let Some((feature_name, flag_path)) = split_flag_path(path) { + let flag = inputs.get_flag(&feature_name)?; + let flag_json = serde_json::to_value(flag).ok()?; + let flag_json_path = JsonPath::parse(&flag_path).ok()?; + return query_scalar(&flag_json_path, &flag_json); + } - // Serialize the context to JSON - let context_json = match serde_json::to_value(ec) { - Ok(v) => v, - Err(_) => return None, + query_scalar(&json_path, inputs.get_context_json()?) +} + +/// Splits a path reading a single flag, e.g. `$.flags['my feature'].enabled`, into the +/// feature name and a path into that flag's result, e.g. `$.enabled`. +/// Expects a valid JSONPath; returns `None` for any path not selecting one flag by name. +fn split_flag_path(path: &str) -> Option<(String, String)> { + let is_blank = |c: char| matches!(c, ' ' | '\t' | '\n' | '\r'); + let rest = path + .strip_prefix(FLAGS_PATH_PREFIX)? + .trim_start_matches(is_blank); + + let (feature_name, flag_path) = if let Some(dot_name) = rest.strip_prefix('.') { + let name_end = dot_name + .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_' || !c.is_ascii())) + .unwrap_or(dot_name.len()); + if name_end == 0 { + return None; + } + (dot_name[..name_end].to_string(), &dot_name[name_end..]) + } else { + let selector = rest.strip_prefix('[')?.trim_start_matches(is_blank); + let quote = selector + .chars() + .next() + .filter(|c| *c == '\'' || *c == '"')?; + let literal = &selector[1..]; + let mut escaped = false; + let literal_end = literal.find(|c: char| { + let is_end = !escaped && c == quote; + escaped = !escaped && c == '\\'; + is_end + })?; + let flag_path = literal[literal_end + 1..] + .trim_start_matches(is_blank) + .strip_prefix(']')?; + ( + unescape_string_literal(&literal[..literal_end], quote)?, + flag_path, + ) }; - // Query the JSON using the path - let result = json_path.query(&context_json); + Some((feature_name, format!("${flag_path}"))) +} - // Get the first match (if any) - let node_list = result.all(); - if node_list.is_empty() { - return None; +/// Unescapes the contents of a JSONPath string literal, which share JSON's escapes plus `\'` +fn unescape_string_literal(contents: &str, quote: char) -> Option { + let mut json_string = String::with_capacity(contents.len() + 2); + json_string.push('"'); + let mut chars = contents.chars(); + while let Some(c) = chars.next() { + match c { + '\\' => match chars.next()? { + '\'' => json_string.push('\''), + escaped => { + json_string.push('\\'); + json_string.push(escaped); + } + }, + '"' if quote == '\'' => json_string.push_str("\\\""), + _ => json_string.push(c), + } } + json_string.push('"'); + serde_json::from_str(&json_string).ok() +} + +/// Gets the first scalar value a JSONPath selects in a JSON document +fn query_scalar(json_path: &JsonPath, json: &serde_json::Value) -> Option { + let node_list = json_path.query(json).all(); // Extract the value from the first match - let value = node_list[0]; + let value = *node_list.first()?; // Convert to FlagsmithValue based on the JSON type match value { diff --git a/tests/engine_eval_test.rs b/tests/engine_eval_test.rs index acf76c3..865a4cb 100644 --- a/tests/engine_eval_test.rs +++ b/tests/engine_eval_test.rs @@ -51,10 +51,10 @@ fn test_engine_evaluation() { .unwrap_or_else(|e| panic!("Failed to deserialize context for {}: {:?}", test_name, e)); // Get the evaluation result - let result = get_evaluation_result(&context); + let mut result = get_evaluation_result(&context); // Deserialize the expected result - let expected: EvaluationResult = serde_json::from_value(test_json["result"].clone()) + let mut expected: EvaluationResult = serde_json::from_value(test_json["result"].clone()) .unwrap_or_else(|e| { panic!( "Failed to deserialize expected result for {}: {:?}", @@ -62,6 +62,10 @@ fn test_engine_evaluation() { ) }); + // Matched segments are unordered, as are the segments of the context + result.segments.sort_by(|a, b| a.name.cmp(&b.name)); + expected.segments.sort_by(|a, b| a.name.cmp(&b.name)); + // Compare results - panic immediately on mismatch assert_eq!( result.flags, expected.flags, diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 4b29dc7..cb06740 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit cb067401093766e55c3354c1de7c73d9331334df diff --git a/tests/flag_dependency_test.rs b/tests/flag_dependency_test.rs new file mode 100644 index 0000000..c392994 --- /dev/null +++ b/tests/flag_dependency_test.rs @@ -0,0 +1,129 @@ +use flagsmith_flag_engine::engine::get_evaluation_result; +use flagsmith_flag_engine::engine_eval::{is_context_in_segment, EngineEvaluationContext}; +use rstest::*; +use serde_json::json; + +#[fixture] +fn prerequisite_name() -> &'static str { + "prerequisite" +} + +#[fixture] +fn context( + prerequisite_name: &str, + #[default("$.flags.prerequisite.enabled")] property: &str, +) -> EngineEvaluationContext { + serde_json::from_value(json!({ + "environment": {"key": "key", "name": "Environment"}, + "features": { + prerequisite_name: { + "key": "1", + "name": prerequisite_name, + "enabled": true, + "value": null + }, + "dependent": { + "key": "2", + "name": "dependent", + "enabled": false, + "value": "off" + } + }, + "segments": { + "1": { + "key": "1", + "name": "dependency", + "rules": [{ + "type": "ALL", + "conditions": [{ + "operator": "EQUAL", + "property": property, + "value": "true" + }] + }], + "overrides": [{ + "key": "2", + "name": "dependent", + "enabled": true, + "value": "on" + }] + } + } + })) + .unwrap() +} + +#[rstest] +#[case("it's on", r#"$.flags["it's on"].enabled"#)] +#[case("it's on", r"$.flags['it\'s on'].enabled")] +#[case(r#"say "on""#, r#"$.flags['say "on"'].enabled"#)] +#[case(r#"say "on""#, r#"$.flags["say \"on\""].enabled"#)] +#[case("café", r"$.flags['caf\u00e9'].enabled")] +#[case("café", "$.flags.café.enabled")] +#[case("prerequisite_2", "$.flags.prerequisite_2.enabled")] +#[case("prerequisite", "$.flags .prerequisite .enabled")] +#[case("prerequisite", "$.flags\t[ 'prerequisite' ]\n.enabled")] +#[case("prerequisite", "$.flags[\r\"prerequisite\" ]['enabled']")] +fn test_flag_named_in_any_jsonpath_notation_can_be_depended_on( + #[case] prerequisite_name: &str, + #[case] property: &str, +) { + // Given + let context = context(prerequisite_name, property); + + // When + let result = get_evaluation_result(&context); + + // Then + let dependent = &result.flags["dependent"]; + assert_eq!(dependent.value.value, "on"); + assert_eq!(dependent.reason, "TARGETING_MATCH; segment=dependency"); + assert_eq!(result.segments.len(), 1); +} + +#[rstest] +#[case("$.flags[?@.enabled == true].enabled")] +#[case("$.flags['prerequisite', 'dependent'].enabled")] +#[case("$.flags..enabled")] +#[case("$.flags.prerequisite")] +#[case("$.flagsprerequisite.enabled")] +fn test_flags_not_selected_by_name_cannot_be_depended_on(#[case] property: &str) { + // Given + let context = context(prerequisite_name(), property); + + // When + let result = get_evaluation_result(&context); + + // Then + let dependent = &result.flags["dependent"]; + assert_eq!(dependent.value.value, "off"); + assert_eq!(dependent.reason, "DEFAULT"); + assert!(result.segments.is_empty()); +} + +#[rstest] +fn test_segment_membership_alone_cannot_read_flags(context: EngineEvaluationContext) { + // Given + let segment = &context.segments["1"]; + + // When + let is_member = is_context_in_segment(&context, segment); + + // Then + assert!(!is_member); +} + +#[rstest] +fn test_segment_membership_alone_can_read_the_context( + #[with(prerequisite_name(), "$.features.prerequisite.enabled")] + context: EngineEvaluationContext, +) { + // Given + let segment = &context.segments["1"]; + + // When + let is_member = is_context_in_segment(&context, segment); + + // Then + assert!(is_member); +}