chore: support multiple key:value search query params

Emyrk · Emyrk · commit 77e37cfde62a · 2024-03-20T16:04:27.000-05:00
GitHub does this in their label search. Including this for an upcoming
feature where the csv format is a bit unwieldy.
diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go
@@ -248,13 +248,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 +287,18 @@ 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 {
+			return def, xerrors.Errorf("multiple values provided for the query param %q, only 1 is supported", queryParam)
+		}
+		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 +314,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])
 }
diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go
@@ -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
@@ -354,6 +359,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 +403,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 {
