Morcatko.AspNetCore.JsonMergePatch icon indicating copy to clipboard operation
Morcatko.AspNetCore.JsonMergePatch copied to clipboard

Cannot add new values to a dictionary

Open jviau opened this issue 3 years ago • 2 comments

Repro steps:

Class:

public class Model
{
    public Dictionary<string, string> Properties { get; set; }
}

Payload:

{
  "properties": {
    "newKey": "newValue"
  }
}

Expected result: The new key is added to the dictionary.

Actual result: An exception is thrown saying the path properties.newKey does not exist.

Cause: JsonMergePatch will process that patch document as a "replace" operation for "newKey". JsonPatchDocument.ApplyTo eventually throws here: https://github.com/dotnet/aspnetcore/blob/main/src/Features/JsonPatch/src/Internal/DictionaryAdapterOfTU.cs#L116.

As per JsonPatch spec, the target location must exist for remove [sic] to be successful

Possible solutions:

  1. Process this operation as an add. + Adheres to JsonPatch spec. // As per JsonPatch spec, if a key already exists, adding should replace the existing value - Doesn't solve the "remove" issue.

  2. Supply a custom IAdapter as part of the call to JsonPatchDocument.ApplyTo, which would permit the replace without the key existing. Can also update remove to allow that to happen without the key existing. This can be done via the overload of JsonPatchDocument.ApplyTo with a custom IObjectAdapter / IAdapterFactory / IAdapter for when the Json contract is a dictionary contract. + Can also make "remove" (ie: { "properties": { "removedKey": null }}) not throw if "removedKey" does not exist. - Violates JsonPatch spec - kind of? The spec is arbitrary here because the customer never defined the JsonPatch operations themselves.

I have the code for option 2 in my project. I can port it to over and make a PR in the next day or two. Option 1 would require more work.

jviau avatar May 06 '21 00:05 jviau

I'm running into the same issue. Has anyone found any other workarounds, or are the two options listed by the OP the best solution?

yelob avatar May 18 '22 22:05 yelob

Hi,

Same issue for me:

Model:


public class Model {
  public IDictionary<string, IDictionary<string, string>> Restrictions { get; set; }
}

Here is workaround:

    //https://github.com/Morcatko/Morcatko.AspNetCore.JsonMergePatch/issues/47
    private static void FixJsonMergePatchForDictionaries(
        JsonMergePatchDocument<TransactionViewCustomization> patch)
    {
        var operationsToModify = patch.Operations.Where(op =>
            op.path.Contains("restrictions")
                && op.OperationType == OperationType.Replace);

        foreach (var op in operationsToModify)
        {
            op.op = "add";
        }
    }

NikiforovAll avatar Jul 09 '22 14:07 NikiforovAll