Fix json_patch_apply handling of removing the whole document (i.e. "path":"").

Enable all disabled tests, add a few more including some with null documents.

json_patch.c

@@ -81,7 +81,10 @@ static int __json_patch_apply_remove(struct json_pointer_get_result *jpres)
 		json_object_object_del(jpres->parent, jpres->key_in_parent);
 		return 0;
 	} else {
-		return json_object_put(jpres->obj);
+		// We're removing the root object
+		(void)json_object_put(jpres->obj);
+		jpres->obj = NULL;
+		return 0;
 	}
 }
 
@@ -99,6 +102,9 @@ static int json_patch_apply_remove(struct json_object **res, const char *path, s
 	rc = __json_patch_apply_remove(&jpres);
 	if (rc < 0)
 		_set_err(EINVAL, "Unable to remove path referenced by 'path' field");
+	// This means we removed and freed the root object, i.e. *res
+	if (jpres.parent == NULL)
+		*res = NULL;
 	return rc;
 }
 
@@ -223,6 +229,8 @@ static int json_patch_apply_move_copy(struct json_object **res,
 		return rc;
 	}
 
+	// Note: it's impossible for json_pointer to find the root obj, due
+	// to the path check above, so from.parent is guaranteed non-NULL
 	json_object_get(from.obj);
 
 	if (!move) {

tests/json_patch_spec_tests.json

@@ -192,8 +192,8 @@
     "patch": [
   { "op": "add", "path": "/baz", "value": "qux", "op": "remove" }
 ],
-    "error": "operation has two 'op' members",
-    "disabled": true
+    "error_wont_happen_in_jsonc": "operation has two 'op' members",
+    "error": "Did not find element referenced by path field"
   },
 
   {

tests/json_patch_tests.json

@@ -52,8 +52,8 @@
     { "comment": "Toplevel scalar values OK?",
       "doc": "foo",
       "patch": [{"op": "replace", "path": "", "value": "bar"}],
-      "expected": "bar",
-      "disabled": true },
+      "expected": "bar"
+      },
 
     { "comment": "replace object document with array document?",
       "doc": {},
@@ -202,6 +202,55 @@
       "patch": [{"op": "replace", "path": "", "value": {"baz": "qux"}}],
       "expected": {"baz": "qux"} },
 
+    { "comment": "add whole document, null",
+      "doc": {},
+      "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it",
+      "patch": [
+        {"op": "remove", "path": ""},
+        {"op": "add", "path": "", "value": {"baz": "qux"}}
+      ],
+      "expected": {"baz": "qux"} },
+
+    { "comment": "replace whole document, null",
+      "doc": {},
+      "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it",
+      "patch": [
+        {"op": "remove", "path": ""},
+        {"op": "replace", "path": "", "value": {"baz": "qux"}}
+      ],
+      "error": "The spec says the target location must exist, so replacing a null document fails"
+    },
+
+    { "comment": "remove whole document",
+      "doc": {"foo": "bar"},
+      "patch": [{"op": "remove", "path": ""}],
+      "expected": null },
+
+    { "comment": "remove whole document",
+      "doc": {"foo": "bar"},
+      "patch": [{"op": "remove", "path": ""}],
+      "expected": null },
+
+    { "comment": "remove whole document, array",
+      "doc": ["foo", "bar"],
+      "patch": [{"op": "remove", "path": ""}],
+      "expected": null },
+
+    { "comment": "remove whole document, string",
+      "doc": "foo",
+      "patch": [{"op": "remove", "path": ""}],
+      "expected": null },
+
+    { "comment": "remove whole document, null",
+      "doc": {},
+      "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it",
+      "patch": [
+          {"op": "remove", "path": ""},
+          {"op": "remove", "path": ""},
+      ],
+      "error": "The spec says the target location must exist, so removing a null document fails"
+    },
+
     { "comment": "test replace with missing parent key should fail",
       "doc": {"bar": "baz"},
       "patch": [{"op": "replace", "path": "/foo/bar", "value": false}],
@@ -261,10 +310,16 @@
       "patch": [{"op": "test", "path": "/foo", "value": [1, 2]}],
       "error": "test op should fail" },
 
-    { "comment": "Whole document",
+    { "comment": "Test the whole document",
       "doc": { "foo": 1 },
       "patch": [{"op": "test", "path": "", "value": {"foo": 1}}],
-      "disabled": true },
+      "expected": { "foo": 1 } },
+
+    { "comment": "Test the whole document, no match",
+      "doc": { "foo": 1 },
+      "patch": [{"op": "test", "path": "", "value": {"foo": 2}}],
+      "expected": { "foo": 1 },
+      "error": "Tested value does not match original doc" },
 
     { "comment": "Empty-string element",
       "doc": { "": 1 },
@@ -438,13 +493,13 @@
       "patch": [ { "op": "move", "from": "/bar", "path": "/foo" } ],
       "error": "missing 'from' location" },
 
