feat: add option explode to control how query params are handled. (#58)

* feat: add option explode to control how query params are handled. * tonic/handler: use strconv.ParseBool to interpret the Explode Tag, with true as default value. * Return an error if repeating variable is present in the query string with explode disabled. * fix explode/default tests cases
loopfz · Feb 11, 2019 · 66acbcf · 66acbcf
1 parent 6c919ca
commit 66acbcf
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 21 deletions.
diff --git a/tonic/handler.go b/tonic/handler.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"reflect"
 	"runtime"
+	"strconv"
 	"strings"
 	"sync"
 
@@ -192,6 +193,14 @@ func bind(c *gin.Context, v reflect.Value, tag string, extract extractor) error
 		if tagValue == "" {
 			continue
 		}
+		// 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)
+			}
+		}
 		_, fieldValues, err := extract(c, tagValue)
 		if err != nil {
 			return BindError{field: ft.Name, typ: t, message: err.Error()}
@@ -200,7 +209,11 @@ 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 {
-			fieldValues = append(fieldValues, def)
+			if c.GetBool(ExplodeTag) {
+				fieldValues = append(fieldValues, strings.Split(def, ",")...)
+			} else {
+				fieldValues = append(fieldValues, def)
+			}
 		}
 		if len(fieldValues) == 0 {
 			continue

diff --git a/tonic/tonic.go b/tonic/tonic.go
@@ -2,6 +2,7 @@ package tonic
 
 import (
 	"encoding"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -28,6 +29,7 @@ const (
 	RequiredTag   = "required"
 	DefaultTag    = "default"
 	ValidationTag = "validate"
+	ExplodeTag    = "explode"
 )
 
 const (
@@ -254,27 +256,39 @@ func extractQuery(c *gin.Context, tag string) (string, []string, error) {
 	if err != nil {
 		return "", nil, err
 	}
-
-	rawQ := c.Request.URL.Query()[name]
-
-	// delete empty elements so default+required will play nice together
-	// append to a new collection to preserve order without too much copying
-	q := make([]string, 0, len(rawQ))
-	for i := range rawQ {
-		if rawQ[i] != "" {
-			q = append(q, rawQ[i])
+	var params []string
+	query := c.Request.URL.Query()[name]
+
+	if c.GetBool(ExplodeTag) {
+		// Delete empty elements so default and required arguments
+		// will play nice together. Append to a new collection to
+		// preserve order without too much copying.
+		params = make([]string, 0, len(query))
+		for i := range query {
+			if query[i] != "" {
+				params = append(params, query[i])
+			}
+		}
+	} else {
+		splitFn := func(c rune) bool {
+			return c == ','
+		}
+		if len(query) > 1 {
+			return name, nil, errors.New("repeating values not supported: use comma-separated list")
+		} else if len(query) == 1 {
+			params = strings.FieldsFunc(query[0], splitFn)
 		}
 	}
 
 	// XXX: deprecated, use of "default" tag is preferred
-	if len(q) == 0 && defaultVal != "" {
+	if len(params) == 0 && defaultVal != "" {
 		return name, []string{defaultVal}, nil
 	}
 	// XXX: deprecated, use of "validate" tag is preferred
-	if len(q) == 0 && required {
+	if len(params) == 0 && required {
 		return "", nil, fmt.Errorf("missing query parameter: %s", name)
 	}
-	return name, q, nil
+	return name, params, nil
 }
 
 // extractPath is an extractor that operates on the path

diff --git a/tonic/tonic_test.go b/tonic/tonic_test.go
@@ -87,6 +87,14 @@ 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
+
 	tester.Run()
 }
 
@@ -143,14 +151,19 @@ func pathHandler(c *gin.Context, in *pathIn) (*pathIn, error) {
 }
 
 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"`
+	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"`
 	*DoubleEmbedded
 }