tonic: Refactor handling of comma-separated lists

loopfz · Sep 6, 2019 · 7b6627b · 7b6627b
1 parent 66ad463
commit 7b6627b
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 33 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,9 +1,9 @@
 language: go
 
 go:
-  - "1.8.x"
-  - "1.9.x"
   - "1.10.x"
+  - "1.11.x"
+  - "1.12.x"
   - "master"
 
 jobs:

diff --git a/tonic/handler.go b/tonic/handler.go
@@ -195,10 +195,10 @@ func bind(c *gin.Context, v reflect.Value, tag string, extract extractor) error
 		}
 		// Set-up context for extractors.
 		// Query.
-		c.Set(ExplodeTag, true) // default
-		if explodeVal, ok := ft.Tag.Lookup(ExplodeTag); ok {
-			if explode, err := strconv.ParseBool(explodeVal); err == nil && !explode {
-				c.Set(ExplodeTag, false)
+		c.Set(ArrayTag, false)
+		if arrayVal, ok := ft.Tag.Lookup(ArrayTag); ok {
+			if array, err := strconv.ParseBool(arrayVal); err == nil && array {
+				c.Set(ArrayTag, true)
 			}
 		}
 		_, fieldValues, err := extract(c, tagValue)
@@ -209,7 +209,7 @@ func bind(c *gin.Context, v reflect.Value, tag string, extract extractor) error
 		// if no values were returned.
 		def, ok := ft.Tag.Lookup(DefaultTag)
 		if ok && len(fieldValues) == 0 {
-			if c.GetBool(ExplodeTag) {
+			if c.GetBool(ArrayTag) {
 				fieldValues = append(fieldValues, strings.Split(def, ",")...)
 			} else {
 				fieldValues = append(fieldValues, def)

diff --git a/tonic/tonic.go b/tonic/tonic.go
@@ -29,7 +29,7 @@ const (
 	RequiredTag   = "required"
 	DefaultTag    = "default"
 	ValidationTag = "validate"
-	ExplodeTag    = "explode"
+	ArrayTag      = "commalist"
 )
 
 const (
@@ -264,7 +264,7 @@ func extractQuery(c *gin.Context, tag string) (string, []string, error) {
 	var params []string
 	query := c.Request.URL.Query()[name]
 
-	if c.GetBool(ExplodeTag) {
+	if !c.GetBool(ArrayTag) {
 		// Delete empty elements so default and required arguments
 		// will play nice together. Append to a new collection to
 		// preserve order without too much copying.
@@ -303,18 +303,28 @@ func extractPath(c *gin.Context, tag string) (string, []string, error) {
 	if err != nil {
 		return "", nil, err
 	}
-	p := c.Param(name)
+
+	var params []string
+
+	if !c.GetBool(ArrayTag) {
+		params = []string{c.Param(name)}
+	} else {
+		splitFn := func(c rune) bool {
+			return c == ','
+		}
+		params = strings.FieldsFunc(c.Param(name), splitFn)
+	}
 
 	// XXX: deprecated, use of "default" tag is preferred
-	if p == "" && defaultVal != "" {
+	if len(params) == 0 && defaultVal != "" {
 		return name, []string{defaultVal}, nil
 	}
 	// XXX: deprecated, use of "validate" tag is preferred
-	if p == "" && required {
+	if len(params) == 0 && required {
 		return "", nil, fmt.Errorf("missing path parameter: %s", name)
 	}
 
-	return name, []string{p}, nil
+	return name, params, nil
 }
 
 // extractHeader is an extractor that operates on the headers

diff --git a/tonic/tonic_test.go b/tonic/tonic_test.go
@@ -32,6 +32,7 @@ func TestMain(m *testing.M) {
 	g.GET("/scalar", tonic.Handler(scalarHandler, 200))
 	g.GET("/error", tonic.Handler(errorHandler, 200))
 	g.GET("/path/:param", tonic.Handler(pathHandler, 200))
+	g.GET("/path-list/:param-path-list", tonic.Handler(pathListHandler, 200))
 	g.GET("/query", tonic.Handler(queryHandler, 200))
 	g.GET("/query-old", tonic.Handler(queryHandlerOld, 200))
 	g.POST("/body", tonic.Handler(bodyHandler, 200))
@@ -87,13 +88,13 @@ func TestPathQuery(t *testing.T) {
 
 	tester.AddCall("query-complex", "GET", fmt.Sprintf("/query?param=foo&param-complex=%s", now), "").Checkers(iffy.ExpectStatus(200), expectString("param-complex", string(now)))
 
-	// Explode.
-	tester.AddCall("query-explode", "GET", "/query?param=foo&param-explode=a&param-explode=b&param-explode=c", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-explode", "a", "b", "c"))
-	tester.AddCall("query-explode-disabled-ok", "GET", "/query?param=foo&param-explode-disabled=x,y,z", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-explode-disabled", "x", "y", "z"))
-	tester.AddCall("query-explode-disabled-error", "GET", "/query?param=foo&param-explode-disabled=a&param-explode-disabled=b", "").Checkers(iffy.ExpectStatus(400))
-	tester.AddCall("query-explode-string", "GET", "/query?param=foo&param-explode-string=x,y,z", "").Checkers(iffy.ExpectStatus(200), expectString("param-explode-string", "x,y,z"))
-	tester.AddCall("query-explode-default", "GET", "/query?param=foo", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-explode-default", "1", "2", "3"))             // default with explode
-	tester.AddCall("query-explode-disabled-default", "GET", "/query?param=foo", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-explode-disabled-default", "1,2,3")) // default without explode
+	// Array split
+	tester.AddCall("query-list-nosplit", "GET", "/query?param=foo&param-list-nosplit=a&param-list-nosplit=b&param-list-nosplit=c", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-list-nosplit", "a", "b", "c"))
+	tester.AddCall("query-list-split", "GET", "/query?param=foo&param-list-split=x,y,z", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-list-split", "x", "y", "z"))
+	tester.AddCall("query-list-split-repeated", "GET", "/query?param=foo&param-list-split=a&param-list-split=b", "").Checkers(iffy.ExpectStatus(400))
+	tester.AddCall("query-list-nosplit-single", "GET", "/query?param=foo&param-list-string-nosplit=x,y,z", "").Checkers(iffy.ExpectStatus(200), expectString("param-list-string-nosplit", "x,y,z"))
+	tester.AddCall("query-list-default", "GET", "/query?param=foo", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-list-default", "1", "2", "3")) // default with explode
+	tester.AddCall("path-list", "GET", "/path-list/1,2,3", "").Checkers(iffy.ExpectStatus(200), expectStringArr("param-path-list", "1", "2", "3"))
 
 	tester.Run()
 }
@@ -150,20 +151,27 @@ func pathHandler(c *gin.Context, in *pathIn) (*pathIn, error) {
 	return in, nil
 }
 
+type pathListIn struct {
+	ParamPathList []string `path:"param-path-list" json:"param-path-list" commalist:"true"`
+}
+
+func pathListHandler(c *gin.Context, in *pathListIn) (*pathListIn, error) {
+	return in, nil
+}
+
 type queryIn struct {
-	Param                       string    `query:"param" json:"param" validate:"required"`
-	ParamOptional               string    `query:"param-optional" json:"param-optional"`
-	Params                      []string  `query:"params" json:"params"`
-	ParamInt                    int       `query:"param-int" json:"param-int"`
-	ParamBool                   bool      `query:"param-bool" json:"param-bool"`
-	ParamDefault                string    `query:"param-default" json:"param-default" default:"default" validate:"required"`
-	ParamPtr                    *string   `query:"param-ptr" json:"param-ptr"`
-	ParamComplex                time.Time `query:"param-complex" json:"param-complex"`
-	ParamExplode                []string  `query:"param-explode" json:"param-explode" explode:"true"`
-	ParamExplodeDisabled        []string  `query:"param-explode-disabled" json:"param-explode-disabled" explode:"false"`
-	ParamExplodeString          string    `query:"param-explode-string" json:"param-explode-string" explode:"true"`
-	ParamExplodeDefault         []string  `query:"param-explode-default" json:"param-explode-default" default:"1,2,3" explode:"true"`
-	ParamExplodeDefaultDisabled []string  `query:"param-explode-disabled-default" json:"param-explode-disabled-default" default:"1,2,3" explode:"false"`
+	Param                  string    `query:"param" json:"param" validate:"required"`
+	ParamOptional          string    `query:"param-optional" json:"param-optional"`
+	Params                 []string  `query:"params" json:"params"`
+	ParamInt               int       `query:"param-int" json:"param-int"`
+	ParamBool              bool      `query:"param-bool" json:"param-bool"`
+	ParamDefault           string    `query:"param-default" json:"param-default" default:"default" validate:"required"`
+	ParamPtr               *string   `query:"param-ptr" json:"param-ptr"`
+	ParamComplex           time.Time `query:"param-complex" json:"param-complex"`
+	ParamListNoSplit       []string  `query:"param-list-nosplit" json:"param-list-nosplit"`
+	ParamListSplit         []string  `query:"param-list-split" json:"param-list-split" commalist:"true"`
+	ParamListStringNoSplit string    `query:"param-list-string-nosplit" json:"param-list-string-nosplit"`
+	ParamListDefault       []string  `query:"param-list-default" json:"param-list-default" default:"1,2,3" commalist:"true"`
 	*DoubleEmbedded
 }