avian2 / jsonmerge

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

Merging on top of non-dictionaries fails #40

Closed CommanderPrS closed 5 years ago

CommanderPrS commented 5 years ago

If the base JSON has some plain value, e.g. a string, for some key and the head JSON is trying to merge a dictionary for the same key, jsonmerge.merge() fails with the following error:

  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 346, in merge
    return merger.merge(base, head)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 301, in merge
    return walk.descend(schema, base, head, meta).val
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 78, in descend
    rv = self.work(strategy, schema, *args, **opts)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 123, in work
    rv = strategy.merge(self, base, head, schema, meta, objclass_menu=self.merger.objclass_menu, **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\strategies.py", line 270, in merge
    base[k] = walk.descend(subschema, base.get(k), v, meta)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 78, in descend
    rv = self.work(strategy, schema, *args, **opts)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\__init__.py", line 123, in work
    rv = strategy.merge(self, base, head, schema, meta, objclass_menu=self.merger.objclass_menu, **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\jsonmerge\strategies.py", line 243, in merge
    raise BaseInstanceError("Base for an 'object' merge strategy is not an object", base)
jsonmerge.exceptions.BaseInstanceError: Base for an 'object' merge strategy is not an object: #/std

This seems wrong. If I am overwriting a plain value that is not a dictionary, the merge should simply throw away the value and keep the new dictionary. This is different than using the "replace" strategy blindly - the merge should still do a recursive merge of dictionaries. It is just in cases where the base is not a dictionary, it should just replace.

This is especially tricky if the two JSONs are coming from some user data. If someone makes a mistake in the base JSON, the merge fails, even though it should not. The code that merges does not really have a way to predict where this could happen, thus it can not provide merge strategy overwrites statically.

In case this matters, this happens with jsonmerge 1.6.0 on Windows with python 3.7.2.

avian2 commented 5 years ago

Hi. Can you post a concise code example where this happens? From your description I would say this behavior is by design. In another application it would be a bug if objectMerge strategy would silently overwrite a string. In general, jsonmerge assumes that the documents already conform to some schema before merging and it doesn't try to validate them. Perhaps a solution to your problem is to run a jsonschema.validate on your data before passing it to jsonmerge.

CommanderPrS commented 5 years ago

@avian2

Can you post a concise code example where this happens?

If you do something like this, you should get the error I described:

jsonmerge.merge(
    {
        "key": "value",
    },
    {
        "key": {
            "nested_key": "nested_value",
        },
    }
)
avian2 commented 5 years ago

I can't think of a good way to solve this in jsonmerge without breaking existing applications. I think it's better for objectMerge to be strict in what is accepted than have the possibility to overwrite wrong data without warning.

Possible ways to address this in your code: validate documents before merging, or make a subclass of strategies.ObjectMerge that ignores strings where an object is expected.