Skip to content

Commit

Permalink
Update resource list endpoint pagination to handle null and empty str…
Browse files Browse the repository at this point in the history 
…ing values better (#160)

* Update the user package to handle all scenarios of pagination on nullable columns and scenarios where the before/after value is empty string

* Update list middleware such that:
1. afterValue and/or beforeValue can be omitted when sorting by a column other than the default column in order to denote NULL.
2. optional values on ListParams are nullable (pointers)

* Update features list tests

* Dereference listParams.Query in fmt.Sprintf calls

* Consolidate users empty string and default cases for beforeValue/afterValue

* Update user queries to correctly handle queries where beforeId and/or beforeValue is passed

When a `beforeId` or `beforeValue` is passed along with a limit, the results passed should be the immediate N objects where the `sortBy` value is < `beforeId` or `beforeValue`.

For example, if we have a query with the following params: `?sortBy=id&sortOrder=ASC` we may get the following result set: `[1, 2, 3, 4, 5]`. If we then updated the query to be: `?sortBy=id&sortOrder=ASC&beforeId=4&limit=2`, we should expect the result set to be `[2, 3]` rather than `[1, 2]`.

This commit updates the beforeId/beforeValue cases to invert the sort order when making the original query, then reversing the result set.

* Update middleware to prevent beforeId/beforeValue from being passed with afterId/afterValue

* Update user queries to handle nil `beforeValue`/`afterValue` for both sort orders and non-default sort bys

* Update pricing tier queries to correct handle all cursor pagination cases

* Update role queries to correct handle all cursor pagination cases

* Update permission queries to correct handle all cursor pagination cases

* Update tenant queries to correct handle all cursor pagination cases

* Update object queries to correct handle all cursor pagination cases

* Update feature queries to correct handle all cursor pagination cases

* Update object type queries to correct handle all cursor pagination cases

* Update error message

* Remove nested if for nil `beforeValue`/`afterValue` cases

* Update postgres queries to sort null objects correctly

* Use transformed defaultSort in tenant and pricing-tier postgres queries

---------

Co-authored-by: stanleyphu <[email protected]>
  • Loading branch information
@kkajla12 @stanleyphu
kkajla12 and stanleyphu authored Jun 20, 2023
1 parent 725e35c commit 3fa1cdc
Show file tree
Hide file tree
Showing 53 changed files with 4,237 additions and 1,133 deletions.
2 changes: 1 addition & 1 deletion pkg/authz/feature/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func GetHandler(svc FeatureService, w http.ResponseWriter, r *http.Request) erro
}

