myquay / jsonpatch Goto Github PK

Now that "add" works according to spec, can you release JsonPatch 2.0.0? If you find that you don't have time to do release management, I will happily help you. I plan on bringing JsonPatch into my current project and have a vested interest in the project's maturation.

In my scenario, ApplyUpdatesTo throws an exception if someone inputs an invalid enum. The returned error message starts like this "Failed to set value at path "/bill_every" while performing "replace" operation" and can be confusing, so I am wanting to return a custom error message. Is there a way to possibly override ApplyUpdatesTo or maybe wrap it in a try catch to create a more generic or larger implementation?

I tried creating a new class that inherits from JsonPatchDocument as shown below:

public class JsonPatchFormatter<TEntity>: JsonPatch.JsonPatchDocument<TEntity> where TEntity : class, new()
{
	public new void ApplyUpdatesTo(TEntity entity)
	{
		try
		{
			base.ApplyUpdatesTo(entity);
		}
		catch (Exception ex)
		{
			throw new Exception("whatever");
		}
		
	}
}

I then tried using the new class like below:

endpoints.MapMethods("/some-resource/{id}", new[] { "PATCH" }, async (Guid id, JsonPatchFormatter<SomeDto> model) =>
{
    var objectToUpdate = repository.GetById(id);
    patchData.ApplyUpdatesTo(objectToUpdate);
	...
});

but then I ran into an issue with the media type formatter as shown below. I think JsonPatchDocument isn't working quite the same like this.
"No MediaTypeFormatter is available to read an object of type 'JsonPatchFormatter`1' from content with media type 'application/json-patch+json'."

Thanks in advance for your time!

When adding a patch operation to a service that has swashbuckle (swagger) setup on it, the example service along with the swagger json definition for the service provides the incorrect definition of what a request should look like... i.e.

{
"operations": [
{
"operation": "add",
"fromPath": "string",
"parsedFromPath": [
{
"name": "string",
"componentType": "string",
"isCollection": true
}
],
"path": "string",
"parsedPath": [
{
"name": "string",
"componentType": "string",
"isCollection": true
}
],
"value": {}
}
],
"hasOperations": true
}

Consider the following FAKE class and instances examples

class A
    {
        public List<B> BList { get; set; }
    }
    class B
    {
        public string Name { get; set; }
    }

