From 975893443cc7e1c258c82e1272fbc3fd7268d1df Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 15:43:10 +0200 Subject: [PATCH 01/11] Add property based test. --- .gitignore | 1 + requirements-dev.txt | 1 + tests.py | 23 +++++++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/.gitignore b/.gitignore index 179770e..56a7db7 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ build dist *.egg-info/ doc/_build +.hypothesis/ diff --git a/requirements-dev.txt b/requirements-dev.txt index 21daf9a..744c41c 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,2 +1,3 @@ wheel pypandoc +hypothesis >= 3 diff --git a/tests.py b/tests.py index 51d9517..60e3352 100755 --- a/tests.py +++ b/tests.py @@ -9,6 +9,8 @@ import jsonpatch import jsonpointer import sys +import string +from hypothesis import given, note, strategies as st class ApplyPatchTestCase(unittest.TestCase): @@ -518,6 +520,26 @@ def test_replace_missing(self): self.assertRaises(jsonpatch.JsonPatchConflict, jsonpatch.apply_patch, src, patch_obj) +json_st = st.recursive( + st.one_of([ + st.none(), + st.booleans(), + st.floats(), + st.text(string.printable)]), + lambda children: + st.lists(children) + | st.dictionaries(st.text(string.printable), children)) + + +class RoundtripTests(unittest.TestCase): + @given(json_st, json_st) + def test_roundtrip(self, src, dst): + patch = jsonpatch.JsonPatch.from_diff(src, dst, False) + note('Patch: %s' % (patch,)) + res = patch.apply(src) + self.assertEqual(res, dst) + + if __name__ == '__main__': modules = ['jsonpatch'] @@ -531,6 +553,7 @@ def get_suite(): suite.addTest(unittest.makeSuite(InvalidInputTests)) suite.addTest(unittest.makeSuite(ConflictTests)) suite.addTest(unittest.makeSuite(OptimizationTests)) + suite.addTest(unittest.makeSuite(RoundtripTests)) return suite From 24d3196fd84e591553b865691d93eabe62002206 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 15:53:41 +0200 Subject: [PATCH 02/11] Don't generate NaNs. JSON doesn't actually allow these (despite the json module in Python happily serializing / parsing 'nan'), and they make assertEqual fail since they're not equal to anything, including themselves. --- tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests.py b/tests.py index 60e3352..e724873 100755 --- a/tests.py +++ b/tests.py @@ -524,7 +524,7 @@ def test_replace_missing(self): st.one_of([ st.none(), st.booleans(), - st.floats(), + st.floats(allow_nan=False), st.text(string.printable)]), lambda children: st.lists(children) From 313e1b34176210a178c404cfbed6f0350b5e92d1 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 15:56:46 +0200 Subject: [PATCH 03/11] Install dev requirements to run the tests. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 77f8b19..473716c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ python: - "pypy3" install: - - travis_retry pip install -r requirements.txt + - travis_retry pip install -r requirements.txt -r requirements-dev.txt - if [ "$TRAVIS_PYTHON_VERSION" == "3.2" ]; then travis_retry pip install 'coverage<4'; fi - travis_retry pip install coveralls From 49b6c74b662471b6849442becf5a165348376420 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 16:03:18 +0200 Subject: [PATCH 04/11] Tolerate Hypothesis being missing (no 2.6, 3.2, or 3.3 support). --- .travis.yml | 3 ++- tests.py | 43 ++++++++++++++++++++++++------------------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/.travis.yml b/.travis.yml index 473716c..c95cbfc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,8 @@ python: - "pypy3" install: - - travis_retry pip install -r requirements.txt -r requirements-dev.txt + - travis_retry pip install -r requirements.txt + - if [ "$TRAVIS_PYTHON_VERSION" != "2.6" -a "$TRAVIS_PYTHON_VERSION" != "3.2" -a "$TRAVIS_PYTHON_VERSION" != "3.3" ]; then travis_retry pip install hypothesis; fi - if [ "$TRAVIS_PYTHON_VERSION" == "3.2" ]; then travis_retry pip install 'coverage<4'; fi - travis_retry pip install coveralls diff --git a/tests.py b/tests.py index e724873..08bea16 100755 --- a/tests.py +++ b/tests.py @@ -10,7 +10,11 @@ import jsonpointer import sys import string -from hypothesis import given, note, strategies as st +try: + import hypothesis + from hypothesis import strategies as st +except ImportError: + hypothesis = None class ApplyPatchTestCase(unittest.TestCase): @@ -520,24 +524,24 @@ def test_replace_missing(self): self.assertRaises(jsonpatch.JsonPatchConflict, jsonpatch.apply_patch, src, patch_obj) -json_st = st.recursive( - st.one_of([ - st.none(), - st.booleans(), - st.floats(allow_nan=False), - st.text(string.printable)]), - lambda children: - st.lists(children) - | st.dictionaries(st.text(string.printable), children)) +if hypothesis is not None: + json_st = st.recursive( + st.one_of([ + st.none(), + st.booleans(), + st.floats(allow_nan=False), + st.text(string.printable)]), + lambda children: + st.lists(children) + | st.dictionaries(st.text(string.printable), children)) - -class RoundtripTests(unittest.TestCase): - @given(json_st, json_st) - def test_roundtrip(self, src, dst): - patch = jsonpatch.JsonPatch.from_diff(src, dst, False) - note('Patch: %s' % (patch,)) - res = patch.apply(src) - self.assertEqual(res, dst) + class RoundtripTests(unittest.TestCase): + @hypothesis.given(json_st, json_st) + def test_roundtrip(self, src, dst): + patch = jsonpatch.JsonPatch.from_diff(src, dst, False) + hypothesis.note('Patch: %s' % (patch,)) + res = patch.apply(src) + self.assertEqual(res, dst) if __name__ == '__main__': @@ -553,7 +557,8 @@ def get_suite(): suite.addTest(unittest.makeSuite(InvalidInputTests)) suite.addTest(unittest.makeSuite(ConflictTests)) suite.addTest(unittest.makeSuite(OptimizationTests)) - suite.addTest(unittest.makeSuite(RoundtripTests)) + if hypothesis is not None: + suite.addTest(unittest.makeSuite(RoundtripTests)) return suite From 6cc61b1f646edac4ed90929b18143da1515e813d Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 16:18:29 +0200 Subject: [PATCH 05/11] Add explicit example. --- tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests.py b/tests.py index 08bea16..0ba2488 100755 --- a/tests.py +++ b/tests.py @@ -536,6 +536,8 @@ def test_replace_missing(self): | st.dictionaries(st.text(string.printable), children)) class RoundtripTests(unittest.TestCase): + @hypothesis.example({}, {u'%20': None}) + @hypothesis.example({u'%20': None}, {}) @hypothesis.given(json_st, json_st) def test_roundtrip(self, src, dst): patch = jsonpatch.JsonPatch.from_diff(src, dst, False) From 6f107152a1c7f04c6eee9098e34ed777d6ddff74 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Thu, 6 Jul 2017 16:25:01 +0200 Subject: [PATCH 06/11] Fix 3.2 compat. --- tests.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests.py b/tests.py index 0ba2488..7e79202 100755 --- a/tests.py +++ b/tests.py @@ -10,12 +10,18 @@ import jsonpointer import sys import string + try: import hypothesis from hypothesis import strategies as st except ImportError: hypothesis = None +try: + unicode +except NameError: + unicode = str + class ApplyPatchTestCase(unittest.TestCase): @@ -536,8 +542,8 @@ def test_replace_missing(self): | st.dictionaries(st.text(string.printable), children)) class RoundtripTests(unittest.TestCase): - @hypothesis.example({}, {u'%20': None}) - @hypothesis.example({u'%20': None}, {}) + @hypothesis.example({}, {unicode('%20'): None}) + @hypothesis.example({unicode('%20'): None}, {}) @hypothesis.given(json_st, json_st) def test_roundtrip(self, src, dst): patch = jsonpatch.JsonPatch.from_diff(src, dst, False) From 637f0c7a4c0e0bee1d54899ab95d93fc9a3f7c20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20K=C3=B6gl?= Date: Sun, 31 Dec 2017 14:27:44 +0100 Subject: [PATCH 07/11] Prefer Python3 when distinguishing str/unicode --- tests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests.py b/tests.py index 55496ab..e1b4300 100755 --- a/tests.py +++ b/tests.py @@ -18,9 +18,9 @@ hypothesis = None try: - unicode + str = unicode # Python 2 except NameError: - unicode = str + pass class ApplyPatchTestCase(unittest.TestCase): @@ -632,8 +632,8 @@ def test_replace_missing(self): | st.dictionaries(st.text(string.printable), children)) class RoundtripTests(unittest.TestCase): - @hypothesis.example({}, {unicode('%20'): None}) - @hypothesis.example({unicode('%20'): None}, {}) + @hypothesis.example({}, {str('%20'): None}) + @hypothesis.example({str('%20'): None}, {}) @hypothesis.given(json_st, json_st) def test_roundtrip(self, src, dst): patch = jsonpatch.JsonPatch.from_diff(src, dst, False) From 1d89ec39bbd2009fd0b9c09c847ead4427e55079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20K=C3=B6gl?= Date: Sun, 31 Dec 2017 14:28:18 +0100 Subject: [PATCH 08/11] Handle version-specific dependencies w/ PEP 508 --- .travis.yml | 4 +--- requirements-dev.txt | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2cad004..c6be8a8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,8 +12,8 @@ python: - pypy3 install: - travis_retry pip install -r requirements.txt +- travis_retry pip install -r requirements-dev.txt - travis_retry pip install coveralls -- if [ "$TRAVIS_PYTHON_VERSION" != "3.3" ]; then travis_retry pip install hypothesis; fi script: - coverage run --source=jsonpointer tests.py after_script: @@ -23,8 +23,6 @@ addons: apt: packages: - pandoc -before_deploy: -- pip install -r requirements-dev.txt deploy: provider: pypi user: skoegl diff --git a/requirements-dev.txt b/requirements-dev.txt index 744c41c..c899c73 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,3 +1,3 @@ wheel pypandoc -hypothesis >= 3 +hypothesis>=3; python_version=="2.7" or python_version>"3.3" From f301608cba9df042b6121e011ce188658109faf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20K=C3=B6gl?= Date: Fri, 5 Jan 2018 19:59:14 +0100 Subject: [PATCH 09/11] Simplify DiffBuilder interface --- jsonpatch.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 422812d..9fca5e3 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -281,8 +281,7 @@ def from_diff(cls, src, dst, optimization=True): True """ - builder = DiffBuilder() - builder._compare_values('', None, src, dst) + builder = DiffBuilder(src, dst) ops = list(builder.execute()) return cls(ops) @@ -625,7 +624,9 @@ def apply(self, obj): class DiffBuilder(object): - def __init__(self): + def __init__(self, src, dst): + self.src = src + self.dst = dst self.index_storage = [{}, {}] self.index_storage2 = [[], []] self.__root = root = [] @@ -682,6 +683,8 @@ def __iter__(self): curr = curr[1] def execute(self): + self._compare_values('', None, self.src, self.dst) + root = self.__root curr = root[1] while curr is not root: From 7724ddd496c4cfb452edd59ff9bb9cd3d262c2b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20K=C3=B6gl?= Date: Mon, 2 Apr 2018 17:09:18 +0200 Subject: [PATCH 10/11] Improve hypothesis test error message --- tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests.py b/tests.py index e1b4300..6a00bd0 100755 --- a/tests.py +++ b/tests.py @@ -639,7 +639,9 @@ def test_roundtrip(self, src, dst): patch = jsonpatch.JsonPatch.from_diff(src, dst, False) hypothesis.note('Patch: %s' % (patch,)) res = patch.apply(src) - self.assertEqual(res, dst) + message = '{src} + {patch} resulted in {res}; {dst} was expected'.format( + src=repr(src), patch=repr(patch), res=repr(res), dst=repr(dst)) + self.assertEqual(res, dst, message) if __name__ == '__main__': From 62241db58ce79c4a46c5f3b18dad59cb4c2ba236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20K=C3=B6gl?= Date: Mon, 2 Apr 2018 17:10:12 +0200 Subject: [PATCH 11/11] Simplify code for creating patches --- jsonpatch.py | 202 +++++++++++++++++++++------------------------------ 1 file changed, 81 insertions(+), 121 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 9fca5e3..a39b23a 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -627,135 +627,72 @@ class DiffBuilder(object): def __init__(self, src, dst): self.src = src self.dst = dst - self.index_storage = [{}, {}] - self.index_storage2 = [[], []] - self.__root = root = [] - root[:] = [root, root, None] - def store_index(self, value, index, st): - try: - storage = self.index_storage[st] - stored = storage.get(value) - if stored is None: - storage[value] = [index] - else: - storage[value].append(index) + self.ops = [] + self.index_changes = {} - except TypeError: - self.index_storage2[st].append((value, index)) + def insert(self, op): + self.ops.append(op) - def take_index(self, value, st): - try: - stored = self.index_storage[st].get(value) - if stored: - return stored.pop() + def execute(self): + self._compare_values('', None, self.src, self.dst) + return [op.operation for op in self.ops] - except TypeError: - storage = self.index_storage2[st] - for i in range(len(storage)-1, -1, -1): - if storage[i][0] == value: - return storage.pop(i)[1] + def _item_added(self, path, key, item, is_sequence): + if is_sequence: + key = self.get_index_change(path, key) - def insert(self, op): - root = self.__root - last = root[0] - last[1] = root[0] = [last, root, op] - return root[0] - - def remove(self, index): - link_prev, link_next, _ = index - link_prev[1] = link_next - link_next[0] = link_prev - index[:] = [] - - def iter_from(self, start): - root = self.__root - curr = start[1] - while curr is not root: - yield curr[2] - curr = curr[1] + new_op = AddOperation({ + 'op': 'add', + 'path': _path_join(path, key), + 'value': item, + }) + self.insert(new_op) - def __iter__(self): - root = self.__root - curr = root[1] - while curr is not root: - yield curr[2] - curr = curr[1] + if is_sequence: + self.add_index_change(path, key, +1) - def execute(self): - self._compare_values('', None, self.src, self.dst) + def _item_moved(self, path, key, item, newkey): + """ The `item` was moved from `key` to `newkey` - root = self.__root - curr = root[1] - while curr is not root: - if curr[1] is not root: - op_first, op_second = curr[2], curr[1][2] - if op_first.location == op_second.location and \ - type(op_first) == RemoveOperation and \ - type(op_second) == AddOperation: - yield ReplaceOperation({ - 'op': 'replace', - 'path': op_second.location, - 'value': op_second.operation['value'], - }).operation - curr = curr[1][1] - continue + [a, b, c, d, e] + 0 1, 2, 3, 4 + + case 1: move 1 to 3 + [a, c, d, b, e] + 0 1, 2, 3, 4 + + """ + + key = self.get_index_change(path, key) + newkey = self.get_index_change(path, newkey) + + if key == newkey: + return + + new_op = MoveOperation({ + 'op': 'move', + 'from': _path_join(path, key), + 'path': _path_join(path, newkey), + }) + self.insert(new_op) + + self.add_index_change(path, key+1, -1) + self.add_index_change(path, newkey+1, +1) + + + def _item_removed(self, path, key, item, is_sequence): + if is_sequence: + key = self.get_index_change(path, key) - yield curr[2].operation - curr = curr[1] - - def _item_added(self, path, key, item): - index = self.take_index(item, _ST_REMOVE) - if index is not None: - op = index[2] - if type(op.key) == int: - for v in self.iter_from(index): - op.key = v._on_undo_remove(op.path, op.key) - - self.remove(index) - if op.location != _path_join(path, key): - new_op = MoveOperation({ - 'op': 'move', - 'from': op.location, - 'path': _path_join(path, key), - }) - self.insert(new_op) - else: - new_op = AddOperation({ - 'op': 'add', - 'path': _path_join(path, key), - 'value': item, - }) - new_index = self.insert(new_op) - self.store_index(item, new_index, _ST_ADD) - - def _item_removed(self, path, key, item): new_op = RemoveOperation({ 'op': 'remove', 'path': _path_join(path, key), }) - index = self.take_index(item, _ST_ADD) - new_index = self.insert(new_op) - if index is not None: - op = index[2] - if type(op.key) == int: - for v in self.iter_from(index): - op.key = v._on_undo_add(op.path, op.key) - - self.remove(index) - if new_op.location != op.location: - new_op = MoveOperation({ - 'op': 'move', - 'from': new_op.location, - 'path': op.location, - }) - new_index[2] = new_op - - else: - self.remove(new_index) + self.insert(new_op) - else: - self.store_index(item, new_index, _ST_REMOVE) + if is_sequence: + self.add_index_change(path, key, -1) def _item_replaced(self, path, key, item): self.insert(ReplaceOperation({ @@ -771,10 +708,10 @@ def _compare_dicts(self, path, src, dst): removed_keys = src_keys - dst_keys for key in removed_keys: - self._item_removed(path, str(key), src[key]) + self._item_removed(path, str(key), src[key], False) for key in added_keys: - self._item_added(path, str(key), dst[key]) + self._item_added(path, str(key), dst[key], False) for key in src_keys & dst_keys: self._compare_values(path, key, src[key], dst[key]) @@ -786,6 +723,7 @@ def _compare_lists(self, path, src, dst): for key in range(max_len): if key < min_len: old, new = src[key], dst[key] + if old == new: continue @@ -798,14 +736,14 @@ def _compare_lists(self, path, src, dst): self._compare_lists(_path_join(path, key), old, new) else: - self._item_removed(path, key, old) - self._item_added(path, key, new) + self._item_removed(path, key, old, True) + self._item_added(path, key, new, True) elif len_src > len_dst: - self._item_removed(path, len_dst, src[key]) + self._item_removed(path, len_dst, src[key], True) else: - self._item_added(path, key, dst[key]) + self._item_added(path, key, dst[key], True) def _compare_values(self, path, key, src, dst): if src == dst: @@ -822,6 +760,28 @@ def _compare_values(self, path, key, src, dst): else: self._item_replaced(path, key, dst) + def add_index_change(self, path, key, change): + """ Store the index change of a certain path + + [a, b, c] + 0, 1, 2 + + When inserting d at position 1, the indexes of b and c change + [a, d, b, c] + 0, 1, 2, 3 + + add_index_change('/', 1, +1) + """ + + path_changes = self.index_changes.get(path, {}) + key_change = path_changes.get(key, 0) + key_change = key_change + change + path_changes[key] = key_change + + def get_index_change(self, path, key): + path_changes = self.index_changes.get(path, {}) + return path_changes.get(key, 0) + key + def _path_join(path, key): if key is None: