File upload

[hantonelli]

This PR adds the feature of uploading files.

I have:

• Added tests covering the feature
• Updated any relevant documentation

Fixes #342
Fixes #656

[txbrown]

I just had myself the need for this. right on time! thanks.

[maxyzli]

This is exactly what I need!

[vektah]

Thanks for the PR, sorry its taken so long to do a review of this.

Overall it looks good, test coverage and docs 😍

[hantonelli]

@vektah Hi Adam! Thank you very much for considering and reviewing the PR. I will resolve the comments. Thanks :)

[hantonelli]

@vektah Hi Adam, sorry it took me so long to implement the changes.
I made a change in the Upload type, now it looks like this:

type Upload struct {
++     FileData []byte
--     File     multipart.File
       Filename string
       Size     int64
}

There were two things that lead me to do this change:

• When to close the file.
  In this line, file, header, err := r.FormFile(key), a file is opened and it can't be closed straight away as the resolver still need to read it. So it could be closed by the resolver implementation and/or closed at the end of ServeHTTP (trying not to close it twice).
• Read the same file more than once.
  In a test where the same file is used in two places, I was using file.Seek(0, 0) because the multipart.File had been already read. In the resolver, it is not clear when something like that would be needed. Another thing, if a resolver processes the request using goroutines to process the part of the request, it's possible that two goroutines would read the same file at the same time.

So for what I see this would be the Pros and Cons of this approach (using []byte, instead of multipart.File):
Pros:

• clean file close.
• no problems reading a file that is used in multiple fields of the request, even it is done concurrently.

Cons:

• files are loaded completely into memory.

Please let me know if you think that using a multipart.File or any other approach would be better.

[vektah]

Yeah this is where it gets tricky :)

Two things I don't like about this approach:

• its in memory, its pretty easy to want to upload gigs of data.
• its the wrong interface if we want to change it in the future - io.Reader is universal

Obviously we cant pass the same handle to two concurrent resolvers, they would be each getting different parts of the file, even if they both seeked to zero.

I think there are a few approaches:

1. Do it in memory for now, but hide the underlying []byte, wrap it in a NopCloser(bytes.NewReader()) for each, gives the right interface until we put the effort in to make it better. Client code down the track will keep on working if we switch to one of the other approaches.
2. Write it to /tmp and give separate file handles to each resolver. Simple, but requires disk access.
3. Use a chain of TeeReaders, these allow you to fan out, but not seek in each of the readers. Requires all resolvers to concurrently read. If one resolver crashes there probably needs to be a panic handler to make sure panic (will be very hard to remove a tee from the chain)
4. Build something custom (BufferedMultiTeeReader), like a TeeReader but handles multiple connections internally, and allows a reader to be removed. Probably should include a fixed buffer so one resolver can get ahead of another by a small amount. Still requires careful panic handling, but now we can gracefully remove the panincing reader so it wont block the other resolvers.

I'm not sure how much you want to bite off in this PR, maybe 1 or 2 are the best bets right now.

[hantonelli]

@vektah ok, I will improve it a bit more in the following days

[hantonelli]

@vektah I implemented the suggested changes, combining approaches 1 and 2.

If a file is used in just one path, then the request form file is used. There is no need to store it in another form, as it is not going to be read by multiple readers.

If a file is used in more than one path, it will be read by multiple readers, so it is handled differently depending on the request size.
If the request is smaller than the uploadMaxMemory, then the file is read and stored in memory, then for each reader we use a new byte reader with the file bytes (bytes.NewReader(fileBytes)).
If it is bigger, then the file is read and stored in a temp file, then for each reader we use a new file descriptor pointing to the temp file (os.Open(tmpName)).

Thanks for the previous comment, let me know if you see other improvements that could be made.

[hantonelli]

@vektah I made an improvement so that if the request is smaller than the uploadMaxMemory, it uses a to shared bytes reader. Let me know if you have any comments or if see other improvements that could be made.

[vektah]

I think you arrived at a great implementation. Nice work.

[hantonelli]

@vektah Thanks Adam!

[warent]

Awesome @hantonelli, thank you!