[release/6.0] Disable fast path serialization for types with properties using custom converters. #58571
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions
bot
requested review from
eiriktsarpalis,
layomia and
steveharter
as code owners
|
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsBackport of #58291 to release/6.0 /cc @steveharter @eiriktsarpalis Customer ImpactTestingRisk
|
can you clarify this? it's not as simple as "if we see this pattern fast path can't support, we silently dont use fast path"? |
Changed the wording and added some clarification. A run-time error will now occur (although a compile-time error I think would be better, but that is a separate discussion). cc @layomia @eiriktsarpalis |
| @@ -790,7 +790,7 @@ private string GenerateFastPathFuncForObject(TypeGenerationSpec typeGenSpec) | |||
| out Dictionary<string, PropertyGenerationSpec>? serializableProperties, | |||
| out bool castingRequiredForProps)) | |||
| { | |||
| string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, use 'JsonSourceGenerationMode.Serialization'."""; | |||
| string exceptionMessage = @$"""Invalid serializable-property configuration specified for type '{typeRef}'. For more information, see 'JsonSourceGenerationMode.Serialization'."""; | |||
There was a problem hiding this comment.
shouldn't this be in \src\libraries\System.Text.Json\gen\Resources\Strings.resx?
There was a problem hiding this comment.
doesn't need fixing in this PR, since it's pre existing.
There was a problem hiding this comment.
This is tracked by #58292. We should fix for 6.
|
Approved. Localized fix to customer reported issue in new feature, helping customers be successful with that feature. Do you want me to merge? |
Yes please |
ghost
locked as resolved and limited conversation to collaborators
Backport of #58291 to release/6.0
/cc @steveharter @eiriktsarpalis
Customer Impact
If a serializable POCO has a property that specifies a custom converter, the property was ignored instead of being serialized with the custom converter.
Testing
Tests added to cover the scenario.
Risk
If
[JsonSourceGenerationOptions(GenerationMode =JsonSourceGenerationMode.Serialization)]is used on the context class, a run-time error will now occur. This is consistent with other features that aren't supported in the "fast path". To fix this,JsonSourceGenerationMode.DefaultorJsonSourceGenerationMode.Metadatashould be used instead.