Fix package type query parameter for V3 search, add capability check by liliankasem · Pull Request #7395 · NuGet/NuGet.Client

liliankasem · 2026-05-14T22:45:18Z

Bug

Fixes: NuGet/Home#8915
Related (closed) prior attempt: #5991

Description

PackageSearchResourceV3 was sending the package-type filter using the legacy packageTypeFilter query parameter. The SearchQueryService/3.5.0 specification uses packageType. As a result, SearchFilter.PackageTypes has been silently ignored by nuget.org and other compliant V3 sources for as long as the property has existed publicly.

This PR is intentionally scoped to the bug fix plus capability detection. The broader API redesign and local-source support discussed in #5991 are deferred to follow-up issues so this PR can land without re-litigating the same design conversation.

What this PR does

Send the correct query parameter. packageType= (single value) instead of packageTypeFilter=.
Capability detection via the service index. A new internal SearchQueryService350 entry is fetched separately. When SearchFilter.PackageTypes is set on a source that does not advertise SearchQueryService/3.5.0, the resource throws NotSupportedException instead of silently returning unfiltered results that the caller assumes are filtered. This addresses the primary concern raised by @zivkan in Add support for package type queries in both local and remote sources #5991.
Endpoint routing. Filtered queries are routed only to capable endpoints; unfiltered queries continue to use the existing endpoint set, so behavior is unchanged when no filter is requested.
Tests for: correct query parameter, capability rejection, and unchanged behavior when no filter is requested.

What this PR explicitly does NOT do (and why)

Deferred change	Reason
Reshape `SearchFilter.PackageTypes` (`IEnumerable<string>` → `string`)	This would be a breaking public API change. The current shipped API is preserved; only the first value is sent, matching the protocol contract. The reshape can be tracked separately.
Add package-type filtering to `LocalPackageSearchResource`	Independent change with its own design considerations (e.g. how nuspec package types map). Not required to fix the reported bug.
Decide aggregated-source ("All") behavior when one source is non-capable	Per the discussion in #5991, this is a front-end concern. `NotSupportedException` gives the front-end a clean signal to skip non-capable sources.

Behavior change

Callers who previously set SearchFilter.PackageTypes against a V3 source that does not advertise SearchQueryService/3.5.0 used to get unfiltered results (silently broken). They will now get a NotSupportedException. This is a strict correctness improvement: the caller's intent was never being honoured before, and now they get an explicit signal.

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. (no user-facing settings changed; the fix is in the protocol layer)

PackageSearchResourceV3 was sending the package type filter using the legacy 'packageTypeFilter' query parameter. The SearchQueryService/3.5.0 specification uses 'packageType'. This meant SearchFilter.PackageTypes was silently ignored by nuget.org and other compliant V3 sources. Changes: * Send 'packageType=' (single value) instead of 'packageTypeFilter='. * Detect capability via the SearchQueryService/3.5.0 service-index entry. When SearchFilter.PackageTypes is set on a source that does not advertise this resource, throw NotSupportedException instead of returning unfiltered results that the caller assumes are filtered. * Route filtered queries only to capable endpoints; fall back to all endpoints when no filter is requested. Scope notes (deferred to follow-up work): * SearchFilter.PackageTypes is left as IEnumerable<string> for now to avoid a breaking public API change. Only the first value is sent, matching the protocol contract. * Local source (LocalPackageSearchResource) package type filtering is not addressed here. Refs: NuGet#5991, NuGet/Home#8915

martinrrm

lgtm

Use TempApiKeyRequestTimeoutInSeconds (10s) instead of 5s, matching the fix applied to sibling tests in NuGet#7390 to address TaskCanceledException under CI load.

liliankasem · 2026-05-19T21:42:02Z

Thanks @martinrrm! Do you mind reapproving? I just fixed the unit test.

nkolev92

Sorry for the delay review.

@NuGet/nuget-client It'd be great to get some more thoughts on the behavior for search resources with package type.

nkolev92 · 2026-05-21T16:35:37Z

+        {
+        }
+
+        internal PackageSearchResourceV3(HttpSource client, IEnumerable<Uri> searchEndpoints, IEnumerable<Uri> packageTypeCapableEndpoints)


Just keep one internal constructor. Especially since we're supporting passing an empty/null anyways.

Likely just the tests need changed

nkolev92 · 2026-05-21T17:08:17Z

