Azure / azure-cosmos-dotnet-v2

Contains samples and utilities relating to the Azure Cosmos DB .NET SDK
MIT License
577 stars 837 forks source link

LINQ not honoring JsonSerializerSettings when specified in constructor #351

Open PeterStephenson opened 7 years ago

PeterStephenson commented 7 years ago

Issue

We require to change the JsonSerializerSettings as we are storing an object where the instance type differs from the declared type (i.e. we need to set TypeNameHandling = TypeNameHandling.Auto)

When setting this through the constructor, the document is written with the $type specified however it does not appear to Deserialize using the setting.

Workaround

We have found that setting the global settings (JsonConvert.DefaultSettings = () => new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto };) makes the setting be used in both cases, which is why we believe that this is an issue with the settings not be universally used in the DocumentDb SDK.

Note: that this workaround is not necessarily desired as we now have to use these settings globally.

Reproduction

The code below (written in LinqPad against the Microsoft.Azure.DocumentDB nuget package version 1.17.0) reproduces the issue.

Commenting out the indicated line will also show that the default settings resolve the problem.

You will need to replace the <DOCUMENTDB URI>, <DOCUMENTDB KEY>, <DATABASE NAME> and <COLLECTION NAME> placeholders with your own document db instance in order to run the script.

The script merely emits a Debug.Assert failure when it doesn't match, no output is expected on success.

void Main()
{
    // Reset the default serializer
    JsonConvert.DefaultSettings = () => new JsonSerializerSettings(); 

    // If you want to see that the default settings work, uncomment this line
    // JsonConvert.DefaultSettings = () => new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto };

    var documentClient = new DocumentClient(
        new Uri("<DOCUMENTDB URI>"),
        "<DOCUMENTDB KEY>",
        new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto },
        new ConnectionPolicy { EnableEndpointDiscovery = false, ConnectionMode = ConnectionMode.Direct });

    var id = Guid.NewGuid().ToString();

    var collectionUri = UriFactory.CreateDocumentCollectionUri("<DATABASE NAME>", "<COLLECTION NAME>");
    var document = new SerializationTest(id, new ItemSubClass());
    documentClient.CreateDocumentAsync(collectionUri, document).Wait();

    var retrievedDocument = (SerializationTest)(dynamic)documentClient.CreateDocumentQuery(collectionUri).Where( d => d.Id == id ).ToList().FirstOrDefault();

    Debug.Assert(retrievedDocument.Item.GetType() == typeof(ItemSubClass), "The retrieved item type doesn't match the subclass");
}

public class SerializationTest 
{
    public string id { get; private set; }

    public Item Item { get; private set; }

    public SerializationTest ( string id, Item item ) 
    {
        this.id = id;   
        this.Item = item;
    }
}

public class Item 
{
}

public class ItemSubClass : Item
{
}
tophallen commented 7 years ago

The serialization settings also do not seem to be honored when generating queries.

If I create a query against a property UserName when I have a CamelCasePropertyNamesContractResolver for the contract resolver in my serialization settings, the query generated without explicitly specifying a JsonPropertyAttribute on the property will be

"query":"SELECT TOP 1 * FROM root WHERE (root[\"UserName\"] = \"user01\") "

Which means I have a workaround - but it's annoying because I have to explicitly specify any property I want to be queried against with a JsonPropertyAttribute. Other aspects are also being ignored such as a custom JsonTypeConverter must be used in an attribute and cannot be specified globally. Defeats the purpose of being able to specify the serialization settings in the constructor IMO.

kirankumarkolli commented 7 years ago

@khdang could you please take a look?

AdrianoRNascimento commented 7 years ago

I am suffering from the exactly same problem described by @tophallen

kirankumarkolli commented 6 years ago

@khdang is this issue addressed in 1.19.1 release?

khdang commented 6 years ago

@PeterStephenson @tophallen @AdrianoRNascimento Thanks for reporting the issue! @kirankumarkolli The 1.19.1 release fixes issue in StoredProcedure and order by queries. However, for JsonSerializerSettings, in general, the support remains limited. There is pending work on making the LINQ query translation to incorporate the serializer settings, which could be widely customed, apart from just property name alias. As @tophallen mentioned, one workaround for property name alias is to use the JsonPropertyAttribute for now.

mattdone01 commented 6 years ago

I am getting intermittent issues with documetnDB not hitting my Json converter that has been declared in the initialization of the DocumentClient _dbClient = new DocumentClient(new Uri(endpointUrl), authorizationKey, new JsonSerializerSettings {TypeNameHandling = TypeNameHandling.None, ContractResolver = new ViewModelContractResolver()}, new ConnectionPolicy { ConnectionMode = ConnectionMode.Direct, ConnectionProtocol = Protocol.Tcp } );

