Feature ask client side -- Surface PackageType query parameter in the NuGet.org search API

NuGet / Home

Repo for NuGet Client issues

Other

1.49k stars 250 forks source link

Feature ask client side -- Surface PackageType query parameter in the NuGet.org search API #8915

Open wli3 opened 4 years ago

wli3 commented 4 years ago

https://github.com/NuGet/NuGetGallery/issues/7594

SDK team has the nuget gallery ask. And Peter Yu and @joelverhagen suggested it is better have a client side counterpart instead of directly query the web API. So all client side .NET code is in nuget client.

However, in the meanwhile SDK will continue directly using the web REST API since it is relatively simple, and we will migrate to use the client once it is available.

nkolev92 commented 4 years ago

wli3 commented 4 years ago

Do we have a decision from the triage?

nkolev92 commented 4 years ago

@wli3 We haven't had a discussion (holidays and all). fyi @aortiz-msft

zivkan commented 4 years ago

This came up in team chat today. We'll need to add an extra property in our search options. However, the question of running search on an old server that does not implement this new search option came up.

From an API design point of view, how should NuGet.Protocol behave when the server implements an old version of the NuGet HTTP protocol and doesn't support packageType filtering?

Throw an exception?
- Also add a TrySearch that returns false when the server doesn't support a filter option (need to also figure out how to communicate which filter option wasn't supported, so this is extensible in the future)?
- Expect the caller to know how to get the service index resource and check the search service version themselves, plus the version they care about?
- Add methods or properties to check for capabilies? (SearchResource.SupportsPackageType)
Silently ignore the issue, make the request anyway, and return results that may not be the package type requested?

API design is hard :(

joelverhagen commented 4 years ago

You could consider leveraging the resource provider pattern and make a dedicated resource (which could inherit an existing resource, if you want) and resource provider. If the repository returns null that means the feature is not supported. Just another option to toss into the hat.

glopesdev commented 3 weeks ago

I am confused about the current API surface. There is a class SearchFilter which includes a PackageTypes property. This is used directly by PackageSearchResourceV3 to build the query string with packageTypeFilter as below:

https://github.com/NuGet/NuGet.Client/blob/0539e8e2516f708c834cb606340c7c9a110f96b3/src/NuGet.Core/NuGet.Protocol/Resources/PackageSearchResourceV3.cs#L145

Strangely, packageTypeFilter doesn't seem to be used anywhere on the server side, where packageType is the default instead. Is there an intended and accepted use for the PackageTypes property in SearchFilter, or is it a faulty implementation of this issue?

nkolev92 commented 3 weeks ago

packageTypeFilter predates search by package type. It's probably just dead code client side.

glopesdev commented 3 weeks ago

@nkolev92 that makes sense. In that case am I understanding correctly that all the API design for this is already made, and the only thing needed would be replacing packageTypeFilter with packageType in the client code?

For consistency I guess it would be nice if package type filtering also applied automatically to local package sources.

nkolev92 commented 3 weeks ago

The protocol is decided and documented: https://learn.microsoft.com/en-us/nuget/api/search-query-service-resource#search-for-packages.

We're just not using it client side yet, so it's not part of our libraries.