Skip to content

Remove JObject ServiceIndex - #7404

Merged
Nigusu-Allehu merged 7 commits into
devfrom
dev-nyenework-remove-jobject-serviceindex
May 27, 2026
Merged

Nigusu-Allehu merged 7 commits into
devfrom
dev-nyenework-remove-jobject-serviceindex

Conversation

@Nigusu-Allehu

@Nigusu-Allehu Nigusu-Allehu commented May 21, 2026 •

Copy link
Copy Markdown
Member

Bug

Fixes: NuGet/Home#14913

Description

Migrates GetServiceIndexResponse.ServiceIndex and GetOperationClaimsRequest.ServiceIndex
from JObject to string in NuGet.Protocol.Plugins.

The migration follows the same breaking change pattern as #7376: the old JObject constructors and
properties are marked [Obsolete] and delegate to the new string-based API. A custom NSJ
converter (NsjRawJsonStringConverter) enforces that only JSON objects are accepted on both read
and write.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@Nigusu-Allehu Nigusu-Allehu self-assigned this May 21, 2026
@Nigusu-Allehu Nigusu-Allehu added the Breaking-change Label for .NET SDK breaking changes. label May 21, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET SDK Breaking Change Notification email list.

You can refer to the .NET SDK breaking change guidelines

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 updates the NuGet protocol plugin message surface to stop exposing JObject for service index payloads, replacing it with a raw JSON string representation while keeping the on-the-wire JSON shape as an object.

Changes:

  • Introduces ServiceIndexJson (string) on GetServiceIndexResponse and GetOperationClaimsRequest, and obsoletes the JObject APIs.
  • Adds a custom Newtonsoft.Json converter (NsjRawJsonStringConverter) intended to enforce “JSON object only” semantics for these raw JSON string properties.
  • Updates unit/functional tests and PublicAPI.Unshipped entries to reflect the new API surface.

Reviewed changes

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

Show a summary per file
File Description
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/RequestHandlers/GetServiceIndexRequestHandlerTests.cs Updates assertions to validate ServiceIndexJson instead of JObject formatting.
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/NsjRawJsonStringConverterTests.cs Adds focused tests for the new raw-JSON-string converter.
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/Messages/GetServiceIndexResponseTests.cs Migrates message tests to ServiceIndexJson and validates JSON shape.
test/NuGet.Core.Tests/NuGet.Protocol.Tests/Plugins/Messages/GetOperationClaimsRequestTests.cs Migrates message tests to ServiceIndexJson and validates JSON shape.
test/NuGet.Core.FuncTests/NuGet.Protocol.FuncTest/PluginTests.cs Updates functional tests to pass service index as a raw JSON string.
src/NuGet.Core/NuGet.Protocol/PublicAPI/net8.0/PublicAPI.Unshipped.txt Adds Unshipped entries for the new string-based APIs.
src/NuGet.Core/NuGet.Protocol/PublicAPI/net472/PublicAPI.Unshipped.txt Adds Unshipped entries for the new string-based APIs.
src/NuGet.Core/NuGet.Protocol/Plugins/RequestHandlers/GetServiceIndexRequestHandler.cs Stops parsing service index into JObject; returns raw JSON string through the new API.
src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs Stops parsing service index into JObject; propagates raw JSON string through operation-claims flow.
src/NuGet.Core/NuGet.Protocol/Plugins/NsjRawJsonStringConverter.cs New Newtonsoft.Json converter for treating raw JSON object values as strings.
src/NuGet.Core/NuGet.Protocol/Plugins/Messages/GetServiceIndexResponse.cs Adds ServiceIndexJson and obsoletes JObject members.
src/NuGet.Core/NuGet.Protocol/Plugins/Messages/GetOperationClaimsRequest.cs Adds ServiceIndexJson and obsoletes JObject members.
src/NuGet.Core/NuGet.Protocol/GlobalSuppressions.cs Updates suppressions for the adjusted PluginManager signatures.