-    { "comment": "duplicate ops",
+    { "comment": "duplicate ops, json-c parses this as op:move",
       "doc": { "foo": "bar" },
       "patch": [ { "op": "add", "path": "/baz", "value": "qux",
                    "op": "move", "from":"/foo" } ],
-      "error": "patch has two 'op' members",
-      "disabled_in_json_c": true,
-      "disabled": true },
+      "error_wont_happen_in_jsonc": "patch has two 'op' members",
+      "expected": { "baz": "bar" }
+      },
 
     { "comment": "unrecognized op should fail",
       "doc": {"foo": 1},

tests/test_json_patch.c

@@ -21,9 +21,9 @@ void test_json_patch_op(struct json_object *jo)
 	const char *comment = json_object_get_string(json_object_object_get(jo, "comment"));
 	struct json_object *doc = json_object_object_get(jo, "doc");
 	struct json_object *patch = json_object_object_get(jo, "patch");
-	struct json_object *expected = json_object_object_get(jo, "expected");
+	struct json_object *expected = NULL;
+	json_bool have_expected = json_object_object_get_ex(jo, "expected", &expected);
 	struct json_object *error = json_object_object_get(jo, "error");
-	int disabled_test = json_object_get_boolean(json_object_object_get(jo, "disabled_in_json_c"));
 	const char *error_s = json_object_get_string(error);
 	struct json_object *res = NULL;
 	int ret;
@@ -32,13 +32,9 @@ void test_json_patch_op(struct json_object *jo)
 		comment ? comment : error_s,
 		json_object_get_string(doc),
 		json_object_get_string(patch));
