Aspnetcore: ExpandoObjectAdapter doesn't play well with JObjects

Created on 1 Jan 2018  路  12Comments  路  Source: dotnet/aspnetcore

_From @NicolaasVB on Thursday, September 21, 2017 4:59:13 AM_

Consider following code snippet (unit test)

```C#
var list = new List();
list.Add("[{ 'op': 'add', 'path': '/foo', 'value': {'bar': 'baz'}}]");
list.Add("[{ 'op': 'add', 'path': '/foo/bar', 'value': 'bazz' }]");

        dynamic obj = new ExpandoObject(); //JObject.Parse("{}"); ???

        foreach (var item in list)
        {
            var patchDocument = JsonConvert.DeserializeObject<JsonPatchDocument>(item);
            patchDocument.ApplyTo(obj);

            // without the 2 lines below, the second apply will fail, because in the first patch
            // the value on the ExpandoObject is set to a JObject representing {"bar": "baz"}
            // and ExpandoObjectAdapter can't handle that. It fails when attempting to cast target to IDictionary<string, object>
            JObject intermediate = JObject.FromObject(obj);
            // basically we convert the expandoObject to JObject and back, so that all the JObject values are converted to an ExpandoObject
            obj = intermediate.ToObject<ExpandoObject>();
        }

        Assert.Equal("bazz", obj.foo.bar);

```

Unless we apply the 2 hacky lines (converting to JObject and back to ExpandoObject), this snippet will fail (crash even...). The reason for that is explained in comments in the snippet.

Now, for my question... How I can solve my use case without the use of those 2 hacky lines of code? (we're implementing our take on event sourcing with json patch diffsets, and our replay mechanism applies the json patch documents one by one)

Also on a sidenote, why doesn't a JObjectAdapter exist? I mean we're applying JSON patch documents... It would only seem logical that you can apply them to a JObject :-)

_Copied from original issue: aspnet/JsonPatch#109_

Done area-mvc enhancement help wanted

Most helpful comment

I'm absolutely interested in helping work on this, but for the next month I probably won't have that much time, as my wife is about to give birth to my first kid.
When I find the time though, I'll try to work on it and send a PR.

All 12 comments

_From @NicolaasVB on Thursday, September 21, 2017 5:13:55 AM_

Oh, I just discovered that if we merge the 2 patch documents in 1, it fails as well, for the same reason
C# list.Add("[{ 'op': 'add', 'path': '/foo', 'value': {'bar': 'baz'}}, { 'op': 'replace', 'path': '/foo/bar', 'value': 'bazz' }]");

In this case my dirty hack isn't going to save me!

I'm having a similar issue where trying to apply an operation to a nested JObject results in the exception

JsonPatchException: 'The path segment 'property' is invalid for an array index.'

which is strange since JObject isn't really an array and the generation of the operation is correct (via JsonPatchDocument<T>.Replace). This happens for JsonPatch 2.0.0 and 2.1.0-preview2-final.

Example code:

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void TestMethod1()
    {
        var someDto = new SomeDto()
        {
            Name = "A",
            JObject = new JObject()
        };
        someDto.JObject["id"] = 5;
        someDto.JObject["property"] = "thisisproperty";

        var patch = new JsonPatchDocument<SomeDto>();
        patch.Replace(dto => dto.JObject["property"], "thisisotherproperty"); // generates the correct path
        patch.ApplyTo(someDto); // <- throws JsonPatchException
    }
}

public class SomeDto
{
    public string Name { get; set; }
    public JObject JObject { get; set; }
}

Closing this issue as there was no community involvement for quite a while now. If you are still experiencing the issue, please reopen it agian.

Just because I didn't reply here in a while (you guys didn't bother to reply in the first place!!!) doesn't suddenly make this NOT A BUG

Thanks for getting back, @NicolaasVB. Wasn't sure it's relevant or not. We'll look into this soon.

This would be a good feature to add, but unfortunately we don't have any real support or tests for JObject. Are you interested in helping work on this?

I'm absolutely interested in helping work on this, but for the next month I probably won't have that much time, as my wife is about to give birth to my first kid.
When I find the time though, I'll try to work on it and send a PR.

Moving this to Backlog. It's yours, @NicolaasVB, when you'll find time.

I'm running into this issue today, where I have a POCO containing an object property that contains JObject values. The fix seems to be straightforward enough: just add a JObjectAdapter. I'll give this a go and submit a PR if it works.

Turns out I submitted my PR too soon - although JObject properties are now correctly handled, there's still the issue as described by the OP. Looking into it some more.

Actually, I take that back - it works! I was testing the old code 馃槄

Done. thanks!

Was this page helpful?
0 / 5 - 0 ratings