Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot add new values to a dictionary #47

Open
jviau opened this issue May 6, 2021 · 2 comments
Open

Cannot add new values to a dictionary #47

jviau opened this issue May 6, 2021 · 2 comments

Comments

@jviau
Copy link

jviau commented May 6, 2021

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.

@yelob
Copy link

yelob commented May 18, 2022

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?

@NikiforovAll
Copy link

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";
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants