Skip to content
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

Named swagger schema #2070

Open
1 task done
andreafspeziale opened this issue Nov 4, 2022 · 4 comments
Open
1 task done

Named swagger schema #2070

andreafspeziale opened this issue Nov 4, 2022 · 4 comments

Comments

@andreafspeziale
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

Wouldn't be nice to have named schema in order to have a coherent look and feel like the others one?

Right now I see the following

Schermata 2022-11-05 alle 00 17 51

Vs a typical schema

Schermata 2022-11-05 alle 00 18 16

Maybe you wont give it a name in order to avoid any kind of application schema name collision (?) Just wondering...

Thanks in advance!

Describe the solution you'd like

Add name property here based on the status? I don't know if it will do the trick.

Teachability, documentation, adoption, migration strategy

I believe it would be quite "impactless".

What is the motivation / use case for changing the behavior?

Coherent look and feel of the generated swagger doc.

@BrunnerLivio
Copy link
Member

@andreafspeziale Sounds good to me! Would you like to create a PR? :)

@andreafspeziale
Copy link
Author

@BrunnerLivio npm i/build is broken on my side using node v18.5.0.

Schermata 2022-11-05 alle 22 01 00

No problems by using node v16.x.x. Wouldn't be useful to specify engines in the package.json?

I've also start testing what I was proposing above but unfortunately it is not 100% what I would like to achieve.

Adding title will add the name to the schema

Schermata 2022-11-06 alle 00 04 28

but it will not possible to smoothly have it listed at the bottom

Schermata 2022-11-06 alle 00 05 50

I'm not sure how to achieve it but my feeling is that the only way would be to have the schemas as classes and import them into the main.ts of the consumer application as extra schemas.

const swaggerDocument = SwaggerModule.createDocument(app, swaggerConfig, {
  extraModels: [Exception],
});

Any suggestion to this brainstorming?

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Nov 7, 2022

@andreafspeziale

No problems by using node v16.x.x. Wouldn't be useful to specify engines in the package.json?

Yeah that'd be nice! Can you specify the version that is defined within the .nvmrc file?

I'm not sure how to achieve it but my feeling is that the only way would be to have the schemas as classes and import them into the main.ts of the consumer application as extra schemas.

I haven't looked too deeply into it, but it may be related to this issue.
Have you tried calling ApiExtraModels within health-check.decorator.ts (similar how ApiServiceUnavailableResponse is called)

@andreafspeziale
Copy link
Author

@BrunnerLivio Oh! I didn't notice the nvm file. Using that was smooth, so far so good!

I checked the issue u linked above and following your suggestion I did some tests but unfortunately I never saw the schemas rendering at the bottom of the swagger page.

Maybe I'm just misunderstanding your suggestion.

Could you try to be more specific? I will be happy to try it out on my side!

By the way I developed a similar use-case, as you can see from the previous comment screenshot I've an Exception schema in the schema list at the bottom of the swagger page.

I achieved it by exporting a class and a decorator from one of my monorepo packages named common.

As mentioned the package exports the class and a decorator to be used in the controllers:

export class Exception extends HttpException {
  constructor(response: ExceptionResponse, status: number) {
    super(response, status);
  }

  @ApiProperty()
  code!: string;

  @ApiProperty()
  message!: string;

  @ApiPropertyOptional({
    oneOf: [
      {
        type: 'array',
        items: {
          type: 'string',
        },
      },
      {
        type: 'object',
        additionalProperties: { type: 'unknown' },
      },
    ],
  })
  details?: string[] | Record<string, unknown>;
}
export const ApiExceptionResponse = ({
  status,
  description,
  example,
  examples,
}: ApiExceptionResponseOptions) =>
  applyDecorators(
    ApiResponse({
      status,
      description,
      content: {
        'application/json': {
          schema: { $ref: getSchemaPath(Exception) },
          example,
          examples,
        },
      },
    }),
  );
export class CatsController {
  constructor(private readonly catsService: CatsService) {}

  @Get(':id')
  @ApiOperation({
    summary: 'Retrieves a cat by id',
    description: 'A Cat is a sample entity. If not found throws a NotFoundException.',
  })
  @ApiExceptionResponse({
    status: 400,
    example: httpExceptionExamples.ValidationException.value,
  })
  @ApiExceptionResponse({
    status: 404,
    example: httpExceptionExamples.NotFoundException.value,
  })
  async getConfiguration(@Param() { id }: CatParam): Promise<Cat> {
    const { id: _, ...res } = await this.catsService.getCat(id);
    return res;
  }
}
...
import { Exception } from 'common';
...
const swaggerConfig = new DocumentBuilder()
  .setTitle(name)
  .setDescription(`${name} RESTful API service.`)
  .setVersion(version)
  .build();

const swaggerDocument = SwaggerModule.createDocument(app, swaggerConfig, {
  extraModels: [Exception],
});

SwaggerModule.setup('swagger', app, swaggerDocument);

Let me know your thoughts!

P.s: I noticed that the package contains some files that I would not include

Schermata 2022-11-09 alle 22 14 39

Do you want me to open an issue and update the [.npmignore](https://github.com/nestjs/terminus/blob/master/.npmignore) ignoring:

  • .nvmrc
  • .husky
  • docker-compose.yml

Cheers and sorry for the late response I've been working on my first OS nestjs module!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants