Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom decoder accepts slice of string but expected to return single item #39

Closed
vearutop opened this issue Jan 29, 2019 · 4 comments
Closed
Assignees
Labels

Comments

@vearutop
Copy link
Contributor

What is the reason to feed custom decoder with slice of strings?

type DecodeCustomTypeFunc func([]string) (interface{}, error)

Decoder function is called at

func (d *decoder) setFieldByType(current reflect.Value, namespace []byte, idx int) (set bool) {

https://github.com/go-playground/form/blob/master/decoder.go#L191

If target field is a slice, setFieldByType is called in a loop with idx iterating through []string. Internally custom decoder is called with that full []string and without any mention of current idx.

How decoder function is expected to deal with multi-element input?

To me it seems correct code should be

	val, err := cf([]string{arr[idx]})

but then I'm not sure why decoder function needs to accept a slice.

@deankarn
Copy link
Contributor

Hey @vearutop apologies in advance for formatting, answering from my phone :)

so each value that is to be decoded is really a URL.Value which is type map[string][]string, that’s where the array of strings comes from that’s passed to the customer decoder functions.

As for not passing idx is because it’s not really needed because the custom decoding function is looked up by namespace, so there are only 2 possibilities:

  1. The namespace is MyStruct.SliceField[0] and so the custom decoder function will only trigger for element 0 and so the index is already handled and not needed.

  2. The namespace is MyStruct.SliceField and the custom decoder function will be passed the whole slice and so again idx won’t be needed as your operating on the whole slice.

Please let me know if that makes sense, sorry again about the formatting and lack of links

@vearutop
Copy link
Contributor Author

vearutop commented Feb 1, 2019

Here is an example:

type customString string

func TestDecoder_RegisterCustomTypeFunc(t *testing.T) {
	type TestStruct struct {
		Slice []customString `form:"slice"`
	}

	d := NewDecoder()
	d.RegisterCustomTypeFunc(func(vals []string) (i interface{}, e error) {
		return customString("custom" + vals[0]), nil
	}, customString(""))

	var v TestStruct
	err := d.Decode(&v, url.Values{"slice": []string{"v1", "v2"}})
	Equal(t, err, nil)

	Equal(t, v, []customString{"customv1", "customv2"})

}

I would expect such test to pass, but it fails with:

{[customv1 customv1]} does not equal [customv1 customv2]

If I return []customString{...} from decoder func I get a panic:

panic: reflect.Set: value of type []form.customString is not assignable to type form.customString

Given that we have a loop over []string outside decoder function, it does not make sense to me to pass whole slice to decoder function.

@deankarn deankarn self-assigned this Feb 3, 2019
@deankarn deankarn added the bug label Feb 3, 2019
@deankarn
Copy link
Contributor

deankarn commented Feb 3, 2019

ah now I understand, thanks @vearutop it is indeed a bug, fixing ASAP

@deankarn
Copy link
Contributor

deankarn commented Feb 3, 2019

As a side note here is an alternative that will also give you what you want:

package main

import (
	"net/url"
	"testing"

	"github.com/go-playground/form"
	. "gopkg.in/go-playground/assert.v1"
)

type customString string

func TestDecoder_RegisterCustomTypeFunc(t *testing.T) {
	type TestStruct struct {
		Slice []customString `form:"slice"`
	}

	d := form.NewDecoder()
	d.RegisterCustomTypeFunc(func(vals []string) (i interface{}, e error) {
		custom := make([]customString, 0, len(vals))
		for i := 0; i < len(vals); i++ {
			custom = append(custom, customString("custom"+vals[i]))
		}
		return custom, nil
	}, []customString{})

	var v TestStruct
	err := d.Decode(&v, url.Values{"slice": []string{"v1", "v2"}})
	Equal(t, err, nil)
	Equal(t, v.Slice, []customString{"customv1", "customv2"})
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants