chore: simplify http query array parsing (#4637)

core/mapping/fieldoptions.go

@@ -8,14 +8,13 @@ type (
 	// use context and OptionalDep option to determine the value of Optional
 	// nothing to do with context.Context
 	fieldOptionsWithContext struct {
-		Inherit        bool
-		FromString     bool
-		Optional       bool
-		Options        []string
-		Default        string
-		EnvVar         string
-		Range          *numberRange
-		FormArrayComma bool
+		Inherit    bool
+		FromString bool
+		Optional   bool
+		Options    []string
+		Default    string
+		EnvVar     string
+		Range      *numberRange
 	}
 
 	fieldOptions struct {

core/mapping/unmarshaler.go

@@ -131,7 +131,7 @@ func (u *Unmarshaler) fillMapFromString(value reflect.Value, mapValue any) error
 }
 
 func (u *Unmarshaler) fillSlice(fieldType reflect.Type, value reflect.Value,
-	mapValue any, fullName string, opts *fieldOptionsWithContext) error {
+	mapValue any, fullName string) error {
 	if !value.CanSet() {
 		return errValueNotSettable
 	}
@@ -152,10 +152,6 @@ func (u *Unmarshaler) fillSlice(fieldType reflect.Type, value reflect.Value,
 		return nil
 	}
 
-	if u.opts.fromArray {
-		refValue = makeStringSlice(refValue, opts)
-	}
-
 	var valid bool
 	conv := reflect.MakeSlice(reflect.SliceOf(baseType), refValue.Len(), refValue.Cap())
 
@@ -174,7 +170,7 @@ func (u *Unmarshaler) fillSlice(fieldType reflect.Type, value reflect.Value,
 				return err
 			}
 		case reflect.Slice:
-			if err := u.fillSlice(dereffedBaseType, conv.Index(i), ithValue, sliceFullName, opts); err != nil {
+			if err := u.fillSlice(dereffedBaseType, conv.Index(i), ithValue, sliceFullName); err != nil {
 				return err
 			}
 		default:
@@ -288,7 +284,7 @@ func (u *Unmarshaler) fillSliceWithDefault(derefedType reflect.Type, value refle
 		defaultCacheLock.Unlock()
 	}
 
-	return u.fillSlice(derefedType, value, slice, fullName, nil)
+	return u.fillSlice(derefedType, value, slice, fullName)
 }
 
 func (u *Unmarshaler) fillStructElement(baseType reflect.Type, target reflect.Value,
@@ -359,7 +355,7 @@ func (u *Unmarshaler) generateMap(keyType, elemType reflect.Type, mapValue any,
 		switch dereffedElemKind {
 		case reflect.Slice:
 			target := reflect.New(dereffedElemType)
-			if err := u.fillSlice(elemType, target.Elem(), keythData, mapFullName, nil); err != nil {
+			if err := u.fillSlice(elemType, target.Elem(), keythData, mapFullName); err != nil {
 				return emptyValue, err
 			}
 
@@ -604,7 +600,7 @@ func (u *Unmarshaler) processFieldNotFromString(fieldType reflect.Type, value re
 			parent:  vp.parent,
 		}, fullName)
 	case typeKind == reflect.Slice && valueKind == reflect.Slice:
-		return u.fillSlice(fieldType, value, mapValue, fullName, opts)
+		return u.fillSlice(fieldType, value, mapValue, fullName)
 	case valueKind == reflect.Map && typeKind == reflect.Map:
 		return u.fillMap(fieldType, value, mapValue, fullName)
 	case valueKind == reflect.String && typeKind == reflect.Map:
@@ -985,7 +981,7 @@ func (u *Unmarshaler) unmarshal(i, v any, fullName string) error {
 			return errTypeMismatch
 		}
 
-		return u.fillSlice(elemType, reflect.ValueOf(v).Elem(), iv, fullName, nil)
+		return u.fillSlice(elemType, reflect.ValueOf(v).Elem(), iv, fullName)
 	default:
 		return errUnsupportedType
 	}
@@ -1189,39 +1185,6 @@ func join(elem ...string) string {
 	return builder.String()
 }
 
-func makeStringSlice(refValue reflect.Value, opts *fieldOptionsWithContext) reflect.Value {
-	if refValue.Len() != 1 {
-		return refValue
-	}
-
-	element := refValue.Index(0)
-	if element.Kind() != reflect.String {
-		return refValue
-	}
-
-	val, ok := element.Interface().(string)
-	if !ok {
-		return refValue
-	}
-	// comma mode is on by default or display designations are split by commas
-	if opts == nil || opts.FormArrayComma {
-		splits := strings.Split(val, comma)
-		if len(splits) <= 1 {
-			return refValue
-		}
-
-		slice := reflect.MakeSlice(stringSliceType, len(splits), len(splits))
-		for i, split := range splits {
-			// allow empty strings
-			slice.Index(i).Set(reflect.ValueOf(split))
-		}
-		return slice
-	} else {
-		return refValue
-	}
-
-}
-
 func newInitError(name string) error {
 	return fmt.Errorf("field %q is not set", name)
 }

core/mapping/unmarshaler_test.go

@@ -1462,9 +1462,7 @@ func TestUnmarshalIntSlice(t *testing.T) {
 
 		ast := assert.New(t)
 		unmarshaler := NewUnmarshaler(defaultKeyName, WithFromArray())
-		if ast.NoError(unmarshaler.Unmarshal(m, &v)) {
-			ast.ElementsMatch([]int{1, 2}, v.Ages)
-		}
+		ast.Error(unmarshaler.Unmarshal(m, &v))
 	})
 }
 
@@ -1546,7 +1544,22 @@ func TestUnmarshalStringSliceFromString(t *testing.T) {
 		ast := assert.New(t)
 		unmarshaler := NewUnmarshaler(defaultKeyName, WithFromArray())
 		if ast.NoError(unmarshaler.Unmarshal(m, &v)) {
-			ast.ElementsMatch([]string{"", ""}, v.Names)
+			ast.ElementsMatch([]string{","}, v.Names)
+		}
+	})
+
+	t.Run("slice from valid strings with comma", func(t *testing.T) {
+		var v struct {
+			Names []string `key:"names"`
+		}
+		m := map[string]any{
+			"names": []string{"aa,bb"},
+		}
+
+		ast := assert.New(t)
+		unmarshaler := NewUnmarshaler(defaultKeyName, WithFromArray())
+		if ast.NoError(unmarshaler.Unmarshal(m, &v)) {
+			ast.ElementsMatch([]string{"aa,bb"}, v.Names)
 		}
 	})
 

core/mapping/utils.go

@@ -22,7 +22,6 @@ const (
 	optionalOption     = "optional"
 	optionsOption      = "options"
 	rangeOption        = "range"
-	formArrayComma     = "arrayComma"
 	optionSeparator    = "|"
 	equalToken         = "="
 	escapeChar         = '\\'
@@ -161,10 +160,6 @@ func doParseKeyAndOptions(field reflect.StructField, value string) (string, *fie
 	}
 
 	var fieldOpts fieldOptions
-
-	// The comma split form array mode was enabled in 1.7.5
-	// so the default value is true in order not to introduce destructiveness
-	fieldOpts.FormArrayComma = true
 	for _, segment := range options {
 		option := strings.TrimSpace(segment)
 		if err := parseOption(&fieldOpts, field.Name, option); err != nil {
@@ -415,12 +410,6 @@ func parseOption(fieldOpts *fieldOptions, fieldName, option string) error {
 		}
 
 		fieldOpts.Range = nr
-	case strings.HasPrefix(option, formArrayComma):
-		val, err := parseEqBoolOption(fieldName, formArrayComma, option)
-		if err != nil {
-			return err
-		}
-		fieldOpts.FormArrayComma = val
 	}
 
 	return nil
@@ -448,21 +437,7 @@ func parseProperty(field, tag, val string) (string, error) {
 
 	return strings.TrimSpace(segs[1]), nil
 }
-func parseEqBoolOption(field, tag, val string) (bool, error) {
-	segs := strings.Split(val, equalToken)
-	switch len(segs) {
-	case 1:
-		return true, nil
-	case 2:
-		parseBool, err := strconv.ParseBool(segs[1])
-		if err != nil {
-			return false, fmt.Errorf("field %q has wrong %s", field, tag)
-		}
-		return parseBool, nil
-	default:
-		return false, fmt.Errorf("field %q has wrong %s", field, tag)
-	}
-}
+
 func parseSegments(val string) []string {
 	var segments []string
 	var escaped, grouped bool

rest/httpx/requests_test.go

@@ -160,7 +160,7 @@ func TestParseFormArray(t *testing.T) {
 			http.NoBody)
 		assert.NoError(t, err)
 		if assert.NoError(t, Parse(r, &v)) {
-			assert.ElementsMatch(t, []string{"1", "2", "3"}, v.Names)
+			assert.ElementsMatch(t, []string{"1,2,3"}, v.Names)
 		}
 	})
 
@@ -189,9 +189,7 @@ func TestParseFormArray(t *testing.T) {
 			"/a?numbers=1,2,3",
 			http.NoBody)
 		assert.NoError(t, err)
-		if assert.NoError(t, Parse(r, &v)) {
-			assert.ElementsMatch(t, []int{1, 2, 3}, v.Numbers)
-		}
+		assert.Error(t, Parse(r, &v))
 	})
 
 	t.Run("slice with one value on array format brackets", func(t *testing.T) {
@@ -268,9 +266,10 @@ func TestParseFormArray(t *testing.T) {
 			assert.ElementsMatch(t, []float64{2}, v.Numbers)
 		}
 	})
-	t.Run("slice with one value on  disable array of comma split format", func(t *testing.T) {
+
+	t.Run("slice with one value", func(t *testing.T) {
 		var v struct {
-			Codes []string `form:"codes,arrayComma=false"`
+			Codes []string `form:"codes"`
 		}
 		r, err := http.NewRequest(
 			http.MethodGet,
@@ -281,23 +280,10 @@ func TestParseFormArray(t *testing.T) {
 			assert.ElementsMatch(t, []string{"aaa,bbb,ccc"}, v.Codes)
 		}
 	})
-	t.Run("slice with multiple value on  disable array of comma split format", func(t *testing.T) {
-		var v struct {
-			Codes []string `form:"codes,arrayComma=false"`
-		}
 
-		r, err := http.NewRequest(
-			http.MethodGet,
-			"/a?codes=aaa,bbb,ccc&codes=ccc,ddd,eee",
-			http.NoBody)
-		assert.NoError(t, err)
-		if assert.NoError(t, Parse(r, &v)) {
-			assert.ElementsMatch(t, []string{"aaa,bbb,ccc", "ccc,ddd,eee"}, v.Codes)
-		}
-	})
-	t.Run("slice with multiple value on enable array of comma split format", func(t *testing.T) {
+	t.Run("slice with multiple values", func(t *testing.T) {
 		var v struct {
-			Codes []string `form:"codes,arrayComma=true"`
+			Codes []string `form:"codes,arrayComma=false"`
 		}
 
 		r, err := http.NewRequest(
@@ -309,20 +295,6 @@ func TestParseFormArray(t *testing.T) {
 			assert.ElementsMatch(t, []string{"aaa,bbb,ccc", "ccc,ddd,eee"}, v.Codes)
 		}
 	})
-	t.Run("slice with one value on enable array of comma split format", func(t *testing.T) {
-		var v struct {
-			Codes []string `form:"codes,arrayComma=true"`
-		}
-
-		r, err := http.NewRequest(
-			http.MethodGet,
-			"/a?codes=aaa,bbb,ccc",
-			http.NoBody)
-		assert.NoError(t, err)
-		if assert.NoError(t, Parse(r, &v)) {
-			assert.ElementsMatch(t, []string{"aaa", "bbb", "ccc"}, v.Codes)
-		}
-	})
 }
 
 func TestParseForm_Error(t *testing.T) {