ibireme / yyjson

The fastest JSON library in C
https://ibireme.github.io/yyjson/doc/doxygen/html/
MIT License
3.04k stars 262 forks source link

Add new keys at end of object with Merge Patch #120

Closed chad-earthscope closed 1 year ago

chad-earthscope commented 1 year ago

Modify the Merge Patch logic to insert new additions to an object at the end of the object.

While technically a JSON object is an "unordered set of name/value pairs", it is nice to be able to control the order in some edge cases (my case is matching a specification for maximum clarity). Further, it is logical that when adding new key/value pairs to an object that they would be added at the end, which is nicely done by yyjson_mut_obj_add() and friends.

The logic of yyjson_merge_patch() and yyjson_mut_merge_patch() lead to additions to objects going before the key/value pairs already existing in the object that are unmodified by the patch.

This PR inverts the order of adding values to the new structure such that modified values are added after unmodified values.

I initially tried replacing:

if (!yyjson_mut_obj_add(builder, mut_key, mut_val)) return NULL;

with

if (!yyjson_mut_obj_insert(builder, mut_key, mut_val, 0)) return NULL;

to add orig elements to the new structure explicitly at the beginning but got some unexpected results. I do not understand the link data structure well enough to figure out why that did not work. So I just swapped the order of additions.

It's not perfect, the root key of any value modified in a nested object structure will move to the bottom (as opposed to staying in place), but at least this is an improvement if one wishes additions to be at the end of an object.

FWIW, this now makes the example test cases in RFC 7386 result in a matching serialized form in terms of order. Specifically this case:

   ORIGINAL        PATCH            RESULT
   ------------------------------------------
...

   {"a":"b"}       {"b":"c"}       {"a":"b",
                                    "b":"c"}
codecov[bot] commented 1 year ago

Codecov Report

Merging #120 (4585a36) into master (f77c915) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
- Coverage   98.50%   98.50%   -0.01%     
==========================================
  Files           2        2              
  Lines        2480     2478       -2     
==========================================
- Hits         2443     2441       -2     
  Misses         37       37              
Impacted Files Coverage Δ
src/yyjson.c 98.91% <100.00%> (-0.01%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ibireme commented 1 year ago

Looks good, let's merge and add some test cases to confirm the ordering of the keys.