Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(730)

Issue 2894513003: Fetch files shared in Team Drives by specifying allTeamDrives corpora. (Closed)

Created:
3 years, 7 months ago by yamaguchi
Modified:
3 years, 6 months ago
Reviewers:
hashimoto
CC:
chromium-reviews, yamaguchi+watch_chromium.org, oka+watch_chromium.org, fukino+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fetch files shared in Team Drives by specifying allTeamDrives corpora. In order to list all files shared by Team Drive, "corpora" parameter should be set in addition to "includeTeamDriveItems". https://developers.google.com/drive/v2/reference/files/list For efficiency we should use teamDrive corpora when it only need files in a single Team Drive. (e.g. when getting list of files under a single directory.) It will need to change existing code that hadn't considered such distinction. So this change will use allTeamDrives corpora as a tentative fix, when Team Drives integration is enabled. It will not cause performance regression when Team Drives integration is disabled. TEST=unit_tests --gtest_filter=DriveApiUrlGeneratorTest.* BUG=723955 Review-Url: https://codereview.chromium.org/2894513003 Cr-Commit-Position: refs/heads/master@{#476951} Committed: https://chromium.googlesource.com/chromium/src/+/e6d18b1ce2297312eb28235f0bbf501fea53d627

Patch Set 1 #

Total comments: 2

Patch Set 2 : Allow setting team_drive_id and corpora parameters #

Patch Set 3 : Rename the new class to FileListScope. Eliminate unrelated changes. #

Patch Set 4 : Rename parameter variables to scope #

Total comments: 8

Patch Set 5 : Return reference to scope_ with the getter. #

Patch Set 6 : Use pair of corpora enum and team drive ID instead of new class. #

Patch Set 7 : Eliminate variables of class type with static storage duration. #

Total comments: 12

Patch Set 8 : Address review comments. #

Total comments: 4

Patch Set 9 : Use class enum and add comments. #

