Merge pull request #75 from Flagsmith/release/1.5.1

Matthew Elwell · web-flow · commit a33097a772f3 · 2022-01-07T10:16:42.000Z
release 1.5.1
diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml
@@ -22,6 +22,7 @@ jobs:
               uses: actions/checkout@v2
               with:
                   fetch-depth: 0
+                  submodules: recursive
 
             - name: Set up Python ${{ matrix.python-version }}
               uses: actions/setup-python@v2
@@ -43,5 +44,4 @@ jobs:
               run: flake8 .
 
             - name: Run Tests
-              run: |
-                  pytest -p no:warnings
+              run: pytest -p no:warnings
diff --git a/.gitmodules b/.gitmodules
@@ -0,0 +1,3 @@
+[submodule "tests/engine_tests/engine-test-data"]
+	path = tests/engine_tests/engine-test-data
+	url = git@github.com:Flagsmith/engine-test-data.git
diff --git a/flag_engine/features/models.py b/flag_engine/features/models.py
@@ -21,13 +21,15 @@ def __hash__(self):
 @dataclass
 class MultivariateFeatureOptionModel:
     value: typing.Any
+    id: int = None
 
 
 @dataclass
 class MultivariateFeatureStateValueModel:
-    id: int
     multivariate_feature_option: MultivariateFeatureOptionModel
     percentage_allocation: float
+    id: int = None
+    mv_fs_value_uuid: str = field(default_factory=uuid.uuid4)
 
 
 @dataclass
@@ -65,7 +67,8 @@ def _get_multivariate_value(self, identity_id: int) -> typing.Any:
         # percentage value.
         start_percentage = 0
         for mv_value in sorted(
-            self.multivariate_feature_state_values, key=lambda v: v.id
+            self.multivariate_feature_state_values,
+            key=lambda v: v.id or v.mv_fs_value_uuid,
         ):
             limit = mv_value.percentage_allocation + start_percentage
             if start_percentage <= percentage_value < limit:
diff --git a/flag_engine/features/schemas.py b/flag_engine/features/schemas.py
@@ -1,13 +1,14 @@
 import uuid
 
-from marshmallow import EXCLUDE, Schema, fields, post_load, validate
+from marshmallow import EXCLUDE, Schema, fields, post_dump, post_load, validate
 
 from flag_engine.features.models import (
     FeatureModel,
     FeatureStateModel,
     MultivariateFeatureOptionModel,
     MultivariateFeatureStateValueModel,
 )
+from flag_engine.utils.exceptions import InvalidPercentageAllocation
 from flag_engine.utils.marshmallow.schemas import LoadToModelSchema
 
 
@@ -21,14 +22,16 @@ class Meta:
 
 
 class MultivariateFeatureOptionSchema(LoadToModelSchema):
+    id = fields.Int(allow_none=True)
     value = fields.Field(allow_none=True)
 
     class Meta:
         model_class = MultivariateFeatureOptionModel
 
 
 class MultivariateFeatureStateValueSchema(LoadToModelSchema):
-    id = fields.Int()
+    id = fields.Int(allow_none=True)
+    mv_fs_value_uuid = fields.Str(dump_default=uuid.uuid4)
     multivariate_feature_option = fields.Nested(MultivariateFeatureOptionSchema)
     percentage_allocation = fields.Decimal(validate=[validate.Range(0, 100)])
 
@@ -50,6 +53,7 @@ class FeatureStateSchema(BaseFeatureStateSchema):
     multivariate_feature_state_values = fields.List(
         fields.Nested(MultivariateFeatureStateValueSchema)
     )
+    django_id = fields.Int(allow_none=True)
 
     class Meta:
         unknown = EXCLUDE
@@ -60,3 +64,18 @@ def make_feature_state(self, data, **kwargs) -> FeatureStateModel:
         feature_state = FeatureStateModel(**data)
         feature_state.set_value(value)
         return feature_state
