Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: patch didn't catch changed index causing the apply to fail #124

Open
zliang11 opened this issue Feb 4, 2021 · 3 comments
Open

Bug: patch didn't catch changed index causing the apply to fail #124

zliang11 opened this issue Feb 4, 2021 · 3 comments

Comments

@zliang11
Copy link

zliang11 commented Feb 4, 2021

I found a bug with an example here that failed when applying the patch that was generated.

>>> L1 = ['a', 'b', ['d', 'e'], 'f']
>>> L2 = ['a', 'd', ['e', 'g']]
>>> patch = jsonpatch.make_patch(L1,L2)
>>> list(patch)
[{u'path': u'/1', u'op': u'remove'}, {u'path': u'/1', u'from': **u'/2/0'**, u'op': u'move'}, {u'path': u'/2/1', u'value': 'g', u'op': u'add'}, {u'path': u'/3', u'op': u'remove'}]

>>> newL2 = jsonpatch.apply_patch(L1,patch)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ...... line 151, in apply_patch
    return patch.apply(doc, in_place)
  File ...... line 669, in apply
    obj = operation.apply(obj)
  File ...... line 386, in apply
    }, pointer_cls=self.pointer_cls).apply(obj)
  File ...... line 238, in apply
    del subobj[part]
TypeError: 'str' object doesn't support item deletion

By looking at the patches, when 'b' is removed, the index for 'd' should be u'/1/0' but the patch still has u'2/0', which causes the error because u'/2' is now 'f', which is not a list, so it can't find it to do the "delete at /2/0 then add it back at /1" - the Move operation. I tested using this alternative jsondiff make function and it worked fine. https://github.com/nxsofsys/jsondiff

>>> make(L1,L2)
[{'path': '/1', 'op': 'remove'}, {'path': '/1', 'from': '/1/0', 'op': 'move'}, {'path': '/2/1', 'value': 'g', 'op': 'add'}, {'path': '/3', 'op': 'remove'}]
>>> jsonpatch.apply_patch(L1, make(L1,L2))
['a', 'd', ['e', 'g']]

@zliang11 zliang11 changed the title Bug: patch didn't catch changed indices causing the apply to fail Bug: patch didn't catch changed index causing the apply to fail Feb 4, 2021
@zliang11
Copy link
Author

zliang11 commented Feb 8, 2021

Hi, @stefankoegl do you perhaps know where it isn't catching the index change?

@zliang11
Copy link
Author

I noticed that the error is probably caused by _undo_remove and _undo_add directly comparing the path and the key. It is very likely for nested lists that the index affected is not the key (last value) of its location. We can probably split the locations by '/', get the index of the key, then compare the value at that corresponding index to do the +,- changes.

@rmorshea
Copy link

I think this may be related: #138 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants