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

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

Open
leevuli opened this issue May 1, 2021 · 6 comments

Comments

@leevuli
Copy link

leevuli commented May 1, 2021

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:

@leevuli leevuli changed the title Property/Field Overwriting Doesn't Manifest In Swagger When Extending A Class [Bug] - Property/Field Overwriting Doesn't Manifest In Swagger When Extending A Class May 1, 2021
@kamilmysliwiec
Copy link
Member

Please provide a minimum reproduction repository.

@leevuli
Copy link
Author

leevuli commented May 4, 2021

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

@leevuli
Copy link
Author

leevuli commented Jun 4, 2021

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.

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@leevuli
Copy link
Author

leevuli commented Jul 31, 2021

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.

@leevuli
Copy link
Author

leevuli commented Aug 12, 2021

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

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