From 710807f5853646cbb1e689b087e89689e8ad5a48 Mon Sep 17 00:00:00 2001
From: A_D <aunderscored@gmail.com>
Date: Sat, 14 Aug 2021 13:41:01 +0200
Subject: [PATCH] made precedence of operators a part of the API

---
 docs/Killswitches.md                   |  5 ++++-
 killswitch.py                          |  8 +++-----
 tests/killswitch.py/test_killswitch.py | 26 ++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/docs/Killswitches.md b/docs/Killswitches.md
index bf66daa2..bfa71e74 100644
--- a/docs/Killswitches.md
+++ b/docs/Killswitches.md
@@ -25,9 +25,12 @@ this to the user (for internal killswitches, anyway).
 | Key (* = required) |       Type       | Description                                                                                   |
 | -----------------: | :--------------: | :-------------------------------------------------------------------------------------------- |
 |          `reason`* |      `str`       | The reason that this killswitch was added                                                     |
-|    `delete_fields` |   `List[str]`    | A list of fields in the matching event to be removed, if they exist.                          |
 |       `set_fields` | `Dict[str, Any]` | A map of key -> contents to update (or overwrite) existing data with                          |
 |    `redact_fields` |   `List[str]`    | A list of fields to redact. This is equivalent to setting the fields to the string "REDACTED" |
+|    `delete_fields` |   `List[str]`    | A list of fields in the matching event to be removed, if they exist.                          |
+
+The order listed above is the precedence for actions. i.e. All set fields are set, then all redact fields are redacted
+then all delete fields are deleted.
 
 An example follows:
 
diff --git a/killswitch.py b/killswitch.py
index d6e7b4a9..728a4350 100644
--- a/killswitch.py
+++ b/killswitch.py
@@ -45,8 +45,9 @@ class SingleKill(NamedTuple):
 
         :param target: data to apply a rule to
         """
-        # fields that contain . might be going deeper into the dict. check here FIRST to see if they exist at the top
-        # level, if not, then work our way down
+
+        for key, value in (self.set_fields if self .set_fields is not None else {}).items():
+            _deep_apply(target, key, value)
 
         for key in (self.redact_fields if self.redact_fields is not None else []):
             _deep_apply(target, key, "REDACTED")
@@ -54,9 +55,6 @@ class SingleKill(NamedTuple):
         for key in (self.delete_fields if self.delete_fields is not None else []):
             _deep_apply(target, key, delete=True)
 
-        for key, value in (self.set_fields if self .set_fields is not None else {}).items():
-            _deep_apply(target, key, value)
-
         return target
 
 
diff --git a/tests/killswitch.py/test_killswitch.py b/tests/killswitch.py/test_killswitch.py
index dc53941e..ebbae1b1 100644
--- a/tests/killswitch.py/test_killswitch.py
+++ b/tests/killswitch.py/test_killswitch.py
@@ -1,4 +1,5 @@
 """Tests of killswitch behaviour."""
+import copy
 from typing import Optional
 
 import pytest
@@ -47,3 +48,28 @@ def test_killswitch(
         return  # we didn't expect any result
 
     assert res == result
+
+
+@pytest.mark.parametrize(
+    ('kill_dict', 'input', 'result'),
+    [
+        ({'set_fields': {'test': None}}, {}, {'test': None}),
+        ({'set_fields': {'test': None}, 'delete_fields': ['test']}, {}, {}),
+        ({'set_fields': {'test': None}, 'redact_fields': ['test']}, {}, {'test': 'REDACTED'}),
+        ({'set_fields': {'test': None}, 'redact_fields': ['test'], 'delete_fields': ['test']}, {}, {}),
+
+    ],
+)
+def test_operator_precedence(
+    kill_dict: killswitch.SingleKillSwitchJSON, input: killswitch.UPDATABLE_DATA, result: killswitch.UPDATABLE_DATA
+) -> None:
+    """Ensure that operators are being applied in the correct order."""
+    kill = killswitch.SingleKill(
+        "", "", kill_dict.get('redact_fields'), kill_dict.get('delete_fields'), kill_dict.get('set_fields')
+    )
+
+    cpy = copy.deepcopy(input)
+
+    kill.apply_rules(cpy)
+
+    assert cpy == result