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

OpenAPI Spec Generation Doesn't Differentiate Between Single Struct and Slice #130

Open
epentland opened this issue Jun 7, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@epentland
Copy link
Contributor

To Reproduce
Steps to reproduce the behavior:

I have two handlers:
fuego.Get(group, "/{id}", rh.GetStat)
fuego.Get(group, "/", rh.ListStats)

func (rh RouteHandler) ListStats(c *fuego.ContextNoBody) ([]models.Stat, error) {
return rh.Service.ListStats()
}

func (rh RouteHandler) GetStat(c *fuego.ContextNoBody) (models.Stat, error) {
id := c.PathParam("id")
return rh.Service.GetStat(id)
}

When the openapi spec is generated, the schema doesn't differentiate between passing a single struct versus a slice, and it only creates a schema for the first instance of the struct registered. So in the example above, it generates the following schema. (if the list function was first it would make it an array type)

Stat:
  properties:
    Description:
      type: string
    ID:
      type: string
    Query:
      type: string
    Result:
      type: integer
  type: object

Expected behavior
I would expect it generate two separate schemas, one as shown above, and another one that is an array of the Stats schema.

Framework version (please check if it happens with the last Fuego version before posting):
0.13.4

Go version (please check if it happens with the last Go version before posting):
1.22.0

@epentland epentland added the bug Something isn't working label Jun 7, 2024
@dylanhitt
Copy link
Collaborator

Hey @epentland, thank you for the issue on this. I actually ran into it the other day at my day job. A workaround for me was to just wrap the object. I understand this is not ideal.

I currently don't have the time to really look into this deeply over ~ the next month. If I find some time I'll get to it. Feel free to submit a PR yourself, or hopefully someone else will get around to it.

@epentland
Copy link
Contributor Author

Yeah it looks like I was able to put together a quick hack, will need to do some proper testing an make it do a reference to the original struct instead of just making it plural. I'll see if I can make a PR.

// openapi.go
func tagFromType(v any) string {
	if v == nil {
		return "unknown-interface"
	}
	return dive(reflect.TypeOf(v), 4)
}

// dive returns the name of the type of the given reflect.Type.
// If the type is a pointer, slice, array, map, channel, function, or unsafe pointer,
// it will dive into the type and return the name of the type it points to.
func dive(t reflect.Type, maxDepth int) string {
	switch t.Kind() {
	case reflect.Slice, reflect.Array:
		if maxDepth == 0 {
			return "default"
		}
		// Makes a new struct that is pluralized
		return dive(t.Elem(), maxDepth-1) + "s"
	case reflect.Ptr, reflect.Map, reflect.Chan, reflect.Func, reflect.UnsafePointer:
		if maxDepth == 0 {
			return "default"
		}
		return dive(t.Elem(), maxDepth-1)
	default:
		return t.Name()
	}
}

@dylanhitt
Copy link
Collaborator

dylanhitt commented Jun 11, 2024

Finally looked at this. I'm curious what your thoughts are by returning a reference?

@EwenQuim chime in here if you desire, but maybe we return a new struct from dive/tagFromType

type struct tag {
  tag string
  type string
}

tag is what we're returning now and type is only set if it's an array?

Maybe that's a bit naive, but we're gonna need some type of logic to differentiate the {reponse/body}Schema in RegisterOpenAPIOperation

like

content := openapi3.NewContentWithSchema(bodySchema.Value, []string{"application/json"})
ref := "#/components/schemas/" + bodyTag.Tag
if bodyTag.Tag == "array" {
  content["application/json"].Schema.Value = &openapi3.Schema{
    Type: "array",
    Items: &openapi3.SchemaRef{
	Ref:  ref,
    },
  } 
} else {
  // what was originally there probably want a func so it's idiomatic or whatever
  content["application/json"].Schema.Ref = "#/components/schemas/" + bodyTag.Tag
}

requestBody.WithContent(content)

Also a good chance for some reason factoring here looks like logic for bodySchema and responseSchema are generally the same.

@dylanhitt
Copy link
Collaborator

Please let me know if anyone has made any progress on this. If not I will implement the above.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants