swagger icon indicating copy to clipboard operation
swagger copied to clipboard

[Bug] - Property/Field Overwriting Doesn't Manifest In Swagger When Extending A Class

Open koxanybak opened this issue 3 years ago • 6 comments

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When I extend a class and try overwrite an inherited property/field. The result of the overwriting doesn't manifest in the Swagger documentation. See code below how to reproduce.

current-behaviour

Expected behavior

The desired behaviour would be how the overwriting manifests in the TypeScript code: The property info of EntityPutDTO should be of type InfoPutDTO, not InfoPostDTO. (See code below)

expected-behaviour

Minimal reproduction of the problem with instructions

export class InfoPostDTO {
    @ApiProperty()
    name: string;
}
export class InfoPutDTO extends InfoPostDTO {
    @ApiProperty()
    id: number;
}

export class EntityPostDTO {
    @ApiProperty()
    id: number;

    @ApiProperty()
    info: InfoPostDTO;
}
export class EntityPutDTO extends EntityPostDTO {
    @ApiProperty()
    info: InfoPutDTO;
}

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

The motivation is that the classes manifest themselves the same way into the documentation that they do in the TypeScript code.

Environment


Nest version: 7.6.15
"@nestjs/common": "^7.6.15",
"@nestjs/core": "^7.6.15",
"@nestjs/platform-express": "^7.6.15",
"@nestjs/swagger": "^4.8.0",

 
For Tooling issues:
- Node version: v15.12.0  
- Platform:  Linux (Ubuntu 20.04)

Others:

koxanybak avatar May 01 '21 07:05 koxanybak

Please provide a minimum reproduction repository.

kamilmysliwiec avatar May 04 '21 09:05 kamilmysliwiec

Here's the minimun reproduction repository: https://github.com/koxanybak/nestjs.swagger.issues.1321

koxanybak avatar May 04 '21 17:05 koxanybak

Any updates on this or planned fixes for future versions?

Or wasn't the minimun reproduction repository not clear enough? If that's the case, tell me and I'll make it better.

koxanybak avatar Jun 04 '21 10:06 koxanybak

Would you like to create a PR for this issue?

kamilmysliwiec avatar Jul 08 '21 08:07 kamilmysliwiec

I worked on the bug and I came to the conclusion that the issue can be found in the file lib/services/schema-object-factory.ts, in the function mergePropertyWithMetadata where the metadata variable is set with the help of Reflect.getMetadata(DECORATORS.API_MODEL_PROPERTIES, prototype, key).

In the example issue above (and in the minimun reproduction repository), when schema objects are gathered for the EntityPutDTO and when looking for the type for the problematic info field, after setting the metadata variable, the variables in the function are as follows: image The metadata.type is the type of the info field in the EntityPostDTO from which the EntityPutDTO inherits. You can also see that the prototype is EntityPostDTO and not EntityPutDTO.

I couldn't figure out if the Reflect.getMetadata doesn't function correctly or if the prototype is not what it should be BUT, if the prototype is not what is should be, the problem can traced to the file lib/explorers/api-response.explorer.ts to the function exploreApiResponseMetadata where the responses array is defined with the help of Reflect.getMetadata(constants_2.DECORATORS.API_RESPONSE, method). The prototype is then get as response.type.prototype. So in any case, in my very unprofessional opinion, the issue stems from Reflect.getMetadata.

I don't really have energy to debug the issue further but if someone else wants to give it a try, this is a good place to start.

koxanybak avatar Jul 31 '21 17:07 koxanybak

I found a workaround/solution for the issue. The key is to not overwrite any fields. I solved this by creating an abstract base entity DTO with all the fields that will be not overwritten. Then all the other DTOs inherit that base entity. This way no fields are overwritten, thus avoiding the bug.

The bug is still there but with this workaround you can avoid it.

export class InfoPostDTO {
    @ApiProperty()
    name: string;
}
export class InfoPutDTO extends InfoPostDTO {
    @ApiProperty()
    id: number;
}


abstract class BaseEntityDTO {
    @ApiProperty()
    id: number;
}

export class EntityPostDTO extends BaseEntityDTO {
    @ApiProperty()
    info: InfoPostDTO;
}
export class EntityPutDTO extends BaseEntityDTO {
    @ApiProperty()
    info: InfoPutDTO;
}

koxanybak avatar Aug 12 '21 07:08 koxanybak