Skip to content

Commit ee35935

Browse files
EmyrkEmyrk
committed
PR feedback
- Unexport workspace search query function - move code into workspaces.go - use strings.SplitFields
1 parent c8813cc commit ee35935

7 files changed

+73
-90
lines changed

‎coderd/httpapi/queryparams.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func NewQueryParamParser() *QueryParamParser {
2626
}
2727
}
2828

29-
func (p *QueryParamParser) Integer(r *http.Request, def int, queryParam string) int {
29+
func (p *QueryParamParser) Int(r *http.Request, def int, queryParam string) int {
3030
v, err := parse(r, strconv.Atoi, def, queryParam)
3131
if err != nil {
3232
p.Errors = append(p.Errors, Error{

‎coderd/httpapi/queryparams_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func TestParseQueryParams(t *testing.T) {
9494
testQueryParams(t, expParams, parser, parser.String)
9595
})
9696

97-
t.Run("Integer", func(t *testing.T) {
97+
t.Run("Int", func(t *testing.T) {
9898
t.Parallel()
9999
expParams := []queryParamTestCase[int]{
100100
{
@@ -128,7 +128,7 @@ func TestParseQueryParams(t *testing.T) {
128128
}
129129

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

134134
t.Run("UUIDArray", func(t *testing.T) {

‎coderd/pagination.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Paginat
1616
params := codersdk.Pagination{
1717
AfterID: parser.UUID(r, uuid.Nil, "after_id"),
1818
// Limit default to "-1" which returns all results
19-
Limit: parser.Integer(r, -1, "limit"),
20-
Offset: parser.Integer(r, 0, "offset"),
19+
Limit: parser.Int(r, -1, "limit"),
20+
Offset: parser.Int(r, 0, "offset"),
2121
}
2222
if len(parser.Errors) > 0 {
2323
httpapi.Write(w, http.StatusBadRequest, httpapi.Response{

‎coderd/workspaces.go

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"net/http"
1010
"strconv"
11+
"strings"
1112
"time"
1213

1314
"github.com/go-chi/chi/v5"
@@ -104,7 +105,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
104105
apiKey := httpmw.APIKey(r)
105106

106107
queryStr := r.URL.Query().Get("q")
107-
values, err := WorkspaceSearchQuery(queryStr)
108+
values, err := workspaceSearchQuery(queryStr)
108109
if err != nil {
109110
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
110111
Message: "Invalid workspace search query.",
@@ -976,3 +977,66 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error
976977
String: *s,
977978
}, nil
978979
}
980+
981+
// workspaceSearchQuery takes a query string and breaks it into its queryparams
982+
// as a set of key=value.
983+
func workspaceSearchQuery(query string) (map[string]string, error) {
984+
searchParams := make(map[string]string)
985+
if query == "" {
986+
return searchParams, nil
987+
}
988+
// Because we do this in 2 passes, we want to maintain quotes on the first
989+
// pass.Further splitting occurs on the second pass and quotes will be
990+
// dropped.
991+
elements := splitQueryParameterByDelimiter(query, ' ', true)
992+
for _, element := range elements {
993+
parts := splitQueryParameterByDelimiter(element, ':', false)
994+
switch len(parts) {
995+
case 1:
996+
// No key:value pair. It is a workspace name, and maybe includes an owner
997+
parts = splitQueryParameterByDelimiter(element, '/', false)
998+
switch len(parts) {
999+
case 1:
1000+
searchParams["name"] = parts[0]
1001+
case 2:
1002+
searchParams["owner"] = parts[0]
1003+
searchParams["name"] = parts[1]
1004+
default:
1005+
return nil, xerrors.Errorf("Query element %q can only contain 1 '/'", element)
1006+
}
1007+
case 2:
1008+
searchParams[parts[0]] = parts[1]
1009+
default:
1010+
return nil, xerrors.Errorf("Query element %q can only contain 1 ':'", element)
1011+
}
1012+
}
1013+
1014+
return searchParams, nil
1015+
}
1016+
1017+
// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements
1018+
// of the query. Each element is separated by a delimiter. All quoted strings are
1019+
// kept as a single element.
1020+
//
1021+
// Although all our names cannot have spaces, that is a validation error.
1022+
// We should still parse the quoted string as a single value so that validation
1023+
// can properly fail on the space. If we do not, a value of `template:"my name"`
1024+
// will search `template:"my name:name"`, which produces an empty list instead of
1025+
// an error.
1026+
// nolint:revive
1027+
func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes bool) []string {
1028+
quoted := false
1029+
parts := strings.FieldsFunc(query, func(r rune) bool {
1030+
if r == '"' {
1031+
quoted = !quoted
1032+
}
1033+
return !quoted && r == delimiter
1034+
})
1035+
if !maintainQuotes {
1036+
for i, part := range parts {
1037+
parts[i] = strings.Trim(part, "\"")
1038+
}
1039+
}
1040+
1041+
return parts
1042+
}

‎coderd/workspacesearch_test.go renamed to ‎coderd/workspaces_internal_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
package coderd_test
1+
package coderd
22

33
import (
44
"testing"
55

6-
"github.com/coder/coder/coderd"
76
"github.com/stretchr/testify/require"
87
)
98

@@ -124,7 +123,7 @@ func TestSearchWorkspace(t *testing.T) {
124123
c := c
125124
t.Run(c.Name, func(t *testing.T) {
126125
t.Parallel()
127-
values, err := coderd.WorkspaceSearchQuery(c.Query)
126+
values, err := workspaceSearchQuery(c.Query)
128127
if c.ExpectedErrorContains != "" {
129128
require.Error(t, err, "expected error")
130129
require.ErrorContains(t, err, c.ExpectedErrorContains)

‎coderd/workspaces_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ func TestWorkspaceFilter(t *testing.T) {
426426
{
427427
Name: "Owner",
428428
Filter: codersdk.WorkspaceFilter{
429-
Owner: users[must(cryptorand.Intn(len(users)))].User.Username,
429+
Owner: users[2].User.Username,
430430
},
431431
FilterF: func(f codersdk.WorkspaceFilter, workspace madeWorkspace) bool {
432432
return workspace.Owner.Username == f.Owner

‎coderd/workspacesearch.go

Lines changed: 0 additions & 80 deletions
This file was deleted.

0 commit comments

Comments
 (0)