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

Struct methods cause schema generation to fail #55

Open
TommoLeedsy opened this issue Jan 22, 2024 · 2 comments
Open

Struct methods cause schema generation to fail #55

TommoLeedsy opened this issue Jan 22, 2024 · 2 comments

Comments

@TommoLeedsy
Copy link
Contributor

TommoLeedsy commented Jan 22, 2024

Issue Description

Gin handlers that are struct methods cause schema generation to fail.

Steps to Reproduce

  1. Create a main.go with this content:
package main

import (
	"database/sql"

	"github.com/gin-gonic/gin"
	"github.com/ls6-events/astra"
	"github.com/ls6-events/astra/inputs"
	"github.com/ls6-events/astra/outputs"
)

type SharedData struct {
	db *sql.DB
}

func (sd SharedData) GetDatabaseConnectionStats(c *gin.Context) {
	c.JSON(200, sd.db.Stats())
}

func main() {
	r := gin.Default()

	db := InitDB()
	defer db.Close()

	sd := SharedData{db: db}

	r.GET("/stats", sd.GetDatabaseConnectionStats)

	gen := astra.New(inputs.WithGinInput(r), outputs.WithOpenAPIOutput("openapi.generated.yaml"))

	config := astra.Config{
		Title:   "Example API",
		Version: "1.0.0",
		Host:    "localhost",
		Port:    8080,
	}

	gen.SetConfig(&config)

	err := gen.Parse()
	if err != nil {
		panic(err)
	}

	r.Run(":8080")
}
  1. Install dependencies: go get .
  2. Run main.go: go run main.go

Expected Behaviour

Bellow is the expected output in openapi.generated.yaml from running main.go.

openapi: 3.0.0
info:
    title: Example API
    description: Generated by astra
    contact: {}
    license:
        name: ""
    version: 1.0.0
servers:
    - url: http://localhost:8080
paths:
    /stats:
        get:
            operationId: getDatabaseConnectionStats
            responses:
                "200":
                    description: ""
                    content:
                        application/json:
                            schema:
                                $ref: '#/components/schemas/sql.DBStats'
components:
    schemas:
        sql.DBStats:
            type: object
            properties:
                Idle:
                    type: integer
                    format: int32
                InUse:
                    type: integer
                    format: int32
                MaxIdleClosed:
                    type: integer
                    format: int64
                MaxIdleTimeClosed:
                    type: integer
                    format: int64
                MaxLifetimeClosed:
                    type: integer
                    format: int64
                MaxOpenConnections:
                    type: integer
                    format: int32
                OpenConnections:
                    type: integer
                    format: int32
                WaitCount:
                    type: integer
                    format: int64
                WaitDuration:
                    $ref: '#/components/schemas/time.Duration'
            description: DBStats contains database statistics.
        time.Duration:
            enum:
                - 1
            type: integer
            format: int32
            description: |-
                A Duration represents the elapsed time between two instants
                as an int64 nanosecond count. The representation limits the
                largest representable duration to approximately 290 years.

Actual Behaviour

No schema is generated and the bellow error is output in the logs:

Rel: can't make <autogenerated> relative to <path to backend>

Additional Information

  • Go Version: 1.21.1
  • Astra Version: 1.19.0
@TommoLeedsy TommoLeedsy changed the title Struct method cause schema generation to fail Struct methods cause schema generation to fail Jan 22, 2024
@indeedhat
Copy link

Iv'e done a little bit of looking into this and it seems to be a bug with runtime.FuncForPc().

NB: this is base on a little bit of scrolling through Github issues so I might have a few things wrong here but this is the issue as I understand it.

When passing around a struct method under the hood the go compiler wraps it in a closure and that is what actually gets passed. The auto generated closure has a synthetic file and line metadata so what is being returned by runtime.FuncForPC() is actually correct for the input it is given, its just not what we actually want.

Interestingly this actually used to work in <= 1.17.x

Take the following example

package main

import (
	"fmt"
	"reflect"
	"runtime"

	"github.com/davecgh/go-spew/spew"
)

type MyStruct struct{}

func (MyStruct) ValueRecv() {}

func (*MyStruct) PtrRecv() {}

func MyFunc() {}

func main() {
	val := MyStruct{}

	inspect(val.ValueRecv)
	inspect((&val).PtrRecv)
	inspect(MyFunc)
	inspect(MyStruct.ValueRecv)
	inspect((*MyStruct).ValueRecv)
}

func inspect(v interface{}) {
	pc := reflect.ValueOf(v).Pointer()
	spew.Dump(runtime.FuncForPC(pc).FileLine(pc))
	spew.Dump(runtime.FuncForPC(pc).Name())
	fmt.Println("-----")
}

when ran in 1.22.2:

:!go run /home/indeedhat/Documents/github/fuckery/funcforpc/main.go
(string) (len=15) "<autogenerated>"
(int) 1
(string) (len=26) "main.MyStruct.ValueRecv-fm"
-----
(string) (len=15) "<autogenerated>"
(int) 1
(string) (len=27) "main.(*MyStruct).PtrRecv-fm"
-----
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 17
(string) (len=11) "main.MyFunc"
-----
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 13
(string) (len=23) "main.MyStruct.ValueRecv"
-----
(string) (len=15) "<autogenerated>"
(int) 1
(string) (len=26) "main.(*MyStruct).ValueRecv"
-----

and 1.17.13

:!go1.17.13 run /home/indeedhat/Documents/github/fuckery/funcforpc/main.go
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 13
(string) (len=26) "main.MyStruct.ValueRecv-fm"
-----
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 15
(string) (len=27) "main.(*MyStruct).PtrRecv-fm"
-----
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 17
(string) (len=11) "main.MyFunc"
-----
(string) (len=58) "/home/indeedhat/Documents/github/fuckery/funcforpc/main.go"
(int) 13
(string) (len=23) "main.MyStruct.ValueRecv"
-----
(string) (len=15) "<autogenerated>"
(int) 1
(string) (len=26) "main.(*MyStruct).ValueRecv"
-----

There are a number of issues open and closed around this and related topics such as the -fm name suffix but I'm not hopeful for a fix any time soon.

My current rabbit hole is trying to find out if I can use the name to get a reference back to the parent type but I'm not hopeful at this point.

@TommoLeedsy
Copy link
Contributor Author

@indeedhat Thank you very much for looking into this issues, we've come to a similar conclusion (probably should have documented that, sorry). We're looking at moving away from using gin.Engine and using the AST to transfer the whole code and pull out the definitions from that. This change is going to be a hell of a lot of work but should give us lots of benefits especially allowing us to create at standalone CLI instead of having to embed the tool at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants