selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: support multiple key:value search query params (#12690) 

This more closely aligns with GitHub's label search style. Actual search params need to be converted to allow this format, by default they will throw an error if they do not support listing.
This commit is contained in:
Steven Masley 2024-03-21 08:37:19 -05:00 committed by GitHub
parent 8499eacf67
commit b4492fffba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 112 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
coderd/httpapi/queryparams.go
View File

@ -61,7 +61,7 @@ func (p *QueryParamParser) UInt(vals url.Values, def uint64, queryParam string)
if err != nil {
p.Errors = append(p.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q must be a valid positive integer (%s)", queryParam, err.Error()),
Detail: fmt.Sprintf("Query param %q must be a valid positive integer: %s", queryParam, err.Error()),
})
return 0
}
@ -73,7 +73,7 @@ func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int
if err != nil {
p.Errors = append(p.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q must be a valid integer (%s)", queryParam, err.Error()),
Detail: fmt.Sprintf("Query param %q must be a valid integer: %s", queryParam, err.Error()),
})
}
return v
@ -97,7 +97,7 @@ func (p *QueryParamParser) PositiveInt32(vals url.Values, def int32, queryParam
if err != nil {
p.Errors = append(p.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q must be a valid 32-bit positive integer (%s)", queryParam, err.Error()),
Detail: fmt.Sprintf("Query param %q must be a valid 32-bit positive integer: %s", queryParam, err.Error()),
})
}
return v
@ -108,7 +108,7 @@ func (p *QueryParamParser) Boolean(vals url.Values, def bool, queryParam string)
if err != nil {
p.Errors = append(p.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q must be a valid boolean (%s)", queryParam, err.Error()),
Detail: fmt.Sprintf("Query param %q must be a valid boolean: %s", queryParam, err.Error()),
})
}
return v
@ -203,9 +203,15 @@ func (p *QueryParamParser) timeWithMutate(vals url.Values, def time.Time, queryP
}
func (p *QueryParamParser) String(vals url.Values, def string, queryParam string) string {
v, _ := parseQueryParam(p, vals, func(v string) (string, error) {
v, err := parseQueryParam(p, vals, func(v string) (string, error) {
return v, nil
}, def, queryParam)
if err != nil {
p.Errors = append(p.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q must be a valid string: %s", queryParam, err.Error()),
})
}
return v
}
@ -248,13 +254,23 @@ func ParseCustom[T any](parser *QueryParamParser, vals url.Values, def T, queryP
return v
}
// ParseCustomList is a function that handles csv query params.
// ParseCustomList is a function that handles csv query params or multiple values
// for a query param.
// Csv is supported as it is a common way to pass multiple values in a query param.
// Multiple values is supported (key=value&key=value2) for feature parity with GitHub issue search.
func ParseCustomList[T any](parser *QueryParamParser, vals url.Values, def []T, queryParam string, parseFunc func(v string) (T, error)) []T {
v, err := parseQueryParam(parser, vals, func(v string) ([]T, error) {
terms := strings.Split(v, ",")
v, err := parseQueryParamSet(parser, vals, func(set []string) ([]T, error) {
// Gather all terms.
allTerms := make([]string, 0, len(set))
for _, s := range set {
// If a term is a csv, break it out into individual terms.
terms := strings.Split(s, ",")
allTerms = append(allTerms, terms...)
}
var badValues []string
var output []T
for _, s := range terms {
for _, s := range allTerms {
good, err := parseFunc(s)
if err != nil {
badValues = append(badValues, s)
@ -277,7 +293,27 @@ func ParseCustomList[T any](parser *QueryParamParser, vals url.Values, def []T,
return v
}
// parseQueryParam expects just 1 value set for the given query param.
func parseQueryParam[T any](parser *QueryParamParser, vals url.Values, parse func(v string) (T, error), def T, queryParam string) (T, error) {
setParse := func(set []string) (T, error) {
if len(set) > 1 {
// Set as a parser.Error rather than return an error.
// Returned errors are errors from the passed in `parse` function, and
// imply the query param value had attempted to be parsed.
// By raising the error this way, we can also more easily control how it
// is presented to the user. A returned error is wrapped with more text.
parser.Errors = append(parser.Errors, codersdk.ValidationError{
Field: queryParam,
Detail: fmt.Sprintf("Query param %q provided more than once, found %d times. Only provide 1 instance of this query param.", queryParam, len(set)),
})
return def, nil
}
return parse(set[0])
}
return parseQueryParamSet(parser, vals, setParse, def, queryParam)
}
func parseQueryParamSet[T any](parser *QueryParamParser, vals url.Values, parse func(set []string) (T, error), def T, queryParam string) (T, error) {
parser.addParsed(queryParam)
// If the query param is required and not present, return an error.
if parser.RequiredNotEmptyParams[queryParam] && (!vals.Has(queryParam) || vals.Get(queryParam) == "") {
@ -293,6 +329,5 @@ func parseQueryParam[T any](parser *QueryParamParser, vals url.Values, parse fun
return def, nil
}
str := vals.Get(queryParam)
return parse(str)
return parse(vals[queryParam])
}

69
coderd/httpapi/queryparams_test.go
View File

@ -17,8 +17,13 @@ import (
type queryParamTestCase[T any] struct {
QueryParam string
// No set does not set the query param, rather than setting the empty value
NoSet bool
Value string
NoSet bool
// Value vs values is the difference between a single query param and multiple
// to the same key.
// -> key=value
Value string
// -> key=value1 key=value2
Values []string
Default T
Expected T
ExpectedErrorContains string
@ -27,6 +32,7 @@ type queryParamTestCase[T any] struct {
func TestParseQueryParams(t *testing.T) {
t.Parallel()
const multipleValuesError = "provided more than once"
t.Run("Enum", func(t *testing.T) {
t.Parallel()
@ -59,6 +65,11 @@ func TestParseQueryParams(t *testing.T) {
Value: fmt.Sprintf("%s,%s", database.ResourceTypeWorkspace, database.ResourceTypeApiKey),
Expected: []database.ResourceType{database.ResourceTypeWorkspace, database.ResourceTypeApiKey},
},
{
QueryParam: "resource_type_as_list",
Values: []string{string(database.ResourceTypeWorkspace), string(database.ResourceTypeApiKey)},
Expected: []database.ResourceType{database.ResourceTypeWorkspace, database.ResourceTypeApiKey},
},
}
parser := httpapi.NewQueryParamParser()
@ -151,6 +162,11 @@ func TestParseQueryParams(t *testing.T) {
Default: "default",
Expected: "default",
},
{
QueryParam: "unexpected_list",
Values: []string{"one", "two"},
ExpectedErrorContains: multipleValuesError,
},
}
parser := httpapi.NewQueryParamParser()
@ -193,6 +209,11 @@ func TestParseQueryParams(t *testing.T) {
Expected: false,
ExpectedErrorContains: "must be a valid boolean",
},
{
QueryParam: "unexpected_list",
Values: []string{"true", "false"},
ExpectedErrorContains: multipleValuesError,
},
}
parser := httpapi.NewQueryParamParser()
@ -230,6 +251,11 @@ func TestParseQueryParams(t *testing.T) {
Expected: 0,
ExpectedErrorContains: "must be a valid integer",
},
{
QueryParam: "unexpected_list",
Values: []string{"5", "10"},
ExpectedErrorContains: multipleValuesError,
},
}
parser := httpapi.NewQueryParamParser()
@ -274,6 +300,11 @@ func TestParseQueryParams(t *testing.T) {
Expected: 0,
ExpectedErrorContains: "must be a valid 32-bit positive integer",
},
{
QueryParam: "unexpected_list",
Values: []string{"5", "10"},
ExpectedErrorContains: multipleValuesError,
},
}
parser := httpapi.NewQueryParamParser()
@ -311,6 +342,11 @@ func TestParseQueryParams(t *testing.T) {
Expected: 0,
ExpectedErrorContains: "must be a valid positive integer",
},
{
QueryParam: "unexpected_list",
Values: []string{"5", "10"},
ExpectedErrorContains: multipleValuesError,
},
}
parser := httpapi.NewQueryParamParser()
@ -354,6 +390,23 @@ func TestParseQueryParams(t *testing.T) {
Default: []uuid.UUID{},
ExpectedErrorContains: "bogus",
},
{
QueryParam: "multiple_keys",
Values: []string{"6c8ef17d-5dd8-4b92-bac9-41944f90f237", "65fb05f3-12c8-4a0a-801f-40439cf9e681"},
Expected: []uuid.UUID{
uuid.MustParse("6c8ef17d-5dd8-4b92-bac9-41944f90f237"),
uuid.MustParse("65fb05f3-12c8-4a0a-801f-40439cf9e681"),
},
},
{
QueryParam: "multiple_and_csv",
Values: []string{"6c8ef17d-5dd8-4b92-bac9-41944f90f237", "65fb05f3-12c8-4a0a-801f-40439cf9e681, 01b94888-1eab-4bbf-aed0-dc7a8010da97"},
Expected: []uuid.UUID{
uuid.MustParse("6c8ef17d-5dd8-4b92-bac9-41944f90f237"),
uuid.MustParse("65fb05f3-12c8-4a0a-801f-40439cf9e681"),
uuid.MustParse("01b94888-1eab-4bbf-aed0-dc7a8010da97"),
},
},
}
parser := httpapi.NewQueryParamParser()
@ -381,7 +434,17 @@ func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], par
if c.NoSet {
continue
}
v.Set(c.QueryParam, c.Value)
if len(c.Values) > 0 && c.Value != "" {
t.Errorf("test case %q has both value and values, choose one, not both!", c.QueryParam)
t.FailNow()
}
if c.Value != "" {
c.Values = append(c.Values, c.Value)
}
for _, value := range c.Values {
v.Add(c.QueryParam, value)
}
}
for _, c := range testCases {

11
coderd/searchquery/search.go
View File

@ -163,17 +163,6 @@ func searchTerms(query string, defaultKey func(term string, values url.Values) e
}
}
for k := range searchValues {
if len(searchValues[k]) > 1 {
return nil, []codersdk.ValidationError{
{
Field: "q",
Detail: fmt.Sprintf("Query parameter %q provided more than once, found %d times", k, len(searchValues[k])),
},
}
}
}
return searchValues, nil
}