From 931493159279633287ba77ee365589a6f77ccd7f Mon Sep 17 00:00:00 2001 From: Gene Gleyzer Date: Sun, 13 Oct 2024 12:05:49 -0400 Subject: [PATCH 1/3] Reformat docs; minor fixes --- lib_json/src/main/x/json/JsonArrayBuilder.x | 16 ++-- lib_json/src/main/x/json/JsonBuilder.x | 77 ++++++++++---------- lib_json/src/main/x/json/JsonMergePatch.x | 17 ++--- lib_json/src/main/x/json/JsonObjectBuilder.x | 10 +-- lib_json/src/main/x/json/JsonPatch.x | 73 ++++++++++--------- lib_json/src/main/x/json/JsonPointer.x | 72 +++++++++--------- lib_net/src/main/x/net/Uri.x | 2 +- 7 files changed, 133 insertions(+), 134 deletions(-) diff --git a/lib_json/src/main/x/json/JsonArrayBuilder.x b/lib_json/src/main/x/json/JsonArrayBuilder.x index d5b530c286..e886fc597b 100644 --- a/lib_json/src/main/x/json/JsonArrayBuilder.x +++ b/lib_json/src/main/x/json/JsonArrayBuilder.x @@ -12,13 +12,13 @@ class JsonArrayBuilder /** * Create a JSON array builder. * - * @param template an optional `JsonArray` to use to populate - * the builder with an initial set of values - * @param factory a factory to create a new mutable `JsonArray` + * @param template (optional) an optional `JsonArray` to use to populate the builder with an + * initial set of values + * @param factory (optional) a factory to create a new mutable `JsonArray` */ construct(JsonArray? template = Null, Factory factory = () -> json.newArray()) { this.factory = factory; - values = new Array(); + this.values = new Array(); if (template.is(JsonArray)) { values.addAll(template); } @@ -136,8 +136,8 @@ class JsonArrayBuilder /** * Merge a `JsonObject` into the array. * - * All of the `JsonObject` keys must be strings that are integer literals - * in the range between zero and the current size of the array being built. + * All of the `JsonObject` keys must be strings that are integer literals in the range between + * zero and the current size of the array being built. */ @Override protected void mergeObject(JsonObject o) { @@ -147,8 +147,8 @@ class JsonArrayBuilder Int? index = pointer.index; assert index != Null as "Cannot merge JSON Object with non-Int keys into a JSON array"; assert 0 <= index < values.size as - $|Cannot merge JSON Object into JSON array - key\ - | "{entry.key}" does not match an existing array entry in the range 0..<{values.size} + $|Cannot merge JSON Object into JSON array - key "{entry.key}" does not match \ + |an existing array entry in the range 0..<{values.size} ; map.put(pointer, entry.value); } diff --git a/lib_json/src/main/x/json/JsonBuilder.x b/lib_json/src/main/x/json/JsonBuilder.x index a8e396c200..573490c676 100644 --- a/lib_json/src/main/x/json/JsonBuilder.x +++ b/lib_json/src/main/x/json/JsonBuilder.x @@ -22,11 +22,10 @@ class JsonBuilder { @Abstract protected Id id(JsonPointer path); /** - * Merge the specified `Doc` into the entry in this builder - * with the specified `Id`. + * Merge the specified `Doc` into the entry in this builder with the specified `Id`. * - * The exact behaviour and merge rules will differ depending on the - * type of JSON value the builder builds. + * The exact behaviour and merge rules will differ depending on the type of JSON value the + * builder builds. */ @Abstract protected void merge(Id id, Doc doc); @@ -41,30 +40,29 @@ class JsonBuilder { /** * Return the `Doc` value the builder contains for the specified `Id`. * - * As a `Doc` type can be `Null`, if there is no entry contained in the - * builder for the specified `Id` then a `Null` value will be returned, - * which is a valid `Doc` value. + * As a `Doc` type can be `Null`, if there is no entry contained in the builder for the + * specified `Id` then a `Null` value will be returned, which is a valid `Doc` value. */ @Abstract protected Doc get(Id id); /** - * Perform a deep merge of the specified JSON structure with - * the JSON structure this builder is building. + * Perform a deep merge of the specified JSON structure with the JSON structure this builder is + * building. * - * How the merge is performed will differ depending on the type of - * structure passed in and the structure being built. + * How the merge is performed will differ depending on the type of structure passed in and the + * structure being built. * - * @param the `JsonStruct` to merge + * @param jsonStruct the `JsonStruct` to merge * * @return this `JsonBuilder` */ - JsonBuilder deepMerge(JsonStruct s) { - switch (s.is(_)) { + JsonBuilder deepMerge(JsonStruct jsonStruct) { + switch (jsonStruct.is(_)) { case JsonObject: - mergeObject(s); + mergeObject(jsonStruct); break; case JsonArray: - mergeArray(s); + mergeArray(jsonStruct); break; default: assert; @@ -104,13 +102,12 @@ class JsonBuilder { } /** - * Deeply merge the entries in a `JsonObject` into the JSON value being - * produced by this builder. + * Deeply merge the entries in a `JsonObject` into the JSON value being produced by this builder. * - * @param o the `JsonObject` to merge + * @param jsonObj the `JsonObject` to merge */ - protected void mergeObject(JsonObject o) { - for (Map.Entry entry : o.entries) { + protected void mergeObject(JsonObject jsonObj) { + for (Map.Entry entry : jsonObj.entries) { deepMerge(JsonPointer.from(entry.key), entry.value); } } @@ -118,14 +115,14 @@ class JsonBuilder { /** * Deeply merge a `JsonObject` value into this builder. * - * @param p the `JsonObject` to merge + * @param obj the `JsonObject` to merge * @param path the path to the location the object value should be merged into * @param doc the value to merge the object into * @param id the id of the entry being merged into */ - protected void mergeObjectMember(JsonObject o, JsonPointer path, Doc doc, Id id) { + protected void mergeObjectMember(JsonObject obj, JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; - JsonObject updated = new JsonObjectBuilder(o).deepMerge(remainder, doc).build(); + JsonObject updated = new JsonObjectBuilder(obj).deepMerge(remainder, doc).build(); update(id, updated); } @@ -133,37 +130,37 @@ class JsonBuilder { * Deeply merge the entries in a `JsonArray` into the JSON value being * produced by this builder. * - * @param a the `JsonArray` to merge + * @param array the `JsonArray` to merge */ - protected void mergeArray(JsonArray a) { - for (Int i : 0 ..< a.size) { - deepMerge(JsonPointer.from(i.toString()), a[i]); + protected void mergeArray(JsonArray array) { + for (Int i : 0 ..< array.size) { + deepMerge(JsonPointer.from(i.toString()), array[i]); } } /** * Deeply merge a `JsonArray` value into this builder. * - * @param a the `JsonArray` to merge - * @param path the path to the location the array value should be merged into - * @param doc the value to merge the array into - * @param id the id of the entry being merged into + * @param array the `JsonArray` to merge + * @param path the path to the location the array value should be merged into + * @param doc the value to merge the array into + * @param id the id of the entry being merged into */ - protected void mergeArrayMember(JsonArray a, JsonPointer path, Doc doc, Id id) { + protected void mergeArrayMember(JsonArray array, JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; - JsonArray updated = new JsonArrayBuilder(a).deepMerge(remainder, doc).build(); + JsonArray updated = new JsonArrayBuilder(array).deepMerge(remainder, doc).build(); update(id, updated); } /** - * Deeply merge a `Primitive` value into this builder. + * Deeply merge a `Primitive` json value into this builder. * - * @param p the `Primitive` to merge + * @param obj the `Primitive` value to merge (TODO JK: value is not used!?) * @param path the path to the location the primitive value should be merged into * @param doc the value to merge the primitive into * @param id the id of the entry being merged into */ - protected void mergePrimitiveMember(Primitive p, JsonPointer path, Doc doc, Id id) { + protected void mergePrimitiveMember(Primitive obj, JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; JsonObject updated = new JsonObjectBuilder().deepMerge(remainder, doc).build(); update(id, updated); @@ -205,13 +202,13 @@ class JsonBuilder { /** * Perform a deep copy of the specified JSON array. * - * @param o the JSON array to copy + * @param array the JSON array to copy * * @return a mutable copy of the JSON array */ - static JsonArray deepCopyArray(JsonArray a) { + static JsonArray deepCopyArray(JsonArray array) { JsonArray copy = json.newArray(); - for (Doc doc : a) { + for (Doc doc : array) { copy.add(deepCopy(doc)); } return copy; diff --git a/lib_json/src/main/x/json/JsonMergePatch.x b/lib_json/src/main/x/json/JsonMergePatch.x index dab2054205..bd0fbbeb9f 100644 --- a/lib_json/src/main/x/json/JsonMergePatch.x +++ b/lib_json/src/main/x/json/JsonMergePatch.x @@ -1,13 +1,13 @@ /** - * An implementation of a JSON Merge Patch as specified - * in [RFC7396](http://tools.ietf.org/html/rfc7396). + * An implementation of a JSON Merge Patch as specified in + * [JSON Merge Patch specification](http://tools.ietf.org/html/rfc7396). * * @param patch the JSON value to apply as a merge patch */ class JsonMergePatch(Doc patch) { /** - * @return True iff this patch is empty, i.e. it will not apply any + * `True` iff this patch is empty, i.e. it will not apply to any. */ Boolean empty.get() { Doc patch = this.patch; @@ -21,9 +21,8 @@ class JsonMergePatch(Doc patch) { * Apply this patch to the specified target. * * @param target the JSON value to apply this patch to - * @param inPlace True to modify the target in place (if applicable), or - * False to leave the target unmodified and return a patched - * copy of the target + * @param inPlace (optional) `True` to modify the target in place (if applicable), or `False` + * to leave the target unmodified and return a patched copy of the target * * @return the JSON value resulting from applying this patch to the target */ @@ -51,7 +50,7 @@ class JsonMergePatch(Doc patch) { if (Doc targetValue := target.get(key)) { merge(key, merge(targetValue, value, inPlace)); } else { - merge(key, merge(json.newObject(), value, True)); + merge(key, merge(json.newObject(), value, True)); // TODO JK: why not "inPlace" } } } @@ -75,8 +74,8 @@ class JsonMergePatch(Doc patch) { /** * Generate a JSON Merge Patch from the source and target {@code JsonValue}. * - * @param source the source - * @param target the target + * @param source the source + * @param target the target * * @return a JSON Patch which when applied to the source, yields the target */ diff --git a/lib_json/src/main/x/json/JsonObjectBuilder.x b/lib_json/src/main/x/json/JsonObjectBuilder.x index 8e409d7bda..4ee3febb97 100644 --- a/lib_json/src/main/x/json/JsonObjectBuilder.x +++ b/lib_json/src/main/x/json/JsonObjectBuilder.x @@ -12,9 +12,9 @@ class JsonObjectBuilder /** * Create a JSON object builder. * - * @param template an optional `Map` to use to populate - * the builder with an initial set of values - * @param factory a factory to create a new mutable `JsonArray` + * @param template (optional) an optional `Map` to use to populate the builder + * with an initial set of values + * @param factory (optional) a factory to create a new mutable `JsonArray` */ construct(JsonObject? template = Null, Factory factory = () -> json.newObject()) { this.factory = factory; @@ -63,7 +63,7 @@ class JsonObjectBuilder JsonObjectBuilder add(String key, JsonBuilder builder) = add(key, builder.build()); /** - * Add all the values contained in the `Map` + * Add all the values contained in the `Map`. * * @param map the map of values to add * @@ -75,7 +75,7 @@ class JsonObjectBuilder } /** - * Add all the values contained in the `JsonObject` + * Add all the values contained in the `JsonObject`. * * @param map the map of values to add * diff --git a/lib_json/src/main/x/json/JsonPatch.x b/lib_json/src/main/x/json/JsonPatch.x index 30f8fccb48..ade7178064 100644 --- a/lib_json/src/main/x/json/JsonPatch.x +++ b/lib_json/src/main/x/json/JsonPatch.x @@ -1,15 +1,13 @@ /** * A representation of a JSON patch, as defined - * by https://datatracker.ietf.org/doc/html/rfc6902 + * [JavaScript Object Notation (JSON) Patch specification](http://tools.ietf.org/html/rfc6902). * - * There are various rules that apply to different types of patch operation. - * These rules are not validated until the patch operations is actually applied. - * This means is it is possible to construct an invalid set of patches but an - * exception will be thrown when the patches are applied. + * There are various rules that apply to different types of patch operation. These rules are not + * validated until the patch operations is actually applied, which means that it is possible to + * construct an invalid set of patches but an exception will be thrown when the patches are applied. */ mixin JsonPatch into Array { - /** * Apply this patch to the specified `Doc`. * @@ -17,14 +15,13 @@ mixin JsonPatch * * If the target `Doc` is immutable, the result returned will be immutable. * - * The `options` parameter allows non-standard behaviour to be applied to the patching - * process. The behaviour of the default options will match the RFC6902 specification. + * The `options` parameter allows non-standard behaviour to be applied to the patching process. + * The behaviour of the default options will match the RFC6902 specification. * * @param target the `Doc` to apply the patch to - * @param options the options to control patching behaviour + * @param options (optional) the options to control patching behaviour * - * @return an JSON `Doc` that is the result of applying this patch - * to the target JSON `Doc` + * @return an JSON `Doc` that is the result of applying this patch to the target JSON `Doc` * * @throws IllegalArgument if any of the operations contains an invalid parameter * @throws IllegalState if applying any of the operations fails @@ -67,7 +64,7 @@ mixin JsonPatch // ----- JsonPatch factory methods ------------------------------------------------------------- /** - * Create an immutable `JsonPatch` from an array of `Operation`s. + * Create an immutable `JsonPatch` from an array of `Operation` objects. * * @param ops the operations to add to the `JsonPatch` * @@ -461,13 +458,13 @@ mixin JsonPatch /** * Validate an array index, converting negative indexes into their corresponding positive value. * - * If `options.supportNegativeIndices` is `True` negative indexes are allowed. In this case -1 refers - * to the last element in the array, -2 the second to last, and so on up to -(array.size - 1) which - * refers to the element at index zero. + * If `options.supportNegativeIndices` is `True` negative indexes are allowed. In this case -1 + * refers to the last element in the array, -2 the second to last, and so on up to + * -(array.size - 1) which refers to the element at index zero. * - * - If the index is zero or positive, it must be in the range 0 ..< array.size - * - If the index is negative, `options.supportNegativeIndices` must be `True` and the index - * must be in the range -array.size .. -1 + * If the index is zero or positive, it must be in the range 0 ..< array.size. + * If the index is negative, `options.supportNegativeIndices` must be `True` and the index must + * be in the range -array.size .. -1. * * @param op the `Action` being executed * @param index the index to validate @@ -484,25 +481,26 @@ mixin JsonPatch if (index > 0) { // invalid positive index - assert:arg as $"Cannot perform {op} on JSON array, index {index} out of bounds, valid range 0 ..< {array.size}"; + assert:arg as $|Cannot perform {op} on JSON array, index {index} out of bounds, valid \ + |range 0 ..< {array.size} + ; } // index is negative if (options.supportNegativeIndices) { - // We allow negative indexes, which means the index counts - // from the end of the array, so valid values are [-array.size >.. 0] - // We already know we are not zero + // We allow negative indexes, which means the index counts from the end of the array, + // so valid values are [-array.size >.. 0] (we already know the value is not zero). if (index < -array.size) { - // the index is too negative + // invalid negative index assert:arg as $|Cannot perform {op} on JSON array, negative array index \ - | {index} out of bounds, expected -{array.size} .. -1 + |{index} out of bounds, expected -{array.size} .. -1 ; } return index + array.size; } // negative indexes not allowed assert:arg as $|Cannot perform {op} on JSON array, negative array index {index} not allowed, \ - | valid range 0 ..< {array.size} + |valid range 0 ..< {array.size} ; } @@ -513,8 +511,10 @@ mixin JsonPatch * * @param op the action the operation will perform * @param path the path to the value to apply the action to - * @param value the value the action will apply (ignored for copy, move, or remove operations) - * @param from the path to the "from" value (required for copy or move operations, ignored for other operations) + * @param value (optional) the value the action will apply (ignored for copy, move, or remove + * operations) + * @param from (optional) the path to the "from" value (used for copy or move operations, + * ignored for other operations) */ static const Operation(Action op, JsonPointer path, Doc value = Null, JsonPointer? from = Null) { @@ -624,14 +624,17 @@ mixin JsonPatch * A set of options that can be used to control the patching behaviour. These options typically change the * behavior from the standard specified by https://datatracker.ietf.org/doc/html/rfc6902 * - * @param ensurePathExistsOnAdd a flag to indicate that an add operation should recursively create the missing - * parts of path. For example adding "/foo/bar" if "foo" does not exist a JSON - * object will be added at key "foo" and then "bar" will be added to that object. - * @param allowMissingPathOnRemove a flag to indicate that remove operations should not fail if the target path is - * missing. The default is `False` - * @param supportNegativeIndices support the non-standard use of negative indices for JSON arrays to mean indices - * starting at the end of an array. For example, -1 points to the last element in - * the array. Valid negative indices are -1 ..< -array.size The default is `False` + * @param ensurePathExistsOnAdd (optional) a flag to indicate that an add operation should + * recursively create the missing parts of path. For example, + * adding "/foo/bar" if "foo" does not exist a JSON object + * will be added at key "foo" and then "bar" will be added to + * that object + * @param allowMissingPathOnRemove (optional) a flag to indicate that remove operations should + * not fail if the target path is missing + * @param supportNegativeIndices (optional) support the non-standard use of negative indices + * for JSON arrays to mean indices starting at the end of an + * array. For example, -1 points to the last element in the + * array. Valid negative indices are -1 ..< -array.size */ static const Options(Boolean ensurePathExistsOnAdd = False, Boolean allowMissingPathOnRemove = False, diff --git a/lib_json/src/main/x/json/JsonPointer.x b/lib_json/src/main/x/json/JsonPointer.x index a7f1b38705..115a3e199c 100644 --- a/lib_json/src/main/x/json/JsonPointer.x +++ b/lib_json/src/main/x/json/JsonPointer.x @@ -1,16 +1,16 @@ /** - * This const represents an immutable implementation of a JSON Pointer - * as defined by http://tools.ietf.org/html/rfc6901 + * This const represents an immutable implementation of a JSON Pointer as defined by + * [JavaScript Object Notation (JSON) Pointer specification](http://tools.ietf.org/html/rfc6901). * - * A JSON Pointer, when applied to a target JSON `Doc`, defines a reference location in the target `Doc`. - * An empty JSON Pointer defines a reference to the target itself. + * A JSON Pointer, when applied to a target JSON `Doc`, defines a reference location in the target + * `Doc`. An empty JSON Pointer defines a reference to the target itself. */ const JsonPointer { /** * Private `JsonPointer` constructor. - * Pointer instances are created via the static `from` method, which - * will properly validate the pointer string and create the correct - * pointer chain. + * + * Pointer instances are created via the static `from` method, which will properly validate the + * pointer string and create the correct pointer chain. */ private construct(String pointer, String key = "", JsonPointer? remainder = Null) { if (key == "-") { @@ -72,11 +72,10 @@ const JsonPointer { Boolean isLeaf; /** - * If this pointer represents an array index, then return - * the index value, otherwise return `Null`. + * If this pointer represents an array index, return the index value, otherwise `Null`. * - * To be a valid array index, the key must be a numeric string - * representing a non-negative `Int` value. + * To be a valid array index, the key must be a numeric string representing a non-negative + * `Int` value. */ @Lazy Int? index.calc() { if (key == AppendKey) { @@ -91,8 +90,8 @@ const JsonPointer { } /** - * A leaf `JsonPointer` that represents the path value to indicate appending to - * the end of a JSON array. + * A leaf `JsonPointer` that represents the path value to indicate appending to the end of a + * JSON array. */ static JsonPointer Append = from("/-"); @@ -102,8 +101,7 @@ const JsonPointer { static String AppendKey = "-"; /** - * Create a `JsonPointer` from a `String` representation of a JSON Pointer as defined - * by by http://tools.ietf.org/html/rfc6901 + * Create a `JsonPointer` from a `String` representation of a JSON Pointer. * * If the JSON Pointer string is non-empty, it must be a sequence of '/' prefixed tokens, * and the target must either be a JSON Array, or a JSON Object. @@ -114,8 +112,7 @@ const JsonPointer { */ static JsonPointer from(String pointer) { // The JSON pointer spec requires all paths to start with a "/". - // Rather than throw an exception, we just ensure that there is always - // a leading "/" + // Rather than throw an exception, we just ensure that there is always a leading "/" if (pointer.size == 0 || pointer[0] != '/') { pointer = "/" + pointer; } @@ -134,23 +131,23 @@ const JsonPointer { } /** - * Determine whether this `JsonPointer` is equivalent to, or is a parent - * of the specified `JsonPointer`. + * Determine whether this `JsonPointer` is equivalent to, or is a parent of the specified + * `JsonPointer`. * - * @param p the `JsonPointer` that may be a child of this `JsonPointer` + * @param pointer the `JsonPointer` that may be a child of this `JsonPointer` * - * @returns `True` iff this `JsonPointer` is equivalent to, or is a parent - * of the specified `JsonPointer` + * @returns `True` iff this `JsonPointer` is equivalent to, or is a parent of the specified + * `JsonPointer` */ - Boolean isParent(JsonPointer p) { + Boolean isParent(JsonPointer pointer) { if (isEmpty) { return True; } - if (this.key != p.key) { + if (this.key != pointer.key) { return False; } JsonPointer? remainderThis = this.remainder; - JsonPointer? remainderOther = p.remainder; + JsonPointer? remainderOther = pointer.remainder; return switch (remainderThis.is(_), remainderOther.is(_)) { case (Null, Null): True; case (Null, JsonPointer): True; @@ -161,16 +158,17 @@ const JsonPointer { } /** - * Obtain the value from the specified JSON `Doc` at the location - * pointed to by this `JsonPointer`. + * Obtain the value from the specified JSON `Doc` at the location pointed to by this + * `JsonPointer`. * - * @param doc the JSON `Doc` to obtain the value from - * @param allowNegativeIndices support the non-standard use of negative indices for JSON arrays to mean indices - * starting at the end of an array. For example, -1 points to the last element in - * the array. Valid negative indices are -1 ..< -array.size The default is `False` + * @param doc the JSON `Doc` to obtain the value from + * @param supportNegativeIndices (optional) pass `True` to allow non-standard use of negative + * indices for JSON arrays, meaning that indices start at the end + * of an array (e.g.: -1 points to the last element in the array); + * valid negative indices are `-array.size >.. -1` * * @return `True` iff the doc contains a value at the location of this pointer - * @return the JSON value in the doc at the location of this pointer + * @return (conditional) the JSON value in the doc at the location of this pointer */ conditional Doc get(Doc doc, Boolean supportNegativeIndices = False) { JsonPointer? remainder = this.remainder; @@ -204,11 +202,13 @@ const JsonPointer { /** * Ensure the specified index is a positive index into the array * - * @param array the array to check this pointer's index value against + * @param array the array to check this pointer's index value against + * @param supportNegativeIndices pass `True` to allow non-standard use of negative indices * - * @return False if the array is empty, or True if this pointer's index is zero or positive in the - * range 0 ..< array.size, or True if the index is negative in the range -array.size >.. -1 - * @return a valid index into the array + * @return `False` if the array is empty, or `True` if this pointer's index is zero or positive + * in the range `0 ..< array.size`, or if the index is negative in the range + * `-array.size >.. -1` + * @return (optional) a valid index into the array */ conditional Int getValidIndex(JsonArray array, Boolean supportNegativeIndices) { Int? index = this.index; diff --git a/lib_net/src/main/x/net/Uri.x b/lib_net/src/main/x/net/Uri.x index 044823dbe3..7826f348f9 100644 --- a/lib_net/src/main/x/net/Uri.x +++ b/lib_net/src/main/x/net/Uri.x @@ -1,7 +1,7 @@ /** * A representation of a Uniform Resource Identifier (URI) reference. * - * @see: https://www.ietf.org/rfc/rfc2396.txt + * @see https://www.ietf.org/rfc/rfc2396.txt */ const Uri implements Destringable { From c8d36e9a27e9539a0c771fe607664fcde82f4ca4 Mon Sep 17 00:00:00 2001 From: Gene Gleyzer Date: Tue, 15 Oct 2024 09:55:35 -0400 Subject: [PATCH 2/3] Post review changes --- lib_json/src/main/x/json/JsonMergePatch.x | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib_json/src/main/x/json/JsonMergePatch.x b/lib_json/src/main/x/json/JsonMergePatch.x index bd0fbbeb9f..3e14c52cd8 100644 --- a/lib_json/src/main/x/json/JsonMergePatch.x +++ b/lib_json/src/main/x/json/JsonMergePatch.x @@ -27,7 +27,7 @@ class JsonMergePatch(Doc patch) { * @return the JSON value resulting from applying this patch to the target */ Doc apply(Doc target, Boolean inPlace = False) { - return merge(target, patch); + return merge(target, patch, inPlace); } private Doc merge(Doc doc, Doc patch, Boolean inPlace = False) { @@ -50,7 +50,8 @@ class JsonMergePatch(Doc patch) { if (Doc targetValue := target.get(key)) { merge(key, merge(targetValue, value, inPlace)); } else { - merge(key, merge(json.newObject(), value, True)); // TODO JK: why not "inPlace" + // merging the value into a new JsonObject (hence inPlace is `True`) + merge(key, merge(json.newObject(), value, True)); } } } @@ -72,7 +73,7 @@ class JsonMergePatch(Doc patch) { } /** - * Generate a JSON Merge Patch from the source and target {@code JsonValue}. + * Generate a JSON Merge Patch from the source and target `JsonValue`. * * @param source the source * @param target the target From adba7d60dd8abd1cfa7bea20ed50ecbe95e7d292 Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Wed, 23 Oct 2024 20:45:11 +0300 Subject: [PATCH 3/3] Cleaning up JsonBuilder.x and JsonMergePatch.x and adding tests --- lib_json/src/main/x/json/JsonBuilder.x | 38 +-- lib_json/src/main/x/json/JsonMergePatch.x | 75 ++++- .../main/x/json/json_test/JsonBuilderTest.x | 46 ++- .../x/json/json_test/JsonMergePatchTest.x | 316 +++++++++++++++++- 4 files changed, 435 insertions(+), 40 deletions(-) diff --git a/lib_json/src/main/x/json/JsonBuilder.x b/lib_json/src/main/x/json/JsonBuilder.x index 573490c676..8f0ea105c2 100644 --- a/lib_json/src/main/x/json/JsonBuilder.x +++ b/lib_json/src/main/x/json/JsonBuilder.x @@ -86,13 +86,14 @@ class JsonBuilder { Doc existing = get(id); switch (existing.is(_)) { case JsonObject: - mergeObjectMember(existing, path, doc, id); + mergeIntoObjectMember(existing, path, doc, id); break; case JsonArray: - mergeArrayMember(existing, path, doc, id); + mergeIntoArrayMember(existing, path, doc, id); break; case Primitive: - mergePrimitiveMember(existing, path, doc, id); + // existing is a primitive so it cannot be merged into and is instead replaced + replaceMember(path, doc, id); break; default: assert; @@ -113,14 +114,14 @@ class JsonBuilder { } /** - * Deeply merge a `JsonObject` value into this builder. + * Deeply merge a JSON value into a `JsonObject` in this builder. * - * @param obj the `JsonObject` to merge - * @param path the path to the location the object value should be merged into - * @param doc the value to merge the object into + * @param obj the `JsonObject` to merge into + * @param path the path to the location the `Doc` value should be merged into + * @param doc the JSON value to merge into the `JsonObject` * @param id the id of the entry being merged into */ - protected void mergeObjectMember(JsonObject obj, JsonPointer path, Doc doc, Id id) { + protected void mergeIntoObjectMember(JsonObject obj, JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; JsonObject updated = new JsonObjectBuilder(obj).deepMerge(remainder, doc).build(); update(id, updated); @@ -139,28 +140,27 @@ class JsonBuilder { } /** - * Deeply merge a `JsonArray` value into this builder. + * Deeply merge a JSON value into a `JsonArray` value in this builder. * - * @param array the `JsonArray` to merge - * @param path the path to the location the array value should be merged into - * @param doc the value to merge the array into + * @param array the `JsonArray` to merge into + * @param path the path to the location the JSON value should be merged into + * @param doc the JSON value to merge the into the `JsonArray` * @param id the id of the entry being merged into */ - protected void mergeArrayMember(JsonArray array, JsonPointer path, Doc doc, Id id) { + protected void mergeIntoArrayMember(JsonArray array, JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; JsonArray updated = new JsonArrayBuilder(array).deepMerge(remainder, doc).build(); update(id, updated); } /** - * Deeply merge a `Primitive` json value into this builder. + * Replace a value in this builder with a specified value. * - * @param obj the `Primitive` value to merge (TODO JK: value is not used!?) - * @param path the path to the location the primitive value should be merged into - * @param doc the value to merge the primitive into - * @param id the id of the entry being merged into + * @param path the path to the location of the value to replace + * @param doc the JSON value to replace any existing value with + * @param id the id of the entry being replaced */ - protected void mergePrimitiveMember(Primitive obj, JsonPointer path, Doc doc, Id id) { + protected void replaceMember(JsonPointer path, Doc doc, Id id) { JsonPointer remainder = path.remainder ?: assert; JsonObject updated = new JsonObjectBuilder().deepMerge(remainder, doc).build(); update(id, updated); diff --git a/lib_json/src/main/x/json/JsonMergePatch.x b/lib_json/src/main/x/json/JsonMergePatch.x index 3e14c52cd8..726ad58597 100644 --- a/lib_json/src/main/x/json/JsonMergePatch.x +++ b/lib_json/src/main/x/json/JsonMergePatch.x @@ -30,33 +30,80 @@ class JsonMergePatch(Doc patch) { return merge(target, patch, inPlace); } + /** + * Perform a merge as described by the pseudo code in RFC 7396. + * + * define MergePatch(Target, Patch): + * if Patch is an Object: + * if Target is not an Object: + * Target = {} # Ignore the contents and set it to an empty Object + * for each Name/Value pair in Patch: + * if Value is null: + * if Name exists in Target: + * remove the Name/Value pair from Target + * else: + * Target[Name] = MergePatch(Target[Name], Value) + * return Target + * else: + * return Patch + * + * * If the `patch` parameter is not a `JsonObject` the `patch` parameter is returned as the result. + * + * * If the target `Doc` is not a `JsonObject` it is ignored and the merge will be applied to + * a new empty `JsonObject`. + * + * * If the target `Doc` is a mutable `JsonObject` and the `inPlace` parameter is `True` the merge will be + * applied directly to the target. + * + * * A `Null` value for a key in the `patch` will cause the corresponding entry in the target to be removed. + * Any `Null` value in the `patch` will not appear in the merged result. + * + * @param doc that target JSON value to apply the patch to + * @param patch the JSON value representing the patch to apply + * @param inPlace (optional) `True` to modify the target in place (if applicable), or `False` + * to leave the target unmodified and return a patched copy of the target + * + * @return the JSON value resulting from applying this patch to the target + */ private Doc merge(Doc doc, Doc patch, Boolean inPlace = False) { if (patch.is(JsonObject)) { JsonObject target; - if (doc.is(JsonObject)) { - target = doc; + if (doc.is(immutable) || !inPlace) { + // we can make in place true as we are making a new target so there is + // no point continually copying target elements from here on + inPlace = True; + target = json.newObject(); + target.putAll(doc); + } else { + target = doc; + } } else { - target = json.newObject(); + // we can make in place true as we are making a new target so there is + // no point continually copying target elements from here on + inPlace = True; + target = json.newObject(); } - JsonObjectBuilder builder = new JsonObjectBuilder(target); - for (Map.Entry entry : patch.entries) { - String key = entry.key; - Doc value = entry.value; + for ((String key, Doc value) : patch) { if (value == Null) { target.remove(key); } else { - if (Doc targetValue := target.get(key)) { - merge(key, merge(targetValue, value, inPlace)); - } else { - // merging the value into a new JsonObject (hence inPlace is `True`) - merge(key, merge(json.newObject(), value, True)); - } + target[key] = merge(target[key], value, inPlace); } } - return builder.build(); + // TODO JK: + // If the original target is immutable the target being returned will be a copy + // that is currently mutable. Should it be made immutable to match the original + // target doc parameter? + // Basically, should the mutability of the result match the mutability of the + // original doc parameter? + return target; } + // TODO JK: + // Should a copy of the patch be returned and should it be immutable? + // If we do make a copy, should the mutability of the result match the mutability of the + // original doc parameter? return patch; } diff --git a/manualTests/src/main/x/json/json_test/JsonBuilderTest.x b/manualTests/src/main/x/json/json_test/JsonBuilderTest.x index 120ca7239c..6295c670e1 100644 --- a/manualTests/src/main/x/json/json_test/JsonBuilderTest.x +++ b/manualTests/src/main/x/json/json_test/JsonBuilderTest.x @@ -76,9 +76,9 @@ class JsonBuilderTest { void assertDeepCopyObject(JsonObject source, JsonObject copy) { assert © != &source as "source and copy should be different object references"; assert copy.inPlace == True as "copy should be mutable"; - for (Map.Entry entry : source.entries) { - assert Doc copyValue := copy.get(entry.key); - assertDeepCopy(entry.value, copyValue); + for ((String key, Doc value) : source) { + assert Doc copyValue := copy.get(key); + assertDeepCopy(value, copyValue); } } @@ -142,10 +142,12 @@ class JsonBuilderTest { JsonObject objExpected1 = Map:["three"=3, "four"=44, "five"=5]; JsonObject source = Map:["c"=Map:["1"=objUpdate1]]; JsonObject result = new JsonObjectBuilder(target).deepMerge(source).build(); + Doc a = result["a"]; Doc c = result["c"]; + assert a.is(String); + assert a == "b"; assert c.is(Array); assert c == Array:[objOrig0, objExpected1, objOrig2]; - //assert result == Map:["a"="b", "c"=Array:[objOrig0, objExpected1, objOrig2]]; } @Test @@ -178,11 +180,45 @@ class JsonBuilderTest { assert target == Map:["a"="b", "c"=Array:["one", "two", "three"]]; } + // ---- Merge into primitive fields ------------------------------------------------------------ + @Test - void shouldMergeObjectWithPrimitiveIntoObjectWithExistingPrimitive() { + void shouldMergePrimitiveIntoObjectWithExistingPrimitiveField() { JsonObject target = Map:["a"="b", "c"="d"]; JsonObject source = Map:["c"="updated"]; JsonObject result = new JsonObjectBuilder(target).deepMerge(source).build(); assert result == Map:["a"="b", "c"="updated"]; } + + @Test + void shouldMergePrimitiveIntoObjectWithoutExistingPrimitiveField() { + JsonObject target = Map:["a"="b"]; + JsonObject source = Map:["c"="d"]; + JsonObject result = new JsonObjectBuilder(target).deepMerge(source).build(); + assert result == Map:["a"="b", "c"="d"]; + } + + @Test + void shouldMergeObjectIntoObjectWithExistingPrimitiveField() { + JsonObject child = Map:["child-one"="value-one", "child-two"="value-two"]; + JsonObject target = Map:["a"="b", "c"="d"]; + JsonObject source = Map:["c"=child]; + JsonObject result = new JsonObjectBuilder(target).deepMerge(source).build(); + assert result["a"] == "b"; + Doc c = result["c"]; + assert c.is(JsonObject); + assert c == child; + } + + @Test + void shouldMergeArrayIntoObjectWithExistingPrimitiveField() { + JsonArray child = Array:[1, 2, 3]; + JsonObject target = Map:["a"="b", "c"="d"]; + JsonObject source = Map:["c"=child]; + JsonObject result = new JsonObjectBuilder(target).deepMerge(source).build(); + assert result["a"] == "b"; + Doc c = result["c"]; + assert c.is(JsonArray); + assert c == child; + } } \ No newline at end of file diff --git a/manualTests/src/main/x/json/json_test/JsonMergePatchTest.x b/manualTests/src/main/x/json/json_test/JsonMergePatchTest.x index 08376a955d..72976b56b0 100644 --- a/manualTests/src/main/x/json/json_test/JsonMergePatchTest.x +++ b/manualTests/src/main/x/json/json_test/JsonMergePatchTest.x @@ -1,14 +1,326 @@ +import json.Doc; +import json.JsonArray; import json.JsonMergePatch; import json.JsonObject; +import json.Primitive; class JsonMergePatchTest { @Test - void shouldBeEmpty() { + void shouldCreateAnEmptyPatch() { JsonObject o = json.newObject(); JsonMergePatch patch = new JsonMergePatch(o); assert patch.empty; } - + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"b"} {"a":"c"} {"a":"c"} + */ + @Test + void shouldMergeReplacingSingleKeyValue() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"="c"]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"="c"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"b"} {"b":"c"} {"a":"b", + * "b":"c"} + */ + @Test + void shouldMergeAddingNewEntry() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch(Map:["b"="c"]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"="b", "b"="c"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"b"} {"a":null} {} + */ + @Test + void shouldMergeRemovingOnlyEntry() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=Null]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result.empty; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"b", {"a":null} {"b":"c"} + * "b":"c"} + */ + @Test + void shouldMergeRemovingEntry() { + JsonObject original = Map:["a"="b", "b"="c"]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=Null]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["b"="c"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":["b"]} {"a":"c"} {"a":"c"} + */ + @Test + void shouldMergeReplacingArrayWithPrimitive() { + JsonArray array = Array:["b"]; + JsonObject original = Map:["a"=array]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"="c"]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"="c"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"c"} {"a":["b"]} {"a":["b"]} + */ + @Test + void shouldMergeReplacingPrimitiveWithArray() { + JsonObject original = Map:["a"="c"]; + JsonArray array = Array:["b"]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=array]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"=array]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a": { {"a": { {"a": { + * "b": "c"} "b": "d", "b": "d" + * } "c": null} } + * } } + */ + @Test + void shouldMergeReplacingChildEntryIgnoringNullValue() { + JsonObject original = Map:["a"=Map:["b"="c"]]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=Map:["b"="d", "c"=Null]]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"=Map:["b"="d"]]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a": [ {"a": [1]} {"a": [1]} + * {"b":"c"} + * ] + * } + */ + @Test + void shouldMergeReplacingChildObjectWithArray() { + JsonObject original = Map:["a"=Map:["b"="c"]]; + JsonArray array = Array:[1]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=array]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"=array]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * ["a","b"] ["c","d"] ["c","d"] + */ + @Test + void shouldMergeWhereTargetAndPatchAreArrays() { + JsonArray original = Array:["a", "b"]; + JsonMergePatch patch = new JsonMergePatch(Array:["c", "d"]); + Doc result = patch.apply(original); + assert result.is(JsonArray); + assert result == Array:["c", "d"]; + } + + @Test + void shouldMergeWhereTargetAndPatchArePrimitives() { + Doc original = "a"; + JsonMergePatch patch = new JsonMergePatch("b"); + Doc result = patch.apply(original); + assert result.is(Primitive); + assert result == "b"; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"b"} ["c"] ["c"] + */ + @Test + void shouldMergeWherePatchIsArray() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch(Array:["c"]); + Doc result = patch.apply(original); + assert result.is(JsonArray); + assert result == Array:["c"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"foo"} null null + */ + @Test + void shouldMergeWherePatchIsNull() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch(Null); + Doc result = patch.apply(original); + assert result.is(Primitive); + assert result == Null; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"a":"foo"} "bar" "bar" + */ + @Test + void shouldMergeWherePatchIsPrimitive() { + JsonObject original = Map:["a"="b"]; + JsonMergePatch patch = new JsonMergePatch("bar"); + Doc result = patch.apply(original); + assert result.is(Primitive); + assert result == "bar"; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {"e":null} {"a":1} {"e":null, + * "a":1} + */ + @Test + void shouldMergeAddingNewEntryWhereTargetContainsNull() { + JsonObject original = Map:["e"=Null]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=1]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["e"=Null, "a"=1]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * [1,2] {"a":"b", {"a":"b"} + * "c":null} + */ + @Test + void shouldMergeWhereTargetIsArrayIgnoringNullValuesInPatch() { + JsonArray original = Array:[1, 2]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"="b", "c"=Null]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result == Map:["a"="b"]; + } + + /** + * Test from the examples in Appendix A of RFC 7396 + * + * ORIGINAL PATCH RESULT + * ------------------------------------------ + * {} {"a": {"a": + * {"bb": {"bb": + * {"ccc": {}}} + * null}}} + */ + @Test + void shouldMergeWhereTargetIsEmptyIgnoringNullValuesInPatch() { + JsonObject original = Map:[]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"=Map:["bb"=Map:["ccc"=Null]]]); + Doc result = patch.apply(original); + assert result.is(JsonObject); + assert result.size == 1; + Doc a = result["a"]; + assert a.is(JsonObject); + assert a.size == 1; + Doc bb = a["bb"]; + assert bb.is(JsonObject); + assert bb.empty; + } + + @Test + void shouldMergeInPlaceWhenTargetIsMutableAndInPlaceIsTrue() { + JsonObject original = json.newObject(); + JsonMergePatch patch = new JsonMergePatch(Map:["a"="b"]); + Doc result = patch.apply(original, True); + assert &result == &original as "source and copy should be the same object reference"; + assert result.is(JsonObject); + assert result == Map:["a"="b"]; + } + + @Test + void shouldMergeMakingCopyWhenTargetIsImmutableAndInPlaceIsTrue() { + JsonObject original = Map:[]; + JsonMergePatch patch = new JsonMergePatch(Map:["a"="b"]); + Doc result = patch.apply(original, True); + assert &result != &original as "source and copy should not be the same object reference"; + assert result.is(JsonObject); + assert result == Map:["a"="b"]; + } + + @Test + void shouldMergeMakingCopyWhenTargetIsMutableAndInPlaceIsFalse() { + JsonObject original = new HashMap(); + JsonMergePatch patch = new JsonMergePatch(Map:["a"="b"]); + Doc result = patch.apply(original, False); + assert &result != &original as "source and copy should not be the same object reference"; + assert result.is(JsonObject); + assert result == Map:["a"="b"]; + } + + @Test + void shouldMergeMakingCopyWhenTargetIsMutableAndInPlaceNotSpecified() { + JsonObject original = new HashMap(); + JsonMergePatch patch = new JsonMergePatch(Map:["a"="b"]); + Doc result = patch.apply(original); + assert &result != &original as "source and copy should not be the same object reference"; + assert result.is(JsonObject); + assert result == Map:["a"="b"]; + } } \ No newline at end of file