swagger
swagger copied to clipboard
nestjs
Nested object properties use a boolean `required` instead of an array
Is there an existing issue for this?
- [X] I have searched the existing issues
Current behavior
Nested object properties are using required: true instead of an array. I am able to generate swagger that look like this:
"OrganizationDto": {
"type": "object",
"properties": {
"id": {
"type": "string"
},
"name": {
"type": "string"
},
"_count": {
"type": "object",
"properties": {
"users": {
"required": true,
"type": "number"
}
}
}
},
"required": [
"id",
"name",
"_count"
]
},
This is rejected by openapi-generate
-attribute components.schemas.OrganizationDto.required is not of type `array`
Minimum reproduction code
https://gist.github.com/jsw/2f0cd1e35bf3f09b22bd02d784165792
Steps to reproduce
No response
Expected behavior
required should be an array instead of a boolean
Package version
6.1.2
NestJS version
9.1.2
Node.js version
16.17.0
In which operating systems have you tested?
- [X] macOS
- [ ] Windows
- [ ] Linux
Other
No response
We are also experiencing this issue, which is preventing us to integrate with many OpenAPI-based tooling. Current workaround is to fix the document's schemas recursively with our own code.
@ArielPrevu3D Could your workaround code be used to start a PR?
No, I just fix the result after the compiler plugin and the swagger module generates the OpenAPI document. Here's the ugly code:
function fixSchema(
schema: SchemaObject,
key: string,
depth: number = 0,
): void {
if (schema.type === 'object' && schema.properties) {
const additionalRequired = [];
for (const [pk, pv] of Object.entries(schema.properties)) {
if ('type' in pv) {
if (typeof pv.required === 'boolean') {
console.warn(
`${' '.repeat(depth)}Found required as boolean in ${key}`,
);
if (pv.required) additionalRequired.push(pk);
delete pv.required;
}
fixSchema(pv, pk, depth + 1);
}
}
if (additionalRequired.length && typeof schema.required !== 'object') {
schema.required = additionalRequired;
console.warn(
`Fixed required for schema ${key}: [${schema.required.join(',')}]`,
);
}
}
}
function fixSchemasInDoc(doc: typeof document): void {
const schemas = doc.components?.schemas;
if (schemas) {
for (const [sk, sv] of Object.entries(schemas)) {
if ('type' in sv) {
fixSchema(sv, sk);
}
}
} else {
console.warn('No schemas ????');
}
}
This is happening for us too -- any idea when this bug was introduced so we can revert to the last package that did not have it?
Running into the same issue, the generated yaml/json does not parse in any openapi compatible tools due to this
Edit: This also leads to the generated documentation being wrong and not having the objects marked as required
I'm happy to jump in a submit a PR for this I just need some guidance on where in the codebase I should start looking -- otherwise can we get this triaged?
I'm happy to jump in a submit a PR for this I just need some guidance on where in the codebase I should start looking -- otherwise can we get this triaged?
It's going to be related to https://github.com/nestjs/swagger/blob/master/lib/plugin/visitors/model-class.visitor.ts#L209 which is where the boolean is added. Also, this is where the OpenAPI decorator is built, so the "required" parameter could be added here https://github.com/nestjs/swagger/blob/23a56456ef627a50179af3fb4be5786c1af5165f/lib/plugin/visitors/model-class.visitor.ts#L194 where the object returned by _OPENAPI_METADATA_FACTORY is created.
If you work on this, you'll realize that the Typescript compiler API is poorly documented, and the code for this plugin is hard to follow. If you have more questions let me know.
Good luck!
I may be oversimplifying but can't we just delete:
!hasPropertyKey('required', existingProperties) &&
factory.createPropertyAssignment(
'required',
createBooleanLiteral(factory, isRequired)
),
I don't think we ever want a case where { required: Boolean }
We would want required: boolean in parameters and requestBody, but not in schemas. If this code only affects OpenAPI schemas, then the answer is yes.
https://swagger.io/docs/specification/basic-structure/
@ArielPrevu3D we tweaked your helper fns slightly to account for nested $refs:
function fixSchema(schema: SchemaObject, key: string, depth = 0): void {
if (schema.type === 'object' && schema.properties) {
const additionalRequired = [];
for (const [pk, pv] of Object.entries(schema.properties)) {
if ('type' in pv) {
if (typeof pv.required === 'boolean') {
if (pv.required) additionalRequired.push(pk);
delete pv.required;
}
fixSchema(pv, pk, depth + 1);
} else if ('$ref' in pv) {
delete pv['required'];
}
}
if (additionalRequired.length && typeof schema.required !== 'object') {
schema.required = additionalRequired;
}
}
}
function fixSchemasInDoc(doc: OpenAPIObject): void {
const schemas = doc.components?.schemas;
if (schemas) {
for (const [sk, sv] of Object.entries(schemas)) {
if ('type' in sv) {
fixSchema(sv, sk);
}
}
}
}
@ArielPrevu3D Right now we are also experiencing this issue. Can you tell me where you added the above code? Thanks in advance.
Using @codybreene 's updated code, you would call fixSchemasInDoc() with your OpenAPIObject document as a parameter. That document usually comes from SwaggerModule.createDocument().
