danielgtaylor / huma

Huma REST/HTTP API Framework for Golang with OpenAPI 3.1
https://huma.rocks/
MIT License
2.12k stars 151 forks source link

Unexported Fields in Schemas are Not Considered for Schema Generation #444

Open srilman opened 5 months ago

srilman commented 5 months ago

Currently in the process of building some of the more complex schemas for an OpenAPI-based API and running into some new issues. I have an example that looks something like this:

In models.go

type commonElems struct {
    Inner string `json:"inner"`
}

type V1 struct {
    A int `json:"a"`
    commonElems
}

type V2 struct {
    B int `json:"b"`
    commonElems
}

In main.go:

func main() {
    x := controller.V1{}
        // It is still possible to access the contents of commonElems
    x.Inner = "Hello"

    registry := huma.NewMapRegistry("#/prefix", huma.DefaultSchemaNamer)
    t := reflect.TypeOf(controller.V1{})
    schema := huma.SchemaFromType(registry, t)
    fmt.Printf("Schema: %v\n", schema)
}

The output only contains the outer field

Schema: &{object false      <nil> [] <nil> false map[a:0x140001f6008] [] <nil> <nil> <nil> <nil> <nil> <nil> <nil>   <nil> <nil> false [a] <nil> <nil> false false false map[] map[] [] [] [] <nil> <nil> <nil> map[a:true] [a] expected value to be one of ""             map[a:expected required property a to be present] map[]}

Changing commonElems to CommonElems so the embedded struct is exported, the output contains both expected fields:

Schema: &{object false      <nil> [] <nil> false map[a:0x14000254008 inner:0x14000254308] [] <nil> <nil> <nil> <nil> <nil> <nil> <nil>   <nil> <nil> false [a inner] <nil> <nil> false false false map[] map[] [] [] [] <nil> <nil> <nil> map[a:true inner:true] [a inner] expected value to be one of ""             map[a:expected required property a to be present inner:expected required property inner to be present] map[]}

Unfortunately, unlike the example, the structs V1 and V2 are located in another library that I don't have easy control over. But furthermore, I think the second output should be the default, since in Go we can still access exported fields inside of unexported embedded structs (as seen in the code example) and JSON marshalling/unmarshalling works the same. What are others thoughts?

srilman commented 5 months ago

And from my understanding, to change this behavior, we just need to change function getFields in file huma/schema.go:

func getFields(typ reflect.Type, visited map[reflect.Type]struct{}) []fieldInfo {
    fields := make([]fieldInfo, 0, typ.NumField())
    var embedded []reflect.StructField

    if _, ok := visited[typ]; ok {
        return fields
    }
    visited[typ] = struct{}{}

    for i := 0; i < typ.NumField(); i++ {
        f := typ.Field(i)
        if !f.IsExported() {
            continue
        }

        if f.Anonymous {
            embedded = append(embedded, f)
            continue
        }

        fields = append(fields, fieldInfo{typ, f})
    }

    for _, f := range embedded {
        newTyp := f.Type
        for newTyp.Kind() == reflect.Ptr {
            newTyp = newTyp.Elem()
        }
        if newTyp.Kind() == reflect.Struct {
            fields = append(fields, getFields(newTyp, visited)...)
        }
    }

    return fields
}

All to change is move this snippet:

        if !f.IsExported() {
            continue
        }

underneath the if statement if f.Anonymous {

danielgtaylor commented 5 months ago

@srilman thanks for the issue! I believe this was done on purpose because Go can't use reflection to access private fields from other packages, so it would be inconsistent to allow access from within your package but disallow it from imported packages. I can dig a bit deeper just to confirm this is the case with embedded private fields.