Elfocrash / Cosmonaut

🌐 A supercharged Azure CosmosDB .NET SDK with ORM support
https://cosmonaut.readthedocs.io
MIT License
342 stars 44 forks source link

CosmosStore.AddAsync with RequestOptions throws RuntimeBinderException when using TypeNameHandling.All #116

Open BjornDeRijcke opened 4 years ago

BjornDeRijcke commented 4 years ago

I'm trying to save a document in the database that has a '$type$' property. I thought about using the Request options parameter of the AddAsync method, however this appears to cause a exception.

Given the Add operation on a store: await store.AddAsync(entity, requestOptions, cancellationToken);

If you pass:

var jsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.All }; requestOptions = new RequestOptions { JsonSerializerSettings = jsonSettings };

An exception is thrown: The best overloaded method match for 'Newtonsoft.Json.Linq.JTokenReader.JTokenReader(Newtonsoft.Json.Linq.JToken)' has some invalid arguments

This happens in the code file: https://github.com/Elfocrash/Cosmonaut/blob/develop/src/Cosmonaut/Extensions/CosmonautHelpers.cs in the method ToCosmonautDocument

The code that is causing the issue is:

var document = JsonConvert.DeserializeObject<dynamic>(JsonConvert.SerializeObject(obj, settings), settings); new JTokenReader(document)

Elfocrash commented 4 years ago

Hey @BjornDeRijcke thanks for raising this.

Can you try something out so I can pinpoint the issue? Does the same thing happen if you set the TypeNameHandling as All on the top level serializer as well?

BjornDeRijcke commented 4 years ago

Yes, the same exception is still thrown. I believe it has something to do with the serialization and deserialization into the dynamic.

When TypeNameHandling is Auto or Full, the resulting object is the class specified by the "$type$" field. When it is None, the resulting object is a JToken.

The JTokenReader only accepts a JToken, so it works when TypeNameHandling is None, but fails when a type is added. (because no constructor exists to accept the dynamic which at runtime is a custom class)

Elfocrash commented 4 years ago

Ah I see. I actually hate that this dynamic conversion is there in the first place. I don't know what I was thinking at the time, but I haven't found a reliable replacement that doesn't break backwards compatibility. Any ideas?

BjornDeRijcke commented 4 years ago

Specifying the generic type as a JToken seems to work for my cases. The resulting object is a JToken.

JsonConvert.DeserializeObject<JToken>(JsonConvert.SerializeObject(entity, settings), settings);

Don't know about all use cases though in light of compatibility.