Skip to content

Commit

Permalink
feat(api): Add configurable page_size on paginated request.
Browse files Browse the repository at this point in the history
This add the query parameter 'page_size' on both GET /transactions, GET /accounts, and GET /balances endpoints.
The parameter is capped to a value of 1000.
The default value still the same as before (15).

The PR also fix some tests around pagination which was working but was not doing the right thing (see api/controllers/pagination_test.go line 209).
  • Loading branch information
gfyrag committed Jul 8, 2022
1 parent eb12452 commit 83bc397
Show file tree
Hide file tree
Showing 22 changed files with 419 additions and 123 deletions.
15 changes: 12 additions & 3 deletions pkg/api/controllers/account_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
c.Query("address") != "" ||
len(c.QueryMap("metadata")) > 0 ||
c.Query("balance") != "" ||
c.Query("balance_operator") != "" {
c.Query("balance_operator") != "" ||
c.Query("page_size") != "" {
ResponseError(c, ledger.NewValidationError(
"no other query params can be set with 'pagination_token'"))
return
Expand All @@ -75,7 +76,8 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
WithAddressFilter(token.AddressRegexpFilter).
WithBalanceFilter(token.BalanceFilter).
WithBalanceOperatorFilter(token.BalanceOperatorFilter).
WithMetadataFilter(token.MetadataFilter)
WithMetadataFilter(token.MetadataFilter).
WithPageSize(token.PageSize)

} else {
balance := c.Query("balance")
Expand All @@ -97,12 +99,19 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
}
}

pageSize, err := getPageSize(c)
if err != nil {
ResponseError(c, err)
return
}

accountsQuery = storage.NewAccountsQuery().
WithAfterAddress(c.Query("after")).
WithAddressFilter(c.Query("address")).
WithBalanceFilter(balance).
WithBalanceOperatorFilter(balanceOperator).
WithMetadataFilter(c.QueryMap("metadata"))
WithMetadataFilter(c.QueryMap("metadata")).
WithPageSize(pageSize)
}

cursor, err = l.(*ledger.Ledger).GetAccounts(c.Request.Context(), *accountsQuery)
Expand Down
80 changes: 80 additions & 0 deletions pkg/api/controllers/account_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/url"
"testing"
Expand All @@ -13,6 +14,7 @@ import (
"github.com/numary/ledger/pkg/api/controllers"
"github.com/numary/ledger/pkg/api/internal"
"github.com/numary/ledger/pkg/core"
"github.com/numary/ledger/pkg/storage"
"github.com/numary/ledger/pkg/storage/sqlstorage"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -336,6 +338,84 @@ func TestGetAccounts(t *testing.T) {
}))
}

func TestGetAccountsWithPageSize(t *testing.T) {
internal.RunTest(t, fx.Invoke(func(lc fx.Lifecycle, api *api.API, driver storage.Driver) {
lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
var previousLog *core.Log
logs := make([]core.Log, 0)
store := internal.GetStore(t, driver, context.Background())

for i := 0; i < 3*controllers.MaxPageSize; i++ {
log := core.NewSetMetadataLog(previousLog, core.SetMetadata{
TargetID: fmt.Sprintf("accounts:%06d", i),
TargetType: core.MetaTargetTypeAccount,
Metadata: map[string]json.RawMessage{
"foo": []byte("{}"),
},
})
logs = append(logs, log)
previousLog = &log
}
require.NoError(t, store.AppendLog(context.Background(), logs...))

t.Run("invalid page size", func(t *testing.T) {
rsp := internal.GetAccounts(api, url.Values{
"page_size": []string{"nan"},
})
assert.Equal(t, http.StatusBadRequest, rsp.Result().StatusCode, rsp.Body.String())

err := sharedapi.ErrorResponse{}
internal.Decode(t, rsp.Body, &err)
assert.EqualValues(t, sharedapi.ErrorResponse{
ErrorCode: controllers.ErrValidation,
ErrorMessage: controllers.ErrInvalidPageSize.Error(),
}, err)
})
t.Run("page size over maximum", func(t *testing.T) {
httpResponse := internal.GetAccounts(api, url.Values{
"page_size": []string{fmt.Sprintf("%d", 2*controllers.MaxPageSize)},
})
assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())

cursor := internal.DecodeCursorResponse[core.Account](t, httpResponse.Body)
assert.Len(t, cursor.Data, controllers.MaxPageSize)
assert.Equal(t, cursor.PageSize, controllers.MaxPageSize)
assert.NotEmpty(t, cursor.Next)
assert.True(t, cursor.HasMore)
})
t.Run("with page size greater than max count", func(t *testing.T) {
httpResponse := internal.GetAccounts(api, url.Values{
"page_size": []string{fmt.Sprintf("%d", controllers.MaxPageSize)},
"after": []string{fmt.Sprintf("accounts:%06d", controllers.MaxPageSize-100)},
})
assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())

cursor := internal.DecodeCursorResponse[core.Account](t, httpResponse.Body)
assert.Len(t, cursor.Data, controllers.MaxPageSize-100)
assert.Equal(t, controllers.MaxPageSize, cursor.PageSize)
assert.Empty(t, cursor.Next)
assert.False(t, cursor.HasMore)
})
t.Run("with page size lower than max count", func(t *testing.T) {
httpResponse := internal.GetAccounts(api, url.Values{
"page_size": []string{fmt.Sprintf("%d", controllers.MaxPageSize/10)},
})
assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())

cursor := internal.DecodeCursorResponse[core.Account](t, httpResponse.Body)
assert.Len(t, cursor.Data, controllers.MaxPageSize/10)
assert.Equal(t, cursor.PageSize, controllers.MaxPageSize/10)
assert.NotEmpty(t, cursor.Next)
assert.True(t, cursor.HasMore)
})

return nil
},
})
}))
}

func TestGetAccount(t *testing.T) {
internal.RunTest(t, fx.Invoke(func(lc fx.Lifecycle, api *api.API) {
lc.Append(fx.Hook{
Expand Down
16 changes: 13 additions & 3 deletions pkg/api/controllers/balance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ func (ctl *BalanceController) GetBalances(c *gin.Context) {

if c.Query("pagination_token") != "" {
if c.Query("after") != "" ||
c.Query("address") != "" {
c.Query("address") != "" ||
c.Query("page_size") != "" {
ResponseError(c, ledger.NewValidationError(
"no other query params can be set with 'pagination_token'"))
return
Expand All @@ -69,12 +70,21 @@ func (ctl *BalanceController) GetBalances(c *gin.Context) {
balancesQuery = balancesQuery.
WithOffset(token.Offset).
WithAfterAddress(token.AfterAddress).
WithAddressFilter(token.AddressRegexpFilter)
WithAddressFilter(token.AddressRegexpFilter).
WithPageSize(token.PageSize)

} else {

pageSize, err := getPageSize(c)
if err != nil {
ResponseError(c, err)
return
}

balancesQuery = balancesQuery.
WithAfterAddress(c.Query("after")).
WithAddressFilter(c.Query("address"))
WithAddressFilter(c.Query("address")).
WithPageSize(pageSize)
}

cursor, err := l.(*ledger.Ledger).GetBalances(c.Request.Context(), *balancesQuery)
Expand Down
Loading

0 comments on commit 83bc397

Please sign in to comment.