Azure / azure-webjobs-sdk-extensions

Azure WebJobs SDK Extensions
MIT License
344 stars 206 forks source link

The document must have an 'id' property #849

Open gokussx4 opened 1 year ago

gokussx4 commented 1 year ago

When using the CosmosDB single item value binder with Azure Functions, this error will occur even if the document read in is a lowercase 'id' property and the ICosmosDBSerializerFactory implementation is injected per the dev blog here.

Adding a document with the IAsyncCollector works as expected without need of workarounds defined.

I think that removing the restriction of a lower case id field in the encapsulated usage of newtonsoft within the binder code would resolve the issue. As long as the id field is found (case insensitive) after the JObject is formed (from the JObject.FromObject call) then the extension should not care due to the fact that the serializer defined by the ICosmosDBSerializerFactory is responsible for the serialization settings. The underlying CosmosSDK, which the extension is dependent on, would throw an error if the id field is not lowercase. https://github.com/Azure/azure-webjobs-sdk-extensions/blob/0af7d172090d1b698b13ebcaef628177a8c3605f/src/WebJobs.Extensions.CosmosDB/Bindings/CosmosDBItemValueBinder.cs#L120

Repro steps

Provide the steps required to reproduce the problem

  1. Implement an azure function that injects a single document from a Cosmos container using the CosmosDB attribute with Id.
  2. Implement and inject ICosmosDBSerializerFactory with a camelCase CosmosPropertyNamingPolicy.
  3. Do not apply jsonproperty attribute on model of document stored in Cosmos container.
  4. Attempt to change any property on the model (except for the id property).

Expected behavior

The document retrieved and changed with the single item value binder should be able to be read and changed without error.

Actual behavior

The document is found and deserialized but is unable to write the change because a lowercase 'id' field could not be found.

Known workarounds

Apply JsonProperty (newtonstoft) attribute forcing a lowercase 'id' field or set JsonConvert.DefaultSettings static property to camel case settings.

Related information

Provide any related information

Example code Program.cs

using Microsoft.Azure.Functions.Extensions.DependencyInjection;
using Microsoft.Azure.WebJobs.Extensions.CosmosDB;
using Microsoft.Extensions.DependencyInjection;
[assembly: FunctionsStartup(typeof(CosmosDBSerialization.Program))]

namespace CosmosDBSerialization
{
    public class Program : FunctionsStartup
    {
        public override void Configure(IFunctionsHostBuilder builder)
        {
            var services = builder.Services;

            services.AddSingleton<ICosmosDBSerializerFactory, CosmosDBSerializationFactory>();
        }
    }
}

CosmosDBSerializationFactory

using Microsoft.Azure.Cosmos;
using Microsoft.Azure.WebJobs.Extensions.CosmosDB;

namespace CosmosDBSerialization
{
    public class CosmosDBSerializationFactory : ICosmosDBSerializerFactory
    {
        public CosmosSerializer CreateSerializer()
        {
            var options = new CosmosSerializationOptions
            {
                PropertyNamingPolicy = CosmosPropertyNamingPolicy.CamelCase,
            };

            return new CosmosNewtonsoftSerializer(options);
        }
    }
}

Function

using System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Azure.WebJobs;
using Microsoft.Azure.WebJobs.Extensions.Http;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using Microsoft.Azure.Cosmos;

namespace CosmosDBSerialization
{
    public class Function1
    {
        private readonly ILogger<Function1> logger;

        public Function1(ILogger<Function1> logger)
        {
            this.logger = logger;
        }

        [FunctionName("CreateTeacher")]
        public async Task<IActionResult> Run(
            [HttpTrigger(AuthorizationLevel.Function, "post", Route = "teachers")] HttpRequest req,
            [CosmosDB("school", "teachers", CreateIfNotExists = true, PartitionKey = "/State")]IAsyncCollector<Teacher> asyncCollector)
        {
            logger.LogInformation("C# HTTP trigger function processed a request.");

            string requestBody = await new StreamReader(req.Body).ReadToEndAsync();
            var data = JsonConvert.DeserializeObject<Teacher>(requestBody);

            string responseMessage = string.IsNullOrEmpty(data.Name)
                ? "This HTTP triggered function executed successfully. Pass a name in the query string or in the request body for a personalized response."
                : $"Hello, {data.Name} - {data.Id}. This HTTP triggered function executed successfully.";

            await asyncCollector.AddAsync(data);

            return new OkObjectResult(responseMessage);
        }

        [FunctionName("UpdateTeacher")]
        public async Task<IActionResult> UpdateTeacherAsync(
            [HttpTrigger(AuthorizationLevel.Function, "post", Route = "teachers/{id}")] HttpRequest req,
            string id,
            [CosmosDB("school","teachers", Id = "{id}", PartitionKey = "FL")] Teacher teacher)
        {
            string requestBody = await new StreamReader(req.Body).ReadToEndAsync();
            var data = JsonConvert.DeserializeObject<Teacher>(requestBody);

            logger.LogInformation("Found {teacher} and adding {moreYears} years of service", teacher.Name, data.YearsOfService);

            teacher.YearsOfService += data.YearsOfService;

            return new OkObjectResult($"{teacher.Name} now has {teacher.YearsOfService} years of service");
        }
    }
}
bhagyshricompany commented 1 year ago

Thanks for informing will check and update you .

ealsur commented 1 year ago

@gokussx4 Thanks for the detailed report. Some clarifying questions:

The error check that is happening is ensuring that the payload has id for the save operation, which is required by the Service.

Removing the check would make the operation fail anyway but fail after making a network request with an HTTP 400 status code. Which means that even if we remove it, the operation will still fail, not succeed. Simply put, no write operation on a document can be done if the payload has no id (the extension is just catching it instead of failing on a network).

Looking at your expected behavior:

The document retrieved and changed with the single item value binder should be able to be read and changed without error.

This is impossible. Even if the check is removed, if you attempt to persist the change without having an id when the model is serialized, it will be rejected by the service.