Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
741 stars 494 forks source link

SDK v3 should support interface to be passed to InsertItemAsync<T>/ReplaceItemAsync<T>/UpsertItemAsync<T> #1266

Closed YohanSciubukgian closed 4 years ago

YohanSciubukgian commented 4 years ago

Describe the bug The scenario where the method InsertItemAsync / ReplaceItemAsync / UpsertItemAsync is called with an interface is working with the SDK v2 but not on the SDK v3.

It's a blocking point to work with a schemaless database and not being able to work with interfaces to insert/replace/upsert documents. We could have a <T> being a List<ICompany> where each ICompany has a list of common properties & each concrete has dedicated properties to the current company.

    public interface ICompany
    {
        string Name { get; set; }
    }

    public class CompanyFoo : ICompany
    {
        public string Name { get; set; }
        public decimal FooValue { get; set; }
    }

    public class CompanyBar : ICompany
    {
        public string Name { get; set; }
        public string BarValue { get; set; }
    }

This is the stacktrace I had using the SDK v3 :

Message: 
    Newtonsoft.Json.JsonSerializationException : Could not create an instance of type CosmosDbConnector.Tests.Dtos.ICompany. Type is an interface or abstract class and cannot be instantiated. Path 'Document[0].Name', line 1, position 21.
  Stack Trace: 
    JsonSerializerInternalReader.CreateNewObject(JsonReader reader, JsonObjectContract objectContract, JsonProperty containerMember, JsonProperty containerProperty, String id, Boolean& createdFromNonDefaultCreator)
    JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    JsonSerializerInternalReader.PopulateList(IList list, JsonReader reader, JsonArrayContract contract, JsonProperty containerProperty, String id)
    JsonSerializerInternalReader.CreateList(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, Object existingValue, String id)
    JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
    JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
    JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
    JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
    JsonSerializer.Deserialize(JsonReader reader, Type objectType)
    JsonSerializer.Deserialize[T](JsonReader reader)
    CosmosJsonDotNetSerializer.FromStream[T](Stream stream)
    CosmosJsonSerializerWrapper.FromStream[T](Stream stream)
    CosmosSerializerCore.FromStream[T](Stream stream)
    CosmosResponseFactory.ToObjectInternal[T](ResponseMessage responseMessage)
    CosmosResponseFactory.<CreateItemResponseAsync>b__5_0[T](ResponseMessage cosmosResponseMessage)
    CosmosResponseFactory.ProcessMessageAsync[T](Task`1 cosmosResponseTask, Func`2 createResponse)
    AzureCosmosDbV3Connector.CreateDocumentAsync[T](String databaseId, String collectionId, DocumentBase`1 item) line 77
    TestCosmosDbConnector.SampleTestV3() line 64
    --- End of stack trace from previous location where exception was thrown ---

To Reproduce I created a repository with a sample usage using the SDK v2 & the SDK v3, it's available here : https://github.com/YohanSciubukgian/CosmosDbConnector

Expected behavior SDK v3 should support interface to be passed to generic type <T> while calling InsertItemAsync (like SDK v2)

Actual behavior An Exception is being raised when InsertItemAsync is being called.

Environment summary Azure Cosmos DB emulator : v2.9.2 Microsoft.Azure.Cosmos : v3.6.0 Microsoft.Azure.DocumentDB.Core : v2.10.1 OS Version: Windows

Additional context I have added the Type declaration DocumentBase<T>on CreateItemAsync/ReplaceItemAsync/UpsertItemAsync method but the exception still occurs. It looks like the type is redundant : image

It looks like (from the stacktrace) that the SDK v3 is trying to create an object based on the generic type for the response. I have added a break point before the Dispose(), on the UnitTests part, in order to analyze the CosmosDB (local emulator) behavior. It looks like the document has been created properly on CosmosDB but the response cannot be processed by the SDK v3. image

j82w commented 4 years ago

The problem is the serialization is based on the type you passed in. It's not possible to deserialize an interface. The concrete type has to be passed into the method. The reason this works in v2 is it's doing a lazy deserialization so you don't get the exception until you try to read the object which your project never does. This optimization should be done to v3, but if your really looking for performance you should use the stream APIs instead.

Solutions 1: Pass the concrete type to the methods:

await _azureCosmosDbV2Connector.CreateDocumentAsync<CompanyBar>(DATABASE_ID_V2, collectionId, document);

Solution 2: Use the stream APIs. Don't serialize the response. Need to create a serializer. This will not throw exceptions for NotFound and other service side exceptions. The typed APIs will throw a CosmosException if the operation failed.

public async Task<bool> CreateDocumentAsync<T>(string databaseId, string collectionId, DocumentBase<T> item)
        {
            var container = _client.GetContainer(databaseId, collectionId);
            PartitionKey pk = new PartitionKey(item.key);
            Stream stream = serializer.ToStream<T>(item);
            using(var response = await container.CreateItemStreamAsync(pk, stream))
            {
                      // Cause it to throw an exception if it is a failure.
                     response.EnsureSuccessStatusCode();
                     return IsResponseValid(response.StatusCode);
            }
        }

Solution 3: This has not been verified, but you can try passing dynamic.

       public async Task<bool> CreateDocumentAsync<T>(string databaseId, string collectionId, DocumentBase<T> item)
        {
            var container = _client.GetContainer(databaseId, collectionId);
           PartitionKey pk = new PartitionKey(item.key);
            var response = await container.CreateItemAsync<dynamic>(pk, item);
            return IsResponseValid(response.StatusCode);
        }
YohanSciubukgian commented 4 years ago

The Solution 1 does not match my current implementation because <T> is a List<ICompany> where each ICompany is a different concret type.

The Solution 2 is half a solution because the project rely today on the current behavior of the SDK (Exceptions included) to ensure that the project is resilient.

The Solution 3 is working thanks ! 👍

Could it be possible to have an option to avoid getting the Document back to the response ?

j82w commented 4 years ago

@YohanSciubukgian that's great. For solution 2 I updated it. There is a method response.EnsureSuccessStatusCode(); that will throw the exception like normal if it is needed.

Cosmos DB is working on supporting this option for fire and forget. It's probably a month or two away from being released.

rasert commented 3 years ago

@j82w inserting as dynamic works. But what about querying after the document is already in the database? How can I make this work?

                /// <summary>
        /// Queries the documents that matches the given predicate
        /// </summary>
        /// <typeparam name="T"></typeparam>
        /// <param name="predicate"></param>
        /// <returns></returns>
        private async Task<IQueryable<T>> QueryAsync<T>(Expression<Func<T, bool>> predicate) where T : class, IEntity
        {
            Container container = _context.GetContainer<T>();
            QueryRequestOptions requestOptions = new()
            {
                MaxItemCount = -1,
                MaxConcurrency = -1
            };

            IQueryable<T> query = container.GetItemLinqQueryable<T>(true, requestOptions: requestOptions);
            if (predicate != null)
            {
                query = query.Where(predicate);
            }
            return await Task.FromResult(query);
        }
j82w commented 3 years ago

The following code should work. If it does not work please create a new issue with the exception you are seeing.

        private async Task<IQueryable<T>> QueryAsync<T>(Expression<Func<T, bool>> predicate) where T : class, IEntity
        {
            Container container = _context.GetContainer<T>();
            QueryRequestOptions requestOptions = new()
            {
                MaxItemCount = -1,
                MaxConcurrency = -1
            };

            // Do not use allowSynchronousQueryExecution. It does blocking calls which cause deadlock and thread starvation
            IQueryable<T> query = container.GetItemLinqQueryable<T>(allowSynchronousQueryExecution: false, requestOptions: requestOptions);
            if (predicate != null)
            {
                query = query.Where(predicate);
            }

            List<T> results = new List<T>();
            using (FeedIterator<T> setIterator = query.ToFeedIterator<T>())
            {
                //Asynchronous query execution
                while (setIterator.HasMoreResults)
                {
                    var response = await setIterator.ReadNextAsync();
                    results.AddRange(response);
                }
            }
            return results;
        }