search

ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.03k stars 724 forks source link

Multipart request specification support #622

Closed Dolfik1 closed 4 years ago

Dolfik1 commented 5 years ago

Summary of the changes

Dolfik1 commented 5 years ago

WIP

Summary of the changes

michaelstaib commented 5 years ago

@Dolfik1 I will have to read into the multi-part spec how this feature is intended to be. With the new Utf8 parser that we are introducing with version 9 we are completely going away from working with strings. Basically, we are now using spans on byte to parse. This improves performance and reduces memory allocation. So, when we are introducing new parsers we have to do that in a similar way.

Do not worry about this part, I will look at that and try to introduce that in the new Utf8RequestParser.

michaelstaib commented 5 years ago

I have moved this to version 9.1.0 so we have enough time to get it right. Thanks for your work on this!

michaelstaib commented 5 years ago

Just as an info https://blog.apollographql.com/file-uploads-with-apollo-server-2-0-5db2f3f60675

michaelstaib commented 4 years ago

@Dolfik1 can you merge in the last changes from the master, then we will start moving forward on this one.

michaelstaib commented 4 years ago

@Dolfik1 and thanks for you patience here. We will include it with version 9.1.

Dolfik1 commented 4 years ago

@Dolfik1 can you merge in the last changes from the master, then we will start moving forward on this one.

I am little bit busy at this moment, but I will try to merge this branch with master today or tomorrow.

OneCyrus commented 4 years ago

just out of interest which graphql client do you use @Dolfik1 ? are there still clients out there which use multipart transport technology? I know apollo used them in the early days but replaced batching with json arrays.

michaelstaib commented 4 years ago

@OneCyrus this is more about file uploads than batching... https://github.com/jaydenseric/graphql-upload

I still not like the spec of this. Since the upload scalar is only an input type and cannot be used as output type. This is in contrast to any other scalar.

Still this is used by Apollo and other graphql implementations.

I am currently refactoring our server implementation so that we are having a more modular system. This means that you will be able to add just the server modules that you want. So, with this you can actually say I do want to opt out of the upload scalar.

Work on this is ongoing here: https://github.com/ChilliCream/hotchocolate/pull/903

michaelstaib commented 4 years ago

@OneCyrus regarding batching....

work on this is ongoing.... we have the basics in ... we now are looking at @export and stuff like that.

https://github.com/ChilliCream/hotchocolate/issues/695

OneCyrus commented 4 years ago

@OneCyrus regarding batching....

work on this is ongoing.... we have the basics in ... we now are looking at @export and stuff like that.

695

just thought because there is a MultipartQueryMiddleware in this PR which is not how apollo works anymore. https://blog.apollographql.com/batching-client-graphql-queries-a685f5bcd41b

michaelstaib commented 4 years ago

@OneCyrus I know ... but this really more about file upload... still you could batch with this. But the batching story we are working on is separate from this one.

The actual batching is integrated in the post and get middleware.....

We will introduce a new batch executor for that.

But this pr is sitting here anyway without really moving forward. At one point I will have to look into how to handle files I think.

michaelstaib commented 4 years ago

regarding batching ... I think we will move in a different direction than Apollo. I think that they put the result into a json list is not good since now you have to wait until all results are received.

But we should really talk about that here: https://github.com/ChilliCream/hotchocolate/issues/695

michaelstaib commented 4 years ago

https://blog.apollographql.com/file-uploads-with-apollo-server-2-0-5db2f3f60675#File-upload-with-schema-param

michaelstaib commented 4 years ago

@Dolfik1 what was the state of this? Was it working already? I would like to get this into the next version.

michaelstaib commented 4 years ago

Since nobody is picking this up we will park it in the multipart_request branch for now.

maryalfred25 commented 4 years ago

Hello @michaelstaib , i'm using latest version of the library and cann;t find Upload Type? is it not merged yet?

michaelstaib commented 4 years ago

No, it was not finished. We parked it. Would you like to work on it?

DeluxZ commented 3 years ago

Is this something that will be worked on in the near future? Looking for something to upload files/images via GraphQL using HotChocolate. Bumped into this PR but I'm sad it isn't finished yet.

tmitchel2 commented 3 years ago

Similar to @DeluxZ this would be a nice feature... Im trying to upload files using Relay which requires multipart form data for the 'uploadables' to work out of the box. Im going to attempt to just use a base64 string within the json instead as a workaround.