Skip to content
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

Remove Set(object) API from MutableJsonElement #38266

Merged
merged 15 commits into from
Aug 22, 2023

Conversation

annelo-msft
Copy link
Member

@annelo-msft annelo-msft commented Aug 17, 2023

Description

With the move of MutableJsonDocument and related types into internal shared source, we want to limit what can be done with its APIs to prevent users from making mistakes with it. This PR removes the Set(object) method from MutableJsonElement, so people can only assign a specific set of known types to these elements. The group of supported types is essentially the same group that is supported by the JsonElement.Get APIs, with the addition of JsonElement to allow for the assignment of arbitrary JSON values.

This PR should have the side benefit of improving performance in a few ways, including removing the allocation of a JsonSerializerOptions on each creation of MutableJsonDocument, and fixing #38215.

What's in this PR

  • Serialization of a non-primitive value to JSON now lives in the DynamicData layer
  • We no longer cache a JsonElement with each change - instead we handle changes as the primitive type
  • MutableJsonElement now has a static method that can create JsonElements from objects without taking buffers from the ArrayPool - these JsonElements are backed by JsonDocuments whose buffers will be garbage collected as normal.

Fixes #38215

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@annelo-msft annelo-msft changed the title Remove Set(object) API from MutableJsonDocument Remove Set(object) API from MutableJsonElement Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MutableJsonChange can drain ArrayPool with undisposed JsonDocuments
6 participants