Skip to content

Commit

Permalink
chore: Deprecate query params and switch to camelCase (#400)
Browse files Browse the repository at this point in the history
  • Loading branch information
@flemzord
Antoine Gelloz authored and flemzord committed Jan 25, 2023
1 parent 700addc commit 1a7e01f
Show file tree
Hide file tree
Showing 18 changed files with 719 additions and 302 deletions.
80 changes: 56 additions & 24 deletions pkg/api/controllers/account_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"net/http"
"strconv"

"github.com/formancehq/go-libs/api"
"github.com/gin-gonic/gin"
"github.com/numary/ledger/pkg/api/apierrors"
"github.com/numary/ledger/pkg/core"
Expand Down Expand Up @@ -40,37 +39,74 @@ func (ctl *AccountController) CountAccounts(c *gin.Context) {
func (ctl *AccountController) GetAccounts(c *gin.Context) {
l, _ := c.Get("ledger")

var cursor api.Cursor[core.Account]
var accountsQuery *ledger.AccountsQuery
var err error
accountsQuery := ledger.NewAccountsQuery()

if c.Query("pagination_token") != "" {
if c.Query(QueryKeyCursor) != "" {
if c.Query("after") != "" ||
c.Query("address") != "" ||
len(c.QueryMap("metadata")) > 0 ||
c.Query("balance") != "" ||
c.Query("balance_operator") != "" ||
c.Query("page_size") != "" {
c.Query(QueryKeyBalanceOperator) != "" ||
c.Query(QueryKeyBalanceOperatorDeprecated) != "" ||
c.Query(QueryKeyPageSize) != "" ||
c.Query(QueryKeyPageSizeDeprecated) != "" {
apierrors.ResponseError(c, ledger.NewValidationError(
"no other query params can be set with 'pagination_token'"))
fmt.Sprintf("no other query params can be set with '%s'", QueryKeyCursor)))
return
}

res, decErr := base64.RawURLEncoding.DecodeString(c.Query("pagination_token"))
if decErr != nil {
res, err := base64.RawURLEncoding.DecodeString(c.Query(QueryKeyCursor))
if err != nil {
apierrors.ResponseError(c, ledger.NewValidationError(
"invalid query value 'pagination_token'"))
fmt.Sprintf("invalid '%s' query param", QueryKeyCursor)))
return
}

token := sqlstorage.AccPaginationToken{}
if err = json.Unmarshal(res, &token); err != nil {
if err := json.Unmarshal(res, &token); err != nil {
apierrors.ResponseError(c, ledger.NewValidationError(
"invalid query value 'pagination_token'"))
fmt.Sprintf("invalid '%s' query param", QueryKeyCursor)))
return
}

accountsQuery = ledger.NewAccountsQuery().
accountsQuery = accountsQuery.
WithOffset(token.Offset).
WithAfterAddress(token.AfterAddress).
WithAddressFilter(token.AddressRegexpFilter).
WithBalanceFilter(token.BalanceFilter).
WithBalanceOperatorFilter(token.BalanceOperatorFilter).
WithMetadataFilter(token.MetadataFilter).
WithPageSize(token.PageSize)

} else if c.Query(QueryKeyCursorDeprecated) != "" {
if c.Query("after") != "" ||
c.Query("address") != "" ||
len(c.QueryMap("metadata")) > 0 ||
c.Query("balance") != "" ||
c.Query(QueryKeyBalanceOperator) != "" ||
c.Query(QueryKeyBalanceOperatorDeprecated) != "" ||
c.Query(QueryKeyPageSize) != "" ||
c.Query(QueryKeyPageSizeDeprecated) != "" {
apierrors.ResponseError(c, ledger.NewValidationError(
fmt.Sprintf("no other query params can be set with '%s'", QueryKeyCursorDeprecated)))
return
}

res, err := base64.RawURLEncoding.DecodeString(c.Query(QueryKeyCursorDeprecated))
if err != nil {
apierrors.ResponseError(c, ledger.NewValidationError(
fmt.Sprintf("invalid '%s' query param", QueryKeyCursorDeprecated)))
return
}

token := sqlstorage.AccPaginationToken{}
if err := json.Unmarshal(res, &token); err != nil {
apierrors.ResponseError(c, ledger.NewValidationError(
fmt.Sprintf("invalid '%s' query param", QueryKeyCursorDeprecated)))
return
}

accountsQuery = accountsQuery.
WithOffset(token.Offset).
WithAfterAddress(token.AfterAddress).
WithAddressFilter(token.AddressRegexpFilter).
Expand All @@ -89,14 +125,10 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
}
}

