Skip to content

Commit 812cc68

Browse files
authored
refact: Isolate FeatureSnapshot and PropertySnapshot from crate::models types (#56)
A refactor going in the direction of isolating our logic from the JSON data returned by the server: * Remove all `crate::models::...` members from `FeatureSnapshot` and `PropertySnapshot` * Encapsulate `TargetingRule`s and `Segment`s into `SegmentRules` object. This new type handles the logic related to `find_applicable_segment_rule_for_entity` and also the one to get the `Value` from the found `TargetingRule` (now hidden into a `SegmentRule`)
2 parents a6615ce + c808e91 commit 812cc68

File tree

3 files changed

+174
-91
lines changed

3 files changed

+174
-91
lines changed

src/client/feature_snapshot.rs

+45-45
Original file line numberDiff line numberDiff line change
@@ -18,66 +18,67 @@ use crate::Feature;
1818
use std::collections::HashMap;
1919

2020
use super::feature_proxy::random_value;
21-
use crate::segment_evaluation::find_applicable_segment_rule_for_entity;
21+
use crate::segment_evaluation::SegmentRules;
2222

2323
use crate::errors::Result;
2424

2525
/// Provides a snapshot of a [`Feature`].
2626
#[derive(Debug)]
2727
pub struct FeatureSnapshot {
28-
feature: crate::models::Feature,
29-
segments: HashMap<String, crate::models::Segment>,
28+
enabled: bool,
29+
enabled_value: Value,
30+
disabled_value: Value,
31+
rollout_percentage: u32,
32+
name: String,
33+
feature_id: String,
34+
segment_rules: SegmentRules,
3035
}
3136

3237
impl FeatureSnapshot {
3338
pub(crate) fn new(
3439
feature: crate::models::Feature,
3540
segments: HashMap<String, crate::models::Segment>,
3641
) -> Self {
37-
Self { feature, segments }
42+
let segment_rules = SegmentRules::new(segments, feature.segment_rules, feature.kind);
43+
Self {
44+
enabled: feature.enabled, // TODO: Based on this enabled value we can create two different implementations, as the disabled one is much more simpler.
45+
enabled_value: (feature.kind, feature.enabled_value)
46+
.try_into()
47+
.expect("TODO: Handle this error"),
48+
disabled_value: (feature.kind, feature.disabled_value)
49+
.try_into()
50+
.expect("TODO: Handle this error"),
51+
rollout_percentage: feature.rollout_percentage,
52+
segment_rules,
53+
name: feature.name,
54+
feature_id: feature.feature_id,
55+
}
3856
}
3957

40-
fn evaluate_feature_for_entity(
41-
&self,
42-
entity: &impl Entity,
43-
) -> Result<crate::models::ConfigValue> {
44-
if !self.feature.enabled {
45-
return Ok(self.feature.disabled_value.clone());
58+
fn evaluate_feature_for_entity(&self, entity: &impl Entity) -> Result<Value> {
59+
if !self.enabled {
60+
return Ok(self.disabled_value.clone());
4661
}
4762

48-
if self.feature.segment_rules.is_empty() || entity.get_attributes().is_empty() {
63+
if self.segment_rules.is_empty() || entity.get_attributes().is_empty() {
4964
// No match possible. Do not consider segment rules:
5065
return self.use_rollout_percentage_to_get_value_from_feature_directly(entity);
5166
}
5267

53-
match find_applicable_segment_rule_for_entity(
54-
&self.segments,
55-
&self.feature.segment_rules,
56-
entity,
57-
)? {
68+
match self
69+
.segment_rules
70+
.find_applicable_segment_rule_for_entity(entity)?
71+
{
5872
Some(segment_rule) => {
5973
// Get rollout percentage
60-
let rollout_percentage = match &segment_rule.rollout_percentage {
61-
Some(value) => {
62-
if value.is_default() {
63-
self.feature.rollout_percentage
64-
} else {
65-
u32::try_from(value.as_u64().expect("Rollout value is not u64."))
66-
.expect("Invalid rollout value. Could not convert to u32.")
67-
}
68-
}
69-
None => panic!("Rollout value is missing."),
70-
};
74+
let rollout_percentage =
75+
segment_rule.rollout_percentage(self.rollout_percentage)?;
7176

7277
// Should rollout?
73-
if Self::should_rollout(rollout_percentage, entity, &self.feature.feature_id) {
74-
if segment_rule.value.is_default() {
75-
Ok(self.feature.enabled_value.clone())
76-
} else {
77-
Ok(segment_rule.value.clone())
78-
}
78+
if Self::should_rollout(rollout_percentage, entity, &self.feature_id) {
79+
segment_rule.value(&self.enabled_value)
7980
} else {
80-
Ok(self.feature.disabled_value.clone())
81+
Ok(self.disabled_value.clone())
8182
}
8283
}
8384
None => self.use_rollout_percentage_to_get_value_from_feature_directly(entity),
@@ -92,28 +93,27 @@ impl FeatureSnapshot {
9293
fn use_rollout_percentage_to_get_value_from_feature_directly(
9394
&self,
9495
entity: &impl Entity,
95-
) -> Result<crate::models::ConfigValue> {
96-
let rollout_percentage = self.feature.rollout_percentage;
97-
if Self::should_rollout(rollout_percentage, entity, &self.feature.feature_id) {
98-
Ok(self.feature.enabled_value.clone())
96+
) -> Result<Value> {
97+
let rollout_percentage = self.rollout_percentage;
98+
if Self::should_rollout(rollout_percentage, entity, &self.feature_id) {
99+
Ok(self.enabled_value.clone())
99100
} else {
100-
Ok(self.feature.disabled_value.clone())
101+
Ok(self.disabled_value.clone())
101102
}
102103
}
103104
}
104105

105106
impl Feature for FeatureSnapshot {
106107
fn get_name(&self) -> Result<String> {
107-
Ok(self.feature.name.clone())
108+
Ok(self.name.clone())
108109
}
109110

110111
fn is_enabled(&self) -> Result<bool> {
111-
Ok(self.feature.enabled)
112+
Ok(self.enabled)
112113
}
113114

114115
fn get_value(&self, entity: &impl Entity) -> Result<Value> {
115-
let model_value = self.evaluate_feature_for_entity(entity)?;
116-
(self.feature.kind, model_value).try_into()
116+
self.evaluate_feature_for_entity(entity)
117117
}
118118

119119
fn get_value_into<T: TryFrom<Value, Error = crate::Error>>(
@@ -186,7 +186,7 @@ pub mod tests {
186186
attributes: entity_attributes.clone(),
187187
};
188188
assert_eq!(
189-
random_value(format!("{}:{}", entity.id, feature.feature.feature_id).as_str()),
189+
random_value(format!("{}:{}", entity.id, feature.feature_id).as_str()),
190190
68
191191
);
192192
let value = feature.get_value(&entity).unwrap();
@@ -198,7 +198,7 @@ pub mod tests {
198198
attributes: entity_attributes,
199199
};
200200
assert_eq!(
201-
random_value(format!("{}:{}", entity.id, feature.feature.feature_id).as_str()),
201+
random_value(format!("{}:{}", entity.id, feature.feature_id).as_str()),
202202
29
203203
);
204204
let value = feature.get_value(&entity).unwrap();

src/client/property_snapshot.rs

+23-26
Original file line numberDiff line numberDiff line change
@@ -18,59 +18,56 @@ use crate::Property;
1818
use std::collections::HashMap;
1919

2020
use crate::errors::Result;
21-
use crate::segment_evaluation::find_applicable_segment_rule_for_entity;
21+
use crate::segment_evaluation::SegmentRules;
2222

2323
/// Provides a snapshot of a [`Property`].
2424
#[derive(Debug)]
2525
pub struct PropertySnapshot {
26-
property: crate::models::Property,
27-
segments: HashMap<String, crate::models::Segment>,
26+
value: Value,
27+
segment_rules: SegmentRules,
28+
name: String,
2829
}
2930

3031
impl PropertySnapshot {
3132
pub(crate) fn new(
3233
property: crate::models::Property,
3334
segments: HashMap<String, crate::models::Segment>,
3435
) -> Self {
35-
Self { property, segments }
36+
let segment_rules = SegmentRules::new(segments, property.segment_rules, property.kind);
37+
Self {
38+
value: (property.kind, property.value)
39+
.try_into()
40+
.expect("TODO: Handle error here"),
41+
segment_rules,
42+
name: property.name,
43+
}
3644
}
3745

38-
fn evaluate_feature_for_entity(
39-
&self,
40-
entity: &impl Entity,
41-
) -> Result<crate::models::ConfigValue> {
42-
if self.property.segment_rules.is_empty() || entity.get_attributes().is_empty() {
46+
fn evaluate_feature_for_entity(&self, entity: &impl Entity) -> Result<Value> {
47+
if self.segment_rules.is_empty() || entity.get_attributes().is_empty() {
4348
// TODO: this makes only sense if there can be a rule which matches
4449
// even on empty attributes
4550
// No match possible. Do not consider segment rules:
46-
return Ok(self.property.value.clone());
51+
return Ok(self.value.clone());
4752
}
4853

49-
match find_applicable_segment_rule_for_entity(
50-
&self.segments,
51-
&self.property.segment_rules,
52-
entity,
53-
)? {
54-
Some(segment_rule) => {
55-
if segment_rule.value.is_default() {
56-
Ok(self.property.value.clone())
57-
} else {
58-
Ok(segment_rule.value.clone())
59-
}
60-
}
61-
None => Ok(self.property.value.clone()),
54+
match self
55+
.segment_rules
56+
.find_applicable_segment_rule_for_entity(entity)?
57+
{
58+
Some(segment_rule) => segment_rule.value(&self.value),
59+
None => Ok(self.value.clone()),
6260
}
6361
}
6462
}
6563

6664
impl Property for PropertySnapshot {
6765
fn get_name(&self) -> Result<String> {
68-
Ok(self.property.name.clone())
66+
Ok(self.name.clone())
6967
}
7068

7169
fn get_value(&self, entity: &impl Entity) -> Result<Value> {
72-
let model_value = self.evaluate_feature_for_entity(entity)?;
73-
(self.property.kind, model_value).try_into()
70+
self.evaluate_feature_for_entity(entity)
7471
}
7572

7673
fn get_value_into<T: TryFrom<Value, Error = crate::Error>>(

0 commit comments

Comments
 (0)