-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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.
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
Fix handling of plenty Nuget package versions #26075
Changes from 2 commits
12da448
efb8181
8e23c79
bc446a1
4e4d329
ea42268
1d30314
19083ae
2da0ebd
e850434
da4ba22
9e7b8a1
cf504cf
6464e7d
c587a06
25aaa2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import ( | |
"fmt" | ||
"io" | ||
"net/http" | ||
"net/url" | ||
"regexp" | ||
"strconv" | ||
"strings" | ||
|
@@ -111,13 +112,14 @@ 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") | ||
skip, take := ctx.FormInt("$skip"), ctx.FormInt("$top") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's just how the spec defines this parameter. The names are |
||
if skip == 0 { | ||
skip = ctx.FormInt("$skip") | ||
skip = ctx.FormInt("skip") | ||
KN4CK3R marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if take == 0 { | ||
take = ctx.FormInt("$top") | ||
take = ctx.FormInt("take") | ||
} | ||
paginator := db.NewAbsoluteListOptions(skip, take) | ||
|
||
pvs, total, err := packages_model.SearchVersions(ctx, &packages_model.PackageSearchOptions{ | ||
OwnerID: ctx.Package.Owner.ID, | ||
|
@@ -126,10 +128,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 +141,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 +212,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 +241,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 +270,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 +299,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 +310,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 +334,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 +389,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) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker
Could you change the
u
,q
,vs
shorthand to some significant names or could you annotate them in comments.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, these are the common abbreviations for these types of variables.
u
is often an url,k
andv
arekey
andvalue
.vs
values
in this case. The variables have a very limited scope, so it's easy to look them up.https://github.com/golang/go/wiki/CodeReviewComments#variable-names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure,