Fix NuGet Package API for $filter with Id equality

[tdesveaux]

Fixes issue when running choco info pkgname where pkgname is also a substring of another package Id.

Relates to #31168

---

This might fix the issue linked, but I'd like to test it with more choco commands before closing the issue in case I find other problems if that's ok.
I'm pretty inexperienced with Go, so feel free to nitpick things.

Not sure I handled this in the best way, so looking for feedback on if I should fix the underlying issue (nil might be a better default for Value?).

[KN4CK3R]

Thank you for the fix. Isn't a change like this enough?

@@ -96,20 +96,34 @@ func FeedCapabilityResource(ctx *context.Context) {
        xmlResponse(ctx, http.StatusOK, Metadata)
 }
 
-var searchTermExtract = regexp.MustCompile(`'([^']+)'`)
+var (
+       searchTermExtract = regexp.MustCompile(`'([^']+)'`)
+       searchTermExact   = regexp.MustCompile(`\s+eq\s+'`)
+)
 
-func getSearchTerm(ctx *context.Context) string {
+func getSearchTerm(ctx *context.Context) packages_model.SearchValue {
        searchTerm := strings.Trim(ctx.FormTrim("searchTerm"), "'")
-       if searchTerm == "" {
-               // $filter contains a query like:
-               // (((Id ne null) and substringof('microsoft',tolower(Id)))
-               // We don't support these queries, just extract the search term.
-               match := searchTermExtract.FindStringSubmatch(ctx.FormTrim("$filter"))
-               if len(match) == 2 {
-                       searchTerm = strings.TrimSpace(match[1])
+       if searchTerm != "" {
+               return packages_model.SearchValue{
+                       Value:      searchTerm,
+                       ExactMatch: false,
+               }
+       }
+
+       // $filter contains a query like:
+       // (((Id ne null) and substringof('microsoft',tolower(Id)))
+       // https://www.odata.org/documentation/odata-version-2-0/uri-conventions/ section 4.5
+       // We don't support these queries, just extract the search term.
+       filter := ctx.FormTrim("$filter")
+       match := searchTermExtract.FindStringSubmatch(filter)
+       if len(match) == 2 {
+               return packages_model.SearchValue{
+                       Value:      strings.TrimSpace(match[1]),
+                       ExactMatch: searchTermExact.MatchString(filter),
                }
        }
-       return searchTerm
+
+       return packages_model.SearchValue{}
 }
 
 // https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs
@@ -118,11 +132,9 @@ func SearchServiceV2(ctx *context.Context) {
        paginator := db.NewAbsoluteListOptions(skip, take)
 
        pvs, total, err := packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{
-               OwnerID: ctx.Package.Owner.ID,
-               Type:    packages_model.TypeNuGet,
-               Name: packages_model.SearchValue{
-                       Value: getSearchTerm(ctx),
-               },
+               OwnerID:    ctx.Package.Owner.ID,
+               Type:       packages_model.TypeNuGet,
+               Name:       getSearchTerm(ctx),
                IsInternal: optional.Some(false),
                Paginator:  paginator,
        })
@@ -169,10 +181,8 @@ func SearchServiceV2(ctx *context.Context) {
 // http://docs.oasis-open.org/odata/odata/v4.0/errata03/os/complete/part2-url-conventions/odata-v4.0-errata03-os-part2-url-conventions-complete.html#_Toc453752351
 func SearchServiceV2Count(ctx *context.Context) {
        count, err := nuget_model.CountPackages(ctx, &packages_model.PackageSearchOptions{
-               OwnerID: ctx.Package.Owner.ID,
-               Name: packages_model.SearchValue{
-                       Value: getSearchTerm(ctx),
-               },
+               OwnerID:    ctx.Package.Owner.ID,
+               Name:       getSearchTerm(ctx),
                IsInternal: optional.Some(false),
        })
        if err != nil {

Could you test if this fixes your choco queries?

[tdesveaux]

I added the queries choco make with it's commands in the issue here

I also tried a choco info '' and it failed before querying, so the edge case 'issue' I found can be ignore for this case.

Thank you for the fix. Isn't a change like this enough?

If I removed the edge-case handling of .Value = "" filter, and added the missing patch of Packages/$count (and code clarity).
The only difference is on the regex used to define if the search is an exact one.
I don't mind doing it this way, but any reason to only look for eq vs the pattern I used which is more cautious and will only match on Id?

Could you test if this fixes your choco queries?

The tests I added pass so I think it's fine.
I'll take time later to test the real choco query.

[tdesveaux]

@KN4CK3R I updated the branch with your implementation suggestion.

There is still the edge-case of Id eq '' but it's an improbable query (at least from choco). So I simply ignored the test case.
I tested with a local build and choco works as expected now.

[KN4CK3R]

Thank you for the tests.

[tdesveaux]

@KN4CK3R apologies for the ping.

Any change this would be backported to 1.22?

CONTRIBUTING.md mention that large PRs are not backport, and I don't know how the large qualifier is attributed, but isn't it set due to the changes in tests?

[KN4CK3R]

You can backport it to 1.22 because it's a bug fix.

[silverwind]

Should have automatically backported, but ok :)

[tdesveaux]

Should have automatically backported, but ok :)

Might have misinterpreted KN4CK3R 's message above.
Should I have done something or waited on your side?

[KN4CK3R]

Everything fine, the Giteabot can create backports too.

[silverwind]

It's preferably if the bot does it, because then we know the content is exactly the same. Manual backport like you did is only when the bot fails to do it because of merge conflict.

Let's see if the bot does it now...

[silverwind]

It didn't because of the backport/manual flag. Next time try via bot first.