Skip to content

Commit 2d7fe4c

Browse files
KN4CK3RKN4CK3R
authored
Fix handling of plenty Nuget package versions (#26075)
Fixes #25953 - Do not load full version information (v3) - Add pagination support (v2)
1 parent 6ce8988 commit 2d7fe4c

File tree

5 files changed

+138
-38
lines changed

5 files changed

+138
-38
lines changed

‎routers/api/packages/nuget/api_v2.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ type FeedResponse struct {
289289
ID string `xml:"id"`
290290
Title TypedValue[string] `xml:"title"`
291291
Updated time.Time `xml:"updated"`
292-
Link FeedEntryLink `xml:"link"`
292+
Links []FeedEntryLink `xml:"link"`
293293
Entries []*FeedEntry `xml:"entry"`
294294
Count int64 `xml:"m:count"`
295295
}
@@ -300,14 +300,24 @@ func createFeedResponse(l *linkBuilder, totalEntries int64, pds []*packages_mode
300300
entries = append(entries, createEntry(l, pd, false))
301301
}
302302

303+
links := []FeedEntryLink{
304+
{Rel: "self", Href: l.Base},
305+
}
306+
if l.Next != nil {
307+
links = append(links, FeedEntryLink{
308+
Rel: "next",
309+
Href: l.GetNextURL(),
310+
})
311+
}
312+
303313
return &FeedResponse{
304314
Xmlns: "http://www.w3.org/2005/Atom",
305315
Base: l.Base,
306316
XmlnsD: "http://schemas.microsoft.com/ado/2007/08/dataservices",
307317
XmlnsM: "http://schemas.microsoft.com/ado/2007/08/dataservices/metadata",
308318
ID: "http://schemas.datacontract.org/2004/07/",
309319
Updated: time.Now(),
310-
Link: FeedEntryLink{Rel: "self", Href: l.Base},
320+
Links: links,
311321
Count: totalEntries,
312322
Entries: entries,
313323
}

‎routers/api/packages/nuget/api_v3.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ type PackageVersionsResponse struct {
166166
Versions []string `json:"versions"`
167167
}
168168

169-
func createPackageVersionsResponse(pds []*packages_model.PackageDescriptor) *PackageVersionsResponse {
170-
versions := make([]string, 0, len(pds))
171-
for _, pd := range pds {
172-
versions = append(versions, pd.Version.Version)
169+
func createPackageVersionsResponse(pvs []*packages_model.PackageVersion) *PackageVersionsResponse {
170+
versions := make([]string, 0, len(pvs))
171+
for _, pv := range pvs {
172+
versions = append(versions, pv.Version)
173173
}
174174

175175
return &PackageVersionsResponse{

‎routers/api/packages/nuget/links.go

+20
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,17 @@ package nuget
55

66
import (
77
"fmt"
8+
"net/url"
89
)
910

11+
type nextOptions struct {
12+
Path string
13+
Query url.Values
14+
}
15+
1016
type linkBuilder struct {
1117
Base string
18+
Next *nextOptions
1219
}
1320

1421
// GetRegistrationIndexURL builds the registration index url
@@ -30,3 +37,16 @@ func (l *linkBuilder) GetPackageDownloadURL(id, version string) string {
3037
func (l *linkBuilder) GetPackageMetadataURL(id, version string) string {
3138
return fmt.Sprintf("%s/Packages(Id='%s',Version='%s')", l.Base, id, version)
3239
}
40+
41+
func (l *linkBuilder) GetNextURL() string {
42+
u, _ := url.Parse(l.Base)
43+
u = u.JoinPath(l.Next.Path)
44+
q := u.Query()
45+
for k, vs := range l.Next.Query {
46+
for _, v := range vs {
47+
q.Add(k, v)
48+
}
49+
}
50+
u.RawQuery = q.Encode()
51+
return u.String()
52+
}

‎routers/api/packages/nuget/nuget.go

+58-26
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"io"
1111
"net/http"
12+
"net/url"
1213
"regexp"
1314
"strconv"
1415
"strings"
@@ -111,13 +112,8 @@ func getSearchTerm(ctx *context.Context) string {
111112

112113
// https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs
113114
func SearchServiceV2(ctx *context.Context) {
114-
skip, take := ctx.FormInt("skip"), ctx.FormInt("take")
115-
if skip == 0 {
116-
skip = ctx.FormInt("$skip")
117-
}
118-
if take == 0 {
119-
take = ctx.FormInt("$top")
120-
}
115+
skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top")
116+
paginator := db.NewAbsoluteListOptions(skip, take)
121117

122118
pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
123119
OwnerID: ctx.Package.Owner.ID,
@@ -126,10 +122,7 @@ func SearchServiceV2(ctx *context.Context) {
126122
Value: getSearchTerm(ctx),
127123
},
128124
IsInternal: util.OptionalBoolFalse,
129-
Paginator: db.NewAbsoluteListOptions(
130-
skip,
131-
take,
132-
),
125+
Paginator: paginator,
133126
})
134127
if err != nil {
135128
apiError(ctx, http.StatusInternalServerError, err)
@@ -142,8 +135,28 @@ func SearchServiceV2(ctx *context.Context) {
142135
return
143136
}
144137

138+
skip, take = paginator.GetSkipTake()
139+
140+
var next *nextOptions
141+
if len(pvs) == take {
142+
next = &nextOptions{
143+
Path: "Search()",
144+
Query: url.Values{},
145+
}
146+
searchTerm := ctx.FormTrim("searchTerm")
147+
if searchTerm != "" {
148+
next.Query.Set("searchTerm", searchTerm)
149+
}
150+
filter := ctx.FormTrim("$filter")
151+
if filter != "" {
152+
next.Query.Set("$filter", filter)
153+
}
154+
next.Query.Set("$skip", strconv.Itoa(skip+take))
155+
next.Query.Set("$top", strconv.Itoa(take))
156+
}
157+
145158
resp := createFeedResponse(
146-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
159+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget", Next: next},
147160
total,
148161
pds,
149162
)
@@ -193,7 +206,7 @@ func SearchServiceV3(ctx *context.Context) {
193206
}
194207

195208
resp := createSearchResultResponse(
196-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
209+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
197210
count,
198211
pds,
199212
)
@@ -222,7 +235,7 @@ func RegistrationIndex(ctx *context.Context) {
222235
}
223236

224237
resp := createRegistrationIndexResponse(
225-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
238+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
226239
pds,
227240
)
228241

@@ -251,7 +264,7 @@ func RegistrationLeafV2(ctx *context.Context) {
251264
}
252265

253266
resp := createEntryResponse(
254-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
267+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
255268
pd,
256269
)
257270

@@ -280,7 +293,7 @@ func RegistrationLeafV3(ctx *context.Context) {
280293
}
281294

282295
resp := createRegistrationLeafResponse(
283-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
296+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
284297
pd,
285298
)
286299

@@ -291,7 +304,19 @@ func RegistrationLeafV3(ctx *context.Context) {
291304
func EnumeratePackageVersionsV2(ctx *context.Context) {
292305
packageName := strings.Trim(ctx.FormTrim("id"), "'")
293306

294-
pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeNuGet, packageName)
307+
skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top")
308+
paginator := db.NewAbsoluteListOptions(skip, take)
309+
310+
pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{
311+
OwnerID: ctx.Package.Owner.ID,
312+
Type: packages_model.TypeNuGet,
313+
Name: packages_model.SearchValue{
314+
ExactMatch: true,
315+
Value: packageName,
316+
},
317+
IsInternal: util.OptionalBoolFalse,
318+
Paginator: paginator,
319+
})
295320
if err != nil {
296321
apiError(ctx, http.StatusInternalServerError, err)
297322
return
@@ -303,9 +328,22 @@ func EnumeratePackageVersionsV2(ctx *context.Context) {
303328
return
304329
}
305330

331+
skip, take = paginator.GetSkipTake()
332+
333+
var next *nextOptions
334+
if len(pvs) == take {
335+
next = &nextOptions{
336+
Path: "FindPackagesById()",
337+
Query: url.Values{},
338+
}
339+
next.Query.Set("id", packageName)
340+
next.Query.Set("$skip", strconv.Itoa(skip+take))
341+
next.Query.Set("$top", strconv.Itoa(take))
342+
}
343+
306344
resp := createFeedResponse(
307-
&linkBuilder{setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget"},
308-
int64(len(pds)),
345+
&linkBuilder{Base: setting.AppURL + "api/packages/" + ctx.Package.Owner.Name + "/nuget", Next: next},
346+
total,
309347
pds,
310348
)
311349

@@ -345,13 +383,7 @@ func EnumeratePackageVersionsV3(ctx *context.Context) {
345383
return
346384
}
347385

348-
pds, err := packages_model.GetPackageDescriptors(ctx, pvs)
349-
if err != nil {
350-
apiError(ctx, http.StatusInternalServerError, err)
351-
return
352-
}
353-
354-
resp := createPackageVersionsResponse(pds)
386+
resp := createPackageVersionsResponse(pvs)
355387

356388
ctx.JSON(http.StatusOK, resp)
357389
}

‎tests/integration/api_packages_nuget_test.go

+44-6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"io"
1313
"net/http"
1414
"net/http/httptest"
15+
neturl "net/url"
1516
"strconv"
1617
"testing"
1718
"time"
@@ -68,10 +69,16 @@ func TestPackageNuGet(t *testing.T) {
6869
Content string `xml:",innerxml"`
6970
}
7071

72+
type FeedEntryLink struct {
73+
Rel string `xml:"rel,attr"`
74+
Href string `xml:"href,attr"`
75+
}
76+
7177
type FeedResponse struct {
72-
XMLName xml.Name `xml:"feed"`
73-
Entries []*FeedEntry `xml:"entry"`
74-
Count int64 `xml:"count"`
78+
XMLName xml.Name `xml:"feed"`
79+
Links []FeedEntryLink `xml:"link"`
80+
Entries []*FeedEntry `xml:"entry"`
81+
Count int64 `xml:"count"`
7582
}
7683

7784
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
@@ -373,6 +380,25 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
373380
})
374381
})
375382

383+
containsOneNextLink := func(t *testing.T, links []FeedEntryLink) func() bool {
384+
return func() bool {
385+
found := 0
386+
for _, l := range links {
387+
if l.Rel == "next" {
388+
found++
389+
u, err := neturl.Parse(l.Href)
390+
assert.NoError(t, err)
391+
q := u.Query()
392+
assert.Contains(t, q, "$skip")
393+
assert.Contains(t, q, "$top")
394+
assert.Equal(t, "1", q.Get("$skip"))
395+
assert.Equal(t, "1", q.Get("$top"))
396+
}
397+
}
398+
return found == 1
399+
}
400+
}
401+
376402
t.Run("SearchService", func(t *testing.T) {
377403
cases := []struct {
378404
Query string
@@ -393,7 +419,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
393419
defer tests.PrintCurrentTest(t)()
394420

395421
for i, c := range cases {
396-
req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='%s'&skip=%d&take=%d", url, c.Query, c.Skip, c.Take))
422+
req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='%s'&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
397423
req = AddBasicAuthHeader(req, user.Name)
398424
resp := MakeRequest(t, req, http.StatusOK)
399425

@@ -403,7 +429,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
403429
assert.Equal(t, c.ExpectedTotal, result.Count, "case %d: unexpected total hits", i)
404430
assert.Len(t, result.Entries, c.ExpectedResults, "case %d: unexpected result count", i)
405431

406-
req = NewRequest(t, "GET", fmt.Sprintf("%s/Search()/$count?searchTerm='%s'&skip=%d&take=%d", url, c.Query, c.Skip, c.Take))
432+
req = NewRequest(t, "GET", fmt.Sprintf("%s/Search()/$count?searchTerm='%s'&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
407433
req = AddBasicAuthHeader(req, user.Name)
408434
resp = MakeRequest(t, req, http.StatusOK)
409435

@@ -432,6 +458,17 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
432458
assert.Equal(t, strconv.FormatInt(c.ExpectedTotal, 10), resp.Body.String(), "case %d: unexpected total hits", i)
433459
}
434460
})
461+
462+
t.Run("Next", func(t *testing.T) {
463+
req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='test'&$skip=0&$top=1", url))
464+
req = AddBasicAuthHeader(req, user.Name)
465+
resp := MakeRequest(t, req, http.StatusOK)
466+
467+
var result FeedResponse
468+
decodeXML(t, resp, &result)
469+
470+
assert.Condition(t, containsOneNextLink(t, result.Links))
471+
})
435472
})
436473

437474
t.Run("v3", func(t *testing.T) {
@@ -558,7 +595,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
558595
t.Run("v2", func(t *testing.T) {
559596
defer tests.PrintCurrentTest(t)()
560597

561-
req := NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()?id='%s'", url, packageName))
598+
req := NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()?id='%s'&$top=1", url, packageName))
562599
req = AddBasicAuthHeader(req, user.Name)
563600
resp := MakeRequest(t, req, http.StatusOK)
564601

@@ -567,6 +604,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
567604

568605
assert.Len(t, result.Entries, 1)
569606
assert.Equal(t, packageVersion, result.Entries[0].Properties.Version)
607+
assert.Condition(t, containsOneNextLink(t, result.Links))
570608

571609
req = NewRequest(t, "GET", fmt.Sprintf("%s/FindPackagesById()/$count?id='%s'", url, packageName))
572610
req = AddBasicAuthHeader(req, user.Name)

0 commit comments

Comments
 (0)