+
+    @post_dump()
+    def validate_percentage_allocations(self, data, **kwargs):
+        """Since we do support modifying percentage allocation on a per identity override bases
+        we need to validate the percentage before building the document(dict)"""
+        total_allocation = sum(
+            mvfsv["percentage_allocation"]
+            for mvfsv in data["multivariate_feature_state_values"]
+        )
+        if total_allocation > 100:
+
+            raise InvalidPercentageAllocation(
+                "Total percentage allocation should not be more than 100"
+            )
+        return data
diff --git a/flag_engine/identities/schemas.py b/flag_engine/identities/schemas.py
@@ -5,6 +5,7 @@
 
 from flag_engine.features.schemas import FeatureStateSchema
 from flag_engine.identities.models import IdentityModel
+from flag_engine.utils.marshmallow.fields import IdentityFeaturesListField
 from flag_engine.utils.marshmallow.schemas import LoadToModelMixin
 
 from .traits.schemas import TraitSchema
@@ -29,7 +30,10 @@ def generate_composite_key(self, data: typing.Dict[str, typing.Any], **kwargs):
 
 class IdentitySchema(LoadToModelMixin, BaseIdentitySchema):
     identity_traits = fields.List(fields.Nested(TraitSchema), required=False)
-    identity_features = fields.List(fields.Nested(FeatureStateSchema), required=False)
+    identity_features = IdentityFeaturesListField(
+        fields.Nested(FeatureStateSchema), required=False
+    )
+
     django_id = fields.Int(required=False, allow_none=True)
     composite_key = fields.Str(dump_only=True)
 
diff --git a/flag_engine/utils/exceptions.py b/flag_engine/utils/exceptions.py
@@ -4,3 +4,7 @@ class FeatureStateNotFound(Exception):
 
 class DuplicateFeatureState(Exception):
     pass
