From ae6d526709084ecc3ee79c09f0a993ef0eba6de3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:17:49 +0000 Subject: [PATCH 01/14] Add opt-in atomic application of in-place patches (#115) With in_place=True, a patch that fails part-way left the changes of the preceding operations in the document. JsonPatch.apply and apply_patch now take atomic=True, which undoes those changes before re-raising, so the document is either fully patched or left unchanged. The built-in operations make their changes through a journal that records how to undo each one (the changed container and the old value), so this does not copy the document. Removed object members are restored at their former position, and the original containers are put back, so references into the document stay valid. Patches with custom operations, whose changes can't be undone, are tried on a copy first instead. The default behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- doc/tutorial.rst | 23 ++++ jsonpatch-stubs/__init__.pyi | 3 +- jsonpatch.py | 219 +++++++++++++++++++++++++++++++---- property_tests.py | 12 ++ tests.py | 213 ++++++++++++++++++++++++++++++++++ typetests/usage.py | 2 + 6 files changed, 449 insertions(+), 23 deletions(-) diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 1af72fa..55a30fa 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -151,6 +151,29 @@ do not roll back changes already made in place: >>> original {'value': 2} +To get all-or-nothing behaviour without copying the document, pass +``atomic=True`` together with ``in_place=True``. If an operation fails, the +changes of the preceding operations are undone before the exception is +raised, so the document is either fully patched or left unchanged: + +.. code-block:: python + + >>> original = {'value': 1} + >>> try: + ... jsonpatch.apply_patch(original, patch, in_place=True, atomic=True) + ... except jsonpatch.JsonPatchTestFailed: + ... print('Test failed') + Test failed + >>> original + {'value': 1} + +This only keeps the values that the patch removes or replaces until it has +been applied, so it needs much less memory than ``in_place=False`` for large +documents. The original containers are restored, so references into the +document stay valid. Custom operations (see :class:`jsonpatch.JsonPatch`'s +``operations``) whose changes can't be undone are first tried on a copy of +the document instead. + Replacing the Whole Document ---------------------------- diff --git a/jsonpatch-stubs/__init__.pyi b/jsonpatch-stubs/__init__.pyi index 1fad29e..db8034d 100644 --- a/jsonpatch-stubs/__init__.pyi +++ b/jsonpatch-stubs/__init__.pyi @@ -43,6 +43,7 @@ def apply_patch( patch: str | bytes | Iterable[_Operation], in_place: bool = False, pointer_cls: type[JsonPointer] = ..., + atomic: bool = False, ) -> Any: ... def make_patch( src: Any, @@ -118,7 +119,7 @@ class JsonPatch: generate_test_ops: bool = False, ) -> Self: ... def to_string(self, dumps: Callable[[Any], str] | None = None) -> str: ... - def apply(self, obj: Any, in_place: bool = False) -> Any: ... + def apply(self, obj: Any, in_place: bool = False, atomic: bool = False) -> Any: ... # Computes the operations for JsonPatch.from_diff; mostly an implementation # detail, so its internal methods are typed loosely. diff --git a/jsonpatch.py b/jsonpatch.py index 3107c82..f08cb42 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -113,7 +113,8 @@ def multidict(ordered_pairs): _jsonloads = functools.partial(json.loads, object_pairs_hook=multidict) -def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): +def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer, + atomic=False): """Apply patch operations to the specified JSON document. :param doc: JSON document, including arrays and scalar values. @@ -129,6 +130,12 @@ def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): :param pointer_cls: JSON pointer class to use. :type pointer_cls: Type[JsonPointer] + :param atomic: With ``in_place=True``, undo the changes of all preceding + operations if an operation fails, so that ``doc`` is + either fully patched or left unchanged. See + :meth:`JsonPatch.apply`. + :type atomic: bool + :return: Resulting document, which may have a different type from ``doc``. :rtype: object @@ -150,7 +157,7 @@ def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): patch = JsonPatch.from_string(patch, pointer_cls=pointer_cls) else: patch = JsonPatch(patch, pointer_cls=pointer_cls) - return patch.apply(doc, in_place) + return patch.apply(doc, in_place, atomic=atomic) def make_patch(src, dst, pointer_cls=JsonPointer, generate_test_ops=False): @@ -186,6 +193,111 @@ def make_patch(src, dst, pointer_cls=JsonPointer, generate_test_ops=False): generate_test_ops=generate_test_ops) +class _NullJournal(object): + """Performs the changes of patch operations without recording them.""" + + @staticmethod + def setitem(container, key, value): + container[key] = value + + @staticmethod + def delitem(container, key): + del container[key] + + @staticmethod + def insert(container, index, value): + container.insert(index, value) + + @staticmethod + def append(container, value): + container.append(value) + + +_NO_JOURNAL = _NullJournal() + + +class _Journal(object): + """Performs the changes of patch operations and records how to undo them. + + Each undo step refers to the changed container itself rather than to a + path. As :meth:`rollback` undoes the steps in reverse order, every + container is in the same state as right after its change when the step is + undone, so list indices still refer to the same items. Removed and + replaced values are kept as they are; they are detached from the + document, so restoring them needs no copy. + """ + + def __init__(self): + self._undo = [] + + def setitem(self, container, key, value): + if isinstance(container, MutableMapping) and key not in container: + container[key] = value + self._undo.append((_undo_delitem, container, key)) + else: + old = container[key] + container[key] = value + self._undo.append((_undo_setitem, container, key, old)) + + def delitem(self, container, key): + old = container[key] + if isinstance(container, MutableSequence): + del container[key] + self._undo.append((_undo_insert, container, key, old)) + elif isinstance(container, MutableMapping): + # remember the position, to restore the order of the members + index = _key_index(container, key) + del container[key] + self._undo.append((_undo_reinsert, container, index, key, old)) + else: + del container[key] + self._undo.append((_undo_setitem, container, key, old)) + + def insert(self, container, index, value): + container.insert(index, value) + self._undo.append((_undo_delitem, container, index)) + + def append(self, container, value): + container.append(value) + # the list may shrink again before this step is undone, but by then + # it has the same length as right after the append + self._undo.append((_undo_delitem, container, len(container) - 1)) + + def rollback(self): + """Undoes all recorded changes, the most recent one first.""" + while self._undo: + step = self._undo.pop() + step[0](*step[1:]) + + +def _undo_setitem(container, key, value): + container[key] = value + + +def _undo_delitem(container, key): + del container[key] + + +def _undo_insert(container, index, value): + container.insert(index, value) + + +def _key_index(mapping, key): + for index, k in enumerate(mapping): + if k == key: + return index + return len(mapping) + + +def _undo_reinsert(mapping, index, key, value): + """Inserts a member into a mapping at its former position, by + re-adding the members that came after it.""" + following = [(k, mapping.pop(k)) for k in list(mapping)[index:]] + mapping[key] = value + for k, v in following: + mapping[k] = v + + class PatchOperation(object): """A single operation inside a JSON Patch.""" @@ -225,7 +337,15 @@ def _from_pointer(self): raise InvalidJsonPatch("Invalid 'from'", op=self) def apply(self, obj): - """Abstract method that applies a patch operation to the specified object.""" + """Applies the patch operation to the specified object.""" + return self._apply(obj, _NO_JOURNAL) + + def _apply(self, obj, journal): + """Applies the operation, making all changes through ``journal``, so + that :meth:`JsonPatch.apply` can undo them with ``atomic=True``. + + Subclasses either implement this method or override :meth:`apply`. + """ raise NotImplementedError('should implement the patch operation.') def __hash__(self): @@ -269,7 +389,7 @@ def set_part(self, index, value): class RemoveOperation(PatchOperation): """Removes an object property or an array element.""" - def apply(self, obj): + def _apply(self, obj, journal): subobj, part = self.pointer.to_last(obj) if part is None: @@ -279,7 +399,7 @@ def apply(self, obj): raise JsonPointerException("invalid array index '{0}'".format(part)) try: - del subobj[part] + journal.delitem(subobj, part) except (KeyError, IndexError): msg = "can't remove a non-existent object '{0}'".format(part) raise JsonPatchConflict(msg, op=self) @@ -292,7 +412,7 @@ def apply(self, obj): class AddOperation(PatchOperation): """Adds an object property or an array element.""" - def apply(self, obj): + def _apply(self, obj, journal): try: value = self.operation["value"] except KeyError: @@ -301,9 +421,9 @@ def apply(self, obj): # Insert a copy so the document does not share mutable values with # the patch; otherwise later operations would modify the patch itself. - return self._add(obj, copy.deepcopy(value)) + return self._add(obj, copy.deepcopy(value), journal) - def _add(self, obj, value): + def _add(self, obj, value, journal=_NO_JOURNAL): subobj, part = self.pointer.to_last(obj) if part is None: @@ -311,16 +431,16 @@ def _add(self, obj, value): if isinstance(subobj, MutableSequence): if part == '-': - subobj.append(value) # pylint: disable=E1103 + journal.append(subobj, value) elif part > len(subobj) or part < 0: raise JsonPatchConflict("can't insert outside of list", op=self) else: - subobj.insert(part, value) # pylint: disable=E1103 + journal.insert(subobj, part, value) elif isinstance(subobj, MutableMapping): - subobj[part] = value + journal.setitem(subobj, part, value) else: raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part), op=self) @@ -330,7 +450,7 @@ def _add(self, obj, value): class ReplaceOperation(PatchOperation): """Replaces an object property or an array element by a new value.""" - def apply(self, obj): + def _apply(self, obj, journal): try: value = self.operation["value"] except KeyError: @@ -361,14 +481,14 @@ def apply(self, obj): else: raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part), op=self) - subobj[part] = value + journal.setitem(subobj, part, value) return obj class MoveOperation(PatchOperation): """Moves an object property or an array element to a new location.""" - def apply(self, obj): + def _apply(self, obj, journal): from_ptr = self._from_pointer() # Checked before anything is resolved, as removing an array element @@ -396,14 +516,14 @@ def apply(self, obj): obj = RemoveOperation({ 'op': 'remove', 'path': self.operation['from'] - }, pointer_cls=self.pointer_cls).apply(obj) + }, pointer_cls=self.pointer_cls)._apply(obj, journal) # the value has been detached from its old location, so no copy # needed obj = AddOperation({ 'op': 'add', 'path': self.location, - }, pointer_cls=self.pointer_cls)._add(obj, value) + }, pointer_cls=self.pointer_cls)._add(obj, value, journal) except JsonPatchException as ex: # report the failure against this operation rather than the # internal, synthetic remove / add operations @@ -441,7 +561,7 @@ def set_from_part(self, index, value): class TestOperation(PatchOperation): """Test value by specified location.""" - def apply(self, obj): + def _apply(self, obj, journal): try: subobj, part = self.pointer.to_last(obj) if part is None: @@ -468,7 +588,7 @@ def apply(self, obj): class CopyOperation(PatchOperation): """ Copies an object property or an array element to a new location """ - def apply(self, obj): + def _apply(self, obj, journal): from_ptr = self._from_pointer() subobj, part = _to_existing_last(from_ptr, obj) @@ -482,7 +602,7 @@ def apply(self, obj): obj = AddOperation({ 'op': 'add', 'path': self.location, - }, pointer_cls=self.pointer_cls)._add(obj, value) + }, pointer_cls=self.pointer_cls)._add(obj, value, journal) except JsonPatchException as ex: # report the failure against this operation rather than the # internal, synthetic add operation @@ -492,6 +612,27 @@ def apply(self, obj): return obj +_JOURNALED_OPERATIONS = ( + RemoveOperation, AddOperation, ReplaceOperation, MoveOperation, + TestOperation, CopyOperation, +) + + +def _supports_journal(operation): + """Whether all changes of ``operation`` go through its journal, which is + the case for the built-in operations unless a subclass changes how they + are applied.""" + cls = type(operation) + if getattr(cls, 'apply', None) is not PatchOperation.apply: + return False + if not any(getattr(cls, '_apply', None) is base._apply + for base in _JOURNALED_OPERATIONS): + return False + if isinstance(operation, AddOperation) and cls._add is not AddOperation._add: + return False + return True + + class JsonPatch(object): """A JSON Patch is an iterable of Patch Operations. @@ -687,7 +828,7 @@ def to_string(self, dumps=None): def _ops(self): return tuple(map(self._get_operation, self.patch)) - def apply(self, obj, in_place=False): + def apply(self, obj, in_place=False, atomic=False): """Applies the patch to a given object. :param obj: JSON document, including arrays and scalar values. @@ -697,6 +838,15 @@ def apply(self, obj, in_place=False): specified `obj` or to its copy. :type in_place: bool + :param atomic: With ``in_place=True``, undo the changes of all + preceding operations if an operation fails, so that + `obj` is either fully patched or left unchanged. This + does not copy the document; it only keeps the values + that the patch removes or replaces until it is + applied. Without ``in_place=True``, `obj` is never + changed, so this has no effect. + :type atomic: bool + :return: Resulting document. Use this return value even with ``in_place=True``, because a root replacement returns a new value rather than modifying the original container. @@ -704,9 +854,34 @@ def apply(self, obj, in_place=False): if not in_place: obj = copy.deepcopy(obj) + atomic = False + + ops = self._ops - for operation in self._ops: - obj = operation.apply(obj) + if not atomic: + for operation in ops: + obj = operation.apply(obj) + return obj + + if not all(map(_supports_journal, ops)): + # The changes of custom operations can't be undone, so check + # that the patch applies to a copy before changing the document + dry_run = copy.deepcopy(obj) + for operation in ops: + dry_run = operation.apply(dry_run) + del dry_run + + for operation in ops: + obj = operation.apply(obj) + return obj + + journal = _Journal() + try: + for operation in ops: + obj = operation._apply(obj, journal) + except BaseException: + journal.rollback() + raise return obj diff --git a/property_tests.py b/property_tests.py index c866385..2406bae 100644 --- a/property_tests.py +++ b/property_tests.py @@ -408,6 +408,18 @@ def test_in_place_gives_same_result(self, case): outcome(jsonpatch.apply_patch, copy.deepcopy(doc), copy.deepcopy(patch), in_place=True)) + @given(docs_with_patches()) + def test_atomic_in_place_gives_same_result(self, case): + doc, patch = case + saved = copy.deepcopy(doc) + expected = outcome(jsonpatch.apply_patch, doc, copy.deepcopy(patch)) + res = outcome(jsonpatch.apply_patch, doc, copy.deepcopy(patch), + in_place=True, atomic=True) + assert_same_outcome(expected, res) + if res[0] == 'error': + # unchanged, including the order of object members + assert json.dumps(doc) == json.dumps(saved), (doc, saved) + @given(docs_with_patches()) def test_patch_as_string_gives_same_result(self, case): doc, patch = case diff --git a/tests.py b/tests.py index 8a74962..1ae1007 100755 --- a/tests.py +++ b/tests.py @@ -2168,6 +2168,218 @@ class JsonPatch(jsonpatch.JsonPatch): self.assertEqual(res, {}) +class AtomicApplyTests(unittest.TestCase): + + def assertUnchanged(self, doc, saved): + # also compares the order of object members + self.assertEqual(json.dumps(doc), json.dumps(saved)) + + def assertRollsBack(self, doc, patch, exc=jsonpatch.JsonPatchException): + saved = copy.deepcopy(doc) + with self.assertRaises(exc): + jsonpatch.apply_patch(doc, patch, in_place=True, atomic=True) + self.assertUnchanged(doc, saved) + + def test_issue_example(self): + patch = [ + {'op': 'add', 'path': '/attrs/language', 'value': 'de-DE'}, + {'op': 'add', 'path': '/attrs/country', 'value': 'US'}, + {'op': 'remove', 'path': '/fish'}, + {'op': 'remove', 'path': '/attrs/copyright'}, + ] + doc = {'attrs': {'copyright': 'me'}} + attrs = doc['attrs'] + self.assertRollsBack(doc, patch, jsonpatch.JsonPatchConflict) + self.assertIs(doc['attrs'], attrs) + + def test_not_atomic_by_default(self): + doc = {'a': 1} + patch = [{'op': 'replace', 'path': '/a', 'value': 2}, + {'op': 'test', 'path': '/a', 'value': 3}] + with self.assertRaises(jsonpatch.JsonPatchTestFailed): + jsonpatch.apply_patch(doc, patch, in_place=True) + self.assertEqual(doc, {'a': 2}) + + def test_failed_operations_are_rolled_back(self): + fail = {'op': 'test', 'path': '/missing', 'value': 1} + cases = [ + # object members + ({'a': 1, 'b': 2}, [{'op': 'add', 'path': '/c', 'value': 3}]), + ({'a': 1, 'b': 2}, [{'op': 'add', 'path': '/a', 'value': 3}]), + ({'a': 1, 'b': 2, 'c': 3}, [{'op': 'remove', 'path': '/a'}]), + ({'a': 1, 'b': 2, 'c': 3}, [{'op': 'remove', 'path': '/b'}, + {'op': 'remove', 'path': '/a'}, + {'op': 'add', 'path': '/b', + 'value': 0}]), + ({'a': 1, 'b': 2}, [{'op': 'replace', 'path': '/a', 'value': 3}]), + # array elements + ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/-', 'value': 4}]), + ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/0', 'value': 4}]), + ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/3', 'value': 4}]), + ({'l': [1, 2, 3]}, [{'op': 'remove', 'path': '/l/1'}]), + ({'l': [1, 2, 3]}, [{'op': 'replace', 'path': '/l/2', 'value': 4}]), + ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/-', 'value': 4}, + {'op': 'remove', 'path': '/l/0'}, + {'op': 'remove', 'path': '/l/2'}, + {'op': 'add', 'path': '/l/-', 'value': 5}]), + # move and copy + ({'a': {'x': 1}, 'b': {}}, + [{'op': 'move', 'from': '/a/x', 'path': '/b/y'}]), + ({'a': 1, 'b': 2}, [{'op': 'move', 'from': '/a', 'path': '/b'}]), + ({'l': [1, 2, 3, 4]}, + [{'op': 'move', 'from': '/l/0', 'path': '/l/3'}]), + ({'l': [1, 2, 3, 4]}, + [{'op': 'move', 'from': '/l/3', 'path': '/l/-'}]), + ({'a': [1], 'b': {}}, + [{'op': 'copy', 'from': '/a', 'path': '/b/c'}, + {'op': 'add', 'path': '/b/c/-', 'value': 2}]), + # nested changes + ({'a': {'b': [{'c': 1}]}}, + [{'op': 'add', 'path': '/a/b/0/d', 'value': {'e': []}}, + {'op': 'add', 'path': '/a/b/0/d/e/-', 'value': 1}, + {'op': 'replace', 'path': '/a/b', 'value': None}, + {'op': 'add', 'path': '/a/c', 'value': 1}]), + ] + for doc, patch in cases: + with self.subTest(doc=doc, patch=patch): + self.assertRollsBack(doc, patch + [fail]) + + def test_restores_original_containers(self): + inner = {'x': [1, 2]} + doc = {'a': inner, 'b': 1} + patch = [{'op': 'remove', 'path': '/a'}, + {'op': 'add', 'path': '/c', 'value': 1}, + {'op': 'remove', 'path': '/missing'}] + self.assertRollsBack(doc, patch) + self.assertIs(doc['a'], inner) + + def test_rolls_back_json_pointer_errors(self): + doc = {'a': 1, 'l': [1]} + patch = [{'op': 'add', 'path': '/b', 'value': 1}, + {'op': 'remove', 'path': '/l/x'}] + self.assertRollsBack(doc, patch, jsonpointer.JsonPointerException) + + def test_rolls_back_unexpected_errors(self): + doc = {'a': {'b': 1}} + patch = [{'op': 'add', 'path': '/c', 'value': 1}, + {'op': 'remove', 'path': '/a/b'}] + original = jsonpatch.RemoveOperation._apply + + def broken(self, obj, journal): + raise RuntimeError('boom') + + with mock.patch.object(jsonpatch.RemoveOperation, '_apply', broken): + self.assertRollsBack(doc, patch, RuntimeError) + self.assertIs(jsonpatch.RemoveOperation._apply, original) + + def test_replaced_root(self): + doc = {'a': 1} + patch = [{'op': 'remove', 'path': '/a'}, + {'op': 'replace', 'path': '', 'value': {'b': []}}, + {'op': 'add', 'path': '/b/-', 'value': 1}, + {'op': 'remove', 'path': '/missing'}] + self.assertRollsBack(doc, patch) + + def test_success(self): + doc = {'a': 1, 'l': [1, 2]} + patch = [{'op': 'add', 'path': '/b', 'value': 2}, + {'op': 'remove', 'path': '/a'}, + {'op': 'move', 'from': '/l/0', 'path': '/l/-'}, + {'op': 'test', 'path': '/l', 'value': [2, 1]}] + expected = jsonpatch.apply_patch(doc, patch) + res = jsonpatch.apply_patch(doc, patch, in_place=True, atomic=True) + self.assertIs(res, doc) + self.assertEqual(res, expected) + + def test_success_replacing_root(self): + doc = {'a': 1} + patch = [{'op': 'replace', 'path': '', 'value': [1]}] + res = jsonpatch.JsonPatch(patch).apply(doc, in_place=True, atomic=True) + self.assertEqual(res, [1]) + self.assertEqual(doc, {'a': 1}) + + def test_without_in_place(self): + doc = {'a': 1} + patch = jsonpatch.JsonPatch([{'op': 'remove', 'path': '/a'}]) + self.assertEqual(patch.apply(doc, atomic=True), {}) + self.assertEqual(doc, {'a': 1}) + with self.assertRaises(jsonpatch.JsonPatchConflict): + patch.apply({}, atomic=True) + + def test_patch_can_be_reapplied(self): + patch = jsonpatch.JsonPatch([{'op': 'add', 'path': '/a/-', 'value': 1}, + {'op': 'test', 'path': '/b', 'value': 1}]) + doc = {'a': [], 'b': 2} + for _ in range(2): + with self.assertRaises(jsonpatch.JsonPatchTestFailed): + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual(doc, {'a': [], 'b': 2}) + doc['b'] = 1 + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual(doc, {'a': [1], 'b': 1}) + + def test_custom_operations(self): + # The changes of custom operations can't be undone; the patch is + # checked against a copy first + + class AppendOperation(jsonpatch.PatchOperation): + def apply(self, obj): + self.pointer.resolve(obj).append(self.operation['value']) + return obj + + class LoggingAdd(jsonpatch.AddOperation): + def _add(self, obj, value, journal=None): + subobj, part = self.pointer.to_last(obj) + subobj[part] = value + return obj + + class JsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, + append=AppendOperation, add=LoggingAdd)) + + for op in ({'op': 'append', 'path': '/l', 'value': 1}, + {'op': 'add', 'path': '/b', 'value': 1}): + with self.subTest(op=op): + doc = {'l': [], 'a': 1} + patch = JsonPatch([{'op': 'remove', 'path': '/a'}, op, + {'op': 'test', 'path': '/c', 'value': 1}]) + with self.assertRaises(jsonpatch.JsonPatchTestFailed): + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual(doc, {'l': [], 'a': 1}) + + patch = JsonPatch([{'op': 'remove', 'path': '/a'}, op]) + expected = patch.apply(doc) + l = doc['l'] + res = patch.apply(doc, in_place=True, atomic=True) + self.assertIs(res, doc) + self.assertIs(res['l'], l) + self.assertEqual(res, expected) + + def test_supports_journal(self): + patch = jsonpatch.JsonPatch([ + {'op': op, 'path': '/a', 'from': '/b', 'value': 1} + for op in jsonpatch.JsonPatch.operations]) + self.assertTrue(all(map(jsonpatch._supports_journal, patch._ops))) + + class OwnApply(jsonpatch.ReplaceOperation): + def apply(self, obj): + return obj + + class OwnInternalApply(jsonpatch.ReplaceOperation): + def _apply(self, obj, journal): + return obj + + class NotAnOperation(object): + def __init__(self, operation, pointer_cls=None): + pass + + for cls in (OwnApply, OwnInternalApply, NotAnOperation): + with self.subTest(cls=cls): + op = cls({'op': 'replace', 'path': '/a', 'value': 1}) + self.assertFalse(jsonpatch._supports_journal(op)) + + class ScriptRunner: """ Runs the command line programs of this checkout """ @@ -2649,6 +2861,7 @@ def get_suite(): suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CustomJsonPointerTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CustomOperationTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ExceptionOperationTests)) + suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(AtomicApplyTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CommandLineTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(DocumentationTests)) return suite diff --git a/typetests/usage.py b/typetests/usage.py index 4b02bb9..fb9e6c5 100644 --- a/typetests/usage.py +++ b/typetests/usage.py @@ -26,6 +26,7 @@ # module-level functions assert_type(jsonpatch.apply_patch(doc, ops), Any) assert_type(jsonpatch.apply_patch(doc, ops, in_place=True), Any) +assert_type(jsonpatch.apply_patch(doc, ops, in_place=True, atomic=True), Any) assert_type(jsonpatch.apply_patch(doc, json.dumps(ops)), Any) assert_type(jsonpatch.apply_patch(doc, json.dumps(ops).encode()), Any) assert_type(jsonpatch.make_patch(doc, {}), JsonPatch) @@ -39,6 +40,7 @@ patch = JsonPatch(ops) JsonPatch(op for op in ops) assert_type(patch.apply(doc), Any) +assert_type(patch.apply(doc, in_place=True, atomic=True), Any) assert_type(patch.to_string(), str) assert_type(patch.to_string(dumps=json.dumps), str) assert_type(bool(patch), bool) From ba61ffb807b474915797bcb5756b4e14e83105a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:25:20 +0000 Subject: [PATCH 02/14] Cover the remaining lines of atomic rollback Test rolling back a removal from a duck-typed mapping, and drop the unreachable fallback of _key_index: the key was just read from the mapping, so iterating over it finds the key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 5 +---- tests.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index f08cb42..796b611 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -283,10 +283,7 @@ def _undo_insert(container, index, value): def _key_index(mapping, key): - for index, k in enumerate(mapping): - if k == key: - return index - return len(mapping) + return next(index for index, k in enumerate(mapping) if k == key) def _undo_reinsert(mapping, index, key, value): diff --git a/tests.py b/tests.py index 1ae1007..6dec945 100755 --- a/tests.py +++ b/tests.py @@ -2253,6 +2253,29 @@ def test_restores_original_containers(self): self.assertRollsBack(doc, patch) self.assertIs(doc['a'], inner) + def test_duck_typed_mapping(self): + class IndexedObject: + def __init__(self): + self.data = {'foo': 1} + + def __getitem__(self, key): + return self.data[key] + + def __setitem__(self, key, value): + self.data[key] = value + + def __delitem__(self, key): + del self.data[key] + + obj = IndexedObject() + doc = {'obj': obj} + patch = [{'op': 'remove', 'path': '/obj/foo'}, + {'op': 'remove', 'path': '/missing'}] + with self.assertRaises(jsonpatch.JsonPatchConflict): + jsonpatch.apply_patch(doc, patch, in_place=True, atomic=True) + self.assertIs(doc['obj'], obj) + self.assertEqual(obj.data, {'foo': 1}) + def test_rolls_back_json_pointer_errors(self): doc = {'a': 1, 'l': [1]} patch = [{'op': 'add', 'path': '/b', 'value': 1}, From 743101658593dc150bc78785f2d2f6ec0a888ab2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:30:19 +0000 Subject: [PATCH 03/14] Fix issues in atomic patch application found in review - Call AddOperation._add without the journal when not applying atomically, so that subclasses overriding _add(obj, value) keep working. - Don't read a missing mapping member before removing it, as that creates it in a defaultdict, and the removal then succeeded. - Patches with custom operations were tried on a copy first, but the application to the document itself was not undone if it failed there. Restore the document's contents from the copy instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- doc/tutorial.rst | 7 +++--- jsonpatch.py | 43 +++++++++++++++++++++++--------- tests.py | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 15 deletions(-) diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 55a30fa..c6ec88f 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -170,9 +170,10 @@ raised, so the document is either fully patched or left unchanged: This only keeps the values that the patch removes or replaces until it has been applied, so it needs much less memory than ``in_place=False`` for large documents. The original containers are restored, so references into the -document stay valid. Custom operations (see :class:`jsonpatch.JsonPatch`'s -``operations``) whose changes can't be undone are first tried on a copy of -the document instead. +document stay valid. If a patch contains custom operations (see +:class:`jsonpatch.JsonPatch`'s ``operations``), whose changes can't be undone, +the document is copied instead, and its contents are restored from the copy +if an operation fails. Replacing the Whole Document diff --git a/jsonpatch.py b/jsonpatch.py index 796b611..c995d9e 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -240,6 +240,9 @@ def setitem(self, container, key, value): self._undo.append((_undo_setitem, container, key, old)) def delitem(self, container, key): + # reading a missing member can create it, e.g. in a defaultdict + if isinstance(container, Mapping) and key not in container: + raise KeyError(key) old = container[key] if isinstance(container, MutableSequence): del container[key] @@ -418,7 +421,11 @@ def _apply(self, obj, journal): # Insert a copy so the document does not share mutable values with # the patch; otherwise later operations would modify the patch itself. - return self._add(obj, copy.deepcopy(value), journal) + value = copy.deepcopy(value) + if journal is _NO_JOURNAL: + # subclasses may override _add without the journal parameter + return self._add(obj, value) + return self._add(obj, value, journal) def _add(self, obj, value, journal=_NO_JOURNAL): subobj, part = self.pointer.to_last(obj) @@ -609,6 +616,15 @@ def _apply(self, obj, journal): return obj +def _restore(obj, saved): + """Restores the contents of a document from a copy of it.""" + if isinstance(obj, MutableMapping): + obj.clear() + obj.update(saved) + elif isinstance(obj, MutableSequence): + obj[:] = saved + + _JOURNALED_OPERATIONS = ( RemoveOperation, AddOperation, ReplaceOperation, MoveOperation, TestOperation, CopyOperation, @@ -840,8 +856,10 @@ def apply(self, obj, in_place=False, atomic=False): `obj` is either fully patched or left unchanged. This does not copy the document; it only keeps the values that the patch removes or replaces until it is - applied. Without ``in_place=True``, `obj` is never - changed, so this has no effect. + applied. Patches with custom operations, whose + changes can't be undone, copy the document instead. + Without ``in_place=True``, `obj` is never changed, so + this has no effect. :type atomic: bool :return: Resulting document. Use this return value even with @@ -861,15 +879,16 @@ def apply(self, obj, in_place=False, atomic=False): return obj if not all(map(_supports_journal, ops)): - # The changes of custom operations can't be undone, so check - # that the patch applies to a copy before changing the document - dry_run = copy.deepcopy(obj) - for operation in ops: - dry_run = operation.apply(dry_run) - del dry_run - - for operation in ops: - obj = operation.apply(obj) + # The changes of custom operations can't be undone, so restore + # the document from a copy instead + saved = copy.deepcopy(obj) + original = obj + try: + for operation in ops: + obj = operation.apply(obj) + except BaseException: + _restore(original, saved) + raise return obj journal = _Journal() diff --git a/tests.py b/tests.py index 6dec945..3bcd9da 100755 --- a/tests.py +++ b/tests.py @@ -1,6 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- +import collections import contextlib import copy import io @@ -2379,6 +2380,70 @@ class JsonPatch(jsonpatch.JsonPatch): self.assertIs(res['l'], l) self.assertEqual(res, expected) + def test_custom_operation_failing_on_original(self): + # the document is restored even if a custom operation fails only on + # the original document, not on a copy of it + originals = [] + + class FailsOnOriginal(jsonpatch.PatchOperation): + def apply(self, obj): + if any(obj is original for original in originals): + raise RuntimeError('original document') + return obj + + class JsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, fail=FailsOnOriginal)) + + cases = [ + ({'a': 1, 'b': [1]}, [{'op': 'remove', 'path': '/a'}, + {'op': 'add', 'path': '/b/-', 'value': 2}]), + ([1, {'a': 2}], [{'op': 'add', 'path': '/-', 'value': 0}, + {'op': 'remove', 'path': '/1/a'}]), + ] + for doc, ops in cases: + with self.subTest(doc=doc): + originals[:] = [doc] + saved = copy.deepcopy(doc) + patch = JsonPatch(ops + [{'op': 'fail', 'path': ''}]) + with self.assertRaises(RuntimeError): + patch.apply(doc, in_place=True, atomic=True) + self.assertUnchanged(doc, saved) + + # nothing can be changed in place in a scalar document + originals[:] = ['scalar'] + patch = JsonPatch([{'op': 'fail', 'path': ''}]) + with self.assertRaises(RuntimeError): + patch.apply('scalar', in_place=True, atomic=True) + + def test_add_subclass_without_journal_parameter(self): + # _add was called without a journal before atomic application + class OldAdd(jsonpatch.AddOperation): + def _add(self, obj, value): + return super()._add(obj, value) + + class JsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, add=OldAdd)) + + patch = JsonPatch([{'op': 'add', 'path': '/a', 'value': 1}]) + self.assertEqual(patch.apply({}), {'a': 1}) + doc = {} + self.assertEqual(patch.apply(doc, in_place=True, atomic=True), + {'a': 1}) + self.assertEqual(doc, {'a': 1}) + + def test_remove_missing_member_of_defaultdict(self): + # reading a missing member of a defaultdict creates it + for atomic in (False, True): + with self.subTest(atomic=atomic): + doc = collections.defaultdict(int, a=1) + with self.assertRaises(jsonpatch.JsonPatchConflict): + jsonpatch.apply_patch( + doc, [{'op': 'remove', 'path': '/missing'}], + in_place=True, atomic=atomic) + self.assertEqual(doc, {'a': 1}) + def test_supports_journal(self): patch = jsonpatch.JsonPatch([ {'op': op, 'path': '/a', 'from': '/b', 'value': 1} From 4fffd78a4e079e2113dcfdde02c0186496888d24 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 10:44:17 +0000 Subject: [PATCH 04/14] Restore mutable children of immutable containers after a failed patch When a patch with custom operations fails with atomic=True, the document is restored from a copy. Only mutable mappings and sequences were restored, so changes to mutable members of an immutable root, such as a dict inside a tuple, remained. Walk immutable containers and restore their mutable descendants. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 7 +++++++ tests.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/jsonpatch.py b/jsonpatch.py index c995d9e..0678779 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -623,6 +623,13 @@ def _restore(obj, saved): obj.update(saved) elif isinstance(obj, MutableSequence): obj[:] = saved + # Immutable containers can't have changed, but their children can + elif isinstance(obj, Mapping): + for key in saved: + _restore(obj[key], saved[key]) + elif isinstance(obj, Sequence) and not isinstance(obj, (str, bytes)): + for item, saved_item in zip(obj, saved): + _restore(item, saved_item) _JOURNALED_OPERATIONS = ( diff --git a/tests.py b/tests.py index 3bcd9da..3085a44 100755 --- a/tests.py +++ b/tests.py @@ -19,6 +19,7 @@ import stat import subprocess import tempfile +from collections.abc import Mapping from types import MappingProxyType from unittest import mock @@ -2410,6 +2411,41 @@ class JsonPatch(jsonpatch.JsonPatch): patch.apply(doc, in_place=True, atomic=True) self.assertUnchanged(doc, saved) + # immutable containers can't change, but their children can + class ChangesChild(jsonpatch.PatchOperation): + def apply(self, obj): + self.pointer.resolve(obj)['a'] = 2 + raise RuntimeError('after a change') + + class ChildPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, change=ChangesChild)) + + class FrozenMapping(Mapping): + def __init__(self, data): + self._data = data + + def __getitem__(self, key): + return self._data[key] + + def __iter__(self): + return iter(self._data) + + def __len__(self): + return len(self._data) + + for doc, path in [(({'a': 1}, 'b'), '/0'), + (((({'a': 1},),),), '/0/0/0'), + (FrozenMapping({'x': {'a': 1}, 'y': 'b'}), '/x')]: + with self.subTest(doc=doc): + saved = copy.deepcopy(dict(doc) if isinstance(doc, Mapping) + else doc) + patch = ChildPatch([{'op': 'change', 'path': path}]) + with self.assertRaises(RuntimeError): + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual( + dict(doc) if isinstance(doc, Mapping) else doc, saved) + # nothing can be changed in place in a scalar document originals[:] = ['scalar'] patch = JsonPatch([{'op': 'fail', 'path': ''}]) From ffc6ff319cda9be84bc0d843fbb9fbe7c69c582f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 13:40:01 +0000 Subject: [PATCH 05/14] Restore sequences without slice assignment after a failed patch deque is a MutableSequence but doesn't support obj[:] = saved, so restoring a deque root raised TypeError, which hid the original exception and left the deque changed. Use clear() and extend() instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 4 +++- tests.py | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/jsonpatch.py b/jsonpatch.py index 6691a8e..9546819 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -640,7 +640,9 @@ def _restore(obj, saved): obj.clear() obj.update(saved) elif isinstance(obj, MutableSequence): - obj[:] = saved + # not obj[:] = saved, as e.g. deque doesn't support slices + obj.clear() + obj.extend(saved) # Immutable containers can't have changed, but their children can elif isinstance(obj, Mapping): for key in saved: diff --git a/tests.py b/tests.py index 58c8754..4c1fb7f 100755 --- a/tests.py +++ b/tests.py @@ -2411,6 +2411,15 @@ class JsonPatch(jsonpatch.JsonPatch): patch.apply(doc, in_place=True, atomic=True) self.assertUnchanged(doc, saved) + # sequences that don't support slice assignment + originals[:] = [collections.deque([1, {'a': 2}])] + doc = originals[0] + patch = JsonPatch([{'op': 'add', 'path': '/-', 'value': 0}, + {'op': 'fail', 'path': ''}]) + with self.assertRaises(RuntimeError): + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual(doc, collections.deque([1, {'a': 2}])) + # immutable containers can't change, but their children can class ChangesChild(jsonpatch.PatchOperation): def apply(self, obj): From 8ae8019e30c9bdc4291288cac1f7689cd4471590 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 21:54:08 +0000 Subject: [PATCH 06/14] Document that atomic rollback doesn't undo insertions made by reads Containers such as defaultdict add members when they are only read, e.g. while a pointer is resolved, a value is tested or the 'from' of a move or copy is read. These additions are not journaled, so they remain after a rollback. Document this rather than resolving pointers without the pointer class, as documents parsed from JSON are not affected. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- doc/tutorial.rst | 7 +++++++ jsonpatch.py | 2 ++ 2 files changed, 9 insertions(+) diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 8d803bb..31328ea 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -177,6 +177,13 @@ document stay valid. If a patch contains custom operations (see the document is copied instead, and its contents are restored from the copy if an operation fails. +Only the changes that the operations make are undone. Containers that change +when they are merely read, such as a ``collections.defaultdict``, which adds +a missing key on lookup, are not supported: looking up a location, testing a +value or reading the ``from`` of a ``move`` or ``copy`` can add members to +them that remain after a rollback. Documents parsed from JSON consist of +plain ``dict`` and ``list`` objects and are not affected. + Ignoring Missing Locations ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/jsonpatch.py b/jsonpatch.py index 9546819..cf453ec 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -885,6 +885,8 @@ def apply(self, obj, in_place=False, atomic=False): that the patch removes or replaces until it is applied. Patches with custom operations, whose changes can't be undone, copy the document instead. + Members that containers such as ``defaultdict`` add + when they are only read are not removed again. Without ``in_place=True``, `obj` is never changed, so this has no effect. :type atomic: bool From b2051349297b602523ee8c010c96761dc4b8d7b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 21:56:44 +0000 Subject: [PATCH 07/14] Avoid quadratic cost of removing many members with atomic=True To restore the order of an object's members, every removal looked up the position of the removed key, and undoing it re-added all members that followed. Removing many members from one large object was therefore quadratic: 20,000 removals from the end took 9 s instead of 0.05 s without atomic. Instead, record the keys of each mapping once, before its first removal, and restore that order once after all steps have been undone. Applying and rolling back are now linear (0.09 s for the case above). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index cf453ec..ac1ada0 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -243,10 +243,16 @@ class _Journal(object): undone, so list indices still refer to the same items. Removed and replaced values are kept as they are; they are detached from the document, so restoring them needs no copy. + + Restoring a removed member adds it at the end of its mapping, so the + order of the members of each mapping that loses one is recorded once, + and restored after all steps have been undone. """ def __init__(self): self._undo = [] + # id of a mapping -> (mapping, its keys before the first removal) + self._orders = {} def setitem(self, container, key, value): if isinstance(container, MutableMapping) and key not in container: @@ -266,10 +272,10 @@ def delitem(self, container, key): del container[key] self._undo.append((_undo_insert, container, key, old)) elif isinstance(container, MutableMapping): - # remember the position, to restore the order of the members - index = _key_index(container, key) + if id(container) not in self._orders: + self._orders[id(container)] = (container, list(container)) del container[key] - self._undo.append((_undo_reinsert, container, index, key, old)) + self._undo.append((_undo_setitem, container, key, old)) else: del container[key] self._undo.append((_undo_setitem, container, key, old)) @@ -289,6 +295,9 @@ def rollback(self): while self._undo: step = self._undo.pop() step[0](*step[1:]) + for mapping, keys in self._orders.values(): + _reorder(mapping, keys) + self._orders.clear() def _undo_setitem(container, key, value): @@ -303,17 +312,13 @@ def _undo_insert(container, index, value): container.insert(index, value) -def _key_index(mapping, key): - return next(index for index, k in enumerate(mapping) if k == key) - - -def _undo_reinsert(mapping, index, key, value): - """Inserts a member into a mapping at its former position, by - re-adding the members that came after it.""" - following = [(k, mapping.pop(k)) for k in list(mapping)[index:]] - mapping[key] = value - for k, v in following: - mapping[k] = v +def _reorder(mapping, keys): + """Restores the order of the members of a mapping, whose keys are + ``keys`` again after a rollback.""" + if list(mapping) != keys: + items = [(key, mapping[key]) for key in keys] + mapping.clear() + mapping.update(items) class PatchOperation(object): From 3fad0a5b9b44dac20834312632a1896727372bc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 22:05:34 +0000 Subject: [PATCH 08/14] Fix restoring the member order after adding before removing The keys of an object are recorded before its first removal, so they include members that the patch added before. The rollback removes these again, and restoring the recorded order then raised KeyError, which hid the original exception and left the members out of order. Leave out the keys that are no longer present. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 9 +++++++-- property_tests.py | 5 +++++ tests.py | 7 +++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index ac1ada0..1d5a2c9 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -313,8 +313,13 @@ def _undo_insert(container, index, value): def _reorder(mapping, keys): - """Restores the order of the members of a mapping, whose keys are - ``keys`` again after a rollback.""" + """Restores the order of the members of a mapping after a rollback. + + ``keys`` are its keys before its first removal. They can include + members that the patch added before, and that the rollback has removed + again; the original members appear in it in their original order. + """ + keys = [key for key in keys if key in mapping] if list(mapping) != keys: items = [(key, mapping[key]) for key in keys] mapping.clear() diff --git a/property_tests.py b/property_tests.py index 2406bae..699367b 100644 --- a/property_tests.py +++ b/property_tests.py @@ -409,6 +409,11 @@ def test_in_place_gives_same_result(self, case): copy.deepcopy(patch), in_place=True)) @given(docs_with_patches()) + # a member added before the first removal from the same object + @example(case=({'a': 1, 'b': 2}, + [{'op': 'add', 'path': '/c', 'value': 3}, + {'op': 'remove', 'path': '/a'}, + {'op': 'remove', 'path': '/missing'}])) def test_atomic_in_place_gives_same_result(self, case): doc, patch = case saved = copy.deepcopy(doc) diff --git a/tests.py b/tests.py index 4c1fb7f..1e47369 100755 --- a/tests.py +++ b/tests.py @@ -2214,6 +2214,13 @@ def test_failed_operations_are_rolled_back(self): {'op': 'add', 'path': '/b', 'value': 0}]), ({'a': 1, 'b': 2}, [{'op': 'replace', 'path': '/a', 'value': 3}]), + # members added before the first removal from the same object + ({'a': 1, 'b': 2}, [{'op': 'add', 'path': '/c', 'value': 3}, + {'op': 'remove', 'path': '/a'}]), + ({'a': 1, 'b': 2}, [{'op': 'add', 'path': '/c', 'value': 3}, + {'op': 'remove', 'path': '/c'}, + {'op': 'remove', 'path': '/a'}, + {'op': 'add', 'path': '/c', 'value': 4}]), # array elements ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/-', 'value': 4}]), ({'l': [1, 2, 3]}, [{'op': 'add', 'path': '/l/0', 'value': 4}]), From b100853fe8ba32559f65ca89cb428479f1d6888e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 22:22:47 +0000 Subject: [PATCH 09/14] Undo appends to full bounded deques correctly A deque with a maxlen drops its first item when appending to it while it is full. The undo step only deleted the appended item, so the dropped one was lost, and after further changes the recorded indices could be out of range, so that the rollback raised IndexError and hid the original exception. Record the dropped item and put it back at the front when undoing such an append. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 16 ++++++++++++++++ tests.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/jsonpatch.py b/jsonpatch.py index 1d5a2c9..b0d5281 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -285,6 +285,17 @@ def insert(self, container, index, value): self._undo.append((_undo_delitem, container, index)) def append(self, container, value): + if (isinstance(container, collections.deque) and + container.maxlen is not None and + len(container) == container.maxlen): + # A full bounded deque drops its first item to make room + if container.maxlen: + dropped = container[0] + container.append(value) + self._undo.append((_undo_dropping_append, container, dropped)) + else: + container.append(value) # it stays empty + return container.append(value) # the list may shrink again before this step is undone, but by then # it has the same length as right after the append @@ -312,6 +323,11 @@ def _undo_insert(container, index, value): container.insert(index, value) +def _undo_dropping_append(deque, dropped): + del deque[-1] + deque.appendleft(dropped) + + def _reorder(mapping, keys): """Restores the order of the members of a mapping after a rollback. diff --git a/tests.py b/tests.py index 1e47369..55a0630 100755 --- a/tests.py +++ b/tests.py @@ -2253,6 +2253,44 @@ def test_failed_operations_are_rolled_back(self): with self.subTest(doc=doc, patch=patch): self.assertRollsBack(doc, patch + [fail]) + def test_bounded_deque(self): + # appending to a full bounded deque drops its first item + fail = {'op': 'remove', 'path': '/missing'} + cases = [ + (3, [{'op': 'add', 'path': '/d/-', 'value': 4}]), + (3, [{'op': 'remove', 'path': '/d/0'}, + {'op': 'add', 'path': '/d/-', 'value': 4}, + {'op': 'add', 'path': '/d/-', 'value': 5}, + {'op': 'replace', 'path': '/d/0', 'value': 6}]), + (4, [{'op': 'add', 'path': '/d/-', 'value': 4}, + {'op': 'add', 'path': '/d/-', 'value': 5}, + {'op': 'move', 'from': '/d/0', 'path': '/d/-'}]), + (0, [{'op': 'add', 'path': '/d/-', 'value': 4}]), + ] + for maxlen, patch in cases: + with self.subTest(maxlen=maxlen, patch=patch): + items = [1, 2, 3][:maxlen] + + def make_doc(): + return {'d': collections.deque(items, maxlen=maxlen)} + + # the same result as without atomic + expected = jsonpatch.apply_patch(make_doc(), patch, + in_place=True) + doc = make_doc() + res = jsonpatch.apply_patch(doc, patch, in_place=True, + atomic=True) + self.assertEqual(res, expected) + + doc = make_doc() + deque = doc['d'] + with self.assertRaises(jsonpatch.JsonPatchPathNotFound): + jsonpatch.apply_patch(doc, patch + [fail], + in_place=True, atomic=True) + self.assertIs(doc['d'], deque) + self.assertEqual(deque, collections.deque(items)) + self.assertEqual(deque.maxlen, maxlen) + def test_restores_original_containers(self): inner = {'x': [1, 2]} doc = {'a': inner, 'b': 1} From e206c6023b8b89244c8a19fcef79c4f7238431c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 22:34:28 +0000 Subject: [PATCH 10/14] Restore member order only for dicts, and honour apply set on operations Restoring the order of a mapping's members with clear() and update() added the members that a ChainMap takes from its other mappings to its first one. Only restore the order of dicts (and dict subclasses), whose order is the order of insertion; other mappings keep the members that the rollback restores in the order they then have. Operations that set apply, _apply or _add on the instance were treated as journal-aware, so atomic=True bypassed the instance's apply. Use the copy-and-restore path for them, which calls it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 18 +++++++++++---- tests.py | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index b0d5281..ed35109 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -245,8 +245,12 @@ class _Journal(object): document, so restoring them needs no copy. Restoring a removed member adds it at the end of its mapping, so the - order of the members of each mapping that loses one is recorded once, - and restored after all steps have been undone. + order of the members of each dict that loses one is recorded once, and + restored after all steps have been undone. This is limited to dicts + (including subclasses such as OrderedDict), whose order is the order of + insertion and which clear() and update() restore exactly; other mappings, + such as a ChainMap, would take over members of the mappings they refer + to. """ def __init__(self): @@ -272,7 +276,8 @@ def delitem(self, container, key): del container[key] self._undo.append((_undo_insert, container, key, old)) elif isinstance(container, MutableMapping): - if id(container) not in self._orders: + if (isinstance(container, dict) and + id(container) not in self._orders): self._orders[id(container)] = (container, list(container)) del container[key] self._undo.append((_undo_setitem, container, key, old)) @@ -686,8 +691,11 @@ def _restore(obj, saved): def _supports_journal(operation): """Whether all changes of ``operation`` go through its journal, which is - the case for the built-in operations unless a subclass changes how they - are applied.""" + the case for the built-in operations unless a subclass, or the operation + object itself, changes how they are applied.""" + overridden = getattr(operation, '__dict__', {}) + if any(name in overridden for name in ('apply', '_apply', '_add')): + return False cls = type(operation) if getattr(cls, 'apply', None) is not PatchOperation.apply: return False diff --git a/tests.py b/tests.py index 55a0630..52d35bf 100755 --- a/tests.py +++ b/tests.py @@ -2291,6 +2291,70 @@ def make_doc(): self.assertEqual(deque, collections.deque(items)) self.assertEqual(deque.maxlen, maxlen) + def test_mapping_order(self): + # the order of dicts is restored, including dict subclasses + for cls in (dict, collections.OrderedDict, + lambda items: collections.defaultdict(int, items)): + with self.subTest(cls=cls): + doc = {'m': cls([('a', 1), ('b', 2), ('c', 3)])} + with self.assertRaises(jsonpatch.JsonPatchPathNotFound): + jsonpatch.apply_patch( + doc, [{'op': 'remove', 'path': '/m/a'}, + {'op': 'remove', 'path': '/m/b'}, + {'op': 'add', 'path': '/m/b', 'value': 4}, + {'op': 'remove', 'path': '/missing'}], + in_place=True, atomic=True) + self.assertEqual(list(doc['m'].items()), + [('a', 1), ('b', 2), ('c', 3)]) + + def test_chain_map(self): + # a ChainMap only changes its first mapping; restoring the order of + # its members would add the members of the others to it + parent = {'b': 2} + chain = collections.ChainMap({'a': 1, 'c': 3}, parent) + with self.assertRaises(jsonpatch.JsonPatchPathNotFound): + jsonpatch.apply_patch( + {'m': chain}, [{'op': 'remove', 'path': '/m/a'}, + {'op': 'remove', 'path': '/missing'}], + in_place=True, atomic=True) + self.assertEqual(chain.maps, [{'a': 1, 'c': 3}, {'b': 2}]) + self.assertIs(chain.maps[1], parent) + + def test_operation_overriding_apply_on_the_instance(self): + calls = [] + + class LoggingRemove(jsonpatch.RemoveOperation): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + apply = self.apply + + def logging_apply(obj): + calls.append(self) + return apply(obj) + + self.apply = logging_apply + + class JsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, remove=LoggingRemove)) + + patch = JsonPatch([{'op': 'add', 'path': '/b', 'value': 2}, + {'op': 'remove', 'path': '/a'}]) + self.assertFalse(jsonpatch._supports_journal(patch._ops[1])) + doc = {'a': 1} + self.assertEqual(patch.apply(doc, in_place=True, atomic=True), + {'b': 2}) + self.assertEqual(len(calls), 1) + + patch = JsonPatch([{'op': 'add', 'path': '/b', 'value': 2}, + {'op': 'remove', 'path': '/a'}, + {'op': 'remove', 'path': '/missing'}]) + doc = {'a': 1} + with self.assertRaises(jsonpatch.JsonPatchPathNotFound): + patch.apply(doc, in_place=True, atomic=True) + self.assertEqual(doc, {'a': 1}) + self.assertEqual(len(calls), 3) + def test_restores_original_containers(self): inner = {'x': [1, 2]} doc = {'a': inner, 'b': 1} From 1a8390efcb27bb7fc4bb2b288808585fc6ec2795 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 22:49:27 +0000 Subject: [PATCH 11/14] Test atomic rollback with patches that build on each other The patches of docs_with_patches draw all their locations from the original document, and most of them fail at an early operation, so in only about 30% of the cases rolling back undid any change, and in 14% more than one. docs_with_failing_patches draws each operation against the document as the preceding ones left it, mixing in operations that refer to existing locations, keeps only operations that succeed, and ends with one that fails. 84% of its cases undo changes, 56% more than one. The new property checks that the document is unchanged afterwards, including member order and the identity of its containers, and that the patch without its failing operation gives the same result as on a copy. It detects the add-before-remove bug fixed in 3fad0a5 without an explicit example, which test_atomic_in_place_gives_same_result did not. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- property_tests.py | 83 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/property_tests.py b/property_tests.py index 699367b..ac0c5c3 100644 --- a/property_tests.py +++ b/property_tests.py @@ -214,6 +214,63 @@ def docs_with_patches(draw): patches = docs_with_patches().map(lambda case: case[1]) +def fails(doc, operation): + """ Whether applying operation to doc fails """ + try: + jsonpatch.apply_patch(doc, [operation]) + except (jsonpatch.JsonPatchException, JsonPointerException): + return True + return False + + +@st.composite +def changing_operations(draw, doc): + """ An operation that changes the container doc, and that refers to + existing locations, so that it is likely to succeed """ + sources = [parts for parts in locations(doc) if parts] + op = draw(st.sampled_from( + ['add', 'remove', 'replace', 'move', 'copy'] if sources else ['add'])) + if op in ('remove', 'replace'): + operation = {'op': op, 'path': to_pointer(draw(existing_locations(doc)))} + else: + parts, key = draw(insert_locations(doc)) + operation = {'op': op, 'path': to_pointer(parts + [key])} + if op in ('move', 'copy'): + operation['from'] = to_pointer(draw(existing_locations(doc))) + if op in ('add', 'replace'): + operation['value'] = draw(json_docs) + return operation + + +@st.composite +def docs_with_failing_patches(draw): + """ A document and a patch whose operations succeed except for the last + one, so that applying it atomically has to undo all their changes. + + Each operation is drawn against the document as the preceding ones left + it, so later operations act on what earlier ones added, moved or + replaced. """ + doc = draw(st.one_of(nonempty_containers, json_docs)) + current = doc + patch = [] + for _ in range(draw(st.integers(min_value=1, max_value=8))): + if isinstance(current, (dict, list)): + operation = draw(st.one_of(changing_operations(current), + operations(current))) + else: + operation = draw(operations(current)) + try: + current = jsonpatch.apply_patch(current, [operation]) + except (jsonpatch.JsonPatchException, JsonPointerException): + continue + patch.append(operation) + patch.append(draw(st.one_of( + st.just({'op': 'remove', 'path': ''}), + operations(current).filter(lambda op: fails(current, op)), + ))) + return doc, patch + + @st.composite def existing_locations(draw, doc): """ The parts of a location in doc other than the root """ @@ -425,6 +482,32 @@ def test_atomic_in_place_gives_same_result(self, case): # unchanged, including the order of object members assert json.dumps(doc) == json.dumps(saved), (doc, saved) + @given(docs_with_failing_patches()) + def test_atomic_rollback_restores_document(self, case): + doc, patch = case + saved = json.dumps(doc) + originals = list(containers(doc)) + expected = jsonpatch.apply_patch(doc, patch[:-1]) + + error = outcome(jsonpatch.apply_patch, doc, patch) + assert error[0] == 'error', error + assert_same_outcome( + outcome(jsonpatch.apply_patch, doc, copy.deepcopy(patch), + in_place=True, atomic=True), + error) + # unchanged, including the order of object members, and made up of + # the same containers as before + assert json.dumps(doc) == saved, (doc, saved) + restored = list(containers(doc)) + assert len(restored) == len(originals) + assert all(a is b for a, b in zip(restored, originals)) + + # without the failing operation, the patch applies as on a copy + assert_json_equal( + jsonpatch.apply_patch(doc, copy.deepcopy(patch[:-1]), + in_place=True, atomic=True), + expected) + @given(docs_with_patches()) def test_patch_as_string_gives_same_result(self, case): doc, patch = case From e6f9a256b3f4794de902343a4d3b8f3f89966d61 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:01:03 +0000 Subject: [PATCH 12/14] Restore Counters and ChainMaps exactly after a failed patch Restoring the member order of a dict passed its items to update(), which a Counter uses to count them, so its contents were replaced by counts of the (key, value) pairs. Move the members from the first misplaced one on to the end with pop() and item assignment instead, the operations the rollback itself uses. Setting a member that a ChainMap takes from another of its mappings adds it to the first one; undoing it set the old value there instead of removing it again. When restoring a ChainMap from a copy, restore each of its mappings, so that the first one doesn't take over the others' members. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 24 ++++++++++++++++++------ tests.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index ed35109..35c821d 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -259,7 +259,11 @@ def __init__(self): self._orders = {} def setitem(self, container, key, value): - if isinstance(container, MutableMapping) and key not in container: + if (isinstance(container, MutableMapping) and key not in container or + # a ChainMap adds the members of its other mappings that are + # set to its first one + isinstance(container, collections.ChainMap) and + key not in container.maps[0]): container[key] = value self._undo.append((_undo_delitem, container, key)) else: @@ -341,10 +345,14 @@ def _reorder(mapping, keys): again; the original members appear in it in their original order. """ keys = [key for key in keys if key in mapping] - if list(mapping) != keys: - items = [(key, mapping[key]) for key in keys] - mapping.clear() - mapping.update(items) + start = next((index for index, (key, original) + in enumerate(zip(mapping, keys)) if key != original), + len(keys)) + # Move the members from the first misplaced one on to the end. Unlike + # update(), which e.g. a Counter uses to add counts, this only uses the + # operations the rollback itself uses + for key in keys[start:]: + mapping[key] = mapping.pop(key) class PatchOperation(object): @@ -667,7 +675,11 @@ def _apply(self, obj, journal): def _restore(obj, saved): """Restores the contents of a document from a copy of it.""" - if isinstance(obj, MutableMapping): + if isinstance(obj, collections.ChainMap): + # restore each mapping, so that none takes over another's members + for mapping, saved_mapping in zip(obj.maps, saved.maps): + _restore(mapping, saved_mapping) + elif isinstance(obj, MutableMapping): obj.clear() obj.update(saved) elif isinstance(obj, MutableSequence): diff --git a/tests.py b/tests.py index 52d35bf..5cd2ca2 100755 --- a/tests.py +++ b/tests.py @@ -2294,7 +2294,8 @@ def make_doc(): def test_mapping_order(self): # the order of dicts is restored, including dict subclasses for cls in (dict, collections.OrderedDict, - lambda items: collections.defaultdict(int, items)): + lambda items: collections.defaultdict(int, items), + lambda items: collections.Counter(dict(items))): with self.subTest(cls=cls): doc = {'m': cls([('a', 1), ('b', 2), ('c', 3)])} with self.assertRaises(jsonpatch.JsonPatchPathNotFound): @@ -2320,6 +2321,36 @@ def test_chain_map(self): self.assertEqual(chain.maps, [{'a': 1, 'c': 3}, {'b': 2}]) self.assertIs(chain.maps[1], parent) + # replacing a member of another mapping adds it to the first one + chain = collections.ChainMap({'a': 1}, parent) + with self.assertRaises(jsonpatch.JsonPatchPathNotFound): + jsonpatch.apply_patch( + {'m': chain}, [{'op': 'replace', 'path': '/m/b', 'value': 5}, + {'op': 'replace', 'path': '/m/b', 'value': 6}, + {'op': 'remove', 'path': '/missing'}], + in_place=True, atomic=True) + self.assertEqual(chain.maps, [{'a': 1}, {'b': 2}]) + + # a document restored from a copy + class ChangesParent(jsonpatch.PatchOperation): + def apply(self, obj): + obj['b'] = 5 + obj.maps[1]['b'] = 6 + raise RuntimeError('after a change') + + class JsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, change=ChangesParent)) + + first = {'a': 1} + chain = collections.ChainMap(first, parent) + with self.assertRaises(RuntimeError): + JsonPatch([{'op': 'change', 'path': ''}]).apply( + chain, in_place=True, atomic=True) + self.assertEqual(chain.maps, [{'a': 1}, {'b': 2}]) + self.assertIs(chain.maps[0], first) + self.assertIs(chain.maps[1], parent) + def test_operation_overriding_apply_on_the_instance(self): calls = [] From 87cb5b313199c9c369e04d9a5ee299595e7320c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:01:47 +0000 Subject: [PATCH 13/14] Update the journal's docstring on restoring the member order Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- jsonpatch.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 35c821d..9cf80c9 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -247,10 +247,9 @@ class _Journal(object): Restoring a removed member adds it at the end of its mapping, so the order of the members of each dict that loses one is recorded once, and restored after all steps have been undone. This is limited to dicts - (including subclasses such as OrderedDict), whose order is the order of - insertion and which clear() and update() restore exactly; other mappings, - such as a ChainMap, would take over members of the mappings they refer - to. + (including subclasses such as OrderedDict or Counter), whose order is the + order of insertion; moving a member of other mappings, such as a + ChainMap, to the end can add it to a different underlying mapping. """ def __init__(self): From 8ccc9317e6f21d70c298bae00a802eba01e55d84 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 12:20:48 +0000 Subject: [PATCH 14/14] Document for which documents atomic rollback is exact Exact restoration, with the same values, member order and container objects, is guaranteed for documents of plain dicts and lists, which includes every document parsed from JSON. Other containers are restored through their own item operations. Besides containers that change when they are read, document that the copy that patches with custom operations are restored from only restores the items of mappings and sequences, not other state such as attributes or a ChainMap's maps. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PewzpjP8bGvTXBq5LST6Xh --- doc/tutorial.rst | 26 ++++++++++++++++++-------- jsonpatch.py | 17 ++++++++++++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 31328ea..879155a 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -175,14 +175,24 @@ documents. The original containers are restored, so references into the document stay valid. If a patch contains custom operations (see :class:`jsonpatch.JsonPatch`'s ``operations``), whose changes can't be undone, the document is copied instead, and its contents are restored from the copy -if an operation fails. - -Only the changes that the operations make are undone. Containers that change -when they are merely read, such as a ``collections.defaultdict``, which adds -a missing key on lookup, are not supported: looking up a location, testing a -value or reading the ``from`` of a ``move`` or ``copy`` can add members to -them that remain after a rollback. Documents parsed from JSON consist of -plain ``dict`` and ``list`` objects and are not affected. +if an operation fails; nested containers are then replaced by their copies. + +Restoring the document exactly, with the same values, the same order of +object members and the same container objects, is guaranteed for documents +made of plain ``dict`` and ``list`` objects, which includes every document +parsed from JSON. Other containers are restored through their own item +operations, which is exact as long as these only change the item they are +given: + +* Containers that change when they are merely read, such as a + ``collections.defaultdict``, which adds a missing key on lookup, are not + supported: looking up a location, testing a value or reading the ``from`` + of a ``move`` or ``copy`` can add members to them that remain after a + rollback. +* The copy that patches with custom operations are restored from only + restores the items of mappings and sequences, not other state that a + custom operation can change, such as the attributes of objects in the + document or the ``maps`` of a ``collections.ChainMap``. Ignoring Missing Locations diff --git a/jsonpatch.py b/jsonpatch.py index 9cf80c9..6a797a2 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -673,7 +673,12 @@ def _apply(self, obj, journal): def _restore(obj, saved): - """Restores the contents of a document from a copy of it.""" + """Restores the contents of a document from a copy of it. + + Only the items of mappings and sequences are restored, not other state + that a custom operation can change, such as attributes or the list of + mappings of a ChainMap. + """ if isinstance(obj, collections.ChainMap): # restore each mapping, so that none takes over another's members for mapping, saved_mapping in zip(obj.maps, saved.maps): @@ -930,10 +935,12 @@ def apply(self, obj, in_place=False, atomic=False): that the patch removes or replaces until it is applied. Patches with custom operations, whose changes can't be undone, copy the document instead. - Members that containers such as ``defaultdict`` add - when they are only read are not removed again. - Without ``in_place=True``, `obj` is never changed, so - this has no effect. + Exact restoration is guaranteed for documents of + plain dicts and lists; members that containers such + as ``defaultdict`` add when they are only read are + not removed again (see the tutorial). Without + ``in_place=True``, `obj` is never changed, so this + has no effect. :type atomic: bool :return: Resulting document. Use this return value even with