Skip to content

Commit

Permalink
addressing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
urvisavla committed Sep 6, 2024
1 parent a492f61 commit 5c8c0cc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
10 changes: 5 additions & 5 deletions services/horizon/internal/actions/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,10 +546,9 @@ func validateAssetParams(aType, code, issuer, prefix string) error {
// For ascending queries, we adjust the cursor to ensure it starts at
// the oldest available ledger.
func validateAndAdjustCursor(ledgerState *ledger.State, pq *db2.PageQuery) error {
err := validateCursorWithinHistory(ledgerState, *pq)

if pq.Order == db2.OrderDescending {
return validateCursorWithinHistory(ledgerState, *pq)
} else if pq.Order == db2.OrderAscending {
if pq.Order == db2.OrderAscending {
// an ascending query should never return a gone response: An ascending query
// prior to known history should return results at the beginning of history,
// and an ascending query beyond the end of history should not error out but
Expand All @@ -559,13 +558,14 @@ func validateAndAdjustCursor(ledgerState *ledger.State, pq *db2.PageQuery) error
// set/modify the cursor for ascending queries to start at the oldest available ledger if it
// precedes the oldest ledger. This avoids inefficient queries caused by index bloat from deleted rows
// that are removed as part of reaping to maintain the retention window.
if pq.Cursor == "" || errors.Is(validateCursorWithinHistory(ledgerState, *pq), &hProblem.BeforeHistory) {
if pq.Cursor == "" || errors.Is(err, &hProblem.BeforeHistory) {
pq.Cursor = toid.AfterLedger(
ordered.Max(0, ledgerState.CurrentStatus().HistoryElder-1),
).String()
return nil
}
}
return nil
return err
}

// validateCursorWithinHistory checks if the cursor is within the known history range.
Expand Down
20 changes: 14 additions & 6 deletions services/horizon/internal/actions/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,15 +401,15 @@ func TestGetPageQueryWithoutCursor(t *testing.T) {
validateCursor(2, "desc", "")
}

func TestGetPageQueryWithCursor(t *testing.T) {
func TestValidateAndAdjustCursor(t *testing.T) {
ledgerState := &ledger.State{}

validateCursor := func(cursor string, limit uint64, order string, expectedCursor string, expectedError error) {
req := makeTestActionRequest(fmt.Sprintf("/foo-bar/blah?cursor=%s&limit=%d&order=%s", cursor, limit, order), testURLParams())
pq, err := GetPageQuery(ledgerState, req)
assert.NoError(t, err)
assert.Equal(t, cursor, pq.Cursor)
err = validateAndAdjustCursor(ledgerState, &pq)
pq := db2.PageQuery{Cursor: cursor,
Limit: limit,
Order: order,
}
err := validateAndAdjustCursor(ledgerState, &pq)
if expectedError != nil {
assert.EqualError(t, expectedError, err.Error())
} else {
Expand All @@ -428,6 +428,10 @@ func TestGetPageQueryWithCursor(t *testing.T) {
ExpHistoryLatest: 7000,
})

// invalid cursor
validateCursor("blah", 2, "asc", "blah", problem.BadRequest)
validateCursor("blah", 2, "desc", "blah", problem.BadRequest)

validateCursor(toid.AfterLedger(0).String(), 2, "asc", toid.AfterLedger(0).String(), nil)
validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(200).String(), nil)
validateCursor(toid.AfterLedger(7001).String(), 2, "asc", toid.AfterLedger(7001).String(), nil)
Expand All @@ -444,6 +448,10 @@ func TestGetPageQueryWithCursor(t *testing.T) {
ExpHistoryLatest: 7000,
})

// invalid cursor
validateCursor("blah", 2, "asc", "blah", problem.BadRequest)
validateCursor("blah", 2, "desc", "blah", problem.BadRequest)

// asc order
validateCursor(toid.AfterLedger(0).String(), 2, "asc", toid.AfterLedger(299).String(), nil)
validateCursor(toid.AfterLedger(200).String(), 2, "asc", toid.AfterLedger(299).String(), nil)
Expand Down
5 changes: 1 addition & 4 deletions services/horizon/internal/actions/trade.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,7 @@ func (handler GetTradeAggregationsHandler) GetResource(w HeaderWriter, r *http.R
if err != nil {
return nil, err
}
err = validateAndAdjustCursor(handler.LedgerState, &pq)
if err != nil {
return nil, err
}

qp := TradeAggregationsQuery{}
if err = getParams(&qp, r); err != nil {
return nil, err
Expand Down

0 comments on commit 5c8c0cc

Please sign in to comment.