Add create to Storage that takes an InputStream

[mziccard]

Storage class now has a method to create an object:

create(BlobInfo blobInfo, final byte[] content, BlobTargetOption... options)

I know that we already provide a BlobWriteChannel but it might be useful to also have:

create(BlobInfo blobInfo, InputStream content, BlobTargetOption... options)

for uploading small files without reading their bytes. Thoughts?

[aozarov]

I don't like having various flavors of the same method/functionality, especially when the difference is so subtle because at some point it makes the API less readable (what about adding also ByteBuffer option,....). This method should be used when content can live in memory. I picked bytes[] because I thought it is a good common denominator but could be convinced to pick something else.

It is fairly simple to convert a InputStream to byte[] (e.g. by using Guava).

[mziccard]

I see your point but let me play the devil's advocate here.
It could argued that having both methods makes the library easier to use, as for instance, a user with a small FileInputStream is not required to read the stream to a byte array (even if it is farly simple) but can upload that with a single library call.

[aozarov]

I assume that your suggestion includes also using a different RPC (apiary with an inputstream for media uploader - see here) rather than us reading the content into a byte array and using the same rpc?

It would make more sense in that case even though I think generally we should recommend using resumable uploads for big files (via BlobWriteChannel) and non-resumables for small ones. Can you check if current implementation of the MediaUploader will really chunk when using direct/non-resumable upload rather than itself reading all in memory before sending the request?

[mziccard]

Yes I was planning on adding a different RPC which makes use of InputStreamContent (see here) rather than ByteArrayContent.

From the docs of MediaHttpUploader I read:

For resumable uploads, when the media content length is known, if the provided InputStream has InputStream.markSupported() as false then it is wrapped in an BufferedInputStream to support the InputStream.mark(int) and InputStream.reset() methods required for handling server errors. If the media content length is unknown then each chunk is stored temporarily in memory. This is required to determine when the last chunk is reached.

Which makes me think that is always tries to use resumable uploads (even for ByteArrayContent).

[aozarov]

Yes, unless we pass true to setDirectUploadEnabled. see here. So my understanding (but I didn't dig deep) is that using this apiary client resumable downloads can work in chunks (for all input types) but for non-resumable (which is what I think we want here) one need to specify direct-upload and I was not sure if that will not load the complete content to memory before sending it (conceptually it does not have to). Can you verify these assumptions?

[mziccard]

When directUploadEnabled is set to true, whether data are read in chunks or not depends on the type of XXXContent. In the case of all subclasses of AbstractInputStreamContent data reads happen in chunks of 2048 bytes.

[aozarov]

You mean this AbstractInputStreamContent? As this
one seems to write all in one write... (or do they override something?)

[mziccard]

I might be wrong but doesn't AbstractInputStreamContent use IOUtils.copy which then calls BufferStreams.copy? That should work in chunks (4096B actually).

[aozarov]

Ok, if you are sure that the destination (out) is streamed when directUploadEnable is true (which would make sense) rather than buffered and sent as one blob then I think your proposal of adding this method (and an extra RPC) make sense. Maybe we can use one RPC with InputStream then....

[aozarov]

Implemented by #186