func ListHandler(svc FeatureService, w http.ResponseWriter, r *http.Request) error {
listParams := service.GetListParamsFromContext(r.Context())
listParams := service.GetListParamsFromContext[FeatureListParamParser](r.Context())
features, err := svc.List(r.Context(), listParams)
if err != nil {
return err
Expand Down
7 changes: 4 additions & 3 deletions pkg/authz/feature/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"time"
)

const defaultSortBy = "featureId"

type FeatureListParamParser struct{}

func (parser FeatureListParamParser) GetDefaultSortBy() string {
return "featureId"
return defaultSortBy
}

func (parser FeatureListParamParser) GetSupportedSortBys() []string {
Expand All @@ -23,14 +25,13 @@ func (parser FeatureListParamParser) ParseValue(val string, sortBy string) (inte
return nil, fmt.Errorf("must be a valid time in the format %s", time.RFC3339)
}

return afterValue, nil
return &afterValue, nil
case "featureId":
if val == "" {
return nil, fmt.Errorf("must not be empty")
}

return val, nil

case "name":
if val == "" {
return "", nil
Expand Down
108 changes: 78 additions & 30 deletions pkg/authz/feature/mysql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,74 +122,122 @@ func (repo MySQLRepository) List(ctx context.Context, listParams service.ListPar
`
replacements := []interface{}{}

if listParams.Query != "" {
searchTermReplacement := fmt.Sprintf("%%%s%%", listParams.Query)
query = fmt.Sprintf("%s AND (featureId LIKE ? OR name LIKE ?)", query)
if listParams.Query != nil {
searchTermReplacement := fmt.Sprintf("%%%s%%", *listParams.Query)
query = fmt.Sprintf("%s AND (%s LIKE ? OR name LIKE ?)", query, defaultSortBy)
replacements = append(replacements, searchTermReplacement, searchTermReplacement)
}

if listParams.AfterId != "" {
if listParams.AfterValue != nil {
if listParams.AfterId != nil {
comparisonOp := "<"
if listParams.SortOrder == service.SortOrderAsc {
comparisonOp = ">"
}

switch listParams.AfterValue {
case nil:
if listParams.SortBy == defaultSortBy {
query = fmt.Sprintf("%s AND %s %s ?", query, defaultSortBy, comparisonOp)
replacements = append(replacements, listParams.AfterId)
} else if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s IS NOT NULL OR (%s %s ? AND %s IS NULL))", query, listParams.SortBy, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.AfterId,
)
} else {
query = fmt.Sprintf("%s AND (%s %s ? AND %s IS NULL)", query, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.AfterId,
)
}
default:
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s > ? OR (featureId > ? AND %s = ?))", query, listParams.SortBy, listParams.SortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR (%s %s ? AND %s = ?))", query, listParams.SortBy, comparisonOp, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.AfterValue,
listParams.AfterId,
listParams.AfterValue,
)
} else {
query = fmt.Sprintf("%s AND (%s < ? OR (featureId < ? AND %s = ?))", query, listParams.SortBy, listParams.SortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR %s IS NULL OR (%s %s ? AND %s = ?))", query, listParams.SortBy, comparisonOp, listParams.SortBy, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.AfterValue,
listParams.AfterId,
listParams.AfterValue,
)
}
} else {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND featureId > ?", query)
replacements = append(replacements, listParams.AfterId)
} else {
query = fmt.Sprintf("%s AND featureId < ?", query)
replacements = append(replacements, listParams.AfterId)
}
}
}

if listParams.BeforeId != "" {
if listParams.BeforeValue != nil {
if listParams.BeforeId != nil {
comparisonOp := ">"
if listParams.SortOrder == service.SortOrderAsc {
comparisonOp = "<"
}

switch listParams.BeforeValue {
case nil:
if listParams.SortBy == defaultSortBy {
query = fmt.Sprintf("%s AND %s %s ?", query, defaultSortBy, comparisonOp)
replacements = append(replacements, listParams.BeforeId)
} else if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s %s ? AND %s IS NULL)", query, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.BeforeId,
)
} else {
query = fmt.Sprintf("%s AND (%s IS NOT NULL OR (%s %s ? AND %s IS NULL))", query, listParams.SortBy, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.BeforeId,
)
}
default:
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s < ? OR (featureId < ? AND %s = ?))", query, listParams.SortBy, listParams.SortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR %s IS NULL OR (%s %s ? AND %s = ?))", query, listParams.SortBy, comparisonOp, listParams.SortBy, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.BeforeValue,
listParams.BeforeId,
listParams.BeforeValue,
)
} else {
query = fmt.Sprintf("%s AND (%s > ? OR (featureId > ? AND %s = ?))", query, listParams.SortBy, listParams.SortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR (%s %s ? AND %s = ?))", query, listParams.SortBy, comparisonOp, defaultSortBy, comparisonOp, listParams.SortBy)
replacements = append(replacements,
listParams.BeforeValue,
listParams.BeforeId,
listParams.BeforeValue,
)
}
}
}

if listParams.BeforeId != nil {
if listParams.SortBy != defaultSortBy {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s ORDER BY %s %s, %s %s LIMIT ?", query, listParams.SortBy, service.SortOrderDesc, defaultSortBy, service.SortOrderDesc)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY %s %s, %s %s LIMIT ?", query, listParams.SortBy, service.SortOrderAsc, defaultSortBy, service.SortOrderAsc)
replacements = append(replacements, listParams.Limit)
}
query = fmt.Sprintf("With result_set AS (%s) SELECT * FROM result_set ORDER BY %s %s, %s %s", query, listParams.SortBy, listParams.SortOrder, defaultSortBy, listParams.SortOrder)
} else {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND featureId < ?", query)
replacements = append(replacements, listParams.AfterId)
query = fmt.Sprintf("%s ORDER BY %s %s LIMIT ?", query, listParams.SortBy, service.SortOrderDesc)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s AND featureId > ?", query)
replacements = append(replacements, listParams.AfterId)
query = fmt.Sprintf("%s ORDER BY %s %s LIMIT ?", query, listParams.SortBy, service.SortOrderAsc)
replacements = append(replacements, listParams.Limit)
}
query = fmt.Sprintf("With result_set AS (%s) SELECT * FROM result_set ORDER BY %s %s", query, listParams.SortBy, listParams.SortOrder)
}
}

if listParams.SortBy != "featureId" {
query = fmt.Sprintf("%s ORDER BY %s %s, featureId %s LIMIT ?", query, listParams.SortBy, listParams.SortOrder, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY featureId %s LIMIT ?", query, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
if listParams.SortBy != defaultSortBy {
query = fmt.Sprintf("%s ORDER BY %s %s, %s %s LIMIT ?", query, listParams.SortBy, listParams.SortOrder, defaultSortBy, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY %s %s LIMIT ?", query, defaultSortBy, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
}
}

err := repo.DB(ctx).SelectContext(
Expand Down
117 changes: 84 additions & 33 deletions pkg/authz/feature/postgres.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,81 +120,132 @@ func (repo PostgresRepository) List(ctx context.Context, listParams service.List
deleted_at IS NULL
`
replacements := []interface{}{}
defaultSort := regexp.MustCompile("([A-Z])").ReplaceAllString(defaultSortBy, `_$1`)
sortBy := regexp.MustCompile("([A-Z])").ReplaceAllString(listParams.SortBy, `_$1`)

if listParams.Query != "" {
searchTermReplacement := fmt.Sprintf("%%%s%%", listParams.Query)
query = fmt.Sprintf("%s AND (feature_id LIKE ? OR name LIKE ?)", query)
if listParams.Query != nil {
searchTermReplacement := fmt.Sprintf("%%%s%%", *listParams.Query)
query = fmt.Sprintf("%s AND (%s LIKE ? OR name LIKE ?)", query, defaultSort)
replacements = append(replacements, searchTermReplacement, searchTermReplacement)
}

sortBy := regexp.MustCompile("([A-Z])").ReplaceAllString(listParams.SortBy, `_$1`)
if listParams.AfterId != "" {
if listParams.AfterValue != nil {
if listParams.AfterId != nil {
comparisonOp := "<"
if listParams.SortOrder == service.SortOrderAsc {
comparisonOp = ">"
}

switch listParams.AfterValue {
case nil:
if listParams.SortBy == defaultSortBy {
query = fmt.Sprintf("%s AND %s %s ?", query, defaultSort, comparisonOp)
replacements = append(replacements, listParams.AfterId)
} else if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s IS NOT NULL OR (%s %s ? AND %s IS NULL))", query, sortBy, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.AfterId,
)
} else {
query = fmt.Sprintf("%s AND (%s %s ? AND %s IS NULL)", query, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.AfterId,
)
}
default:
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s > ? OR (feature_id > ? AND %s = ?))", query, sortBy, sortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR (%s %s ? AND %s = ?))", query, sortBy, comparisonOp, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.AfterValue,
listParams.AfterId,
listParams.AfterValue,
)
} else {
query = fmt.Sprintf("%s AND (%s < ? OR (feature_id < ? AND %s = ?))", query, sortBy, sortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR %s IS NULL OR (%s %s ? AND %s = ?))", query, sortBy, comparisonOp, sortBy, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.AfterValue,
listParams.AfterId,
listParams.AfterValue,
)
}
} else {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND feature_id > ?", query)
replacements = append(replacements, listParams.AfterId)
} else {
query = fmt.Sprintf("%s AND feature_id < ?", query)
replacements = append(replacements, listParams.AfterId)
}
}
}

if listParams.BeforeId != "" {
if listParams.BeforeValue != nil {
if listParams.BeforeId != nil {
comparisonOp := ">"
if listParams.SortOrder == service.SortOrderAsc {
comparisonOp = "<"
}

switch listParams.BeforeValue {
case nil:
if listParams.SortBy == defaultSortBy {
query = fmt.Sprintf("%s AND %s %s ?", query, defaultSort, comparisonOp)
replacements = append(replacements, listParams.BeforeId)
} else if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s %s ? AND %s IS NULL)", query, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.BeforeId,
)
} else {
query = fmt.Sprintf("%s AND (%s IS NOT NULL OR (%s %s ? AND %s IS NULL))", query, sortBy, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.BeforeId,
)
}
default:
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND (%s < ? OR (feature_id < ? AND %s = ?))", query, sortBy, sortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR %s IS NULL OR (%s %s ? AND %s = ?))", query, sortBy, comparisonOp, sortBy, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.BeforeValue,
listParams.BeforeId,
listParams.BeforeValue,
)
} else {
query = fmt.Sprintf("%s AND (%s > ? OR (feature_id > ? AND %s = ?))", query, sortBy, sortBy)
query = fmt.Sprintf("%s AND (%s %s ? OR (%s %s ? AND %s = ?))", query, sortBy, comparisonOp, defaultSort, comparisonOp, sortBy)
replacements = append(replacements,
listParams.BeforeValue,
listParams.BeforeId,
listParams.BeforeValue,
)
}
} else {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s AND feature_id < ?", query)
replacements = append(replacements, listParams.AfterId)
} else {
query = fmt.Sprintf("%s AND feature_id > ?", query)
replacements = append(replacements, listParams.AfterId)
}
}
}

