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

setupRequest doesn't work with jsonschema.Struct #109

Closed
ArujReis opened this issue May 20, 2024 · 2 comments · Fixed by #110
Closed

setupRequest doesn't work with jsonschema.Struct #109

ArujReis opened this issue May 20, 2024 · 2 comments · Fixed by #110

Comments

@ArujReis
Copy link

Describe the bug
When a jsonschema.Struct is passed to setupRequest it doesn't work. For example, when used with oc.AddReqStructure. As far as I can tell, it happens because refl.HasTaggedFields (called from ReflectRequestBody) works on the Struct's fields instead of the Field elements.

To Reproduce
Here is an example for HasTaggedFields returning an incorrect answer.

Expected behavior
setupRequest should work on jsonschema.Struct the same way setupResponse does.

Additional context
I'm trying to add an OpenAPI schema to an existing APIs. I don't want to rewrite all the input structs, but wanted to extend them to include the URL params. Unfortunately, this didn't work due the aforementioned issue.

@vearutop
Copy link
Member

Please try latest v0.2.51.

@ArujReis
Copy link
Author

Works! Thanks for the quick fix!

As a side note, it still doesn't work if jsonschema.Struct is passed directly, rather than being embedded in a struct, since the check doesn't check the input itself:

	refl.WalkFieldsRecursively(reflect.ValueOf(input), func(v reflect.Value, _ reflect.StructField, _ []reflect.StructField) {
		if v.Type() == reflect.TypeOf(jsonschema.Struct{}) {
			hasJSONSchemaStruct = true
		}
	}

I.e. this works:

	type Req struct {
		jsonschema.Struct
	}
	req := Req{}
	...
	oc.AddReqStructure(*req, reqOpts...)

This does't:

	req := jsonschema.Struct{}
	...
	oc.AddReqStructure(*req, reqOpts...)

It doesn't impact my use case, but the behavior is not very intuitive.
Thanks again.

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

Successfully merging a pull request may close this issue.

2 participants