patch document looks like this:
{"op":"replace", "path":"/blist/0/name", "value":"plain"}
trying to apply the patch document results
Marvin.JsonPatch.Exceptions.JsonPatchException`1: 'Patch failed: property at location path: /blist/0/name does not exist'

I'm trying to patch the Status field on something, where Status is an Enum. We're using EnumMember on our enum values to specify how the client should see them, e.g.

public Enum Status
{
    [EnumMember(Value="open")]
    Open = 1,

    [EnumMember(Value="in_progress")]
    InProgress = 1,

    [EnumMember(Value="closed")]
    Closed = 1,
}

When we try to patch this status to in_progress it can't map that back to the Status.InProgress.

[
    {
        op: "replace", path: "/status", value: "in_progress"
    }
]

I'm happy to put a PR in for this, but thought I'd get your take on how you'd like to see this implemented first. Thought you might have some preferences about using JsonProperty Description EnumMember etc attributes, things like that.

JsonPatch and more so RFC 6901 (http://tools.ietf.org/html/rfc6901) allows for the use of "-" to denote a position after the last position of an array. This is important as at the moment the library only allows for you to use an integer past the last position, and then it will add onto the end.

The issue is if there is more than one addition to an array, we have to rely on the operations being ran in order. For example.

Existing Array = 0,1,2
New Additions = 3,4

If the operations are ran as 3 then 4. It works fine. If it's run as 4,3. Then the array ends up looking like so 0,1,2,4 (And then it likely errors, I haven't tested this, but I'm unsure what would happen if you called ADD on an array with an index that is within the range already, it may actually end up looking like 0,1,2,3 with no 4 instead).

I'm currently working on implementing a "DiffHelper" where it will output a list of "JsonPatchOperations" that you can then either output via JSON serialization, or move around internally to still use the existing "Patching" operations. But this definitely hinders that capability.

@myquay, when are you planning to release version 2?

What is that Save() method? In ASP.NET Web API, does that mean db.SaveChanges() ?

Let me explain the issue with an example:
Client data being sent to server:

[
  { "op": "replace", "path": "/Name", "value": "Foo"},   
  { "op": "replace", "path": "/SiteMenuItemExclusion", "value": [{"SiteId":"157b2110-b253-462c-86ca-366c43f19c4c"}]},
]

Server side code:

public HttpResponseMessage Patch(Guid menuItemId, JsonPatchDocument<MenuItem> patchData)
        {
                //Validation and other things  
                patchData.ApplyUpdatesTo(menuItemEntity);
                dbContext.SaveChanges();
               //more irrelevant code for this example here
         }

The above will in fact replace the value of the Name property of the entity to "Foo" but instead of replacing the value of the SiteMenuItemExclusion property it will add that SiteId to it.

I also tried:

[
  { "op": "replace", "path": "/Name", "value": "Item with site exclusion 3"},   
  { "op": "remove", "path": "/SiteMenuItemExclusion"},
  { "op": "replace", "path": "/SiteMenuItemExclusion", "value": [{"SiteId":"157b2110-b253-462c-86ca-366c43f19c4c"}]},
]

But that does not seem to work.

We were desperatly looking for a JSON Patch library handling the JsonProperty attribute and it looks like this one does that. The issue is that the NuGet package (even the prerelease one) is very old and does not support this functionality...

Is it possible to publish a new version of the package on NuGet with the latest changes, or is it still under testing?

The Nuspec file specifies a dependency version of Microsoft.AspNet.WebApi.Client 5.2.0 and the csproj has 5.2.3
If you create a project from scratch, add Microsoft.AspNet.WebApi.Client 5.2.0 and then add JsonPatch it will fail to compile saying that JsonPatch references 5.2.3 but the project uses 5.2.0

I'm guessing it has to do with the reference in the csproj being a fully qualified assembly reference (with the version, public key token, etc.) instead of just having the assembly name.

Just noticed a potential issue with the implementation for move. In PathHelper.ConvertValue(), we currently set values by serializing then deserializing the object (in order to convert it to the appropriate type at the destination path):

private static object ConvertValue(object value, Type type)
{
    return JsonConvert.DeserializeObject(JsonConvert.SerializeObject(value), type);
}

This generally works well for add and replace, since value is going to be coming in as a JObject, which is how it gets read in from the stream by Json.NET. It's not likely that value is going to be of the appropriate type (unless the user's entity is working directly with JObject), so we need to convert it, and this serialization/deserialization technique does the trick (an alternative would be to use JToken.ToObject).

However, this conversion is unnecessary and can actually be problematic for move, since value is now going to be a reference to whatever object was inside the from path. I think it makes more sense to just keep the same object reference when assigning the value at the destination path (rather than destroying the old instance at the from path, and newing up a brand new instance at the destination path). Doing otherwise can be surprising, and maybe even problematic (especially if the PATCH is being applied directly against an EF entity class - you could run into JsonSerializationException inside JsonConvert.SerializeObject() due to self-referencing loops, or run into errors in Entity Framework when actually trying to save the entity, because it's doing reference comparisons internally to validate relationships).

My proposed fix would be to change ConvertValue to check if the value is already an instance of the given type, and if so, then just return it - otherwise, keep the existing conversion logic:

private static object ConvertValue(object value, Type type)
{
    if (type.IsInstanceOfType(value))
    {
        return value;
    }

    return JsonConvert.DeserializeObject(JsonConvert.SerializeObject(value), type);
}

Does that sound OK? If so, I'll also add some tests and put up a pull request.

I noticed there's no support yet for the move operation. This would be highly useful when trying to express the operation of moving an array element (though that's not the only use case):

An example target JSON document:

{ "foo": [ "all", "grass", "cows", "eat" ] }

A JSON Patch document:

[
  { "op": "move", "from": "/foo/1", "path": "/foo/3" }
]

The resulting JSON document:

{ "foo": [ "all", "cows", "eat", "grass" ] }

Would you be open to a pull request on this? I can try to take a stab at it, unless this is something you would rather take a look at yourself.

Hi I'm testing out this library, and it looks really cool :)

However I'm having some issues getting the provided example to work.

public void Patch(Guid id, JsonPatchDocument<SomeDto> patchData)
{
    //Remember to do some validation and all that fun stuff
    var objectToUpdate = repository.GetById(id);
    patchData.ApplyUpdatesTo(objectToUpdate);
    repository.Save(objectToUpdate);
}

JsonPatchDocument<SomeDto> patchData implies that it works with DTOs.
But the patchData.ApplyUpdatesTo() takes the same type as patchData which is the DTO.

To my knowledge you don't use DTOs in your repository but domain models. So there's a type mismatch.

Putting some types on the example make the issue obvious :)

public void Patch(Guid id, JsonPatchDocument<FooDto> patchData)
{
    //Remember to do some validation and all that fun stuff
    Foo objectToUpdate = repository.GetById(id);
    patchData.ApplyUpdatesTo(objectToUpdate); // <-- fails: expects a FooDto but was Foo
    repository.Save(objectToUpdate);
}

Am I use it wrong? Or does JsonPatch not support DTOs in this way?

If you do something like this inside your startup Web API config to convert serialized property names to camel case:

// Convert JSON property names to camel case
config.Formatters.JsonFormatter.SerializerSettings.ContractResolver = new CamelCasePropertyNamesContractResolver();

You won't be able to use camel case property names in your patch document (since the paths are case sensitive). That means in order to be able to use this library, you either have to get rid of the camel casing in your API entirely (forcing the clients to use pascal case, which goes against the convention), or force the client to send paths with different casing than what they received (so as to match the original casing on the C# properties). Neither of these options is very appealing...

Would it be possible to make case-sensitivity of paths configurable, or disable it entirely? I noticed there's a pull request for this (#7), but the author decided to close it...

It is not uncommon for JSON objects and properties to be represented in POCOs as Dictionaries, be it Dictionary<string, int> or Dictionary<string, string> or whatever.

Json.Net seamlessly handles parsing JSON to/from dictionaries without any special setup.

JsonPatch however does not support patching POCOs with Dictionaries.

Suppose I have a JSON document:

{
    'dynamicProperties': {
        'field1': 'foo',
        'field2': 'bar'
    }
}

Mapped to the the POCO:

class MyClass
{
    IDictionary<string, string> dynamicProperties {get; set;}
}

And I send the following patch:

[
    {"op": "replace", "path": "/dyncamicProperties/field1", "value": "something else"}
]

The path \"/dyncamicProperties/field1\" is not valid: There is no property named \"field1\" on type Dictionary

For my specific scenario, I am trying to upgrade to the latest version. I am coming from version 1.0.2 which is a bit old to be honest. In my testing, I noticed that is now seems enum case sensitivity is now being enforced. For example, here is a sample class and setup:

//Class setup
public class EnumEntity
{
	public SampleEnum Foo { get; set; }
}

//Enum setup
public enum SampleEnum
{
	FirstEnum,
	SecondEnum,
	ThirdEnum,
}


//Code to patch the enum
var entity = new EnumEntity
{
	Foo = SampleEnum.FirstEnum
};

var newEnumValue = "secondenum";

patchDocument.Replace(nameof(EnumEntity.Foo), newEnumValue);
patchDocument.ApplyUpdatesTo(entity);

So "secondenum" won't work (it throws an exception) but "SecondEnum" does which is the normal/expected use case. I checked the release notes and commits and cannot find much about this. I think the expectation is that case sensitivity should not matter for enums.

@myquay I know what specific parts of the code need tweaked to change this as shown in my screenshot below:

Do you have any concerns if I make this tweak, add some unit tests, and open a pull request?

It'd be nice to have this up on nuget.org. If you are thinking to push this to nuget live feed, I can help writing the scripts to automate the process a little. let me know.

See here: aspnet/Mvc#650 (comment)

From the looks of it, you will have to create an InputFormatter and OutputFormatter, the rest doesn't seem to require changes. You can look at StringOutputFormatter or StreamOutputFormatter as an example for a greedy formatter. There is no sample for an input counterpart, but its rather simple.

Also, as explained here:

While in ASP.NET Web API formatters were bi-directional – single object handling both input and output, in MVC 6 the concept of formatters became uni-directional.

So, the current JsonPatchFormatter should be made into an input formatter. I'm not sure yet if we actually need an output formatter or not - there's nothing special about the response (the spec doesn't say anything about it). Technically, it's actually up to the library user to send the response in their Patch handler method (our work ends at the call to ApplyUpdatesTo). They can just send a 200 OK with the full entity in the response (serialized as JSON or XML, or whatever other serializer was chosen by content negotation), or maybe a 204 No Content.

Currently IDictionary collections are not supported but dictionaries may be very useful for some scenarios to store key-value data. It should work in similar way as IEnumerable collections with some minor differences:

[
   { "op": "add", "path": "/Metadata/FooKey1", "value": "Bar" },  
   { "op": "replace", "path": "/Metadata/FooKey2", "value": "Bar" }, 
   { "op": "remove", "path": "/Metadata/FooKey3" }
]

The minor differences are:

• If there is already a key 'FooKey1' in metadata then it should throw an exception.
• If there is no key 'FooKey2' then it should add a new item.
• If there is no key 'FooKey3' then it should not throw an exception.

The differences are caused by default behavior of Dictionary and probably it's better to keep them as is.

Hello thanks for creating this amazing library.

I'm trying to integrate it into one of my projects, the problem is our models are using the JsonPropertyName attribute so I need to implement my own Resolver to handle it, just like AttributePropertyPathResolver.

But the GetProperty method is marked as internal so I don't know how to implement it, any suggestion?

Declaring a property as IDictionary<K,V> instead of Dictionary<K,V> prevents JsonPatch from treating it as a dictionary.

You can see that behavior by changing the type of Foo in DictionaryEntity.cs to IDictionary<TKey, string> and then running the tests.

A fix for this is changing the logic of IsDictionary in PathComponent.cs to the following:

return typeof (IDictionary).IsAssignableFrom(ComponentType) ||
    typeof(IDictionary<,>).IsAssignableFrom(ComponentType) || (
        ComponentType.IsGenericType && (
        typeof(IDictionary).IsAssignableFrom(ComponentType.GetGenericTypeDefinition()) ||
        typeof(IDictionary<,>).IsAssignableFrom(ComponentType.GetGenericTypeDefinition())
    ));

P.S. Though the above passes all tests, there may be a more optimal way to implement the check.

I am trying to patch a date object on my model, but am getting the error

The JSON value could not be converted to System.Nullable`1[System.DateTimeOffset]

