Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.26k stars 4.6k forks source link

Missing critical information about serialization #31862

Open pettys opened 1 year ago

pettys commented 1 year ago

This page should describe or have a link to a page that describes the details of how implementations of ITableEntity are handled by this library. For example:

I've hunted through the documentation at least three separate times for quite a while each time and cannot find the information. In general for any library that serializes/deserialized C# objects, it is critical to have an overview about how the constraints, expectations, and configurability of that serialization work.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

jsquire commented 1 year ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

christothes commented 1 year ago

Thanks for the feedback - This information should definitely be better documented.

In the meantime, here is a sample that demonstrates how to ignore and rename entity properties for during serialization.

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/tables/Azure.Data.Tables/tests/samples/Sample8_CustomizingSerialization.cs

pettys commented 1 year ago

@christothes thanks so much for the comment. The link you sent was helpful. I had looked over all of these samples, searching for the answer:

https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/tables/Azure.Data.Tables/samples

I didn't realize there were more samples here: https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/tables/Azure.Data.Tables/tests/samples

Definitely looks like there's some overlap in those sample sets... seems odd. Also, for what it's worth, this rework of the tables SDK is missing a lot of the serialization flexibility that the previous versions I've used had -- kinda a bummer, but I can work around it.

Anyway, thanks for the quick response, it's very much appreciated!

christothes commented 1 year ago

Thanks, @pettys,

We are aware of the current gaps in serialization customization and have been compiling the most important user scenarios to inform any possible future enhancements. Would you mind describing your current use cases as well as any info you'd be willing to share about your workarounds?

pettys commented 1 year ago

@christothes Thanks for asking and sorry for the delay in the response. I would reiterate that my main request with this issue would be to have the documentation about the serialization more discoverable -- if a developer knows what there is to work with up front, and especially the limitations, then that developer can decide how to build the objects accordingly.

The particular case I was trying to figure out this go around was a basic composition - completely analogous to serializing a Person with an Address property, where Address is class with Line1, Line2, City, State, Zip. When I was digging through the open odata serialization code, I thought maybe the serializer would flatten the composition, which would have been just fine, or maybe the column names would have been PersonId, PersonName, Address.Line1, Address.Line2, etc.

Once I understood the limitations of the serialization from your quick responses (again, thank you for that) I decided to separate the domain/business logic classes from the serialization classes -- that is to say, if I save a Person, the storage code will copy the Person object's data into a separate PersonTableRecord object, which is then tuned to serialize.

I think with a little more control over the serialization, that extra layer and repetitive property-copying code wouldn't be necessary. It just took me a while to decide to bite the bullet and add that in because I couldn't tell what was going to be possible.