Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.1] Improve JsonSerializer support for derived types (#40654) - #41523

Merged
layomia merged 1 commit into
dotnet:release/3.1from
layomia:derived_types
Oct 4, 2019
Merged

[release/3.1] Improve JsonSerializer support for derived types (#40654)#41523
layomia merged 1 commit into
dotnet:release/3.1from
layomia:derived_types

Conversation

@layomia

@layomia layomia commented Oct 3, 2019

Copy link
Copy Markdown
Contributor

Ports #40654 to 3.1

Description

  • When determining whether to handle a derived type as IList (i.e populating directly, without using a converter), check whether the runtime type is assignable to IList, not the declared type.
    This fixes https://github.com/dotnet/corefx/issues/40597.
  • Preemptively throw NotSupportedException on deserialization of implementing types that don't have default constructors. This prevents a NullReferenceException when we try to create the instance.
  • Detect and use the implemented type of abstract implementing types for (de)serialization. This allows us to correctly throw NotSupportedException on deserialization (because abstract types have no default constructor).
  • Document expectations of (de)serializing collections in System.Collections.ObjectModel in tests.

Customer Impact

Regression?

No.

Risk

Low. This PR does not modify already existing features, so the possibility of breaking functionality or introducing regressions is limited. Extensive test cases were added to ensure that the new changes work as expected.

@ahsonkhan

Copy link
Copy Markdown

cc @danmosemsft, @ericstj

@danmoseley

Copy link
Copy Markdown

This PR does not modify already existing features, so the possibility of breaking functionality or introducing regressions is limited

I'm not sure what 'modify existing features' means exactly - I see the changes are in existing code paths.

@danmoseley

Copy link
Copy Markdown

Thanks @layomia . I'm fine with merging this if @ericstj approves it. It's bug fix level work to address mainstream customer reported issues in JSON.

@layomia

layomia commented Oct 4, 2019

Copy link
Copy Markdown
Contributor Author

This PR does not modify already existing features, so the possibility of breaking functionality or introducing regressions is limited

I'm not sure what 'modify existing features' means exactly - I see the changes are in existing code paths.

@danmosemsft I meant "modify the behavior of supported (de)serialization scenarios".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants