[BUG] serialization of List of polymorphic models does not include the discriminator property

[weidongxu-microsoft]

Describe the bug

Discussion here https://github.com/Azure/azure-sdk-for-java/pull/37789/files#r1402879273

The ChatMessageContentItem class is a polymorphic model. https://github.com/Azure/azure-sdk-for-java/blob/4cfe19f91ce3e42d9929555619e9094500195951/sdk/openai/azure-ai-openai/src/main/java/com/azure/ai/openai/models/ChatMessageContentItem.java

Cause in Jackson https://stackoverflow.com/questions/34193177/why-does-jackson-polymorphic-serialization-not-work-in-lists

The type erasure on Java generic makes Jackson not able to serialize the model correctly.

Candidate solutions:

1. transfer to stream-style-serialization option, it won't solve Jackson, but it likely would correct the serialization as it uses azure-json
2. modify the class, to always include the discriminator property (e.g. kind) in class variable (this be somewhat like having pass-discriminator-to-child-deserialization option; however it got another bug client, model serialized to JSON does not have discriminator property, in option "pass-discriminator-to-child-deserialization" autorest.java#2411)

Exception or Stack Trace
Add the exception log and stack trace if available

To Reproduce
Steps to reproduce the behavior:

Code Snippet
Add the code snippet that causes the issue.

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Setup (please complete the following information):

• OS: [e.g. iOS]
• IDE: [e.g. IntelliJ]
• Library/Libraries: com.azure:azure-core:1.45.0
• Java version: [e.g. 8]
• App Server/Environment: [e.g. Tomcat, WildFly, Azure Function, Apache Spark, Databricks, IDE plugin or anything special]
• Frameworks: [e.g. Spring Boot, Micronaut, Quarkus, etc]

If you suspect a dependency version mismatch (e.g. you see NoClassDefFoundError, NoSuchMethodError or similar), please check out Troubleshoot dependency version conflict article first. If it doesn't provide solution for the problem, please provide:

• verbose dependency tree (mvn dependency:tree -Dverbose)
• exception message, full stack trace, and any available logs

Additional context
Add any other context about the problem here.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

• Bug Description Added
• Repro Steps Added
• Setup information Added

[alzimmermsft]

This would be a moderate API change for all polymorphic models but I wonder if we change how the discriminator type is exposed in both Jackson and stream-style serialization to always be a property on the model and have it exposed. This will help out in scenarios where the discriminator property doesn't match anything and falls back to the parent type and will be a closer representation of the model compared to Swagger/TypeSpec. This will add an additional getter to the model but I think it results in us fully exposing the model as-is and could help us clean up the many configurations Autorest Java supports.

@srnagar @JonathanGiles, thoughts on change the API for polymorphic models to expose the discriminator property rather than hiding it as we do now? I believe there is also skew throughout the SDKs on whether it is exposed or hidden and this would let us create a clearer guideline of "always expose the polymorphic discriminator property"

[srnagar]

This will require adding a public getKind() or getType() method for all polymorphic types. What will this return? Will it be a String or an ExpandableStringEnum type with all known sub-classes? Also, how would the customers use it?

scenarios where the discriminator property doesn't match anything and falls back to the parent type

When would this happen? It would seem like the service sent a polymorphic type that we don't have a sub-class for. What is the current behavior without the getKind() method?

[alzimmermsft]

This will require adding a public getKind() or getType() method for all polymorphic types. What will this return? Will it be a String or an ExpandableStringEnum type with all known sub-classes? Also, how would the customers use it?

The return type will be based on the Swagger definition, I've seen cases where it has been a String and an ExpandableStringEnum.

scenarios where the discriminator property doesn't match anything and falls back to the parent type

When would this happen? It would seem like the service sent a polymorphic type that we don't have a sub-class for. What is the current behavior without the getKind() method?

A historical example of this is Azure AI Search where skills aren't associated with service versions but are (were? I forget if this changed) released to all supported service versions once available. If the SDK hasn't been updated to the latest version the new skills would fall back to the base class. Without exposing kind, the deserialized skill we didn't know about would only have the basic information of all skills, which could cause round-tripping problems if it's sent back to the service. Exposing the deserialized discriminator could help here by giving the customer more information in debugging.

[weidongxu-microsoft]

Yes, the class would depend on the definition. TSP also could be String and an ExpandableStringEnum (they could define a Union, but it usually convert to ExpandableStringEnum or Enum) in Java.

---

The issue itself is probably corner case. Personally, I only encountered this in this OpenAI case.

I am not sure if BinaryData.fromObject(List<T>).toBytes() where T implements JsonSerializable<> on DefaultJsonSerializer would take the azure-json path. If yes, I think we won't have this problem after migrate to stream-style-serialization.
Currently seems not for non-class like List.

[weidongxu-microsoft]

BinaryData.fromObject(List<T>) or BinaryData.fromObject(Map<T>) may have the same problem under stream-style-serialization due to type erasure (same reason)...

[github-actions]

Hi @weidongxu-microsoft, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.