From 7bb85267361205ac60466f6455be975ea1b649c3 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 27 Jul 2023 05:41:40 +0800 Subject: [PATCH] Fix handling of plenty Nuget package versions (#26075) (#26173) Backport #26075 by @KN4CK3R Fixes #25953 - Do not load full version information (v3) - Add pagination support (v2) Co-authored-by: KN4CK3R (cherry picked from commit 54614767a256923912c65bbdc35471f83c00cf66) --- routers/api/packages/nuget/api_v2.go | 14 +++- routers/api/packages/nuget/api_v3.go | 8 +- routers/api/packages/nuget/links.go | 20 +++++ routers/api/packages/nuget/nuget.go | 84 ++++++++++++++------ tests/integration/api_packages_nuget_test.go | 50 ++++++++++-- 5 files changed, 138 insertions(+), 38 deletions(-) diff --git a/routers/api/packages/nuget/api_v2.go b/routers/api/packages/nuget/api_v2.go index 7d0ac64a8a..a726065ad0 100644 --- a/routers/api/packages/nuget/api_v2.go +++ b/routers/api/packages/nuget/api_v2.go @@ -289,7 +289,7 @@ type FeedResponse struct { ID string `xml:"id"` Title TypedValue[string] `xml:"title"` Updated time.Time `xml:"updated"` - Link FeedEntryLink `xml:"link"` + Links []FeedEntryLink `xml:"link"` Entries []*FeedEntry `xml:"entry"` Count int64 `xml:"m:count"` } @@ -300,6 +300,16 @@ func createFeedResponse(l *linkBuilder, totalEntries int64, pds []*packages_mode entries = append(entries, createEntry(l, pd, false)) } + links := []FeedEntryLink{ + {Rel: "self", Href: l.Base}, + } + if l.Next != nil { + links = append(links, FeedEntryLink{ + Rel: "next", + Href: l.GetNextURL(), + }) + } + return &FeedResponse{ Xmlns: "http://www.w3.org/2005/Atom", Base: l.Base, @@ -307,7 +317,7 @@ func createFeedResponse(l *linkBuilder, totalEntries int64, pds []*packages_mode XmlnsM: "http://schemas.microsoft.com/ado/2007/08/dataservices/metadata", ID: "http://schemas.datacontract.org/2004/07/", Updated: time.Now(), - Link: FeedEntryLink{Rel: "self", Href: l.Base}, + Links: links, Count: totalEntries, Entries: entries, } diff --git a/routers/api/packages/nuget/api_v3.go b/routers/api/packages/nuget/api_v3.go index 28626f9294..af52125e2e 100644 --- a/routers/api/packages/nuget/api_v3.go +++ b/routers/api/packages/nuget/api_v3.go @@ -166,10 +166,10 @@ type PackageVersionsResponse struct { Versions []string `json:"versions"` } -func createPackageVersionsResponse(pds []*packages_model.PackageDescriptor) *PackageVersionsResponse { - versions := make([]string, 0, len(pds)) - for _, pd := range pds { - versions = append(versions, pd.Version.Version) +func createPackageVersionsResponse(pvs []*packages_model.PackageVersion) *PackageVersionsResponse { + versions := make([]string, 0, len(pvs)) + for _, pv := range pvs { + versions = append(versions, pv.Version) } return &PackageVersionsResponse{ diff --git a/routers/api/packages/nuget/links.go b/routers/api/packages/nuget/links.go index 1b02e46184..4c573fe316 100644 --- a/routers/api/packages/nuget/links.go +++ b/routers/api/packages/nuget/links.go @@ -5,10 +5,17 @@ package nuget import ( "fmt" + "net/url" ) +type nextOptions struct { + Path string + Query url.Values +} + type linkBuilder struct { Base string + Next *nextOptions } // GetRegistrationIndexURL builds the registration index url @@ -30,3 +37,16 @@ func (l *linkBuilder) GetPackageDownloadURL(id, version string) string { func (l *linkBuilder) GetPackageMetadataURL(id, version string) string { return fmt.Sprintf("%s/Packages(Id='%s',Version='%s')", l.Base, id, version) } + +func (l *linkBuilder) GetNextURL() string { + u, _ := url.Parse(l.Base) + u = u.JoinPath(l.Next.Path) + q := u.Query() + for k, vs := range l.Next.Query { + for _, v := range vs { + q.Add(k, v) + } + } + u.RawQuery = q.Encode() + return u.String() +} diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 3c09bcb2ec..f79292dd20 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "net/http" + "net/url" "regexp" "strconv" "strings" @@ -111,13 +112,8 @@ func getSearchTerm(ctx *context.Context) string { // https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs func SearchServiceV2(ctx *context.Context) { - skip, take := ctx.FormInt("skip"), ctx.FormInt("take") - if skip == 0 { - skip = ctx.FormInt("$skip") - } - if take == 0 { - take = ctx.FormInt("$top") - } + skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") + paginator := db.NewAbsoluteListOptions(skip, take) pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ OwnerID: ctx.Package.Owner.ID, @@ -126,10 +122,7 @@ func SearchServiceV2(ctx *context.Context) { Value: getSearchTerm(ctx), }, IsInternal: util.OptionalBoolFalse, - Paginator: db.NewAbsoluteListOptions( - skip, - take, - ), + Paginator: paginator, }) if err != nil { apiError(ctx, http.StatusInternalServerError, err) @@ -142,8 +135,28 @@ func SearchServiceV2(ctx *context.Context) { return } + skip, take = paginator.GetSkipTake() + + var next *nextOptions + if len(pvs) == take { + next = &nextOptions{ + Path: "Search()", + Query: url.Values{}, + } + searchTerm := ctx.FormTrim("searchTerm") + if searchTerm != "" { + next.Query.Set("searchTerm", searchTerm) + } + filter := ctx.FormTrim("$filter") + if filter != "" { + next.Query.Set("$filter", filter) + } + next.Query.Set("$skip", strconv.Itoa(skip+take)) + next.Query.Set("$top", strconv.Itoa(take)) + } + resp := createFeedResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget", Next: next}, total, pds, ) @@ -193,7 +206,7 @@ func SearchServiceV3(ctx *context.Context) { } resp := createSearchResultResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, count, pds, ) @@ -222,7 +235,7 @@ func RegistrationIndex(ctx *context.Context) { } resp := createRegistrationIndexResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, pds, ) @@ -251,7 +264,7 @@ func RegistrationLeafV2(ctx *context.Context) { } resp := createEntryResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, pd, ) @@ -280,7 +293,7 @@ func RegistrationLeafV3(ctx *context.Context) { } resp := createRegistrationLeafResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, pd, ) @@ -291,7 +304,19 @@ func RegistrationLeafV3(ctx *context.Context) { func EnumeratePackageVersionsV2(ctx *context.Context) { packageName := strings.Trim(ctx.FormTrim("id"), "'") - pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeNuGet, packageName) + skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") + paginator := db.NewAbsoluteListOptions(skip, take) + + pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ + OwnerID: ctx.Package.Owner.ID, + Type: packages_model.TypeNuGet, + Name: packages_model.SearchValue{ + ExactMatch: true, + Value: packageName, + }, + IsInternal: util.OptionalBoolFalse, + Paginator: paginator, + }) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return @@ -303,9 +328,22 @@ func EnumeratePackageVersionsV2(ctx *context.Context) { return } + skip, take = paginator.GetSkipTake() + + var next *nextOptions + if len(pvs) == take { + next = &nextOptions{ + Path: "FindPackagesById()", + Query: url.Values{}, + } + next.Query.Set("id", packageName) + next.Query.Set("$skip", strconv.Itoa(skip+take)) + next.Query.Set("$top", strconv.Itoa(take)) + } + resp := createFeedResponse( - &linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"}, - int64(len(pds)), + &linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget", Next: next}, + total, pds, ) @@ -345,13 +383,7 @@ func EnumeratePackageVersionsV3(ctx *context.Context) { return } - pds, err := packages_model.GetPackageDescriptors(ctx, pvs) - if err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - - resp := createPackageVersionsResponse(pds) + resp := createPackageVersionsResponse(pvs) ctx.JSON(http.StatusOK, resp) } diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 3592d64db2..e4ea92ee11 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "net/http/httptest" + neturl "net/url" "strconv" "testing" "time" @@ -68,10 +69,16 @@ func TestPackageNuGet(t *testing.T) { Content string `xml:",innerxml"` } + type FeedEntryLink struct { + Rel string `xml:"rel,attr"` + Href string `xml:"href,attr"` + } + type FeedResponse struct { - XMLName xml.Name `xml:"feed"` - Entries []*FeedEntry `xml:"entry"` - Count int64 `xml:"count"` + XMLName xml.Name `xml:"feed"` + Links []FeedEntryLink `xml:"link"` + Entries []*FeedEntry `xml:"entry"` + Count int64 `xml:"count"` } user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) @@ -373,6 +380,25 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) }) }) + containsOneNextLink := func(t *testing.T, links []FeedEntryLink) func() bool { + return func() bool { + found := 0 + for _, l := range links { + if l.Rel == "next" { + found++ + u, err := neturl.Parse(l.Href) + assert.NoError(t, err) + q := u.Query() + assert.Contains(t, q, "$skip") + assert.Contains(t, q, "$top") + assert.Equal(t, "1", q.Get("$skip")) + assert.Equal(t, "1", q.Get("$top")) + } + } + return found == 1 + } + } + t.Run("SearchService", func(t *testing.T) { cases := []struct { Query string @@ -393,7 +419,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) defer tests.PrintCurrentTest(t)() for i, c := range cases { - req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='%s'&skip=%d&take=%d", url, c.Query, c.Skip, c.Take)) + req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='%s'&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)) req = AddBasicAuthHeader(req, user.Name) resp := MakeRequest(t, req, http.StatusOK) @@ -403,7 +429,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) 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/Search()/$count?searchTerm='%s'&skip=%d&take=%d", url, c.Query, c.Skip, c.Take)) + req = NewRequest(t, "GET", fmt.Sprintf("%s/Search()/$count?searchTerm='%s'&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take)) req = AddBasicAuthHeader(req, user.Name) resp = MakeRequest(t, req, http.StatusOK) @@ -432,6 +458,17 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) 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)) + req = AddBasicAuthHeader(req, user.Name) + resp := MakeRequest(t, req, http.StatusOK) + + var result FeedResponse + decodeXML(t, resp, &result) + + assert.Condition(t, containsOneNextLink(t, result.Links)) + }) }) t.Run("v3", func(t *testing.T) { @@ -558,7 +595,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) t.Run("v2", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()?id='%s'", url, packageName)) + req := NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()?id='%s'&$top=1", url, packageName)) req = AddBasicAuthHeader(req, user.Name) resp := MakeRequest(t, req, http.StatusOK) @@ -567,6 +604,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) assert.Len(t, result.Entries, 1) assert.Equal(t, packageVersion, result.Entries[0].Properties.Version) + assert.Condition(t, containsOneNextLink(t, result.Links)) req = NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()/$count?id='%s'", url, packageName)) req = AddBasicAuthHeader(req, user.Name)