karlvr / openapi-generator-plus-generators

Other
21 stars 8 forks source link

Request body of type=string format=binary is incorrectly modelled as a "string" by @openapi-generator-plus/typescript-fetch-client-generator #18

Closed rcook closed 3 years ago

rcook commented 3 years ago

I have a request body with the following type (this corresponds to a byte array):

        "requestBody": {
          "content": {
            "application/octet-stream": {
              "schema": {
                "type": "string",
                "format": "binary"
              }
            }
          },
          "required": true
        },

The corresponding method in the generated TypeScript

    public myOperation(request: string, options?: RequestInit) {

I would expect that an octet stream would be represented by something Buffer or ArrayBufferLike etc. As a string, I am not able to send arbitrary byte payloads to the server.

karlvr commented 3 years ago

@rcook thank you. Yes, that seems a bit dumb. Which of those two do you think is most appropriate? Or perhaps the API could support both.

karlvr commented 3 years ago

We use FormData for multipart posts… and it looks like that wants a Blob. https://developer.mozilla.org/en-US/docs/Web/API/FormData/append

I haven't done much work with files and binary data on the client side. The generated code can definitely support doing some conversions to make it all work.

karlvr commented 3 years ago

Another thought, this generator is currently intended for being used in browsers and in node. It's not a big deal to make a node specific generator if we need to, but ideally we can use types that exist and are convenient in both.

karlvr commented 3 years ago

@rcook it looks like I will need to have different generators for browser and node... already I'm using FormData, which I think needs to be imported in node, and if I use Blob, which is what FormData wants, then that needs importing in node too... stand by.

rcook commented 3 years ago

I've seen some other OpenAPI generators (e.g. https://github.com/OpenAPITools/openapi-generator, https://openapi-generator.tech/docs/generators/typescript-node/) use something like:

type RequestFile = string | Buffer | fs.ReadStream | RequestDetailedFile;

So it can handle streams from files (e.g. fs.ReadStream) or in-memory buffers (e.g. Buffer).

karlvr commented 3 years ago

@rcook thanks for that comment. I have been working on making a separate generator for node... I suspect there will be other problems trying to use the generated code on Node.

Did you successfully generate an API client using @openapi-generator-plus/typescript-fetch-client-generator and run it in Node? I would have thought URLSearchParams would have been missing, as would RequestInit and possibly other things depending upon your API. That's what I've seen.

rcook commented 3 years ago

Yes, I successfully ran a client in Node. Apart from the handling of byte array request bodies, it worked well. I did not explore every possible type. I think I used the portable-fetch package.

karlvr commented 3 years ago

@rcook Right... I'm surprised that the URLSearchParams stuff worked honestly! That class should always be used, and I didn't think it would be a global in node—I'm intending to import it from the url package.

Anyway, with my work on this to date, I think Buffer will be the right (and simplest) type to use in a node world. It looks like it's easy to make a Buffer from an ArrayBuffer.

Then I think the generated code will work for the multipart/form-data scenario. However I don't think it will work currently for sending binary data as the whole request body, I'm putting in a fix for that too...

karlvr commented 3 years ago

@rcook OK, I've pushed some new versions that I hope will help with this. Please check out https://www.npmjs.com/package/@openapi-generator-plus/typescript-fetch-node-client-generator

rcook commented 3 years ago

Thanks. I'll take a look when I'm next at my computer.