Skip to content

fix: api description contain invalid request type for GET requests#1507

Merged
jeremydmiller merged 1 commit intoJasperFx:mainfrom
uniquelau:main
Jun 27, 2025
Merged

fix: api description contain invalid request type for GET requests#1507
jeremydmiller merged 1 commit intoJasperFx:mainfrom
uniquelau:main

Conversation

@uniquelau
Copy link
Contributor

@uniquelau uniquelau commented Jun 19, 2025

Tiny adjustment, the ensures that when generating the open api documentation, GET endpoints do not have request bodies and adds a couple of tests.

Currently, if the GET endpoint uses [FromQuery] ComplexT the type will shows up as the request body and is required.
This is incorrect, as these values are expected on the query string, not the request body.

This means that the swagger tooling can not be used to submit a request to the API and the generated JSON does not match the api specification.

In an ideal standard compliant world, the GET would support a request body (even though it's weird), but from looking at how RequestType works within the system, it would be a larger change, and from looking at some of the code, I'm not sure Wolverine supports this anyway.

  • Added test to replicate issue
  • Added fix to ensure generated API valid
  • Ran tests

- in practice this means, [FromQuery] T ends up as the request body
- as everything is a RequestType, it's not straight forward to exclude concrete query items
- this is likely to be a problem with a POST request that combines a body and a concrete query
- for now updating to check if the documentation is for a GET
@uniquelau uniquelau marked this pull request as ready for review June 27, 2025 08:57
@jeremydmiller jeremydmiller merged commit ec50bf7 into JasperFx:main Jun 27, 2025
1 check failed
@uniquelau
Copy link
Contributor Author

Just got a notification that the build has failed, I did check on my local using the docker setup, so apologies if I have missed something. I'll re-run locally and see what is happening.

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 this pull request may close these issues.

2 participants