I've installed the nuget, and am running

model.ApplyTo(toUpdate)

With the patch body of

{
        "op": "add",
        "path": "/validatedDate",
        "value": "2023/11/21 22:41:11 +00:00"
    }

Not sure where to go from here!

Add JsonConverter

I'm currently working on implementing "move" for #11 and noticed something about how "add" (and possibly "remove") are implemented. In PathHelper.SetValueFromPath(), as we're traversing the path, we're instantiating objects and creating arrays as we go:

//If we're still traversing the path, make sure we've instantiated objects along the way
var propertyValue = property.GetValue(entity);
var propertyType = property.PropertyType;

if (propertyValue == null)
{
    if (property.PropertyType.IsArray)
    {
        propertyValue = Activator.CreateInstance(property.PropertyType, new object[] { 0 });
    }
    else
    {
        propertyValue = Activator.CreateInstance(property.PropertyType);
    }
}

propertyValue = SetValueFromPath(propertyType, String.Join("/", pathComponents.Skip(1)), propertyValue, value, operationType);
property.SetValue(entity, propertyValue);

I'm not sure that's valid, per the spec. We should only be setting values / adding array entries at the target path (the final destination) - we should not be setting any properties or instantiating arrays for any intermediate parts of the path.

Here's the relevant part of the spec (towards the end of 4.1):

