Jesus / dropbox_api

Ruby client library for Dropbox API v2
MIT License
171 stars 113 forks source link

Fix Content-Length error #30

Closed tyler-boyd closed 7 years ago

tyler-boyd commented 7 years ago

I ran into #26 as well, and this seems like a simple fix that wouldn't have any side effects.

To help you replicate the issue, I'm calling upload as follows:

client.upload('some_path.xlsx', contents, mode: :overwrite)

contents is generated as follows:

contents = Tempfile.new([order.number, '.xlsx'], binmode: true).tap do |f|
  f.write #...some binary data
  f.flush
  f.rewind
end

By applying this commit, it works fine for me.

cc/ @simi for the inspiration on the fix

simi commented 7 years ago

Specs are failing since content is nil in there.

simi commented 7 years ago

Also if I understand well, old ruby dropbox sdk was using Content-Length also (and provided special ChunkedUploader for chunked upload). So this is the way to go.

see https://github.com/dropbox/dropbox-sdk-ruby/blob/1b33a9e0a91b9afc0e00ecf8c25b9f760ff2f2f5/lib/dropbox_sdk.rb#L190-L210

tyler-boyd commented 7 years ago

Updated, bump for 👀 @simi and @Jesus

Jesus commented 7 years ago

Thank you both for this patch. I think I would never have been able to reproduce this without your help.

I just pointed out a minor issue on the new code, but I think this is ready for merge.

simi commented 7 years ago

Is there any chance to add specs @Mavvie? I can handle that if you want.

tyler-boyd commented 7 years ago

Hey @simi, I'd appreciate that! I tried adding a test but I don't really have time right now to make a test account to get VCR working.

Probably can be a pretty simple test, just try uploading a Tempfile

Jesus commented 7 years ago

Hi @simi, if you're planning to write a test I'll wait for it to merge this. Just let me know so we can move forward, thanks!

simi commented 7 years ago

Hello. Give me day or two please to prepare patch.

Jesus commented 7 years ago

No rush, I'll wait.

simi commented 7 years ago

specs are ready in https://github.com/Mavvie/dropbox_api/pull/1, @Mavvie Can you review and merge?

Jesus commented 7 years ago

Closing in favor of #31