var balanceOperator = ledger.DefaultBalanceOperator
if balanceOperatorStr := c.Query("balance_operator"); balanceOperatorStr != "" {
var ok bool
if balanceOperator, ok = ledger.NewBalanceOperator(balanceOperatorStr); !ok {
apierrors.ResponseError(c, ledger.NewValidationError(
"invalid parameter 'balance_operator', should be one of 'e, gt, gte, lt, lte'"))
return
}
balanceOperator, err := getBalanceOperator(c)
if err != nil {
apierrors.ResponseError(c, err)
return
}

pageSize, err := getPageSize(c)
Expand All @@ -105,7 +137,7 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
return
}

accountsQuery = ledger.NewAccountsQuery().
accountsQuery = accountsQuery.
WithAfterAddress(c.Query("after")).
WithAddressFilter(c.Query("address")).
WithBalanceFilter(balance).
Expand All @@ -114,7 +146,7 @@ func (ctl *AccountController) GetAccounts(c *gin.Context) {
WithPageSize(pageSize)
}

cursor, err = l.(*ledger.Ledger).GetAccounts(c.Request.Context(), *accountsQuery)
cursor, err := l.(*ledger.Ledger).GetAccounts(c.Request.Context(), *accountsQuery)
if err != nil {
apierrors.ResponseError(c, err)
return
Expand Down
83 changes: 42 additions & 41 deletions pkg/api/controllers/account_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,59 +160,60 @@ func TestGetAccounts(t *testing.T) {
to := sqlstorage.AccPaginationToken{}
raw, err := json.Marshal(to)
require.NoError(t, err)
t.Run("valid empty pagination_token", func(t *testing.T) {

t.Run(fmt.Sprintf("valid empty %s", controllers.QueryKeyCursor), func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"pagination_token": []string{base64.RawURLEncoding.EncodeToString(raw)},
controllers.QueryKeyCursor: []string{base64.RawURLEncoding.EncodeToString(raw)},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode, rsp.Body.String())
})

t.Run("valid empty pagination_token with any other param is forbidden", func(t *testing.T) {
t.Run(fmt.Sprintf("valid empty %s with any other param is forbidden", controllers.QueryKeyCursor), func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"pagination_token": []string{base64.RawURLEncoding.EncodeToString(raw)},
"after": []string{"bob"},
controllers.QueryKeyCursor: []string{base64.RawURLEncoding.EncodeToString(raw)},
"after": []string{"bob"},
})
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: apierrors.ErrValidation,
ErrorMessage: "no other query params can be set with 'pagination_token'",
ErrorMessage: fmt.Sprintf("no other query params can be set with '%s'", controllers.QueryKeyCursor),
ErrorCodeDeprecated: apierrors.ErrValidation,
ErrorMessageDeprecated: "no other query params can be set with 'pagination_token'",
ErrorMessageDeprecated: fmt.Sprintf("no other query params can be set with '%s'", controllers.QueryKeyCursor),
}, err)
})

t.Run("invalid pagination_token", func(t *testing.T) {
t.Run(fmt.Sprintf("invalid %s", controllers.QueryKeyCursor), func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"pagination_token": []string{"invalid"},
controllers.QueryKeyCursor: []string{"invalid"},
})
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: apierrors.ErrValidation,
ErrorMessage: "invalid query value 'pagination_token'",
ErrorMessage: fmt.Sprintf("invalid '%s' query param", controllers.QueryKeyCursor),
ErrorCodeDeprecated: apierrors.ErrValidation,
ErrorMessageDeprecated: "invalid query value 'pagination_token'",
ErrorMessageDeprecated: fmt.Sprintf("invalid '%s' query param", controllers.QueryKeyCursor),
}, err)
})

t.Run("invalid pagination_token not base64", func(t *testing.T) {
t.Run(fmt.Sprintf("invalid %s not base64", controllers.QueryKeyCursor), func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"pagination_token": []string{"\n*@"},
controllers.QueryKeyCursor: []string{"\n*@"},
})
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: apierrors.ErrValidation,
ErrorMessage: "invalid query value 'pagination_token'",
ErrorMessage: fmt.Sprintf("invalid '%s' query param", controllers.QueryKeyCursor),
ErrorCodeDeprecated: apierrors.ErrValidation,
ErrorMessageDeprecated: "invalid query value 'pagination_token'",
ErrorMessageDeprecated: fmt.Sprintf("invalid '%s' query param", controllers.QueryKeyCursor),
}, err)
})

