1
\$\begingroup\$

Background

Recently, I was making some updates to an "older" library that would handle PATCH-style modifications to an object that is persisted in a JSON format on our document-storage databases (e.g., CosmosDB).

I took a fresh approach of this, and started on a blank slate and decided to make use of the DynamicObjectConverter which was introduced back in late 2020 to the System.Text.Json library.

The goal is to handle a PATCH operation to an existing JSON object.

For example from an existing JSON document with:

{ "id": "e001", "name": "foo" } 

with a patch operation of

{ "name": "bar" } 

the result:

{ "id": "e001", "name": "bar" } 

I also wanted to be able to handle adding additional new properties to a collection of existing JSON documents (as a sweeping task) making patch updates across various documents that all have different schemas (hail schema-less DBs!). Such as adding a metadata, or isHidden property to all JSON documents.

The Extension Class

There are 5 methods to the extension class.

This question contains the complete class, you just need to put these 5 methods into a single static class.

DynamicUpdate() Method

The main extension method that extends the IDictionary<string, object type (which is commonly found in our code as ExpandoObject type implementation)

internal static JsonElement DynamicUpdate( this IDictionary<string, object> entity, JsonDocument doc, bool addPropertyIfNotExists = false, bool useTypeValidation = true, JsonDocumentOptions options = default) { if (doc == null) throw new ArgumentNullException(nameof(doc)); if (doc.RootElement.ValueKind != JsonValueKind.Object) throw new NotSupportedException("Only objects are supported."); foreach (JsonProperty jsonProperty in doc.RootElement.EnumerateObject()) { string propertyName = jsonProperty.Name; JsonElement newElement = doc.RootElement.GetProperty(propertyName); bool hasProperty = entity.TryGetValue(propertyName, out object oldValue); // sanity checks JsonElement? oldElement = null; if (oldValue != null) { if (!oldValue.GetType().IsAssignableTo(typeof(JsonElement))) throw new ArgumentException($"Type mismatch. Must be {nameof(JsonElement)}.", nameof(entity)); oldElement = (JsonElement)oldValue; } if (!hasProperty && !addPropertyIfNotExists) continue; entity[propertyName] = GetNewValue( oldElement, newElement, propertyName, addPropertyIfNotExists, useTypeValidation, options); } using JsonDocument finalDoc = JsonDocument.Parse(JsonSerializer.Serialize(entity)); return finalDoc.RootElement.Clone(); } 
GetNewValue() Method

This method gets the value (recursively for object properties) and also deals with validation based on the passed in options as arguments.

private static JsonElement GetNewValue( JsonElement? oldElementNullable, JsonElement newElement, string propertyName, bool addPropertyIfNotExists, bool useTypeValidation, JsonDocumentOptions options) { if (oldElementNullable == null) return newElement.Clone(); JsonElement oldElement = (JsonElement)oldElementNullable; // type validation if (useTypeValidation && !IsValidType(oldElement, newElement)) throw new ArgumentException($"Type mismatch. The property '{propertyName}' must be of type '{oldElement.ValueKind}'.", nameof(newElement)); // recursively go down the tree for objects if (oldElement.ValueKind == JsonValueKind.Object) { string oldJson = oldElement.GetRawText(); string newJson = newElement.ToString(); IDictionary<string, object> entity = JsonSerializer.Deserialize<ExpandoObject>(oldJson); return DynamicUpdate(entity, newJson, addPropertyIfNotExists, useTypeValidation, options); } return newElement.Clone(); } 
IsValidType() Method

This method handles the validation for types. (i.e. trying to replace a string with an int will return false.

private static bool IsValidType(JsonElement oldElement, JsonElement newElement) { if (newElement.ValueKind == JsonValueKind.Null) return true; // 'true' --> 'false' if (oldElement.ValueKind == JsonValueKind.True && newElement.ValueKind == JsonValueKind.False) return true; // 'false' --> 'true' if (oldElement.ValueKind == JsonValueKind.False && newElement.ValueKind == JsonValueKind.True) return true; // type validation return (oldElement.ValueKind == newElement.ValueKind); } 
IsValidJsonPropertyName() Method

This method just a quick way to make sure there isn't a totally malformed property name.

private static bool IsValidJsonPropertyName(string value) { if (string.IsNullOrEmpty(value)) return false; // this is validation for our specific use case (C#) // note that the official docs don't prohibit this though. // https://datatracker.ietf.org/doc/html/rfc7159 for (int i = 0; i < value.Length; i++) { if (char.IsLetterOrDigit(value[i])) continue; switch (value[i]) { case '-': case '_': default: break; } } return true; } 
Overloaded DynamicUpdate() Method

An overloaded method so that passing in a JSON string is also possible for tests, etc.

internal static JsonElement DynamicUpdate( this IDictionary<string, object> entity, string patchJson, bool addPropertyIfNotExists = false, bool useTypeValidation = true, JsonDocumentOptions options = default) { using JsonDocument doc = JsonDocument.Parse(patchJson, options); return DynamicUpdate(entity, doc, addPropertyIfNotExists, useTypeValidation, options); } 

How to use it

Here is a sample snippet to test the extension method.

string original = @"{""foo"":[1,2,3],""parent"":{""childInt"":1},""bar"":""example""}"; string patch = @"{""foo"":[9,8,7],""parent"":{""childInt"":9,""childString"":""woot!""},""bar"":null}"; Console.WriteLine(original); // change this value to see the different types of patching method bool addPropertyIfNotExists = false; ExpandoObject expandoObject = JsonSerializer.Deserialize<ExpandoObject>(original); // patch it! expandoObject.DynamicUpdate(patch, addPropertyIfNotExists); Console.WriteLine(JsonSerializer.Serialize(expandoObject)); 

Note: For the example above, the JSON is a string, but in practice reading in the document comes as some form of a UTF8 binary stream, which is where the JsonDocument shines.

Important Note: Keep in mind that property names are case sensitive, so foo and FoO are unique and valid property names. It would be trivial to add a method to support ignoring case, but in my use-case this is the desired use.

Question for code review

It would be interesting to know if there are any better design patterns that can minimize the back-and-fourth of serializing the inner objects and materializing it as a JsonElement that happens in recursive section of the code found in the GetNewValue() method.

For a large object nested JSON object, there is a lot of packing up and cloning of the JsonElement struct. It's quite uncommon to have very "deep" properties in the wild, but I can't help but wonder if there is a smarter approach to this.

Of course, any other feedback is welcome --- always looking to improve the code!

\$\endgroup\$

    1 Answer 1

    1
    +50
    \$\begingroup\$

    1. Optimize

    1.1 Get rid of clones

    Let's take a look what JsonElement::Clone() does. See JsonElement::Clone(). It calls JsonDocument:CloneElement(int index). Which creates new internal JsonDocument & what's important: it doesn't disposes that document. The reason why you need to call Clone() - it's because you call Dispose() (via using statement). So if we stop calling Dispose(), we don't need to call Clone()

    Well, you still need to call clone on doc.RootElement, because doc is passed outside - but it's only once in DynamicUpdate.

    See diff

    1.2 Get rid of serialize/deserialize newElement

    So, you have next code:

    string newJson = newElement.ToString(); … return DynamicUpdate(entity, newJson, addPropertyIfNotExists, useTypeValidation, options); 

    So, here we creating newJson from existing data (newElement), because DynamicUpdate accept either string or JsonDocument, but not JsonElement. This can be easily fixed, because your overloads actually doesn't really need full JsonDocument to be present.

    We can also notice that after such refactoring we don't need to pass JsonDocumentOptions options everywhere.

    See diff

    1.3 Get rid of serialize/deserialize oldElement

    So, now we have next code

    string oldJson = oldElement.GetRawText(); IDictionary<string, object> entity = JsonSerializer.Deserialize<ExpandoObject>(oldJson); return DynamicUpdate(entity, newElement, addPropertyIfNotExists, useTypeValidation); 

    So, DynamicUpdate requires us to pass ExpandoObject as entity. And what we have - is JsonElement.

    There are no way to get rid of such conversion. But we are doing too much during in our existing implementation - serializing & deserializing. What we need - is to touch our top-level properties only (because nested properties will be touched on other rounds of serializing). Let's write our manual implementation:

    private static ExpandoObject ToExpandoObject(JsonElement jsonElement) { var obj = new ExpandoObject(); foreach (var property in jsonElement.EnumerateObject()) { (obj as IDictionary<string, object>)[property.Name] = property.Value; } return obj; } 

    See diff

    1.4 Get rid of serialize/deserialize entity

    So, you have next lines:

    JsonDocument finalDoc = JsonDocument.Parse(JsonSerializer.Serialize(entity)); return finalDoc.RootElement; 

    Why do we need that? Because entity is IDictionary<string, object> and inside of GetNewValue we return either JsonElement newElement or updated object, which can't be be JsonElement.

    Let's use object as return type so we don't need to convert anything.

    See diff

    2. Refactor

    2.1 Get rid of returning data

    Returning data violates CQS. And in our case — for nothing.

    Each overload of DynamicUpdate returns data. In two cases (when accepting string or JsonDocument) it's not required. In third case (JsonElement) we "need" that because of next contents in GetNewValue:

    return DynamicUpdate(ToExpandoObject(oldElement), newElement, addPropertyIfNotExists, useTypeValidation); 

    Well, but DynamicUpdate modifies ToExpandoObject(oldElement), so we can return it:

    var oldObject = ToExpandoObject(oldElement); DynamicUpdate(oldObject, newElement, addPropertyIfNotExists, useTypeValidation); return oldObject; 

    See diff

    2.2 Introduce DynamicUpdateOptions

    So, you have the next parameters:

    bool addPropertyIfNotExists = false, bool useTypeValidation = true 

    These parameters are options/settings. They should be moved into separate class in order to preserve maintainability. Just imagine what will happen in case if you need to add one more option. This also will reduce the number of parameters from 4-5 to 3-4. See Refactoring: introduce parameter object

    See diff

    2.3 Extract resolving JsonProperty

    Inside of DynamicUpdate you have next lines:

    bool hasProperty = entity.TryGetValue(propertyName, out object oldValue); // sanity checks JsonElement? oldElement = null; if (oldValue != null) { if (!oldValue.GetType().IsAssignableTo(typeof(JsonElement))) throw new ArgumentException($"Type mismatch. Must be {nameof(JsonElement)}.", nameof(entity)); oldElement = (JsonElement)oldValue; } 

    These lines are not good, because they pollute code & violate Single level of abstraction principle

    So we will extract this code to new GetJsonProperty method. In order to do that we also need to introduce var hasProperty = entity.ContainsKey(propertyName); inside of DynamicUpdate - because we don't want to GetJsonProperty to return both object & flag.

    See diff

    2.3.1 Reduce nesting in GetJsonProperty

    We have nested if inside of getting property. This is not good. Now, when we have separate method, we can easily refactor it. Let's apply Refactoring "Replace nested conditional with guard clauses":

    entity.TryGetValue(propertyName, out object oldValue); if (oldValue == null) return null; if (!oldValue.GetType().IsAssignableTo(typeof(JsonElement))) throw new ArgumentException($"Type mismatch. Must be {nameof(JsonElement)}.", nameof(entity)); return (JsonElement)oldValue; 

    See diff

    2.4 Avoid executing redundant code

    So, your code has next nice line:

    if (!hasProperty && !updateOptions.AddPropertyIfNotExists) continue; 

    But this line is executed after resolving newElement and oldElement. Let's fix it!

    See diff

    2.5 Do not expose API over IDictionary<string, object>

    So, you built extension over IDictionary<string, object>. But your code assumes that nested objects will be of type JsonElement. What if we will use your code over Newtonsoft.Json deserializer? What if we will use your code over JsonConverter which performs nested deserialization of ExpandObject (like DynamicJsonConverter in https://www.codetd.com/en/article/8462216)?

    Well, your code will fail in runtime. This is really bad for you.

    Instead public instances of this method should accept either JsonDocument or string and return updated content. As we don't know required parsing options, it's better to accept JsonDocument.

    We will return IDictionary<string, object> from DynamicUpdate. Maybe it's not totally good idea, because ExpandoObject will contain JsonElements inside — and this is not something that we will expect to see in IDictionary. But I'm afraid that we will do too much work in case if we will choose to return JsonDocument or string.

    By this we also made version of method without side-effects - and this is much way better than previous.

    By this we also can simplify contents of GetNewValue.

    See diff

    3. Support more cases

    3.1 Support arrays

    I'm not going to provide code for it. Probably you already know that your implementation doesn't works with arrays properly.

    4. Beautify

    4.1 Naming

    1. DynamicUpdateGetPatched / Patch; and:
      • DynamicUpdateOptionsPatchOptions -updateOptionspatchOptions
    2. entitytoPatch
    3. patchJson, docpatch
    4. convertedEntitypatched
    5. jsonPropertypatchChildProp:
      • Also, newElement is dropped in favor of patchChildProp.Value
    6. hasPropertytoPatchHasProperty
    7. Inside of DynamicUpdate (which is Patch now): oldElementtoPatchChild
    8. GetNewValueGetPatched, because well, it's really the same as our main method
    9. Inside of GetNewValue (which is now GetPatched): oldElementNullabledocumentToPatch, oldElementoriginal, oldElementnewElementpatch`

    See diff

    4.2 var

    Your code contains variables with explicit types even when they are obvious. See:

    JsonDocument doc = JsonDocument.Parse(patch, jsonDocumentOptions); JsonElement oldElement = (JsonElement)toPatch; 

    Some code styles doesn't allow it, but let's leverage var everywhere.

    See diff

    4.3 Comments

    See Best practices for writing code comments

    Inside of your extensions all comments are duplicating code:

    // type validation if (patchOptions.UseTypeValidation && !IsValidType(oldElement, patch)) 

    and

    // recursively go down the tree for objects if (oldElement.ValueKind == JsonValueKind.Object) { return GetPatched(oldElement, patch, patchOptions); } 

    Let's drop it.

    4.4 if/for consistent formatting

    There are 3 styles of formatting inside of your codebase. You should pick one. I suggest to pick next as most typical for C#-codebase:

    if (…) { … } 

    See diff

    4.5 Consistent null-guards

    In some cases you have null-guards, in some not. Let's do

    See diff

    Alternative solutions

    1. Consider implementing JSON Patch
    2. Consider the next idea:
      1. Deserialize json into IDictionary<> in deep manner. See https://www.codetd.com/en/article/8462216.
        IDictionary<> DeserializeDynamicDeep(string json)` { … } 
      2. Write method that merges deeply two IDictionary<>.
        IDictionary<> GetPatched(IDictionary<> toPatch, IDictionary<> patch) { … } 
      3. Write sugar method that allows to work you with json/streams
        public static string GetPatched(string toPatch, string patch) => GetPatched( DeserializeDynamicDeep(toPatch), DeserializeDynamicDeep(patch) ); 

    Final solution

    You can see it on github/CodeReviewPatchJson265739 or directly here.

    Code

    using System; using System.Collections.Generic; using System.Dynamic; using System.Text.Json; namespace CodeReviewPatchJson265739 { public static class JsonExtensions { public static IDictionary<string, object> GetPatched( this JsonDocument toPatch, string patch, PatchOptions patchOptions = default, JsonDocumentOptions jsonDocumentOptions = default) { if (toPatch == null) { throw new ArgumentNullException(nameof(toPatch)); } if (patch == null) { throw new ArgumentNullException(nameof(patch)); } using var doc = JsonDocument.Parse(patch, jsonDocumentOptions); return GetPatched(toPatch, doc, patchOptions); } public static IDictionary<string, object> GetPatched( this JsonDocument toPatch, JsonDocument patch, PatchOptions patchOptions = default) { if (toPatch == null) { throw new ArgumentNullException(nameof(toPatch)); } if (patch == null) { throw new ArgumentNullException(nameof(patch)); } return GetPatched(toPatch.RootElement.Clone(), patch.RootElement.Clone(), patchOptions); } private static IDictionary<string, object> GetPatched( JsonElement toPatch, JsonElement patch, PatchOptions patchOptions) { var patched = ToExpandoObject(toPatch); Patch(patched, patch, patchOptions); return patched; } private static void Patch( IDictionary<string, object> toPatch, JsonElement patch, PatchOptions patchOptions) { patchOptions ??= new PatchOptions(); if (patch.ValueKind != JsonValueKind.Object) { throw new NotSupportedException("Only objects are supported."); } foreach (var patchChildProp in patch.EnumerateObject()) { var propertyName = patchChildProp.Name; var toPatchHasProperty = toPatch.ContainsKey(propertyName); if (!toPatchHasProperty && !patchOptions.AddPropertyIfNotExists) continue; var toPatchChild = GetJsonProperty(toPatch, propertyName); toPatch[propertyName] = GetPatched( toPatchChild, patchChildProp.Value, propertyName, patchOptions); } } private static JsonElement? GetJsonProperty(IDictionary<string, object> entity, string propertyName) { entity.TryGetValue(propertyName, out var oldValue); if (oldValue == null) { return null; } if (!oldValue.GetType().IsAssignableTo(typeof(JsonElement))) { throw new ArgumentException($"Type mismatch. Must be {nameof(JsonElement)}.", nameof(entity)); } return (JsonElement)oldValue; } private static object GetPatched( JsonElement? toPatch, JsonElement patch, string propertyName, PatchOptions patchOptions) { if (toPatch == null) { return patch; } var oldElement = (JsonElement)toPatch; if (patchOptions.UseTypeValidation && !IsValidType(oldElement, patch)) { throw new ArgumentException($"Type mismatch. The property '{propertyName}' must be of type '{oldElement.ValueKind}'.", nameof(patch)); } if (oldElement.ValueKind == JsonValueKind.Object) { return GetPatched(oldElement, patch, patchOptions); } return patch; } public class PatchOptions { public bool AddPropertyIfNotExists { get; set; } = false; public bool UseTypeValidation { get; set; } = true; } private static ExpandoObject ToExpandoObject(JsonElement jsonElement) { var obj = new ExpandoObject(); foreach (var property in jsonElement.EnumerateObject()) { (obj as IDictionary<string, object>)[property.Name] = property.Value; } return obj; } private static bool IsValidType(JsonElement oldElement, JsonElement newElement) { if (newElement.ValueKind == JsonValueKind.Null) { return true; } if (oldElement.ValueKind == JsonValueKind.True && newElement.ValueKind == JsonValueKind.False) { return true; } if (oldElement.ValueKind == JsonValueKind.False && newElement.ValueKind == JsonValueKind.True) { return true; } return (oldElement.ValueKind == newElement.ValueKind); } } } 

    Usage

    string original = @"{""foo"":[1,2,3],""parent"":{""childInt"":1},""bar"":""example""}"; string patch = @"{""foo"":[9,8,7],""parent"":{""childInt"":9,""childString"":""woot!""},""bar"":null}"; Console.WriteLine(original); // change this value to see the different types of patching method bool addPropertyIfNotExists = false; // patch it! var expandoObject = JsonDocument.Parse(original).GetPatched(patch, new JsonExtensions.PatchOptions { AddPropertyIfNotExists = addPropertyIfNotExists }); Console.WriteLine(JsonSerializer.Serialize(expandoObject)); ```
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.