Accept the pagination information in the separated query string (#10991)

Accept the pagination information in the separated query string

Signed-off-by: Wenkai Yin <yinw@vmware.com>

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.

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).

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)

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)
 	}

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

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)
 }
 

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)
 	}