However, the object itself or an array containing it does need to
exist, and it remains an error for that not to be the case.  For
example, an "add" with a target location of "/a/b" starting with this
document:

{ "a": { "foo": 1 } }

is not an error, because "a" exists, and "b" will be added to its
value.  It is an error in this document:

{ "q": { "bar": 2 } }

because "a" does not exist.

Based on the above, I don't think this test case is valid:

[TestMethod]
public void SetValueFromPath_AddToEmptyArray_CreatesArrayAndAddsValue()
{
    //arrange
    var entity = new ComplexEntity { };

    //act
    PathHelper.SetValueFromPath(typeof(ComplexEntity), "/Foo/Foo/0", entity, "Element One", JsonPatchOperationType.add);

    //assert
    Assert.IsNotNull(entity.Foo);
    Assert.IsNotNull(entity.Foo.Foo);
    Assert.AreEqual(1, entity.Foo.Foo.Length);
    Assert.AreEqual("Element One", entity.Foo.Foo[0]);
}

Let me know if I might have misinterpreted the spec or the code (it is late). Assuming I understood everything correctly, should I just go ahead and roll the fix for this into the same pull request as for #11 (would be easier, as I'll be making major changes to the PathHelper class anyway)?.

I am using ASP.NET Web API (System.Web.Http) and have attributes on my models to help validate the data coming in. Here is an example of the one I am using [Range(1, int.MaxValue)]. If I patch a field that has one of these attributes using ApplyUpdatesTo by passing say a negative number like -1. When I check the ModelState.IsValid, this returns True. Is there a way to get this to work with this library? Or is validating the attributes with the data passed in outside the scope of this library?

