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

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

Open
donnol opened this issue Nov 18, 2022 · 4 comments
Labels

Comments

@donnol
Copy link

donnol commented Nov 18, 2022

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 donnol added the bug label Nov 18, 2022
@zak905
Copy link
Contributor

zak905 commented Nov 18, 2022

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
Copy link
Contributor

zak905 commented Nov 24, 2022

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

@donnol
Copy link
Author

donnol commented Nov 25, 2022

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

@zak905
Copy link
Contributor

zak905 commented Nov 25, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants