Chunk upload for GDrive #23323

PVince81 · 2016-03-16T17:00:38Z

Instead of storing the WHOLE file in memory in a PHP variable, use the
library's chunk upload support.

TEST: upload small file (less than a chunk)
TEST: upload big file (more than a chunk), about 62MB
TEST: upload+overwrite small file (less than a chunk)
TEST: upload+overwrite big file (more than a chunk), about 62MB
TEST: run GDrive unit tests

Fixes #21357 (upload part)

@icewind1991 @LukasReschke @nickvergessen @schiesbn

Instead of storing the WHOLE file in memory in a PHP variable, use the library's chunk upload support.

mention-bot · 2016-03-16T17:00:45Z

By analyzing the blame information on this pull request, we identified @Xenopathic, @MTGap and @icewind1991 to be potential reviewers

PVince81 · 2016-03-16T17:17:02Z

Okay, weird stuff happening with the new calls:

........EE..................................F..............F..F.. 65 / 80 ( 81%)
FF..F..........                                                   80 / 80 (100%)

Time: 10.27 minutes, Memory: 65.75Mb

There were 2 errors:

1) Test\Files\Storage\Google::testGetPutContents with data set #0 ('/srv/www/htdocs/owncloud/test...em.txt')
Google_Service_Exception: Error calling PUT https://www.googleapis.com/upload/drive/v2/files/0B969ZNfFU5SZM25iMTVyeFVMRDA?uploadType=resumable&upload_id=AEnB2UquUle0VjFmdeq7igkWXKa6eP5aZLnMGO9IJ_8ZOVjWINxqYb_fyUstCGSu1MFMO10b5rReHRedR5MKLH1I3nXawzuRPg: (400) Failed to parse Content-Range header.

/srv/www/htdocs/owncloud/apps/files_external/3rdparty/google-api-php-client/src/Google/Http/REST.php:79
/srv/www/htdocs/owncloud/apps/files_external/3rdparty/google-api-php-client/src/Google/Http/MediaFileUpload.php:185
/srv/www/htdocs/owncloud/apps/files_external/lib/google.php:532
/srv/www/htdocs/owncloud/lib/private/files/stream/close.php:103
/srv/www/htdocs/owncloud/lib/private/files/storage/common.php:192
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:143

2) Test\Files\Storage\Google::testGetPutContents with data set #1 ('/srv/www/htdocs/owncloud/test...ig.txt')
Google_Service_Exception: Error calling PUT https://www.googleapis.com/upload/drive/v2/files/0B969ZNfFU5SZM25iMTVyeFVMRDA?uploadType=resumable&upload_id=AEnB2UrEZgu7tYJJrvr0jnV--JTFdRs4AGajHTM-55CyjYby7xoeiWLhYc4wCZlddiLVnaLKtLl9k38Ozhnp_DbmjMMXK2R1hw: (400) Failed to parse Content-Range header.

/srv/www/htdocs/owncloud/apps/files_external/3rdparty/google-api-php-client/src/Google/Http/REST.php:79
/srv/www/htdocs/owncloud/apps/files_external/3rdparty/google-api-php-client/src/Google/Http/MediaFileUpload.php:185
/srv/www/htdocs/owncloud/apps/files_external/lib/google.php:532
/srv/www/htdocs/owncloud/lib/private/files/stream/close.php:103
/srv/www/htdocs/owncloud/lib/private/files/storage/common.php:192
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:143

--

There were 6 failures:

1) Test\Files\Storage\Google::testStat
Failed asserting that false is true.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:288

2) Test\Files\Storage\Google::testRenameOverWriteDirectory
target/test1.txt has not been overwritten
Failed asserting that false matches expected 'foo'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:489

3) Test\Files\Storage\Google::testCopyOverWriteDirectory
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:540

4) Test\Files\Storage\Google::testCopyFromSameStorage with data set #0 ('/source.txt', '/target.txt')
Expected /target.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

5) Test\Files\Storage\Google::testCopyFromSameStorage with data set #1 ('/source.txt', '/target with space.txt')
Expected /target with space.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

6) Test\Files\Storage\Google::testCopyFromSameStorage with data set #4 ('/source.txt', '/tärgét.txt')
Expected /tärgét.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

FAILURES!
Tests: 80, Assertions: 407, Errors: 2, Failures: 6.

PVince81 · 2016-03-17T11:22:09Z

Okay, got it. The problem is when writing/updating a file to zero bytes. The API doesn't like empty chunks.

This will need a separate code path to simply store the file.

PVince81 · 2016-03-17T11:38:24Z

Changed the code to only use chunks when needed.

Test result is like master:

............................................F..............F..F.. 65 / 80 ( 81%)
FF..F..........                                                   80 / 80 (100%)

Time: 8.95 minutes, Memory: 56.00Mb

There were 6 failures:

