From 3184814c18ba56a4335320534bb29ec0d79da0f5 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 14 Feb 2025 14:54:12 +0000 Subject: [PATCH 1/3] feat(coderd/httpapi): add QueryParamParser.JSONStringMap --- coderd/httpapi/queryparams.go | 18 +++++++++ coderd/httpapi/queryparams_test.go | 64 ++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go index 15a67caa651a8..9eb5325eca53e 100644 --- a/coderd/httpapi/queryparams.go +++ b/coderd/httpapi/queryparams.go @@ -2,6 +2,7 @@ package httpapi import ( "database/sql" + "encoding/json" "errors" "fmt" "net/url" @@ -257,6 +258,23 @@ func (p *QueryParamParser) Strings(vals url.Values, def []string, queryParam str }) } +func (p *QueryParamParser) JSONStringMap(vals url.Values, def map[string]string, queryParam string) map[string]string { + v, err := parseQueryParam(p, vals, func(v string) (map[string]string, error) { + var m map[string]string + if err := json.NewDecoder(strings.NewReader(v)).Decode(&m); err != nil { + return nil, err + } + return m, 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 JSON object: %s", queryParam, err.Error()), + }) + } + return v +} + // ValidEnum represents an enum that can be parsed and validated. type ValidEnum interface { // Add more types as needed (avoid importing large dependency trees). diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go index 16cf805534b05..e95ce292404b2 100644 --- a/coderd/httpapi/queryparams_test.go +++ b/coderd/httpapi/queryparams_test.go @@ -473,6 +473,70 @@ func TestParseQueryParams(t *testing.T) { testQueryParams(t, expParams, parser, parser.UUIDs) }) + t.Run("JSONStringMap", func(t *testing.T) { + t.Parallel() + + expParams := []queryParamTestCase[map[string]string]{ + { + QueryParam: "valid_map", + Value: `{"key1": "value1", "key2": "value2"}`, + Expected: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + { + QueryParam: "empty", + Value: "{}", + Default: map[string]string{}, + Expected: map[string]string{}, + }, + { + QueryParam: "no_value", + NoSet: true, + Default: map[string]string{}, + Expected: map[string]string{}, + }, + { + QueryParam: "default", + NoSet: true, + Default: map[string]string{"key": "value"}, + Expected: map[string]string{"key": "value"}, + }, + { + QueryParam: "null", + Value: "null", + Expected: map[string]string(nil), + }, + { + QueryParam: "undefined", + Value: "undefined", + Expected: map[string]string(nil), + }, + { + QueryParam: "invalid_map", + Value: `{"key1": "value1", "key2": "value2"`, // missing closing brace + Expected: map[string]string(nil), + Default: map[string]string{}, + ExpectedErrorContains: `Query param "invalid_map" must be a valid JSON object: unexpected EOF`, + }, + { + QueryParam: "incorrect_type", + Value: `{"key1": 1, "key2": true}`, + Expected: map[string]string(nil), + ExpectedErrorContains: `Query param "incorrect_type" must be a valid JSON object: json: cannot unmarshal number into Go value of type string`, + }, + { + QueryParam: "multiple_keys", + Values: []string{`{"key1": "value1"}`, `{"key2": "value2"}`}, + Expected: map[string]string(nil), + ExpectedErrorContains: `Query param "multiple_keys" provided more than once, found 2 times.`, + }, + } + parser := httpapi.NewQueryParamParser() + testQueryParams(t, expParams, parser, parser.JSONStringMap) + }) + t.Run("Required", func(t *testing.T) { t.Parallel() From 4fec855ec14e11a479323af0a9ef84d1a3c83305 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 18 Feb 2025 12:56:49 +0000 Subject: [PATCH 2/3] update usages --- coderd/provisionerdaemons.go | 13 +------------ coderd/provisionerjobs.go | 13 +------------ 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 6495c4eb15bee..906aee6c02f5f 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -44,7 +44,7 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) { p := httpapi.NewQueryParamParser() limit := p.PositiveInt32(qp, 50, "limit") ids := p.UUIDs(qp, nil, "ids") - tagsRaw := p.String(qp, "", "tags") + tags := p.JSONStringMap(qp, nil, "tags") p.ErrorExcessParams(qp) if len(p.Errors) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -54,17 +54,6 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) { return } - tags := database.StringMap{} - if tagsRaw != "" { - if err := tags.Scan([]byte(tagsRaw)); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid tags query parameter", - Detail: err.Error(), - }) - return - } - } - daemons, err := api.Database.GetProvisionerDaemonsWithStatusByOrganization( ctx, database.GetProvisionerDaemonsWithStatusByOrganizationParams{ diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index b51c38021c7ad..2005b679a72ed 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -108,7 +108,7 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt if ids == nil { ids = p.UUIDs(qp, nil, "ids") } - tagsRaw := p.String(qp, "", "tags") + tags := p.JSONStringMap(qp, nil, "tags") p.ErrorExcessParams(qp) if len(p.Errors) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -118,17 +118,6 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt return nil, false } - tags := database.StringMap{} - if tagsRaw != "" { - if err := tags.Scan([]byte(tagsRaw)); err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid tags query parameter", - Detail: err.Error(), - }) - return nil, false - } - } - jobs, err := api.Database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx, database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{ OrganizationID: org.ID, Status: slice.StringEnums[database.ProvisionerJobStatus](status), From ff84210ecc1926cf90c53e37648dd1c732baf4c8 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 18 Feb 2025 12:58:34 +0000 Subject: [PATCH 3/3] non-nil default --- coderd/provisionerdaemons.go | 2 +- coderd/provisionerjobs.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 906aee6c02f5f..332ae3b352e0a 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -44,7 +44,7 @@ func (api *API) provisionerDaemons(rw http.ResponseWriter, r *http.Request) { p := httpapi.NewQueryParamParser() limit := p.PositiveInt32(qp, 50, "limit") ids := p.UUIDs(qp, nil, "ids") - tags := p.JSONStringMap(qp, nil, "tags") + tags := p.JSONStringMap(qp, database.StringMap{}, "tags") p.ErrorExcessParams(qp) if len(p.Errors) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index 2005b679a72ed..47963798f4d32 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -108,7 +108,7 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt if ids == nil { ids = p.UUIDs(qp, nil, "ids") } - tags := p.JSONStringMap(qp, nil, "tags") + tags := p.JSONStringMap(qp, database.StringMap{}, "tags") p.ErrorExcessParams(qp) if len(p.Errors) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{