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}

components/drive/service/drive_api_service.cc

@@ -17,6 +17,7 @@
 #include "google_apis/drive/base_requests.h"
 #include "google_apis/drive/drive_api_parser.h"
 #include "google_apis/drive/drive_api_requests.h"
+#include "google_apis/drive/drive_switches.h"
 #include "google_apis/drive/files_list_request_runner.h"
 #include "google_apis/drive/request_sender.h"
 #include "google_apis/google_api_keys.h"
@@ -39,6 +40,7 @@ using google_apis::FileList;
 using google_apis::FileListCallback;
 using google_apis::FileResource;
 using google_apis::FileResourceCallback;
+using google_apis::FilesListCorpora;
 using google_apis::FilesListRequestRunner;
 using google_apis::GetContentCallback;
 using google_apis::GetShareUrlCallback;
@@ -347,6 +349,10 @@ CancelCallback DriveAPIService::GetAllFileList(
   request->set_max_results(kMaxNumFilesResourcePerRequest);
   request->set_q("trashed = false");  // Exclude trashed files.
   request->set_fields(kFileListFields);
+  if (google_apis::GetTeamDrivesIntegrationSwitch() ==
+      google_apis::TEAM_DRIVES_INTEGRATION_ENABLED) {
+    request->set_corpora(google_apis::FilesListCorpora::ALL_TEAM_DRIVES);
+  }
   return sender_->StartRequestWithAuthRetry(std::move(request));
 }
 