1) Test\Files\Storage\Google::testStat
Failed asserting that false is true.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:288

2) Test\Files\Storage\Google::testRenameOverWriteDirectory
target/test1.txt has not been overwritten
Failed asserting that false matches expected 'foo'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:489

3) Test\Files\Storage\Google::testCopyOverWriteDirectory
Failed asserting that true is false.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:540

4) Test\Files\Storage\Google::testCopyFromSameStorage with data set #0 ('/source.txt', '/target.txt')
Expected /target.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

5) Test\Files\Storage\Google::testCopyFromSameStorage with data set #1 ('/source.txt', '/target with space.txt')
Expected /target with space.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

6) Test\Files\Storage\Google::testCopyFromSameStorage with data set #4 ('/source.txt', '/tärgét.txt')
Expected /tärgét.txt to be a copy of /srv/www/htdocs/owncloud/tests/data/lorem.txt
Failed asserting that false matches expected 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.'.

/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:196
/srv/www/htdocs/owncloud/tests/lib/files/storage/storage.php:570

FAILURES!
Tests: 80, Assertions: 409, Failures: 6.

I'll do a manual retest:

TEST: upload empty file
TEST: upload+overwrite empty file
TEST: upload small file (less than a chunk)
TEST: upload big file (more than a chunk), about 62MB
TEST: upload+overwrite small file (less than a chunk)
TEST: upload+overwrite big file (more than a chunk), about 62MB

PVince81 · 2016-03-17T14:52:12Z

The unit tests are the same that were expected to fail like in #22481

Please review @icewind1991 @LukasReschke @schiesbn @Xenopathic

PVince81 · 2016-03-17T14:54:08Z

@karlitschek backport to 9.0.1 ? This solves memory issues when uploading because the old impl stored the whole file in memory.

karlitschek · 2016-03-17T15:11:32Z

good catch. please backport

PVince81 · 2016-03-17T15:14:34Z

stable9: #23361

PVince81 · 2016-03-21T12:52:32Z

@icewind1991 @nickvergessen @DeepDiver1975 please review.
I already tested this several times, so a glance at the code should be fine. (I know how painful it is to setup GDrive)

PVince81 · 2016-03-21T12:52:51Z

@SergioBertolinSG

icewind1991 · 2016-03-21T16:02:50Z

Code looks good 👍

SergioBertolinSG · 2016-03-21T16:28:42Z

Works fine and seems faster than before 👍

Chunk upload for GDrive

lock · 2019-08-06T20:02:28Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Chunk upload for GDrive

aa2674e

Instead of storing the WHOLE file in memory in a PHP variable, use the library's chunk upload support.

PVince81 added 3 - To Review app:files_external labels Mar 16, 2016

PVince81 added this to the 9.1-current milestone Mar 16, 2016

DeepDiver1975 added the in progress label Mar 16, 2016

PVince81 mentioned this pull request Mar 16, 2016

Stream upload/download for GDrive #21357

Closed

PVince81 added 2 - Developing and removed 3 - To Review labels Mar 16, 2016

PVince81 self-assigned this Mar 16, 2016

Only use GDrive chunks when needed

5db41df

PVince81 added 3 - To Review and removed 2 - Developing labels Mar 17, 2016

PVince81 mentioned this pull request Mar 17, 2016

[stable9] Chunk upload for GDrive #23361

Merged

PVince81 added 4 - To release and removed 3 - To Review labels Mar 21, 2016

DeepDiver1975 added a commit that referenced this pull request Mar 21, 2016

Merge pull request #23323 from owncloud/gdrive-chunkupload

736b80f

Chunk upload for GDrive

DeepDiver1975 merged commit 736b80f into master Mar 21, 2016

DeepDiver1975 deleted the gdrive-chunkupload branch March 21, 2016 17:02

DeepDiver1975 removed the in progress label Mar 21, 2016

MorrisJobke added the 9.0.1-current label Mar 22, 2016

lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chunk upload for GDrive #23323

Chunk upload for GDrive #23323

PVince81 commented Mar 16, 2016

mention-bot commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

karlitschek commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 21, 2016

PVince81 commented Mar 21, 2016

icewind1991 commented Mar 21, 2016

SergioBertolinSG commented Mar 21, 2016

lock bot commented Aug 6, 2019

Chunk upload for GDrive #23323

Chunk upload for GDrive #23323

Conversation

PVince81 commented Mar 16, 2016

mention-bot commented Mar 16, 2016

PVince81 commented Mar 16, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 17, 2016

karlitschek commented Mar 17, 2016

PVince81 commented Mar 17, 2016

PVince81 commented Mar 21, 2016

PVince81 commented Mar 21, 2016

icewind1991 commented Mar 21, 2016

SergioBertolinSG commented Mar 21, 2016

lock bot commented Aug 6, 2019