+
+
+class InvalidPercentageAllocation(Exception):
+    pass
diff --git a/flag_engine/utils/marshmallow/fields.py b/flag_engine/utils/marshmallow/fields.py
@@ -0,0 +1,8 @@
+from marshmallow import fields
+
+from flag_engine.utils.collections import IdentityFeaturesList
+
+
+class IdentityFeaturesListField(fields.List):
+    def _deserialize(self, value, attr, data, **kwargs) -> IdentityFeaturesList:
+        return IdentityFeaturesList(super()._deserialize(value, attr, data, **kwargs))
diff --git a/setup.py b/setup.py
@@ -2,7 +2,7 @@
 
 setup(
     name="flagsmith-flag-engine",
-    version="1.5.0",
+    version="1.5.1",
     author="Flagsmith",
     author_email="support@flagsmith.com",
     packages=find_packages(include=["flag_engine", "flag_engine.*"]),
diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data
@@ -0,0 +1 @@
+Subproject commit f10e3e5279ae8545e00bb66cb1e1835b52853bd5
diff --git a/tests/engine_tests/test_engine.py b/tests/engine_tests/test_engine.py
@@ -0,0 +1,75 @@
+import json
+import typing
+from pathlib import Path
+
+import pytest
+
+from flag_engine.engine import get_identity_feature_states
+from flag_engine.environments.builders import build_environment_model
+from flag_engine.environments.models import EnvironmentModel
+from flag_engine.identities.builders import build_identity_model
+from flag_engine.identities.models import IdentityModel
+
+MODULE_PATH = Path(__file__).parent.resolve()
+
+
+def _extract_test_cases(
+    file_path: Path,
+) -> typing.Iterable[typing.Tuple[EnvironmentModel, IdentityModel, dict]]:
+    """
+    Extract the test cases from the json data file which should be in the following
+    format.
+
+        {
+          "environment": {...},  // the environment document as found in DynamoDB
+          "identities_and_responses": [
+            {
+              "identity": {...},  // the identity as found in DynamoDB,
+              "response": {...},  // the response that was obtained from the current API
+            }
+          ]
+        }
+
+    :param file_path: the path to the json data file
+    :return: a list of tuples containing the environment, identity and api response
+    """
+    with open(file_path, "r") as f:
+        test_data = json.loads(f.read())
+        environment_model = build_environment_model(test_data["environment"])
+        return [
+            (
+                environment_model,
+                build_identity_model(test_case["identity"]),
+                test_case["response"],
+            )
+            for test_case in test_data["identities_and_responses"]
+        ]
+
+
+@pytest.mark.parametrize(
+    "environment_model, identity_model, api_response",
+    _extract_test_cases(
+        MODULE_PATH / "engine-test-data/data/environment_n9fbf9h3v4fFgH3U3ngWhb.json"
+    ),
+)
+def test_engine(environment_model, identity_model, api_response):
+    # When
+    # we get the feature states from the engine
+    engine_response = get_identity_feature_states(environment_model, identity_model)
+
+    # and we sort the flags and feature states so we can iterate over them and compare
+    sorted_engine_flags = sorted(engine_response, key=lambda fs: fs.feature.name)
+    sorted_api_flags = sorted(api_response["flags"], key=lambda f: f["feature"]["name"])
+
+    # Then
+    # there are an equal number of flags and feature states
+    assert len(sorted_engine_flags) == len(sorted_api_flags)
+
+    # and the values and enabled status of each of the feature states returned by the
+    # engine are identical to those returned by the Django API (i.e. the test data).
+    for i, feature_state in enumerate(sorted_engine_flags):
+        assert (
+            feature_state.get_value(identity_model.django_id)
+            == sorted_api_flags[i]["feature_state_value"]
+        )
+        assert feature_state.enabled == sorted_api_flags[i]["enabled"]
diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
@@ -129,7 +129,9 @@ def segment_override_fs(segment, feature_1):
 def mv_feature_state_value():
     return MultivariateFeatureStateValueModel(
         id=1,
-        multivariate_feature_option=MultivariateFeatureOptionModel(value="test_value"),
+        multivariate_feature_option=MultivariateFeatureOptionModel(
+            id=1, value="test_value"
+        ),
         percentage_allocation=100,
     )
 
diff --git a/tests/unit/environments/test_environments_builders.py b/tests/unit/environments/test_environments_builders.py
@@ -112,12 +112,17 @@ def test_build_environment_model_with_multivariate_flag():
                     {
                         "id": 1,
                         "percentage_allocation": 10.0,
-                        "multivariate_feature_option": {"value": variate_1_value},
+                        "multivariate_feature_option": {
+                            "value": variate_1_value,
+                        },
                     },
                     {
                         "id": 2,
                         "percentage_allocation": 10.0,
-                        "multivariate_feature_option": {"value": variate_2_value},
+                        "multivariate_feature_option": {
+                            "value": variate_2_value,
+                            "id": 2,
+                        },
                     },
                 ],
             },
