Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix GDrive handling of office files #22405

Merged
merged 1 commit into from
Feb 19, 2016
Merged

Fix GDrive handling of office files #22405

merged 1 commit into from
Feb 19, 2016

Conversation

PVince81
Copy link
Contributor

  1. Properly detect empty file extension, can be null.

  2. When renaming part file to final file, use the correct file name
    without extension, if it exists

  3. When renaming a file, do not delete the original file if it had the
    same id, which can happen with part files

  • TEST: create a GDrive file, let sync client download, modify locally, let it reupload
  • TEST: rename a non-office file locally, let sync client rename
  • TEST: rename an office file locally, let sync client rename
  • TEST: move a non-office locally to another folder, let sync client move
  • TEST: move an office file locally to another folder, let sync client move
  • TEST: run unit tests

Might fix #9958 (but not sure about those negative sizes)

Fix GDrive handling of office files
075dd54 
1) Properly detect empty file extension, can be null.

2) When renaming part file to final file, use the correct file name
without extension, if it exists

3) When renaming a file, do not delete the original file if it had the
same id, which can happen with part files
@mention-bot
Copy link

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

@PVince81 PVince81 added this to the 9.0-current milestone Feb 15, 2016
@PVince81 PVince81 mentioned this pull request Feb 15, 2016
Closed
@PVince81
Copy link
Contributor Author

There's also an issue with encrypted files on master, when uploaded with the sync client. It looks like this PR fixes that too, possible related to the rename fix.

@ferdiga
Copy link

ferdiga commented Feb 15, 2016

still the problem for Google documents

image

@PVince81
Copy link
Contributor Author

@ferdiga does the rest work at least when not using resuming / range requests ?

@ferdiga
Copy link

ferdiga commented Feb 16, 2016

the web interface looks better, the client sync not - I still get the errors described.

BTW I applied the fix to 8.2.1

@PVince81
Copy link
Contributor Author

Ran the unit tests, some fail:

Configuration read from /srv/www/htdocs/owncloud/tests/phpunit-autotest.xml

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

Time: 9.69 minutes, Memory: 47.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.

@PVince81
Copy link
Contributor Author

I ran the same tests on master, the same ones fail. So this PR doesn't cause new breakage.

@PVince81
Copy link
Contributor Author

Raised #22481 to have a look at those old failing unit tests

@PVince81
Copy link
Contributor Author

Please review @icewind1991 @MorrisJobke @Xenopathic @LukasReschke

@icewind1991
Copy link
Contributor

👍 looks good

@PVince81
Copy link
Contributor Author

Needs a second reviewer @davitol @SergioBertolinSG @jvillafanez

@MorrisJobke
Copy link
Contributor

👍

DeepDiver1975 added a commit that referenced this pull request Feb 19, 2016
@DeepDiver1975
Merge pull request #22405 from owncloud/gdrive-mimetype-and-office-fixes
ae2304f 
Fix GDrive handling of office files
@DeepDiver1975 DeepDiver1975 merged commit ae2304f into master Feb 19, 2016
@DeepDiver1975 DeepDiver1975 deleted the gdrive-mimetype-and-office-fixes branch February 19, 2016 08:13
@Dianafg76
Copy link

I tested this issue, with two differents accounts and Is not working for my.

screen shot 2016-02-19 at 10 41 19

@PVince81
Copy link
Contributor Author

@Dianafg76 check your owncloud.log, if you see "User Rate Limit Exceeded" then it's the same issue that @davitol saw yesterday. For some reason the GDrive account you guys are using is running out of API quota quickly. (this is unrelated to this PR, you'll likely observed this on v8.2.2 and older versions too)

@PVince81
Copy link
Contributor Author

See #20481 for the user rate limit issue

@Dianafg76
Copy link

it's the same issue that @davitol saw yesterday

LOG

"message":"Exception: {\"Exception\":\"Google_Service_Exception\",\"Message\":\"Error calling GET https:\\\/\\\/www.googleapis.com\\\/drive\\\/v2\\\/files?q=title%3D%27PHP+Solutions%2C+3rd+Edition.pdf%27+and+%270AO1F3b2KzGBaUk9PVA%27+in+parents+and+trashed+%3D+false: (403) User Rate Limit Exceeded\",\"Code\":403

@PVince81 Thanks :)

@PVince81 PVince81 mentioned this pull request May 13, 2016
Closed
@lock
Copy link

lock bot commented Aug 7, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
4 - To release app:files_external
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.

GoogleDrive: 'Invalid Content Length' and folder duplicates synchronizing with linux-client
7 participants
@PVince81 @mention-bot @ferdiga @icewind1991 @MorrisJobke @Dianafg76 @DeepDiver1975