Skip to content

Conversation

@codex-m
Copy link

@codex-m codex-m commented Feb 10, 2019

Thanks for this library. I'm using this in a project where it uploads several large files to Dropbox using API. I'm using this upload sessions and tracking the percent progress of uploads via chunk(offset size) / total size. This computation is done outside of your library (in my app)

While working with your library, I came across several bugs that made it impossible to start, append and finish the upload sessions. Also the entry class is misplaced so I fix some of it.

In your upload session methods, it assumes a file path is provided and read it via file_get_contents. This almost difficult for me to work since it assumes the file is already broken down into. This will also have memory issues (if it tries to read large chunks or even the whole file.

My approach is simple. This is how I'm using your library in my app right now for upload sessions. I simply read the large file to upload on a chunk by chunk basis using generator like this:

    /**
     * 
     * @param Resource $handle
     * @param $chunk_size
     * @return \Generator
     */
    protected function fileGetContentsChunked($handle, $chunk_size) 
    {            
        while (!feof($handle)) {
            yield fread($handle, $chunk_size);
        }        
        fclose($handle);
    }

Where the $handle is the output of fopen for the large file and $chunk_size is the size of the chunk in bytes. This is opened like this (where $path is the path to the large file to be uploaded):

$handle = fopen($path, 'rb');

Then I do a foreach loop for each chunk and call to your Dropbox upload session library methods passing each chunk instance. E.g. like this: (this is not complete code but at least you will get an idea as to how it is being used)

        foreach ($this->fileGetContentsChunked($handle, $chunk_size) as $content) {
            $remaining = $fileSize - $uploaded;
            $session_id = $this->getSessionId($id);            
            if ( $session_id && $remaining < $chunk_size )  { 
               //This is the last chunk, we will finish the upload session
                             
                $entry = $this->generateEntry($session_id, $uploaded, $target_filename);                
                $result = $dropbox->files->upload_session_finish($entry, $content);    
                
                if ( ! empty($result['path_display'])) {
                    return $result['path_display'];
                }
                break;                
            } elseif ($session_id && $uploaded < $fileSize) {              
                //We are currently appending chunks
              $ret = $dropbox->files->upload_session_append_v2($session_id, $uploaded, $content, 'false'); 
                if (false === $ret) {
                   //Generic error 
                    return false;    
                }
                if (true !== $ret) {
                   //specific error
                    return $ret;
                }
                if (true === $ret) {
                    //upload append success, monitor the $uploaded (this is the offset)
                    $uploaded = $this->getUploadedCurrentSize($content, $uploaded, $fileSize, $id);     
                } else {
                    //Generic error
                    return false;    
                }
                                           
            } elseif (false === $session_id) {
               //No session, first time let's create one
                $session_id = $dropbox->files->upload_session_start($content, false);
                if ( ! $session_id ) {
                    return false;
                }

                $this->setSessionId($session_id, $id);
                $uploaded = $this->getUploadedCurrentSize($content, $uploaded, $fileSize, $id);                
            } 
        }

I tested this thoroughly in my app and now works good for very large files (example uploading > 200MB in slow connection will still work). Feel free to review and merge this to your library if you think its useful.

For now I'm pointing my own forked version to my app composer.json.

@lukeb2014 lukeb2014 self-assigned this Feb 15, 2019
@lukeb2014
Copy link
Owner

I will review this tonight and get back to you. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants