Merge pull request #129 from Flagsmith/release/2.0.6

Release 2.0.6

Author: matthewelwell

flag_engine/api/filters.py

@@ -1,26 +1,7 @@
 import typing
 
 
-class Reverser:
-    """
-    Helper class to allow us to reverse the sort direction for individual keys when
-    sorting by multiple keys
-
-        e.g. s = sorted(l, key=lambda e: (attr1, Reverser(attr2)))
-
-    """
-
-    def __init__(self, obj: typing.Any):
-        self.obj = obj
-
-    def __eq__(self, other):
-        return self.obj == other.obj
-
-    def __lt__(self, other):
-        return other.obj < self.obj
-
-
-def sort_and_filter_feature_segments(
+def filter_feature_segments(
     feature_segments: typing.Iterable, environment_api_key: str
 ) -> typing.List:
     """
@@ -41,7 +22,4 @@ def sort_and_filter_feature_segments(
             )
         )
 
-    # TODO: determine why this sorting is necessary
-    return sorted(
-        feature_segments, key=lambda fs: (fs.feature_id, Reverser(fs.priority))
-    )
+    return feature_segments

flag_engine/api/schemas.py

@@ -6,7 +6,7 @@
     DjangoFeatureStatesRelatedManagerField,
     DjangoRelatedManagerField,
 )
-from flag_engine.api.filters import sort_and_filter_feature_segments
+from flag_engine.api.filters import filter_feature_segments
 from flag_engine.environments.schemas import (
     BaseEnvironmentAPIKeySchema,
     BaseEnvironmentSchema,
@@ -83,7 +83,7 @@ def serialize_feature_states(self, instance: typing.Any) -> typing.List[dict]:
         # TODO: move this logic to Django so we can optimise queries
         # api key is set in the context using a pre_dump method on EnvironmentSchema.
         environment_api_key = self.context.get("environment_api_key")
-        feature_segments = sort_and_filter_feature_segments(
+        feature_segments = filter_feature_segments(
             instance.feature_segments.all(), environment_api_key
         )
 
@@ -100,7 +100,6 @@ def serialize_feature_states(self, instance: typing.Any) -> typing.List[dict]:
                     feature_state.version > existing_feature_state.version
                 ):
                     feature_states[feature_state.feature_id] = feature_state
-
         return self.feature_state_schema.dump(list(feature_states.values()), many=True)
 
 

flag_engine/engine.py

@@ -101,9 +101,11 @@ def _get_identity_feature_states_dict(
     identity_segments = get_identity_segments(environment, identity, override_traits)
     for matching_segment in identity_segments:
         for feature_state in matching_segment.feature_states:
-            # note that feature states are stored on the segment in descending priority
-            # order so we only care that the last one is added
-            # TODO: can we optimise this?
+            if feature_state.feature in feature_states:
+                if feature_states[feature_state.feature].is_higher_segment_priority(
+                    feature_state
+                ):
+                    continue
             feature_states[feature_state.feature] = feature_state
 
     # Override with any feature states defined directly the identity

flag_engine/features/models.py

@@ -1,3 +1,4 @@
+import math
 import typing
 import uuid
 from dataclasses import dataclass, field
@@ -32,11 +33,17 @@ class MultivariateFeatureStateValueModel:
     mv_fs_value_uuid: str = field(default_factory=uuid.uuid4)
 
 
+@dataclass
+class FeatureSegmentModel:
+    priority: int = None
+
+
 @dataclass
 class FeatureStateModel:
     feature: FeatureModel
     enabled: bool
     django_id: int = None
+    feature_segment: FeatureSegmentModel = None
     featurestate_uuid: str = field(default_factory=uuid.uuid4)
     feature_state_value: typing.Any = field(default=None, init=False)
     multivariate_feature_state_values: typing.List[
@@ -58,6 +65,34 @@ def get_value(self, identity_id: typing.Union[int, str] = None) -> typing.Any:
             return self._get_multivariate_value(identity_id)
         return self.feature_state_value
 
+    def is_higher_segment_priority(self, other: "FeatureStateModel") -> bool:
+        """
+        Returns `True` if `self` is higher segment priority than `other`
+        (i.e. has lower value for feature_segment.priority)
+
+        NOTE:
+            A segment will be considered higher priority only if:
+            1. `other` does not have a feature segment(i.e: it is an environment feature state or it's a
+            feature state with feature segment but from an old document that does not have `feature_segment.priority`)
+            but `self` does.
+
+            2. `other` have a feature segment with high priority
+
+        """
+
+        try:
+            return (
+                getattr(
+                    self.feature_segment,
+                    "priority",
+                    math.inf,
+                )
+                < other.feature_segment.priority
+            )
+
+        except (TypeError, AttributeError):
+            return False
+
     def _get_multivariate_value(
         self, identity_id: typing.Union[int, str]
     ) -> typing.Any:

flag_engine/features/schemas.py

@@ -4,6 +4,7 @@
 
 from flag_engine.features.models import (
     FeatureModel,
+    FeatureSegmentModel,
     FeatureStateModel,
     MultivariateFeatureOptionModel,
     MultivariateFeatureStateValueModel,
@@ -12,6 +13,13 @@
 from flag_engine.utils.marshmallow.schemas import LoadToModelSchema
 
 
+class FeatureSegmentSchema(LoadToModelSchema):
+    priority = fields.Int()
+
+    class Meta:
+        model_class = FeatureSegmentModel
+
+
 class FeatureSchema(LoadToModelSchema):
     id = fields.Int()
     name = fields.Str()
@@ -43,6 +51,8 @@ class BaseFeatureStateSchema(Schema):
     featurestate_uuid = fields.Str(dump_default=uuid.uuid4)
     feature = fields.Nested(FeatureSchema)
     enabled = fields.Bool()
+    # Used for storing feature segment priority
+    feature_segment = fields.Nested(FeatureSegmentSchema, allow_none=True)
 
     class Meta:
         unknown = EXCLUDE

setup.py

@@ -2,7 +2,7 @@
 
 setup(
     name="flagsmith-flag-engine",
-    version="2.0.5",
+    version="2.0.6",
     author="Flagsmith",
     author_email="support@flagsmith.com",
     packages=find_packages(include=["flag_engine", "flag_engine.*"]),

tests/end_to_end/conftest.py

@@ -51,6 +51,7 @@ def mock_django_segment(
     )
     django_feature_segment = DjangoFeatureSegment(
         id_=1,
+        priority=0,
         environment=mock_environment,
         feature_states=[
             DjangoFeatureState(

tests/engine_tests/engine-test-data

@@ -72,11 +72,13 @@ class DjangoFeatureSegment:
     def __init__(
         self,
         id_: int,
+        priority: int,
         environment: "DjangoEnvironment",
         feature_states: typing.List["DjangoFeatureState"] = None,
     ):
         self.id = id_
         self.environment = environment
+        self.priority = priority
         self.feature_states = DjangoFeatureStateRelatedManager(feature_states or [])
 
 

tests/mock_django_classes.py

@@ -166,15 +166,18 @@ def django_segment_rule(django_segment_condition):
 
 
 @pytest.fixture()
-def django_feature_segment(
-    mocker, django_disabled_feature_state, django_environment_api_key
-):
+def django_feature_segment(mocker, django_disabled_feature_state, random_api_key):
+    environment = mocker.MagicMock(api_key=random_api_key)
     feature_state = copy.deepcopy(django_disabled_feature_state)
     feature_state.id += 1
+    feature_state.feature_segment = DjangoFeatureSegment(
+        id_=1, priority=0, environment=environment
+    )
     feature_state.enabled = True
     return DjangoFeatureSegment(
         id_=1,
-        environment=mocker.MagicMock(api_key=django_environment_api_key),
+        priority=0,
+        environment=environment,
         feature_states=[feature_state],
     )