Re: [apps-discuss] draft-snell-merge-patch-08 in Ruby

[James M Snell <[e-mail]>]

On Mon, May 20, 2013 at 4:50 PM, Manger, James H
<[e-mail]> wrote:
>> Given James M's example cases, the results should be:
>>
>> > Document + patch = ?
>>
>> > [1,2]  +  {"a":"foo","b":null}
>>
>>   Result: {"a":"foo"}
>
> The JavaScript in draft-snell-merge-patch-08 incorrectly gives the result as [1,2].
>

Fixed in the todays updated draft.

>
>> > {"a":"foo"}  +  {"b":[3,null,{"x":null}]}
>>
>>   Result: {"a":"foo", "b":[3,{}]}
>
> NO. The result should be: {"a":"foo", "b":[3,null,{"x":null}]}.
> Merge-patch does NOT merge arrays; it treats them like other primitives (strings, numbers, booleans) so a merge-patch implementation should NOT be looking inside an array for nulls.
>

No, the correct result is that nulls are treated as if they are
undefined and not present, They are to be removed from the result.

> The JavaScript in draft-snell-merge-patch-08 gives neither of these results. It incorrectly gives {"a":"foo", "b":[3,{"x":null}]}
>

Fixed in todays updated draft.

>
>> > {"a":"foo"}  +  null
>>
>>   Result: Invalid. The patch document MUST be a valid JSON document,
>> which means the root must be either a JSON object or array. A null
>> patch document is invalid.
>
> I thought the one non-controversial change planned for RFC4627 "JSON" was to allow any JSON value (not just arrays and objects) as a JSON-text.
>

The draft is based on the current definition of RFC4627 and not on any
"planned" changes that might happen in the future.

> A patch of null certainly should not return the original unchanged (as the JavaScript in draft-snell-merge-patch-08 does). Either throw an error, or return an undefined value.
>

Agree. An error would be the most appropriate outcome.

>
>> If there is an error in the javascript impl in the draft, I'll make
>> sure that gets corrected in the next iteration.
>
> My suggested JavaScript implementation:
>
> function merge(orig, patch) {
>         var result;
>         if (patch === null) {
>                 // leave result undefined
>         }
>         else if (typeof patch === 'string' ||
>                 typeof patch === 'number' ||
>                 typeof patch === 'boolean' ||
>                 Array.isArray(patch))
>         {
>                 result = patch;
>         }
>         else { // patch is an object
>                 result = {};
>                 if (orig instanceof Object && !Array.isArray(orig)) {
>                         for (m in orig)
>                                 result[m] = orig[m];
>                 }
>                 for (m in patch) {
>                         result[m] = merge(result[m], patch[m]);
>                 }
>         }
>         return result;
> }
>
> See it in action at http://id.cto.telstra.com/json/merge.html
>

I will take a more in depth look at this a bit later tonight or tomorrow.

- James

> --
> James Manger