gorilla/schema

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

donnol opened this issue · 6 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.

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"}}

@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.

	{
		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.

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 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.

Sure, I can look into it !