From 5894323b699834d6677c8ec8d6a30225abc59ffa Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Fri, 14 Nov 2025 12:59:38 +0530 Subject: [PATCH 1/5] wip: add new engine --- src/flagsmith/models.rs | 25 ++++++++++++++ tests/fixtures/environment.json | 25 ++++++++++++++ tests/fixtures/mod.rs | 60 ++------------------------------- tests/integration_test.rs | 26 ++++++++++++++ 4 files changed, 78 insertions(+), 58 deletions(-) diff --git a/src/flagsmith/models.rs b/src/flagsmith/models.rs index 5d26a2a..95c4611 100644 --- a/src/flagsmith/models.rs +++ b/src/flagsmith/models.rs @@ -1,5 +1,6 @@ use crate::flagsmith::analytics::AnalyticsProcessor; use core::f64; +use flagsmith_flag_engine::engine_eval::EvaluationResult; use flagsmith_flag_engine::features::FeatureState; use flagsmith_flag_engine::identities::Trait; use flagsmith_flag_engine::types::{FlagsmithValue, FlagsmithValueType}; @@ -115,6 +116,30 @@ impl Flags { }); } + pub fn from_evaluation_result( + result: &EvaluationResult, + analytics_processor: Option, + default_flag_handler: Option Flag>, + _identity_id: Option<&str>, + ) -> Flags { + let mut flags: HashMap = HashMap::new(); + for (feature_name, flag_result) in &result.flags { + let flag = Flag { + feature_name: flag_result.name.clone(), + is_default: false, + enabled: flag_result.enabled, + value: flag_result.value.clone(), + feature_id: flag_result.metadata.feature_id, + }; + flags.insert(feature_name.clone(), flag); + } + return Flags { + flags, + analytics_processor, + default_flag_handler, + }; + } + // Returns a vector of all `Flag` structs pub fn all_flags(&self) -> Vec { return self.flags.clone().into_values().collect(); diff --git a/tests/fixtures/environment.json b/tests/fixtures/environment.json index 321e40f..99dc1ad 100644 --- a/tests/fixtures/environment.json +++ b/tests/fixtures/environment.json @@ -54,5 +54,30 @@ "segment_id": null, "enabled": true } + ], + "identity_overrides": [ + { + "identifier": "overridden-id", + "identity_uuid": "0f21cde8-63c5-4e50-baca-87897fa6cd01", + "created_date": "2019-08-27T14:53:45.698555Z", + "updated_at": "2023-07-14 16:12:00.000000", + "environment_api_key": "B62qaMZNwfiqT76p38ggrQ", + "identity_features": [ + { + "id": 1, + "feature": { + "id": 1, + "name": "some_feature", + "type": "STANDARD" + }, + "featurestate_uuid": "1bddb9a5-7e59-42c6-9be9-625fa369749f", + "feature_state_value": "some-overridden-value", + "enabled": false, + "environment": 1, + "identity": null, + "feature_segment": null + } + ] + } ] } diff --git a/tests/fixtures/mod.rs b/tests/fixtures/mod.rs index 4600f2f..d393a66 100644 --- a/tests/fixtures/mod.rs +++ b/tests/fixtures/mod.rs @@ -12,64 +12,8 @@ pub const ENVIRONMENT_KEY: &str = "ser.test_environment_key"; #[fixture] pub fn environment_json() -> serde_json::Value { - serde_json::json!({ - "api_key": "B62qaMZNwfiqT76p38ggrQ", - "project": { - "name": "Test project", - "organisation": { - "feature_analytics": false, - "name": "Test Org", - "id": 1, - "persist_trait_data": true, - "stop_serving_flags": false - }, - "id": 1, - "hide_disabled_flags": false, - "segments": [ - { - "id": 1, - "name": "Test Segment", - "feature_states":[], - "rules": [ - { - "type": "ALL", - "conditions": [], - "rules": [ - { - "type": "ALL", - "rules": [], - "conditions": [ - { - "operator": "EQUAL", - "property_": "foo", - "value": "bar" - } - ] - } - ] - } - ] - } - ] - }, - "segment_overrides": [], - "id": 1, - "feature_states": [ - { - "multivariate_feature_state_values": [], - "feature_state_value": FEATURE_1_STR_VALUE, - "id": 1, - "featurestate_uuid": "40eb539d-3713-4720-bbd4-829dbef10d51", - "feature": { - "name": FEATURE_1_NAME, - "type": "STANDARD", - "id": FEATURE_1_ID - }, - "segment_id": null, - "enabled": true - } - ] - }) + let json_str = include_str!("environment.json"); + serde_json::from_str(json_str).unwrap() } #[fixture] diff --git a/tests/integration_test.rs b/tests/integration_test.rs index d466490..efae60f 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -762,3 +762,29 @@ fn test_get_identity_segments_with_valid_trait(local_eval_flagsmith: Flagsmith) assert_eq!(segments.len(), 1); assert_eq!(segments[0].name, "Test Segment"); } + +#[rstest] +fn test_get_identity_segments_filters_identity_override_segments(local_eval_flagsmith: Flagsmith) { + // Given - environment fixture includes identity override for "overridden-id" + // Use the overridden identity from the fixture + let identifier = "overridden-id"; + + // Test with traits that match the API segment (foo=bar) + let traits = vec![Trait { + trait_key: "foo".to_string(), + trait_value: FlagsmithValue { + value: "bar".to_string(), + value_type: FlagsmithValueType::String, + }, + }]; + + // When + let segments = local_eval_flagsmith + .get_identity_segments(identifier, Some(traits)) + .unwrap(); + + // Then - should only return API segments with source "api", + assert_eq!(segments.len(), 1, "Should only return API-sourced segments"); + assert_eq!(segments[0].name, "Test Segment", "Should return the matching API segment"); + assert_eq!(segments[0].id, 1, "Should have correct segment ID"); +} From 2b61f62a5970d3810251940a769fdadd19e462a6 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Mon, 17 Nov 2025 12:30:14 +0530 Subject: [PATCH 2/5] fix: remove segment for get_enviornment_flags --- src/flagsmith/mod.rs | 137 ++++++++++++++++++++++---------------- src/flagsmith/models.rs | 3 +- tests/fixtures/mod.rs | 68 +++++++++++++++++++ tests/integration_test.rs | 34 ++++++++++ 4 files changed, 181 insertions(+), 61 deletions(-) diff --git a/src/flagsmith/mod.rs b/src/flagsmith/mod.rs index 2a9f4ac..fad0114 100644 --- a/src/flagsmith/mod.rs +++ b/src/flagsmith/mod.rs @@ -1,11 +1,13 @@ use self::analytics::AnalyticsProcessor; use self::models::{Flag, Flags}; use super::error; -use flagsmith_flag_engine::engine; +use flagsmith_flag_engine::engine::get_evaluation_result; +use flagsmith_flag_engine::engine_eval::{ + add_identity_to_context, environment_to_context, EngineEvaluationContext, SegmentSource, +}; use flagsmith_flag_engine::environments::builders::build_environment_struct; use flagsmith_flag_engine::environments::Environment; -use flagsmith_flag_engine::identities::{Identity, Trait}; -use flagsmith_flag_engine::segments::evaluator::get_identity_segments; +use flagsmith_flag_engine::identities::Trait; use flagsmith_flag_engine::segments::Segment; use log::debug; use models::SDKTrait; @@ -70,7 +72,7 @@ pub struct Flagsmith { struct DataStore { environment: Option, - identities_with_overrides_by_identifier: HashMap, + evaluation_context: Option, } impl Flagsmith { @@ -122,7 +124,10 @@ impl Flagsmith { // Put the environment model behind mutex to // to share it safely between threads - let ds = Arc::new(Mutex::new(DataStore { environment: None, identities_with_overrides_by_identifier: HashMap::new() })); + let ds = Arc::new(Mutex::new(DataStore { + environment: None, + evaluation_context: None, + })); let (tx, rx) = mpsc::sync_channel::(1); let flagsmith = Flagsmith { @@ -138,14 +143,18 @@ impl Flagsmith { if flagsmith.options.offline_handler.is_some() { let mut data = flagsmith.datastore.lock().unwrap(); - data.environment = Some( - flagsmith - .options - .offline_handler - .as_ref() - .unwrap() - .get_environment(), - ) + let environment = flagsmith + .options + .offline_handler + .as_ref() + .unwrap() + .get_environment(); + + // Create evaluation context from offline environment + let eval_context = environment_to_context(environment.clone()); + data.evaluation_context = Some(eval_context); + + data.environment = Some(environment); } // Create a thread to update environment document @@ -176,9 +185,9 @@ impl Flagsmith { //Returns `Flags` struct holding all the flags for the current environment. pub fn get_environment_flags(&self) -> Result { let data = self.datastore.lock().unwrap(); - if data.environment.is_some() { - let environment = data.environment.as_ref().unwrap(); - return Ok(self.get_environment_flags_from_document(environment)); + if data.evaluation_context.is_some() { + let eval_context = data.evaluation_context.as_ref().unwrap(); + return Ok(self.get_environment_flags_from_document(eval_context)); } return self.default_handler_if_err(self.get_environment_flags_from_api()); } @@ -211,12 +220,11 @@ impl Flagsmith { ) -> Result { let data = self.datastore.lock().unwrap(); let traits = traits.unwrap_or(vec![]); - if data.environment.is_some() { - let environment = data.environment.as_ref().unwrap(); + if data.evaluation_context.is_some() { + let eval_context = data.evaluation_context.as_ref().unwrap(); let engine_traits: Vec = traits.into_iter().map(|t| t.into()).collect(); return self.get_identity_flags_from_document( - environment, - &data.identities_with_overrides_by_identifier, + eval_context, identifier, engine_traits, ); @@ -234,17 +242,37 @@ impl Flagsmith { traits: Option>, ) -> Result, error::Error> { let data = self.datastore.lock().unwrap(); - if data.environment.is_none() { + if data.evaluation_context.is_none() { return Err(error::Error::new( error::ErrorKind::FlagsmithClientError, "Local evaluation required to obtain identity segments.".to_string(), )); } - let environment = data.environment.as_ref().unwrap(); - let identities_with_overrides_by_identifier = &data.identities_with_overrides_by_identifier; - let identity_model = - self.get_identity_model(&environment, &identities_with_overrides_by_identifier, identifier, traits.clone().unwrap_or(vec![]))?; - let segments = get_identity_segments(environment, &identity_model, traits.as_ref()); + let eval_context = data.evaluation_context.as_ref().unwrap(); + let traits = traits.unwrap_or(vec![]); + + // Add identity to context + let context_with_identity = add_identity_to_context(eval_context, identifier, &traits); + + // Evaluate + let result = get_evaluation_result(&context_with_identity); + + // Extract segments from result - convert SegmentResult to Segment + // Only return segments with source "api" + let segments: Vec = result + .segments + .iter() + .filter(|seg_result| { + seg_result.metadata.source == SegmentSource::Api + }) + .map(|seg_result| Segment { + id: seg_result.metadata.segment_id.unwrap_or(0) as u32, + name: seg_result.name.clone(), + rules: vec![], + feature_states: vec![], + }) + .collect(); + return Ok(segments); } @@ -268,12 +296,19 @@ impl Flagsmith { } } } - fn get_environment_flags_from_document(&self, environment: &Environment) -> models::Flags { - return models::Flags::from_feature_states( - &environment.feature_states, + fn get_environment_flags_from_document(&self, eval_context: &EngineEvaluationContext) -> models::Flags { + // Clear segments and identity for environment evaluation + let environment_eval_ctx = EngineEvaluationContext { + environment: eval_context.environment.clone(), + features: eval_context.features.clone(), + segments: HashMap::new(), + identity: None, + }; + let result = get_evaluation_result(&environment_eval_ctx); + return models::Flags::from_evaluation_result( + &result, self.analytics_processor.clone(), self.options.default_flag_handler, - None, ); } pub fn update_environment(&mut self) -> Result<(), error::Error> { @@ -282,41 +317,24 @@ impl Flagsmith { fn get_identity_flags_from_document( &self, - environment: &Environment, - identities_with_overrides_by_identifier: &HashMap, + eval_context: &EngineEvaluationContext, identifier: &str, traits: Vec, ) -> Result { - let identity = self.get_identity_model(environment, identities_with_overrides_by_identifier, identifier, traits.clone())?; - let feature_states = - engine::get_identity_feature_states(environment, &identity, Some(traits.as_ref())); - let flags = Flags::from_feature_states( - &feature_states, + // Add identity data to context + let context_with_identity = add_identity_to_context(eval_context, identifier, &traits); + + // Evaluate + let result = get_evaluation_result(&context_with_identity); + + let flags = Flags::from_evaluation_result( + &result, self.analytics_processor.clone(), self.options.default_flag_handler, - Some(&identity.composite_key()), ); return Ok(flags); } - fn get_identity_model( - &self, - environment: &Environment, - identities_with_overrides_by_identifier: &HashMap, - identifier: &str, - traits: Vec, - ) -> Result { - let mut identity: Identity; - - if identities_with_overrides_by_identifier.contains_key(identifier) { - identity = identities_with_overrides_by_identifier.get(identifier).unwrap().clone(); - } else { - identity = Identity::new(identifier.to_string(), environment.api_key.clone()); - } - - identity.identity_traits = traits; - return Ok(identity.to_owned()) - } fn get_identity_flags_from_api( &self, identifier: &str, @@ -396,9 +414,10 @@ fn update_environment( &client, environment_url.clone(), )?); - for identity in &environment.as_ref().unwrap().identity_overrides { - data.identities_with_overrides_by_identifier.insert(identity.identifier.clone(), identity.clone()); - } + // Create evaluation context from environment + let eval_context = environment_to_context(environment.as_ref().unwrap().clone()); + data.evaluation_context = Some(eval_context); + data.environment = environment; return Ok(()); } diff --git a/src/flagsmith/models.rs b/src/flagsmith/models.rs index 95c4611..98b6e41 100644 --- a/src/flagsmith/models.rs +++ b/src/flagsmith/models.rs @@ -120,7 +120,6 @@ impl Flags { result: &EvaluationResult, analytics_processor: Option, default_flag_handler: Option Flag>, - _identity_id: Option<&str>, ) -> Flags { let mut flags: HashMap = HashMap::new(); for (feature_name, flag_result) in &result.flags { @@ -249,7 +248,7 @@ mod tests { fn can_create_flag_from_feature_state() { // Given let feature_state: FeatureState = - serde_json::from_str(FEATURE_STATE_JSON_STRING.clone()).unwrap(); + serde_json::from_str(FEATURE_STATE_JSON_STRING).unwrap(); // When let flag = Flag::from_feature_state(feature_state.clone(), None); // Then diff --git a/tests/fixtures/mod.rs b/tests/fixtures/mod.rs index d393a66..765a364 100644 --- a/tests/fixtures/mod.rs +++ b/tests/fixtures/mod.rs @@ -6,6 +6,7 @@ use flagsmith::{Flagsmith, FlagsmithOptions}; pub static FEATURE_1_NAME: &str = "feature_1"; pub static FEATURE_1_ID: u32 = 1; pub static FEATURE_1_STR_VALUE: &str = "some_value"; +pub static SEGMENT_OVERRIDE_VALUE: &str = "segment_override"; pub static DEFAULT_FLAG_HANDLER_FLAG_VALUE: &str = "default_flag_handler_flag_value"; pub const ENVIRONMENT_KEY: &str = "ser.test_environment_key"; @@ -16,6 +17,73 @@ pub fn environment_json() -> serde_json::Value { serde_json::from_str(json_str).unwrap() } +#[fixture] +pub fn environment_json_with_context_value_override() -> serde_json::Value { + serde_json::json!({ + "api_key": "B62qaMZNwfiqT76p38ggrQ", + "name": "Test Environment", + "updated_at": "2023-12-06T10:21:54.079725Z", + "project": { + "name": "Test project", + "organisation": { + "feature_analytics": false, + "name": "Test Org", + "id": 1, + "persist_trait_data": true, + "stop_serving_flags": false + }, + "id": 1, + "hide_disabled_flags": false, + "segments": [{ + "id": 1, + "name": "Test Segment", + "feature_states": [{ + "multivariate_feature_state_values": [], + "feature_state_value": SEGMENT_OVERRIDE_VALUE, + "id": 2, + "featurestate_uuid": "3b0b2736-d77c-4b88-9d70-4615d6ff55f1", + "feature": { + "name": FEATURE_1_NAME, + "type": "STANDARD", + "id": FEATURE_1_ID + }, + "segment_id": 1, + "enabled": true + }], + "rules": [{ + "type": "ALL", + "conditions": [], + "rules": [{ + "type": "ALL", + "rules": [], + "conditions": [{ + "operator": "EQUAL", + "property": "$.environment.name", + "value": "Test Environment" + }] + }] + }] + }] + }, + "segment_overrides": [], + "id": 1, + "feature_states": [{ + "multivariate_feature_state_values": [], + "feature_state_value": FEATURE_1_STR_VALUE, + "id": 1, + "featurestate_uuid": "40eb539d-3713-4720-bbd4-829dbef10d51", + "feature": { + "name": FEATURE_1_NAME, + "type": "STANDARD", + "id": FEATURE_1_ID + }, + "segment_id": null, + "enabled": true + }], + "identity_overrides": [] + }) +} + #[fixture] pub fn flags_json() -> serde_json::Value { serde_json::json!( diff --git a/tests/integration_test.rs b/tests/integration_test.rs index efae60f..1beb975 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -11,11 +11,13 @@ mod fixtures; use fixtures::default_flag_handler; use fixtures::environment_json; +use fixtures::environment_json_with_context_value_override; use fixtures::flags_json; use fixtures::identities_json; use fixtures::local_eval_flagsmith; use fixtures::mock_server; use fixtures::ENVIRONMENT_KEY; +use fixtures::SEGMENT_OVERRIDE_VALUE; #[rstest] #[should_panic(expected = "default_flag_handler cannot be used with offline_handler")] @@ -89,6 +91,38 @@ fn test_get_environment_flags_uses_local_environment_when_available( api_mock.assert(); } +#[rstest] +fn test_get_environment_flags_ignores_segment_overrides( + mock_server: MockServer, + environment_json_with_context_value_override: serde_json::Value, +) { + // Given: a document with a segment override that would match the environment + let api_mock = mock_server.mock(|when, then| { + when.method(GET) + .path("/api/v1/environment-document/") + .header("X-Environment-Key", ENVIRONMENT_KEY); + then.status(200) + .json_body(environment_json_with_context_value_override); + }); + let url = mock_server.url("/api/v1/"); + let flagsmith_options = FlagsmithOptions { + api_url: url, + enable_local_evaluation: true, + ..Default::default() + }; + let flagsmith = Flagsmith::new(ENVIRONMENT_KEY.to_string(), flagsmith_options); + + // When + let flags = flagsmith.get_environment_flags().unwrap(); + let flag_value = flags + .get_feature_value_as_string(fixtures::FEATURE_1_NAME) + .unwrap(); + + // Then: should return environment default value + assert_eq!(flag_value, fixtures::FEATURE_1_STR_VALUE); + api_mock.assert(); +} + #[rstest] fn test_get_environment_flags_calls_api_when_no_local_environment( mock_server: MockServer, From d0420bbaeac8cebdb43c21602cb2736f6d9c5875 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Mon, 17 Nov 2025 14:15:02 +0530 Subject: [PATCH 3/5] fix: improve error handling --- src/error.rs | 7 +++++-- src/flagsmith/mod.rs | 13 ++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/error.rs b/src/error.rs index bbf6131..c3a2a13 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,5 @@ use std::convert::From; +use std::error; use std::fmt; /// Wraps several types of errors. @@ -25,12 +26,14 @@ impl Error{ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self.kind { - ErrorKind::FlagsmithClientError => write!(f, "Flagsmith API error: {}", &self.msg), - ErrorKind::FlagsmithAPIError => write!(f, "Flagsmith client error: {}", &self.msg), + ErrorKind::FlagsmithClientError => write!(f, "Flagsmith client error: {}", &self.msg), + ErrorKind::FlagsmithAPIError => write!(f, "Flagsmith API error: {}", &self.msg), } } } +impl error::Error for Error {} + impl From for Error { fn from(e: url::ParseError) -> Self { Error::new(ErrorKind::FlagsmithClientError, e.to_string()) diff --git a/src/flagsmith/mod.rs b/src/flagsmith/mod.rs index fad0114..a9fce09 100644 --- a/src/flagsmith/mod.rs +++ b/src/flagsmith/mod.rs @@ -110,6 +110,9 @@ impl Flagsmith { { panic!("offline_handler cannot be used with local evaluation") } + if flagsmith_options.enable_local_evaluation && !environment_key.starts_with("ser.") { + panic!("In order to use local evaluation, please use a server-side environment key (starts with 'ser.')") + } // Initialize analytics processor let analytics_processor = match flagsmith_options.enable_analytics { @@ -164,8 +167,10 @@ impl Flagsmith { if flagsmith.options.enable_local_evaluation { // Update environment once... - update_environment(&client, &ds, &environment_url).unwrap(); - + if let Err(e) = update_environment(&client, &ds, &environment_url) { + log::warn!("Failed to fetch environment on initialization: {}. Will retry in background.", e); + } + // ...and continue updating in the background let ds = Arc::clone(&ds); thread::spawn(move || loop { @@ -177,7 +182,9 @@ impl Flagsmith { Err(TryRecvError::Empty) => {} } thread::sleep(Duration::from_millis(environment_refresh_interval_mills)); - update_environment(&client, &ds, &environment_url).unwrap(); + if let Err(e) = update_environment(&client, &ds, &environment_url) { + log::warn!("Failed to update environment: {}. Will retry on next interval.", e); + } }); } return flagsmith; From bf87c0c905200f5b0412c006c5c1193bdccdd77c Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Mon, 17 Nov 2025 16:27:39 +0530 Subject: [PATCH 4/5] bump engine --- Cargo.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4089d55..2b6eddf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,8 +21,7 @@ url = "2.1" chrono = { version = "0.4" } log = "0.4" flume = "0.10.14" - -flagsmith-flag-engine = "0.4.0" +flagsmith-flag-engine = "0.5.0" [dev-dependencies] httpmock = "0.6" From 9739e5858edad163341a79e1f4e50a5c5a3ec2fc Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Mon, 17 Nov 2025 16:53:38 +0530 Subject: [PATCH 5/5] cleanup --- src/flagsmith/mod.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/flagsmith/mod.rs b/src/flagsmith/mod.rs index a9fce09..d28bd12 100644 --- a/src/flagsmith/mod.rs +++ b/src/flagsmith/mod.rs @@ -258,14 +258,10 @@ impl Flagsmith { let eval_context = data.evaluation_context.as_ref().unwrap(); let traits = traits.unwrap_or(vec![]); - // Add identity to context let context_with_identity = add_identity_to_context(eval_context, identifier, &traits); - // Evaluate let result = get_evaluation_result(&context_with_identity); - // Extract segments from result - convert SegmentResult to Segment - // Only return segments with source "api" let segments: Vec = result .segments .iter() @@ -328,10 +324,8 @@ impl Flagsmith { identifier: &str, traits: Vec, ) -> Result { - // Add identity data to context let context_with_identity = add_identity_to_context(eval_context, identifier, &traits); - // Evaluate let result = get_evaluation_result(&context_with_identity); let flags = Flags::from_evaluation_result( @@ -421,7 +415,7 @@ fn update_environment( &client, environment_url.clone(), )?); - // Create evaluation context from environment + let eval_context = environment_to_context(environment.as_ref().unwrap().clone()); data.evaluation_context = Some(eval_context);