Add typescript-inversify code generator #7885

gualtierim · 2018-03-21T17:13:01Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

We added a code generator called typescript-inversify to support InversifyJS: a powerful and lightweight inversion of control container for JavaScript & Node.js apps powered by TypeScript. Inside the README is explained how to integrate services generated in an existing project.

macjohnny

in general, this looks very nice.

macjohnny · 2018-03-23T07:01:49Z

bin/security/typescript-inversify.sh

@@ -0,0 +1,31 @@
+#!/bin/sh


please add a .cmd version of this file for windows-users

macjohnny · 2018-03-23T07:02:02Z

bin/typescript-inversify-petstore.sh

@@ -0,0 +1,31 @@
+#!/bin/sh


please add a .cmd version of this file for windows-users

macjohnny · 2018-03-23T07:07:12Z

modules/swagger-codegen/src/main/resources/typescript-inversify/HttpClient.mustache

@@ -0,0 +1,62 @@
+import IHttpClient from "./IHttpClient";
+import * as Rx from "rx";


wouldn't it be better to

import { Observable } from "rxjs/Observable";

?

macjohnny · 2018-03-23T07:10:06Z

modules/swagger-codegen/src/main/resources/typescript-inversify/api.service.mustache

+        if ({{paramName}}) {
+        {{#isCollectionFormatMulti}}
+            {{paramName}}.forEach((element) => {
+                queryParameters.push("{{paramName}}="+String({{paramName}}));


you need to encodeURIComponent(),

queryParameters.push("{{paramName}}="+encodeURIComponent(String({{paramName}})));

macjohnny · 2018-03-23T07:11:20Z

modules/swagger-codegen/src/main/resources/typescript-inversify/api.service.mustache

+        {{^isListContainer}}
+        if ({{paramName}} !== undefined) {
+        {{#isDateTime}}
+           queryParameters.push("{{paramName}}="+<any>{{paramName}}.toISOString());


you need to encodeURIComponent(),

queryParameters.push("{{paramName}}="+encodeURIComponent(<any>{{paramName}}.toISOString()));

macjohnny · 2018-03-23T07:16:16Z

modules/swagger-codegen/src/main/resources/typescript-inversify/api.service.mustache

+{{/isKeyInHeader}}
+{{#isKeyInQuery}}
+        if (this.APIConfiguration.apiKeys["{{keyParamName}}"]) {
+            queryParameters.set('{{keyParamName}}', this.APIConfiguration.apiKeys["{{keyParamName}}"]);


how can queryParameters.set() work on a string[] ?
should be

queryParameters.push("{{keyParamName}}="+encodeURIComponent(this.APIConfiguration.apiKeys["{{keyParamName}}"]));

macjohnny · 2018-03-23T07:19:09Z

modules/swagger-codegen/src/main/resources/typescript-inversify/api.service.mustache

+
+{{/hasFormParams}}
+
+        return this.httpClient.{{httpMethod}}(`${this.basePath}{{{path}}}{{#hasQueryParams}}?${encodeURIComponent(queryParameters.join('&'))}{{/hasQueryParams}}`{{#bodyParam}}, {{paramName}} {{/bodyParam}}{{#hasFormParams}}, body{{/hasFormParams}}, headers)


encodeURIComponent(queryParameters.join('&'))`` should be queryParameters.join('&'), otherwise the =between the parameter name and the value will be converted to%3D`

macjohnny · 2018-03-23T07:21:33Z

modules/swagger-codegen/src/main/resources/typescript-inversify/apiInterface.mustache

+    * {{notes}}
+    {{#allParams}}* @param {{paramName}} {{description}}
+    {{/allParams}}*/
+    {{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}extraHttpRequestParams?: any): Observable<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}{}{{/returnType}}>;


the signature should correspond to the api service, i.e. should be

{{nickname}}({{#allParams}}{{paramName}}{{^required}}?{{/required}}: {{{dataType}}}, {{/allParams}}headers: Dictionary<string> = {}): Promise<{{#returnType}}{{{returnType}}}{{#isResponseTypeFile}}|undefined{{/isResponseTypeFile}}{{/returnType}}{{^returnType}}any{{/returnType}}>

macjohnny · 2018-03-23T07:23:24Z

modules/swagger-codegen/src/main/resources/typescript-inversify/package.mustache

+  "dependencies": {
+    "inversify": "^4.3.0",
+    "lodash": "^4.17.5",
+    "rx": "~4.1.0",


wouldn't you want to have rxjs: "~5.5.2" ?

macjohnny · 2018-03-23T07:24:19Z

.../io/swagger/codegen/typescript/typescriptinversify/TypeScriptInversifyClientOptionsTest.java

+import mockit.Expectations;
+import mockit.Tested;
+
+public class TypeScriptInversifyClientOptionsTest extends AbstractOptionsTest {


macjohnny · 2018-03-23T07:27:15Z

cc @TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx

Add windows script

gualtierim · 2018-03-26T13:11:52Z

Hi, thanks for all yours suggestions. I should have solved all the problems listed.

macjohnny · 2018-03-26T13:53:38Z

samples/client/petstore-security-test/typescript-inversify/IHttpClient.ts

+import { Dictionary } from "lodash";
+
+interface IHttpClient {
+    get(url:string, headers?:Dictionary<string>):Rx.Observable<HttpResponse>


do you really want to use a lodash dictionary? why not use a plain JavaScript object?

The lodash dictionary restrict the type of values and not permit nested Object. Also Angular's HttpClient allows only string as value (https://angular.io/api/http/Headers)

you can restrict the value of an object as follows:

let myObject: {[key: string]: string}

You have reason, i'm removing lodash.

macjohnny · 2018-03-26T13:55:34Z

samples/client/petstore/typescript-inversify/api/pet.service.ts

+        headers['Accept'] = 'application/xml';
+
+
+        return this.httpClient.get(`${this.basePath}/pet/findByStatus?${encodeURIComponent(queryParameters.join('&'))}`, headers)


it looks like you need to re-generate the file, since the encodeURIComponent should be removed here

macjohnny · 2018-03-26T13:59:35Z

samples/client/petstore/typescript-inversify/api/store.service.ts

+        headers['Content-Type'] = 'application/json';
+
+
+        return this.httpClient.post(`${this.basePath}/store/order`, body , headers)


are you sure you want to convert all response observables to promises?
returning them as observables would make it convenient to chain with RxJS, similar to the Angular HttpClient.
As a drawback, you always need to subscribe to the API call observable, otherwise the request is not performed.

On this subject I was undecided, because the observable returned by the HttpClient complete immediately and returns only a value (as every Promise). But you have reason that returing a observable would make convenient to chain with RxJS, but you can do it with the operator fromPromise if necessary... I'm very undecided about this.

I would suggest to adapt the interface of the Angular HttpClient with Observables.
Anyone else got an opinion on this?

I would add a configuration variable that allow the user to decide if use Promise or Observable. What about this solution?

@gualtierim this sounds like a great idea!

@macjohnny I have added the usePromise config's variable.

macjohnny · 2018-03-26T14:47:59Z

in general, I would not use lodash here, as it is not needed for any functionality and it adds a dependency. moreover, not everyone wants to use lodash.

Remove lodash dependency

macjohnny · 2018-03-27T09:00:43Z

modules/swagger-codegen/src/main/resources/typescript-inversify/apiInterface.mustache

+    */
+{{/description}}
+export interface {{classname}}Interface {
+    [others: string]: any;


is this needed?

If you set the additional parameters withInterfaces, the apiInterface should be necessary.

I was referring to [others: string]: any;

macjohnny · 2018-03-27T09:01:20Z

modules/swagger-codegen/src/main/resources/typescript-inversify/configuration.mustache

@@ -1,79 +0,0 @@
-export interface ConfigurationParameters {


@gualtierim why did you remove the configuration template?

Because i only use the IApiConfiguration that you have to implement and bind inside the container.

macjohnny · 2018-03-29T05:24:48Z

@gualtierim your code looks great.
One thing that I think would be useful is to be able to access the response headers, etc, too. Maybe you could implement this similar to the typescript-angular generator?

gualtierim · 2018-03-29T10:29:06Z

@macjohnny i have added the possibility to receive all the httpResponse. I tried to implement it as in typescript-angular generator.

macjohnny · 2018-03-29T11:16:03Z

samples/client/petstore/typescript-inversify/api/pet.service.ts

    /**
     * Add a new pet to the store
     * 
     * @param body Pet object that needs to be added to the store

     */
-
-    public addPet(body: Pet, headers: Headers = {}): Observable<any> {
+    public addPet(body: Pet, observe?: 'body', headers?: Headers): Observable<any>;


@gualtierim can you find out why the response is any? it should be Pet, shouldn't it?

ah, it's because the addPet endpoint does not return anything, so it is ok

macjohnny · 2018-03-29T11:18:29Z

modules/swagger-codegen/src/main/resources/typescript-inversify/api.service.mustache

-        return this.httpClient.{{httpMethod}}(`${this.basePath}{{{path}}}{{#hasQueryParams}}?${queryParameters.join('&')}{{/hasQueryParams}}`{{#bodyParam}}, {{paramName}} {{/bodyParam}}{{#hasFormParams}}, body{{/hasFormParams}}, headers)
-                    .map(httpResponse => <{{#returnType}}{{{returnType}}}{{#isResponseTypeFile}}|undefined{{/isResponseTypeFile}}{{/returnType}}{{^returnType}}any{{/returnType}}>(httpResponse.response)){{#usePromise}}.toPromise(){{/usePromise}};
+        const response: Observable<HttpResponse<{{#returnType}}{{{returnType}}}{{#isResponseTypeFile}}|undefined{{/isResponseTypeFile}}{{/returnType}}{{^returnType}}any{{/returnType}}>> = this.httpClient.{{httpMethod}}(`${this.basePath}{{{path}}}{{#hasQueryParams}}?${queryParameters.join('&')}{{/hasQueryParams}}`{{#bodyParam}}, {{paramName}} {{/bodyParam}}{{#hasFormParams}}, body{{/hasFormParams}}, headers);
+        if(observe == 'body')


please wrap the if block with curly braces { and }

macjohnny · 2018-03-29T11:19:28Z

@gualtierim looks great!

gualtierim · 2018-03-29T12:24:34Z

@macjohnny thanks to your support

macjohnny

I haven't tested it, but the code looks good

macjohnny · 2018-03-29T12:39:18Z

@gualtierim did you test the generated code in an actual project?

gualtierim · 2018-03-29T12:53:55Z

Yes, i have tested it inside a project and it seems to work.

wing328 · 2018-03-30T09:28:21Z

I'll take another look and merge this weekend. Thanks for the new generator!

wing328 · 2018-04-06T13:47:24Z

It seems like there's an issue with as I got the following errors when running "mvn clean test " locally:

Failed tests: 
  SpringOptionsTest>AbstractOptionsTest.checkOptionsHelp:39 » NullPointer
  SpringOptionsTest>AbstractOptionsTest.checkOptionsProcessing:28 » NullPointer

(which seems totally unrelated)
I've removed it via #7983 for the time being. Please take a look when you've time.

Add typescript-inversify language

08e0e30

gualtierim changed the title ~~Add typescript-inversify language~~ Add typescript-inversify code generator Mar 21, 2018

macjohnny suggested changes Mar 23, 2018

View reviewed changes

gualtierim added 2 commits March 26, 2018 15:05

Add windows script

ac4a2a5

Add windows script

Use rxjs instead of rx and encoding only the value of URL parameters

25c4b67

macjohnny reviewed Mar 26, 2018

View reviewed changes

gualtierim added 4 commits March 26, 2018 17:32

Remove lodash dependency

f15829d

Remove lodash dependency

Readd linux user detail on run-docker script

589e669

Solve import problems

1f08343

Remove configuration template

66fa4f2

macjohnny reviewed Mar 27, 2018

View reviewed changes

gualtierim added 2 commits March 28, 2018 17:36

Add usePromise config's variable

dc65a51

Align Test to check usePromise config parameter

c34b167

Add possibility to receive all the httpResponse

e9f6774

macjohnny reviewed Mar 29, 2018

View reviewed changes

gualtierim added 2 commits March 29, 2018 14:20

Better ts-lint on api service

e851962

Update sample client example with new ts-lint

85fdd91

Update petstore sample with new ts-lint

caa00ea

macjohnny approved these changes Mar 29, 2018

View reviewed changes

wing328 added Client: TypeScript Enhancement: New generator labels Apr 3, 2018

wing328 added this to the v2.4.0 milestone Apr 3, 2018

wing328 merged commit e2c58fa into swagger-api:master Apr 3, 2018

		@@ -0,0 +1,62 @@
		import IHttpClient from "./IHttpClient";
		import * as Rx from "rx";


		{{/hasFormParams}}

		return this.httpClient.{{httpMethod}}(`${this.basePath}{{{path}}}{{#hasQueryParams}}?${encodeURIComponent(queryParameters.join('&'))}{{/hasQueryParams}}`{{#bodyParam}}, {{paramName}} {{/bodyParam}}{{#hasFormParams}}, body{{/hasFormParams}}, headers)

		headers['Accept'] = 'application/xml';


		return this.httpClient.get(`${this.basePath}/pet/findByStatus?${encodeURIComponent(queryParameters.join('&'))}`, headers)

		headers['Content-Type'] = 'application/json';


		return this.httpClient.post(`${this.basePath}/store/order`, body , headers)

Add typescript-inversify code generator #7885

Add typescript-inversify code generator #7885

Conversation

gualtierim commented Mar 21, 2018 • edited Loading

PR checklist

Description of the PR

macjohnny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macjohnny commented Mar 23, 2018

gualtierim commented Mar 26, 2018

Choose a reason for hiding this comment

gualtierim Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gualtierim Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

macjohnny commented Mar 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macjohnny commented Mar 29, 2018

gualtierim commented Mar 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macjohnny commented Mar 29, 2018

gualtierim commented Mar 29, 2018

macjohnny left a comment

Choose a reason for hiding this comment

macjohnny commented Mar 29, 2018

gualtierim commented Mar 29, 2018

wing328 commented Mar 30, 2018

wing328 commented Apr 6, 2018

gualtierim commented Mar 21, 2018 •

edited

Loading

gualtierim Mar 26, 2018 •

edited

Loading

gualtierim Mar 28, 2018 •

edited

Loading