Merge pull request #122 from RyanSept/119-120-fix-diffbuilderv1.29

#119 #120 Fix make_patch() to avoid casting numeric string to int on item moved

2 files changed, 65 insertions, 3 deletions

diff --git a/jsonpatch.py b/jsonpatch.py
index 84f6fb3..b4ff24b 100644
--- a/jsonpatch.py
+++ b/jsonpatch.py
@@ -635,7 +635,7 @@ class JsonPatch(object):
         True
         """
         json_dumper = dumps or cls.json_dumper
-        builder = DiffBuilder(json_dumper, pointer_cls=pointer_cls)
+        builder = DiffBuilder(src, dst, json_dumper, pointer_cls=pointer_cls)
         builder._compare_values('', None, src, dst)
         ops = list(builder.execute())
         return cls(ops, pointer_cls=pointer_cls)
@@ -688,12 +688,14 @@ class JsonPatch(object):
 
 class DiffBuilder(object):
 
-    def __init__(self, dumps=json.dumps, pointer_cls=JsonPointer):
+    def __init__(self, src_doc, dst_doc, dumps=json.dumps, pointer_cls=JsonPointer):
         self.dumps = dumps
         self.pointer_cls = pointer_cls
         self.index_storage = [{}, {}]
         self.index_storage2 = [[], []]
         self.__root = root = []
+        self.src_doc = src_doc
+        self.dst_doc = dst_doc
         root[:] = [root, root, None]
 
     def store_index(self, value, index, st):
@@ -800,7 +802,12 @@ class DiffBuilder(object):
         new_index = self.insert(new_op)
         if index is not None:
             op = index[2]
-            if type(op.key) == int:
+            # We can't rely on the op.key type since PatchOperation casts
+            # the .key property to int and this path wrongly ends up being taken
+            # for numeric string dict keys while the intention is to only handle lists.
+            # So we do an explicit check on the item affected by the op instead.
+            added_item = op.pointer.to_last(self.dst_doc)[0]
+            if type(added_item) == list:
                 for v in self.iter_from(index):
                     op.key = v._on_undo_add(op.path, op.key)
 
diff --git a/tests.py b/tests.py
index 28fde9b..a56ffc0 100755
--- a/tests.py
+++ b/tests.py
@@ -490,6 +490,61 @@ class MakePatchTestCase(unittest.TestCase):
         self.assertEqual(res, dst)
         self.assertIsInstance(res['A'], float)
 
+    def test_issue119(self):
+        """Make sure it avoids casting numeric str dict key to int"""
+        src = [
+            {'foobar': {u'1': [u'lettuce', u'cabbage', u'bok choy', u'broccoli'], u'3': [u'ibex'], u'2': [u'apple'], u'5': [], u'4': [u'gerenuk', u'duiker'], u'10_1576156603109': [], u'6': [], u'8_1572034252560': [u'thompson', u'gravie', u'mango', u'coconut'], u'7_1572034204585': []}},
+            {'foobar':{u'description': u'', u'title': u''}}
+        ]
+        dst = [
+            {'foobar': {u'9': [u'almond'], u'10': u'yes', u'12': u'', u'16_1598876845275': [], u'7': [u'pecan']}},
+            {'foobar': {u'1': [u'lettuce', u'cabbage', u'bok choy', u'broccoli'], u'3': [u'ibex'], u'2': [u'apple'], u'5': [], u'4': [u'gerenuk', u'duiker'], u'10_1576156603109': [], u'6': [], u'8_1572034252560': [u'thompson', u'gravie', u'mango', u'coconut'], u'7_1572034204585': []}},
+            {'foobar': {u'description': u'', u'title': u''}}
+        ]
+        patch = jsonpatch.make_patch(src, dst)
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
+    def test_issue120(self):
+        """Make sure it avoids casting numeric str dict key to int"""
+        src = [{'foobar': {'821b7213_b9e6_2b73_2e9c_cf1526314553': ['Open Work'],
+                '6e3d1297_0c5a_88f9_576b_ad9216611c94': ['Many Things'],
+                '1987bcf0_dc97_59a1_4c62_ce33e51651c7': ['Product']}},
+            {'foobar': {'2a7624e_0166_4d75_a92c_06b3f': []}},
+            {'foobar': {'10': [],
+                '11': ['bee',
+                'ant',
+                'wasp'],
+                '13': ['phobos',
+                'titan',
+                'gaea'],
+                '14': [],
+                '15': 'run3',
+                '16': 'service',
+                '2': ['zero', 'enable']}}]
+        dst = [{'foobar': {'1': [], '2': []}},
+            {'foobar': {'821b7213_b9e6_2b73_2e9c_cf1526314553': ['Open Work'],
+                '6e3d1297_0c5a_88f9_576b_ad9216611c94': ['Many Things'],
+                '1987bcf0_dc97_59a1_4c62_ce33e51651c7': ['Product']}},
+            {'foobar': {'2a7624e_0166_4d75_a92c_06b3f': []}},
+            {'foobar': {'b238d74d_dcf4_448c_9794_c13a2f7b3c0a': [],
+                'dcb0387c2_f7ae_b8e5bab_a2b1_94deb7c': []}},
+            {'foobar': {'10': [],
+                '11': ['bee',
+                'ant',
+                'fly'],
+                '13': ['titan',
+                'phobos',
+                'gaea'],
+                '14': [],
+                '15': 'run3',
+                '16': 'service',
+                '2': ['zero', 'enable']}}
+        ]
+        patch = jsonpatch.make_patch(src, dst)
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
     def test_custom_types_diff(self):
         old = {'value': decimal.Decimal('1.0')}
         new = {'value': decimal.Decimal('1.00')}