From 1bf52629a34c0c129c25ff84185d253307876f0e Mon Sep 17 00:00:00 2001 From: Leo Vasanko Date: Wed, 2 Sep 2026 15:11:52 +0000 Subject: [PATCH] Simplify diff producer: no $replace, no value escaping The producer now emits only plain key assignment (full-value replacement for scalars, lists and type changes, including dict-over-non-dict) and $delete for removed keys. $-prefixed keys are still escaped as $$; values are stored verbatim. The consumer remains compatible with jsondiff-produced diffs ($replace, positional $insert/$delete, per-index list diffs). A dict diff against a list state is treated as jsondiff list-ops only when every key is a command or integer position, otherwise as our wholesale replacement. Unescaping is narrowed to the $$ prefix so verbatim single-$ values pass through untouched. --- kanta/diff.py | 36 +++++++++----------- kanta/serialization/base.py | 65 +++++++++++++++++++------------------ tests/test_diff.py | 46 +++++++++++++++++++++----- 3 files changed, 86 insertions(+), 61 deletions(-) diff --git a/kanta/diff.py b/kanta/diff.py index 1f4db0d..ce7d36c 100644 --- a/kanta/diff.py +++ b/kanta/diff.py @@ -1,22 +1,21 @@ """Diff computation and replay utilities. -Diffs use the jsondiff *marshal* format: JSON-serializable dicts where -command keys are ``$delete``, ``$insert`` and ``$replace``, and any user -string (key or value) starting with ``$`` is escaped by prepending another -``$`` (``$foo`` -> ``$$foo``). +Diff format: JSON-serializable dicts where ``$delete`` is the only command +our producer emits; added keys and changed values (scalars, lists, type +changes — lists always wholesale) are plain assignments. A dict value +assigned over a non-dict needs no ``$replace``: the consumer can see from +the old value whether to patch (old is a dict) or replace. User keys +starting with ``$`` are escaped by prepending another ``$`` +(``$foo`` -> ``$$foo``); values are stored verbatim. -Our producer is deliberately simpler than jsondiff's: only object keys are -added/removed recursively; every other change (scalars, lists, type -changes) is a plain full-value assignment. Removed keys are always emitted -as ``$delete``, even when the object becomes empty. The output stays a -subset of jsondiff's marshaled syntax, so ``jsondiff.patch(..., -marshal=True)`` can apply it, and our patcher can apply -jsondiff-produced diffs (including positional ``$insert``/``$delete`` -list edits and per-index nested diffs). +The consumer additionally stays compatible with jsondiff's marshaled +syntax, so it can replay diffs produced by jsondiff itself: ``$replace``, +positional ``$insert``/``$delete`` and per-index nested diffs on lists, +and jsondiff's escaping of ``$``-prefixed values. """ from kanta.structs import ChangeRecord -from kanta.serialization.base import ReplayResult, apply_diff, marshal_value, replay +from kanta.serialization.base import ReplayResult, apply_diff, replay from kanta.serialization.framing import LineFramer from kanta.serialization.json import JsonSerializer @@ -38,21 +37,16 @@ def _diff(previous, current): if deleted: result["$delete"] = deleted for key, new_value in current.items(): - ekey = _escape_key(key) if key not in previous: - result[ekey] = marshal_value(new_value) + result[_escape_key(key)] = new_value else: sub = _diff(previous[key], new_value) if sub is not _UNCHANGED: - result[ekey] = sub + result[_escape_key(key)] = sub return result if result else _UNCHANGED if previous == current: return _UNCHANGED - if isinstance(current, dict): - # A dict assigned over a non-dict is ambiguous with a nested diff; - # like jsondiff, use $replace for that case. - return {"$replace": marshal_value(current)} - return marshal_value(current) + return current def compute_diff(previous: dict, current: dict) -> dict | None: diff --git a/kanta/serialization/base.py b/kanta/serialization/base.py index abb37f3..0970e4f 100644 --- a/kanta/serialization/base.py +++ b/kanta/serialization/base.py @@ -124,39 +124,25 @@ def _patch_state(state: dict, diff: dict) -> dict: return apply_diff(state, diff) -_COMMAND_KEYS = frozenset({"$delete", "$insert", "$replace"}) - - -def _escape(value: str) -> str: - """Escape a user string for the marshaled format (``$x`` -> ``$$x``).""" - return "$" + value if value.startswith("$") else value - - def _unescape(value: str) -> str: - """Reverse escaping; command strings pass through unchanged.""" - if value in _COMMAND_KEYS: - return value - if value.startswith("$"): + """Reverse jsondiff's ``$$`` escaping; command strings pass through. + + Only a ``$$`` prefix is stripped: jsondiff escapes ``$x`` to ``$$x``, + while single ``$`` strings occur verbatim in our own diffs (we do not + escape values) and must be left alone. + """ + if value.startswith("$$"): return value[1:] return value -def marshal_value(value: Any) -> Any: - """Escape all ``$``-prefixed strings in a value stored in a diff.""" - if isinstance(value, dict): - return { - _escape(k) if isinstance(k, str) else k: marshal_value(v) - for k, v in value.items() - } - if isinstance(value, list): - return [marshal_value(v) for v in value] - if isinstance(value, str): - return _escape(value) - return value - - def unmarshal(diff: Any) -> Any: - """Unescape a marshaled diff (keys, values and ``$delete`` entries).""" + """Unescape a marshaled diff (keys, values and ``$delete`` entries). + + Needed for jsondiff-produced diffs, which escape ``$``-prefixed values + as well as keys; our own producer escapes keys only, so unescaping + values is a no-op for them. + """ if isinstance(diff, dict): return { _unescape(k) if isinstance(k, str) else k: unmarshal(v) @@ -170,15 +156,28 @@ def unmarshal(diff: Any) -> Any: def apply_diff(state: Any, diff: Any) -> Any: - """Apply a marshaled jsondiff-format diff. + """Apply a diff. - Mirrors ``jsondiff.patch(..., marshal=True)``: supports ``$replace``, - key-based ``$delete`` on dicts, and positional ``$delete``/``$insert`` - plus per-index nested diffs on lists. + Understands our own format (plain assignment + ``$delete``) and + jsondiff's marshaled syntax: ``$replace``, positional + ``$delete``/``$insert`` and per-index nested diffs on lists. A bare + dict over a non-dict old value is a wholesale replacement. """ return _apply(state, unmarshal(diff)) +def _is_list_patch(diff: dict) -> bool: + """Whether a dict diff against a list state is a jsondiff list edit.""" + for key in diff: + if key in ("$delete", "$insert"): + continue + try: + int(key) + except (ValueError, TypeError): + return False + return True + + def _apply(state: Any, diff: Any) -> Any: if not isinstance(diff, dict): return diff @@ -188,6 +187,10 @@ def _apply(state: Any, diff: Any) -> Any: return diff["$replace"] if isinstance(state, list): + if not _is_list_patch(diff): + # Our own producer replaces a list with a dict (or any other + # type) by plain assignment — no $replace wrapper. + return diff result = list(state) deletes = diff.get("$delete") if deletes: diff --git a/tests/test_diff.py b/tests/test_diff.py index f6fe410..c92dbce 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -63,9 +63,9 @@ def test_list_with_unchanged_prefix_is_full_assignment(): def test_type_changes_are_full_assignment(): assert compute_diff({"a": {"x": 1}}, {"a": [1]}) == {"a": [1]} - # A dict replacing a non-dict needs $replace (a bare dict would read as - # a nested diff); this matches jsondiff. - assert compute_diff({"a": [1]}, {"a": {"x": 1}}) == {"a": {"$replace": {"x": 1}}} + # A dict replacing a non-dict is a plain assignment too: the consumer + # sees from the old value whether to patch (dict) or replace. + assert compute_diff({"a": [1]}, {"a": {"x": 1}}) == {"a": {"x": 1}} assert compute_diff({"a": 1}, {"a": None}) == {"a": None} @@ -79,12 +79,12 @@ def test_dollar_keys_escaped(): assert compute_diff({"$weird": 1}, {}) == {"$delete": ["$$weird"]} -def test_dollar_values_escaped(): - assert compute_diff({"s": 1}, {"s": "$y"}) == {"s": "$$y"} - assert compute_diff({"s": 1}, {"s": "$delete"}) == {"s": "$$delete"} - # Nested values in wholesale assignments are escaped too. +def test_dollar_values_not_escaped(): + # Only keys are escaped; values are stored verbatim, even "$delete". + assert compute_diff({"s": 1}, {"s": "$y"}) == {"s": "$y"} + assert compute_diff({"s": 1}, {"s": "$delete"}) == {"s": "$delete"} assert compute_diff({}, {"o": {"s": "$y", "l": ["$z"]}}) == { - "o": {"s": "$$y", "l": ["$$z"]} + "o": {"s": "$y", "l": ["$z"]} } @@ -144,8 +144,12 @@ def test_apply_list_per_index_nested_diff(): def test_apply_escaped_keys_and_values(): assert apply_diff({}, {"$$weird": 1}) == {"$weird": 1} assert apply_diff({"$weird": 1}, {"$delete": ["$$weird"]}) == {} + # jsondiff escapes $-values as "$$.."; those are unescaped on apply. assert apply_diff({"s": 1}, {"s": "$$y"}) == {"s": "$y"} assert apply_diff({"s": 1}, {"s": "$$delete"}) == {"s": "$delete"} + # Our own producer stores values verbatim; single-$ stays as-is. + assert apply_diff({"s": 1}, {"s": "$y"}) == {"s": "$y"} + assert apply_diff({"s": 1}, {"s": "$delete"}) == {"s": "$delete"} assert apply_diff({}, {"o": {"s": "$$y", "l": ["$$z"]}}) == { "o": {"s": "$y", "l": ["$z"]} } @@ -159,6 +163,19 @@ def test_apply_diff_on_missing_state(): assert apply_diff({}, {"a": {"b": 1}}) == {"a": {"b": 1}} +def test_apply_bare_dict_replaces_non_dict(): + # Our own producer emits no $replace; a dict over a non-dict old value + # is a wholesale replacement. + assert apply_diff({"a": [1, 2]}, {"a": {"x": 1}}) == {"a": {"x": 1}} + assert apply_diff({"a": 5}, {"a": {"x": 1}}) == {"a": {"x": 1}} + assert apply_diff({"a": None}, {"a": {"x": 1}}) == {"a": {"x": 1}} + + +def test_apply_list_patch_still_works_on_lists(): + # jsondiff-style per-index diff keeps list-op semantics on list state. + assert apply_diff({"l": [1, 2]}, {"l": {"1": 9}}) == {"l": [1, 9]} + + # --- jsondiff compatibility, both directions -------------------------------- COMPAT_CASES = [ @@ -188,7 +205,18 @@ COMPAT_CASES = [ ] -@pytest.mark.parametrize("name,old,new", COMPAT_CASES, ids=[c[0] for c in COMPAT_CASES]) +# jsondiff.patch cannot apply our patches for "type change list->dict" +# (we emit a bare dict where jsondiff needs $replace) and "dollar value" +# (we do not escape "$"-prefixed values; jsondiff.patch would strip the +# "$"), so those cases are excluded from this direction. +JSONDIFF_APPLIES_CASES = [ + c for c in COMPAT_CASES if c[0] not in {"type change list->dict", "dollar value"} +] + + +@pytest.mark.parametrize( + "name,old,new", JSONDIFF_APPLIES_CASES, ids=[c[0] for c in JSONDIFF_APPLIES_CASES] +) def test_jsondiff_applies_our_patches(name, old, new): diff = compute_diff(old, new) assert diff is not None