Patch Set 10 : Make comment style consistent with existing one #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -27 lines) Patch
M components/drive/service/drive_api_service.cc View 1 2 3 4 5 6 7 8 6 chunks +23 lines, -2 lines 0 comments Download
M google_apis/drive/drive_api_requests.h View 1 2 3 4 5 6 7 2 chunks +10 lines, -0 lines 0 comments Download
M google_apis/drive/drive_api_requests.cc View 1 2 3 4 5 6 7 8 1 chunk +7 lines, -6 lines 0 comments Download
M google_apis/drive/drive_api_url_generator.h View 1 2 3 4 5 6 7 8 9 2 chunks +13 lines, -0 lines 0 comments Download
M google_apis/drive/drive_api_url_generator.cc View 1 2 3 4 5 6 7 8 4 chunks +30 lines, -3 lines 0 comments Download
M google_apis/drive/drive_api_url_generator_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +24 lines, -9 lines 0 comments Download
M google_apis/drive/files_list_request_runner.h View 1 2 3 4 5 2 chunks +4 lines, -0 lines 0 comments Download
M google_apis/drive/files_list_request_runner.cc View 1 2 3 4 5 4 chunks +9 lines, -3 lines 0 comments Download
M google_apis/drive/files_list_request_runner_unittest.cc View 1 2 3 4 5 6 7 8 4 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 55 (39 generated)
yamaguchi
Will you take a look at this first? This is a fix for the full ...
3 years, 7 months ago (2017-05-18 07:24:05 UTC) #6
hashimoto
https://codereview.chromium.org/2894513003/diff/1/google_apis/drive/drive_api_url_generator.cc File google_apis/drive/drive_api_url_generator.cc (right): https://codereview.chromium.org/2894513003/diff/1/google_apis/drive/drive_api_url_generator.cc#newcode196 google_apis/drive/drive_api_url_generator.cc:196: "default,allTeamDrives"); 1. Is allTeamDrives the best option we can ...
3 years, 7 months ago (2017-05-19 11:14:26 UTC) #9
yamaguchi
> 1. Is allTeamDrives the best option we can use here? The reference says you ...
3 years, 7 months ago (2017-05-22 07:00:12 UTC) #10
yamaguchi
https://codereview.chromium.org/2894513003/diff/1/google_apis/drive/drive_api_url_generator.cc File google_apis/drive/drive_api_url_generator.cc (right): https://codereview.chromium.org/2894513003/diff/1/google_apis/drive/drive_api_url_generator.cc#newcode196 google_apis/drive/drive_api_url_generator.cc:196: "default,allTeamDrives"); On 2017/05/19 11:14:26, hashimoto wrote: > 1. Is ...
3 years, 7 months ago (2017-05-24 01:32:44 UTC) #23
hashimoto
Sorry for belated response. I still don't understand how this code will be used. How ...
3 years, 7 months ago (2017-05-26 09:09:19 UTC) #26
yamaguchi
On 2017/05/26 09:09:19, hashimoto wrote: > Sorry for belated response. > I still don't understand ...
3 years, 6 months ago (2017-05-29 07:15:28 UTC) #27
yamaguchi
https://codereview.chromium.org/2894513003/diff/60001/google_apis/drive/drive_api_requests.h File google_apis/drive/drive_api_requests.h (right): https://codereview.chromium.org/2894513003/diff/60001/google_apis/drive/drive_api_requests.h#newcode535 google_apis/drive/drive_api_requests.h:535: const FilesListScope scope() const { return scope_; } On ...
3 years, 6 months ago (2017-05-29 08:38:21 UTC) #28
hashimoto
BTW, could you answer to the questions on the design doc you shared offline? https://codereview.chromium.org/2894513003/diff/60001/google_apis/drive/drive_api_url_generator.h ...
3 years, 6 months ago (2017-05-30 08:34:57 UTC) #29
yamaguchi
https://codereview.chromium.org/2894513003/diff/60001/google_apis/drive/drive_api_url_generator.cc File google_apis/drive/drive_api_url_generator.cc (right): https://codereview.chromium.org/2894513003/diff/60001/google_apis/drive/drive_api_url_generator.cc#newcode403 google_apis/drive/drive_api_url_generator.cc:403: std::string FilesListScope::kDefaultAndAllTeamDrivesCorpus_ = On 2017/05/26 09:09:19, hashimoto wrote: > ...
3 years, 6 months ago (2017-05-31 08:21:30 UTC) #36
hashimoto
Sorry for being late to respond. https://codereview.chromium.org/2894513003/diff/120001/components/drive/service/drive_api_service.cc File components/drive/service/drive_api_service.cc (right): https://codereview.chromium.org/2894513003/diff/120001/components/drive/service/drive_api_service.cc#newcode381 components/drive/service/drive_api_service.cc:381: kMaxNumFilesResourcePerRequest, corpora, "", ...
3 years, 6 months ago (2017-06-02 09:44:46 UTC) #39
yamaguchi
https://codereview.chromium.org/2894513003/diff/120001/components/drive/service/drive_api_service.cc File components/drive/service/drive_api_service.cc (right): https://codereview.chromium.org/2894513003/diff/120001/components/drive/service/drive_api_service.cc#newcode381 components/drive/service/drive_api_service.cc:381: kMaxNumFilesResourcePerRequest, corpora, "", On 2017/06/02 09:44:46, hashimoto wrote: > ...
3 years, 6 months ago (2017-06-02 12:04:02 UTC) #42
hashimoto
Almost looking good https://codereview.chromium.org/2894513003/diff/140001/google_apis/drive/drive_api_url_generator.h File google_apis/drive/drive_api_url_generator.h (right): https://codereview.chromium.org/2894513003/diff/140001/google_apis/drive/drive_api_url_generator.h#newcode18 google_apis/drive/drive_api_url_generator.h:18: enum FilesListCorpora { Please add comments ...
3 years, 6 months ago (2017-06-05 04:29:22 UTC) #45
yamaguchi
https://codereview.chromium.org/2894513003/diff/140001/google_apis/drive/drive_api_url_generator.h File google_apis/drive/drive_api_url_generator.h (right): https://codereview.chromium.org/2894513003/diff/140001/google_apis/drive/drive_api_url_generator.h#newcode18 google_apis/drive/drive_api_url_generator.h:18: enum FilesListCorpora { On 2017/06/05 04:29:22, hashimoto wrote: > ...
3 years, 6 months ago (2017-06-05 05:09:08 UTC) #48
hashimoto
lgtm
3 years, 6 months ago (2017-06-05 05:12:29 UTC) #49
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2894513003/180001
3 years, 6 months ago (2017-06-05 06:16:53 UTC) #52
commit-bot: I haz the power
3 years, 6 months ago (2017-06-05 06:21:55 UTC) #55
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/e6d18b1ce2297312eb28235f0bbf...

Powered by Google App Engine
This is Rietveld 408576698