schema icon indicating copy to clipboard operation
schema copied to clipboard

[bug] schema: converter not found for Page, when the struct name is same with field name.

Open donnol opened this issue 3 years ago • 4 comments

Steps to Reproduce

playground

//go:build ignore

package main

import (
	"log"

	"github.com/gorilla/schema"
)

type Page struct {
	Page     int `schema:"page"`
	PageSize int `schema:"pageSize"`
}

type PageAlias struct {
	Page     int `schema:"page"`
	PageSize int `schema:"pageSize"`
}

type Outer struct {
	Page
}

type OuterAlias struct {
	PageAlias
}

func main() {
	decoder := schema.NewDecoder()

	{
		pp := Outer{}
		if err := decoder.Decode(&pp, map[string][]string{
			"page": {"1"},
		}); err != nil {
			log.Printf("decode failed with Outer: %v\n", err)
		} else {
			log.Printf("decode result with Outer: %+v\n", pp)
		}
	}

	{
		pp := OuterAlias{}
		if err := decoder.Decode(&pp, map[string][]string{
			"page": {"1"},
		}); err != nil {
			log.Printf("decode failed with OuterAlias: %v\n", err)
		} else {
			log.Printf("decode result with OuterAlias: %+v\n", pp)
		}
	}
}

Expected behavior

What output or behaviour were you expecting instead?

Parse form to Outer struct successfully.

donnol avatar Nov 18 '22 06:11 donnol

Hi @donnol,

I believe this is expected and cannot be considered as a bug because:

  • when you do not use the schema tag, schema uses the field name in lower case to decode, and therefore:
type Outer struct {
	Page
}

is equivalent to:

type Outer struct {
	Page  `schema:"page"`
}

when you try to decode map[string][]string{"page": {"1"}}

you are trying to decode a string into a struct which schema does know how to do. To work around it, you need to use the full path in your query string: map[string][]string{"page.page": {"1"}, "page.pageSize": {"22"}}

zak905 avatar Nov 18 '22 18:11 zak905

@donnol Does my answer help ? because schema uses reflection, it's only aware of the Page field. I know in golang that embedding a struct is like unwrapping all the fields inside the target struct, so doing pp.PageSize works as well as doing pp.Page.PageSize. However, reflection only sees the embedded struct field:


	tp := reflect.TypeOf(Outer{})

        //prints 1 
	fmt.Println(tp.NumField())

	i := 0

	for i < tp.NumField() {
		fmt.Println(tp.Field(i).Name)
		i++
	}

      //prints Page

I hope this helps.

zak905 avatar Nov 24 '22 10:11 zak905

	{
		tp := reflect.TypeOf(Outer{})

		//prints 1
		fmt.Println(tp.NumField())

		i := 0

		for i < tp.NumField() {
			if tp.Field(i).Anonymous {
				fmt.Println("anonymous", tp.Field(i).Type, tp.Field(i).Type.NumField())
			}
			fmt.Println(tp.Field(i).Name)
			i++
		}
	}

I think we can use tp.Field(i).Anonymous to identify a field if is Anonymous , and then get its inner field.

donnol avatar Nov 25 '22 01:11 donnol

I believe it should be feasible, thanks for the information. I propose adding a flag when decoding/encoding that can define the behavior of the decoder/encoder when dealing embded structs. The flag can define whether we need to unwrap the structs or just treat them as a field. We can name it for example UnwrapEmbeddedStructs. Based on the valuie of the field, we can check for Anonymous and take it from there. This would keep things backward compatible.

zak905 avatar Nov 25 '22 11:11 zak905

@zak905 if you have time, if you'd like to open a new ticket for this as a feature request we can look at adding it.

jaitaiwan avatar Jun 15 '24 03:06 jaitaiwan

Sure, I can look into it !

zak905 avatar Jun 19 '24 18:06 zak905