Fix test operation to respect JSON types (boolean vs number) - #187
cognis-digital wants to merge 1 commit into
Conversation
RFC 6902 requires the test operation to only consider values equal when they are of the same JSON type. Because Python treats bool as a subclass of int (True == 1, False == 0), a test against the boolean true/false literal wrongly succeeded against the numbers 1/0 (and vice versa). Introduce a small recursive equality helper that keeps booleans distinct from numbers while still comparing numbers numerically (1 == 1.0) and recursing into arrays and objects. Wire it into TestOperation.apply and add regression tests.
|
Validated this independently and it looks complete for the I wrote a 40-case probe from RFC 8259 sections 4, 6 and 9 rather than from the diff, then ran it against both revisions through the public
One structural note, since it is the part that is easy to get wrong: the recursion is doing real work in both directions, not just for the nested cases. A shallow Separately, the same root cause is still live in the diff path on this branch, in case you would rather land the two together: >>> jsonpatch.make_patch({"a": [1]}, {"a": [True]})
[] # empty diff: treated as equal
>>> jsonpatch.make_patch({"a": 1}, {"a": True})
[{'op': 'replace', 'path': '/a', 'value': True}]The scalar case already behaves correctly on |
What
The
testoperation currently compares values with a plain!=. Because Python treatsboolas asubclass of
int(True == 1,False == 0), atestagainst the JSONtrue/falseliteralincorrectly succeeds against the numbers
1/0, and vice versa.Why
RFC 6902 §4.6 defines
testequality as requiring the two values to be "of the same JSON type". Aboolean literal and a number are different JSON types and must not compare equal. This mirrors the
intent already present in
DiffBuilder._compare_values, which usesjson.dumpsspecifically so itcan "recognize the difference between 1 and True".
How
Adds a small recursive helper,
_compare_json_values, that keeps booleans distinct from numbers,still compares numbers numerically (
1 == 1.0), and recurses into arrays and objects so the rulealso holds for nested values.
TestOperation.applyuses it in place of!=. The error type andmessage are unchanged; there is no public API change.
Tests
Adds regression tests covering scalar
bool/number mismatches in both directions, the0/falsecase,
1/1.0numeric equality (still passes), same-typed booleans (still pass), and nested[1]vs[true]/{"q": 1}vs{"q": true}cases. Full suite: 114 passing.