Skip to content

Commit 5687dbe

Browse files
EmyrkEmyrk
committed
Add unit test for parsing query params
1 parent 0f7bb21 commit 5687dbe

File tree

2 files changed

+206
-11
lines changed

2 files changed

+206
-11
lines changed

‎coderd/httpapi/queryparams.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,7 @@ func (p *QueryParamParser) ParseUUIDorMe(r *http.Request, def uuid.UUID, me uuid
4545
if r.URL.Query().Get(queryParam) == "me" {
4646
return me
4747
}
48-
49-
v, err := parse(r, uuid.Parse, def, queryParam)
50-
if err != nil {
51-
p.errors = append(p.errors, Error{
52-
Field: queryParam,
53-
Detail: fmt.Sprintf("Query param %q must be a valid uuid", queryParam),
54-
})
55-
}
56-
return v
48+
return p.ParseUUID(r, def, queryParam)
5749
}
5850

5951
func (p *QueryParamParser) ParseUUID(r *http.Request, def uuid.UUID, queryParam string) uuid.UUID {
@@ -73,7 +65,7 @@ func (p *QueryParamParser) ParseUUIDArray(r *http.Request, def []uuid.UUID, quer
7365
strs := strings.Split(v, ",")
7466
ids := make([]uuid.UUID, 0, len(strs))
7567
for _, s := range strs {
76-
id, err := uuid.Parse(s)
68+
id, err := uuid.Parse(strings.TrimSpace(s))
7769
if err != nil {
7870
badValues = append(badValues, v)
7971
continue
@@ -82,7 +74,7 @@ func (p *QueryParamParser) ParseUUIDArray(r *http.Request, def []uuid.UUID, quer
8274
}
8375

8476
if len(badValues) > 0 {
85-
return nil, xerrors.Errorf("%s", strings.Join(badValues, ","))
77+
return []uuid.UUID{}, xerrors.Errorf("%s", strings.Join(badValues, ","))
8678
}
8779
return ids, nil
8880
}, def, queryParam)

‎coderd/httpapi/queryparams_test.go

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
package httpapi_test
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/url"
7+
"testing"
8+
9+
"github.com/coder/coder/coderd/httpapi"
10+
"github.com/google/uuid"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
type queryParamTestCase[T any] struct {
15+
QueryParam string
16+
// No set does not set the query param, rather than setting the empty value
17+
NoSet bool
18+
Value string
19+
Default T
20+
Expected T
21+
ExpectedErrorContains string
22+
Parse func(r *http.Request, def T, queryParam string) T
23+
}
24+
25+
func TestParseQueryParams(t *testing.T) {
26+
t.Parallel()
27+
28+
t.Run("UUID", func(t *testing.T) {
29+
t.Parallel()
30+
me := uuid.New()
31+
expParams := []queryParamTestCase[uuid.UUID]{
32+
{
33+
QueryParam: "valid_id",
34+
Value: "afe39fbf-0f52-4a62-b0cc-58670145d773",
35+
Expected: uuid.MustParse("afe39fbf-0f52-4a62-b0cc-58670145d773"),
36+
},
37+
{
38+
QueryParam: "me",
39+
Value: "me",
40+
Expected: me,
41+
},
42+
{
43+
QueryParam: "invalid_id",
44+
Value: "bogus",
45+
ExpectedErrorContains: "must be a valid uuid",
46+
},
47+
{
48+
QueryParam: "long_id",
49+
Value: "afe39fbf-0f52-4a62-b0cc-58670145d773-123",
50+
ExpectedErrorContains: "must be a valid uuid",
51+
},
52+
{
53+
QueryParam: "no_value",
54+
NoSet: true,
55+
Default: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
56+
Expected: uuid.MustParse("11111111-1111-1111-1111-111111111111"),
57+
},
58+
{
59+
QueryParam: "empty",
60+
Value: "",
61+
ExpectedErrorContains: "must be a valid uuid",
62+
},
63+
}
64+
65+
parser := httpapi.NewQueryParamParser()
66+
testQueryParams(t, expParams, parser, func(r *http.Request, def uuid.UUID, queryParam string) uuid.UUID {
67+
return parser.ParseUUIDorMe(r, def, me, queryParam)
68+
})
69+
})
70+
71+
t.Run("String", func(t *testing.T) {
72+
t.Parallel()
73+
expParams := []queryParamTestCase[string]{
74+
{
75+
QueryParam: "valid_string",
76+
Value: "random",
77+
Expected: "random",
78+
},
79+
{
80+
QueryParam: "empty",
81+
Value: "",
82+
Expected: "",
83+
},
84+
{
85+
QueryParam: "no_value",
86+
NoSet: true,
87+
Default: "default",
88+
Expected: "default",
89+
},
90+
}
91+
92+
parser := httpapi.NewQueryParamParser()
93+
testQueryParams(t, expParams, parser, parser.ParseString)
94+
})
95+
96+
t.Run("Integer", func(t *testing.T) {
97+
t.Parallel()
98+
expParams := []queryParamTestCase[int]{
99+
{
100+
QueryParam: "valid_integer",
101+
Value: "100",
102+
Expected: 100,
103+
},
104+
{
105+
QueryParam: "empty",
106+
Value: "",
107+
Expected: 0,
108+
ExpectedErrorContains: "must be a valid integer",
109+
},
110+
{
111+
QueryParam: "no_value",
112+
NoSet: true,
113+
Default: 5,
114+
Expected: 5,
115+
},
116+
{
117+
QueryParam: "negative",
118+
Value: "-10",
119+
Expected: -10,
120+
Default: 5,
121+
},
122+
{
123+
QueryParam: "invalid_integer",
124+
Value: "bogus",
125+
Expected: 0,
126+
ExpectedErrorContains: "must be a valid integer",
127+
},
128+
}
129+
130+
parser := httpapi.NewQueryParamParser()
131+
testQueryParams(t, expParams, parser, parser.ParseInteger)
132+
})
133+
134+
t.Run("UUIDArray", func(t *testing.T) {
135+
t.Parallel()
136+
expParams := []queryParamTestCase[[]uuid.UUID]{
137+
{
138+
QueryParam: "valid_ids_with_spaces",
139+
Value: "6c8ef17d-5dd8-4b92-bac9-41944f90f237, 65fb05f3-12c8-4a0a-801f-40439cf9e681 , 01b94888-1eab-4bbf-aed0-dc7a8010da97",
140+
Expected: []uuid.UUID{
141+
uuid.MustParse("6c8ef17d-5dd8-4b92-bac9-41944f90f237"),
142+
uuid.MustParse("65fb05f3-12c8-4a0a-801f-40439cf9e681"),
143+
uuid.MustParse("01b94888-1eab-4bbf-aed0-dc7a8010da97"),
144+
},
145+
},
146+
{
147+
QueryParam: "empty",
148+
Value: "",
149+
Expected: []uuid.UUID{},
150+
},
151+
{
152+
QueryParam: "no_value",
153+
NoSet: true,
154+
Default: []uuid.UUID{},
155+
Expected: []uuid.UUID{},
156+
},
157+
{
158+
QueryParam: "default",
159+
NoSet: true,
160+
Default: []uuid.UUID{uuid.Nil},
161+
Expected: []uuid.UUID{uuid.Nil},
162+
},
163+
{
164+
QueryParam: "invalid_id_in_set",
165+
Value: "6c8ef17d-5dd8-4b92-bac9-41944f90f237,bogus",
166+
Expected: []uuid.UUID{},
167+
Default: []uuid.UUID{},
168+
ExpectedErrorContains: "bogus",
169+
},
170+
}
171+
172+
parser := httpapi.NewQueryParamParser()
173+
testQueryParams(t, expParams, parser, parser.ParseUUIDArray)
174+
})
175+
}
176+
177+
func testQueryParams[T any](t *testing.T, testCases []queryParamTestCase[T], parser *httpapi.QueryParamParser, parse func(r *http.Request, def T, queryParam string) T) {
178+
v := url.Values{}
179+
for _, c := range testCases {
180+
if c.NoSet {
181+
continue
182+
}
183+
v.Set(c.QueryParam, c.Value)
184+
}
185+
r, err := http.NewRequest("GET", "https://example.com", nil)
186+
require.NoError(t, err, "new request")
187+
r.URL.RawQuery = v.Encode()
188+
189+
for _, c := range testCases {
190+
// !! Do not run these in parallel !!
191+
t.Run(c.QueryParam, func(t *testing.T) {
192+
v := parse(r, c.Default, c.QueryParam)
193+
require.Equal(t, c.Expected, v, fmt.Sprintf("param=%q value=%q", c.QueryParam, c.Value))
194+
if c.ExpectedErrorContains != "" {
195+
errors := parser.ValidationErrors()
196+
require.True(t, len(errors) > 0, "error exist")
197+
last := errors[len(errors)-1]
198+
require.True(t, last.Field == c.QueryParam, fmt.Sprintf("query param %q did not fail", c.QueryParam))
199+
require.Contains(t, last.Detail, c.ExpectedErrorContains, "correct error")
200+
}
201+
})
202+
}
203+
}

0 commit comments

Comments
 (0)