avian2 / jsonmerge

Merge a series of JSON documents.
MIT License
214 stars 25 forks source link

Fix for arrayMergeById when iterator oversteps merge item #31

Closed fdearle closed 6 years ago

fdearle commented 6 years ago

Under certain circumstances the arrayMergeById strategy fails with and "Unresolvable JSON pointer" error. It depends entirely on the structure of the base and head JSON that is used.

I was able to recreate the bug with a single line change to the existing tests for arrayMergeById. By reordering the head items in that test we get the failure below.

The problem is that as the code iterates over head and base arrays while matching it can step over a matched item and then try and merge the wrong item.

By recording the matched item we can avoid this.

@avian2 Could you let me know if this PR is acceptable and when you might be able to make a new release with it. I am currently having to carefully construct JSON to avoid this issue.

The easiest way to reproduce is to apply my change to the test first and rerun the tests. The change in strategies.py will then fix the failed test.

Regards

Fergal

Error output from test run : Traceback (most recent call last): File "/Users/fdearle/github/jsonmerge/tests/test_jsonmerge.py", line 653, in test_merge_by_id base = merger.merge(base, b) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 288, in merge return walk.descend(schema, base, head, meta).val File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 80, in descend rv = self.work(strategy, schema, *args, **opts) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 125, in work rv = strategy.merge(self, base, head, schema, meta, objclass_menu=self.merger.objclass_menu, **kwargs) File "/Users/fdearle/github/jsonmerge/jsonmerge/strategies.py", line 269, in merge base[k] = walk.descend(subschema, base.get(k), v, meta) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 80, in descend rv = self.work(strategy, schema, *args, **opts) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 125, in work rv = strategy.merge(self, base, head, schema, meta, objclass_menu=self.merger.objclass_menu, **kwargs) File "/Users/fdearle/github/jsonmerge/jsonmerge/strategies.py", line 182, in merge base[j] = walk.descend(subschema, base_item, head_item, meta) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 80, in descend rv = self.work(strategy, schema, *args, **opts) File "/Users/fdearle/github/jsonmerge/jsonmerge/__init__.py", line 118, in work with self.base_resolver.resolving(base.ref) as resolved: File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 17, in __enter__ return self.gen.next() File "/Library/Python/2.7/site-packages/jsonschema/validators.py", line 366, in resolving url, resolved = self.resolve(ref) File "/Library/Python/2.7/site-packages/jsonschema/validators.py", line 375, in resolve return url, self._remote_cache(url) File "/Library/Python/2.7/site-packages/jsonschema/validators.py", line 387, in resolve_from_url return self.resolve_fragment(document, fragment) File "/Library/Python/2.7/site-packages/jsonschema/validators.py", line 421, in resolve_fragment "Unresolvable JSON pointer: %r" % fragment RefResolutionError: Unresolvable JSON pointer: u'awards/2'

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.006%) to 97.38% when pulling 47c3fab57402a08bd17c4586907995627fe9c271 on fdearle:master into 92b05d6a8fed908eee35b56289bdd65833ca1cdc on avian2:master.

avian2 commented 6 years ago

This was indeed quite a bad bug. Thank you very much for reporting it and providing a concise patch! I merged your fix and will be releasing jsonmerge 1.5.1 shortly.

I prefer adding tests for newly discovered bugs instead of modifying existing ones, so I reverted your change in test_jsonmerge.py and added two new tests specific to this issue.

For posterity, here is a more detailed description of the effects of the bug:

fdearle commented 6 years ago

Figured you would change it yourself. Thanks for the prompt reply.

BTW. Awesome library. Saved me a bunch of code.