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

Improve JsonSerializer support for derived types#40654

Merged
layomia merged 1 commit into
dotnet:masterfrom
layomia:collection
Aug 29, 2019
Merged

Improve JsonSerializer support for derived types#40654
layomia merged 1 commit into
dotnet:masterfrom
layomia:collection

Conversation

@layomia

@layomia layomia commented Aug 28, 2019

Copy link
Copy Markdown
Contributor

Specifically,

  • 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.

Todo: audit behavior of collections in System.Collections.Specialized (https://github.com/dotnet/corefx/issues/40370), and System.Collections.Concurrent (https://github.com/dotnet/corefx/issues/40657).

@steveharter steveharter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than some comment suggestions, LGTM.

@layomia

layomia commented Aug 29, 2019

Copy link
Copy Markdown
Contributor Author

Will update comments in a follow up PR for https://github.com/dotnet/corefx/issues/40370 to avoid CI restart.

@layomia layomia merged commit c59f5bf into dotnet:master Aug 29, 2019
@layomia layomia deleted the collection branch August 29, 2019 15:08
@ahsonkhan ahsonkhan requested a review from ericstj August 29, 2019 20:10

@ahsonkhan ahsonkhan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@layomia

layomia commented Oct 2, 2019

Copy link
Copy Markdown
Contributor Author

This PR addresses https://github.com/dotnet/corefx/issues/41242.

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.

Preview 8 System.Text.Json error deserializing ObservableCollection

3 participants