gorilla / schema

Package gorilla/schema fills a struct with form values.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
1.39k stars 231 forks source link

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

Closed donnol closed 5 months ago

donnol commented 2 years ago

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.

zak905 commented 2 years ago

Hi @donnol,

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

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 commented 1 year ago

@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 commented 1 year ago
    {
        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 commented 1 year ago

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.

jaitaiwan commented 5 months ago

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

zak905 commented 5 months ago

Sure, I can look into it !