diff --git a/tests/unit/features/test_features_models.py b/tests/unit/features/test_features_models.py
@@ -16,6 +16,14 @@ def test_initializing_feature_state_creates_default_feature_state_uuid(feature_1
     assert feature_state.featurestate_uuid is not None
 
 
+def test_initializing_multivariate_feature_state_value_creates_default_uuid():
+    mv_feature_option = MultivariateFeatureOptionModel(value="value")
+    mv_fs_value_model = MultivariateFeatureStateValueModel(
+        multivariate_feature_option=mv_feature_option, id=1, percentage_allocation=10
+    )
+    assert mv_fs_value_model.mv_fs_value_uuid is not None
+
+
 def test_feature_state_get_value_no_mv_values(feature_1):
     # Given
     value = "foo"
@@ -49,8 +57,8 @@ def test_feature_state_get_value_mv_values(
     my_feature = FeatureModel(id=1, name="mv_feature", type=STANDARD)
 
     # with some multivariate feature options
-    mv_feature_option_1 = MultivariateFeatureOptionModel(value=mv_feature_value_1)
-    mv_feature_option_2 = MultivariateFeatureOptionModel(value=mv_feature_value_2)
+    mv_feature_option_1 = MultivariateFeatureOptionModel(id=1, value=mv_feature_value_1)
+    mv_feature_option_2 = MultivariateFeatureOptionModel(id=2, value=mv_feature_value_2)
 
     # and associated values
     mv_feature_state_value_1 = MultivariateFeatureStateValueModel(
diff --git a/tests/unit/features/test_schemas.py b/tests/unit/features/test_schemas.py
@@ -0,0 +1,48 @@
+import pytest
+
+from flag_engine.features.schemas import (
+    FeatureStateSchema,
+    MultivariateFeatureOptionSchema,
+    MultivariateFeatureStateValueSchema,
+)
+from flag_engine.utils.exceptions import InvalidPercentageAllocation
+
+
+def test_can_load_multivariate_feature_option_dict_without_id_field():
+    MultivariateFeatureOptionSchema().load({"value": 1})
+
+
+def test_can_load_multivariate_feature_state_value_without_id_field():
+    MultivariateFeatureStateValueSchema().load(
+        {
+            "multivariate_feature_option": {"value": 1},
+            "percentage_allocation": 10,
+        }
+    )
+
+
+def test_dumping_fs_schema_raises_invalid_percentage_allocation_for_invalid_allocation():
+    # Given
+    data = {
+        "multivariate_feature_state_values": [
+            {"multivariate_feature_option": 12, "percentage_allocation": 100},
+            {"multivariate_feature_option": 9, "percentage_allocation": 80},
+        ],
+        "feature_state_value": "value",
+    }
+    # Then
+    with pytest.raises(InvalidPercentageAllocation):
+        FeatureStateSchema().dump(data)
+
+
+def test_dumping_fs_schema_works_for_valid_allocation():
+    # Given
+    data = {
+        "multivariate_feature_state_values": [
+            {"multivariate_feature_option": 12, "percentage_allocation": 20},
+            {"multivariate_feature_option": 9, "percentage_allocation": 80},
+        ],
+        "feature_state_value": "value",
+    }
+    # Then
+    FeatureStateSchema().dump(data)
diff --git a/tests/unit/identities/test_identities_builders.py b/tests/unit/identities/test_identities_builders.py
@@ -2,6 +2,7 @@
 from flag_engine.features.models import FeatureStateModel
 from flag_engine.identities.builders import build_identity_dict, build_identity_model
 from flag_engine.identities.models import IdentityModel
+from flag_engine.utils.collections import IdentityFeaturesList
 
 
 def test_build_identity_model_from_dictionary_no_feature_states():
@@ -23,6 +24,33 @@ def test_build_identity_model_from_dictionary_no_feature_states():
     assert len(identity_model.identity_traits) == 1
 
 
+def test_build_identity_model_from_dictionary_uses_identity_feature_list_for_identity_features():
+    # Given
+    identity_dict = {
+        "id": 1,
+        "identifier": "test-identity",
+        "environment_api_key": "api-key",
+        "created_date": "2021-08-22T06:25:23.406995Z",
+        "identity_features": [
+            {
+                "id": 1,
+                "feature": {
+                    "id": 1,
+                    "name": "test_feature",
+                    "type": STANDARD,
+                },
+                "enabled": True,
+                "feature_state_value": "some-value",
+            }
+        ],
+    }
+
+    # When
+    identity_model = build_identity_model(identity_dict)
+    # Then
+    assert isinstance(identity_model.identity_features, IdentityFeaturesList)
+
+
 def test_build_build_identity_model_from_dict_creates_identity_uuid():
     identity_model = build_identity_model(
         {"identifier": "test_user", "environment_api_key": "some_key"}
diff --git a/tests/unit/utils/marshmallow/test_fields.py b/tests/unit/utils/marshmallow/test_fields.py
@@ -0,0 +1,13 @@
+from marshmallow import fields
+
+from flag_engine.utils.collections import IdentityFeaturesList
+from flag_engine.utils.marshmallow.fields import IdentityFeaturesListField
+
+
+def test_identity_features_list_field_serialize():
+    # Given
+    field = IdentityFeaturesListField(fields.Int())
+    # When
+    value = field.deserialize([1, 2, 3])
+    # Then
+    assert isinstance(value, IdentityFeaturesList)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+[submodule "tests/engine_tests/engine-test-data"]`
	`2`	`+ path = tests/engine_tests/engine-test-data`
	`3`	`+ url = [email protected]:Flagsmith/engine-test-data.git`