diff options
author | mtomasz <mtomasz@chromium.org> | 2015-06-30 01:41:43 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-30 08:42:17 +0000 |
commit | a8a2de415f6597088f4a2f1e8610b5133c177cfd (patch) | |
tree | 67b53737feca6e3ddf5d77a3d42e33c457e32954 /google_apis | |
parent | 917537c881d749152267d30b49c5b48431c1bb28 (diff) | |
download | chromium_src-a8a2de415f6597088f4a2f1e8610b5133c177cfd.zip chromium_src-a8a2de415f6597088f4a2f1e8610b5133c177cfd.tar.gz chromium_src-a8a2de415f6597088f4a2f1e8610b5133c177cfd.tar.bz2 |
Implement a DRIVE_REQUEST_TOO_LARGE backoff.
1. Added DRIVE_REQUEST_TOO_LARGE error code.
2. Implemented google_apis::FilesListRequestRunner (with backoff)
3. Renamed StartRequestWithRetry to *WithAuthRetry, so it's clear what is that
retry about (as we have now two types of retries in google_apis:: drive
code).
TEST=google_apis_unittests: *FilesListRequestRunner*
BUG=433716
Review URL: https://codereview.chromium.org/1218773003
Cr-Commit-Position: refs/heads/master@{#336742}
Diffstat (limited to 'google_apis')
-rw-r--r-- | google_apis/BUILD.gn | 2 | ||||
-rw-r--r-- | google_apis/drive/base_requests.cc | 3 | ||||
-rw-r--r-- | google_apis/drive/base_requests_server_unittest.cc | 4 | ||||
-rw-r--r-- | google_apis/drive/base_requests_unittest.cc | 13 | ||||
-rw-r--r-- | google_apis/drive/drive_api_error_codes.cc | 49 | ||||
-rw-r--r-- | google_apis/drive/drive_api_error_codes.h | 1 | ||||
-rw-r--r-- | google_apis/drive/drive_api_requests_unittest.cc | 78 | ||||
-rw-r--r-- | google_apis/drive/files_list_request_runner.cc | 77 | ||||
-rw-r--r-- | google_apis/drive/files_list_request_runner.h | 71 | ||||
-rw-r--r-- | google_apis/drive/files_list_request_runner_unittest.cc | 273 | ||||
-rw-r--r-- | google_apis/drive/request_sender.cc | 6 | ||||
-rw-r--r-- | google_apis/drive/request_sender.h | 5 | ||||
-rw-r--r-- | google_apis/drive/request_sender_unittest.cc | 15 | ||||
-rw-r--r-- | google_apis/google_apis.gyp | 3 |
14 files changed, 519 insertions, 81 deletions
diff --git a/google_apis/BUILD.gn b/google_apis/BUILD.gn index 6cf7526..38e045d 100644 --- a/google_apis/BUILD.gn +++ b/google_apis/BUILD.gn @@ -153,6 +153,8 @@ source_set("google_apis") { "drive/drive_api_url_generator.cc", "drive/drive_api_url_generator.h", "drive/drive_common_callbacks.h", + "drive/files_list_request_runner.cc", + "drive/files_list_request_runner.h", "drive/request_sender.cc", "drive/request_sender.h", "drive/request_util.cc", diff --git a/google_apis/drive/base_requests.cc b/google_apis/drive/base_requests.cc index 6043244..e526012 100644 --- a/google_apis/drive/base_requests.cc +++ b/google_apis/drive/base_requests.cc @@ -152,6 +152,7 @@ google_apis::DriveApiErrorCode MapJsonError( const char kErrorReasonRateLimitExceeded[] = "rateLimitExceeded"; const char kErrorReasonUserRateLimitExceeded[] = "userRateLimitExceeded"; const char kErrorReasonQuotaExceeded[] = "quotaExceeded"; + const char kErrorReasonResponseTooLarge[] = "responseTooLarge"; scoped_ptr<const base::Value> value(google_apis::ParseJson(error_body)); const base::DictionaryValue* dictionary = NULL; @@ -177,6 +178,8 @@ google_apis::DriveApiErrorCode MapJsonError( } if (reason == kErrorReasonQuotaExceeded) return google_apis::DRIVE_NO_SPACE; + if (reason == kErrorReasonResponseTooLarge) + return google_apis::DRIVE_RESPONSE_TOO_LARGE; } } diff --git a/google_apis/drive/base_requests_server_unittest.cc b/google_apis/drive/base_requests_server_unittest.cc index c4fe388..e8da79d 100644 --- a/google_apis/drive/base_requests_server_unittest.cc +++ b/google_apis/drive/base_requests_server_unittest.cc @@ -84,7 +84,7 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_ValidFile) { test_server_.GetURL("/files/drive/testfile.txt"), GetTestCachedFilePath( base::FilePath::FromUTF8Unsafe("cached_testfile.txt"))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -118,7 +118,7 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_NonExistentFile) { test_server_.GetURL("/files/gdata/no-such-file.txt"), GetTestCachedFilePath( base::FilePath::FromUTF8Unsafe("cache_no-such-file.txt"))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } EXPECT_EQ(HTTP_NOT_FOUND, result_code); diff --git a/google_apis/drive/base_requests_unittest.cc b/google_apis/drive/base_requests_unittest.cc index fc9cdca..2bd0e03 100644 --- a/google_apis/drive/base_requests_unittest.cc +++ b/google_apis/drive/base_requests_unittest.cc @@ -175,12 +175,11 @@ TEST_F(BaseRequestsTest, UrlFetchRequestBaseResponseCodeOverride) { DriveApiErrorCode error = DRIVE_OTHER_ERROR; base::RunLoop run_loop; - sender_->StartRequestWithRetry( - new FakeUrlFetchRequest( - sender_.get(), - test_util::CreateQuitCallback( - &run_loop, test_util::CreateCopyResultCallback(&error)), - test_server_.base_url())); + sender_->StartRequestWithAuthRetry(new FakeUrlFetchRequest( + sender_.get(), + test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback(&error)), + test_server_.base_url())); run_loop.Run(); // HTTP_FORBIDDEN (403) is overridden by the error reason. @@ -209,7 +208,7 @@ TEST_F(MultipartUploadRequestBaseTest, Basic) { scoped_ptr<drive::SingleBatchableDelegateRequest> request( new drive::SingleBatchableDelegateRequest( sender_.get(), multipart_request)); - sender_->StartRequestWithRetry(request.release()); + sender_->StartRequestWithAuthRetry(request.release()); run_loop.Run(); EXPECT_EQ("multipart/related; boundary=TESTBOUNDARY", upload_content_type); EXPECT_EQ( diff --git a/google_apis/drive/drive_api_error_codes.cc b/google_apis/drive/drive_api_error_codes.cc index cae5cfd..d39fabe 100644 --- a/google_apis/drive/drive_api_error_codes.cc +++ b/google_apis/drive/drive_api_error_codes.cc @@ -12,79 +12,82 @@ namespace google_apis { std::string DriveApiErrorCodeToString(DriveApiErrorCode error) { switch (error) { case HTTP_SUCCESS: - return"HTTP_SUCCESS"; + return "HTTP_SUCCESS"; case HTTP_CREATED: - return"HTTP_CREATED"; + return "HTTP_CREATED"; case HTTP_NO_CONTENT: - return"HTTP_NO_CONTENT"; + return "HTTP_NO_CONTENT"; case HTTP_FOUND: - return"HTTP_FOUND"; + return "HTTP_FOUND"; case HTTP_NOT_MODIFIED: - return"HTTP_NOT_MODIFIED"; + return "HTTP_NOT_MODIFIED"; case HTTP_RESUME_INCOMPLETE: - return"HTTP_RESUME_INCOMPLETE"; + return "HTTP_RESUME_INCOMPLETE"; case HTTP_BAD_REQUEST: - return"HTTP_BAD_REQUEST"; + return "HTTP_BAD_REQUEST"; case HTTP_UNAUTHORIZED: - return"HTTP_UNAUTHORIZED"; + return "HTTP_UNAUTHORIZED"; case HTTP_FORBIDDEN: - return"HTTP_FORBIDDEN"; + return "HTTP_FORBIDDEN"; case HTTP_NOT_FOUND: - return"HTTP_NOT_FOUND"; + return "HTTP_NOT_FOUND"; case HTTP_CONFLICT: - return"HTTP_CONFLICT"; + return "HTTP_CONFLICT"; case HTTP_GONE: return "HTTP_GONE"; case HTTP_LENGTH_REQUIRED: - return"HTTP_LENGTH_REQUIRED"; + return "HTTP_LENGTH_REQUIRED"; case HTTP_PRECONDITION: - return"HTTP_PRECONDITION"; + return "HTTP_PRECONDITION"; case HTTP_INTERNAL_SERVER_ERROR: - return"HTTP_INTERNAL_SERVER_ERROR"; + return "HTTP_INTERNAL_SERVER_ERROR"; case HTTP_NOT_IMPLEMENTED: return "HTTP_NOT_IMPLEMENTED"; case HTTP_BAD_GATEWAY: - return"HTTP_BAD_GATEWAY"; + return "HTTP_BAD_GATEWAY"; case HTTP_SERVICE_UNAVAILABLE: - return"HTTP_SERVICE_UNAVAILABLE"; + return "HTTP_SERVICE_UNAVAILABLE"; case DRIVE_PARSE_ERROR: - return"DRIVE_PARSE_ERROR"; + return "DRIVE_PARSE_ERROR"; case DRIVE_FILE_ERROR: - return"DRIVE_FILE_ERROR"; + return "DRIVE_FILE_ERROR"; case DRIVE_CANCELLED: - return"DRIVE_CANCELLED"; + return "DRIVE_CANCELLED"; case DRIVE_OTHER_ERROR: - return"DRIVE_OTHER_ERROR"; + return "DRIVE_OTHER_ERROR"; case DRIVE_NO_CONNECTION: - return"DRIVE_NO_CONNECTION"; + return "DRIVE_NO_CONNECTION"; case DRIVE_NOT_READY: - return"DRIVE_NOT_READY"; + return "DRIVE_NOT_READY"; case DRIVE_NO_SPACE: - return"DRIVE_NO_SPACE"; + return "DRIVE_NO_SPACE"; + + case DRIVE_RESPONSE_TOO_LARGE: + return "DRIVE_RESPONSE_TOO_LARGE"; } return "UNKNOWN_ERROR_" + base::IntToString(error); diff --git a/google_apis/drive/drive_api_error_codes.h b/google_apis/drive/drive_api_error_codes.h index eafcc11..10aa901 100644 --- a/google_apis/drive/drive_api_error_codes.h +++ b/google_apis/drive/drive_api_error_codes.h @@ -36,6 +36,7 @@ enum DriveApiErrorCode { DRIVE_NO_CONNECTION = -104, DRIVE_NOT_READY = -105, DRIVE_NO_SPACE = -106, + DRIVE_RESPONSE_TOO_LARGE = -107 }; // Returns a string representation of DriveApiErrorCode. diff --git a/google_apis/drive/drive_api_requests_unittest.cc b/google_apis/drive/drive_api_requests_unittest.cc index 0f0e7c2..8ca6e17 100644 --- a/google_apis/drive/drive_api_requests_unittest.cc +++ b/google_apis/drive/drive_api_requests_unittest.cc @@ -515,7 +515,7 @@ TEST_F(DriveApiRequestsTest, DriveApiDataRequest_Fields) { test_util::CreateCopyResultCallback(&error, &about_resource))); request->set_fields("kind,quotaBytesTotal,quotaBytesUsedAggregate," "largestChangeId,rootFolderId"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -565,7 +565,7 @@ TEST_F(DriveApiRequestsTest, FilesInsertRequest) { request->add_parent("root"); request->set_title("new directory"); request->set_properties(testing_properties_); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -630,7 +630,7 @@ TEST_F(DriveApiRequestsTest, FilesPatchRequest) { request->add_parent("parent_resource_id"); request->set_properties(testing_properties_); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -670,7 +670,7 @@ TEST_F(DriveApiRequestsTest, AboutGetRequest_ValidJson) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &about_resource))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -705,7 +705,7 @@ TEST_F(DriveApiRequestsTest, AboutGetRequest_InvalidJson) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &about_resource))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -733,7 +733,7 @@ TEST_F(DriveApiRequestsTest, AppsListRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &app_list))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -761,7 +761,7 @@ TEST_F(DriveApiRequestsTest, ChangesListRequest) { request->set_include_deleted(true); request->set_start_change_id(100); request->set_max_results(500); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -789,7 +789,7 @@ TEST_F(DriveApiRequestsTest, ChangesListNextPageRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &result))); request->set_next_link(test_server_.GetURL("/continue/get/change/list")); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -823,7 +823,7 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest) { request->set_modified_date(base::Time::FromUTCExploded(kModifiedDate)); request->add_parent("parent_resource_id"); request->set_title("new title"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -860,7 +860,7 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest_EmptyParentResourceId) { test_util::CreateCopyResultCallback(&error, &file_resource))); request->set_file_id("resource_id"); request->set_title("new title"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -891,7 +891,7 @@ TEST_F(DriveApiRequestsTest, FilesListRequest) { test_util::CreateCopyResultCallback(&error, &result))); request->set_max_results(50); request->set_q("\"abcde\" in parents"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -919,7 +919,7 @@ TEST_F(DriveApiRequestsTest, FilesListNextPageRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &result))); request->set_next_link(test_server_.GetURL("/continue/get/file/list")); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -942,7 +942,7 @@ TEST_F(DriveApiRequestsTest, FilesDeleteRequest) { &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_file_id("resource_id"); request->set_etag(kTestETag); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -972,7 +972,7 @@ TEST_F(DriveApiRequestsTest, FilesTrashRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &file_resource))); request->set_file_id("resource_id"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1002,7 +1002,7 @@ TEST_F(DriveApiRequestsTest, ChildrenInsertRequest) { test_util::CreateCopyResultCallback(&error))); request->set_folder_id("parent_resource_id"); request->set_id("resource_id"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1031,7 +1031,7 @@ TEST_F(DriveApiRequestsTest, ChildrenDeleteRequest) { test_util::CreateCopyResultCallback(&error))); request->set_child_id("resource_id"); request->set_folder_id("parent_resource_id"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1071,7 +1071,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); request->set_properties(testing_properties_); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1116,7 +1116,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1167,7 +1167,7 @@ TEST_F(DriveApiRequestsTest, UploadNewEmptyFileRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1207,7 +1207,7 @@ TEST_F(DriveApiRequestsTest, UploadNewEmptyFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1256,7 +1256,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1295,7 +1295,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry))); - request_sender_->StartRequestWithRetry(get_upload_status_request); + request_sender_->StartRequestWithAuthRetry(get_upload_status_request); run_loop.Run(); } @@ -1341,7 +1341,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1385,7 +1385,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry))); - request_sender_->StartRequestWithRetry(get_upload_status_request); + request_sender_->StartRequestWithAuthRetry(get_upload_status_request); run_loop.Run(); } @@ -1438,7 +1438,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileWithMetadataRequest) { request->set_modified_date(base::Time::FromUTCExploded(kModifiedDate)); request->set_last_viewed_by_me_date( base::Time::FromUTCExploded(kLastViewedByMeDate)); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1489,7 +1489,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); request->set_properties(testing_properties_); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1529,7 +1529,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1580,7 +1580,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1616,7 +1616,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1669,7 +1669,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETagConflicting) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1714,7 +1714,7 @@ TEST_F(DriveApiRequestsTest, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1755,7 +1755,7 @@ TEST_F(DriveApiRequestsTest, &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(resume_request); run_loop.Run(); } @@ -1816,7 +1816,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileWithMetadataRequest) { request->set_last_viewed_by_me_date( base::Time::FromUTCExploded(kLastViewedByMeDate)); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1860,7 +1860,7 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest) { test_util::CreateCopyResultCallback(&result_code, &temp_file)), GetContentCallback(), ProgressCallback()); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1897,7 +1897,7 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest_GetContentCallback) { test_util::CreateCopyResultCallback(&result_code, &temp_file)), base::Bind(&AppendContent, &contents), ProgressCallback()); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1932,7 +1932,7 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { request->set_role(drive::PERMISSION_ROLE_COMMENTER); request->set_type(drive::PERMISSION_TYPE_USER); request->set_value("user@example.com"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1967,7 +1967,7 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { request->set_role(drive::PERMISSION_ROLE_WRITER); request->set_type(drive::PERMISSION_TYPE_DOMAIN); request->set_value("example.com"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); run_loop.Run(); } @@ -1998,7 +1998,7 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequest) { drive::BatchUploadRequest* const request = new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); request->SetBoundaryForTesting("OUTERBOUNDARY"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); // Create child request. DriveApiErrorCode errors[] = {DRIVE_OTHER_ERROR, DRIVE_OTHER_ERROR}; @@ -2086,7 +2086,7 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequestWithBodyIncludingZero) { drive::BatchUploadRequest* const request = new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); request->SetBoundaryForTesting("OUTERBOUNDARY"); - request_sender_->StartRequestWithRetry(request); + request_sender_->StartRequestWithAuthRetry(request); // Create child request. { diff --git a/google_apis/drive/files_list_request_runner.cc b/google_apis/drive/files_list_request_runner.cc new file mode 100644 index 0000000..8bf50be --- /dev/null +++ b/google_apis/drive/files_list_request_runner.cc @@ -0,0 +1,77 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "google_apis/drive/files_list_request_runner.h" + +#include "base/bind.h" +#include "google_apis/drive/drive_api_error_codes.h" +#include "google_apis/drive/drive_api_requests.h" +#include "google_apis/drive/request_sender.h" + +namespace google_apis { + +FilesListRequestRunner::FilesListRequestRunner( + RequestSender* request_sender, + const google_apis::DriveApiUrlGenerator& url_generator) + : request_sender_(request_sender), + url_generator_(url_generator), + weak_ptr_factory_(this) { +} + +FilesListRequestRunner::~FilesListRequestRunner() { +} + +CancelCallback FilesListRequestRunner::CreateAndStartWithSizeBackoff( + int max_results, + const std::string& q, + const std::string& fields, + const FileListCallback& callback) { + base::Closure* const cancel_callback = new base::Closure; + drive::FilesListRequest* const request = new drive::FilesListRequest( + request_sender_, url_generator_, + base::Bind(&FilesListRequestRunner::OnCompleted, + weak_ptr_factory_.GetWeakPtr(), max_results, q, fields, + callback, base::Owned(cancel_callback))); + request->set_max_results(max_results); + request->set_q(q); + request->set_fields(fields); + *cancel_callback = request_sender_->StartRequestWithAuthRetry(request); + + // The cancellation callback is owned by the completion callback, so it must + // not be used after |callback| is called. + return base::Bind(&FilesListRequestRunner::OnCancel, + weak_ptr_factory_.GetWeakPtr(), + base::Unretained(cancel_callback)); +} + +void FilesListRequestRunner::OnCancel(base::Closure* cancel_callback) { + DCHECK(cancel_callback); + DCHECK(!cancel_callback->is_null()); + cancel_callback->Run(); +} + +void FilesListRequestRunner::OnCompleted(int max_results, + const std::string& q, + const std::string& fields, + const FileListCallback& callback, + CancelCallback* cancel_callback, + DriveApiErrorCode error, + scoped_ptr<FileList> entry) { + if (!request_completed_callback_for_testing_.is_null()) + request_completed_callback_for_testing_.Run(); + + if (error == google_apis::DRIVE_RESPONSE_TOO_LARGE && max_results > 1) { + CreateAndStartWithSizeBackoff(max_results / 2, q, fields, callback); + return; + } + + callback.Run(error, entry.Pass()); +} + +void FilesListRequestRunner::SetRequestCompletedCallbackForTesting( + const base::Closure& callback) { + request_completed_callback_for_testing_ = callback; +} + +} // namespace google_apis diff --git a/google_apis/drive/files_list_request_runner.h b/google_apis/drive/files_list_request_runner.h new file mode 100644 index 0000000..fc416be --- /dev/null +++ b/google_apis/drive/files_list_request_runner.h @@ -0,0 +1,71 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef GOOGLE_APIS_DRIVE_FILES_LIST_REQUEST_RUNNER_H_ +#define GOOGLE_APIS_DRIVE_FILES_LIST_REQUEST_RUNNER_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/callback_forward.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "google_apis/drive/drive_api_requests.h" +#include "google_apis/drive/drive_api_url_generator.h" +#include "google_apis/drive/drive_common_callbacks.h" + +namespace google_apis { + +class RequestSender; + +// Runs file list requests (the FileListRequest class) with a backoff retry +// logic in case of the DRIVE_RESPONSE_TOO_LARGE error code. +class FilesListRequestRunner { + public: + FilesListRequestRunner( + RequestSender* request_sender, + const google_apis::DriveApiUrlGenerator& url_generator); + + // Creates a FilesListRequest instance and starts the request with a backoff + // retry in case of DRIVE_RESPONSE_TOO_LARGE error code. + CancelCallback CreateAndStartWithSizeBackoff( + int max_results, + const std::string& q, + const std::string& fields, + const FileListCallback& callback); + + ~FilesListRequestRunner(); + + void SetRequestCompletedCallbackForTesting(const base::Closure& callback); + + private: + // Called when the cancelling callback returned by + // CreateAndStartWithSizeBackoff is invoked. Once called cancels the current + // request. + void OnCancel(CancelCallback* cancel_callback); + + // Called when a single request is completed with either a success or an + // error. In case of DRIVE_RESPONSE_TOO_LARGE it will retry the request with + // half of the requests. + void OnCompleted(int max_results, + const std::string& q, + const std::string& fields, + const FileListCallback& callback, + CancelCallback* cancel_callback, + DriveApiErrorCode error, + scoped_ptr<FileList> entry); + + RequestSender* request_sender_; // Not owned. + const google_apis::DriveApiUrlGenerator url_generator_; // Not owned. + base::Closure request_completed_callback_for_testing_; + + // Note: This should remain the last member so it'll be destroyed and + // invalidate its weak pointers before any other members are destroyed. + base::WeakPtrFactory<FilesListRequestRunner> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(FilesListRequestRunner); +}; + +} // namespace google_apis + +#endif // GOOGLE_APIS_DRIVE_FILES_LIST_REQUEST_RUNNER_H_ diff --git a/google_apis/drive/files_list_request_runner_unittest.cc b/google_apis/drive/files_list_request_runner_unittest.cc new file mode 100644 index 0000000..94d7312 --- /dev/null +++ b/google_apis/drive/files_list_request_runner_unittest.cc @@ -0,0 +1,273 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "google_apis/drive/files_list_request_runner.h" + +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "base/sequenced_task_runner.h" +#include "base/strings/string_number_conversions.h" +#include "base/thread_task_runner_handle.h" +#include "google_apis/drive/base_requests.h" +#include "google_apis/drive/dummy_auth_service.h" +#include "google_apis/drive/request_sender.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/embedded_test_server/http_request.h" +#include "net/test/embedded_test_server/http_response.h" +#include "net/url_request/url_request_test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace google_apis { +namespace { + +const int kMaxResults = 4; +const char kQuery[] = "testing-query"; +const char kFields[] = "testing-fields"; +const char kTestUserAgent[] = "test-user-agent"; + +const char kSuccessResource[] = + "{\n" + " \"kind\": \"drive#fileList\",\n" + " \"etag\": \"etag\",\n" + " \"items\": []\n" + "}\n"; + +const char kResponseTooLargeErrorResource[] = + "{\n" + " \"error\": {\n" + " \"errors\": [\n" + " {\n" + " \"reason\": \"responseTooLarge\"\n" + " }\n" + " ]\n" + " }\n" + "}\n"; + +const char kQuotaExceededErrorResource[] = + "{\n" + " \"error\": {\n" + " \"errors\": [\n" + " {\n" + " \"reason\": \"quotaExceeded\"\n" + " }\n" + " ]\n" + " }\n" + "}\n"; + +} // namespace + +class FilesListRequestRunnerTest : public testing::Test { + public: + FilesListRequestRunnerTest() {} + + void SetUp() override { + request_context_getter_ = + new net::TestURLRequestContextGetter(message_loop_.task_runner()); + + request_sender_.reset( + new RequestSender(new DummyAuthService, request_context_getter_.get(), + message_loop_.task_runner(), kTestUserAgent)); + + test_server_.RegisterRequestHandler( + base::Bind(&FilesListRequestRunnerTest::OnFilesListRequest, + base::Unretained(this), test_server_.base_url())); + ASSERT_TRUE(test_server_.InitializeAndWaitUntilReady()); + + runner_.reset(new FilesListRequestRunner( + request_sender_.get(), + google_apis::DriveApiUrlGenerator(test_server_.base_url(), + test_server_.GetURL("/download/")))); + } + + void TearDown() override { + on_completed_callback_ = base::Closure(); + http_request_.reset(); + response_error_.reset(); + response_entry_.reset(); + } + + // Called when the request is completed and no more backoff retries will + // happen. + void OnCompleted(DriveApiErrorCode error, scoped_ptr<FileList> entry) { + response_error_.reset(new DriveApiErrorCode(error)); + response_entry_ = entry.Pass(); + on_completed_callback_.Run(); + } + + protected: + // Sets a fake Drive API server response to be returned for the upcoming HTTP + // request. + void SetFakeServerResponse(net::HttpStatusCode code, + const std::string& content) { + fake_server_response_.reset(new net::test_server::BasicHttpResponse); + fake_server_response_->set_code(code); + fake_server_response_->set_content(content); + fake_server_response_->set_content_type("application/json"); + } + + // Handles a HTTP request to the Drive API server and returns a fake response. + scoped_ptr<net::test_server::HttpResponse> OnFilesListRequest( + const GURL& base_url, + const net::test_server::HttpRequest& request) { + http_request_.reset(new net::test_server::HttpRequest(request)); + return fake_server_response_.Pass(); + } + + base::MessageLoopForIO message_loop_; // Test server needs IO thread. + scoped_ptr<RequestSender> request_sender_; + net::test_server::EmbeddedTestServer test_server_; + scoped_ptr<FilesListRequestRunner> runner_; + scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_; + base::Closure on_completed_callback_; + + // Response set by test cases to be returned from the HTTP server. + scoped_ptr<net::test_server::BasicHttpResponse> fake_server_response_; + + // A requests and a response stored for verification in test cases. + scoped_ptr<net::test_server::HttpRequest> http_request_; + scoped_ptr<DriveApiErrorCode> response_error_; + scoped_ptr<FileList> response_entry_; +}; + +TEST_F(FilesListRequestRunnerTest, Success_NoBackoff) { + SetFakeServerResponse(net::HTTP_OK, kSuccessResource); + runner_->CreateAndStartWithSizeBackoff( + kMaxResults, kQuery, kFields, + base::Bind(&FilesListRequestRunnerTest::OnCompleted, + base::Unretained(this))); + + base::RunLoop run_loop; + on_completed_callback_ = run_loop.QuitClosure(); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=4&q=testing-query&fields=testing-fields", + http_request_->relative_url); + + ASSERT_TRUE(response_error_.get()); + EXPECT_EQ(HTTP_SUCCESS, *response_error_); + EXPECT_TRUE(response_entry_.get()); +} + +TEST_F(FilesListRequestRunnerTest, Success_Backoff) { + SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR, + kResponseTooLargeErrorResource); + runner_->CreateAndStartWithSizeBackoff( + kMaxResults, kQuery, kFields, + base::Bind(&FilesListRequestRunnerTest::OnCompleted, + base::Unretained(this))); + { + base::RunLoop run_loop; + runner_->SetRequestCompletedCallbackForTesting(run_loop.QuitClosure()); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=4&q=testing-query&fields=testing-fields", + http_request_->relative_url); + EXPECT_FALSE(response_error_.get()); + } + + // Backoff will decreasing the number of results by 2, which will succeed. + { + SetFakeServerResponse(net::HTTP_OK, kSuccessResource); + + base::RunLoop run_loop; + on_completed_callback_ = run_loop.QuitClosure(); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=2&q=testing-query&fields=testing-fields", + http_request_->relative_url); + + ASSERT_TRUE(response_error_.get()); + EXPECT_EQ(HTTP_SUCCESS, *response_error_); + EXPECT_TRUE(response_entry_.get()); + } +} + +TEST_F(FilesListRequestRunnerTest, Failure_TooManyBackoffs) { + SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR, + kResponseTooLargeErrorResource); + runner_->CreateAndStartWithSizeBackoff( + kMaxResults, kQuery, kFields, + base::Bind(&FilesListRequestRunnerTest::OnCompleted, + base::Unretained(this))); + { + base::RunLoop run_loop; + runner_->SetRequestCompletedCallbackForTesting(run_loop.QuitClosure()); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=4&q=testing-query&fields=testing-fields", + http_request_->relative_url); + EXPECT_FALSE(response_error_.get()); + } + + // Backoff will decreasing the number of results by 2, which will still fail + // due to too large response. + { + SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR, + kResponseTooLargeErrorResource); + + base::RunLoop run_loop; + runner_->SetRequestCompletedCallbackForTesting(run_loop.QuitClosure()); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=2&q=testing-query&fields=testing-fields", + http_request_->relative_url); + EXPECT_FALSE(response_error_.get()); + } + + // The last backoff, decreasing the number of results to 1. + { + SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR, + kResponseTooLargeErrorResource); + + base::RunLoop run_loop; + on_completed_callback_ = run_loop.QuitClosure(); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=1&q=testing-query&fields=testing-fields", + http_request_->relative_url); + + ASSERT_TRUE(response_error_.get()); + EXPECT_EQ(DRIVE_RESPONSE_TOO_LARGE, *response_error_); + EXPECT_FALSE(response_entry_.get()); + } +} + +TEST_F(FilesListRequestRunnerTest, Failure_AnotherError) { + SetFakeServerResponse(net::HTTP_INTERNAL_SERVER_ERROR, + kQuotaExceededErrorResource); + runner_->CreateAndStartWithSizeBackoff( + kMaxResults, kQuery, kFields, + base::Bind(&FilesListRequestRunnerTest::OnCompleted, + base::Unretained(this))); + + base::RunLoop run_loop; + on_completed_callback_ = run_loop.QuitClosure(); + run_loop.Run(); + + ASSERT_TRUE(http_request_.get()); + EXPECT_EQ( + "/drive/v2/files?maxResults=4&q=testing-query&fields=testing-fields", + http_request_->relative_url); + + // There must be no backoff in case of an error different than + // DRIVE_RESPONSE_TOO_LARGE. + ASSERT_TRUE(response_error_.get()); + EXPECT_EQ(DRIVE_NO_SPACE, *response_error_); + EXPECT_FALSE(response_entry_.get()); +} + +} // namespace google_apis diff --git a/google_apis/drive/request_sender.cc b/google_apis/drive/request_sender.cc index e8a39c9..2f8b4c8 100644 --- a/google_apis/drive/request_sender.cc +++ b/google_apis/drive/request_sender.cc @@ -31,7 +31,7 @@ RequestSender::~RequestSender() { in_flight_requests_.end()); } -base::Closure RequestSender::StartRequestWithRetry( +base::Closure RequestSender::StartRequestWithAuthRetry( AuthenticatedRequestInterface* request) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -72,7 +72,7 @@ void RequestSender::OnAccessTokenFetched( if (code == HTTP_SUCCESS) { DCHECK(auth_service_->HasAccessToken()); - StartRequestWithRetry(request.get()); + StartRequestWithAuthRetry(request.get()); } else { request->OnAuthFailed(code); } @@ -84,7 +84,7 @@ void RequestSender::RetryRequest(AuthenticatedRequestInterface* request) { auth_service_->ClearAccessToken(); // User authentication might have expired - rerun the request to force // auth token refresh. - StartRequestWithRetry(request); + StartRequestWithAuthRetry(request); } void RequestSender::CancelRequest( diff --git a/google_apis/drive/request_sender.h b/google_apis/drive/request_sender.h index ff5130d..6ace797 100644 --- a/google_apis/drive/request_sender.h +++ b/google_apis/drive/request_sender.h @@ -69,7 +69,8 @@ class RequestSender { // // Returns a closure to cancel the request. The closure cancels the request // if it is in-flight, and does nothing if it is already terminated. - base::Closure StartRequestWithRetry(AuthenticatedRequestInterface* request); + base::Closure StartRequestWithAuthRetry( + AuthenticatedRequestInterface* request); // Notifies to this RequestSender that |request| has finished. // TODO(kinaba): refactor the life time management and make this at private. @@ -87,7 +88,7 @@ class RequestSender { void RetryRequest(AuthenticatedRequestInterface* request); // Cancels the request. Used for implementing the returned closure of - // StartRequestWithRetry. + // StartRequestWithAuthRetry. void CancelRequest( const base::WeakPtr<AuthenticatedRequestInterface>& request); diff --git a/google_apis/drive/request_sender_unittest.cc b/google_apis/drive/request_sender_unittest.cc index 63e8a8f..fb4a156 100644 --- a/google_apis/drive/request_sender_unittest.cc +++ b/google_apis/drive/request_sender_unittest.cc @@ -139,7 +139,8 @@ TEST_F(RequestSenderTest, StartAndFinishRequest) { &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); - base::Closure cancel_closure = request_sender_.StartRequestWithRetry(request); + base::Closure cancel_closure = + request_sender_.StartRequestWithAuthRetry(request); EXPECT_TRUE(!cancel_closure.is_null()); // Start is called with the specified access token. Let it succeed. @@ -162,7 +163,8 @@ TEST_F(RequestSenderTest, StartAndCancelRequest) { &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); - base::Closure cancel_closure = request_sender_.StartRequestWithRetry(request); + base::Closure cancel_closure = + request_sender_.StartRequestWithAuthRetry(request); EXPECT_TRUE(!cancel_closure.is_null()); EXPECT_TRUE(start_called); @@ -182,7 +184,8 @@ TEST_F(RequestSenderTest, NoRefreshToken) { &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); - base::Closure cancel_closure = request_sender_.StartRequestWithRetry(request); + base::Closure cancel_closure = + request_sender_.StartRequestWithAuthRetry(request); EXPECT_TRUE(!cancel_closure.is_null()); // The request is not started at all because no access token is obtained. @@ -201,7 +204,8 @@ TEST_F(RequestSenderTest, ValidRefreshTokenAndNoAccessToken) { &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); - base::Closure cancel_closure = request_sender_.StartRequestWithRetry(request); + base::Closure cancel_closure = + request_sender_.StartRequestWithAuthRetry(request); EXPECT_TRUE(!cancel_closure.is_null()); // Access token should indicate that this is the first retry. @@ -221,7 +225,8 @@ TEST_F(RequestSenderTest, AccessTokenRejectedSeveralTimes) { &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); - base::Closure cancel_closure = request_sender_.StartRequestWithRetry(request); + base::Closure cancel_closure = + request_sender_.StartRequestWithAuthRetry(request); EXPECT_TRUE(!cancel_closure.is_null()); EXPECT_TRUE(start_called); diff --git a/google_apis/google_apis.gyp b/google_apis/google_apis.gyp index c3263d0..260367e 100644 --- a/google_apis/google_apis.gyp +++ b/google_apis/google_apis.gyp @@ -57,6 +57,8 @@ 'drive/drive_api_url_generator.cc', 'drive/drive_api_url_generator.h', 'drive/drive_common_callbacks.h', + 'drive/files_list_request_runner.cc', + 'drive/files_list_request_runner.h', 'drive/request_sender.cc', 'drive/request_sender.h', 'drive/request_util.cc', @@ -154,6 +156,7 @@ 'drive/drive_api_parser_unittest.cc', 'drive/drive_api_requests_unittest.cc', 'drive/drive_api_url_generator_unittest.cc', + 'drive/files_list_request_runner_unittest.cc', 'drive/request_sender_unittest.cc', 'drive/request_util_unittest.cc', 'drive/time_util_unittest.cc', |