From 307dbc6fba8233ee7057c6398fbc44ba785e17ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Mon, 9 Mar 2020 22:19:42 +0800 Subject: [PATCH] Accept the pagination information in the separated query string (#10991) Accept the pagination information in the separated query string Signed-off-by: Wenkai Yin --- api/v2.0/swagger.yaml | 6 ++-- src/pkg/q/builder.go | 46 ++++++++++++--------------- src/pkg/q/builder_test.go | 32 +++++++++---------- src/server/v2.0/handler/artifact.go | 2 +- src/server/v2.0/handler/base.go | 22 ++++++------- src/server/v2.0/handler/base_test.go | 22 +++++++++---- src/server/v2.0/handler/repository.go | 2 +- 7 files changed, 67 insertions(+), 65 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 412cf9c47d..b714de2579 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -26,10 +26,9 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/projectName' - # TODO remove it + - $ref: '#/parameters/query' - $ref: '#/parameters/page' - $ref: '#/parameters/pageSize' - - $ref: '#/parameters/query' # TODO remove it - name: name in: query @@ -149,9 +148,10 @@ paths: - $ref: '#/parameters/requestId' - $ref: '#/parameters/projectName' - $ref: '#/parameters/repositoryName' + - $ref: '#/parameters/query' - $ref: '#/parameters/page' - $ref: '#/parameters/pageSize' - - $ref: '#/parameters/query' + # TODO remove the query strings included in "query" - name: type in: query description: Query the artifacts by type. Valid values can be "IMAGE", "CHART", etc. diff --git a/src/pkg/q/builder.go b/src/pkg/q/builder.go index 59878f9037..3b1157bdff 100644 --- a/src/pkg/q/builder.go +++ b/src/pkg/q/builder.go @@ -17,23 +17,37 @@ package q import ( "fmt" ierror "github.com/goharbor/harbor/src/internal/error" + "net/url" "strconv" "strings" "time" ) -// Build query sting into the Query model -// query string format: q=k=v,k=~v,k=[min~max],k={v1 v2 v3},k=(v1 v2 v3),page=1,page_size=10 +// Build query sting and pagination information into the Query model +// query string format: q=k=v,k=~v,k=[min~max],k={v1 v2 v3},k=(v1 v2 v3) // exact match: k=v // fuzzy match: k=~v // range: k=[min~max] // or list: k={v1 v2 v3} // and list: k=(v1 v2 v3) -func Build(q string) (*Query, error) { - if len(q) == 0 { - return nil, nil +func Build(q string, pageNumber, pageSize int64) (*Query, error) { + query := &Query{ + PageNumber: pageNumber, + PageSize: pageSize, + Keywords: map[string]interface{}{}, } - query := &Query{Keywords: map[string]interface{}{}} + if len(q) == 0 { + return query, nil + } + // unescape the query string + // when the query string is returned in the link header, they are all escaped + qs, err := url.QueryUnescape(q) + if err != nil { + return nil, ierror.New(err). + WithCode(ierror.BadRequestCode). + WithMessage("invalid query string: %s", q) + } + q = qs params := strings.Split(q, ",") for _, param := range params { strs := strings.SplitN(param, "=", 2) @@ -42,26 +56,6 @@ func Build(q string) (*Query, error) { WithCode(ierror.BadRequestCode). WithMessage(`the query string must contain "=" and the key/value cannot be empty`) } - if strs[0] == "page" { - i, err := strconv.ParseInt(strs[1], 10, 64) - if err != nil { - return nil, ierror.New(nil). - WithCode(ierror.BadRequestCode). - WithMessage("page must be integer") - } - query.PageNumber = i - continue - } - if strs[0] == "page_size" { - i, err := strconv.ParseInt(strs[1], 10, 64) - if err != nil { - return nil, ierror.New(nil). - WithCode(ierror.BadRequestCode). - WithMessage("page_size must be integer") - } - query.PageSize = i - continue - } value, err := parsePattern(strs[1]) if err != nil { return nil, ierror.New(err). diff --git a/src/pkg/q/builder_test.go b/src/pkg/q/builder_test.go index 81966c1d9a..96c1dcdfca 100644 --- a/src/pkg/q/builder_test.go +++ b/src/pkg/q/builder_test.go @@ -244,28 +244,28 @@ func TestParsePattern(t *testing.T) { func TestBuild(t *testing.T) { // empty string q := `` - query, err := Build(q) + query, err := Build(q, 1, 10) require.Nil(t, err) - assert.Nil(t, query) + require.NotNil(t, query) + assert.Equal(t, int64(1), query.PageNumber) + assert.Equal(t, int64(10), query.PageSize) - // contains only ";" + // contains only "," q = `,` - query, err = Build(q) - require.NotNil(t, err) - - // invalid page - q = `page=a` - query, err = Build(q) - require.NotNil(t, err) - - // invalid page - q = `page_size=a` - query, err = Build(q) + query, err = Build(q, 1, 10) require.NotNil(t, err) // valid query string - q = `k=v,page=1,page_size=10` - query, err = Build(q) + q = `k=v` + query, err = Build(q, 1, 10) + require.Nil(t, err) + assert.Equal(t, int64(1), query.PageNumber) + assert.Equal(t, int64(10), query.PageSize) + assert.Equal(t, "v", query.Keywords["k"].(string)) + + // contains escaped characters + q = `k%3Dv` + query, err = Build(q, 1, 10) require.Nil(t, err) assert.Equal(t, int64(1), query.PageNumber) assert.Equal(t, int64(10), query.PageSize) diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go index 4bcec6d991..bc5968a2a5 100644 --- a/src/server/v2.0/handler/artifact.go +++ b/src/server/v2.0/handler/artifact.go @@ -66,7 +66,7 @@ func (a *artifactAPI) ListArtifacts(ctx context.Context, params operation.ListAr } // set query - query, err := a.BuildQuery(ctx, params.Q) + query, err := a.BuildQuery(ctx, params.Q, params.Page, params.PageSize) if err != nil { return a.SendError(ctx, err) } diff --git a/src/server/v2.0/handler/base.go b/src/server/v2.0/handler/base.go index c99ac7691c..797d28b4a9 100644 --- a/src/server/v2.0/handler/base.go +++ b/src/server/v2.0/handler/base.go @@ -101,24 +101,22 @@ func (b *BaseAPI) RequireProjectAccess(ctx context.Context, projectIDOrName inte } // BuildQuery builds the query model according to the query string -func (b *BaseAPI) BuildQuery(ctx context.Context, query *string) (*q.Query, error) { +func (b *BaseAPI) BuildQuery(ctx context.Context, query *string, pageNumber, pageSize *int64) (*q.Query, error) { var ( - qy *q.Query - err error + qs string + pn int64 + ps int64 ) if query != nil { - qy, err = q.Build(*query) - if err != nil { - return nil, err - } + qs = *query } - if qy == nil { - qy = &q.Query{} + if pageNumber != nil { + pn = *pageNumber } - if qy.Keywords == nil { - qy.Keywords = map[string]interface{}{} + if pageSize != nil { + ps = *pageSize } - return qy, nil + return q.Build(qs, pn, ps) } // Links return Links based on the provided pagination information diff --git a/src/server/v2.0/handler/base_test.go b/src/server/v2.0/handler/base_test.go index 8cbef7682c..93b213faa0 100644 --- a/src/server/v2.0/handler/base_test.go +++ b/src/server/v2.0/handler/base_test.go @@ -30,18 +30,28 @@ func (b *baseHandlerTestSuite) SetupSuite() { } func (b *baseHandlerTestSuite) TestBuildQuery() { - // nil query string pointer - var query *string - q, err := b.base.BuildQuery(nil, query) + // nil query string and pagination pointer + var ( + query *string + pageNumber *int64 + pageSize *int64 + ) + q, err := b.base.BuildQuery(nil, query, pageNumber, pageSize) b.Require().Nil(err) b.Require().NotNil(q) b.NotNil(q.Keywords) - // not nil query string - str := "q=a=b" - q, err = b.base.BuildQuery(nil, &str) + // not nil query string and pagination pointer + var ( + qs = "q=a=b" + pn int64 = 1 + ps int64 = 10 + ) + q, err = b.base.BuildQuery(nil, &qs, &pn, &ps) b.Require().Nil(err) b.Require().NotNil(q) + b.Equal(int64(1), q.PageNumber) + b.Equal(int64(10), q.PageSize) b.NotNil(q.Keywords) } diff --git a/src/server/v2.0/handler/repository.go b/src/server/v2.0/handler/repository.go index 9cd5c83aea..47a94f26d2 100644 --- a/src/server/v2.0/handler/repository.go +++ b/src/server/v2.0/handler/repository.go @@ -54,7 +54,7 @@ func (r *repositoryAPI) ListRepositories(ctx context.Context, params operation.L } // set query - query, err := r.BuildQuery(ctx, params.Q) + query, err := r.BuildQuery(ctx, params.Q, params.Page, params.PageSize) if err != nil { return r.SendError(ctx, err) }