-
Notifications
You must be signed in to change notification settings - Fork 83
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
Test for requestBody
in Operations which don't support a request body
#616
Conversation
Can one of the admins verify this patch? |
vr.body("paths.'/zepplins/{id}'.head.requestBody.$ref", equalTo("#/paths~1zepplins~1{id}/delete/requestBody")); | ||
vr.body("paths.'/zepplins/{id}'.get.requestBody.$ref", equalTo("#/paths~1zepplins~1{id}/delete/requestBody")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vr.body("paths.'/zepplins/{id}'.head.requestBody.$ref", equalTo("#/paths~1zepplins~1{id}/delete/requestBody")); | |
vr.body("paths.'/zepplins/{id}'.get.requestBody.$ref", equalTo("#/paths~1zepplins~1{id}/delete/requestBody")); | |
vr.body("paths.'/zepplins/{id}'.head.requestBody.$ref", equalTo("#/paths/~1zepplins~1{id}/delete/requestBody")); | |
vr.body("paths.'/zepplins/{id}'.get.requestBody.$ref", equalTo("#/paths/~1zepplins~1{id}/delete/requestBody")); |
c1f60da
to
228aa0a
Compare
@eclipse-microprofile-bot please test this |
050c84d
to
cbf27af
Compare
With the prereqs in I have cleaned this up. All ready to go. |
public Response headZepplin(@RequestBody(ref = "#/paths/~1zepplins~1{id}/delete/requestBody") String string) { | ||
return Response.ok().build(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test looks good, but I'm slightly concerned that Jakarta REST may not be required to support having an entity parameter (implying it can receive a request body) on a HEAD or GET request. Will take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see anything in the Jakarta REST spec that says either way whether this is permitted. You're welcome to also have a look, but unless we're sure that this is required to be supported, I think we should test this in StaticDocumentTest
or ModelReaderAppTest
.
We only want to test that their MP OpenAPI implementation can document this kind of method, not that their Jakarta REST implementation allows it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one thing needs fixed.
public Response headZepplin(String string) { | ||
return Response.ok().build(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this file don't affect the tests any more, I guess you left them in because you've used them in other PRs that build on this one?
That's ok, but the @GET
and @HEAD
methods still have the string
entity parameter and this should be removed.
…body to model reader test
c59cb99
to
b7f400a
Compare
requestBody
in Operations which don't support a request body
This resolves #591
This is built on top of #615 and will be marked as a draft until that is delivered.
There is no changes to SmallRye required to pass this test. I did not find any documentation mentioning the removed limitation. And ModelConstructionTest already covers RequestBody