diff --git a/.sampo/changesets/group-and-mixed-targeting-local-eval.md b/.sampo/changesets/group-and-mixed-targeting-local-eval.md new file mode 100644 index 0000000..14b2ed8 --- /dev/null +++ b/.sampo/changesets/group-and-mixed-targeting-local-eval.md @@ -0,0 +1,7 @@ +--- +cargo/posthog-rs: minor +--- + +feat(flags): support group-targeted and mixed-targeting feature flags in local evaluation + +Adds local evaluation support for pure group flags (where `aggregation_group_type_index` is set at the flag level) and mixed-targeting flags (where individual conditions can override the aggregation per condition). `LocalEvaluator::evaluate_flag`, `evaluate_flag_simple`, and `evaluate_all_flags` now take `groups` and `group_properties` parameters; `match_feature_flag` and `match_feature_flag_with_context` have updated signatures. Backwards-incompatible at the public-API level — see PR description for migration notes. diff --git a/src/client/async_client.rs b/src/client/async_client.rs index ee2f65d..60a0e7b 100644 --- a/src/client/async_client.rs +++ b/src/client/async_client.rs @@ -236,9 +236,20 @@ impl Client { // Try local evaluation first if available if let Some(ref evaluator) = self.local_evaluator { - let empty = HashMap::new(); - let props = person_properties.as_ref().unwrap_or(&empty); - match evaluator.evaluate_flag(&key_str, &distinct_id_str, props) { + let empty_props = HashMap::new(); + let empty_groups: HashMap = HashMap::new(); + let empty_group_props: HashMap> = + HashMap::new(); + let props = person_properties.as_ref().unwrap_or(&empty_props); + let groups_ref = groups.as_ref().unwrap_or(&empty_groups); + let group_props_ref = group_properties.as_ref().unwrap_or(&empty_group_props); + match evaluator.evaluate_flag( + &key_str, + &distinct_id_str, + props, + groups_ref, + group_props_ref, + ) { Ok(Some(value)) => { debug!(flag = %key_str, ?value, "Flag evaluated locally"); return Ok(Some(value)); @@ -339,14 +350,32 @@ impl Client { Ok(payloads.get(&key_str).cloned()) } - /// Evaluate a feature flag locally (requires feature flags to be loaded) + /// Evaluate a feature flag locally (requires feature flags to be loaded). + /// + /// `groups` and `group_properties` are only consulted when the flag (or one + /// of its conditions) targets a group; pass empty maps for person flags. + #[allow(clippy::too_many_arguments)] pub fn evaluate_feature_flag_locally( &self, flag: &FeatureFlag, distinct_id: &str, person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, ) -> Result { - match_feature_flag(flag, distinct_id, person_properties) - .map_err(|e| Error::InconclusiveMatch(e.message)) + let group_type_mapping = self + .local_evaluator + .as_ref() + .map(|ev| ev.cache().get_group_type_mapping()) + .unwrap_or_default(); + match_feature_flag( + flag, + distinct_id, + person_properties, + groups, + group_properties, + &group_type_mapping, + ) + .map_err(|e| Error::InconclusiveMatch(e.message)) } } diff --git a/src/client/blocking.rs b/src/client/blocking.rs index 80ed3d7..5188203 100644 --- a/src/client/blocking.rs +++ b/src/client/blocking.rs @@ -232,9 +232,20 @@ impl Client { // Try local evaluation first if available if let Some(ref evaluator) = self.local_evaluator { - let empty = HashMap::new(); - let props = person_properties.as_ref().unwrap_or(&empty); - match evaluator.evaluate_flag(&key_str, &distinct_id_str, props) { + let empty_props = HashMap::new(); + let empty_groups: HashMap = HashMap::new(); + let empty_group_props: HashMap> = + HashMap::new(); + let props = person_properties.as_ref().unwrap_or(&empty_props); + let groups_ref = groups.as_ref().unwrap_or(&empty_groups); + let group_props_ref = group_properties.as_ref().unwrap_or(&empty_group_props); + match evaluator.evaluate_flag( + &key_str, + &distinct_id_str, + props, + groups_ref, + group_props_ref, + ) { Ok(Some(value)) => { debug!(flag = %key_str, ?value, "Flag evaluated locally"); return Ok(Some(value)); @@ -330,14 +341,32 @@ impl Client { Ok(payloads.get(&key_str).cloned()) } - /// Evaluate a feature flag locally (requires feature flags to be loaded) + /// Evaluate a feature flag locally (requires feature flags to be loaded). + /// + /// `groups` and `group_properties` are only consulted when the flag (or one + /// of its conditions) targets a group; pass empty maps for person flags. + #[allow(clippy::too_many_arguments)] pub fn evaluate_feature_flag_locally( &self, flag: &FeatureFlag, distinct_id: &str, person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, ) -> Result { - match_feature_flag(flag, distinct_id, person_properties) - .map_err(|e| Error::InconclusiveMatch(e.message)) + let group_type_mapping = self + .local_evaluator + .as_ref() + .map(|ev| ev.cache().get_group_type_mapping()) + .unwrap_or_default(); + match_feature_flag( + flag, + distinct_id, + person_properties, + groups, + group_properties, + &group_type_mapping, + ) + .map_err(|e| Error::InconclusiveMatch(e.message)) } } diff --git a/src/feature_flags.rs b/src/feature_flags.rs index 0230d6d..261f403 100644 --- a/src/feature_flags.rs +++ b/src/feature_flags.rs @@ -115,6 +115,10 @@ pub struct FeatureFlagFilters { /// JSON payloads associated with flag variants #[serde(default)] pub payloads: HashMap, + /// Group type index this flag targets at the flag level. `None` means person + /// targeting (or mixed, when individual conditions set their own). + #[serde(default)] + pub aggregation_group_type_index: Option, } /// A single condition group within a feature flag's targeting rules. @@ -130,6 +134,11 @@ pub struct FeatureFlagCondition { pub rollout_percentage: Option, /// Specific variant to serve for this condition (for variant overrides) pub variant: Option, + /// Optional per-condition aggregation override used by mixed-targeting flags. + /// When set, this condition targets the specified group type instead of the + /// flag-level aggregation. `None` means person targeting under a mixed flag. + #[serde(default)] + pub aggregation_group_type_index: Option, } /// A property filter used in feature flag targeting. @@ -219,11 +228,20 @@ impl CohortDefinition { } } -/// Context for evaluating properties that may depend on cohorts or other flags +/// Context for evaluating properties that may depend on cohorts or other flags. +/// +/// `groups`, `group_properties`, and `group_type_mapping` are used to resolve +/// group-targeted and mixed-targeting flags. A group condition (one whose +/// `aggregation_group_type_index` is set, either at the flag or condition level) +/// is bucketed on the group key and matched against group properties looked up +/// via the group type mapping. pub struct EvaluationContext<'a> { pub cohorts: &'a HashMap, pub flags: &'a HashMap, pub distinct_id: &'a str, + pub groups: &'a HashMap, + pub group_properties: &'a HashMap>, + pub group_type_mapping: &'a HashMap, } /// Configuration for multivariate (A/B/n) feature flags. @@ -398,17 +416,78 @@ pub fn get_matching_variant(flag: &FeatureFlag, distinct_id: &str) -> Option { + /// Use these for bucketing and property matching. + Use { + bucketing: String, + properties: &'a HashMap, + }, + /// Skip this condition (group type unknown or required group not passed in). + Skip, + /// Required group properties not provided — try other conditions, surface + /// inconclusive if nothing else matches. + Inconclusive, +} + +/// Resolve effective bucketing id and properties for a single condition based on +/// the (possibly per-condition) aggregation group type index. Pure-person flags +/// fall through with `distinct_id` and `person_properties`. Group conditions +/// (either flag-level or per-condition aggregation) require the corresponding +/// group key and group properties to be present. +fn resolve_condition_target<'a>( + condition: &FeatureFlagCondition, + flag_aggregation: Option, + distinct_id: &str, + person_properties: &'a HashMap, + groups: &HashMap, + group_properties: &'a HashMap>, + group_type_mapping: &HashMap, +) -> ConditionTarget<'a> { + // Per-condition aggregation falls back to the flag-level value when absent. + // The two together drive whether this condition is person- or group-targeted. + let effective_aggregation = condition.aggregation_group_type_index.or(flag_aggregation); + + match effective_aggregation { + None => ConditionTarget::Use { + bucketing: distinct_id.to_string(), + properties: person_properties, + }, + Some(idx) => { + let key = idx.to_string(); + let Some(group_type) = group_type_mapping.get(&key) else { + return ConditionTarget::Skip; + }; + let Some(group_key) = groups.get(group_type) else { + return ConditionTarget::Skip; + }; + let Some(props) = group_properties.get(group_type) else { + return ConditionTarget::Inconclusive; + }; + ConditionTarget::Use { + bucketing: group_key.clone(), + properties: props, + } + } + } +} + #[must_use = "feature flag evaluation result should be used"] +#[allow(clippy::too_many_arguments)] pub fn match_feature_flag( flag: &FeatureFlag, distinct_id: &str, - properties: &HashMap, + person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, + group_type_mapping: &HashMap, ) -> Result { if !flag.active { return Ok(FlagValue::Boolean(false)); } let conditions = &flag.filters.groups; + let flag_aggregation = flag.filters.aggregation_group_type_index; // Sort conditions to evaluate variant overrides first let mut sorted_conditions = conditions.clone(); @@ -417,7 +496,27 @@ pub fn match_feature_flag( let mut is_inconclusive = false; for condition in sorted_conditions { - match is_condition_match(flag, distinct_id, &condition, properties) { + let (effective_bucketing, effective_properties) = match resolve_condition_target( + &condition, + flag_aggregation, + distinct_id, + person_properties, + groups, + group_properties, + group_type_mapping, + ) { + ConditionTarget::Use { + bucketing, + properties, + } => (bucketing, properties), + ConditionTarget::Skip => continue, + ConditionTarget::Inconclusive => { + is_inconclusive = true; + continue; + } + }; + + match is_condition_match(flag, &effective_bucketing, &condition, effective_properties) { Ok(true) => { if let Some(variant_override) = &condition.variant { // Check if variant is valid @@ -435,7 +534,7 @@ pub fn match_feature_flag( } // Try to get matching variant or return true - if let Some(variant) = get_matching_variant(flag, distinct_id) { + if let Some(variant) = get_matching_variant(flag, &effective_bucketing) { return Ok(FlagValue::String(variant)); } return Ok(FlagValue::Boolean(true)); @@ -458,7 +557,7 @@ pub fn match_feature_flag( fn is_condition_match( flag: &FeatureFlag, - distinct_id: &str, + bucketing_id: &str, condition: &FeatureFlagCondition, properties: &HashMap, ) -> Result { @@ -471,7 +570,7 @@ fn is_condition_match( // If all properties match (or no properties), check rollout percentage if let Some(rollout_percentage) = condition.rollout_percentage { - let hash_value = hash_key(&flag.key, distinct_id, ROLLOUT_HASH_SALT); + let hash_value = hash_key(&flag.key, bucketing_id, ROLLOUT_HASH_SALT); if hash_value > (rollout_percentage / 100.0) { return Ok(false); } @@ -481,12 +580,15 @@ fn is_condition_match( } /// Match a feature flag with full context (cohorts, other flags) -/// This version supports cohort membership checks and flag dependency checks +/// This version supports cohort membership checks and flag dependency checks. +/// +/// `person_properties` carries person-level property values; group-level +/// properties are looked up from `ctx.group_properties` when a condition (or +/// the flag itself) targets a group via `aggregation_group_type_index`. #[must_use = "feature flag evaluation result should be used"] pub fn match_feature_flag_with_context( flag: &FeatureFlag, - distinct_id: &str, - properties: &HashMap, + person_properties: &HashMap, ctx: &EvaluationContext, ) -> Result { if !flag.active { @@ -494,6 +596,7 @@ pub fn match_feature_flag_with_context( } let conditions = &flag.filters.groups; + let flag_aggregation = flag.filters.aggregation_group_type_index; // Sort conditions to evaluate variant overrides first let mut sorted_conditions = conditions.clone(); @@ -502,7 +605,33 @@ pub fn match_feature_flag_with_context( let mut is_inconclusive = false; for condition in sorted_conditions { - match is_condition_match_with_context(flag, distinct_id, &condition, properties, ctx) { + let (effective_bucketing, effective_properties) = match resolve_condition_target( + &condition, + flag_aggregation, + ctx.distinct_id, + person_properties, + ctx.groups, + ctx.group_properties, + ctx.group_type_mapping, + ) { + ConditionTarget::Use { + bucketing, + properties, + } => (bucketing, properties), + ConditionTarget::Skip => continue, + ConditionTarget::Inconclusive => { + is_inconclusive = true; + continue; + } + }; + + match is_condition_match_with_context( + flag, + &effective_bucketing, + &condition, + effective_properties, + ctx, + ) { Ok(true) => { if let Some(variant_override) = &condition.variant { // Check if variant is valid @@ -520,7 +649,7 @@ pub fn match_feature_flag_with_context( } // Try to get matching variant or return true - if let Some(variant) = get_matching_variant(flag, distinct_id) { + if let Some(variant) = get_matching_variant(flag, &effective_bucketing) { return Ok(FlagValue::String(variant)); } return Ok(FlagValue::Boolean(true)); @@ -543,7 +672,7 @@ pub fn match_feature_flag_with_context( fn is_condition_match_with_context( flag: &FeatureFlag, - distinct_id: &str, + bucketing_id: &str, condition: &FeatureFlagCondition, properties: &HashMap, ctx: &EvaluationContext, @@ -557,7 +686,7 @@ fn is_condition_match_with_context( // If all properties match (or no properties), check rollout percentage if let Some(rollout_percentage) = condition.rollout_percentage { - let hash_value = hash_key(&flag.key, distinct_id, ROLLOUT_HASH_SALT); + let hash_value = hash_key(&flag.key, bucketing_id, ROLLOUT_HASH_SALT); if hash_value > (rollout_percentage / 100.0) { return Ok(false); } @@ -649,9 +778,17 @@ fn match_flag_dependency_property( InconclusiveMatchError::new(&format!("Flag '{}' not found in local cache", flag_key)) })?; - // Evaluate the dependent flag for this user (with empty properties to avoid recursion issues) + // Evaluate the dependent flag for this user (with empty properties to avoid recursion issues). + // Group context flows through from the outer ctx so dependent group/mixed flags can resolve. let empty_props = HashMap::new(); - let flag_value = match_feature_flag(flag, ctx.distinct_id, &empty_props)?; + let flag_value = match_feature_flag( + flag, + ctx.distinct_id, + &empty_props, + ctx.groups, + ctx.group_properties, + ctx.group_type_mapping, + )?; // Compare the flag value with the expected value let expected = &property.value; @@ -1136,14 +1273,24 @@ mod tests { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; let properties = HashMap::new(); - let result = match_feature_flag(&flag, "user-123", &properties).unwrap(); + let result = match_feature_flag( + &flag, + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); assert_eq!(result, FlagValue::Boolean(true)); } @@ -1175,6 +1322,7 @@ mod tests { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: Some(MultivariateFilter { variants: vec![ @@ -1189,11 +1337,20 @@ mod tests { ], }), payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; let properties = HashMap::new(); - let result = match_feature_flag(&flag, "user-123", &properties).unwrap(); + let result = match_feature_flag( + &flag, + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); match result { FlagValue::String(variant) => { @@ -1213,14 +1370,24 @@ mod tests { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; let properties = HashMap::new(); - let result = match_feature_flag(&flag, "user-123", &properties).unwrap(); + let result = match_feature_flag( + &flag, + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); assert_eq!(result, FlagValue::Boolean(false)); } @@ -1234,9 +1401,11 @@ mod tests { properties: vec![], rollout_percentage: Some(30.0), // 30% rollout variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; @@ -1245,7 +1414,15 @@ mod tests { // Test with multiple users to ensure distribution let mut enabled_count = 0; for i in 0..1000 { - let result = match_feature_flag(&flag, &format!("user-{}", i), &properties).unwrap(); + let result = match_feature_flag( + &flag, + &format!("user-{}", i), + &properties, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); if result == FlagValue::Boolean(true) { enabled_count += 1; } @@ -1363,11 +1540,20 @@ mod tests { groups: vec![], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; let properties = HashMap::new(); - let result = match_feature_flag(&flag, "user-123", &properties).unwrap(); + let result = match_feature_flag( + &flag, + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); assert_eq!(result, FlagValue::Boolean(false)); } @@ -1663,6 +1849,9 @@ mod tests { cohorts: &cohorts, flags: &HashMap::new(), distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; assert!(match_property_with_context(&prop, &properties, &ctx).unwrap()); @@ -1701,6 +1890,9 @@ mod tests { cohorts: &cohorts, flags: &HashMap::new(), distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; // User with status = active should NOT be in the blocked cohort (so not_in returns true) assert!(match_property_with_context(&prop, &properties, &ctx).unwrap()); @@ -1726,6 +1918,9 @@ mod tests { cohorts: &cohorts, flags: &HashMap::new(), distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; let result = match_property_with_context(&prop, &properties, &ctx); @@ -1748,9 +1943,11 @@ mod tests { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }, ); @@ -1768,6 +1965,9 @@ mod tests { cohorts: &HashMap::new(), flags: &flags, distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; // The prerequisite flag is enabled for user-123, so this should match @@ -1786,6 +1986,7 @@ mod tests { groups: vec![], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }, ); @@ -1803,6 +2004,9 @@ mod tests { cohorts: &HashMap::new(), flags: &flags, distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; // The flag is disabled, so checking for true should fail @@ -1822,6 +2026,7 @@ mod tests { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: Some(MultivariateFilter { variants: vec![ @@ -1836,6 +2041,7 @@ mod tests { ], }), payloads: HashMap::new(), + aggregation_group_type_index: None, }, }, ); @@ -1853,6 +2059,9 @@ mod tests { cohorts: &HashMap::new(), flags: &flags, distinct_id: "user-gets-control", // This distinct_id should deterministically get "control" + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; // The result depends on the hash - we just check it doesn't error @@ -1876,6 +2085,9 @@ mod tests { cohorts: &HashMap::new(), flags: &flags, distinct_id: "user-123", + groups: &HashMap::new(), + group_properties: &HashMap::new(), + group_type_mapping: &HashMap::new(), }; let result = match_property_with_context(&prop, &properties, &ctx); diff --git a/src/local_evaluation.rs b/src/local_evaluation.rs index 24837f6..02b1767 100644 --- a/src/local_evaluation.rs +++ b/src/local_evaluation.rs @@ -135,6 +135,11 @@ impl FlagCache { self.flags.read().unwrap().clone() } + /// Get the group type mapping (group type index → group type name). + pub fn get_group_type_mapping(&self) -> HashMap { + self.group_type_mapping.read().unwrap().clone() + } + pub fn clear(&self) { self.flags.write().unwrap().clear(); self.group_type_mapping.write().unwrap().clear(); @@ -533,29 +538,47 @@ impl LocalEvaluator { Self { cache } } - /// Evaluate a feature flag locally with full context support - /// This supports cohort membership checks and flag dependency evaluation - #[instrument(skip(self, person_properties), level = "trace")] + /// Access the underlying flag cache (e.g. to read group type mappings). + pub fn cache(&self) -> &FlagCache { + &self.cache + } + + /// Evaluate a feature flag locally with full context support. + /// + /// Supports cohort membership checks, flag dependency evaluation, and + /// group / mixed-targeting flags. `groups` and `group_properties` are + /// only consulted when the flag (or one of its conditions) targets a + /// group via `aggregation_group_type_index`; pass empty maps for + /// person-targeted flags. + #[instrument( + skip(self, person_properties, groups, group_properties), + level = "trace" + )] pub fn evaluate_flag( &self, key: &str, distinct_id: &str, person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, ) -> Result, InconclusiveMatchError> { match self.cache.get_flag(key) { Some(flag) => { - // Build evaluation context with cohorts and flags for dependency resolution + // Build evaluation context with cohorts, flags, and group info let cohorts = self.cache.get_cohort_definitions(); let flags = self.cache.get_flags_map(); + let group_type_mapping = self.cache.get_group_type_mapping(); let ctx = EvaluationContext { cohorts: &cohorts, flags: &flags, distinct_id, + groups, + group_properties, + group_type_mapping: &group_type_mapping, }; - let result = - match_feature_flag_with_context(&flag, distinct_id, person_properties, &ctx); + let result = match_feature_flag_with_context(&flag, person_properties, &ctx); trace!(key, ?result, "Local flag evaluation"); result.map(Some) } @@ -566,18 +589,31 @@ impl LocalEvaluator { } } - /// Evaluate a feature flag locally (simple version without cohort/flag dependency support) - /// Use this when you know the flag doesn't have cohort or flag dependency conditions - #[instrument(skip(self, person_properties), level = "trace")] + /// Evaluate a feature flag locally (simple version without cohort/flag dependency support). + /// Use this when you know the flag doesn't have cohort or flag dependency conditions. + #[instrument( + skip(self, person_properties, groups, group_properties), + level = "trace" + )] pub fn evaluate_flag_simple( &self, key: &str, distinct_id: &str, person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, ) -> Result, InconclusiveMatchError> { match self.cache.get_flag(key) { Some(flag) => { - let result = match_feature_flag(&flag, distinct_id, person_properties); + let group_type_mapping = self.cache.get_group_type_mapping(); + let result = match_feature_flag( + &flag, + distinct_id, + person_properties, + groups, + group_properties, + &group_type_mapping, + ); trace!(key, ?result, "Local flag evaluation (simple)"); result.map(Some) } @@ -588,28 +624,36 @@ impl LocalEvaluator { } } - /// Get all flags and evaluate them with full context support - #[instrument(skip(self, person_properties), level = "debug")] + /// Get all flags and evaluate them with full context support. + #[instrument( + skip(self, person_properties, groups, group_properties), + level = "debug" + )] pub fn evaluate_all_flags( &self, distinct_id: &str, person_properties: &HashMap, + groups: &HashMap, + group_properties: &HashMap>, ) -> HashMap> { let mut results = HashMap::new(); // Build evaluation context once for all flags let cohorts = self.cache.get_cohort_definitions(); let flags = self.cache.get_flags_map(); + let group_type_mapping = self.cache.get_group_type_mapping(); let ctx = EvaluationContext { cohorts: &cohorts, flags: &flags, distinct_id, + groups, + group_properties, + group_type_mapping: &group_type_mapping, }; for flag in self.cache.get_all_flags() { - let result = - match_feature_flag_with_context(&flag, distinct_id, person_properties, &ctx); + let result = match_feature_flag_with_context(&flag, person_properties, &ctx); results.insert(flag.key.clone(), result); } diff --git a/tests/test_local_evaluation.rs b/tests/test_local_evaluation.rs index 74f7e0f..5053db8 100644 --- a/tests/test_local_evaluation.rs +++ b/tests/test_local_evaluation.rs @@ -25,9 +25,11 @@ fn test_local_evaluation_basic() { properties: vec![], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; @@ -41,7 +43,13 @@ fn test_local_evaluation_basic() { // Test evaluation let properties = HashMap::new(); - let result = evaluator.evaluate_flag("test-flag", "user-123", &properties); + let result = evaluator.evaluate_flag( + "test-flag", + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + ); assert!(result.is_ok()); assert_eq!(result.unwrap(), Some(FlagValue::Boolean(true))); @@ -66,9 +74,11 @@ fn test_local_evaluation_with_properties() { }], rollout_percentage: Some(100.0), variant: None, + aggregation_group_type_index: None, }], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }; @@ -84,7 +94,13 @@ fn test_local_evaluation_with_properties() { let mut properties = HashMap::new(); properties.insert("plan".to_string(), json!("premium")); - let result = evaluator.evaluate_flag("premium-feature", "user-123", &properties); + let result = evaluator.evaluate_flag( + "premium-feature", + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + ); assert!(result.is_ok()); assert_eq!(result.unwrap(), Some(FlagValue::Boolean(true))); @@ -92,7 +108,13 @@ fn test_local_evaluation_with_properties() { let mut properties = HashMap::new(); properties.insert("plan".to_string(), json!("free")); - let result = evaluator.evaluate_flag("premium-feature", "user-456", &properties); + let result = evaluator.evaluate_flag( + "premium-feature", + "user-456", + &properties, + &HashMap::new(), + &HashMap::new(), + ); assert!(result.is_ok()); assert_eq!(result.unwrap(), Some(FlagValue::Boolean(false))); } @@ -103,7 +125,13 @@ fn test_local_evaluation_missing_flag() { let evaluator = LocalEvaluator::new(cache); let properties = HashMap::new(); - let result = evaluator.evaluate_flag("non-existent", "user-123", &properties); + let result = evaluator.evaluate_flag( + "non-existent", + "user-123", + &properties, + &HashMap::new(), + &HashMap::new(), + ); assert!(result.is_ok()); assert_eq!(result.unwrap(), None); @@ -208,6 +236,7 @@ fn test_cache_operations() { groups: vec![], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }, FeatureFlag { @@ -217,6 +246,7 @@ fn test_cache_operations() { groups: vec![], multivariate: None, payloads: HashMap::new(), + aggregation_group_type_index: None, }, }, ]; @@ -698,3 +728,262 @@ fn test_sync_no_etag_from_server() { "Flag should be in cache" ); } + +// ---- Group-targeted and mixed-targeting tests ---- + +fn mixed_flag() -> FeatureFlag { + FeatureFlag { + key: "mixed-flag".to_string(), + active: true, + filters: FeatureFlagFilters { + groups: vec![ + // Group condition: company plan == enterprise + FeatureFlagCondition { + properties: vec![Property { + key: "plan".to_string(), + value: json!("enterprise"), + operator: "exact".to_string(), + property_type: Some("group".to_string()), + }], + rollout_percentage: Some(100.0), + variant: None, + aggregation_group_type_index: Some(0), + }, + // Person condition: email == test@example.com + FeatureFlagCondition { + properties: vec![Property { + key: "email".to_string(), + value: json!("test@example.com"), + operator: "exact".to_string(), + property_type: Some("person".to_string()), + }], + rollout_percentage: Some(100.0), + variant: None, + aggregation_group_type_index: None, + }, + ], + multivariate: None, + payloads: HashMap::new(), + aggregation_group_type_index: None, // null = mixed + }, + } +} + +fn only_group_flag() -> FeatureFlag { + FeatureFlag { + key: "only-group-flag".to_string(), + active: true, + filters: FeatureFlagFilters { + groups: vec![FeatureFlagCondition { + properties: vec![Property { + key: "plan".to_string(), + value: json!("enterprise"), + operator: "exact".to_string(), + property_type: Some("group".to_string()), + }], + rollout_percentage: Some(100.0), + variant: None, + aggregation_group_type_index: None, + }], + multivariate: None, + payloads: HashMap::new(), + // Pure group flag at the flag level + aggregation_group_type_index: Some(0), + }, + } +} + +fn cache_with(flag: FeatureFlag) -> FlagCache { + let cache = FlagCache::new(); + let mut group_type_mapping = HashMap::new(); + group_type_mapping.insert("0".to_string(), "company".to_string()); + cache.update(LocalEvaluationResponse { + flags: vec![flag], + group_type_mapping, + cohorts: HashMap::new(), + }); + cache +} + +#[test] +fn test_pure_group_flag_matches_with_group_passed_in() { + let evaluator = LocalEvaluator::new(cache_with(only_group_flag())); + let mut groups = HashMap::new(); + groups.insert("company".to_string(), "acme".to_string()); + let mut group_props = HashMap::new(); + let mut acme_props = HashMap::new(); + acme_props.insert("plan".to_string(), json!("enterprise")); + group_props.insert("company".to_string(), acme_props); + + let result = evaluator + .evaluate_flag( + "only-group-flag", + "any-distinct-id", + &HashMap::new(), + &groups, + &group_props, + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(true))); +} + +#[test] +fn test_pure_group_flag_returns_false_when_groups_not_passed() { + // Pure group flag with no groups passed in: skip the condition, return false. + let evaluator = LocalEvaluator::new(cache_with(only_group_flag())); + let result = evaluator + .evaluate_flag( + "only-group-flag", + "any-distinct-id", + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(false))); +} + +#[test] +fn test_mixed_flag_person_condition_matches_when_no_groups() { + let evaluator = LocalEvaluator::new(cache_with(mixed_flag())); + let mut person = HashMap::new(); + person.insert("email".to_string(), json!("test@example.com")); + + let result = evaluator + .evaluate_flag( + "mixed-flag", + "user-1", + &person, + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(true))); +} + +#[test] +fn test_mixed_flag_group_condition_matches_with_group_props() { + let evaluator = LocalEvaluator::new(cache_with(mixed_flag())); + let mut groups = HashMap::new(); + groups.insert("company".to_string(), "acme".to_string()); + let mut group_props = HashMap::new(); + let mut acme = HashMap::new(); + acme.insert("plan".to_string(), json!("enterprise")); + group_props.insert("company".to_string(), acme); + let mut person = HashMap::new(); + person.insert("email".to_string(), json!("nope@example.com")); + + let result = evaluator + .evaluate_flag("mixed-flag", "user-2", &person, &groups, &group_props) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(true))); +} + +#[test] +fn test_mixed_flag_no_match_when_both_fail() { + let evaluator = LocalEvaluator::new(cache_with(mixed_flag())); + let mut groups = HashMap::new(); + groups.insert("company".to_string(), "acme".to_string()); + let mut group_props = HashMap::new(); + let mut acme = HashMap::new(); + acme.insert("plan".to_string(), json!("free")); + group_props.insert("company".to_string(), acme); + let mut person = HashMap::new(); + person.insert("email".to_string(), json!("nope@example.com")); + + let result = evaluator + .evaluate_flag("mixed-flag", "user-3", &person, &groups, &group_props) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(false))); +} + +#[test] +fn test_mixed_flag_only_group_condition_no_groups_returns_false() { + // Mixed flag with only group conditions and no groups passed: skip, return false. + let flag = FeatureFlag { + key: "mixed-only-group".to_string(), + active: true, + filters: FeatureFlagFilters { + groups: vec![FeatureFlagCondition { + properties: vec![Property { + key: "plan".to_string(), + value: json!("enterprise"), + operator: "exact".to_string(), + property_type: Some("group".to_string()), + }], + rollout_percentage: Some(100.0), + variant: None, + aggregation_group_type_index: Some(0), + }], + multivariate: None, + payloads: HashMap::new(), + aggregation_group_type_index: None, + }, + }; + let evaluator = LocalEvaluator::new(cache_with(flag)); + let result = evaluator + .evaluate_flag( + "mixed-only-group", + "user-1", + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(false))); +} + +#[test] +fn test_group_condition_uses_group_key_for_bucketing() { + // Group condition at 50% rollout. Use two groups whose hash on + // ("rollout-flag", group_key) straddles 0.5, with a distinct_id whose hash + // is also out-of-bucket. If the matcher regressed to bucketing on + // distinct_id, the in-bucket assertion would fail. + let flag = FeatureFlag { + key: "rollout-flag".to_string(), + active: true, + filters: FeatureFlagFilters { + groups: vec![FeatureFlagCondition { + properties: vec![], + rollout_percentage: Some(50.0), + variant: None, + aggregation_group_type_index: None, + }], + multivariate: None, + payloads: HashMap::new(), + aggregation_group_type_index: Some(0), + }, + }; + let evaluator = LocalEvaluator::new(cache_with(flag)); + let distinct_id = "user-0"; // hash > 0.5 against "rollout-flag" + + // Pre-computed: hash("rollout-flag.company-7") ~ 0.118 → in bucket + let mut groups_in = HashMap::new(); + groups_in.insert("company".to_string(), "company-7".to_string()); + let mut group_props = HashMap::new(); + group_props.insert("company".to_string(), HashMap::new()); + let result = evaluator + .evaluate_flag( + "rollout-flag", + distinct_id, + &HashMap::new(), + &groups_in, + &group_props, + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(true))); + + // Pre-computed: hash("rollout-flag.company-2") ~ 0.803 → out of bucket + let mut groups_out = HashMap::new(); + groups_out.insert("company".to_string(), "company-2".to_string()); + let result = evaluator + .evaluate_flag( + "rollout-flag", + distinct_id, + &HashMap::new(), + &groups_out, + &group_props, + ) + .unwrap(); + assert_eq!(result, Some(FlagValue::Boolean(false))); +}