-	if (disabled_test) {
-		printf("SKIPPING - disabled in the test spec\n");
-		return;
-	}
-	if (!error && !expected) {
-		printf("SKIPPING - no expected or error conditions in test\n");
-		return;
+	if (!error && !have_expected) {
+		printf("BAD TEST - no expected or error conditions in test: %s\n", json_object_to_json_string(jo));
+		assert(0);
 	}
 	fflush(stdout);
 	struct json_patch_error jperr;
@@ -54,13 +50,13 @@ void test_json_patch_op(struct json_object *jo)
 		if (ret) {
 			fprintf(stderr, "json_patch_apply() returned '%d'\n", ret);
 			fprintf(stderr, "Expected: %s\n", json_object_get_string(expected));
-			fprintf(stderr, "Got: %s\n", json_object_get_string(res));
+			fprintf(stderr, "Got: %s\n", res ? json_object_get_string(res) : "(null)");
 			fprintf(stderr, "json_patch_apply failed: %s at patch idx %zu: %s\n",
 				strerror(jperr.errno_code), jperr.patch_failure_idx, jperr.errmsg);
 			fflush(stderr);
 			assert(0);
 		}
-		assert(res != NULL);
+		// Note: res might be NULL if the whole document was removed
 		assert(jperr.errno_code == 0);
 		ret = json_object_equal(expected, res);
 		if (ret == 0) {
@@ -83,6 +79,7 @@ void test_json_patch_using_file(const char *testdir, const char *filename)
 	(void)snprintf(full_filename, sizeof(full_filename), "%s/%s", testdir, filename);
 	size_t ii;
 
+	printf("Testing using file %s\n", filename);
 	json_object *jo = json_object_from_file(full_filename);
 	if (!jo) {
 		fprintf(stderr, "FAIL: unable to open %s: %s\n", full_filename, strerror(errno));

tests/test_json_patch.expected

@@ -1,3 +1,4 @@
+Testing using file json_patch_spec_tests.json
 Testing '4.1. add with missing object', doc '{ "q": { "bar": 2 } }' patch '[ { "op": "add", "path": "\/a\/b", "value": 1 } ]' : OK
  => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Failed to set value at path referenced by 'path' field
 Testing 'A.1.  Adding an Object Member', doc '{ "foo": "bar" }' patch '[ { "op": "add", "path": "\/baz", "value": "qux" } ]' : OK
@@ -20,6 +21,7 @@ Testing 'A.14. ~ Escape Ordering', doc '{ "\/": 9, "~1": 10 }' patch '[ { "op":
 Testing 'A.15. Comparing Strings and Numbers', doc '{ "\/": 9, "~1": 10 }' patch '[ { "op": "test", "path": "\/~01", "value": "10" } ]' : OK
  => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field
 Testing 'A.16. Adding an Array Value', doc '{ "foo": [ "bar" ] }' patch '[ { "op": "add", "path": "\/foo\/-", "value": [ "abc", "def" ] } ]' : OK
+Testing using file json_patch_tests.json
 Testing 'empty list, empty docs', doc '{ }' patch '[ ]' : OK
 Testing 'empty patch list', doc '{ "foo": 1 }' patch '[ ]' : OK
 Testing 'rearrangements OK?', doc '{ "foo": 1, "bar": 2 }' patch '[ ]' : OK
@@ -69,6 +71,15 @@ Testing '(null)', doc '[ "" ]' patch '[ { "op": "replace", "path": "\/0", "value
 Testing '(null)', doc '[ "" ]' patch '[ { "op": "replace", "path": "\/0", "value": null } ]' : OK
 Testing 'value in array replace not flattened', doc '[ "foo", "sil" ]' patch '[ { "op": "replace", "path": "\/1", "value": [ "bar", "baz" ] } ]' : OK
 Testing 'replace whole document', doc '{ "foo": "bar" }' patch '[ { "op": "replace", "path": "", "value": { "baz": "qux" } } ]' : OK
+Testing 'add whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "add", "path": "", "value": { "baz": "qux" } } ]' : OK
+Testing 'replace whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "replace", "path": "", "value": { "baz": "qux" } } ]' : OK
+ => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 1: Invalid path field
+Testing 'remove whole document', doc '{ "foo": "bar" }' patch '[ { "op": "remove", "path": "" } ]' : OK
+Testing 'remove whole document', doc '{ "foo": "bar" }' patch '[ { "op": "remove", "path": "" } ]' : OK
+Testing 'remove whole document, array', doc '[ "foo", "bar" ]' patch '[ { "op": "remove", "path": "" } ]' : OK
+Testing 'remove whole document, string', doc 'foo' patch '[ { "op": "remove", "path": "" } ]' : OK
+Testing 'remove whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "remove", "path": "" } ]' : OK
+ => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 1: Invalid path field
 Testing 'test replace with missing parent key should fail', doc '{ "bar": "baz" }' patch '[ { "op": "replace", "path": "\/foo\/bar", "value": false } ]' : OK
  => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Did not find element referenced by path field
 Testing 'spurious patch properties', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "\/foo", "value": 1, "spurious": 1 } ]' : OK
@@ -83,7 +94,9 @@ Testing 'test should pass despite (nested) rearrangement', doc '{ "foo": [ { "fo
 Testing 'test should pass - no error', doc '{ "foo": { "bar": [ 1, 2, 5, 4 ] } }' patch '[ { "op": "test", "path": "\/foo", "value": { "bar": [ 1, 2, 5, 4 ] } } ]' : OK
 Testing 'test op should fail', doc '{ "foo": { "bar": [ 1, 2, 5, 4 ] } }' patch '[ { "op": "test", "path": "\/foo", "value": [ 1, 2 ] } ]' : OK
  => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field
-Testing 'Whole document', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 1 } } ]' : SKIPPING - no expected or error conditions in test
+Testing 'Test the whole document', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 1 } } ]' : OK
+Testing 'Test the whole document, no match', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 2 } } ]' : OK
+ => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field
 Testing 'Empty-string element', doc '{ "": 1 }' patch '[ { "op": "test", "path": "\/", "value": 1 } ]' : OK
 Testing '(null)', doc '{ "foo": [ "bar", "baz" ], "": 0, "a\/b": 1, "c%d": 2, "e^f": 3, "g|h": 4, "i\\j": 5, "k\"l": 6, " ": 7, "m~n": 8 }' patch '[ { "op": "test", "path": "\/foo", "value": [ "bar", "baz" ] }, { "op": "test", "path": "\/foo\/0", "value": "bar" }, { "op": "test", "path": "\/", "value": 0 }, { "op": "test", "path": "\/a~1b", "value": 1 }, { "op": "test", "path": "\/c%d", "value": 2 }, { "op": "test", "path": "\/e^f", "value": 3 }, { "op": "test", "path": "\/g|h", "value": 4 }, { "op": "test", "path": "\/i\\j", "value": 5 }, { "op": "test", "path": "\/k\"l", "value": 6 }, { "op": "test", "path": "\/ ", "value": 7 }, { "op": "test", "path": "\/m~0n", "value": 8 } ]' : OK
 Testing 'Move to same location has no effect', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/foo", "path": "\/foo" } ]' : OK
@@ -129,7 +142,7 @@ Testing 'missing from parameter to move', doc '{ "foo": 1 }' patch '[ { "op": "m
  => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 0: Patch does not contain a 'from' field
 Testing 'missing from location to move', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/bar", "path": "\/foo" } ]' : OK
  => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Did not find element referenced by from field
-Testing 'duplicate ops', doc '{ "foo": "bar" }' patch '[ { "op": "move", "path": "\/baz", "value": "qux", "from": "\/foo" } ]' : SKIPPING - disabled in the test spec
+Testing 'duplicate ops, json-c parses this as op:move', doc '{ "foo": "bar" }' patch '[ { "op": "move", "path": "\/baz", "value": "qux", "from": "\/foo" } ]' : OK
 Testing 'unrecognized op should fail', doc '{ "foo": 1 }' patch '[ { "op": "spam", "path": "\/foo", "value": 1 } ]' : OK
  => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 0: Patch object has invalid 'op' field
 Testing 'test with bad array number that has leading zeros', doc '[ "foo", "bar" ]' patch '[ { "op": "test", "path": "\/00", "value": "foo" } ]' : OK