fix: PERCENTAGE_SPLIT working for undefined keys (#256)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: khvn26

.github/workflows/pull-request.yml

@@ -35,17 +35,6 @@ jobs:
           python -m pip install --upgrade pip
           pip install -r requirements.txt -r requirements-dev.txt
 
-      - name: Check Formatting
-        run: black --check .
-
-      - name: Check Imports
-        run: |
-          git ls-files | grep '\.py$' | xargs absolufy-imports
-          isort . --check
-
-      - name: Check flake8 linting
-        run: flake8 .
-
       - name: Check Typing
         run: mypy --strict .
 

.pre-commit-config.yaml

@@ -1,10 +1,4 @@
 repos:
-  - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.5.1
-    hooks:
-      - id: mypy
-        args: [--strict]
-        additional_dependencies: [pydantic, pytest, pytest_mock, types-pytest-lazy-fixture, types-setuptools, semver]
   - repo: https://github.com/MarcoGorelli/absolufy-imports
     rev: v0.3.1
     hooks:
@@ -33,3 +27,15 @@ repos:
     rev: v2.7.1
     hooks:
       - id: prettier
+  - repo: local
+    hooks:
+      - id: python-typecheck
+        name: python-typecheck
+        language: system
+        entry: mypy --strict flag_engine tests
+        require_serial: true
+        pass_filenames: false
+        types: [python]
+
+ci:
+  skip: [python-typecheck]

.release-please-manifest.json

@@ -1,3 +1,3 @@
 {
-    ".": "7.0.0"
+  ".": "7.0.0"
 }

CHANGELOG.md

@@ -216,10 +216,15 @@ def context_matches_condition(
     if condition["operator"] == constants.PERCENTAGE_SPLIT:
         if context_value is not None:
             object_ids = [segment_key, context_value]
+        elif identity_context := context.get("identity"):
+            object_ids = [segment_key, identity_context["key"]]
         else:
-            object_ids = [segment_key, get_context_value(context, "$.identity.key")]
+            return False
 
-        float_value = float(condition["value"])
+        try:
+            float_value = float(condition["value"])
+        except ValueError:
+            return False
         return get_hashed_percentage_for_object_ids(object_ids) <= float_value
 
     if condition["operator"] == constants.IS_NOT_SET:

flag_engine/segments/evaluator.py

@@ -1,62 +1,62 @@
 {
-    "bootstrap-sha": "902aa899ce156012e6551fe9a8499848ccafdfbf",
-    "packages": {
-        ".": {
-            "release-type": "python",
-            "changelog-path": "CHANGELOG.md",
-            "bump-minor-pre-major": false,
-            "bump-patch-for-minor-pre-major": false,
-            "draft": false,
-            "prerelease": false,
-            "include-component-in-tag": false
-        }
-    },
-    "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json",
-    "changelog-sections": [
-        {
-            "type": "feat",
-            "hidden": false,
-            "section": "Features"
-        },
-        {
-            "type": "fix",
-            "hidden": false,
-            "section": "Bug Fixes"
-        },
-        {
-            "type": "ci",
-            "hidden": false,
-            "section": "CI"
-        },
-        {
-            "type": "docs",
-            "hidden": false,
-            "section": "Docs"
-        },
-        {
-            "type": "deps",
-            "hidden": false,
-            "section": "Dependency Updates"
-        },
-        {
-            "type": "perf",
-            "hidden": false,
-            "section": "Performance Improvements"
-        },
-        {
-            "type": "refactor",
-            "hidden": false,
-            "section": "Refactoring"
-        },
-        {
-            "type": "test",
-            "hidden": false,
-            "section": "Tests"
-        },
-        {
-            "type": "chore",
-            "hidden": false,
-            "section": "Other"
-        }
-    ]
+  "bootstrap-sha": "902aa899ce156012e6551fe9a8499848ccafdfbf",
+  "packages": {
+    ".": {
+      "release-type": "python",
+      "changelog-path": "CHANGELOG.md",
+      "bump-minor-pre-major": false,
+      "bump-patch-for-minor-pre-major": false,
+      "draft": false,
+      "prerelease": false,
+      "include-component-in-tag": false
+    }
+  },
+  "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json",
+  "changelog-sections": [
+    {
+      "type": "feat",
+      "hidden": false,
+      "section": "Features"
+    },
+    {
+      "type": "fix",
+      "hidden": false,
+      "section": "Bug Fixes"
+    },
+    {
+      "type": "ci",
+      "hidden": false,
+      "section": "CI"
+    },
+    {
+      "type": "docs",
+      "hidden": false,
+      "section": "Docs"
+    },
+    {
+      "type": "deps",
+      "hidden": false,
+      "section": "Dependency Updates"
+    },
+    {
+      "type": "perf",
+      "hidden": false,
+      "section": "Performance Improvements"
+    },
+    {
+      "type": "refactor",
+      "hidden": false,
+      "section": "Refactoring"
+    },
+    {
+      "type": "test",
+      "hidden": false,
+      "section": "Tests"
+    },
+    {
+      "type": "chore",
+      "hidden": false,
+      "section": "Other"
+    }
+  ]
 }

release-please-config.json

@@ -224,7 +224,13 @@ def test_context_in_segment(
 
 @pytest.mark.parametrize(
     "segment_split_value, identity_hashed_percentage, expected_result",
-    ((10, 1, True), (100, 50, True), (0, 1, False), (10, 20, False)),
+    (
+        (10, 1, True),
+        (100, 50, True),
+        (0, 1, False),
+        (10, 20, False),
+        ("invalid", 100, False),
+    ),
 )
 def test_context_in_segment_percentage_split(
     mocker: MockerFixture,
@@ -270,6 +276,49 @@ def test_context_in_segment_percentage_split(
     assert result == expected_result
 
 
+def test_context_in_segment_percentage_split__no_identity__returns_expected(
+    mocker: MockerFixture,
+    context: EvaluationContext,
+) -> None:
+    # Given
+    del context["identity"]
+
+    segment_context: SegmentContext = {
+        "key": "1",
+        "name": "% split",
+        "rules": [
+            {
+                "type": constants.ALL_RULE,
+                "conditions": [],
+                "rules": [
+                    {
+                        "type": constants.ALL_RULE,
+                        "conditions": [
+                            {
+                                "operator": constants.PERCENTAGE_SPLIT,
+                                "property": "",
+                                "value": "10",
+                            }
+                        ],
+                        "rules": [],
+                    }
+                ],
+            }
+        ],
+    }
+
+    mock_get_hashed_percentage = mocker.patch(
+        "flag_engine.segments.evaluator.get_hashed_percentage_for_object_ids"
+    )
+
+    # When
+    result = is_context_in_segment(context=context, segment_context=segment_context)
+
+    # Then
+    mock_get_hashed_percentage.assert_not_called()
+    assert result is False
+
+
 def test_context_in_segment_percentage_split__trait_value__calls_expected(
     mocker: MockerFixture,
     context: EvaluationContext,