+
+            if (packageTypeFilterRequested && _packageTypeCapableEndpoints.Count == 0)
+            {
+                throw new NotSupportedException(Strings.Protocol_PackageTypeFilterNotSupported);


I know @zivkan requested this, but this behavior makes me wonder what we would do in the case of multi feed search.
I am trying to think forward about how we'd be shaping the API if something like compat is added to the search query.

It also doesn't really provide a means to figure out whether package type filtering is supported when you have that object, so now you have to effectively account for that exception at all times. I think that was part 2 of the feedback.
I'm honestly not sure if we have a great shape for that API right now.
We'd probably want something generic rather than IsPackageTypeFiltering supported or anything like that.

Also, if we're establishing a precedent of throwing when package type is unsupported, which is an ABI breaking change which I'm not too worried about, I think we'd need to make sure all of the implementers of PackageSearchResource actually have the same behavior.

nkolev92 · 2026-05-21T17:09:41Z

+                    // SearchQueryService/3.5.0 specifies a single 'packageType' parameter.
+                    // We send the first value; passing more than one is a caller error
+                    // (the protocol does not define multi-value semantics).
+                    queryString += "&packageType=" + filters.PackageTypes.First();


If that's an unknown syntax, I think we should actually throw in this case.

jeffkl · 2026-05-21T17:25:36Z

            _client = client ?? throw new ArgumentNullException(nameof(client));
            _searchEndpoints = searchEndpoints?.ToArray() ?? throw new ArgumentNullException(nameof(searchEndpoints));
+            _packageTypeCapableEndpoints = packageTypeCapableEndpoints == null
+                ? new HashSet<Uri>()


Can we make _packageTypeCapableEndpoints nullable so we aren't allocating a HashSet<T> in the "legacy" code path?

private readonly HashSet<Uri>? _packageTypeCapableEndpoints

Suggested change

? new HashSet<Uri>()

? null

Or we can store it as a ICollection<T>, set it to Array.Empty<T>(), and use .Distinct() on the IEnumerable<T> that's passed in here which under the covers creates a HashSet<T> for us? Ideally the input parameter should probably be IReadOnlyList<Uri> so we can call .Count to make a hashset that doesn't need to be resized? Do we deduplicate in other code paths?

jeffkl · 2026-05-21T17:36:13Z

    <value>The source does not have the 'version' property.</value>
  </data>
+  <data name="Protocol_PackageTypeFilterNotSupported" xml:space="preserve">
+    <value>The source does not advertise a 'SearchQueryService/3.5.0' resource and therefore does not support filtering search results by package type.</value>


Suggested change

<value>The source does not advertise a 'SearchQueryService/3.5.0' resource and therefore does not support filtering search results by package type.</value>

<value>The source does not support filtering search results by package type.</value>

Not sure if the message needs to include such verbose information like the remedy. Also, would it make sense to include the URI of the feed in the message?

dotnet-policy-service Bot added the Community PRs created by someone not in the NuGet team label May 14, 2026

liliankasem marked this pull request as ready for review May 14, 2026 22:47

liliankasem requested a review from a team as a code owner May 14, 2026 22:47

liliankasem requested review from jebriede and nkolev92 May 14, 2026 22:47

martinrrm previously approved these changes May 18, 2026

View reviewed changes

Fix flaky PackageUpdateResource_AllowsNoApiKeyWhenPushingAsync

13e4fda

Use TempApiKeyRequestTimeoutInSeconds (10s) instead of 5s, matching the fix applied to sibling tests in NuGet#7390 to address TaskCanceledException under CI load.

liliankasem dismissed martinrrm’s stale review via 13e4fda May 19, 2026 17:00

nkolev92 reviewed May 21, 2026

View reviewed changes

jeffkl reviewed May 21, 2026

View reviewed changes

jeffkl self-assigned this May 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix package type query parameter for V3 search, add capability check#7395

Fix package type query parameter for V3 search, add capability check#7395
liliankasem wants to merge 2 commits into
NuGet:devfrom
liliankasem:liliankasem/bug/package-type-search-query

liliankasem commented May 14, 2026

Uh oh!

martinrrm left a comment

Uh oh!

liliankasem commented May 19, 2026

Uh oh!

nkolev92 left a comment

Uh oh!

nkolev92 May 21, 2026

Uh oh!

nkolev92 May 21, 2026

Uh oh!

nkolev92 May 21, 2026

Uh oh!

nkolev92 May 21, 2026

Uh oh!

jeffkl May 21, 2026

Uh oh!

jeffkl May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	<value>The source does not advertise a 'SearchQueryService/3.5.0' resource and therefore does not support filtering search results by package type.</value>
	<value>The source does not support filtering search results by package type.</value>

Conversation

liliankasem commented May 14, 2026

Bug

Description

What this PR does

What this PR explicitly does NOT do (and why)

Behavior change

PR Checklist

Uh oh!

martinrrm left a comment

Choose a reason for hiding this comment

Uh oh!

liliankasem commented May 19, 2026

Uh oh!

nkolev92 left a comment

Choose a reason for hiding this comment

Uh oh!

nkolev92 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

nkolev92 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

nkolev92 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

nkolev92 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

jeffkl May 21, 2026

Choose a reason for hiding this comment

Uh oh!

jeffkl May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants