PR feedback

- Unexport workspace search query function - move code into workspaces.go - use strings.SplitFields
coder · Emyrk · Jun 14, 2022 · Jun 7, 2022 · Jun 8, 2022 · Jun 8, 2022
commit ee3593500e82fcd2f9a7adfbd26fe4d3ee41410e
diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go
@@ -26,7 +26,7 @@ func NewQueryParamParser() *QueryParamParser {
 	}
 }
 
-func (p *QueryParamParser) Integer(r *http.Request, def int, queryParam string) int {
+func (p *QueryParamParser) Int(r *http.Request, def int, queryParam string) int {
 	v, err := parse(r, strconv.Atoi, def, queryParam)
 	if err != nil {
 		p.Errors = append(p.Errors, Error{

diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go
@@ -94,7 +94,7 @@ func TestParseQueryParams(t *testing.T) {
 		testQueryParams(t, expParams, parser, parser.String)
 	})
 
-	t.Run("Integer", func(t *testing.T) {
+	t.Run("Int", func(t *testing.T) {
 		t.Parallel()
 		expParams := []queryParamTestCase[int]{
 			{
@@ -128,7 +128,7 @@ func TestParseQueryParams(t *testing.T) {
 		}
 
 		parser := httpapi.NewQueryParamParser()
-		testQueryParams(t, expParams, parser, parser.Integer)
+		testQueryParams(t, expParams, parser, parser.Int)
 	})
 
 	t.Run("UUIDArray", func(t *testing.T) {

diff --git a/coderd/pagination.go b/coderd/pagination.go
@@ -16,8 +16,8 @@ func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Paginat
 	params := codersdk.Pagination{
 		AfterID: parser.UUID(r, uuid.Nil, "after_id"),
 		// Limit default to "-1" which returns all results
-		Limit:  parser.Integer(r, -1, "limit"),
-		Offset: parser.Integer(r, 0, "offset"),
+		Limit:  parser.Int(r, -1, "limit"),
+		Offset: parser.Int(r, 0, "offset"),
 	}
 	if len(parser.Errors) > 0 {
 		httpapi.Write(w, http.StatusBadRequest, httpapi.Response{

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/go-chi/chi/v5"
@@ -104,7 +105,7 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
 	apiKey := httpmw.APIKey(r)
 
 	queryStr := r.URL.Query().Get("q")
-	values, err := WorkspaceSearchQuery(queryStr)
+	values, err := workspaceSearchQuery(queryStr)
 	if err != nil {
 		httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
 			Message: "Invalid workspace search query.",
@@ -976,3 +977,66 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error
 		String: *s,
 	}, nil
 }
+
+// workspaceSearchQuery takes a query string and breaks it into its queryparams
+// as a set of key=value.
+func workspaceSearchQuery(query string) (map[string]string, error) {
+	searchParams := make(map[string]string)
+	if query == "" {
+		return searchParams, nil
+	}
+	// Because we do this in 2 passes, we want to maintain quotes on the first
+	// pass.Further splitting occurs on the second pass and quotes will be
+	// dropped.
+	elements := splitQueryParameterByDelimiter(query, ' ', true)
+	for _, element := range elements {
+		parts := splitQueryParameterByDelimiter(element, ':', false)
+		switch len(parts) {
+		case 1:
+			// No key:value pair. It is a workspace name, and maybe includes an owner
+			parts = splitQueryParameterByDelimiter(element, '/', false)
+			switch len(parts) {
+			case 1:
+				searchParams["name"] = parts[0]
+			case 2:
+				searchParams["owner"] = parts[0]
+				searchParams["name"] = parts[1]
+			default:
+				return nil, xerrors.Errorf("Query element %q can only contain 1 '/'", element)
+			}
+		case 2:
+			searchParams[parts[0]] = parts[1]
+		default:
+			return nil, xerrors.Errorf("Query element %q can only contain 1 ':'", element)
+		}
+	}
+
+	return searchParams, nil
+}
+
+// splitQueryParameterByDelimiter takes a query string and splits it into the individual elements
+// of the query. Each element is separated by a delimiter. All quoted strings are
+// kept as a single element.
+//
+// Although all our names cannot have spaces, that is a validation error.
+// We should still parse the quoted string as a single value so that validation
+// can properly fail on the space. If we do not, a value of `template:"my name"`
+// will search `template:"my name:name"`, which produces an empty list instead of
+// an error.
+// nolint:revive
+func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes bool) []string {
+	quoted := false
+	parts := strings.FieldsFunc(query, func(r rune) bool {
+		if r == '"' {
+			quoted = !quoted
+		}
+		return !quoted && r == delimiter
+	})
+	if !maintainQuotes {
+		for i, part := range parts {
+			parts[i] = strings.Trim(part, "\"")
+		}
+	}
+
+	return parts
+}
diff --git a/coderd/workspacesearch_test.go → coderd/workspaces_internal_test.go b/coderd/workspacesearch_test.go → coderd/workspaces_internal_test.go
@@ -1,9 +1,8 @@
-package coderd_test
+package coderd
 
 import (
 	"testing"
 
-	"github.com/coder/coder/coderd"
 	"github.com/stretchr/testify/require"
 )
 
@@ -124,7 +123,7 @@ func TestSearchWorkspace(t *testing.T) {
 		c := c
 		t.Run(c.Name, func(t *testing.T) {
 			t.Parallel()
-			values, err := coderd.WorkspaceSearchQuery(c.Query)
+			values, err := workspaceSearchQuery(c.Query)
 			if c.ExpectedErrorContains != "" {
 				require.Error(t, err, "expected error")
 				require.ErrorContains(t, err, c.ExpectedErrorContains)

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -426,7 +426,7 @@ func TestWorkspaceFilter(t *testing.T) {
 		{
 			Name: "Owner",
 			Filter: codersdk.WorkspaceFilter{
-				Owner: users[must(cryptorand.Intn(len(users)))].User.Username,
+				Owner: users[2].User.Username,
 			},
 			FilterF: func(f codersdk.WorkspaceFilter, workspace madeWorkspace) bool {
 				return workspace.Owner.Username == f.Owner

diff --git a/coderd/workspacesearch.go b/coderd/workspacesearch.go