From 471138829b0c24fe8c621dbb866ae8bb45ebc674 Mon Sep 17 00:00:00 2001
From: KN4CK3R <admin@oldschoolhack.me>
Date: Wed, 16 Aug 2023 08:01:20 +0200
Subject: [PATCH] Fix NuGet search endpoints (#25613) (#26499)

Backport of #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

(cherry picked from commit 762d4245fb22a927861d30c6314d81e27eb1a06a)
---
 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 af6130c9f2..d44f7ca2ae 100644
--- a/models/db/common.go
+++ b/models/db/common.go
@@ -20,3 +20,19 @@ func BuildCaseInsensitiveLike(key, value string) builder.Cond {
 	}
 	return builder.Like{"UPPER(" + key + ")", strings.ToUpper(value)}
 }
+
+// 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 0000000000..53cdf2d4ad
--- /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 ab1bcddae5..4392c70f79 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 af52125e2e..2fe25dc0f8 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 f79292dd20..b05e3518e0 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 e4ea92ee11..520d1dd0a7 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) {