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

URL Query params should be hinted as "form", not "json" #500

Closed
djeer opened this issue Dec 16, 2021 · 6 comments
Closed

URL Query params should be hinted as "form", not "json" #500

djeer opened this issue Dec 16, 2021 · 6 comments

Comments

@djeer
Copy link

djeer commented Dec 16, 2021

I've run into in issue that my parsed query params are always nil. It was resolved by manually changing query params hint from "json" to "form" in the generated types. The problem may be reproduced using petstore-expanded.yaml, generated code (not working):

type FindPetsParams struct {
	// tags to filter by
	Tags *[]string `json:"tags,omitempty"`

	// maximum number of results to return
	Limit *int32 `json:"limit,omitempty"`
}

code I expect to be generated (working):

type FindPetsParams struct {
	// tags to filter by
	Tags *[]string `form:"tags,omitempty"`

	// maximum number of results to return
	Limit *int32 `form:"limit,omitempty"`
}

Code I am using to parse query params:

func ParseFilter(c *gin.Context) (myapi.FindPetsParams, error) {
	f := tasksapi.FindPetsParams{}
	if err := c.BindQuery(&f); err != nil {
		return f, err
	}
	return f, nil
}
@djeer
Copy link
Author

djeer commented Dec 16, 2021

I've found exact place where it is generated https://github.com/deepmap/oapi-codegen/blob/master/pkg/codegen/schema.go#L447

if p.Required || p.Nullable || !omitEmpty {
	fieldTags["json"] = p.JsonFieldName
} else {
	fieldTags["json"] = p.JsonFieldName + ",omitempty"
}

@djeer
Copy link
Author

djeer commented Dec 16, 2021

Checked the code from master branch, auto-generated server reads query parameters as "form", so it's just annotations that are wrong representing them as "json" (annotations are not used by generated server). Created PR to fix this. Build passed green.

image

@deepmap-marcinr
Copy link
Contributor

The annotation on those fields isn't related to the parameter type in BindQueryParameter. There is something missing in the problem description. By changing the annotation to form:"...", you are changing the way JSON is parsed, so perhaps that's changing things up in a way that make them work.

@deepmap-marcinr
Copy link
Contributor

Oh, I see what's going on.

We need both annotations, then, both form and json on those objects.

Perhaps we add the form annotation for gin.

@djeer
Copy link
Author

djeer commented Jan 21, 2022

@deepmap-marcinr I want to clarify, an auto-generated server works fine since it generates boilerplate code using BindQueryParameter and not using any annotations. But we are not using an auto-generated server and therefore the annotations are needed for BindQuery to work if we want to parse all parameters without writing much code. It was not clear at first why they are always nil.

Both form and json annotations should work fine. Thanks.

@djeer
Copy link
Author

djeer commented May 19, 2022

Very nice, thanks!

adrianpk pushed a commit to foorester/oapi-codegen that referenced this issue Jan 16, 2024
This fixes deepmap#500, by adding a form tag to properties that come
from request parameters and have a style of "form".
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