From 1e4364de29cb9d1855ab7a940f105705888f1f77 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Sun, 22 Dec 2024 15:49:15 -0100 Subject: [PATCH] IOTData: Simplify delta calculation (#8423) --- moto/iotdata/models.py | 66 +++++++++++------------------- setup.cfg | 7 +--- tests/test_iotdata/test_iotdata.py | 41 +++++++++++++++++++ 3 files changed, 66 insertions(+), 48 deletions(-) diff --git a/moto/iotdata/models.py b/moto/iotdata/models.py index eecf3e37e3b4..009a293b6d75 100644 --- a/moto/iotdata/models.py +++ b/moto/iotdata/models.py @@ -2,8 +2,6 @@ import time from typing import Any, Dict, List, Optional, Tuple -import jsondiff - from moto.core.base_backend import BackendDict, BaseBackend from moto.core.common_models import BaseModel from moto.core.utils import merge_dicts @@ -77,51 +75,35 @@ def parse_payload(cls, desired: Any, reported: Any) -> Any: # type: ignore[misc elif reported is None and desired: delta = desired elif desired and reported: - delta = jsondiff.diff(reported, desired, syntax="symmetric") - delta.pop(jsondiff.add, None) - delta.pop(jsondiff.delete, None) - delta.pop(jsondiff.replace, None) - cls._resolve_nested_deltas(desired, reported, delta) + delta = cls._compute_delta_dict(desired, reported) else: delta = None return delta @classmethod - def _resolve_nested_deltas(cls, desired: Any, reported: Any, delta: Any) -> None: # type: ignore[misc] - for key, value in delta.items(): - if isinstance(value, dict): - # delta = {insert: [(0, val1),, ], delete: [(0, val2),..] } - if isinstance(reported.get(key), list): - # Delta is a dict, but were supposed to have a list - # Explicitly insert/delete from the original list - list_delta = reported[key].copy() - if jsondiff.delete in value: - for idx, _ in sorted(value[jsondiff.delete], reverse=True): - del list_delta[idx] - if jsondiff.insert in value: - for new_val in sorted(value[jsondiff.insert], reverse=True): - list_delta.insert(*new_val) - delta[key] = list_delta - if isinstance(reported.get(key), dict): - # Delta is a dict, exactly what we're expecting - # Just delete any unknown symbols - value.pop(jsondiff.add, None) - value.pop(jsondiff.delete, None) - value.pop(jsondiff.replace, None) - elif isinstance(value, list): - # delta = [v1, v2] - # If the actual value is a string/bool/int and our delta is a list, - # that means that the delta reports both values - reported and desired - # But we only want to show the desired value here - # (Note that bool is a type of int, so we don't have to explicitly mention it) - if isinstance(desired.get(key), (str, int, float)): - delta[key] = desired[key] - - # Resolve nested deltas - if isinstance(delta[key], dict): - cls._resolve_nested_deltas( - desired.get(key), reported.get(key), delta[key] - ) + def _compute_delta_dict(cls, desired: Any, reported: Any) -> Dict[str, Any]: # type: ignore[misc] + delta = {} + for key, value in desired.items(): + delta_value = cls._compute_delta(reported.get(key), value) + + if delta_value is not None: + delta[key] = delta_value + return delta + + @classmethod + def _compute_delta(cls, reported_value: Any, desired_value: Any) -> Any: # type: ignore[misc] + if reported_value == desired_value: + return None + + if isinstance(desired_value, dict) and isinstance(reported_value, dict): + return cls._compute_delta_dict(desired_value, reported_value) + + # Types are different, or + # Both types are intrinsic values (str, int, etc), or + # Both types are lists: + # + # Just return the desired value + return desired_value def _create_metadata_from_state(self, state: Any, ts: Any) -> Any: """ diff --git a/setup.cfg b/setup.cfg index 06b0c0461d98..086c4acbcc49 100644 --- a/setup.cfg +++ b/setup.cfg @@ -55,7 +55,6 @@ all = jsonschema openapi-spec-validator>=0.5.0 pyparsing>=3.0.7 - jsondiff>=1.1.2 py-partiql-parser==0.5.6 aws-xray-sdk!=0.96,>=0.93 setuptools @@ -70,7 +69,6 @@ proxy = cfn-lint>=0.40.0 openapi-spec-validator>=0.5.0 pyparsing>=3.0.7 - jsondiff>=1.1.2 py-partiql-parser==0.5.6 aws-xray-sdk!=0.96,>=0.93 setuptools @@ -85,7 +83,6 @@ server = cfn-lint>=0.40.0 openapi-spec-validator>=0.5.0 pyparsing>=3.0.7 - jsondiff>=1.1.2 py-partiql-parser==0.5.6 aws-xray-sdk!=0.96,>=0.93 setuptools @@ -120,7 +117,6 @@ cloudformation = cfn-lint>=0.40.0 openapi-spec-validator>=0.5.0 pyparsing>=3.0.7 - jsondiff>=1.1.2 py-partiql-parser==0.5.6 aws-xray-sdk!=0.96,>=0.93 setuptools @@ -173,7 +169,7 @@ guardduty = iam = inspector2 = iot = -iotdata = jsondiff>=1.1.2 +iotdata = ivs = kinesis = kinesisvideo = @@ -209,7 +205,6 @@ resourcegroupstaggingapi = cfn-lint>=0.40.0 openapi-spec-validator>=0.5.0 pyparsing>=3.0.7 - jsondiff>=1.1.2 py-partiql-parser==0.5.6 route53 = route53resolver = diff --git a/tests/test_iotdata/test_iotdata.py b/tests/test_iotdata/test_iotdata.py index 234c2cc361cc..6b6b80d6830c 100644 --- a/tests/test_iotdata/test_iotdata.py +++ b/tests/test_iotdata/test_iotdata.py @@ -196,6 +196,7 @@ def test_delete_field_from_device_shadow(name: Optional[str] = None) -> None: @pytest.mark.parametrize( "desired,initial_delta,reported,delta_after_report", [ + # Boolean flip ( {"desired": {"online": True}}, {"desired": {"online": True}, "delta": {"online": True}}, @@ -215,7 +216,9 @@ def test_delete_field_from_device_shadow(name: Optional[str] = None) -> None: "reported": {"online": False, "enabled": True}, }, ), + # No data ({}, {}, {"reported": {"online": False}}, {"reported": {"online": False}}), + # Missing data ({}, {}, {"reported": {"online": None}}, {}), ( {"desired": {}}, @@ -223,6 +226,24 @@ def test_delete_field_from_device_shadow(name: Optional[str] = None) -> None: {"reported": {"online": False}}, {"reported": {"online": False}}, ), + # Missing key + ( + {"desired": {"enabled": True}}, + {"desired": {"enabled": True}, "delta": {"enabled": True}}, + {"reported": {}}, + {"desired": {"enabled": True}, "delta": {"enabled": True}}, + ), + # Changed key + ( + {"desired": {"enabled": True}}, + {"desired": {"enabled": True}, "delta": {"enabled": True}}, + {"reported": {"online": True}}, + { + "desired": {"enabled": True}, + "reported": {"online": True}, + "delta": {"enabled": True}, + }, + ), # Remove from list ( {"reported": {"list": ["value_1", "value_2"]}}, @@ -278,6 +299,26 @@ def test_delete_field_from_device_shadow(name: Optional[str] = None) -> None: "reported": {"a": {"b": {"c": "d", "e": "f"}}}, }, ), + ( + {"reported": {"a1": {"b1": {"c": "d"}}}}, + {"reported": {"a1": {"b1": {"c": "d"}}}}, + {"desired": {"a1": {"b1": {"c": "d"}}, "a2": {"b2": "sth"}}}, + { + "delta": {"a2": {"b2": "sth"}}, + "desired": {"a1": {"b1": {"c": "d"}}, "a2": {"b2": "sth"}}, + "reported": {"a1": {"b1": {"c": "d"}}}, + }, + ), + ( + {"reported": {"a": {"b1": {"c": "d"}}}}, + {"reported": {"a": {"b1": {"c": "d"}}}}, + {"desired": {"a": {"b1": {"c": "d"}, "b2": "sth"}}}, + { + "delta": {"a": {"b2": "sth"}}, + "desired": {"a": {"b1": {"c": "d"}, "b2": "sth"}}, + "reported": {"a": {"b1": {"c": "d"}}}, + }, + ), ], ) def test_delta_calculation(