nullSortClause := "NULLS LAST"
invertedNullSortClause := "NULLS FIRST"
if listParams.SortOrder == service.SortOrderAsc {
nullSortClause = "NULLS FIRST"
invertedNullSortClause = "NULLS LAST"
}

if listParams.SortBy != "featureId" {
query = fmt.Sprintf("%s ORDER BY %s %s %s, feature_id %s LIMIT ?", query, sortBy, listParams.SortOrder, nullSortClause, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
if listParams.BeforeId != nil {
if listParams.SortBy != defaultSortBy {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s ORDER BY %s %s %s, %s %s LIMIT ?", query, sortBy, service.SortOrderDesc, invertedNullSortClause, defaultSort, service.SortOrderDesc)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY %s %s %s, %s %s LIMIT ?", query, sortBy, service.SortOrderAsc, invertedNullSortClause, defaultSort, service.SortOrderAsc)
replacements = append(replacements, listParams.Limit)
}
query = fmt.Sprintf("With result_set AS (%s) SELECT * FROM result_set ORDER BY %s %s %s, %s %s", query, sortBy, listParams.SortOrder, nullSortClause, defaultSort, listParams.SortOrder)
} else {
if listParams.SortOrder == service.SortOrderAsc {
query = fmt.Sprintf("%s ORDER BY %s %s %s LIMIT ?", query, sortBy, service.SortOrderDesc, invertedNullSortClause)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY %s %s %s LIMIT ?", query, sortBy, service.SortOrderAsc, invertedNullSortClause)
replacements = append(replacements, listParams.Limit)
}
query = fmt.Sprintf("With result_set AS (%s) SELECT * FROM result_set ORDER BY %s %s %s", query, sortBy, listParams.SortOrder, nullSortClause)
}
} else {
query = fmt.Sprintf("%s ORDER BY feature_id %s %s LIMIT ?", query, listParams.SortOrder, nullSortClause)
replacements = append(replacements, listParams.Limit)
if listParams.SortBy != defaultSortBy {
query = fmt.Sprintf("%s ORDER BY %s %s %s, %s %s LIMIT ?", query, sortBy, listParams.SortOrder, nullSortClause, defaultSort, listParams.SortOrder)
replacements = append(replacements, listParams.Limit)
} else {
query = fmt.Sprintf("%s ORDER BY %s %s %s LIMIT ?", query, defaultSort, listParams.SortOrder, nullSortClause)
replacements = append(replacements, listParams.Limit)
}
}

err := repo.DB(ctx).SelectContext(
Expand Down
Loading

0 comments on commit 3fa1cdc

Please sign in to comment.