Skip to content

Commit 45f26fe

Browse files
denyomacjohnny
denyo
authored and
macjohnny
committed
[typescript-rxjs] performance improvements, bugfix for falsy parameters (#4250)
* perf(typescript-rxjs): remove redundant check when building auth header * feat(typescript-rxjs): destructure request parameters, add throwIfNullOrUndefined helper, build query object more efficently * fix(typescript-rxjs): change form checks back from null to undefined * feat(typescript-rxjs): regenerate samples * feat(typescript-rxjs): add hasRequiredQueryParams flag for improved query object generation * feat(typescript-rxjs): remove trailing comma in param destructuring, improve formatting via hasOptionalQueryParams flag * feat(typescript-rxjs): remove useless generics in BaseAPI * feat(typescript-rxjs): regenerate samples * feat(typescript-rxjs): extend CodegenParameter by output.paramNameAlternative and output.paramNameOrAlternative * refactor(typescript-rxjs): remove obsolete reservedWords RequiredError and exists * feat(typescript-rxjs): add reservedParamNames list with headers, query and formData, extend param processing * feat(typescript-rxjs): use paramNameOrAlternative in api template * refactor(typescript-rxjs): replace paramNameOrAlternative prop with mustache partial * refactor(typescript-rxjs): reduce branching in configuration's apiKey() and accessToken() * refactor(typescript-rxjs): remove unused ModelPropertyNaming * feat(typescript-rxjs): regenerate samples * feat(typescript-rxjs): remove CodegenParamter's paramNameAlternative, use vendorExtensions instead * docs(typescript-rxjs): regenerate readme
1 parent 4f350bc commit 45f26fe

21 files changed

Lines changed: 661 additions & 587 deletions

File tree

‎docs/generators/typescript-rxjs.md‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,8 @@ sidebar_label: typescript-rxjs
6767
<li>HttpMethod</li>
6868
<li>HttpQuery</li>
6969
<li>Middleware</li>
70-
<li>ModelPropertyNaming</li>
7170
<li>RequestArgs</li>
7271
<li>RequestOpts</li>
73-
<li>RequiredError</li>
7472
<li>ResponseArgs</li>
7573
<li>abstract</li>
7674
<li>await</li>
@@ -90,7 +88,6 @@ sidebar_label: typescript-rxjs
9088
<li>double</li>
9189
<li>else</li>
9290
<li>enum</li>
93-
<li>exists</li>
9491
<li>export</li>
9592
<li>extends</li>
9693
<li>false</li>

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptRxjsClientCodegen.java‎

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,20 @@
2323
import org.openapitools.codegen.*;
2424
import org.openapitools.codegen.meta.features.DocumentationFeature;
2525
import org.openapitools.codegen.utils.ModelUtils;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
2628

2729
import java.io.File;
28-
import java.util.TreeSet;
29-
import java.util.List;
30-
import java.util.ArrayList;
31-
import java.util.Map;
30+
import java.util.*;
3231

3332
public class TypeScriptRxjsClientCodegen extends AbstractTypeScriptClientCodegen {
33+
private static final Logger LOGGER = LoggerFactory.getLogger(AbstractTypeScriptClientCodegen.class);
3434

3535
public static final String NPM_REPOSITORY = "npmRepository";
3636
public static final String WITH_INTERFACES = "withInterfaces";
3737

3838
protected String npmRepository = null;
39+
protected Set<String> reservedParamNames = new HashSet<>();
3940

4041
public TypeScriptRxjsClientCodegen() {
4142
super();
@@ -58,6 +59,11 @@ public TypeScriptRxjsClientCodegen() {
5859

5960
this.cliOptions.add(new CliOption(NPM_REPOSITORY, "Use this property to set an url your private npmRepo in the package.json"));
6061
this.cliOptions.add(new CliOption(WITH_INTERFACES, "Setting this property to true will generate interfaces next to the default class implementations.", SchemaTypeUtil.BOOLEAN_TYPE).defaultValue(Boolean.FALSE.toString()));
62+
63+
// these are used in the api template for more efficient destructuring
64+
this.reservedParamNames.add("headers");
65+
this.reservedParamNames.add("query");
66+
this.reservedParamNames.add("formData");
6167
}
6268

6369
@Override
@@ -252,38 +258,65 @@ private void updateOperationParameterEnumInformation(Map<String, Object> operati
252258
operations.put("hasEnums", hasEnums);
253259
}
254260

261+
private void setParamNameAlternative(CodegenParameter param, String paramName, String paramNameAlternative) {
262+
if (param.paramName.equals(paramName)) {
263+
param.vendorExtensions.put("paramNameAlternative", paramNameAlternative);
264+
}
265+
}
266+
255267
private void addConditionalImportInformation(Map<String, Object> operations) {
256268
// This method will determine if there are required parameters and if there are list containers
257269
Map<String, Object> _operations = (Map<String, Object>) operations.get("operations");
258270
List<ExtendedCodegenOperation> operationList = (List<ExtendedCodegenOperation>) _operations.get("operation");
259271

260-
boolean hasRequiredParameters = false;
272+
boolean hasRequiredParams = false;
261273
boolean hasListContainers = false;
262274
boolean hasHttpHeaders = false;
263275
boolean hasQueryParams = false;
264276
boolean hasPathParams = false;
265277

266278
for (ExtendedCodegenOperation op : operationList) {
267279
if (op.getHasRequiredParams()) {
268-
hasRequiredParameters = true;
280+
hasRequiredParams = true;
269281
}
270-
271-
for (CodegenParameter param : op.headerParams) {
272-
if (param.isListContainer) {
273-
hasListContainers = true;
274-
break;
282+
283+
for (CodegenParameter p: op.allParams) {
284+
String paramNameAlternative = null;
285+
286+
if(this.reservedParamNames.contains(p.paramName)){
287+
paramNameAlternative = p.paramName + "Alias";
288+
LOGGER.info("param: "+p.paramName+" isReserved ––> "+paramNameAlternative);
275289
}
276-
}
277-
for (CodegenParameter param : op.queryParams) {
278-
if (param.isListContainer && !param.isCollectionFormatMulti) {
279-
hasListContainers = true;
280-
break;
290+
setParamNameAlternative(p, p.paramName, paramNameAlternative);
291+
292+
for (CodegenParameter param : op.headerParams) {
293+
if (param.isListContainer) {
294+
hasListContainers = true;
295+
}
296+
setParamNameAlternative(param, p.paramName, paramNameAlternative);
281297
}
282-
}
283-
for (CodegenParameter param : op.formParams) {
284-
if (param.isListContainer && !param.isCollectionFormatMulti) {
285-
hasListContainers = true;
286-
break;
298+
299+
for (CodegenParameter param : op.queryParams) {
300+
if (param.isListContainer && !param.isCollectionFormatMulti) {
301+
hasListContainers = true;
302+
}
303+
if (param.required) {
304+
op.hasRequiredQueryParams = true;
305+
} else {
306+
op.hasOptionalQueryParams = true;
307+
}
308+
setParamNameAlternative(param, p.paramName, paramNameAlternative);
309+
}
310+
311+
for (CodegenParameter param : op.formParams) {
312+
if (param.isListContainer && !param.isCollectionFormatMulti) {
313+
hasListContainers = true;
314+
}
315+
setParamNameAlternative(param, p.paramName, paramNameAlternative);
316+
}
317+
318+
for (CodegenParameter param : op.pathParams) {
319+
setParamNameAlternative(param, p.paramName, paramNameAlternative);
287320
}
288321
}
289322

@@ -296,13 +329,9 @@ private void addConditionalImportInformation(Map<String, Object> operations) {
296329
if (op.getHasPathParams()) {
297330
hasPathParams = true;
298331
}
299-
300-
if(hasRequiredParameters && hasListContainers && hasHttpHeaders && hasQueryParams && hasPathParams){
301-
break;
302-
}
303332
}
304333

305-
operations.put("hasRequiredParameters", hasRequiredParameters);
334+
operations.put("hasRequiredParams", hasRequiredParams);
306335
operations.put("hasListContainers", hasListContainers);
307336
operations.put("hasHttpHeaders", hasHttpHeaders);
308337
operations.put("hasQueryParams", hasQueryParams);
@@ -312,26 +341,25 @@ private void addConditionalImportInformation(Map<String, Object> operations) {
312341
private void addExtraReservedWords() {
313342
this.reservedWords.add("BASE_PATH");
314343
this.reservedWords.add("BaseAPI");
315-
this.reservedWords.add("RequiredError");
316344
this.reservedWords.add("COLLECTION_FORMATS");
317345
this.reservedWords.add("ConfigurationParameters");
318346
this.reservedWords.add("Configuration");
319347
this.reservedWords.add("HttpMethod");
320348
this.reservedWords.add("HttpHeaders");
321349
this.reservedWords.add("HttpQuery");
322350
this.reservedWords.add("HttpBody");
323-
this.reservedWords.add("ModelPropertyNaming");
324351
this.reservedWords.add("RequestArgs");
325352
this.reservedWords.add("RequestOpts");
326353
this.reservedWords.add("ResponseArgs");
327-
this.reservedWords.add("exists");
328354
this.reservedWords.add("Middleware");
329355
this.reservedWords.add("AjaxRequest");
330356
this.reservedWords.add("AjaxResponse");
331357
}
332358

333359
class ExtendedCodegenOperation extends CodegenOperation {
334360
public boolean hasHttpHeaders;
361+
public boolean hasRequiredQueryParams;
362+
public boolean hasOptionalQueryParams;
335363

336364
public ExtendedCodegenOperation(CodegenOperation o) {
337365
super();
@@ -405,6 +433,8 @@ public ExtendedCodegenOperation(CodegenOperation o) {
405433

406434
// new fields
407435
this.hasHttpHeaders = o.getHasHeaderParams() || o.getHasBodyParam() || o.hasAuthMethods;
436+
this.hasRequiredQueryParams = false; // will be updated within addConditionalImportInformation
437+
this.hasOptionalQueryParams = false; // will be updated within addConditionalImportInformation
408438
}
409439
}
410440
}

‎modules/openapi-generator/src/main/resources/typescript-rxjs/apis.mustache‎

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// tslint:disable
22
{{>licenseInfo}}
33
import { Observable } from 'rxjs';
4-
import { BaseAPI{{#hasHttpHeaders}}, HttpHeaders{{/hasHttpHeaders}}{{#hasQueryParams}}, HttpQuery{{/hasQueryParams}}{{#hasRequiredParameters}}, throwIfRequired{{/hasRequiredParameters}}{{#hasPathParams}}, encodeURI{{/hasPathParams}}{{#hasListContainers}}, COLLECTION_FORMATS{{/hasListContainers}} } from '../runtime';
4+
import { BaseAPI{{#hasHttpHeaders}}, HttpHeaders{{/hasHttpHeaders}}{{#hasQueryParams}}, HttpQuery{{/hasQueryParams}}{{#hasRequiredParams}}, throwIfNullOrUndefined{{/hasRequiredParams}}{{#hasPathParams}}, encodeURI{{/hasPathParams}}{{#hasListContainers}}, COLLECTION_FORMATS{{/hasListContainers}} } from '../runtime';
55
{{#imports.0}}
66
import {
77
{{#imports}}
@@ -37,11 +37,11 @@ export class {{classname}} extends BaseAPI {
3737
* {{&summary}}
3838
{{/summary}}
3939
*/
40-
{{nickname}} = ({{#allParams.0}}requestParameters: {{operationIdCamelCase}}Request{{/allParams.0}}): Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> => {
40+
{{nickname}} = ({{#allParams.0}}{ {{#allParams}}{{paramName}}{{#vendorExtensions.paramNameAlternative}}: {{vendorExtensions.paramNameAlternative}}{{/vendorExtensions.paramNameAlternative}}{{^-last}}, {{/-last}}{{/allParams}} }: {{operationIdCamelCase}}Request{{/allParams.0}}): Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}> => {
4141
{{#hasParams}}
4242
{{#allParams}}
4343
{{#required}}
44-
throwIfRequired(requestParameters, '{{paramName}}', '{{nickname}}');
44+
throwIfNullOrUndefined({{> paramNamePartial}}, '{{nickname}}');
4545
{{/required}}
4646
{{/allParams}}
4747

@@ -58,15 +58,15 @@ export class {{classname}} extends BaseAPI {
5858
{{/bodyParam}}
5959
{{#headerParams}}
6060
{{#isListContainer}}
61-
...(requestParameters.{{paramName}} && { '{{baseName}}': requestParameters.{{paramName}}.join(COLLECTION_FORMATS['{{collectionFormat}}'])) }),
61+
...({{> paramNamePartial}} != null ? { '{{baseName}}': {{> paramNamePartial}}.join(COLLECTION_FORMATS['{{collectionFormat}}'])) } : undefined),
6262
{{/isListContainer}}
6363
{{^isListContainer}}
64-
...(requestParameters.{{paramName}} && { '{{baseName}}': String(requestParameters.{{paramName}}) }),
64+
...({{> paramNamePartial}} != null ? { '{{baseName}}': String({{> paramNamePartial}}) } : undefined),
6565
{{/isListContainer}}
6666
{{/headerParams}}
6767
{{#authMethods}}
6868
{{#isBasic}}
69-
...(this.configuration.username && this.configuration.password && { Authorization: `Basic ${btoa(this.configuration.username + ':' + this.configuration.password)}` }),
69+
...(this.configuration.username != null && this.configuration.password != null ? { Authorization: `Basic ${btoa(this.configuration.username + ':' + this.configuration.password)}` } : undefined),
7070
{{/isBasic}}
7171
{{#isApiKey}}
7272
{{#isKeyInHeader}}
@@ -75,77 +75,109 @@ export class {{classname}} extends BaseAPI {
7575
{{/isApiKey}}
7676
{{#isOAuth}}
7777
// oauth required
78-
...(this.configuration.accessToken && {
79-
Authorization: this.configuration.accessToken && (typeof this.configuration.accessToken === 'function'
78+
...(this.configuration.accessToken != null
79+
? { Authorization: typeof this.configuration.accessToken === 'function'
8080
? this.configuration.accessToken('{{name}}', [{{#scopes}}'{{{scope}}}'{{^-last}}, {{/-last}}{{/scopes}}])
81-
: this.configuration.accessToken)
82-
}),
81+
: this.configuration.accessToken }
82+
: undefined
83+
),
8384
{{/isOAuth}}
8485
{{/authMethods}}
8586
};
8687

8788
{{/hasHttpHeaders}}
8889
{{#hasQueryParams}}
89-
const query: HttpQuery = {
90+
{{^hasRequiredQueryParams}}
91+
const query: HttpQuery = {};
92+
{{/hasRequiredQueryParams}}
93+
{{#hasRequiredQueryParams}}
94+
const query: HttpQuery = { // required parameters are used directly since they are already checked by throwIfNullOrUndefined
9095
{{#queryParams}}
96+
{{#required}}
9197
{{#isListContainer}}
9298
{{#isCollectionFormatMulti}}
93-
...(requestParameters.{{paramName}} && { '{{baseName}}': requestParameters.{{paramName}} }),
99+
'{{baseName}}': {{> paramNamePartial}},
94100
{{/isCollectionFormatMulti}}
95101
{{^isCollectionFormatMulti}}
96-
...(requestParameters.{{paramName}} && { '{{baseName}}': requestParameters.{{paramName}}.join(COLLECTION_FORMATS['{{collectionFormat}}']) }),
102+
'{{baseName}}': {{> paramNamePartial}}.join(COLLECTION_FORMATS['{{collectionFormat}}']),
97103
{{/isCollectionFormatMulti}}
98104
{{/isListContainer}}
99105
{{^isListContainer}}
100106
{{#isDateTime}}
101-
...(requestParameters.{{paramName}} && { '{{baseName}}': (requestParameters.{{paramName}} as any).toISOString() }),
107+
'{{baseName}}': ({{> paramNamePartial}} as any).toISOString(),
102108
{{/isDateTime}}
103109
{{^isDateTime}}
104110
{{#isDate}}
105-
...(requestParameters.{{paramName}} && { '{{baseName}}': (requestParameters.{{paramName}} as any).toISOString() }),
111+
'{{baseName}}': ({{> paramNamePartial}} as any).toISOString(),
106112
{{/isDate}}
107113
{{^isDate}}
108-
...(requestParameters.{{paramName}} && { '{{baseName}}': requestParameters.{{paramName}} }),
114+
'{{baseName}}': {{> paramNamePartial}},
109115
{{/isDate}}
110116
{{/isDateTime}}
111117
{{/isListContainer}}
118+
{{/required}}
112119
{{/queryParams}}
113-
{{#authMethods}}
114-
{{#isApiKey}}
115-
{{#isKeyInQuery}}
116-
...(this.configuration.apiKey && { '{{keyParamName}}': this.configuration.apiKey && this.configuration.apiKey('{{keyParamName}}') }), // {{name}} authentication
117-
{{/isKeyInQuery}}
118-
{{/isApiKey}}
119-
{{/authMethods}}
120120
};
121+
{{/hasRequiredQueryParams}}
122+
{{#hasOptionalQueryParams}}
123+
124+
{{#queryParams}}
125+
{{^required}}
126+
{{#isListContainer}}
127+
{{#isCollectionFormatMulti}}
128+
if ({{> paramNamePartial}} != null) { query['{{baseName}}'] = {{> paramNamePartial}}; }
129+
{{/isCollectionFormatMulti}}
130+
{{^isCollectionFormatMulti}}
131+
if ({{> paramNamePartial}} != null) { query['{{baseName}}'] = {{> paramNamePartial}}.join(COLLECTION_FORMATS['{{collectionFormat}}']); }
132+
{{/isCollectionFormatMulti}}
133+
{{/isListContainer}}
134+
{{^isListContainer}}
135+
{{#isDateTime}}
136+
if ({{> paramNamePartial}} != null) { query['{{baseName}}'] = ({{> paramNamePartial}} as any).toISOString(); }
137+
{{/isDateTime}}
138+
{{^isDateTime}}
139+
{{#isDate}}
140+
if ({{> paramNamePartial}} != null) { query['{{baseName}}'] = ({{> paramNamePartial}} as any).toISOString(); }
141+
{{/isDate}}
142+
{{^isDate}}
143+
if ({{> paramNamePartial}} != null) { query['{{baseName}}'] = {{> paramNamePartial}}; }
144+
{{/isDate}}
145+
{{/isDateTime}}
146+
{{/isListContainer}}
147+
{{/required}}
148+
{{/queryParams}}
149+
{{/hasOptionalQueryParams}}
150+
{{#authMethods}}
151+
{{#isApiKey}}
152+
{{#isKeyInQuery}}
153+
if (this.configuration.apiKey != null) { query['{{keyParamName}}'] = this.configuration.apiKey('{{keyParamName}}'); } // {{name}} authentication
154+
{{/isKeyInQuery}}
155+
{{/isApiKey}}
156+
{{/authMethods}}
121157

122158
{{/hasQueryParams}}
123159
{{#hasFormParams}}
124160
const formData = new FormData();
125-
{{/hasFormParams}}
126161
{{#formParams}}
127162
{{#isListContainer}}
128-
if (requestParameters.{{paramName}}) {
163+
if ({{> paramNamePartial}} !== undefined) {
129164
{{#isCollectionFormatMulti}}
130-
requestParameters.{{paramName}}.forEach((element) => {
131-
formData.append('{{baseName}}', element as any);
132-
})
165+
{{> paramNamePartial}}.forEach((element) => formData.append('{{baseName}}', element as any))
133166
{{/isCollectionFormatMulti}}
134167
{{^isCollectionFormatMulti}}
135-
formData.append('{{baseName}}', requestParameters.{{paramName}}.join(COLLECTION_FORMATS['{{collectionFormat}}']));
168+
formData.append('{{baseName}}', {{> paramNamePartial}}.join(COLLECTION_FORMATS['{{collectionFormat}}']));
136169
{{/isCollectionFormatMulti}}
137170
}
138171

139172
{{/isListContainer}}
140173
{{^isListContainer}}
141-
if (requestParameters.{{paramName}} !== undefined) {
142-
formData.append('{{baseName}}', requestParameters.{{paramName}} as any);
143-
}
144-
174+
if ({{> paramNamePartial}} !== undefined) { formData.append('{{baseName}}', {{> paramNamePartial}} as any); }
145175
{{/isListContainer}}
146176
{{/formParams}}
177+
178+
{{/hasFormParams}}
147179
return this.request<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/returnType}}>({
148-
path: '{{{path}}}'{{#pathParams}}.replace({{=<% %>=}}'{<%baseName%>}'<%={{ }}=%>, encodeURI(requestParameters.{{paramName}})){{/pathParams}},
180+
path: '{{{path}}}'{{#pathParams}}.replace({{=<% %>=}}'{<%baseName%>}'<%={{ }}=%>, encodeURI({{> paramNamePartial}})){{/pathParams}},
149181
method: '{{httpMethod}}',
150182
{{#hasHttpHeaders}}
151183
headers,
@@ -156,14 +188,14 @@ export class {{classname}} extends BaseAPI {
156188
{{#hasBodyParam}}
157189
{{#bodyParam}}
158190
{{#isContainer}}
159-
body: requestParameters.{{paramName}},
191+
body: {{paramName}},
160192
{{/isContainer}}
161193
{{^isContainer}}
162194
{{^isPrimitiveType}}
163-
body: requestParameters.{{paramName}},
195+
body: {{paramName}},
164196
{{/isPrimitiveType}}
165197
{{#isPrimitiveType}}
166-
body: requestParameters.{{paramName}} as any,
198+
body: {{paramName}} as any,
167199
{{/isPrimitiveType}}
168200
{{/isContainer}}
169201
{{/bodyParam}}

‎modules/openapi-generator/src/main/resources/typescript-rxjs/paramNamePartial.mustache‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{! helper to output the alias of a parameter if provided and to not clutter the main template }}
2+
{{#vendorExtensions.paramNameAlternative}}{{vendorExtensions.paramNameAlternative}}{{/vendorExtensions.paramNameAlternative}}{{^vendorExtensions.paramNameAlternative}}{{paramName}}{{/vendorExtensions.paramNameAlternative}}

0 commit comments

Comments
 (0)