From ecd51f710b7b08eddc952d518f0d097367221388 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Mon, 14 Aug 2023 04:50:55 +0200 Subject: [PATCH] Fix NuGet search endpoints (#25613) Fixes #25564 Fixes #23191 - Api v2 search endpoint should return only the latest version matching the query - Api v3 search endpoint should return `take` packages not package versions --- models/db/common.go | 16 +++++ models/packages/nuget/search.go | 70 ++++++++++++++++++++ models/packages/package_version.go | 10 +-- routers/api/packages/nuget/api_v3.go | 13 +++- routers/api/packages/nuget/nuget.go | 9 ++- tests/integration/api_packages_nuget_test.go | 18 ++--- 6 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 models/packages/nuget/search.go diff --git a/models/db/common.go b/models/db/common.go index 2a5043a8e78..ea628bf2a0a 100644 --- a/models/db/common.go +++ b/models/db/common.go @@ -37,3 +37,19 @@ func BuildCaseInsensitiveIn(key string, values []string) builder.Cond { return builder.In("UPPER("+key+")", uppers) } + +// BuilderDialect returns the xorm.Builder dialect of the engine +func BuilderDialect() string { + switch { + case setting.Database.Type.IsMySQL(): + return builder.MYSQL + case setting.Database.Type.IsSQLite3(): + return builder.SQLITE + case setting.Database.Type.IsPostgreSQL(): + return builder.POSTGRES + case setting.Database.Type.IsMSSQL(): + return builder.MSSQL + default: + return "" + } +} diff --git a/models/packages/nuget/search.go b/models/packages/nuget/search.go new file mode 100644 index 00000000000..53cdf2d4ad8 --- /dev/null +++ b/models/packages/nuget/search.go @@ -0,0 +1,70 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package nuget + +import ( + "context" + "strings" + + "code.gitea.io/gitea/models/db" + packages_model "code.gitea.io/gitea/models/packages" + + "xorm.io/builder" +) + +// SearchVersions gets all versions of packages matching the search options +func SearchVersions(ctx context.Context, opts *packages_model.PackageSearchOptions) ([]*packages_model.PackageVersion, int64, error) { + cond := toConds(opts) + + e := db.GetEngine(ctx) + + total, err := e. + Where(cond). + Count(&packages_model.Package{}) + if err != nil { + return nil, 0, err + } + + inner := builder. + Dialect(db.BuilderDialect()). // builder needs the sql dialect to build the Limit() below + Select("*"). + From("package"). + Where(cond). + OrderBy("package.name ASC") + if opts.Paginator != nil { + skip, take := opts.GetSkipTake() + inner = inner.Limit(take, skip) + } + + sess := e. + Where(opts.ToConds()). + Table("package_version"). + Join("INNER", inner, "package.id = package_version.package_id") + + pvs := make([]*packages_model.PackageVersion, 0, 10) + return pvs, total, sess.Find(&pvs) +} + +// CountPackages counts all packages matching the search options +func CountPackages(ctx context.Context, opts *packages_model.PackageSearchOptions) (int64, error) { + return db.GetEngine(ctx). + Where(toConds(opts)). + Count(&packages_model.Package{}) +} + +func toConds(opts *packages_model.PackageSearchOptions) builder.Cond { + var cond builder.Cond = builder.Eq{ + "package.is_internal": opts.IsInternal.IsTrue(), + "package.owner_id": opts.OwnerID, + "package.type": packages_model.TypeNuGet, + } + if opts.Name.Value != "" { + if opts.Name.ExactMatch { + cond = cond.And(builder.Eq{"package.lower_name": strings.ToLower(opts.Name.Value)}) + } else { + cond = cond.And(builder.Like{"package.lower_name", strings.ToLower(opts.Name.Value)}) + } + } + return cond +} diff --git a/models/packages/package_version.go b/models/packages/package_version.go index ab1bcddae58..4392c70f79b 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -189,7 +189,7 @@ type PackageSearchOptions struct { db.Paginator } -func (opts *PackageSearchOptions) toConds() builder.Cond { +func (opts *PackageSearchOptions) ToConds() builder.Cond { cond := builder.NewCond() if !opts.IsInternal.IsNone() { cond = builder.Eq{ @@ -283,7 +283,7 @@ func (opts *PackageSearchOptions) configureOrderBy(e db.Engine) { // SearchVersions gets all versions of packages matching the search options func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { sess := db.GetEngine(ctx). - Where(opts.toConds()). + Where(opts.ToConds()). Table("package_version"). Join("INNER", "package", "package.id = package_version.package_id") @@ -300,7 +300,7 @@ func SearchVersions(ctx context.Context, opts *PackageSearchOptions) ([]*Package // SearchLatestVersions gets the latest version of every package matching the search options func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*PackageVersion, int64, error) { - cond := opts.toConds(). + cond := opts.ToConds(). And(builder.Expr("pv2.id IS NULL")) joinCond := builder.Expr("package_version.package_id = pv2.package_id AND (package_version.created_unix < pv2.created_unix OR (package_version.created_unix = pv2.created_unix AND package_version.id < pv2.id))") @@ -328,7 +328,7 @@ func SearchLatestVersions(ctx context.Context, opts *PackageSearchOptions) ([]*P // ExistVersion checks if a version matching the search options exist func ExistVersion(ctx context.Context, opts *PackageSearchOptions) (bool, error) { return db.GetEngine(ctx). - Where(opts.toConds()). + Where(opts.ToConds()). Table("package_version"). Join("INNER", "package", "package.id = package_version.package_id"). Exist(new(PackageVersion)) @@ -337,7 +337,7 @@ func ExistVersion(ctx context.Context, opts *PackageSearchOptions) (bool, error) // CountVersions counts all versions of packages matching the search options func CountVersions(ctx context.Context, opts *PackageSearchOptions) (int64, error) { return db.GetEngine(ctx). - Where(opts.toConds()). + Where(opts.ToConds()). Table("package_version"). Join("INNER", "package", "package.id = package_version.package_id"). Count(new(PackageVersion)) diff --git a/routers/api/packages/nuget/api_v3.go b/routers/api/packages/nuget/api_v3.go index af52125e2e4..2fe25dc0f80 100644 --- a/routers/api/packages/nuget/api_v3.go +++ b/routers/api/packages/nuget/api_v3.go @@ -9,6 +9,9 @@ import ( packages_model "code.gitea.io/gitea/models/packages" nuget_module "code.gitea.io/gitea/modules/packages/nuget" + + "golang.org/x/text/collate" + "golang.org/x/text/language" ) // https://docs.microsoft.com/en-us/nuget/api/service-index#resources @@ -207,9 +210,15 @@ func createSearchResultResponse(l *linkBuilder, totalHits int64, pds []*packages grouped[pd.Package.Name] = append(grouped[pd.Package.Name], pd) } + keys := make([]string, 0, len(grouped)) + for key := range grouped { + keys = append(keys, key) + } + collate.New(language.English, collate.IgnoreCase).SortStrings(keys) + data := make([]*SearchResult, 0, len(pds)) - for _, group := range grouped { - data = append(data, createSearchResult(l, group)) + for _, key := range keys { + data = append(data, createSearchResult(l, grouped[key])) } return &SearchResultResponse{ diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 9c97ba8e776..6f63c1d4c21 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/db" packages_model "code.gitea.io/gitea/models/packages" + nuget_model "code.gitea.io/gitea/models/packages/nuget" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" packages_module "code.gitea.io/gitea/modules/packages" @@ -115,7 +116,7 @@ func SearchServiceV2(ctx *context.Context) { skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") paginator := db.NewAbsoluteListOptions(skip, take) - pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + pvs, total, err := packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, Type: packages_model.TypeNuGet, Name: packages_model.SearchValue{ @@ -166,9 +167,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 := packages_model.CountVersions(ctx, &packages_model.PackageSearchOptions{ + count, err := nuget_model.CountPackages(ctx, &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, - Type: packages_model.TypeNuGet, Name: packages_model.SearchValue{ Value: getSearchTerm(ctx), }, @@ -184,9 +184,8 @@ func SearchServiceV2Count(ctx *context.Context) { // https://docs.microsoft.com/en-us/nuget/api/search-query-service-resource#search-for-packages func SearchServiceV3(ctx *context.Context) { - pvs, count, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + pvs, count, err := nuget_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, - Type: packages_model.TypeNuGet, Name: packages_model.SearchValue{Value: ctx.FormTrim("q")}, IsInternal: util.OptionalBoolFalse, Paginator: db.NewAbsoluteListOptions( diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index e4ea92ee111..520d1dd0a70 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -414,6 +414,10 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) {"test", 1, 10, 1, 0}, } + req := NewRequestWithBody(t, "PUT", url, createPackage(packageName, "1.0.99")) + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusCreated) + t.Run("v2", func(t *testing.T) { t.Run("Search()", func(t *testing.T) { defer tests.PrintCurrentTest(t)() @@ -493,10 +497,6 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) req = AddBasicAuthHeader(req, user.Name) MakeRequest(t, req, http.StatusCreated) - req = NewRequestWithBody(t, "PUT", url, createPackage(packageName, "1.0.99")) - req = AddBasicAuthHeader(req, user.Name) - MakeRequest(t, req, http.StatusCreated) - req = NewRequest(t, "GET", fmt.Sprintf("%s/query?q=%s", url, packageName)) req = AddBasicAuthHeader(req, user.Name) resp := MakeRequest(t, req, http.StatusOK) @@ -504,7 +504,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) var result nuget.SearchResultResponse DecodeJSON(t, resp, &result) - assert.EqualValues(t, 3, result.TotalHits) + assert.EqualValues(t, 2, result.TotalHits) assert.Len(t, result.Data, 2) for _, sr := range result.Data { if sr.ID == packageName { @@ -517,12 +517,12 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, packageName+".dummy", "1.0.0")) req = AddBasicAuthHeader(req, user.Name) MakeRequest(t, req, http.StatusNoContent) - - req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, packageName, "1.0.99")) - req = AddBasicAuthHeader(req, user.Name) - MakeRequest(t, req, http.StatusNoContent) }) }) + + req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, packageName, "1.0.99")) + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) }) t.Run("RegistrationService", func(t *testing.T) {