When writing a request handler returning a nested type you will face a type name generation issue within the OpenAPI specification.
Example:
public testGet(): ResponseWrapper<Array<T>> {}
The name generated will be:
But is expected to be:
Asterix112
All 13 comments
Same as the other issue you opened:
Before I try to reproduce this, please include
- the full controller file that uses this model
- the interface model definition
- the Swagger YAML that is produced
- the Swagger YAML that you wish was produced
The reason why I’m asking this is because I don’t see how that would compile without giving testGet a generic parameter. Additionally, it doesn’t make sense to me to have a generic route because the model it returns should be a definite type.
dgreene1
on 11 Aug 2019
@Asterix112 it's been 5 days and we still haven't received any steps to reproduce this. Please see above. Due to that we'll close this for housekeeping purposes. However, if you are able to submit the information above we can re-open the issue and try to solve it. Thank you.
dgreene1
on 16 Aug 2019
@dgreene1 Sorry for my late response. Because of project protection purposes i cannot send the whole controller to you. I could send you everything that is directly in touch with the problem we are facing (e.g. controller function including annotations and the controller class itself, also including annotations and of course the interfaces)
Controller:
@Route('')
@injectable()
export class DefaultController extends Controller {
@inject(LoggingServiceImpl)
private logging: LoggingServiceImpl;
@inject(CachingServiceImpl)
private caching: CachingServiceImpl;
constructor() {
super();
}
/***
* Retrieve the current substitution plan blob by moduleId
* @param request
* @param moduleId Module ID obtained
*/
@Get('subst/{moduleId}')
@Security('sia', ['sia'])
public async substGet(@Request() request: ExpressRequest, @Path() moduleId: string): Promise<GenericResponse<Array<Substitutionplan>>> {
this.logging.info(`User: ${JSON.stringify(request.user)}`);
this.logging.info("Retrieving substitution plan blob from caching service...");
const data = <SubstitutionplanMap><any>(await this.caching.getBlob(CacheBlobType.SUBSTITUTION_PLAN, request.user["aclid"], moduleId));
this.logging.debug(`Data:\n${JSON.stringify(data)}`);
const result = new Array<Substitutionplan>();
for (const e of Object.keys(data)) {
result.push(data[e]);
}
return {
data: result,
state: ResponseState.SUCCEEDED
}
}
}
Swagger:
{
"basePath": "/",
"consumes": [
"application/json"
],
"definitions": {
"ResponseState": {
"enum": [
"succeeded",
"failed"
],
"type": "string"
},
"SubstitutionplanEntryTime": {
"properties": {
"lesson": {
"type": "string"
},
"length": {
"type": "number",
"format": "double"
}
},
"required": [
"lesson",
"length"
],
"type": "object"
},
"SubstitutionplanEntry": {
"properties": {
"date": {
"type": "string"
},
"time": {
"$ref": "#/definitions/SubstitutionplanEntryTime"
},
"class": {
"type": "string"
},
"missingTeacher": {
"type": "string"
},
"subject": {
"type": "string"
},
"teacher": {
"type": "string"
},
"room": {
"type": "string"
},
"newSubject": {
"type": "string"
},
"comment": {
"type": "string"
},
"type": {
"type": "string"
},
"typeData": {
"type": "string"
}
},
"required": [
"date",
"time",
"class",
"missingTeacher",
"subject",
"teacher",
"room",
"newSubject",
"comment",
"type",
"typeData"
],
"type": "object"
},
"Substitutionplan": {
"properties": {
"date": {
"type": "string"
},
"entries": {
"type": "array",
"items": {
"$ref": "#/definitions/SubstitutionplanEntry"
}
},
"messages": {
"type": "array",
"items": {
"type": "string"
}
}
},
"required": [
"date",
"entries",
"messages"
],
"type": "object"
},
"GenericResponseArray": {
"properties": {
"state": {
"$ref": "#/definitions/ResponseState"
},
"data": {
"type": "array",
"items": {
"$ref": "#/definitions/Substitutionplan"
}
},
"error": {
"type": "string",
"x-nullable": true
}
},
"required": [
"state",
"data"
],
"type": "object"
}
},
"info": {
"title": "<..>",
"version": "<..>",
"description": "<..>",
"license": {
"name": "<..>"
}
},
"paths": {
"/subst/{moduleId}": {
"get": {
"operationId": "SubstListGet",
"produces": [
"application/json"
],
"responses": {
"200": {
"description": "Ok",
"schema": {
"$ref": "#/definitions/GenericResponseArray"
}
}
},
"description": "*\nRetrieve the current substitution plan blob by moduleId",
"security": [
{
"sia": [
"sia"
]
}
],
"parameters": [
{
"description": "Module ID obtained",
"in": "path",
"name": "moduleId",
"required": true,
"type": "string"
}
]
}
},
"/surveys": {
"get": {
"operationId": "SurveysGet",
"produces": [
"application/json"
],
"responses": {
"200": {
"description": "Ok",
"schema": {
"$ref": "#/definitions/GenericResponseArray"
}
}
},
"description": "*\nFor polling polls from the API",
"security": [
{
"sia": [
"sia"
]
}
],
"parameters": [
{
"description": "First document to fetch",
"in": "query",
"name": "offset",
"required": true,
"format": "double",
"type": "number"
},
{
"description": "Amount of documents to fetch",
"in": "query",
"name": "count",
"required": true,
"format": "double",
"type": "number"
}
]
}
}
},
"produces": [
"application/json"
],
"swagger": "2.0",
"securityDefinitions": {
"sia": {
"type": "oauth2",
"clientId": "",
"clientSecret": "",
"tokenUrl": "<..>",
"flow": "password",
"scopes": {
"sia": "For accessing the API."
}
}
},
"host": "<..>"
}
If you take a look at the naming of the response models, you will see the issue implicitly produced by the invalid naming: Models of other endpoints are attached to any endpoint using "GenericResponseArray" as the model.
Are you fine with this and then able to figure out what the issue might be?
Asterix112
on 18 Aug 2019
Please reopen ASAP
Asterix112
on 18 Aug 2019
I understand that you want a better name to be produced for GenericResponseArray, but the name that us produced is valid.
I will reopen the ticket but since this is a potentially breaking change it will have to come out after the more pressing big fixes that are currently in PRs.
So if you’d like to see these two issues you've created get fixed, please submit a PR for them yourself. All tsoa support is done by unpaid people, so this will not get fixed ”ASAP” unless you submit a PR.
dgreene1
on 18 Aug 2019
Actually it is not only about the naming and causes quite some real problems for us:
Example given:
We have these two functions:
substGet(): GenericResponse<Array<Substitutionplan>> {}
"GenericResponseArray": {
"properties": {
"state": {
"$ref": "#/definitions/ResponseState"
},
"data": {
"type": "array",
"items": {
"$ref": "#/definitions/Substitutionplan"
}
},
"error": {
"type": "string",
"x-nullable": true
}
},
"required": [
"state",
"data"
],
"type": "object"
}
As you can see "GenericResponseArray has data => items.
And those items point to #/definitions/Substitutionplan
But in our second case we need it to be:
/definitions/Examplan
So we get a lot of type errors because of this.
LasseRosenow
on 18 Aug 2019
This is an easy fix since it is only about resolving the name clash, I opened #427 to address it.
I fundamentally disagree with @dgreene1 on what a bugfix is vs. a breaking change (in terms of versioning) here, so you might have to build it from here until it is merged.
WoH
on 19 Aug 2019
Thanks A LOT! :):)
LasseRosenow
on 19 Aug 2019
@WoH can you clarify what you mean by this?
I fundamentally disagree with @dgreene1 on what a bugfix is vs. a breaking change (in terms of versioning) here, so you might have to build it from here until it is merged.
Also, what version number do you think this should get?
dgreene1
on 19 Aug 2019
And to clarify, I only though that this was ”potentially a breaking fix” when I thought that it was the difference between [] and Array (I confused the other PR @Asterix112 made). But now that @lazylazyllama has clarified the acceptance criteria, it’s clear that this is a necessary change.
So I will review your PR @WoH as soon as I can. I’d prefer that you don’t recommend to others that they use a fork of tsoa. I’m committed to regularly publishing new versions to NPM.
And our community would probably benefit from a conversation about what the “rules of the game are” for breaking changes within tsoa. I actually happen to think that the names of the models are probably not a breaking change, but I wouldn’t be surprised if there is a tsoa user out there that does. So if we were to publish our rules of the game for versioning out in the readme then that might help consumers to accept the level of risk they find appropriate.
dgreene1
on 19 Aug 2019
Also, what version number do you think this should get?
Whatever the next patch is at the time of the merge. Since this is an issue that only affects users that were using nested type parameters (which were broken).
And our community would probably benefit from a conversation about what the “rules of the game are” for breaking changes within tsoa.
So if we were to publish our rules of the game for versioning out in the readme then that might help consumers to accept the level of risk they find appropriate.
I agree.
I’d prefer that you don’t recommend to others that they use a fork of tsoa. I’m committed to regularly publishing new versions to NPM.
I take note of your objection. It seemed to me like you were not comfortable with merging it in so I offered a possible way to compromise between both parties for a limited amount of time.
WoH
on 19 Aug 2019
@Asterix112 / @lazylazyllama the fix for this was released in 2.4.10. Thank you for informing us of the issue. Please let us know if it's resolved. You can also see what else was released in our release notes: https://github.com/lukeautry/tsoa/releases
Lastly, if you're enjoying using tsoa, then please consider sharing your enjoyment on Twitter, Reddit, or dev.to. :)
dgreene1
on 19 Aug 2019
@dgreene1 @WoH Thank you for your fast work and the well done fix! Everything works as expected now.
Asterix112
on 20 Aug 2019
Related issues
picturelink
·
5Comments
fahrradflucht
·
4Comments
alxwrd
·
5Comments
amcdnl
·
6Comments
Izabrr
·
4Comments