Subject: Re: [apps-discuss] Updated Merge Patch

From: "Manger, James H" <[email-masked]>
Date: Wed, 7 Nov 2012 15:54:20 +1100

James,

draft-snell-merge-patch-06 doesn’t quite fix the glitches I mentioned last month.
Having the javascript in draft-snell-merge-patch-07 is great in showing these cases.

Original + merge =
   result by 07 javascript
   result by 07 text
   result is what it should be (according to me)

[1,2] + {"a":3, "b":null} =
   [1,2] by 07 javascript;
   {"a":3} by 07 text, as it should be

[1,2] + [7, null] =
   [7] by 07 javascript and text;
   [7, null] is what it should be
There is no possibility of the null in this patch (in an array) having any affect on the original so why bother searching the patch to remove it? Better to treat arrays as “primitive”.

[1,2] + null =
   [1,2] by 07 javascript
   Undefined by 07 text
   <empty> is what it should be

The 2 rules in draft-07 start:

1.       If either the original or patch are arrays...

2.       If the patch is an object...
This fails to cover cases where the original or patch are primitive (string, boolean, number, or null). If roots cannot be primitive then rule 2 doesn’t need its “if” statement.

--
James Manger

From: [email-masked] [mailto:[email-masked]] On Behalf Of Manger, James H
Sent: Friday, 19 October 2012 11:14 AM
To: James M Snell; IETF Apps Discuss
Subject: Re: [apps-discuss] Updated Merge Patch

James,

draft-snell-merge-patch-05 looks better, but is not quite right.

§2 step 1
 If either the root of the JSON data provided in the payload or
 the root of the target resource are JSON Arrays, the target
 resource is to be replaced, in whole, by the provided data.
Implies (target + merge = result):
[1,2] + {"a":3, "b":null} = {"a":3, "b":null}
But the result should instead be {"a":3}.

The same mistake is made in step 2, 4th dot point.

Step 1 could be rewritten as:
  “If the root of the patch is a string, number, boolean, or array, the target is replaced with the patch value.”

This rule is not actually specific to the root so it probably doesn’t need to be a special case.

My stab at the processing rules:

“The processing steps walk through the object elements in the merge-patch document and the corresponding elements in the document being modified. Let ‘patch’ and ‘target’ represent the current position in these documents respectively. Initially ‘patch’ is the root of the merge-patch document, and ‘target’ is the root of the document being modified. At any point ‘target’ may not exist.

1.       If the patch value is null: remove target.
It is not an error if target does not exist, there is simply no modification to make.
Removing the root leaves an empty document (ie a resource with a content-length of 0).

2.       Otherwise, if the patch value is a string, number, boolean, or array: set target to patch. This creates a new object element if target didn’t exist, or replaces target’s value if it did exist.

3.       Otherwise, the patch value is an object. Perform the next two sub-steps:

a.       If target does not exist or if its value is not an object: set target to an element with the patch name, and an empty object as its value (ie {}).
For the root there isn’t really an element name, just a value.

b.      For each element of the patch value object: repeat the processing steps with patch being that element, and target being the element of the previous target with the same name (if such an element exists).
”

This is harder to write in English than code!

It would be worth including examples of the less obvious cases.

Target   + merge = result

{"a":[1,{"bb":2, "cc":3}]} + {"a":[1,{"bb":4, "cc":null}]} = {"a":[1,{"bb":4, "cc":null}]}
  (showing that you don’t merge objects within an array)

{"a":1} + {"b":{"cc":{"ddd":null}}} = {"a":1, "b":{"cc":{ }}}
  (showing that objects on a path get created (step 3a), even if the only tasks at the end of the path turns out to be removing elements)

[1,2] + {"a":3, "b":null} = {"a":3}
  (showing you still have to walk through the patch, even when the corresponding part of the target wasn’t also an object)

{"a":1} + null = <empty>

Splitting the processing steps and the worked example (section 2) into separate sections would be an improvement.

Then can we have this operation in json-patch: {"op":"merge", "path":<pointer>, "value":<merge-patch>}?

--
James Manger

From: [email-masked]<mailto:[email-masked]> [mailto:[email-masked]] On Behalf Of James M Snell
Sent: Friday, 19 October 2012 4:22 AM
To: IETF Apps Discuss
Subject: [apps-discuss] Updated Merge Patch

Based on received feedback, mostly editorial modifications and a few updates to the rules to cover a couple additional cases (thanks to James Manger for pointing out the gaps and providing new language for the intro).

  http://www.ietf.org/id/draft-snell-merge-patch-05.txt

- James

---

From: "Manger, James H" <[email-masked]>
Date: Wed, 7 Nov 2012 17:35:55 +1100

James,

Here is some alternative javascript for Appendix A "Example JavaScript Implementation" of draft-snell-merge-patch-07; and there is a web page at http://id.cto.telstra.com/json/merge.html that tests this code and the draft-07 code.

function better_apply(orig, patch) {
	var result;
	if (patch == null) {
		// leave result undefined
	}
	else if (typeof patch == 'string' ||
		typeof patch == 'number' ||
		typeof patch == 'boolean' ||
		patch instanceof Array)
	{
		result = patch;
	}
	else { // patch is an object
		result = {};
		if (orig instanceof Object && !(orig instanceof Array)) {
			for (m in orig)
				result[m] = orig[m];
		}
		for (m in patch) {
			result[m] = better_apply(result[m], patch[m]);
		}
	}
	return result;
}

P.S. From an programmer not that experienced with javascript -- beware.

--
James Manger