Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix handling of plenty Nuget package versions (#26075) #26173

Merged
merged 2 commits into from
Jul 26, 2023
Merged

Fix handling of plenty Nuget package versions (#26075) #26173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed80739
Fix handling of plenty Nuget package versions (#26075)
KN4CK3R Jul 26, 2023
8c8101c
Merge branch 'release/v1.20' into backport-26075-v1.20
GiteaBot Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions routers/api/packages/nuget/api_v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -300,14 +300,24 @@ 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,
XmlnsD: "http://schemas.microsoft.com/ado/2007/08/dataservices",
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,
}
Expand Down
8 changes: 4 additions & 4 deletions routers/api/packages/nuget/api_v3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
20 changes: 20 additions & 0 deletions routers/api/packages/nuget/links.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
}
84 changes: 58 additions & 26 deletions routers/api/packages/nuget/nuget.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"net/http"
"net/url"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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,
)

Expand All @@ -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
Expand All @@ -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,
)

Expand Down Expand Up @@ -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)
}
Expand Down
50 changes: 44 additions & 6 deletions tests/integration/api_packages_nuget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"io"
"net/http"
"net/http/httptest"
neturl "net/url"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -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})
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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)
Expand Down