[WIP] Support for file uploads

[PowerKiKi]

This is a work in progress to implement GraphQL multipart request specification to support file uploads.

The StandardServer now supports file uploads for request with multipart/form-data content type, but only for PSR-7 requests (IMHO it would be out of scope to parse raw request for uploaded files). A new type File was introduced to represent uploaded file in the schema.

Usage should look like:

$request = getPsr7RequestFromAnotherLibrary();

$schema = new Schema([
    'query' => new ObjectType([
        'name' => 'Query',
    ]),
    'mutation' => new ObjectType([
        'name' => 'Mutation',
        'fields' => [
            'testUpload' => [
                'type' => Type::string(),
                'args' => [
                    'text' => Type::string(),
                    'file' => Type::file(),
                ],
                'resolve' => function ($root, array $args) {

                    /** @var UploadedFileInterface $file */
                    $file = $args['file'];
                    $this->assertInstanceOf(UploadedFileInterface::class, $file);

                    // Do something more interesting with the file
                    // $file->moveTo('some/folder/in/my/project');

                    return 'Uploaded file was ' . $file->getClientFilename() . ' (' . $file->getClientMediaType() . ') with description: ' . $args['text'];
                },
            ],

        ],
    ]),
]);

$server = new StandardServer([
    'schema' => $schema,
]);

$response = $server->executePsrRequest($request);

On the implementation side of things, I would need advice on how to properly test \GraphQL\Type\Definition\FileType. For now we have \GraphQL\Tests\Executor\UploadTest which is more or less a functionnal tests for the entire stack. It's nice and working, but it only test a single case. It might be best to have unit tests covering only FileType, with more different cases, but I could not find anything similar. Would you have any recommendations about that ? where and how it should be done ?

Like I mentionned it includes PR #218, because at this point it would probably not be wise to parse uploaded file ourselves. But I suppose I could rebase without #218 if that's a problem for you.

It should be a solution for #178 (and folkloreinc/laravel-graphql#125)

I'll write some docs, once we agree on the code.

[coveralls]

Coverage increased (+0.004%) to 92.87% when pulling 9c855db on PowerKiKi:upload into a3f18b5 on webonyx:master.

[vladar]

Looks awesome, but I have couple notes:

1. I would like to separate utils like this from core types defined by GraphQL spec. So we should use something like Utils\UtilTypes::psrFile() for this instead of Type::file(). Also the type itself should live somewhere under Utils namespace (or maybe all of it should be placed under the Server namespace - what do you think?).

2. The server has to support "multipart/form-data" for default PHP globals method as well. Right now when you send multipart/form-data the server just won't do anything. But it should actually process the query passed in a request the same way it does for regular application/x-www-form-urlencoded. Then one can use $_FILES manually in the mutation resolver.
   
   We can probably implement a similar scalar type (e.g. Utils\UtilTypes::phpFile()) which just returns corresponding entry from $_FILES in the resolver.

3. I am not convinced that https://github.com/jaydenseric/graphql-multipart-request-spec is a way to go for us because it introduces separate request format which is different from what we currently use for application/x-www-form-urlencoded. I'd prefer to stay consistent here. We do not support batched queries for application/x-www-form-urlencoded anyways.
   
   We should probably stay closer to express-graphql, e.g. how they suggest doing uploads (with a difference that we use scalar type vs object type as in their example)

Please let me know what you think.

[PowerKiKi]

separate utils like this from core

I suspected you would say that. I really don't mind at all. I would tend to go Utils\UtilTypes rather than mix with server, because they could be used outside of standard server too.

support "multipart/form-data" for default PHP globals

I am not sure about that. I am actually not sure about the "raw" server at all. I feel supporting raw request (as opposed to PSR request) double our workload for really no gain. It's very easy to add a PSR compliant lib to parse globals just before calling the standard server. Supporting "raw" request is definitely convenient for new users, but I feel like it will bite us in the end, because we have to re-implement lots of PSR related things and maintain two very similar yet different paths in our code (one for raw, and one for PSR).

Instead of implementing multipart/form-data for globals, I would take the drastic opposite decision of dropping the raw server entirely.

not convinced about graphql-multipart-request-spec

This might be a deal breaker for me. I use Apollo Client on the browser side and it seemed to me that apollo-upload-client was the de-facto standard in that context. It's about 1 year old and rather popular according to GitHub stars and forks. So I can only assume that the spec that came out of that project should reflect all the experience accumulated from real-world experiences. And I did made a proof-of-concept with this PR and apollo-upload-client in my project very easily. It's also the only attempt to create a specification about uploads AFAIK.

I am not quite sure, but it seems that what express-graphql suggests would not be compatible (at all) with apollo-upload-client. And that would be a problem for me. I am willing to be convinced to use another format if there are ready-to-be-used solutions out there for Apollo, but I am not willing to work on a format that I could not use with Apollo on the client side almost out of the box.

What do you use on client side ? would you have a suggestion to combine express-graphql and Apollo Client ?

[vladar]

We have a separate endpoint for file uploads (not doing it via GraphQL). And also use our own simple GraphQL client (not Apollo or Relay)

I actually didn't realize that you want to implement some 3rd-party spec, just thought we'll mirror multipart/form-data after application/x-www-form-urlencoded with some sugar (as express-server does via multer).

As for graphql-multipart-request-spec... the problem is that we can't really say that it is a standard. Apollo server by itself doesn't support file uploads, Relay recommends doing it this way (a-la express).

I think it's better to extract this implementation as a separate project (same way as apollo-upload-server is separate from apollo-server). But still, the question remains if we can make life simpler in a Standard Server for those using such tools.

[vladar]

Actually apollo-server (at least for express) uses the same approach as express-server, so this actually looks like a standard for me.

P.S. I somehow managed to miss the part about graphql-multipart-request-spec you mentioned in the original PR proposal, my apologies for this.

[PowerKiKi]

Fair enough, I'll extract this PR as an independent project for the time being, and we can always re-consider the situation if/when a standard more clearly emerges.

[PowerKiKi]

Just released graphql-upload 1.0.0 for anyone interested...