Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

transfers: read various filter params in http request #474

Merged
merged 4 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions client/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1168,8 +1168,8 @@ paths:
minimum: 0
type: integer
style: form
- description: Filter objects created after this date. ISO-8601 format YYYY-MM-DD.
Can optionally be used with endDate to specify a date range.
- description: |
Return Transfers that are scheduled for this date or later in ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range.
explode: true
in: query
name: startDate
Expand All @@ -1178,8 +1178,8 @@ paths:
format: date-time
type: string
style: form
- description: Filter objects created before this date. ISO-8601 format YYYY-MM-DD.
Can optionally be used with startDate to specify a date range.
- description: |
Return Transfers that are scheduled for this date or earlier in ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range.
explode: true
in: query
name: endDate
Expand Down
4 changes: 2 additions & 2 deletions client/api_transfers.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,8 +802,8 @@ List all Transfers created for the given userID.
* @param optional nil or *GetTransfersOpts - Optional Parameters:
* @param "Offset" (optional.Int32) - The number of items to skip before starting to collect the result set
* @param "Limit" (optional.Int32) - The number of items to return
* @param "StartDate" (optional.Time) - Filter objects created after this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range.
* @param "EndDate" (optional.Time) - Filter objects created before this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range.
* @param "StartDate" (optional.Time) - Return Transfers that are scheduled for this date or later in ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range.
* @param "EndDate" (optional.Time) - Return Transfers that are scheduled for this date or earlier in ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range.
* @param "XRequestID" (optional.String) - Optional Request ID allows application developer to trace requests through the systems logs
@return []Transfer
*/
Expand Down
4 changes: 2 additions & 2 deletions client/docs/TransfersApi.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ Name | Type | Description | Notes

**offset** | **optional.Int32**| The number of items to skip before starting to collect the result set | [default to 0]
**limit** | **optional.Int32**| The number of items to return | [default to 25]
**startDate** | **optional.Time**| Filter objects created after this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range. |
**endDate** | **optional.Time**| Filter objects created before this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range. |
**startDate** | **optional.Time**| Return Transfers that are scheduled for this date or later in ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range. |
**endDate** | **optional.Time**| Return Transfers that are scheduled for this date or earlier in ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range. |
**xRequestID** | **optional.String**| Optional Request ID allows application developer to trace requests through the systems logs |

### Return type
Expand Down
36 changes: 36 additions & 0 deletions internal/route/params.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2020 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package route

import (
"math"
"net/http"
"strconv"
)

const (
maxLimit int64 = 1000
)

// ReadOffset returns the "offset" query param from a request or zero if it's missing.
func ReadOffset(r *http.Request) int64 {
return readIntQueryParam(r, "offset", math.MaxInt64)
}

// ReadLimit returns the "limit" query param from a request or zero if it's missing.
func ReadLimit(r *http.Request) int64 {
return readIntQueryParam(r, "limit", maxLimit)
}

func readIntQueryParam(r *http.Request, key string, max int64) int64 {
if v := r.URL.Query().Get(key); v != "" {
limit, _ := strconv.ParseInt(v, 10, 32)
if limit > max {
return max
}
return limit
}
return 0
}
47 changes: 47 additions & 0 deletions internal/route/params_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2020 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package route

import (
"net/http"
"net/url"
"testing"
)

func TestReadLimit(t *testing.T) {
req := makeRequest(t, "http://localhost:8082/transfers?limit=27")
if limit := ReadLimit(req); limit != 27 {
t.Errorf("limit=%d", limit)
}

req = makeRequest(t, "http://localhost:8082/transfers?limit=2700")
if limit := ReadLimit(req); limit != 1000 {
t.Errorf("limit=%d", limit)
}

req = makeRequest(t, "http://localhost:8082/transfers")
if limit := ReadLimit(req); limit != 0 {
t.Errorf("limit=%d", limit)
}
}

func TestReadOffset(t *testing.T) {
req := makeRequest(t, "http://localhost:8082/transfers?offset=27")
if offset := ReadOffset(req); offset != 27 {
t.Errorf("offset=%d", offset)
}

req = makeRequest(t, "http://localhost:8082/transfers")
if offset := ReadOffset(req); offset != 0 {
t.Errorf("offset=%d", offset)
}
}

func makeRequest(t *testing.T, in string) *http.Request {
u, _ := url.Parse(in)
return &http.Request{
URL: u,
}
}
2 changes: 1 addition & 1 deletion internal/transfers/mock_transfer_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type MockRepository struct {
Status model.TransferStatus
}

