Skip to content

Commit e9e913d

Browse files
EmyrkEmyrk
committedJun 10, 2022
Address PR comments
1 parent 4512a9b commit e9e913d

File tree

4 files changed

+31
-34
lines changed

4 files changed

+31
-34
lines changed
 

‎coderd/httpapi/queryparams.go

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,51 +15,47 @@ import (
1515
// errors in 1 sweep. This means all invalid fields are returned at once,
1616
// rather than only returning the first error
1717
type QueryParamParser struct {
18-
errors []Error
18+
// Errors is the set of errors to return via the API. If the length
19+
// of this set is 0, there are no errors!.
20+
Errors []Error
1921
}
2022

2123
func NewQueryParamParser() *QueryParamParser {
2224
return &QueryParamParser{
23-
errors: []Error{},
25+
Errors: []Error{},
2426
}
2527
}
2628

27-
// ValidationErrors is the set of errors to return via the API. If the length
28-
// of this set is 0, there was no errors.
29-
func (p QueryParamParser) ValidationErrors() []Error {
30-
return p.errors
31-
}
32-
33-
func (p *QueryParamParser) ParseInteger(r *http.Request, def int, queryParam string) int {
29+
func (p *QueryParamParser) Integer(r *http.Request, def int, queryParam string) int {
3430
v, err := parse(r, strconv.Atoi, def, queryParam)
3531
if err != nil {
36-
p.errors = append(p.errors, Error{
32+
p.Errors = append(p.Errors, Error{
3733
Field: queryParam,
3834
Detail: fmt.Sprintf("Query param %q must be a valid integer (%s)", queryParam, err.Error()),
3935
})
4036
}
4137
return v
4238
}
4339

44-
func (p *QueryParamParser) ParseUUIDorMe(r *http.Request, def uuid.UUID, me uuid.UUID, queryParam string) uuid.UUID {
40+
func (p *QueryParamParser) UUIDorMe(r *http.Request, def uuid.UUID, me uuid.UUID, queryParam string) uuid.UUID {
4541
if r.URL.Query().Get(queryParam) == "me" {
4642
return me
4743
}
48-
return p.ParseUUID(r, def, queryParam)
44+
return p.UUID(r, def, queryParam)
4945
}
5046

51-
func (p *QueryParamParser) ParseUUID(r *http.Request, def uuid.UUID, queryParam string) uuid.UUID {
47+
func (p *QueryParamParser) UUID(r *http.Request, def uuid.UUID, queryParam string) uuid.UUID {
5248
v, err := parse(r, uuid.Parse, def, queryParam)
5349
if err != nil {
54-
p.errors = append(p.errors, Error{
50+
p.Errors = append(p.Errors, Error{
5551
Field: queryParam,
5652
Detail: fmt.Sprintf("Query param %q must be a valid uuid", queryParam),
5753
})
5854
}
5955
return v
6056
}
6157

62-
func (p *QueryParamParser) ParseUUIDArray(r *http.Request, def []uuid.UUID, queryParam string) []uuid.UUID {
58+
func (p *QueryParamParser) UUIDArray(r *http.Request, def []uuid.UUID, queryParam string) []uuid.UUID {
6359
v, err := parse(r, func(v string) ([]uuid.UUID, error) {
6460
var badValues []string
6561
strs := strings.Split(v, ",")
@@ -79,20 +75,20 @@ func (p *QueryParamParser) ParseUUIDArray(r *http.Request, def []uuid.UUID, quer
7975
return ids, nil
8076
}, def, queryParam)
8177
if err != nil {
82-
p.errors = append(p.errors, Error{
78+
p.Errors = append(p.Errors, Error{
8379
Field: queryParam,
8480
Detail: fmt.Sprintf("Query param %q has invalid uuids: %q", queryParam, err.Error()),
8581
})
8682
}
8783
return v
8884
}
8985

90-
func (p *QueryParamParser) ParseString(r *http.Request, def string, queryParam string) string {
86+
func (p *QueryParamParser) String(r *http.Request, def string, queryParam string) string {
9187
v, err := parse(r, func(v string) (string, error) {
9288
return v, nil
9389
}, def, queryParam)
9490
if err != nil {
95-
p.errors = append(p.errors, Error{
91+
p.Errors = append(p.Errors, Error{
9692
Field: queryParam,
9793
Detail: fmt.Sprintf("Query param %q must be a valid string", queryParam),
9894
})

‎coderd/httpapi/queryparams_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestParseQueryParams(t *testing.T) {
6565

6666
parser := httpapi.NewQueryParamParser()
6767
testQueryParams(t, expParams, parser, func(r *http.Request, def uuid.UUID, queryParam string) uuid.UUID {
68-
return parser.ParseUUIDorMe(r, def, me, queryParam)
68+
return parser.UUIDorMe(r, def, me, queryParam)
6969
})
7070
})
7171

@@ -91,7 +91,7 @@ func TestParseQueryParams(t *testing.T) {
9191
}
9292

9393
parser := httpapi.NewQueryParamParser()
94-
testQueryParams(t, expParams, parser, parser.ParseString)
94+
testQueryParams(t, expParams, parser, parser.String)
9595
})
9696

9797
t.Run("Integer", func(t *testing.T) {
@@ -128,7 +128,7 @@ func TestParseQueryParams(t *testing.T) {
128128
}
129129

130130
parser := httpapi.NewQueryParamParser()
131-
testQueryParams(t, expParams, parser, parser.ParseInteger)
131+
testQueryParams(t, expParams, parser, parser.Integer)
132132
})
133133

134134
t.Run("UUIDArray", func(t *testing.T) {
@@ -171,7 +171,7 @@ func TestParseQueryParams(t *testing.T) {
171171
}
172172

173173
parser := httpapi.NewQueryParamParser()
174-
testQueryParams(t, expParams, parser, parser.ParseUUIDArray)
174+
testQueryParams(t, expParams, parser, parser.UUIDArray)
175175
})
176176
}
177177

@@ -193,7 +193,7 @@ func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], par
193193
v := parse(r, c.Default, c.QueryParam)
194194
require.Equal(t, c.Expected, v, fmt.Sprintf("param=%q value=%q", c.QueryParam, c.Value))
195195
if c.ExpectedErrorContains != "" {
196-
errors := parser.ValidationErrors()
196+
errors := parser.Errors
197197
require.True(t, len(errors) > 0, "error exist")
198198
last := errors[len(errors)-1]
199199
require.True(t, last.Field == c.QueryParam, fmt.Sprintf("query param %q did not fail", c.QueryParam))

‎coderd/pagination.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@ import (
1414
func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Pagination, ok bool) {
1515
parser := httpapi.NewQueryParamParser()
1616
params := codersdk.Pagination{
17-
AfterID: parser.ParseUUID(r, uuid.Nil, "after_id"),
17+
AfterID: parser.UUID(r, uuid.Nil, "after_id"),
1818
// Limit default to "-1" which returns all results
19-
Limit: parser.ParseInteger(r, -1, "limit"),
20-
Offset: parser.ParseInteger(r, 0, "offset"),
19+
Limit: parser.Integer(r, -1, "limit"),
20+
Offset: parser.Integer(r, 0, "offset"),
2121
}
22-
if len(parser.ValidationErrors()) > 0 {
22+
if len(parser.Errors) > 0 {
2323
httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
2424
Message: "Query parameters have invalid values.",
25-
Validations: parser.ValidationErrors(),
25+
Validations: parser.Errors,
2626
})
2727
return params, false
2828
}

‎coderd/workspaces.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"errors"
88
"fmt"
99
"net/http"
10+
"net/url"
1011
"strconv"
1112
"time"
1213

@@ -116,7 +117,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
116117
}
117118

118119
// Set all the query params from the "q" field.
119-
q := r.URL.Query()
120+
q := url.Values{}
120121
for k, v := range values {
121122
// Do not allow overriding if the user also set query param fields
122123
// outside the query string.
@@ -133,14 +134,14 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
133134
parser := httpapi.NewQueryParamParser()
134135
filter := database.GetWorkspacesWithFilterParams{
135136
Deleted: false,
136-
OwnerUsername: parser.ParseString(r, "", "owner"),
137-
TemplateName: parser.ParseString(r, "", "template"),
138-
Name: parser.ParseString(r, "", "name"),
137+
OwnerUsername: parser.String(r, "", "owner"),
138+
TemplateName: parser.String(r, "", "template"),
139+
Name: parser.String(r, "", "name"),
139140
}
140-
if len(parser.ValidationErrors()) > 0 {
141+
if len(parser.Errors) > 0 {
141142
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
142143
Message: "Query parameters have invalid values.",
143-
Validations: parser.ValidationErrors(),
144+
Validations: parser.Errors,
144145
})
145146
return
146147
}

0 commit comments

Comments
 (0)