From 12e600af374d4d1fd5e9d4ec1f93ea577dbcea2d Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Thu, 30 May 2024 18:53:51 +0000 Subject: [PATCH 1/5] tests NuGet: Add closely named packages to test Search context --- tests/integration/api_packages_nuget_test.go | 28 ++++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 83947ff9671ec..57dcd64739ba5 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -435,16 +435,26 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) ExpectedTotal int64 ExpectedResults int }{ - {"", 0, 0, 1, 1}, - {"", 0, 10, 1, 1}, + {"", 0, 0, 4, 4}, + {"", 0, 10, 4, 4}, {"gitea", 0, 10, 0, 0}, {"test", 0, 10, 1, 1}, {"test", 1, 10, 1, 0}, + {"almost.similar", 0, 0, 3, 3}, } - req := NewRequestWithBody(t, "PUT", url, createPackage(packageName, "1.0.99")). - AddBasicAuth(user.Name) - MakeRequest(t, req, http.StatusCreated) + fakePackages := []string{ + packageName, + "almost.similar.dependency", + "almost.similar", + "almost.similar.dependant", + } + + for _, fakePackageName := range fakePackages { + req := NewRequestWithBody(t, "PUT", url, createPackage(fakePackageName, "1.0.99")). + AddBasicAuth(user.Name) + MakeRequest(t, req, http.StatusCreated) + } t.Run("v2", func(t *testing.T) { t.Run("Search()", func(t *testing.T) { @@ -548,9 +558,11 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) }) }) - req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, packageName, "1.0.99")). - AddBasicAuth(user.Name) - MakeRequest(t, req, http.StatusNoContent) + for _, fakePackageName := range fakePackages { + req := NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, fakePackageName, "1.0.99")). + AddBasicAuth(user.Name) + MakeRequest(t, req, http.StatusNoContent) + } }) t.Run("RegistrationService", func(t *testing.T) { From bc7fcb7dba7f24fd0ff17de111d3a1a79f97438e Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Thu, 30 May 2024 19:17:00 +0000 Subject: [PATCH 2/5] tests NuGet: Add test for SearchService Id eq --- tests/integration/api_packages_nuget_test.go | 76 +++++++++++++++++--- 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 57dcd64739ba5..64e4c36994aad 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -429,18 +429,19 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) t.Run("SearchService", func(t *testing.T) { cases := []struct { - Query string - Skip int - Take int - ExpectedTotal int64 - ExpectedResults int + Query string + Skip int + Take int + ExpectedTotal int64 + ExpectedResults int + ExpectedExactMatch bool }{ - {"", 0, 0, 4, 4}, - {"", 0, 10, 4, 4}, - {"gitea", 0, 10, 0, 0}, - {"test", 0, 10, 1, 1}, - {"test", 1, 10, 1, 0}, - {"almost.similar", 0, 0, 3, 3}, + {"", 0, 0, 4, 4, false}, + {"", 0, 10, 4, 4, false}, + {"gitea", 0, 10, 0, 0, false}, + {"test", 0, 10, 1, 1, false}, + {"test", 1, 10, 1, 0, false}, + {"almost.similar", 0, 0, 3, 3, true}, } fakePackages := []string{ @@ -501,6 +502,59 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) } }) + t.Run("Packages()", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + t.Run("substringof", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + for i, c := range cases { + req := NewRequest(t, "GET", fmt.Sprintf("%s/Packages()?$filter=substringof('%s',tolower(Id))&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). + AddBasicAuth(user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result FeedResponse + decodeXML(t, resp, &result) + + assert.Equal(t, c.ExpectedTotal, result.Count, "case %d: unexpected total hits", i) + assert.Len(t, result.Entries, c.ExpectedResults, "case %d: unexpected result count", i) + + req = NewRequest(t, "GET", fmt.Sprintf("%s/Packages()/$count?$filter=substringof('%s',tolower(Id))&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). + AddBasicAuth(user.Name) + resp = MakeRequest(t, req, http.StatusOK) + + assert.Equal(t, strconv.FormatInt(c.ExpectedTotal, 10), resp.Body.String(), "case %d: unexpected total hits", i) + } + }) + + t.Run("IdEq", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + for i, c := range cases { + req := NewRequest(t, "GET", fmt.Sprintf("%s/Packages()?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). + AddBasicAuth(user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result FeedResponse + decodeXML(t, resp, &result) + + if c.ExpectedExactMatch { + assert.Equal(t, int64(1), result.Count, "case %d: unexpected total hits", i) + assert.Len(t, result.Entries, 1, "case %d: unexpected result count", i) + } else { + assert.Equal(t, int64(0), result.Count, "case %d: unexpected total hits", i) + assert.Len(t, result.Entries, 0, "case %d: unexpected result count", i) + } + + req = NewRequest(t, "GET", fmt.Sprintf("%s/Packages()/$count?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). + AddBasicAuth(user.Name) + resp = MakeRequest(t, req, http.StatusOK) + + assert.Equal(t, strconv.FormatInt(c.ExpectedTotal, 10), resp.Body.String(), "case %d: unexpected total hits", i) + } + }) + }) + t.Run("Next", func(t *testing.T) { req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='test'&$skip=0&$top=1", url)). AddBasicAuth(user.Name) From 70f87e11b5caf1ee441ae71c7eba1831f45897d4 Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Thu, 30 May 2024 19:17:48 +0000 Subject: [PATCH 3/5] NuGet: Fix Search with package Id equality term --- routers/api/packages/nuget/nuget.go | 35 +++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 26b0ae226e45b..3542b361ea1f9 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -96,6 +96,12 @@ func FeedCapabilityResource(ctx *context.Context) { xmlResponse(ctx, http.StatusOK, Metadata) } +var searchTermEqPattern = regexp.MustCompile(`(?i)(?:tolower\(Id\)|Id)\s+eq\s+'[^']*'`) + +func isSearchTermExact(ctx *context.Context) bool { + return searchTermEqPattern.MatchString(ctx.FormTrim("$filter")) +} + var searchTermExtract = regexp.MustCompile(`'([^']+)'`) func getSearchTerm(ctx *context.Context) string { @@ -117,15 +123,26 @@ func SearchServiceV2(ctx *context.Context) { skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") 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), - }, - IsInternal: optional.Some(false), - Paginator: paginator, - }) + searchValue := packages_model.SearchValue{ + Value: getSearchTerm(ctx), + ExactMatch: isSearchTermExact(ctx), + } + + pvs := []*packages_model.PackageVersion{} + total := int64(0) + err := error(nil) + + // PackageSearchOptions.ToConds doesn't handle this correctly + // https://github.com/go-gitea/gitea/blob/fb7b743bd0f305a6462896398bcba2a74c6e391e/models/packages/package_version.go#L213-L214 + if !(searchValue.ExactMatch && searchValue.Value == "") { + pvs, total, err = packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeNuGet, + Name: searchValue, + IsInternal: optional.Some(false), + Paginator: paginator, + }) + } if err != nil { apiError(ctx, http.StatusInternalServerError, err) return From d70b72ca62566948e615ec5459dc7e494a66314a Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Mon, 3 Jun 2024 12:28:16 +0200 Subject: [PATCH 4/5] tests NuGet: fix SearchService Id eq test on route --- tests/integration/api_packages_nuget_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 64e4c36994aad..85158472f7b17 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -538,19 +538,19 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) var result FeedResponse decodeXML(t, resp, &result) + expectedCount := 0 if c.ExpectedExactMatch { - assert.Equal(t, int64(1), result.Count, "case %d: unexpected total hits", i) - assert.Len(t, result.Entries, 1, "case %d: unexpected result count", i) - } else { - assert.Equal(t, int64(0), result.Count, "case %d: unexpected total hits", i) - assert.Len(t, result.Entries, 0, "case %d: unexpected result count", i) + expectedCount = 1 } + assert.Equal(t, int64(expectedCount), result.Count, "case %d: unexpected total hits", i) + assert.Len(t, result.Entries, expectedCount, "case %d: unexpected result count", i) + req = NewRequest(t, "GET", fmt.Sprintf("%s/Packages()/$count?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). AddBasicAuth(user.Name) resp = MakeRequest(t, req, http.StatusOK) - assert.Equal(t, strconv.FormatInt(c.ExpectedTotal, 10), resp.Body.String(), "case %d: unexpected total hits", i) + assert.Equal(t, strconv.FormatInt(int64(expectedCount), 10), resp.Body.String(), "case %d: unexpected total hits", i) } }) }) From b8e1d2d2f78d81f6fda483d2e92125a9bad4f0ac Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Mon, 3 Jun 2024 12:54:18 +0200 Subject: [PATCH 5/5] NuGet: use implementation suggested by KN4CK3R in #31188 This is cleaner (and correctly fix the / route). With the drawback of not correctly handling "$filter=Id eq ''" queries, so it's ignored in tests. --- routers/api/packages/nuget/nuget.go | 73 +++++++++----------- tests/integration/api_packages_nuget_test.go | 4 ++ 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 3542b361ea1f9..3633d0d00704c 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -96,26 +96,34 @@ func FeedCapabilityResource(ctx *context.Context) { xmlResponse(ctx, http.StatusOK, Metadata) } -var searchTermEqPattern = regexp.MustCompile(`(?i)(?:tolower\(Id\)|Id)\s+eq\s+'[^']*'`) - -func isSearchTermExact(ctx *context.Context) bool { - return searchTermEqPattern.MatchString(ctx.FormTrim("$filter")) -} - -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, } } - return searchTerm + + // $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 packages_model.SearchValue{} } // https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs @@ -123,26 +131,13 @@ func SearchServiceV2(ctx *context.Context) { skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") paginator := db.NewAbsoluteListOptions(skip, take) - searchValue := packages_model.SearchValue{ - Value: getSearchTerm(ctx), - ExactMatch: isSearchTermExact(ctx), - } - - pvs := []*packages_model.PackageVersion{} - total := int64(0) - err := error(nil) - - // PackageSearchOptions.ToConds doesn't handle this correctly - // https://github.com/go-gitea/gitea/blob/fb7b743bd0f305a6462896398bcba2a74c6e391e/models/packages/package_version.go#L213-L214 - if !(searchValue.ExactMatch && searchValue.Value == "") { - pvs, total, err = packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{ - OwnerID: ctx.Package.Owner.ID, - Type: packages_model.TypeNuGet, - Name: searchValue, - IsInternal: optional.Some(false), - Paginator: paginator, - }) - } + pvs, total, err := packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeNuGet, + Name: getSearchTerm(ctx), + IsInternal: optional.Some(false), + Paginator: paginator, + }) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return @@ -186,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 { diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 85158472f7b17..630b4de3f92b6 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -531,6 +531,10 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) defer tests.PrintCurrentTest(t)() for i, c := range cases { + if c.Query == "" { + // Ignore the `tolower(Id) eq ''` as it's unlikely to happen + continue + } req := NewRequest(t, "GET", fmt.Sprintf("%s/Packages()?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)). AddBasicAuth(user.Name) resp := MakeRequest(t, req, http.StatusOK)