From 80e27968562213ac9ec31b7bb5b1c28a43858875 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 13:28:41 +0000 Subject: [PATCH 1/2] Add option to generate test operations in diffs make_patch, JsonPatch.from_diff and DiffBuilder take a new keyword argument generate_test_ops (default False). When set, each remove and replace operation of the generated patch is preceded by a test operation that checks the value it removes or replaces. The patch then records the previous values and fails with JsonPatchTestFailed when applied to a document that differs there. Removals that the diff turns into moves are not tested. jsondiff gets a matching -t/--test-ops option. Based on #154 by Robert Cochran, reimplemented on top of the current diff engine, which resolves array indices when replaying operations, so the test operations get the same indices as the operations they guard. The new argument is added after the existing ones, so positional callers (including of `optimization`) are unaffected. Co-Authored-By: Robert Cochran Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpidDeSzMcCa4Krt54E --- doc/commandline.rst | 4 +- doc/tutorial.rst | 19 +++++++ jsonpatch.py | 50 +++++++++++++++--- jsonpatch_cli.py | 5 +- property_tests.py | 19 +++++++ tests.py | 122 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 209 insertions(+), 10 deletions(-) diff --git a/doc/commandline.rst b/doc/commandline.rst index e6e0965..b844336 100644 --- a/doc/commandline.rst +++ b/doc/commandline.rst @@ -13,7 +13,7 @@ in-place, it also writes UTF-8. The program ``jsondiff`` can be used to create a JSON patch by comparing two JSON files :: - usage: jsondiff [-h] [--indent INDENT] [-u] [-v] FILE1 FILE2 + usage: jsondiff [-h] [--indent INDENT] [-u] [-t] [-v] FILE1 FILE2 Diff two JSON files @@ -27,6 +27,8 @@ JSON files :: -u, --preserve-unicode Output Unicode character as-is without using Code Point + -t, --test-ops Precede each remove and replace operation by a test of + the value it changes -v, --version show program's version number and exit Example diff --git a/doc/tutorial.rst b/doc/tutorial.rst index b30b85c..1af72fa 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -79,6 +79,25 @@ Applying it to any other object, such as ``dst``, usually fails with a ... jsonpatch.JsonPatchConflict: can't remove a non-existent object 'foo' +Applying it to an object with the same structure but different values may +succeed, though, and overwrite those values. With ``generate_test_ops=True`` +each ``remove`` and ``replace`` operation is preceded by a ``test`` operation +that checks the value it removes or replaces. The patch then also records the +previous values, and fails with a ``JsonPatchTestFailed`` if they differ: + +.. code-block:: python + + >>> patch = jsonpatch.make_patch({'a': 1, 'b': 2}, {'a': 3}, + ... generate_test_ops=True) + >>> patch.patch # doctest: +NORMALIZE_WHITESPACE + [{'op': 'test', 'path': '/b', 'value': 2}, {'op': 'remove', 'path': '/b'}, + {'op': 'test', 'path': '/a', 'value': 1}, + {'op': 'replace', 'path': '/a', 'value': 3}] + >>> patch.apply({'a': 5, 'b': 2}) + Traceback (most recent call last): + ... + jsonpatch.JsonPatchTestFailed: 5 () is not equal to tested value 1 () + If a patch is only used once, it is not necessary to create a patch object explicitly. diff --git a/jsonpatch.py b/jsonpatch.py index b9d80d7..f85958e 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -140,7 +140,7 @@ def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): return patch.apply(doc, in_place) -def make_patch(src, dst, pointer_cls=JsonPointer): +def make_patch(src, dst, pointer_cls=JsonPointer, generate_test_ops=False): """Generates patch by comparing two document objects. Actually is a proxy to :meth:`JsonPatch.from_diff` method. The resulting patch transforms `src` into `dst`, so it has to be applied to `src` (or to a @@ -155,6 +155,12 @@ def make_patch(src, dst, pointer_cls=JsonPointer): :param pointer_cls: JSON pointer class to use. :type pointer_cls: Type[JsonPointer] + :param generate_test_ops: Precede each ``remove`` and ``replace`` + operation by a ``test`` operation that checks + the value it removes or replaces, so the patch + fails on documents that differ from `src` there. + :type generate_test_ops: bool + >>> src = {'foo': 'bar', 'numbers': [1, 3, 4, 8]} >>> dst = {'baz': 'qux', 'numbers': [1, 4, 7]} >>> patch = make_patch(src, dst) @@ -163,7 +169,8 @@ def make_patch(src, dst, pointer_cls=JsonPointer): True """ - return JsonPatch.from_diff(src, dst, pointer_cls=pointer_cls) + return JsonPatch.from_diff(src, dst, pointer_cls=pointer_cls, + generate_test_ops=generate_test_ops) class PatchOperation(object): @@ -597,7 +604,7 @@ def from_string(cls, patch_str, loads=None, pointer_cls=JsonPointer): @classmethod def from_diff( cls, src, dst, optimization=True, dumps=None, - pointer_cls=JsonPointer, + pointer_cls=JsonPointer, generate_test_ops=False, ): """Creates JsonPatch instance based on comparison of two document objects. Json patch would be created for `src` argument against `dst` @@ -621,6 +628,12 @@ def from_diff( :param pointer_cls: JSON pointer class to use. :type pointer_cls: Type[JsonPointer] + :param generate_test_ops: Precede each ``remove`` and ``replace`` + operation by a ``test`` operation that checks + the value it removes or replaces, so the patch + fails on documents that differ from `src` there. + :type generate_test_ops: bool + :return: :class:`JsonPatch` instance. >>> src = {'foo': 'bar', 'numbers': [1, 3, 4, 8]} @@ -632,7 +645,8 @@ def from_diff( """ json_dumper = dumps or cls.json_dumper builder = DiffBuilder(src, dst, json_dumper, pointer_cls=pointer_cls, - optimization=optimization) + optimization=optimization, + generate_test_ops=generate_test_ops) builder._compare_values((), None, src, dst) ops = list(builder.execute()) return cls(ops, pointer_cls=pointer_cls) @@ -691,10 +705,14 @@ def _get_operation(self, operation): class DiffBuilder(object): def __init__(self, src_doc, dst_doc, dumps=json.dumps, pointer_cls=JsonPointer, - optimization=True): + optimization=True, generate_test_ops=False): self.dumps = dumps self.pointer_cls = pointer_cls self.optimization = optimization + self.generate_test_ops = generate_test_ops + # the index of the test operation preceding a remove operation, by + # the id of the remove operation + self.remove_tests = {} self.index_storage = [{}, {}] self.__root = root = [] self.src_doc = src_doc @@ -841,6 +859,10 @@ def _item_added(self, path, key, item): source = self._parts(removed) # RFC 6902 does not allow moving a value into its own children if not _is_inside(target, source): + # the value is moved rather than removed, so it is not tested + test_index = self.remove_tests.pop(id(index[2]), None) + if test_index is not None: + self.remove(test_index) self.remove(index) if source != target: self.insert({'op': 'move', 'from': removed, @@ -875,14 +897,26 @@ def _item_removed(self, path, key, item): self._set_present(added_location, False) return + test_index = self._insert_test(location, item) self._set_present(location, False) new_index = self.insert({'op': 'remove', 'path': location}) + if test_index is not None: + self.remove_tests[id(new_index[2])] = test_index self.store_index(item, new_index, _ST_REMOVE) - def _item_replaced(self, path, key, item): + def _insert_test(self, location, value): + """ Inserts a test operation that checks the value at location, if + test operations are generated """ + if self.generate_test_ops: + return self.insert({'op': 'test', 'path': location, + 'value': value}) + + def _item_replaced(self, path, key, item, old_item): + location = self._location(_path_join(path, key)) + self._insert_test(location, old_item) self.insert({ 'op': 'replace', - 'path': self._location(_path_join(path, key)), + 'path': location, 'value': item, }) @@ -1017,7 +1051,7 @@ def _compare_values(self, path, key, src, dst): return else: - self._item_replaced(path, key, dst) + self._item_replaced(path, key, dst, src) def _positional_cost(old, new): diff --git a/jsonpatch_cli.py b/jsonpatch_cli.py index 261aec1..1149e36 100644 --- a/jsonpatch_cli.py +++ b/jsonpatch_cli.py @@ -18,6 +18,9 @@ help='Indent output by n spaces') jsondiff_parser.add_argument('-u', '--preserve-unicode', action='store_true', help='Output Unicode character as-is without using Code Point') +jsondiff_parser.add_argument('-t', '--test-ops', action='store_true', + help='Precede each remove and replace operation ' + 'by a test of the value it changes') jsondiff_parser.add_argument('-v', '--version', action='version', version='%(prog)s ' + jsonpatch.__version__) @@ -34,7 +37,7 @@ def diff_files(): args = jsondiff_parser.parse_args() doc1 = json.load(args.FILE1) doc2 = json.load(args.FILE2) - patch = jsonpatch.make_patch(doc1, doc2) + patch = jsonpatch.make_patch(doc1, doc2, generate_test_ops=args.test_ops) if patch.patch: print(json.dumps(patch.patch, indent=args.indent, ensure_ascii=not(args.preserve_unicode))) sys.exit(1) diff --git a/property_tests.py b/property_tests.py index 8dc1c97..c866385 100644 --- a/property_tests.py +++ b/property_tests.py @@ -299,6 +299,25 @@ def test_roundtrip_without_move_optimization(self, docs): for op in patch)) assert_json_equal(patch.apply(src), dst) + @given(doc_pairs, st.booleans()) + def test_generate_test_ops(self, docs, optimization): + src, dst = docs + plain = jsonpatch.JsonPatch.from_diff( + src, dst, optimization=optimization).patch + patch = jsonpatch.JsonPatch.from_diff( + src, dst, optimization=optimization, generate_test_ops=True) + note(patch.patch) + # the same operations, each remove and replace preceded by a test + # of the value it changes, which holds when applied to src + self.assertEqual([op for op in patch if op['op'] != 'test'], plain) + expected = [] + for op in plain: + if op['op'] in ('remove', 'replace'): + expected.append(('test', op['path'])) + expected.append((op['op'], op['path'])) + self.assertEqual([(op['op'], op['path']) for op in patch], expected) + assert_json_equal(patch.apply(src), dst) + @given(safe_doc_pairs) # the diff considered e.g. 1 and true equal, #180 @example(docs=([0], [False])) diff --git a/tests.py b/tests.py index 6b5fd7d..d145471 100755 --- a/tests.py +++ b/tests.py @@ -1322,6 +1322,113 @@ def test_long_list_of_repeated_items_is_matched_up(self): ]) +class GenerateTestOpsTests(unittest.TestCase): + """ make_patch(..., generate_test_ops=True) tests the values it removes + or replaces """ + + def make_patch(self, src, dst, **kwargs): + patch = jsonpatch.make_patch(src, dst, generate_test_ops=True, **kwargs) + self.assertEqual(patch.apply(src), dst) + return patch.patch + + def test_disabled_by_default(self): + patch = jsonpatch.make_patch({'a': 1, 'b': 2}, {'a': 3}) + self.assertNotIn('test', [op['op'] for op in patch]) + + def test_replace(self): + self.assertEqual(self.make_patch({'x/y': 1}, {'x/y': 2}), [ + {'op': 'test', 'path': '/x~1y', 'value': 1}, + {'op': 'replace', 'path': '/x~1y', 'value': 2}, + ]) + + def test_replace_root(self): + self.assertEqual(self.make_patch(1, [1]), [ + {'op': 'test', 'path': '', 'value': 1}, + {'op': 'replace', 'path': '', 'value': [1]}, + ]) + + def test_remove(self): + src = {'name': 'fred', 'friend': 'barney', 'spouse': 'wilma'} + dst = {'name': 'fred', 'spouse': 'wilma'} + self.assertEqual(self.make_patch(src, dst), [ + {'op': 'test', 'path': '/friend', 'value': 'barney'}, + {'op': 'remove', 'path': '/friend'}, + ]) + + def test_replaced_array_item(self): + # a remove and an add of the same array item are merged to a replace + self.assertEqual(self.make_patch({'foo': [1, 2, 3]}, {'foo': [3, 2, 3]}), [ + {'op': 'test', 'path': '/foo/0', 'value': 1}, + {'op': 'replace', 'path': '/foo/0', 'value': 3}, + ]) + + def test_array_indices_of_tests(self): + # each test is at the index the item has when it is removed + self.assertEqual(self.make_patch([1, 2, 3, 4], [2, 4]), [ + {'op': 'test', 'path': '/0', 'value': 1}, + {'op': 'remove', 'path': '/0'}, + {'op': 'test', 'path': '/1', 'value': 3}, + {'op': 'remove', 'path': '/1'}, + ]) + + def test_moves_are_not_tested(self): + # the removed value is moved instead, so it is not tested + self.assertEqual(self.make_patch({'a': {'x': 1}}, {'b': {'x': 1}}), + [{'op': 'move', 'from': '/a', 'path': '/b'}]) + self.assertEqual(self.make_patch([4, 1, 2, 3], [1, 2, 3, 4]), + [{'op': 'move', 'from': '/0', 'path': '/3'}]) + + def test_without_move_optimization(self): + patch = jsonpatch.JsonPatch.from_diff( + {'a': {'x': 1}}, {'b': {'x': 1}}, optimization=False, + generate_test_ops=True) + self.assertEqual(patch.patch, [ + {'op': 'test', 'path': '/a', 'value': {'x': 1}}, + {'op': 'remove', 'path': '/a'}, + {'op': 'add', 'path': '/b', 'value': {'x': 1}}, + ]) + + def test_same_operations_besides_tests(self): + documents = [ + ({'foo': 'bar', 'numbers': [1, 3, 4, 8]}, + {'baz': 'qux', 'numbers': [1, 4, 7]}), + ([8, 7, 2, 1, 0, 9, 4, 3, 5, 6], [7, 2, 1, 0, 9, 4, 3, 6, 5, 8]), + ({'school': {'names': ['Kevin', 'Carl']}}, + {'school': {'names': ['Carl', 'Kate', 'Kevin', 'Jake']}}), + ({'A': {'D': 1.0}, 'B': {'E': 'a'}}, + {'A': {'C': 'a'}, 'B': {'C': True}}), + ([{'a': 1, 'b': 2, 'd': 5}], [{'d': 6}]), + ] + for src, dst in documents: + with self.subTest(src=src, dst=dst): + plain = jsonpatch.make_patch(src, dst).patch + tested = self.make_patch(src, dst) + self.assertEqual([op for op in tested if op['op'] != 'test'], + plain) + for i, op in enumerate(tested): + if op['op'] == 'test': + self.assertIn(tested[i + 1]['op'], + ('remove', 'replace')) + self.assertEqual(tested[i + 1]['path'], op['path']) + self.assertEqual( + sum(op['op'] == 'test' for op in tested), + sum(op['op'] in ('remove', 'replace') for op in plain)) + + def test_fails_on_changed_values(self): + patch = jsonpatch.make_patch({'a': 1, 'b': [1, 2]}, {'b': [1]}, + generate_test_ops=True) + for doc in ({'a': 2, 'b': [1, 2]}, {'a': 1, 'b': [1, 3]}): + with self.subTest(doc=doc): + with self.assertRaises(jsonpatch.JsonPatchTestFailed): + patch.apply(doc) + + def test_from_diff(self): + patch = jsonpatch.JsonPatch.from_diff({'a': 1}, {}, + generate_test_ops=True) + self.assertEqual(patch.patch, [{'op': 'test', 'path': '/a', 'value': 1}, + {'op': 'remove', 'path': '/a'}]) + + class ArrayItemsTests(unittest.TestCase): """ The labels that order the items of an array in make_patch run out between two items, and have to be spread out again. make_patch gets there @@ -2048,6 +2155,20 @@ def test_jsondiff_indent(self): patch = jsonpatch.make_patch({'a': 1}, {'b': 1}).patch self.assertEqual(res.stdout, json.dumps(patch, indent=2) + '\n') + def test_jsondiff_test_ops(self): + src = self.write('a.json', {'a': 1, 'b': 2}) + dst = self.write('b.json', {'a': 3}) + for option in ('-t', '--test-ops'): + with self.subTest(option=option): + res = self.run_script('jsondiff', option, src, dst) + self.assertEqual(res.returncode, 1, res.stderr) + self.assertEqual(json.loads(res.stdout), [ + {'op': 'test', 'path': '/b', 'value': 2}, + {'op': 'remove', 'path': '/b'}, + {'op': 'test', 'path': '/a', 'value': 1}, + {'op': 'replace', 'path': '/a', 'value': 3}, + ]) + def test_jsondiff_preserve_unicode(self): src = self.write('a.json', {'a': 'x'}) dst = self.write('b.json', {'a': 'รค'}) @@ -2420,6 +2541,7 @@ def get_suite(): suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ConflictTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(StringIndexingTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(OptimizationTests)) + suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(GenerateTestOpsTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ArrayItemsTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(JsonPointerTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(JsonPatchCreationTest)) From b5385ccd66ae27d62fa2f224a8f45e88f6c586da Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 20:55:28 +0000 Subject: [PATCH 2/2] Copy the values of generated test operations The test operations referred to the values in src, so changing src after diffing changed what the patch tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpidDeSzMcCa4Krt54E --- jsonpatch.py | 3 ++- tests.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/jsonpatch.py b/jsonpatch.py index f85958e..f57f1e6 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -908,8 +908,9 @@ def _insert_test(self, location, value): """ Inserts a test operation that checks the value at location, if test operations are generated """ if self.generate_test_ops: + # a copy, so that changing src afterwards does not change the test return self.insert({'op': 'test', 'path': location, - 'value': value}) + 'value': copy.deepcopy(value)}) def _item_replaced(self, path, key, item, old_item): location = self._location(_path_join(path, key)) diff --git a/tests.py b/tests.py index d145471..5672e3e 100755 --- a/tests.py +++ b/tests.py @@ -1422,6 +1422,23 @@ def test_fails_on_changed_values(self): with self.assertRaises(jsonpatch.JsonPatchTestFailed): patch.apply(doc) + def test_tested_values_are_copies(self): + # changing src after diffing does not change what is tested + src = {'a': {'nested': [1]}, 'b': [1]} + original = copy.deepcopy(src) + patch = jsonpatch.make_patch(src, {'b': 2}, generate_test_ops=True) + src['a']['nested'].append(2) + src['b'].append(2) + self.assertEqual(patch.patch, [ + {'op': 'test', 'path': '/a', 'value': {'nested': [1]}}, + {'op': 'remove', 'path': '/a'}, + {'op': 'test', 'path': '/b', 'value': [1]}, + {'op': 'replace', 'path': '/b', 'value': 2}, + ]) + self.assertEqual(patch.apply(original), {'b': 2}) + with self.assertRaises(jsonpatch.JsonPatchTestFailed): + patch.apply(src) + def test_from_diff(self): patch = jsonpatch.JsonPatch.from_diff({'a': 1}, {}, generate_test_ops=True)