swaggest / jsonschema-go

JSON Schema mapping for Go
https://pkg.go.dev/github.com/swaggest/jsonschema-go
MIT License
102 stars 13 forks source link

"reflect: NumField of non-struct type `*yodb.DateTime`" (which means `*time.Time`) #99

Closed metaleap closed 1 week ago

metaleap commented 7 months ago

Sorry for the terse panic report =)

type DateTime time.Time, which makes *DateTime structurally *time.Time.

Here's the stack trace:

Stack:
     2  0x00000000004add90 in reflect.(*rtype).NumField
         at /usr/lib/go/src/reflect/type.go:762
     3  0x0000000000fa5fd0 in github.com/swaggest/jsonschema-go.(*Reflector).makeFields
         at /home/_/c/go/pkg/mod/github.com/swaggest/jsonschema-go@v0.3.62/reflect.go:868
     4  0x0000000000fa66c5 in github.com/swaggest/jsonschema-go.(*Reflector).walkProperties
         at /home/_/c/go/pkg/mod/github.com/swaggest/jsonschema-go@v0.3.62/reflect.go:878
     5  0x0000000000fa4b45 in github.com/swaggest/jsonschema-go.(*Reflector).kindSwitch
         at /home/_/c/go/pkg/mod/github.com/swaggest/jsonschema-go@v0.3.62/reflect.go:696
     6  0x0000000000fa0cb6 in github.com/swaggest/jsonschema-go.(*Reflector).reflect
         at /home/_/c/go/pkg/mod/github.com/swaggest/jsonschema-go@v0.3.62/reflect.go:485
     7  0x0000000000f9e026 in github.com/swaggest/jsonschema-go.(*Reflector).Reflect
         at /home/_/c/go/pkg/mod/github.com/swaggest/jsonschema-go@v0.3.62/reflect.go:261
     8  0x0000000000fe5135 in github.com/swaggest/openapi-go/openapi31.(*Reflector).parseParametersIn.func2

My first non-codebase-insider guess: makeFields might, before NumField, need something like:

[for|if] ty.Kind() == reflect.Pointer { ty = ty.Elem() }

But what boggles the mind is: it already has that! Right at the start...

vearutop commented 7 months ago

I tried to reproduce and could not:

func TestReflector_Reflect_customTime(t *testing.T) {
    type MyTime time.Time
    type MyPtrTime *time.Time

    type MyStruct struct {
        T1 MyTime     `json:"t1"`
        T2 *MyTime    `json:"t2"`
        T3 *MyPtrTime `json:"t3"`
    }

    r := jsonschema.Reflector{}
    s, err := r.Reflect(MyStruct{})

    require.NoError(t, err)
    assertjson.EqMarshal(t, `{
      "definitions":{"JsonschemaGoTestMyTime":{"type":"object"}},
      "properties":{
        "t1":{"$ref":"#/definitions/JsonschemaGoTestMyTime"},
        "t2":{"$ref":"#/definitions/JsonschemaGoTestMyTime"},
        "t3":{"type":["null","string"],"format":"date-time"}
      },
      "type":"object"
    }`, s)
}

Could you share some more details about the reflected structure?

metaleap commented 7 months ago

Wow, thanks for checking! This is actually from openapi-go usage, I should have added (but throws from jsonschema-go). And sorry for the lack of repro last night. But fresh morning, now here's a repro_test.go that will reliably provoke the above-quoted panic at my end, and hopefully at yours too:

package repro_test

import (
    "reflect"
    "testing"
    "time"

    "github.com/swaggest/jsonschema-go"
    "github.com/swaggest/openapi-go"
    "github.com/swaggest/openapi-go/openapi31"
)

type yodb_I64 int64
type yodb_DateTime time.Time
type Return[T any] struct{ Result T }

type Post struct {
    Id     yodb_I64 // note: panic occurs also _with_ json: struct-field tags placed here
    DtMade *yodb_DateTime // as well as without — so, not a factor
}

func Test_Repro(t *testing.T) {
    oarefl := openapi31.NewReflector()
    oarefl.Spec.Info.WithTitle("kaffe.local")
    oarefl.JSONSchemaReflector().DefaultOptions = append(oarefl.JSONSchemaReflector().DefaultOptions, jsonschema.ProcessWithoutTags)

    {
        var dummy_in Post
        var dummy_out Return[yodb_I64]
        ty_args, ty_ret := reflect.TypeOf(dummy_in), reflect.TypeOf(dummy_out)
        op, err := oarefl.NewOperationContext("POST", "/_/postNew")
        if err != nil {
            t.Fatal(err)
        }
        op.AddReqStructure(reflect.New(ty_args).Elem().Interface(), openapi.WithContentType("application/json")) 
        op.AddRespStructure(reflect.New(ty_ret).Elem().Interface(), openapi.WithHTTPStatus(200)) 
        // note on above 2 `Elem()`s: panic occurs with or without them
        if err = oarefl.AddOperation(op); err != nil {
            t.Fatal(err)
        }
    }

    src_json, err := oarefl.Spec.MarshalJSON()
    if err != nil {
        t.Fatal(err)
    }
    t.Log(string(src_json))
}

func (me *yodb_DateTime) UnmarshalJSON(data []byte) error {
    return ((*time.Time)(me)).UnmarshalJSON(data)
}
func (me *yodb_DateTime) MarshalJSON() ([]byte, error) { return ((*time.Time)(me)).MarshalJSON() }

With openapi-go@v0.2.42 and jsonschema-go@v0.3.62 according to stacktrace.

metaleap commented 7 months ago

Rolling my own stuff in the meanwhile, I ran into the seemingly-same issue, and looks like it kinda evaporated by changing from this pattern:

func dummyOf(ty reflect.Type, level int, typesDone map[reflect.Type]bool) *reflect.Value {
    dummy := reflect.New(ty).Elem()
    for ty.Kind() == reflect.Pointer {
        ty = ty.Elem()
        dummy = dummy.Elem()
    }
    myAssert(dummy.IsValid()) // panics on hitting the culprit-type of this Issue thread
    // ...

to this pattern instead:

func dummyOf(ty reflect.Type, level int, typesDone map[reflect.Type]bool) *reflect.Value {
    if ty.Kind() == reflect.Pointer {
        return dummyOf(ty.Elem(), level, typesDone)
    }
    dummy := reflect.New(ty).Elem()
    myAssert(dummy.IsValid()) // no more panic
    // ...