@@ -357,6 +363,15 @@ CancelCallback DriveAPIService::GetFileListInDirectory(
   DCHECK(!directory_resource_id.empty());
   DCHECK(!callback.is_null());
 
+  // TODO(yamaguchi): Use FileListScope::CreateForTeamDrive instead of
+  // kAllTeamDrives for efficiency. It'll require to add a new parameter to tell
+  // which team drive the directory resource belongs to.
+  FilesListCorpora corpora =
+      (google_apis::GetTeamDrivesIntegrationSwitch() ==
+       google_apis::TEAM_DRIVES_INTEGRATION_ENABLED)
+          ? google_apis::FilesListCorpora::ALL_TEAM_DRIVES
+          : google_apis::FilesListCorpora::DEFAULT;
+
   // Because children.list method on Drive API v2 returns only the list of
   // children's references, but we need all file resource list.
   // So, here we use files.list method instead, with setting parents query.
@@ -365,7 +380,7 @@ CancelCallback DriveAPIService::GetFileListInDirectory(
   // to client side.
   // We aren't interested in files in trash in this context, neither.
   return files_list_request_runner_->CreateAndStartWithSizeBackoff(
-      kMaxNumFilesResourcePerRequest,
+      kMaxNumFilesResourcePerRequest, corpora, std::string(),
       base::StringPrintf(
           "'%s' in parents and trashed = false",
           util::EscapeQueryStringValue(directory_resource_id).c_str()),
@@ -379,8 +394,14 @@ CancelCallback DriveAPIService::Search(
   DCHECK(!search_query.empty());
   DCHECK(!callback.is_null());
 
+  FilesListCorpora corpora =
+      (google_apis::GetTeamDrivesIntegrationSwitch() ==
+       google_apis::TEAM_DRIVES_INTEGRATION_ENABLED)
+          ? google_apis::FilesListCorpora::ALL_TEAM_DRIVES
+          : google_apis::FilesListCorpora::DEFAULT;
+
   return files_list_request_runner_->CreateAndStartWithSizeBackoff(
-      kMaxNumFilesResourcePerRequestForSearch,
+      kMaxNumFilesResourcePerRequestForSearch, corpora, std::string(),
       util::TranslateQuery(search_query), kFileListFields, callback);
 }
 

google_apis/drive/drive_api_requests.cc

@@ -557,20 +557,21 @@ GURL TeamDriveListRequest::GetURLInternal() const {
 
 //============================= FilesListRequest =============================
 
-FilesListRequest::FilesListRequest(
-    RequestSender* sender,
-    const DriveApiUrlGenerator& url_generator,
-    const FileListCallback& callback)
+FilesListRequest::FilesListRequest(RequestSender* sender,
+                                   const DriveApiUrlGenerator& url_generator,
+                                   const FileListCallback& callback)
     : DriveApiDataRequest<FileList>(sender, callback),
       url_generator_(url_generator),
-      max_results_(100) {
+      max_results_(100),
+      corpora_(FilesListCorpora::DEFAULT) {
   DCHECK(!callback.is_null());
 }
 
 FilesListRequest::~FilesListRequest() {}
 
 GURL FilesListRequest::GetURLInternal() const {
-  return url_generator_.GetFilesListUrl(max_results_, page_token_, q_);
+  return url_generator_.GetFilesListUrl(max_results_, page_token_, corpora_,
+                                        team_drive_id_, q_);
 }
 
 //======================== FilesListNextPageRequest =========================

google_apis/drive/drive_api_requests.h

@@ -532,6 +532,14 @@ class FilesListRequest : public DriveApiDataRequest<FileList> {
     page_token_ = page_token;
   }
 
+  FilesListCorpora corpora() const { return corpora_; }
+  void set_corpora(FilesListCorpora corpora) { corpora_ = corpora; }
+
+  const std::string& team_drive_id() const { return team_drive_id_; }
+  void set_team_drive_id(const std::string& team_drive_id) {
+    team_drive_id_ = team_drive_id;
+  }
+
   const std::string& q() const { return q_; }
   void set_q(const std::string& q) { q_ = q; }
 
@@ -543,6 +551,8 @@ class FilesListRequest : public DriveApiDataRequest<FileList> {
   const DriveApiUrlGenerator url_generator_;
   int max_results_;
   std::string page_token_;
+  FilesListCorpora corpora_;
+  std::string team_drive_id_;
   std::string q_;
 
   DISALLOW_COPY_AND_ASSIGN(FilesListRequest);

google_apis/drive/drive_api_url_generator.cc

@@ -42,6 +42,11 @@ const char kDriveV2TeamDrivesUrl[] = "drive/v2/teamdrives";
 
 const char kIncludeTeamDriveItems[] = "includeTeamDriveItems";
 const char kSupportsTeamDrives[] = "supportsTeamDrives";
+const char kCorpora[] = "corpora";
+const char kCorporaAllTeamDrives[] = "default,allTeamDrives";
+const char kCorporaDefault[] = "default";
+const char kCorporaTeamDrive[] = "teamDrive";
+const char kTeamDriveId[] = "teamDriveId";
 
 // apps.delete and file.authorize API is exposed through a special endpoint
 // v2internal that is accessible only by the official API key for Chrome.
@@ -59,6 +64,19 @@ GURL AddMultipartUploadParam(const GURL& url) {
   return net::AppendOrReplaceQueryParameter(url, "uploadType", "multipart");
 }
 
+const char* GetCorporaString(FilesListCorpora corpora) {
+  switch (corpora) {
+    case FilesListCorpora::DEFAULT:
+      return kCorporaDefault;
+    case FilesListCorpora::TEAM_DRIVE:
+      return kCorporaTeamDrive;
+    case FilesListCorpora::ALL_TEAM_DRIVES:
+      return kCorporaAllTeamDrives;
+  }
+  NOTREACHED();
+  return kCorporaDefault;
+}
+
 }  // namespace
 
 DriveApiUrlGenerator::DriveApiUrlGenerator(
@@ -185,12 +203,21 @@ GURL DriveApiUrlGenerator::GetFilesCopyUrl(
 
 GURL DriveApiUrlGenerator::GetFilesListUrl(int max_results,
                                            const std::string& page_token,
+                                           FilesListCorpora corpora,
+                                           const std::string& team_drive_id,
                                            const std::string& q) const {
   GURL url = base_url_.Resolve(kDriveV2FilesUrl);
   if (enable_team_drives_) {
     url = net::AppendOrReplaceQueryParameter(url, kSupportsTeamDrives, "true");
-    url = net::AppendOrReplaceQueryParameter(url, kIncludeTeamDriveItems,
-                                             "true");
+    if (corpora != FilesListCorpora::DEFAULT) {
+      url = net::AppendOrReplaceQueryParameter(url, kIncludeTeamDriveItems,
+                                               "true");
+    }
+    url = net::AppendOrReplaceQueryParameter(url, kCorpora,
+                                             GetCorporaString(corpora));
+    if (!team_drive_id.empty())
+      url =
+          net::AppendOrReplaceQueryParameter(url, kTeamDriveId, team_drive_id);
   }
   // maxResults is 100 by default.
   if (max_results != 100) {
@@ -238,7 +265,7 @@ GURL DriveApiUrlGenerator::GetChangesListUrl(
                                              "true");
     if (!team_drive_id.empty()) {
       url =
-          net::AppendOrReplaceQueryParameter(url, "teamDriveId", team_drive_id);
+          net::AppendOrReplaceQueryParameter(url, kTeamDriveId, team_drive_id);
     }
   }
   // includeDeleted is "true" by default.

google_apis/drive/drive_api_url_generator.h

@@ -15,6 +15,17 @@
 
 namespace google_apis {
 
+// This enum class is used to express a corpora parameter configuration for
+// Files:list.
+enum class FilesListCorpora {
+  // 'default': The user's subscribed items.
+  DEFAULT,
+  // 'teamDrives': A Team Drive.
+  TEAM_DRIVE,
+  // 'default,allTeamDrives': All Team Drives and the user's subscribed items.
+  ALL_TEAM_DRIVES
+};
+
 // This class is used to generate URLs for communicating with drive api
 // servers for production, and a local server for testing.
 class DriveApiUrlGenerator {
@@ -68,6 +79,8 @@ class DriveApiUrlGenerator {
   // Returns a URL to fetch file list.
   GURL GetFilesListUrl(int max_results,
                        const std::string& page_token,
+                       FilesListCorpora corpora,
+                       const std::string& team_drive_id,
                        const std::string& q) const;
 
   // Returns a URL to delete a resource with the given |file_id|.

google_apis/drive/drive_api_url_generator_unittest.cc

@@ -204,24 +204,39 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilesListUrl) {
   const std::string kV2FilesUrlPrefix =
       "https://www.example.com/drive/v2/files";
   const std::string kV2FilesUrlPrefixWithTeamDrives =
-      "https://www.example.com/drive/v2/files?"
-      "supportsTeamDrives=true&includeTeamDriveItems=true";
+      "https://www.example.com/drive/v2/files?supportsTeamDrives=true&"
+      "includeTeamDriveItems=true&corpora=default%2CallTeamDrives";
 
   for (size_t i = 0; i < arraysize(kTestPatterns); ++i) {
     EXPECT_EQ(kV2FilesUrlPrefix +
                   (kTestPatterns[i].expected_query.empty() ? "" : "?") +
                   kTestPatterns[i].expected_query,
-              url_generator_.GetFilesListUrl(kTestPatterns[i].max_results,
-                                             kTestPatterns[i].page_token,
-                                             kTestPatterns[i].q).spec());
+              url_generator_
+                  .GetFilesListUrl(kTestPatterns[i].max_results,
+                                   kTestPatterns[i].page_token,
+                                   FilesListCorpora::DEFAULT, std::string(),
+                                   kTestPatterns[i].q)
+                  .spec());
     EXPECT_EQ(kV2FilesUrlPrefixWithTeamDrives +
                   (kTestPatterns[i].expected_query.empty() ? "" : "&") +
                   kTestPatterns[i].expected_query,
-              team_drives_url_generator_.GetFilesListUrl(
-                  kTestPatterns[i].max_results,
-                  kTestPatterns[i].page_token,
-                  kTestPatterns[i].q).spec());
+              team_drives_url_generator_
+                  .GetFilesListUrl(kTestPatterns[i].max_results,
+                                   kTestPatterns[i].page_token,
+                                   FilesListCorpora::ALL_TEAM_DRIVES,
+                                   std::string(), kTestPatterns[i].q)
+                  .spec());
   }
+
+  EXPECT_EQ(
+      "https://www.example.com/drive/v2/files?supportsTeamDrives=true&"
+      "includeTeamDriveItems=true&corpora=teamDrive&"
+      "teamDriveId=TheTeamDriveId&q=query",
+      team_drives_url_generator_
+          .GetFilesListUrl(100, std::string() /* page_token */,
+                           FilesListCorpora::TEAM_DRIVE, "TheTeamDriveId",
+                           "query")
+          .spec());
 }
 
 TEST_F(DriveApiUrlGeneratorTest, GetFilesDeleteUrl) {

google_apis/drive/files_list_request_runner.cc

@@ -29,6 +29,8 @@ FilesListRequestRunner::~FilesListRequestRunner() {
 
 CancelCallback FilesListRequestRunner::CreateAndStartWithSizeBackoff(
     int max_results,
+    FilesListCorpora corpora,
+    const std::string& team_drive_id,
     const std::string& q,
     const std::string& fields,
     const FileListCallback& callback) {
@@ -39,8 +41,9 @@ CancelCallback FilesListRequestRunner::CreateAndStartWithSizeBackoff(
       base::MakeUnique<drive::FilesListRequest>(
           request_sender_, url_generator_,
           base::Bind(&FilesListRequestRunner::OnCompleted,
-                     weak_ptr_factory_.GetWeakPtr(), max_results, q, fields,
-                     callback, base::Owned(cancel_callback)));
+                     weak_ptr_factory_.GetWeakPtr(), max_results, corpora,
+                     team_drive_id, q, fields, callback,
+                     base::Owned(cancel_callback)));
   request->set_max_results(max_results);
   request->set_q(q);
   request->set_fields(fields);
@@ -61,6 +64,8 @@ void FilesListRequestRunner::OnCancel(base::Closure* cancel_callback) {
 }
 
 void FilesListRequestRunner::OnCompleted(int max_results,
+                                         FilesListCorpora corpora,
+                                         const std::string& team_drive_id,
                                          const std::string& q,
                                          const std::string& fields,
                                          const FileListCallback& callback,
@@ -74,7 +79,8 @@ void FilesListRequestRunner::OnCompleted(int max_results,
       "Drive.FilesListRequestRunner.ApiErrorCode", error);
 
   if (error == google_apis::DRIVE_RESPONSE_TOO_LARGE && max_results > 1) {
-    CreateAndStartWithSizeBackoff(max_results / 2, q, fields, callback);
+    CreateAndStartWithSizeBackoff(max_results / 2, corpora, team_drive_id, q,
+                                  fields, callback);
     return;
   }
 

google_apis/drive/files_list_request_runner.h

@@ -31,6 +31,8 @@ class FilesListRequestRunner {
   // retry in case of DRIVE_RESPONSE_TOO_LARGE error code.
   CancelCallback CreateAndStartWithSizeBackoff(
       int max_results,
+      FilesListCorpora corpora,
+      const std::string& team_drive_id,
       const std::string& q,
       const std::string& fields,
       const FileListCallback& callback);
@@ -49,6 +51,8 @@ class FilesListRequestRunner {
   // error. In case of DRIVE_RESPONSE_TOO_LARGE it will retry the request with
   // half of the requests.
   void OnCompleted(int max_results,
+                   FilesListCorpora corpora,
+                   const std::string& team_drive_id,
                    const std::string& q,
                    const std::string& fields,
                    const FileListCallback& callback,

google_apis/drive/files_list_request_runner_unittest.cc

@@ -139,7 +139,7 @@ class FilesListRequestRunnerTest : public testing::Test {
 TEST_F(FilesListRequestRunnerTest, Success_NoBackoff) {
   SetFakeServerResponse(net::HTTP_OK, kSuccessResource);
   runner_->CreateAndStartWithSizeBackoff(
-      kMaxResults, kQuery, kFields,
+      kMaxResults, FilesListCorpora::DEFAULT, std::string(), kQuery, kFields,
       base::Bind(&FilesListRequestRunnerTest::OnCompleted,
                  base::Unretained(this)));
 
@@ -161,7 +161,7 @@ TEST_F(FilesListRequestRunnerTest, Success_Backoff) {
   SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR,
                         kResponseTooLargeErrorResource);
   runner_->CreateAndStartWithSizeBackoff(
-      kMaxResults, kQuery, kFields,
+      kMaxResults, FilesListCorpora::DEFAULT, std::string(), kQuery, kFields,
       base::Bind(&FilesListRequestRunnerTest::OnCompleted,
                  base::Unretained(this)));
   {
@@ -199,7 +199,7 @@ TEST_F(FilesListRequestRunnerTest, Failure_TooManyBackoffs) {
   SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR,
                         kResponseTooLargeErrorResource);
   runner_->CreateAndStartWithSizeBackoff(
-      kMaxResults, kQuery, kFields,
+      kMaxResults, FilesListCorpora::DEFAULT, std::string(), kQuery, kFields,
       base::Bind(&FilesListRequestRunnerTest::OnCompleted,
                  base::Unretained(this)));
   {
@@ -255,7 +255,7 @@ TEST_F(FilesListRequestRunnerTest, Failure_AnotherError) {
   SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR,
                         kQuotaExceededErrorResource);
   runner_->CreateAndStartWithSizeBackoff(
-      kMaxResults, kQuery, kFields,
+      kMaxResults, FilesListCorpora::DEFAULT, std::string(), kQuery, kFields,
       base::Bind(&FilesListRequestRunnerTest::OnCompleted,
                  base::Unretained(this)));