Expand All @@ -239,8 +240,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance >= 50", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"50"},
"balance_operator": []string{"gte"},
"balance": []string{"50"},
controllers.QueryKeyBalanceOperator: []string{"gte"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -251,8 +252,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance >= 120", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"120"},
"balance_operator": []string{"gte"},
"balance": []string{"120"},
controllers.QueryKeyBalanceOperator: []string{"gte"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -262,8 +263,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance > 120", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"120"},
"balance_operator": []string{"gt"},
"balance": []string{"120"},
controllers.QueryKeyBalanceOperator: []string{"gt"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -273,8 +274,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance < 0", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"0"},
"balance_operator": []string{"lt"},
"balance": []string{"0"},
controllers.QueryKeyBalanceOperator: []string{"lt"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -284,8 +285,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance < 100", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"100"},
"balance_operator": []string{"lt"},
"balance": []string{"100"},
controllers.QueryKeyBalanceOperator: []string{"lt"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -295,8 +296,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance <= 100", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"100"},
"balance_operator": []string{"lte"},
"balance": []string{"100"},
controllers.QueryKeyBalanceOperator: []string{"lte"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -307,8 +308,8 @@ func TestGetAccounts(t *testing.T) {

t.Run("filter by balance = 100", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"100"},
"balance_operator": []string{"e"},
"balance": []string{"100"},
controllers.QueryKeyBalanceOperator: []string{"e"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -319,8 +320,8 @@ func TestGetAccounts(t *testing.T) {
// test filter by balance != 100
t.Run("filter by balance != 100", func(t *testing.T) {
rsp = internal.GetAccounts(api, url.Values{
"balance": []string{"100"},
"balance_operator": []string{"ne"},
"balance": []string{"100"},
controllers.QueryKeyBalanceOperator: []string{"ne"},
})
assert.Equal(t, http.StatusOK, rsp.Result().StatusCode)
cursor := internal.DecodeCursorResponse[core.Account](t, rsp.Body)
Expand All @@ -345,20 +346,20 @@ func TestGetAccounts(t *testing.T) {
}, err)
})

t.Run("invalid balance_operator", func(t *testing.T) {
t.Run("invalid balance operator", func(t *testing.T) {
rsp := internal.GetAccounts(api, url.Values{
"balance": []string{"100"},
"balance_operator": []string{"toto"},
"balance": []string{"100"},
controllers.QueryKeyBalanceOperator: []string{"toto"},
})
assert.Equal(t, http.StatusBadRequest, rsp.Result().StatusCode)

err := sharedapi.ErrorResponse{}
internal.Decode(t, rsp.Body, &err)
assert.EqualValues(t, sharedapi.ErrorResponse{
ErrorCode: apierrors.ErrValidation,
ErrorMessage: "invalid parameter 'balance_operator', should be one of 'e, gt, gte, lt, lte'",
ErrorMessage: controllers.ErrInvalidBalanceOperator.Error(),
ErrorCodeDeprecated: apierrors.ErrValidation,
ErrorMessageDeprecated: "invalid parameter 'balance_operator', should be one of 'e, gt, gte, lt, lte'",
ErrorMessageDeprecated: controllers.ErrInvalidBalanceOperator.Error(),
}, err)
})

Expand All @@ -383,7 +384,7 @@ func TestGetAccountsWithPageSize(t *testing.T) {

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

Expand All @@ -398,7 +399,7 @@ func TestGetAccountsWithPageSize(t *testing.T) {
})
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)},
controllers.QueryKeyPageSize: []string{fmt.Sprintf("%d", 2*controllers.MaxPageSize)},
})
assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())

Expand All @@ -410,8 +411,8 @@ func TestGetAccountsWithPageSize(t *testing.T) {
})
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)},
controllers.QueryKeyPageSize: []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())

Expand All @@ -423,7 +424,7 @@ func TestGetAccountsWithPageSize(t *testing.T) {
})
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)},
controllers.QueryKeyPageSize: []string{fmt.Sprintf("%d", controllers.MaxPageSize/10)},
})
assert.Equal(t, http.StatusOK, httpResponse.Result().StatusCode, httpResponse.Body.String())

Expand Down
Loading

0 comments on commit 1a7e01f

Please sign in to comment.