-
Notifications
You must be signed in to change notification settings - Fork 285
Support Polymorphic input payload deserialization (isolated dotnet) #3250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
d2546d8
f091627
77d66ec
ae2f632
669ac2c
bc543f1
da5d84f
0580caa
e7ef327
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,9 @@ | |||||
|
|
||||||
| using System; | ||||||
| using System.IO; | ||||||
| using System.Reflection; | ||||||
| using System.Text; | ||||||
| using System.Text.Json.Serialization; | ||||||
| using Azure.Core.Serialization; | ||||||
| using Microsoft.DurableTask; | ||||||
|
|
||||||
|
|
@@ -39,7 +41,69 @@ public ObjectConverterShim(ObjectSerializer serializer) | |||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| BinaryData data = this.serializer.Serialize(value, value.GetType(), default); | ||||||
| Type valueType = value.GetType(); | ||||||
|
|
||||||
| // Special handling for object[] arrays - DTFx wraps activity inputs in object[] | ||||||
| // When System.Text.Json serializes object[], it treats elements as type 'object' (the static array element type), | ||||||
| // not as their runtime concrete type, which prevents polymorphic type discriminators from being added. | ||||||
| // We must serialize each element individually with its polymorphic base type and build the JSON array manually. | ||||||
| if (valueType == typeof(object[])) | ||||||
| { | ||||||
| object[] array = (object[])value; | ||||||
| var jsonBuilder = new StringBuilder(); | ||||||
| jsonBuilder.Append('['); | ||||||
|
|
||||||
| for (int i = 0; i < array.Length; i++) | ||||||
| { | ||||||
| if (i > 0) | ||||||
| { | ||||||
| jsonBuilder.Append(','); | ||||||
| } | ||||||
|
|
||||||
| object? element = array[i]; | ||||||
| if (element != null) | ||||||
| { | ||||||
| // Serialize each element with its polymorphic base type to include type discriminators | ||||||
| Type elementType = element.GetType(); | ||||||
| Type serializeAs = GetPolymorphicBaseType(elementType) ?? elementType; | ||||||
|
|
||||||
| BinaryData elementData = this.serializer.Serialize(element, serializeAs, default); | ||||||
| jsonBuilder.Append(elementData.ToString()); | ||||||
|
||||||
| jsonBuilder.Append(elementData.ToString()); | |
| jsonBuilder.Append(elementData); |
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetPolymorphicBaseType method only searches for JsonPolymorphicAttribute on base classes using the inheritance chain (BaseType property), but does not check implemented interfaces. According to issue #3182, users need to support polymorphic deserialization for interface-typed activity inputs (e.g., IVehicle). Consider also checking the type's implemented interfaces using GetInterfaces() to find interfaces decorated with JsonPolymorphicAttribute.
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation only supports System.Text.Json's JsonPolymorphicAttribute, but the original issue #3182 specifically mentions using Newtonsoft.Json with TypeNameHandling.All. The current implementation will not detect polymorphic types when using Newtonsoft.Json, as it won't find the attribute. Consider either documenting that this feature only works with System.Text.Json, or extend the implementation to also support Newtonsoft.Json's type handling mechanisms (e.g., checking for $type property in the serialized JSON).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description checklist indicates that "My changes should not be added to the release notes for the next release" is checked, suggesting this change should not appear in the release notes. However, a release note entry has been added. Either the release note should be removed, or the PR description checklist should be updated to reflect that release notes are needed.