Nested object properties use a boolean `required` instead of an array

[jsw]

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

[ArielPrevu3D]

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.

[kamilmysliwiec]

Would you like to create a PR for this issue?

[jsw]

@ArielPrevu3D Could your workaround code be used to start a PR?

[ArielPrevu3D]

@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 ????');
  }
}

[codybreene]

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?

[Mydayyy]

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

[codybreene]

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?

[ArielPrevu3D]

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!

[codybreene]

Thanks for the code pointer @ArielPrevu3D ! I will take a look

[codybreene]

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 }

[ArielPrevu3D]

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/

[codybreene]

@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);
      }
    }
  }
}

[ThomasMoritz]

@ArielPrevu3D Right now we are also experiencing this issue. Can you tell me where you added the above code? Thanks in advance.

[ArielPrevu3D]

Using @codybreene 's updated code, you would call fixSchemasInDoc() with your OpenAPIObject document as a parameter. That document usually comes from SwaggerModule.createDocument().

[ThomasMoritz]

@ArielPrevu3D Thank you so much! Worked like a charm!!!

[mrskiro]

any new update?

[TheSalarKhan]

This is amazing. Thank you @ArielPrevu3D and @codybreene. As a solution to this @kamilmysliwiec would you approve if we use the version edit by @codybreene 's and call it just before returning from SwaggerModule.createDocument?

Or do you think this should be done some other way? Or do you think this doesn't need an option and must always be done this way?

Asking because I would be more than happy to raise a PR for this issue.

Looking forward to your response.

[rfreydi]

Another workaround is to avoid nested properties and create dedicated classes instead:

This example throw attribute components.schemas.Entity.required is not of type 'array':

export class Entity {
  nested?: {
    value: string;
  }
}

This one work perfectly:

class EntityNested {
  value!: string;
}

export class Entity {
  nested?: EntityNested;
}

[kamilmysliwiec]

https://github.com/nestjs/swagger/commit/92df1991c799f39503c6506d77b1145ee0fe804b