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

[typescript-rxjs]: support reponseType blob #3437

Merged
merged 6 commits into from
Jul 25, 2019

Conversation

denyo
Copy link
Contributor

@denyo denyo commented Jul 24, 2019

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, ./bin/openapi3/{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\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

The goal is to generate the correct responseType which is passed to rxjs/ajax.
By default this is json as visible here: https://github.com/ReactiveX/rxjs/blob/master/src/internal/observable/dom/AjaxObservable.ts#L167) but it can be overriden.

I am not too sure about all the possible types but I found json, text, blob and arraybuffer to be supported by rxjs/ajax based on https://github.com/Reactive-Extensions/RxJS-DOM/pull/121/files#diff-d3b897234759c07ed79441f35e56b67fR22. Perhaps it's also possible to pass document and ms-stream as described there for completeness: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType.

The one that I personally need for a project is blob in order to download an excel file.

The relevant lines for this in the api.json are:

"/api/export": {  
    "get": {
        "produces": ["*/*"],
        "responses": {
            "200": {
                "description": "OK",
                "schema": {  
                    "type": "string",
                    "format": "byte" // <–– here
                }
            }
        }
    }
}

However AbstractTypeScriptClientCodegen maps this to a string because of typeMapping.put("ByteArray", "string");.
I tried removing this via this.typeMapping.remove("ByteArray"); in TypeScriptRxjsClientCodegen but then I end up with another import being generated for a model ByteArray which does not exist.

Here are the changes on the call:

-    exportUsingGET = (): Observable<object> => {
-        return this.request<object>({
+    exportUsingGET = (): Observable<ByteArray> => {
+        return this.request<ByteArray>({
             path: '/api/export',
             method: 'GET',
             responseType: 'blob' // <–– that's the one I need
         });
     }

In order to solve this I need some input on how to get there.
Or asked differently what's the reason for the mapping of ByteArray to string?
Or any how to figure out the appropriate responseType?

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10) @akehir (2019/07)

@auto-labeler
Copy link

auto-labeler bot commented Jul 24, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@denyo
Copy link
Contributor Author

denyo commented Jul 25, 2019

I think I got it for a spec like

"/api/export": {  
    "get": {
        "produces": ["*/*"],
        "responses": {
            "200": {
                "description": "OK",
                "schema": {  
                    "type": "file", // <–– crucial 
                }
            }
        }
    }
}

the generated code then is

exportUsingGET = (): Observable<Blob> => {
    return this.request<Blob>({
        path: '/api/export',
        method: 'GET',
        responseType: 'blob'
    });
}

The generated samples don't show this.

@denyo denyo changed the title [typescript-rxjs]: support reponseType (WIP) [typescript-rxjs]: support reponseType blob Jul 25, 2019
@macjohnny
Copy link
Member

@denyo if you have tested it and confirm it works, we can merge it.

@macjohnny
Copy link
Member

@denyo the CI is happy, can we merge it?

@macjohnny macjohnny added this to the 4.1.0 milestone Jul 25, 2019
@denyo
Copy link
Contributor Author

denyo commented Jul 25, 2019

@denyo the CI is happy, can we merge it?

Yes.

@macjohnny macjohnny merged commit 8231cbf into OpenAPITools:master Jul 25, 2019
@denyo denyo deleted the feature/rxjs-reponseType-support branch July 25, 2019 13:59
@wing328
Copy link
Member

wing328 commented Aug 10, 2019

@denyo thanks for the PR, which has been included in the 4.1.0 release: https://twitter.com/oas_generator/status/1160000504455319553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants