Retry file upload FIX

[josueruiz7]

The purpose of this PR is to fix the issue in the retry handling when uploading files... the fix consist in transforming any file data or file url into a Input Stream in the Multipart Creation.

[apollo-cla]

@josueruiz7: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[designatednerd]

Hey @josueruiz7, thanks for taking a stab at this! The thing I'm worried about with this implementation is that you still wouldn't be able to use the retry functionality with an InputStream - the stream would still get exhausted after one use, and data would still be nil since you set it up as an input stream.

I'm thinking we should make the InputStream initializer non-public (god bless still being at 0.x.x where we can make breaking changes without a major version bump 😇:trollface:), and then either hang on to the fileURL or data and use those to have a throwing generateInputStream method based on those things that throws errors if it can't regenerate that stream.

What do you think of that strategy?

[josueruiz7]

What you mean is to remove the possibility to send an inputStream to the SDK? Right?
Yeah that make sense.

[designatednerd]

Basically, if you use the initializer that takes an InputStream, there's no way to recreate it if you don't have the file URL or the data. But if you have one or the other, it's reasonably easy to recreate. So it seems like something where we should allow a constructor that has either the data or a file URL, but not a direct input stream, because that's impossible to recreate.

That's my thought, anyway.

[josueruiz7]

I updated the code changes to follow your suggestion, please take a look on them 👍
Github shows the testLoadingHeroAndFriendsNamesQueryWithIDs Test as failed, for me locally is running successfully.

[designatednerd]

@josueruiz7 Yeah that test has been flaking a bit, rerunning it now - will get a look at the code in a bit, dealing with some other stuff at the moment

[designatednerd]

I'm OK with this as a starting point - I have a few things I'd like to change before shipping (basically, not keeping the data in memory from files) but I can handle that if you'd prefer.

[designatednerd]

I think I was thinking more hang on to the fileURL - otherwise this could cause an absoltutely enormous file to get stored in memory

[designatednerd]

@josueruiz7 let me know if you want to make more changes or if you'd prefer I merge this and make them myself

[josueruiz7]

I just pushed my last change, based on your suggestion.
If you see any improvement, feel free to address it 😄
Hoping it will be released soon

[josueruiz7]

Hey @designatednerd just a question, I would like to know when this fix can be released?
Thanks for your help.

[designatednerd]

Sorry, things are a bit insane on my end right now. I'll get a look at this in a bit. Hopefully out either tonight or tomorrow, no guarantees.

[designatednerd]

I'll probably wind up making more specific errors here but overall this looks way better. Thank you very much for your help!