func (r *MockRepository) getUserTransfers(userID id.User) ([]*model.Transfer, error) {
func (r *MockRepository) getUserTransfers(userID id.User, params transferFilterParams) ([]*model.Transfer, error) {
if r.Err != nil {
return nil, r.Err
}
Expand Down
10 changes: 6 additions & 4 deletions internal/transfers/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

type Repository interface {
getUserTransfers(userID id.User) ([]*model.Transfer, error)
getUserTransfers(userID id.User, params transferFilterParams) ([]*model.Transfer, error)
getUserTransfer(id id.Transfer, userID id.User) (*model.Transfer, error)
UpdateTransferStatus(id id.Transfer, status model.TransferStatus) error

Expand Down Expand Up @@ -60,15 +60,17 @@ func (r *SQLRepo) Close() error {
return r.db.Close()
}

func (r *SQLRepo) getUserTransfers(userID id.User) ([]*model.Transfer, error) {
query := `select transfer_id from transfers where user_id = ? and deleted_at is null`
func (r *SQLRepo) getUserTransfers(userID id.User, params transferFilterParams) ([]*model.Transfer, error) {
query := `select transfer_id from transfers
where user_id = ? and created_at >= ? and created_at <= ? and deleted_at is null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an index for created_at thats desc ?

Copy link
Member

@InfernoJJ InfernoJJ Apr 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that its X or equal to Is it fine to get duplicates if they query this by sequential dates? ignore me lol doesn't apply here as I was thinking more about it,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no index on created_at desc for transfers. We should add one... Thanks for the catch

order by created_at desc limit ? offset ?;`
stmt, err := r.db.Prepare(query)
if err != nil {
return nil, err
}
defer stmt.Close()

rows, err := stmt.Query(userID)
rows, err := stmt.Query(userID, params.StartDate, params.EndDate, params.Limit, params.Offset)
Copy link
Member

@InfernoJJ InfernoJJ Apr 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we query is params.EndDate time segment 23:59:999.000 ? I could have missed it but didn't see anything below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's now() + "24 hours" basically. We could bump it forward more.

EndDate:   time.Now().Add(24 * time.Hour),

In the future we need to support #447 (scheduled transfers) where I think this endpoint should order by scheduled_at or created_at (coalesce the two) because transfers without scheduled_at are to be uploaded/posted on the current day.

if err != nil {
return nil, err
}
Expand Down
8 changes: 5 additions & 3 deletions internal/transfers/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ func TestTransfers__transactionID(t *testing.T) {
t.Fatal(err)
}

transfers, err := repo.getUserTransfers(userID)
params := readTransferFilterParams(nil)
transfers, err := repo.getUserTransfers(userID, params)
if err != nil || len(transfers) != 1 {
t.Errorf("got %d Transfers (error=%v): %v", len(transfers), err, transfers)
}
Expand Down Expand Up @@ -198,7 +199,8 @@ func TestTransfers__SetReturnCode(t *testing.T) {
t.Fatal(err)
}

transfers, err := repo.getUserTransfers(userID)
params := readTransferFilterParams(nil)
transfers, err := repo.getUserTransfers(userID, params)
if err != nil || len(transfers) != 1 {
t.Errorf("got %d Transfers (error=%v): %v", len(transfers), err, transfers)
}
Expand All @@ -209,7 +211,7 @@ func TestTransfers__SetReturnCode(t *testing.T) {
}

// Verify
transfers, err = repo.getUserTransfers(userID)
transfers, err = repo.getUserTransfers(userID, params)
if err != nil || len(transfers) != 1 {
t.Errorf("got %d Transfers (error=%v): %v", len(transfers), err, transfers)
}
Expand Down
38 changes: 37 additions & 1 deletion internal/transfers/transfers.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"io/ioutil"
"net/http"
"strings"
"time"

"github.com/moov-io/ach"
"github.com/moov-io/base"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/moov-io/paygate/internal/receivers"
"github.com/moov-io/paygate/internal/remoteach"
"github.com/moov-io/paygate/internal/route"
"github.com/moov-io/paygate/internal/util"
"github.com/moov-io/paygate/pkg/achclient"
"github.com/moov-io/paygate/pkg/id"

Expand Down Expand Up @@ -185,14 +187,48 @@ func getTransferID(r *http.Request) id.Transfer {
return id.Transfer("")
}

type transferFilterParams struct {
StartDate time.Time
EndDate time.Time
Limit int64
Offset int64
}

func readTransferFilterParams(r *http.Request) transferFilterParams {
params := transferFilterParams{
StartDate: time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC),
EndDate: time.Now().Add(24 * time.Hour),
Limit: 100,
Offset: 0,
}
if r == nil {
return params
}
if v := r.URL.Query().Get("startDate"); v != "" {
params.StartDate = util.FirstParsedTime(v, base.ISO8601Format, util.YYMMDDTimeFormat)
}
if v := r.URL.Query().Get("endDate"); v != "" {
params.EndDate, _ = time.Parse(base.ISO8601Format, v)
fmt.Printf("params.EndDate=%v\n", params.EndDate)
}
if limit := route.ReadLimit(r); limit != 0 {
params.Limit = limit
}
if offset := route.ReadOffset(r); offset != 0 {
params.Offset = offset
}
return params
}

func (c *TransferRouter) getUserTransfers() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
responder := route.NewResponder(c.logger, w, r)
if responder == nil {
return
}

transfers, err := c.transferRepo.getUserTransfers(responder.XUserID)
params := readTransferFilterParams(r)
transfers, err := c.transferRepo.getUserTransfers(responder.XUserID, params)
if err != nil {
responder.Log("transfers", fmt.Sprintf("error getting user transfers: %v", err))
responder.Problem(err)
Expand Down
21 changes: 21 additions & 0 deletions internal/transfers/transfers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"
Expand All @@ -29,6 +30,7 @@ import (
"github.com/moov-io/paygate/internal/receivers"
"github.com/moov-io/paygate/internal/route"
"github.com/moov-io/paygate/internal/secrets"
"github.com/moov-io/paygate/internal/util"
"github.com/moov-io/paygate/pkg/achclient"
"github.com/moov-io/paygate/pkg/id"

Expand Down Expand Up @@ -657,6 +659,25 @@ func TestTransfers__getUserTransfer(t *testing.T) {
}
}

func TestTransfers__readTransferFilterParams(t *testing.T) {
u, _ := url.Parse("http://localhost:8082/transfers?startDate=2020-04-06&limit=10")
req := &http.Request{URL: u}
params := readTransferFilterParams(req)

if params.StartDate.Format(util.YYMMDDTimeFormat) != "2020-04-06" {
t.Errorf("unexpected StartDate: %v", params.StartDate)
}
if !params.EndDate.After(time.Now()) {
t.Errorf("unexpected EndDate: %v", params.EndDate)
}
if params.Limit != 10 {
t.Errorf("unexpected limit: %d", params.Limit)
}
if params.Offset != 0 {
t.Errorf("unexpected offset: %d", params.Offset)
}
}

func TestTransfers__getUserTransfers(t *testing.T) {
db := database.CreateTestSqliteDB(t)
defer db.Close()
Expand Down
24 changes: 24 additions & 0 deletions internal/util/time.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2020 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package util

import (
"time"
)

const (
YYMMDDTimeFormat = "2006-01-02"
)

// FirstParsedTime attempts to parse v with all provided formats in order.
// The first parsed time.Time that doesn't error is returned.
func FirstParsedTime(v string, formats ...string) time.Time {
for i := range formats {
if tt, err := time.Parse(formats[i], v); err == nil {
return tt
}
}
return time.Time{}
}
27 changes: 27 additions & 0 deletions internal/util/time_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2020 The Moov Authors
// Use of this source code is governed by an Apache License
// license that can be found in the LICENSE file.

package util

import (
"testing"
"time"
)

func TestFirstParsedTime(t *testing.T) {
tt := FirstParsedTime("2020-04-07")
if !tt.IsZero() {
t.Errorf("expected zero, got %v", tt)
}

tt = FirstParsedTime("2020-04-07", YYMMDDTimeFormat)
if v := tt.Format(YYMMDDTimeFormat); v != "2020-04-07" {
t.Errorf("got %v", v)
}

tt = FirstParsedTime(time.Now().Format(time.RFC3339), YYMMDDTimeFormat)
if !tt.IsZero() {
t.Errorf("expected zero, got %v", tt)
}
}
6 changes: 4 additions & 2 deletions openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -929,14 +929,16 @@ paths:
example: 10
- name: startDate
in: query
description: Filter objects created after this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range.
description: |
Return Transfers that are scheduled for this date or later in ISO-8601 format YYYY-MM-DD. Can optionally be used with endDate to specify a date range.
schema:
type: string
format: date-time
example: 2006-01-02T15:04:05Z07:00
- name: endDate
in: query
description: Filter objects created before this date. ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range.
description: |
Return Transfers that are scheduled for this date or earlier in ISO-8601 format YYYY-MM-DD. Can optionally be used with startDate to specify a date range.
schema:
type: string
format: date-time
Expand Down