Open starkovsky opened 2 years ago
It is expected behavior. Since params
is the variables of path
, it must be required.
@climba03003 Just encountered this.
I understand your point, but Fastify supports making last parameter optional, see here.
It would be nice for "create/update" endpoints.
I even mirrored this in my schema
@StepanMynarik
Are you willing to provide a PR for this?
@Uzlopak
Thank you for the lighting quick response!
Let me see what I can do. This will be my first.
@Uzlopak
So I tried it only to later realize I have no idea what I'm doing π
Not well-versed within tap, swagger/openapi nor this repository specifics. At first glance seemed like a simple change, well...
Also few unit tests break as soon as "required" is not always true and I don't know how to fix these since I don't fully understand all of them.
You can provide the code for guidance. No worry about not being familiar with the tools we are using.
For your endpoint, I recommend to split it using POST for creation and PUT or PATCH for update.
For the PR I would accept, only the last parameter is allowed to be optional. Otherwise, it seems like a design fault of API since it allow something like endpoint/////
to call.
The code was nothing to speak of. Few lines and it was definitely wrong. Wasn't able to even commit to my own fork due to weird errors (something about bash, probably related to the fact unit tests weren't passing?).
At least I can point to new relevant code locations:
For easy "isLast" detection I changed this:
.map((propKey) => {
const jsonSchema = toOpenapiProp(propKey, obj[propKey])
to this:
.map((propKey, index, array) => {
const isLast = index === array.length - 1
const jsonSchema = toOpenapiProp(propKey, obj[propKey], isLast)
As per the OAS 3.0 spec, itβs not allowed to have a path parameter to be optional.
Determines whether this parameter is mandatory. If the parameter location is "path", this property is REQUIRED and its value MUST be true. Otherwise, the property MAY be included and its default value is false.
So looks like this is correct behaviour.
This makes sense except for the last parameter at the end of the path.
Consider the following endpoints:
But yeah, can be fairly easily implemented by making three different endpoints in Fastify right now (GET, PUT and POST).
It's just convenient to use the same POST endpoint for both create and update with optional ID param in path.
But your example is wrong, as they are definetly three different endpoints?!
Sorry, tired after a long day π
The example can be implemented using two endpoints, GET for read by ID and POST for create/update based on whether ID parameter is provided or not. For this to work, you need the ID parameter to be optional.
It's just convenient to have create/update operations in a single POST endpoint when putting something together quick and you repeat that for multiple resources.
This is just one possible use of this, not very clean, I admit.
For me the main problem is that Fastify allows you to specify the last param as optional explicitly by appending questionmark like so ":param_name?". When this is a documented intentional feature of Fastify, I would expect this package to support that behavior as well.
I think that specific entry should be duplicated in the OAS to support the optional parameter.
can be fixed using this transform method in @fastify/swagger
's plugin config:
transformObject: ({ openapiObject }) => {
// fix for optional path parameters
for (const [url, path] of Object.entries(openapiObject.paths)) {
for (const method of Object.values(path)) {
if (!method.parameters) continue;
for (const param of method.parameters) {
if (param.in === 'path') param.required = !url.includes(`{${param.name}}?`);
}
}
}
return openapiObject;
},
looks good at least in Swagger UI
Prerequisites
Fastify version
3.x
Plugin version
5.1.1
Node.js version
16.14.0
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
Chrome: 100
Description
Optional parameters are specified as required
Steps to Reproduce
Start fastify server
Open swagger page
Expected Behavior
Optional parameters required if set