OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 158 forks source link

ODataResourceSerializer and ODataResource.SkipPropertyVerification #1258

Open uffelauesen opened 4 months ago

uffelauesen commented 4 months ago

Assemblies affected ASP.NET Core OData 8.x

Describe the bug Not a bug as such, but something that I think is missing...

ODataResource got added this little performance optimization option to skip validation of properties in ODL. We have rather "large" entities (100's of properties) and I would like to try the effect of skipping validation of properties, but it cannot be set early enough.

The Microsoft.AspNetCore.OData.Formatter.Serialization.ODataResourceSerializer CreateResource creates the ODataResource and assigns/sets the Properties. As far as I can see there is no way to set SkipPropertyVerification to true before the Properties are set as the code does this...

            ODataResource resource = new ODataResource
            {
                TypeName = typeName ?? "Edm.Untyped",
                Properties = CreateStructuralPropertyBag(selectExpandNode, resourceContext),
            };

I could override the entire CreateResource method in a derived ODataResourceSerializer, but it seems like a lot of code to duplicate for something that hopefully could be added at to the standard serializer fairly easy?

Expected behavior Some way of getting the SkipPropertyVerification set before Properties getting assigned/set in the serializer.

Screenshots

Additional context

habbes commented 4 months ago

Great question @uffelauesen. Yes, that flag was introduced to help cut the cost of verifying all resources that are written. But there are concerns and questions regarding the best way to surface this setting to the user in ASP.NET Core OData. One consideration would be add or use the existing validation configuration in MessageWriterSettings that are passed to ODataMessageWriter which could then be used to stop the validation. However, this specific validation happens before the resources are passed to the message writer.

We could consider moving the validation out of the resources and into the message writer (and reader), but that means people can potentially create and operate on invalid resources. I would also be a potential breaking change and would require more code to be written and changed in the library,

We could also remove the validation all together and allow resource properties to contain resource values. But that's a separate discussion.

My suggestion would be for us to enable this flag in our built-in ODataResourceSerializer. Since we have control of what the serializer does, we can ensure by construction that it doesn't produce invalid properties. Disabling validation in the ODataResourceSerializer for this scenario would improve performance for the vast majority of scenarios out of the box. If there's a concern that this could break some scenarios, we could make the change it in 9.x preview first.

What are your thoughts @gathogojr @xuzhg @mikepizzo @ElizabethOkerio @corranrogue9?