Mostly happens when you retrieve a collection. It only happens sometimes.

lukenuttall commented 6 years ago

I'm having the same problem. It only work if I decorate the property with [JsonProperty("username")], which works, but it's a pain.

kirankumarkolli commented 6 years ago

@mattdone01 , @lukenuttall serializer usage shouldn't be intermittent for the same workload. If possible could you please share the repro?

lukenuttall commented 6 years ago

I'll have to put one together. For me it broke when used in a SelectMany linq statement.

johlrich commented 6 years ago

@khdang I see you replied in December 2017 stating there were fixes with StoredProcedures. I have a sproc that returns an updated document and the results from ExecuteStoredProcedureAsync does try and use the custom serializer settings even if I explicitly state them in the RequestOptions.

Looking at the callstack it appears settings may be passed in based on the signature, but aren't used as I don't see Fable.JsonConverter in there. This is from Core 2.0.0-preview

Exception System.Runtime.Serialization.SerializationException: Failed to deserialize stored procedure response or convert it to type 'AccountServices.Inventory+Projector+ProjectionModel': Unexpected property 'totalValue' found when reading union. Path 'totalValue', line 1, position 34. ---> Newtonsoft.Json.JsonSerializationException: Unexpected property 'totalValue' found when reading union. Path 'totalValue', line 1, position 34. at Newtonsoft.Json.Converters.DiscriminatedUnionConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSerializer serializer) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.ResolvePropertyAndCreatorValues(JsonObjectContract contract, JsonProperty containerProperty, JsonReader reader, Type objectType) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObjectUsingCreatorWithParameters(JsonReader reader, JsonObjectContract contract, JsonProperty containerProperty, ObjectConstructor`1 creator, String id) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue) at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent) at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType) at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings) at Microsoft.Azure.Documents.Client.StoredProcedureResponse`1..ctor(DocumentServiceResponse response, JsonSerializerSettings serializerSettings) --- End of inner exception stack trace --- at Microsoft.Azure.Documents.Client.StoredProcedureResponse`1..ctor(DocumentServiceResponse response, JsonSerializerSettings serializerSettings) at Microsoft.Azure.Documents.Client.DocumentClient.ExecuteStoredProcedurePrivateAsync[TValue](String storedProcedureLink, RequestOptions options, Object[] procedureParams) at Microsoft.Azure.Documents.BackoffRetryUtility`1.<>c__DisplayClass1_0.<b__0>d.MoveNext() --- End of stack trace from previous location where exception was thrown --- ...
johlrich commented 6 years ago

What appears to actually be happening is if I use ExecuteStoredProcedureAsync<T> with Document it will use my serialization settings when I do something like GetPropertyValue<T> on a problematic property + type, but will not when used with other types like ExecuteStoredProcedureAsync<MyResult>. I would expect bother approaches to be valid to be consistent with the other methods.

danejur commented 6 years ago

@khdang Do we have an update on when we might expect to see support for a custom JsonSerializerSettings when executing LINQ queries (as illustrated in the initial post on this issue)? While decorating properties with JsonPropertyAttribute is a functional workaround, it is tacky and definitely not viable long term.

Edit: The issue I meant to reference was the one brought up in @tophallen's post. Sorry about that.

brijeshb commented 6 years ago

It's been nearly two years since the fact that LINQ query generation does not respect json serializer settings has been reported, is there any roadmap to fix this or is has it been dropped?

This little restriction forces us to sprinkle JsonProperty Attributes all over our codebase.

zmarty commented 5 years ago

Any update on this? Even internal MSFT services are affected by this @rnagpal

lundmikkel commented 5 years ago

Is there seriously still no update on this issue? It seems ridiculous to have to override the default settings.

j82w commented 5 years ago

@lundmikkel camel case is supported for linq in the v3 SDK.

lundmikkel commented 5 years ago

I'm not interested in camel case. I'm interested in being able to query my NodaTime values using the same JSON serializer, that I gave in the constructor.

kirankumarkolli commented 5 years ago

CosmosSerializationOptions and Serializer are two exclusive sets. Serializer if specified will still be honored. Can you please validate if 3.2.0-preview2 fixes your issue.

j82w commented 5 years ago

The custom serializer should be honored in linq after this PR is merged for v3.

lundmikkel commented 5 years ago

@kirankumarkolli Okay, I'll have a look later and get back to you.

dhtek commented 4 years ago

Any updates ? We really need this feature: LINQ query generation should respect json serializer settings

JohnLTaylor commented 4 years ago

This is still a problem for us. We use a custom serializer to lowercase the first char when writing and it works great there. LINQ querying doesn't respect the same setting.

There are two places that you have to init the serializer. 1) JsonConvert.DefaultSettings in the Program static constructor 2) DocumentClient constructor