Skip to content

Replace Newtonsoft.Json with System.Text.Json in dotnet-user-secrets#65686

Merged
MichaelSimons merged 10 commits into
mainfrom
copilot/replace-newtonsoft-json-dependency
May 5, 2026
Merged

Replace Newtonsoft.Json with System.Text.Json in dotnet-user-secrets#65686
MichaelSimons merged 10 commits into
mainfrom
copilot/replace-newtonsoft-json-dependency

Conversation

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Replaces the Newtonsoft.Json dependency in dotnet-user-secrets with System.Text.Json, using the default STJ character escaping (non-ASCII characters are Unicode-escaped rather than written literally). Consolidates list command tests from SecretManagerTests into the new ListCommandTest, and adds integration tests to cover --json CLI option plumbing and nested JSON flattening behavior.

Description

  • Removed <Reference Include="Newtonsoft.Json" /> from dotnet-user-secrets.csproj.
  • Replaced JObject/Formatting.Indented in ListCommand.cs and SecretsStore.cs with JsonSerializer.Serialize using WriteIndented = true and the default STJ encoder (non-ASCII characters are Unicode-escaped).
  • Extracted the shared JsonSerializerOptions into a static readonly SecretsStore.SerializerOptions field reused by both SecretsStore.Save() and ListCommand.ReportJson().
  • Added ListCommandTest with unit tests covering JSON output format, non-ASCII Unicode escaping, //BEGIN///END markers, empty store JSON output, non-JSON key = value format, and non-JSON empty store error message.
  • Consolidated overlapping list tests: removed List_Flattens_Nested_Objects, List_Json, and List_Empty_Secrets_File from SecretManagerTests (all now covered by ListCommandTest unit tests).
  • Added List_Json_OptionIsWired integration test to SecretManagerTests to verify that the --json CLI option is correctly wired through CommandLineOptions.ParseListCommand.ConfigureListCommand.Execute.
  • Added List_Flattens_Nested_Objects integration test to SecretManagerTests to verify that nested JSON secrets files on disk (e.g., { "AzureAd": { "ClientSecret": "..." } }) are correctly flattened to key:subkey format by SecretsStore.Load().

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@github-actions github-actions Bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 6, 2026
…tool

Co-authored-by: MichaelSimons <8290530+MichaelSimons@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace Newtonsoft.Json with System.Text.Json in dotnet-user-secrets Replace Newtonsoft.Json with System.Text.Json in dotnet-user-secrets Mar 6, 2026
@MichaelSimons

Copy link
Copy Markdown
Member

@dotnet/aspnet-build, please provide feedback on if it is important to preserve Newtonsoft's default behavior of writing non-ASCII characters literally. Should we use the default System.Text.Json escaping? See https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/migrate-from-newtonsoft?pivots=dotnet-10-0#minimal-character-escaping for details.

@MichaelSimons MichaelSimons marked this pull request as ready for review March 9, 2026 14:23
Copilot AI review requested due to automatic review settings March 9, 2026 14:23

Copilot AI 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.

Pull request overview

This PR removes the Newtonsoft.Json dependency from dotnet-user-secrets in favor of System.Text.Json, part of a broader SDK migration effort (dotnet/sdk#53287). The output format is preserved by using WriteIndented = true and JavaScriptEncoder.UnsafeRelaxedJsonEscaping to match Newtonsoft's default behavior of emitting non-ASCII characters literally.

Changes:

  • ListCommand.cs and SecretsStore.cs replace JObject/Formatting.Indented with JsonSerializer.Serialize using equivalent options.
  • dotnet-user-secrets.csproj removes the <Reference Include="Newtonsoft.Json" /> item.
  • ListCommandTest.cs adds unit tests covering JSON format, non-ASCII preservation, //BEGIN///END markers, and empty store output.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Internal/ListCommand.cs Replaces JObject + Newtonsoft formatting with JsonSerializer using UnsafeRelaxedJsonEscaping
src/Internal/SecretsStore.cs Replaces JObject build loop and Newtonsoft.Json.Linq with direct JsonSerializer.Serialize(_secrets, options)
src/dotnet-user-secrets.csproj Removes the Newtonsoft.Json framework reference
test/ListCommandTest.cs Adds new unit tests for the --json output format to guard against regressions

Comment thread src/Tools/dotnet-user-secrets/src/Internal/ListCommand.cs Outdated
Comment thread src/Tools/dotnet-user-secrets/src/Internal/SecretsStore.cs Outdated
…s static field

Co-authored-by: MichaelSimons <8290530+MichaelSimons@users.noreply.github.com>
@MichaelSimons MichaelSimons requested a review from a team March 17, 2026 14:13
@MichaelSimons

Copy link
Copy Markdown
Member

@wtgodbe - Can you take a look at this? Also would like feedback on #65686 (comment). TIA

@halter73

Copy link
Copy Markdown
Member

please provide feedback on if it is important to preserve Newtonsoft's default behavior of writing non-ASCII characters literally. Should we use the default System.Text.Json escaping

I think the default System.Text.Json escaping should be fine. We usually use S.T.J to read these secrets via a JsonConfigurationSource.

…CII characters

Co-authored-by: MichaelSimons <8290530+MichaelSimons@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/48edeec7-7ad3-4117-ba13-1fcb361f202f
Copilot AI requested a review from MichaelSimons March 24, 2026 21:17
@MichaelSimons

Copy link
Copy Markdown
Member

I think the default System.Text.Json escaping should be fine. We usually use S.T.J to read these secrets via a JsonConfigurationSource.

Done in 66f8e89.

I also noticed an opportunity to consolidate the list command tests which was handled in 59d29fe

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/Tools/dotnet-user-secrets/test/ListCommandTest.cs
Comment thread src/Tools/dotnet-user-secrets/test/ListCommandTest.cs
…on plumbing

Co-authored-by: MichaelSimons <8290530+MichaelSimons@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/10b33337-7f61-4287-a7c5-84cb4e85797a
…re.Load() disk reading

Co-authored-by: MichaelSimons <8290530+MichaelSimons@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/f9992d14-6c0c-41cf-baa1-e0da01e46226

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread src/Tools/dotnet-user-secrets/src/Internal/SecretsStore.cs
Comment thread src/Tools/dotnet-user-secrets/test/ListCommandTest.cs Outdated
MichaelSimons and others added 2 commits May 5, 2026 09:50
@MichaelSimons MichaelSimons enabled auto-merge (squash) May 5, 2026 14:52
@MichaelSimons MichaelSimons merged commit 4b6bdf3 into main May 5, 2026
25 checks passed
@MichaelSimons MichaelSimons deleted the copilot/replace-newtonsoft-json-dependency branch May 5, 2026 15:56
@dotnet-policy-service dotnet-policy-service Bot added this to the 11.0-preview5 milestone May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants