diff --git a/examples/demo.rs b/examples/demo.rs index 98c8738..52e611e 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -14,7 +14,7 @@ use std::{collections::HashMap, env, thread, time::Duration}; -use appconfiguration_rust_sdk::{client::AppConfigurationClient, AttrValue, Entity}; +use appconfiguration_rust_sdk::{client::AppConfigurationClient, AttrValue, Entity, Feature}; use dotenvy::dotenv; use std::error::Error; @@ -69,13 +69,13 @@ fn main() -> Result<()> { match client.get_feature_proxy(&feature_id) { Ok(feature) => { - println!("Feature name: {}", feature.get_name()); + println!("Feature name: {}", feature.get_name()?); println!("Feature id: {}", feature.get_id()); - println!("Feature data type: {}", feature.get_data_type()); - println!("Is feature enabled: {}", feature.is_enabled()); + println!("Feature data type: {}", feature.get_data_type()?); + println!("Is feature enabled: {}", feature.is_enabled()?); println!( - "Feature evaluated value is: {}", - feature.get_current_value(&entity) + "Feature evaluated value is: {:?}", + feature.get_value(&entity)? ); } Err(error) => { diff --git a/src/client/app_configuration_client.rs b/src/client/app_configuration_client.rs index 82877b2..5800658 100644 --- a/src/client/app_configuration_client.rs +++ b/src/client/app_configuration_client.rs @@ -13,7 +13,7 @@ // limitations under the License. use crate::client::cache::ConfigurationSnapshot; -use crate::client::feature::Feature; +use crate::client::feature_snapshot::FeatureSnapshot; pub use crate::client::feature_proxy::FeatureProxy; use crate::client::http; use crate::client::property::Property; @@ -184,7 +184,7 @@ impl AppConfigurationClient { .collect()) } - pub fn get_feature(&self, feature_id: &str) -> Result { + pub fn get_feature(&self, feature_id: &str) -> Result { let config_snapshot = self.latest_config_snapshot.lock()?; // Get the feature from the snapshot @@ -221,21 +221,18 @@ impl AppConfigurationClient { segments }; - Ok(Feature::new(feature.clone(), segments)) + Ok(FeatureSnapshot::new(feature.clone(), segments)) } /// Searches for the feature `feature_id` inside the current configured /// collection, and environment. /// /// Return `Ok(feature)` if the feature exists or `Err` if it does not. - pub fn get_feature_proxy(&self, feature_id: &str) -> Result { + pub fn get_feature_proxy<'a>(&'a self, feature_id: &str) -> Result> { // FIXME: there is and was no validation happening if the feature exists. // Comments and error messages in FeatureProxy suggest that this should happen here. // same applies for properties. - Ok(FeatureProxy::new( - self.latest_config_snapshot.clone(), - feature_id.to_string(), - )) + Ok(FeatureProxy::new(self, feature_id.to_string())) } pub fn get_property_ids(&self) -> Result> { diff --git a/src/client/feature_proxy.rs b/src/client/feature_proxy.rs index e7a1a7e..4a1786c 100644 --- a/src/client/feature_proxy.rs +++ b/src/client/feature_proxy.rs @@ -12,204 +12,56 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::Arc; -use std::{io::Cursor, sync::Mutex}; +use std::io::Cursor; use murmur3::murmur3_32; -use crate::{ - client::cache::ConfigurationSnapshot, models, - segment_evaluation::find_applicable_segment_rule_for_entity, -}; - use crate::entity::Entity; +use crate::Feature; -use crate::errors::ConfigurationAccessError; - -const MISSING_FEATURE_ERROR_MSG: &str = "The feature should exist in the configuration_snapshot. It should have been validated in `AppConfigurationClient::get_feature()`."; +use super::feature_snapshot::FeatureSnapshot; +use super::AppConfigurationClient; -/// A feature in a collection and environment. Use the `get_feature()` -/// method of the `AppConfigurationClient` to create instances of features. -#[derive(Debug)] -pub struct FeatureProxy { - configuration_snapshot: Arc>, +pub struct FeatureProxy<'a> { + client: &'a AppConfigurationClient, feature_id: String, } -impl FeatureProxy { - pub(crate) fn new( - configuration_snapshot: Arc>, - feature_id: String, - ) -> Self { - FeatureProxy { - configuration_snapshot, - feature_id, - } +impl<'a> FeatureProxy<'a> { + pub(crate) fn new(client: &'a AppConfigurationClient, feature_id: String) -> Self { + Self { client, feature_id } } - /// Returns the name of the feature. - pub fn get_name(&self) -> String { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .name - .clone() - } - - /// Returns the disable value as a `models::ConfigValue`. - pub fn get_disabled_value(&self) -> models::ConfigValue { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .disabled_value - .clone() - } - - /// Returns the enabled value as a `models::ConfigValue`. - pub fn get_enabled_value(&self) -> models::ConfigValue { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .enabled_value - .clone() - } - - /// Returns the id of the feature. - pub fn get_id(&self) -> String { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .feature_id - .clone() - } - - /// Returns the data type as a member of the `models::ValueKind` enumeration. - pub fn get_data_type(&self) -> models::ValueKind { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .kind - } - - /// Gets the `Some(data_format)` if the feature data type is - /// `models::ValueKind::STRING`, or `None` otherwise. - pub fn get_data_format(&self) -> Option { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .format - .clone() - } - - /// Returns the rollout peArcentage as a positive integer. - pub fn get_rollout_percentage(&self) -> u32 { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .rollout_percentage + pub fn snapshot(&self) -> crate::errors::Result { + self.client.get_feature(&self.feature_id) } +} - /// Returns the targeting rules for the feature. I.e.: what value to - /// associate with an entity, under what ciArcumnstances, and how frequent - /// it applies. - pub fn get_targeting_rules(&self) -> Vec { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .segment_rules - .clone() +impl<'a> Feature for FeatureProxy<'a> { + fn get_id(&self) -> &str { + &self.feature_id } - /// Returns if the feature is enabled or not. - pub fn is_enabled(&self) -> bool { - self.configuration_snapshot - .lock() - .unwrap_or_else(|_| panic!("{}", ConfigurationAccessError::LockAcquisitionError)) - .get_feature(&self.feature_id) - .expect(MISSING_FEATURE_ERROR_MSG) - .enabled + fn get_name(&self) -> crate::errors::Result { + self.client.get_feature(&self.feature_id)?.get_name() } - /// Evaluates the feature for `entity` and returns the evaluation as a - /// `models::ConfigValue`. - pub fn get_current_value(&self, entity: &impl Entity) -> models::ConfigValue { - if !self.is_enabled() { - self.get_disabled_value() - } else { - self.evaluate_feature_for_entity(entity) - } + fn get_data_type(&self) -> crate::errors::Result { + self.client.get_feature(&self.feature_id)?.get_data_type() } - fn evaluate_feature_for_entity(&self, entity: &impl Entity) -> models::ConfigValue { - let tag = format!("{}:{}", entity.get_id(), self.get_id()); - - if self.get_targeting_rules().len() == 0 && entity.get_attributes().len() == 0 { - // TODO rollout percentage evaluation - } - - let segment_rule = find_applicable_segment_rule_for_entity( - &self - .configuration_snapshot - .lock() - .unwrap_or_else(|e| panic!("Failed to acquire configuration snapshot lock: {e}")) - .segments, - self.get_targeting_rules().into_iter(), - entity, - ) - .unwrap_or_else(|e| panic!("Failed to evaluate segment rules: {e}")); - if let Some(segment_rule) = segment_rule { - let rollout_percentage = self.resolve_rollout_percentage(&segment_rule); - if rollout_percentage == 100 || random_value(&tag) < rollout_percentage { - self.resolve_enabled_value(&segment_rule) - } else { - self.get_disabled_value() - } - } else { - let rollout_percentage = self.get_rollout_percentage(); - if rollout_percentage == 100 || random_value(&tag) < rollout_percentage { - self.get_enabled_value() - } else { - self.get_disabled_value() - } - } + fn is_enabled(&self) -> crate::errors::Result { + self.client.get_feature(&self.feature_id)?.is_enabled() } - fn resolve_rollout_percentage(&self, segment_rule: &models::TargetingRule) -> u32 { - let missing_rollout_msg = "Rollout velue is missing."; - let rollout_value = segment_rule - .rollout_percentage - .as_ref() - .expect(missing_rollout_msg); - if rollout_value.is_default() { - self.get_rollout_percentage() - } else { - u32::try_from(rollout_value.as_u64().expect(missing_rollout_msg)) - .expect("Invalid rollout value.") - } + fn get_enabled_value(&self) -> crate::errors::Result { + self.client + .get_feature(&self.feature_id)? + .get_enabled_value() } - fn resolve_enabled_value(&self, segment_rule: &models::TargetingRule) -> models::ConfigValue { - if segment_rule.value.is_default() { - self.get_enabled_value() - } else { - segment_rule.value.clone() - } + fn get_value(&self, entity: &impl Entity) -> crate::errors::Result { + self.client.get_feature(&self.feature_id)?.get_value(entity) } } diff --git a/src/client/feature.rs b/src/client/feature_snapshot.rs similarity index 93% rename from src/client/feature.rs rename to src/client/feature_snapshot.rs index e2b96fd..c608924 100644 --- a/src/client/feature.rs +++ b/src/client/feature_snapshot.rs @@ -14,6 +14,7 @@ use crate::client::value::{NumericValue, Value}; use crate::entity::Entity; +use crate::Feature; use std::collections::HashMap; use super::feature_proxy::random_value; @@ -22,12 +23,12 @@ use crate::segment_evaluation::find_applicable_segment_rule_for_entity; use crate::errors::{Error, Result}; #[derive(Debug)] -pub struct Feature { +pub struct FeatureSnapshot { feature: crate::models::Feature, segments: HashMap, } -impl Feature { +impl FeatureSnapshot { pub(crate) fn new( feature: crate::models::Feature, segments: HashMap, @@ -35,30 +36,6 @@ impl Feature { Self { feature, segments } } - pub fn get_value(&self, entity: &impl Entity) -> Result { - let model_value = self.evaluate_feature_for_entity(entity)?; - - let value = match self.feature.kind { - crate::models::ValueKind::Numeric => { - Value::Numeric(NumericValue(model_value.0.clone())) - } - crate::models::ValueKind::Boolean => Value::Boolean( - model_value - .0 - .as_bool() - .ok_or(Error::ProtocolError("Expected Boolean".into()))?, - ), - crate::models::ValueKind::String => Value::String( - model_value - .0 - .as_str() - .ok_or(Error::ProtocolError("Expected String".into()))? - .to_string(), - ), - }; - Ok(value) - } - fn evaluate_feature_for_entity( &self, entity: &impl Entity, @@ -124,6 +101,52 @@ impl Feature { } } +impl Feature for FeatureSnapshot { + fn get_id(&self) -> &str { + &self.feature.feature_id + } + + fn get_name(&self) -> Result { + Ok(self.feature.name.clone()) + } + + fn get_data_type(&self) -> Result { + Ok(self.feature.kind) + } + + fn is_enabled(&self) -> Result { + Ok(self.feature.enabled) + } + + fn get_enabled_value(&self) -> Result { + Ok(self.feature.enabled_value.clone()) + } + + fn get_value(&self, entity: &impl Entity) -> Result { + let model_value = self.evaluate_feature_for_entity(entity)?; + + let value = match self.feature.kind { + crate::models::ValueKind::Numeric => { + Value::Numeric(NumericValue(model_value.0.clone())) + } + crate::models::ValueKind::Boolean => Value::Boolean( + model_value + .0 + .as_bool() + .ok_or(Error::ProtocolError("Expected Boolean".into()))?, + ), + crate::models::ValueKind::String => Value::String( + model_value + .0 + .as_str() + .ok_or(Error::ProtocolError("Expected String".into()))? + .to_string(), + ), + }; + Ok(value) + } +} + #[cfg(test)] pub mod tests { @@ -143,16 +166,16 @@ pub mod tests { id: entity_id.into(), attributes: HashMap::new(), }; - let result = Feature::should_rollout(100, &entity, "f1"); + let result = FeatureSnapshot::should_rollout(100, &entity, "f1"); assert!(result); - let result = Feature::should_rollout(0, &entity, "f1"); + let result = FeatureSnapshot::should_rollout(0, &entity, "f1"); assert!(!result); - let result = Feature::should_rollout(50, &entity, "f1"); + let result = FeatureSnapshot::should_rollout(50, &entity, "f1"); assert_eq!(result, partial_rollout_expectation); - let result = Feature::should_rollout(50, &entity, "f4"); + let result = FeatureSnapshot::should_rollout(50, &entity, "f4"); // We chose feature ID here so that we rollout exactly inverted to "f1" assert_eq!(result, !partial_rollout_expectation); } @@ -181,7 +204,7 @@ pub mod tests { enabled: true, rollout_percentage: 50, }; - let feature = Feature::new(inner_feature, HashMap::new()); + let feature = FeatureSnapshot::new(inner_feature, HashMap::new()); // One entity and feature combination which leads to no rollout: let entity = crate::tests::GenericEntity { @@ -222,7 +245,7 @@ pub mod tests { enabled: false, rollout_percentage: 100, }; - let feature = Feature::new(inner_feature, HashMap::new()); + let feature = FeatureSnapshot::new(inner_feature, HashMap::new()); let entity = crate::tests::TrivialEntity {}; let value = feature.get_value(&entity).unwrap(); @@ -253,7 +276,7 @@ pub mod tests { enabled: true, rollout_percentage: 50, }; - let feature = Feature::new( + let feature = FeatureSnapshot::new( inner_feature, HashMap::from([( "some_segment_id".into(), @@ -321,7 +344,7 @@ pub mod tests { enabled: true, rollout_percentage: 50, }; - let feature = Feature::new( + let feature = FeatureSnapshot::new( inner_feature, HashMap::from([( "some_segment_id".into(), @@ -371,7 +394,7 @@ pub mod tests { enabled: true, rollout_percentage: 0, }; - let feature = Feature::new( + let feature = FeatureSnapshot::new( inner_feature, HashMap::from([( "some_segment_id".into(), @@ -429,7 +452,7 @@ pub mod tests { enabled: true, rollout_percentage: 100, }; - let feature = Feature::new( + let feature = FeatureSnapshot::new( inner_feature, HashMap::from([ ( diff --git a/src/client/mod.rs b/src/client/mod.rs index 8a70538..45abf89 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -15,7 +15,7 @@ mod app_configuration_client; pub(crate) mod cache; -pub mod feature; +pub mod feature_snapshot; pub(crate) mod feature_proxy; pub(crate) mod http; pub mod property; diff --git a/src/feature.rs b/src/feature.rs new file mode 100644 index 0000000..ebc77dd --- /dev/null +++ b/src/feature.rs @@ -0,0 +1,31 @@ +// (C) Copyright IBM Corp. 2024. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::client::value::Value; +use crate::errors::Result; +use crate::Entity; + +pub trait Feature { + fn get_id(&self) -> &str; + + fn get_name(&self) -> Result; + + fn get_data_type(&self) -> Result; + + fn is_enabled(&self) -> Result; + + fn get_enabled_value(&self) -> Result; + + fn get_value(&self, entity: &impl Entity) -> Result; +} diff --git a/src/lib.rs b/src/lib.rs index aa8f211..b695c0e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,9 @@ pub mod entity; pub mod errors; pub mod models; mod segment_evaluation; +mod feature; +pub use feature::Feature; pub use entity::{AttrValue, Entity}; #[cfg(test)] diff --git a/src/tests/test_get_feature.rs b/src/tests/test_get_feature.rs index 01f3aea..0eb36a4 100644 --- a/src/tests/test_get_feature.rs +++ b/src/tests/test_get_feature.rs @@ -20,6 +20,7 @@ use rstest::*; use super::client_enterprise; use crate::models::tests::configuration_feature1_enabled; +use crate::feature::Feature; #[rstest] fn test_get_feature_persistence( diff --git a/src/tests/test_using_example_data.rs b/src/tests/test_using_example_data.rs index b2051ad..2c52752 100644 --- a/src/tests/test_using_example_data.rs +++ b/src/tests/test_using_example_data.rs @@ -17,20 +17,21 @@ use crate::client::AppConfigurationClient; use rstest::*; use super::client_enterprise; +use crate::feature::Feature; #[rstest] fn test_get_a_specific_feature(client_enterprise: AppConfigurationClient) { use crate::models::ValueKind; let specific_feature = client_enterprise.get_feature_proxy("f1").unwrap(); - let name = specific_feature.get_name(); - let data_type = specific_feature.get_data_type(); - let is_enabled = specific_feature.is_enabled(); + let name = specific_feature.get_name().unwrap(); + let data_type = specific_feature.get_data_type().unwrap(); + let is_enabled = specific_feature.is_enabled().unwrap(); assert_eq!(name, "F1".to_string()); assert_eq!(data_type, ValueKind::Numeric); assert_eq!(is_enabled, true); - assert_eq!(specific_feature.get_enabled_value().as_i64().unwrap(), 5); + assert_eq!(specific_feature.get_enabled_value().unwrap().as_i64().unwrap(), 5); } #[rstest] diff --git a/tests/test_app_config.rs b/tests/test_app_config.rs index 2ded5ca..712e13b 100644 --- a/tests/test_app_config.rs +++ b/tests/test_app_config.rs @@ -18,6 +18,7 @@ use rstest::*; use appconfiguration_rust_sdk::client::AppConfigurationClient; use appconfiguration_rust_sdk::models::ValueKind; use std::env; +use appconfiguration_rust_sdk::Feature; #[fixture] fn setup_client() -> AppConfigurationClient { @@ -46,9 +47,9 @@ fn test_get_a_specific_feature(setup_client: AppConfigurationClient) { .get_feature_proxy("test-feature-flag-1") .unwrap(); - let name = specific_feature.get_name(); - let data_type = specific_feature.get_data_type(); - let is_enabled = specific_feature.is_enabled(); + let name = specific_feature.get_name().unwrap(); + let data_type = specific_feature.get_data_type().unwrap(); + let is_enabled = specific_feature.is_enabled().unwrap(); assert_eq!(name, "test feature flag 1".to_string()); assert_eq!(data_type, ValueKind::Boolean);