genqlient icon indicating copy to clipboard operation
genqlient copied to clipboard

Implement Upload spec

Open aaahrens opened this issue 2 years ago • 3 comments

It would be really nice to implement the graph multipart spec in order to be able to test it.

aaahrens avatar Oct 18 '21 12:10 aaahrens

I think this was implemented in the gqlgen test client in https://github.com/99designs/gqlgen/pull/1661 and shouldn't be too difficult to accomplish here.

StevenACoffman avatar Oct 18 '21 13:10 StevenACoffman

Neat idea! We'll need to figure out what the API should look like: is a file just an io.Reader or is it some custom type? (Can you rebind it to be os.File or fs.File to avoid having to loop through a []os.File to make it an []io.Reader?) Do we need any other metadata beyond that specified in the query? In general I'm having a little trouble parsing out exactly what is necessary or would make sense as an API, but if someone more familiar can do that I think it would make great sense to add!

benjaminjkraft avatar Oct 21 '21 17:10 benjaminjkraft

The gqlgen file upload example README breaks down what the expected Request payload should look like and how to construct it using curl.

The WithFiles() option for he gqlgen test client https://github.com/99designs/gqlgen/pull/1661 is a simple helper option to not only transform the GraphQL request into a 'multipart/form-data' Content Type that should have been supported previously, but it also loops though the provided variables and looks for os.File types to then transform them into the appropriate part within the 'multipart/form-data', with the help of http.DetectContentType() from the temporary in-memory file (so it should be able handle images, plain text files or binary files all the same). It also relies on the File type to help bind the name of the file to name of the variable within the GraphQL request in the 'Content-Disposition' for each file part.

Then there is some extra stuff to dedupe file uploads in the variables & request, so a duplicate file can be referenced multiple times within the variables, but exist once in the 'multipart/form-data' of the request.

The WithFiles() option is mostly a refactor of what was already there within the example/fileupload/fileupload_test.go, but slightly retooled for use outside its own tests and relies on the os.File from core Go to fill in the blanks when building the 'multipart/form-data' body of the request rather than a custom file struct type.

could it use io.Reader [instead]?

Yes, but only for the content of the File. Some other custom type would be needed to let the developer fill in the blanks or you set them to be unique/random assuming they do not care about the filename within the body of the request.

Can you rebind it to be os.File or fs.File to avoid having to loop through a []os.File to make it an []io.Reader?

Then you may end up dropping support for an array of files in your request variables; e.g. https://github.com/99designs/gqlgen/blob/master/_examples/fileupload/readme.md#file-list

{
  query: `
    mutation($files: [Upload!]!) {
      multipleUpload(files: $files) {
        id
      }
    }
  `,
  variables: {
    files: [
      File, // b.txt
      File  // c.txt
    ]
  }
}

It depends on how you construct your request and provide variables. If the developer provide the whole 'multipart/form-data' body of the request up front you can avoid os.File, but it puts onus on them rather than the test client to build it out.

An io.Reader might be possible where it does read in the file;

b, _ := ioutil.ReadFile(fileData[0].file.Name()) // Bravo file content.

https://github.com/99designs/gqlgen/pull/1661/files#diff-c89143397afbd44160aa66cf858dff15d64c0f0dfba958cf1450c97912980df9R111

Which is used to fill out this example part of the 'multipart/form-data' body of the request

--------------------------d7aca2a93c3655e0
Content-Disposition: form-data; name="0"; filename="b.txt"
Content-Type: text/plain

Bravo file content.

The Content-Type could be determine from the read in content of io.Reader value, but filename="?" would likely need to be made up. This would be the filling in of the blanks I mentioned earlier.

Also, name="?" references the key in the map used to reference files within the GraphQL request variables; e.g.

--------------------------d7aca2a93c3655e0
Content-Disposition: form-data; name="map"

{ "0": ["variables.files.0"], "1": ["variables.files.1"] }

A 'multipart/form-data' GraphQL request puts the query in the name="operations" part and the "file" variables in the name="map" part. The single file upload is a simple example of this, but it can also be done without files and just query & variables or just query when there is nothing to map

{
  query: `
    query($name: String!) {
      echo(name: $name)
    }
  `,
  variables: {
      name: "hello world",
  }
}
--------------------------e6b2b29561e71173
Content-Disposition: form-data; name="operations"

{ "query": "query { echo }", "variables": { "name": "hello world" } }

Do we need any other metadata beyond that specified in the query?

No not really. Filling the blanks for filename="?" should be easy enough. You would just need to construct the map between the files variables and the 'multipart/form-data' GraphQL request body.

That map can be a bit tricky, since it should accommodate for the following:

  1. Single variable
  2. Array variable
  3. Array of Graph type variables (e.g. type Foo { name: String! file: File! } & mutation($input: [Foo!]!))
  4. Duplicate files; same file in multiple variables
// --b7955bd2e1d17b67ac157b9e9ddb6238888caefc6f3541920a1debad284d
// Content-Disposition: form-data; name="map"
//
// `{ "0":["variables.input.file"] }`
// or
// `{ "0":["variables.input.files.0"], "1":["variables.input.files.1"] }`
// or
// `{ "0": ["variables.input.0.file"], "1": ["variables.input.1.file"] }`
// or
// `{ "0": ["variables.req.0.file", "variables.req.1.file"] }`

Sonna avatar Feb 23 '22 08:02 Sonna