Comment thread src/NuGet.Core/NuGet.Protocol/Plugins/NsjRawJsonStringConverter.cs

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 13 out of 13 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/NuGet.Core/NuGet.Protocol/Plugins/Messages/GetServiceIndexResponse.cs:52

  • Adding the new string overload alongside the existing JObject constructor makes common calls like new GetServiceIndexResponse(MessageResponseCode.NotFound, null) ambiguous at compile time (null can convert to both string and JObject). This is a source-breaking change compared to the previous API and also forces internal callers to add casts. Consider adding a dedicated overload/factory for the no-service-index cases (e.g., a ctor that omits the serviceIndex parameter for NotFound/non-Success scenarios) so callers don’t need to pass null to disambiguate overloads.
        [JsonConstructor]
        public GetServiceIndexResponse(MessageResponseCode responseCode, string serviceIndex)
        {
            if (!Enum.IsDefined(typeof(MessageResponseCode), responseCode))
            {

src/NuGet.Core/NuGet.Protocol/Plugins/Messages/GetOperationClaimsRequest.cs:46

  • The new (string, string) overload combined with the existing (string, JObject) constructor makes new GetOperationClaimsRequest(packageSourceRepository, null) ambiguous at compile time (null is convertible to both reference types). Since the remarks explicitly allow null serviceIndex/packageSourceRepository for source-agnostic requests, consider adding a dedicated overload/factory for the source-agnostic/null cases so callers don’t need casts to select the intended overload.
        /// <param name="serviceIndex">The service index (index.json) as a raw JSON string.</param>
        /// <remarks>Both packageSourceRepository and service index can be null. If they are, the operation claims request is considered as source agnostic</remarks>
        [JsonConstructor]
        public GetOperationClaimsRequest(string packageSourceRepository, string serviceIndex)
        {
            PackageSourceRepository = packageSourceRepository;

@Nigusu-Allehu
Nigusu-Allehu marked this pull request as ready for review May 21, 2026 19:12
@Nigusu-Allehu
Nigusu-Allehu requested a review from a team as a code owner May 21, 2026 19:12
Comment thread src/NuGet.Core/NuGet.Protocol/Plugins/Messages/GetServiceIndexResponse.cs Outdated
@Nigusu-Allehu
Nigusu-Allehu requested a review from nkolev92 May 26, 2026 22:10
@jeffkl jeffkl changed the title Remove Jobject ServiceIndex Remove JObject ServiceIndex May 26, 2026
@Nigusu-Allehu
Nigusu-Allehu merged commit 7660599 into dev May 27, 2026
17 of 18 checks passed
@Nigusu-Allehu
Nigusu-Allehu deleted the dev-nyenework-remove-jobject-serviceindex branch May 27, 2026 00:35
eerhardt added a commit to microsoft/aspire that referenced this pull request Sep 23, 2026
## Description

Bundled Aspire CLI operations no longer need to start `aspire-managed`
to search for, restore, or inspect NuGet packages. The Native AOT
`aspire.exe` now calls NuGet.Client APIs in-process, reducing process
boundaries while preserving the existing CLI behavior. Non-bundled
package search continues to use `dotnet package search`.

This adds an in-process `NuGetClient`, rewires the bundle NuGet service
and cache, and removes the superseded `aspire-managed nuget`
implementations. Package source mapping, signature verification,
dependency resolution, extraction, and manifest generation remain
supported.

The CLI consumes the official NuGet.Client `7.12.0-rc.25` packages from
the `dotnet11` feed, pinned through the `NuGetPackageVersionForCli`
property in `eng/Versions.props`. The temporary locally-built packages
and the repository-local `dist` package source have been removed. The
upstream Native AOT work is tracked by
[NuGet/Home#14913](NuGet/Home#14913), with the
downstream-visible suppression fix in
[NuGet/NuGet.Client#7404](NuGet/NuGet.Client#7404).

`dotnet11` carries only prerelease `NuGet.*` versions while
`dotnet-public` carries only the stable ones the rest of the repository
uses, so `NuGet.config` maps `NuGet.*` to both sources at equal
specificity. Giving exact patterns to just one source makes it beat the
other's wildcard and breaks that consumer with `NU1103`.

### Intentional behavior change: NuGet credential providers

The `aspire-managed` helper never set up NuGet's credential service, so
bundled search and restore could only authenticate with credentials
stored in `nuget.config`; feeds that rely on a credential provider
plugin, such as Azure Artifacts, returned 401. The in-process client now
initializes the credential service in non-interactive mode, so installed
credential providers are used.

This is the only intended behavior change. Credential provider
diagnostics go only to the debug log, so they cannot change the failure
messages described below.

### Behavior parity

This change is meant to move the logic between processes, not change it,
so the in-process client was compared line by line against the helper
and brought back in line wherever it had diverged:

- **Restore:** the package spec, restore arguments, settings loading,
and source resolution are identical to the helper's.
- **Search:** one page per source, deduplicated to one entry per package
ID by the highest version *string*, sorted with the default comparer,
and capped at the requested count. Failed sources are reported and
skipped rather than failing the search. A `--nuget-config` path that
does not exist falls back to normal discovery.
- **Exact-match lookups** (`GetPackageVersionsAsync`) are an ordinary
search whose result with an ordinally matching ID supplies the versions,
rather than a package-metadata query merged across sources.
- **Failure messages** keep the helper-era text: `Package restore
failed: …`, `Manifest creation failed: …`, and the localized search
failure message. The embedded detail is the helper's stderr,
reconstructed with the same prefixes, verbose filtering, and trailing
error lines.
- **Logging:** NuGet's output is logged at Debug, as the helper's stderr
was.
- **`DOTNET_NUGET_SIGNATURE_VERIFICATION`** is set only for the duration
of a restore, as the helper only ever received it itself, instead of
leaking into every child process the CLI starts afterwards.
- **Trust store:** an initialization failure is reported and the restore
continues.
- **Process state:** NuGet keeps process-wide state between operations:
the credential service with its cached credentials, credential provider
plugin processes, the HTTP throttle, and other caches. The helper
discarded it by exiting after every operation, so the client raises
NuGet's own end-of-build reset when the last overlapping operation ends.
- **Restore cache key:** sources are sorted, so their order does not
force a new restore, and the key fingerprints the binary that performs
the restore. A settings fingerprint added earlier in this PR was
removed: it hashed the per-invocation temporary config path, so the
cache never hit.
- **Bundle extraction for `aspire doctor`:** on `main`, bundled NuGet
search extracted the bundle before launching `aspire-managed`, and the
background CLI update check runs that search on startup, so the bundle
was on disk by the time `aspire doctor` looked for DCP. In-process NuGet
no longer extracts it, so the DCP health check now does: it asks layout
discovery first, exactly as on `main`, so an `ASPIRE_DCP_PATH` override
or an already extracted bundle still wins, and extracts the bundle only
when discovery finds nothing. This was the only code relying on NuGet
search having extracted the bundle.

A few differences are inherent to running in-process under Native AOT:
NuGet moves from `7.9.0` to `7.12.0-rc.25`; NuGet's System.Text.Json
deserialization is enabled and Newtonsoft's serialization,
component-model, and dynamic features are disabled (see below); the
Linux trust store is initialized through
`X509TrustStore.InitializeForDotNetSdk` instead of `DispatchProxy`,
using the same embedded SDK certificate bundles; and NuGet operations no
longer require an extracted bundle layout or hold a bundle lease.
Per-source search failures log the exception type rather than its
message, because NuGet formats feed URLs, including credentials, into
those messages.

### Native AOT and Newtonsoft.Json

`7.12.0-rc.25` still reaches Newtonsoft.Json in places, so
`Aspire.Cli.csproj` disables Newtonsoft's serialization,
component-model, and dynamic feature switches. The dynamic-dispatch
warnings that remain surface from `Microsoft.CSharp` and
`System.Linq.Expressions`, which are collapsed to one warning per
assembly. Both can be removed once NuGet drops Newtonsoft.Json entirely
([NuGet/NuGet.Client#7601](NuGet/NuGet.Client#7601)).

### User-facing usage

Existing commands continue to work without starting `aspire-managed` for
bundled NuGet operations:

```text
aspire integration list
```

Validation:

- Repository restore completed successfully, with the CLI resolving
`7.12.0-rc.25` from `dotnet11` and `Aspire.RuntimeIdentifier.Tool`
resolving stable `7.9.0` from `dotnet-public` on a cold package cache.
- NuGet client, bundle service, package-cache, signature-verification,
DCP health check, and `PrebuiltAppHostServerTests` passed: 246
succeeded.
- The tests that run real restores, manifests, and searches also passed
(170 succeeded) with the CLI's exact runtime switches applied,
confirming none of the disabled Newtonsoft paths are reached at runtime.
- Native AOT `win-x64` publish completed with no ILC diagnostics, using
the feature switches and per-assembly warning collapsing described
above.
- A dogfood build of an earlier commit created and ran TypeScript
AppHosts end to end: `aspire new`, `aspire add`, and `aspire run` for
both `aspire-ts-empty` and `aspire-ts-starter`, including a cold-cache
restore that produced byte-identical generated modules.

Fixes # (issue)

## Checklist

- Is this feature complete?
  - [ ] Yes. Ready to ship.
  - [x] No. Follow-up changes expected.
- Are you including unit tests for the changes and scenario tests if
relevant?
  - [x] Yes
  - [ ] No
- Did you add public API?
  - [ ] Yes
    - If yes, did you have an API Review for it?
      - [ ] Yes
      - [ ] No
- Did you add `<remarks />` and `<code />` elements on your triple slash
comments?
      - [ ] Yes
      - [ ] No
  - [x] No
- Does the change make any security assumptions or guarantees?
  - [ ] Yes
    - If yes, have you done a threat model and had a security review?
      - [ ] Yes
      - [ ] No
  - [x] No

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: baa86aab-4a9f-44c7-a92e-34f6498c9e4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-change Label for .NET SDK breaking changes. needs-breaking-change-doc-created

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin ServiceIndex is tied to Newtonsoft.Json (JObject), blocking STJ migration

4 participants