TEntity generic parameter of JsonPatchDocument class contains constraint new(), but it not used and can be removed.
It allows to apply updates to entities without open parameterless constructor. This kind of entities often used in DDD approach

Consider the following FAKE class and instances examples

using System.Collections.Generic;

 class Program
    {
        class Car
        {
            public ComplexCarPart ComplexParts { get; set; }
        }
        class ComplexCarPart
        {
            public CarPart Parts { get; set; }
        }
        class CarPart
        {
            public List<Property> Properties { get; set; }
        }
        class Property
        {
            public string Name { get; set; }
        }

        static void Main(string[] args)
        {
            Car c = new Car();
            c.ComplexParts = new ComplexCarPart();
            c.ComplexParts.Parts = new CarPart();
            c.ComplexParts.Parts.Properties = new List<Property>() { new Property() { Name = "Unique" } };
        }
    }

patch document looks like this:
{
"op": "replace",
"path": "/complexparts/parts/properties/0",
"value": {
"Name": "plain"
}
}
trying to apply the patch document results
Marvin.JsonPatch.Exceptions.JsonPatchException`1: 'Patch failed: provided path is invalid for array property type at location path: /complexparts/parts/properties/0: position larger than array size'

It looks like the paths on the operations directly correspond to C# expressions. So /User/FirstName refers to User.FirstName. There's another issue that suggests to make the paths case-insensitive, but to me that's only a part of the problem.

For instance, you might have a property called 'UserId' in your code, but mark it with [DataMember(Name="id")] so when UserId goes over the wire it shows up to the consumer as { "id" : 12 } not { "UserId": 12 }. Then if they were to try to PATCH that back in, the consumer would assume the model structure required "id" not "UserId."

Another issue is with Json Serializer settings. If you set your contract resolver to use CamelCasePropertyNamesContractResolver, even "UserId" will serialize and deserialize as "userId" which won't match up exactly with the C# expression.

Anyways, just wanted to bring these scencarios to your attention.