Skip to content

Commit

Permalink
Use new API to download blobs from Google Drive.
Browse files Browse the repository at this point in the history
The URL to download them is changed from
https://www.googledrive.com/p/host/{resourceId}
to
https://www.googleapis.com/drive/v2/files/{resourceId}?alt=media

BUG=496969
TEST=run google_apis_unittests

Review-Url: https://codereview.chromium.org/1965963003
Cr-Commit-Position: refs/heads/master@{#393206}
  • Loading branch information
fukino authored and Commit bot committed May 12, 2016
1 parent 874db6f commit 5ef444b
Show file tree
Hide file tree
Showing 12 changed files with 21 additions and 43 deletions.
1 change: 0 additions & 1 deletion chrome/browser/apps/drive/drive_service_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ void DriveServiceBridgeImpl::Initialize() {
profile_->GetRequestContext(),
drive_task_runner.get(),
GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseDownloadUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction),
std::string() /* custom_user_agent */));
SigninManagerBase* signin_manager =
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chromeos/drive/drive_integration_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ DriveIntegrationService::DriveIntegrationService(
oauth_service, g_browser_process->system_request_context(),
blocking_task_runner_.get(),
GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseDownloadUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction),
GetDriveUserAgent()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ void FillEntryPropertiesValueForDrive(const drive::ResourceEntry& entry_proto,
if (!entry_proto.resource_id().empty()) {
DriveApiUrlGenerator url_generator(
(GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction)),
(GURL(
google_apis::DriveApiUrlGenerator::kBaseDownloadUrlForProduction)),
(GURL(google_apis::DriveApiUrlGenerator::
kBaseThumbnailUrlForProduction)));
properties->thumbnail_url.reset(new std::string(
Expand Down Expand Up @@ -1105,7 +1103,6 @@ void FileManagerPrivateInternalGetDownloadUrlFunction::OnGetResourceEntry(

DriveApiUrlGenerator url_generator(
(GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction)),
(GURL(google_apis::DriveApiUrlGenerator::kBaseDownloadUrlForProduction)),
(GURL(
google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction)));
download_url_ = url_generator.GenerateDownloadFileUrl(entry->resource_id());
Expand Down Expand Up @@ -1138,7 +1135,7 @@ void FileManagerPrivateInternalGetDownloadUrlFunction::OnTokenFetched(
}

const std::string url =
download_url_.Resolve("?access_token=" + access_token).spec();
download_url_.Resolve("?alt=media&access_token=" + access_token).spec();
SetResult(new base::StringValue(url));

SendResponse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ SyncEngine::DriveServiceFactory::CreateDriveService(
drive::DriveServiceInterface>(new drive::DriveAPIService(
oauth2_token_service, url_request_context_getter, blocking_task_runner,
GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseDownloadUrlForProduction),
GURL(google_apis::DriveApiUrlGenerator::kBaseThumbnailUrlForProduction),
std::string() /* custom_user_agent */));
}
Expand Down
3 changes: 1 addition & 2 deletions components/drive/service/drive_api_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,12 @@ DriveAPIService::DriveAPIService(
net::URLRequestContextGetter* url_request_context_getter,
base::SequencedTaskRunner* blocking_task_runner,
const GURL& base_url,
const GURL& base_download_url,
const GURL& base_thumbnail_url,
const std::string& custom_user_agent)
: oauth2_token_service_(oauth2_token_service),
url_request_context_getter_(url_request_context_getter),
blocking_task_runner_(blocking_task_runner),
url_generator_(base_url, base_download_url, base_thumbnail_url),
url_generator_(base_url, base_thumbnail_url),
custom_user_agent_(custom_user_agent) {
}

Expand Down
3 changes: 0 additions & 3 deletions components/drive/service/drive_api_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ class DriveAPIService : public DriveServiceInterface,
// |url_request_context_getter| is used to initialize URLFetcher.
// |blocking_task_runner| is used to run blocking tasks (like parsing JSON).
// |base_url| is used to generate URLs for communication with the drive API.
// |base_download_url| is used to generate URLs for downloading file from the
// drive API.
// |base_thumbnail_url| is used to generate URLs for downloading thumbnail
// from image server.
// |custom_user_agent| will be used for the User-Agent header in HTTP
Expand All @@ -105,7 +103,6 @@ class DriveAPIService : public DriveServiceInterface,
net::URLRequestContextGetter* url_request_context_getter,
base::SequencedTaskRunner* blocking_task_runner,
const GURL& base_url,
const GURL& base_download_url,
const GURL& base_thumbnail_url,
const std::string& custom_user_agent);
~DriveAPIService() override;
Expand Down
3 changes: 1 addition & 2 deletions components/drive/service/drive_api_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class TestAuthService : public google_apis::DummyAuthService {

TEST(DriveAPIServiceTest, BatchRequestConfiguratorWithAuthFailure) {
const GURL test_base_url("http://localhost/");
google_apis::DriveApiUrlGenerator url_generator(
test_base_url, test_base_url, test_base_url);
google_apis::DriveApiUrlGenerator url_generator(test_base_url, test_base_url);
scoped_refptr<base::TestSimpleTaskRunner> task_runner =
new base::TestSimpleTaskRunner();
scoped_refptr<net::TestURLRequestContextGetter> request_context_getter =
Expand Down
17 changes: 10 additions & 7 deletions google_apis/drive/drive_api_requests_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ const char kTestPermissionResponse[] =

const char kTestUploadExistingFilePath[] = "/upload/existingfile/path";
const char kTestUploadNewFilePath[] = "/upload/newfile/path";
const char kTestDownloadPathPrefix[] = "/host/";
const char kTestDownloadPathPrefix[] = "/drive/v2/files/";
const char kTestDownloadFileQuery[] = "alt=media";

// Used as a GetContentCallback.
void AppendContent(std::string* out,
Expand Down Expand Up @@ -167,7 +168,7 @@ class DriveApiRequestsTest : public testing::Test {

GURL test_base_url = test_util::GetBaseUrlForTesting(test_server_.port());
url_generator_.reset(
new DriveApiUrlGenerator(test_base_url, test_base_url, test_base_url));
new DriveApiUrlGenerator(test_base_url, test_base_url));

// Reset the server's expected behavior just in case.
ResetExpectedResponse();
Expand Down Expand Up @@ -442,9 +443,9 @@ class DriveApiRequestsTest : public testing::Test {

const GURL absolute_url = test_server_.GetURL(request.relative_url);
std::string id;
if (!test_util::RemovePrefix(absolute_url.path(),
kTestDownloadPathPrefix,
&id)) {
if (!test_util::RemovePrefix(
absolute_url.path(), kTestDownloadPathPrefix, &id) ||
absolute_url.query() != kTestDownloadFileQuery) {
return std::unique_ptr<net::test_server::HttpResponse>();
}

Expand Down Expand Up @@ -1877,7 +1878,8 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest) {

EXPECT_EQ(HTTP_SUCCESS, result_code);
EXPECT_EQ(net::test_server::METHOD_GET, http_request_.method);
EXPECT_EQ(kTestDownloadPathPrefix + kTestId, http_request_.relative_url);
EXPECT_EQ(kTestDownloadPathPrefix + kTestId + "?" + kTestDownloadFileQuery,
http_request_.relative_url);
EXPECT_EQ(kDownloadedFilePath, temp_file);

const std::string expected_contents = kTestId + kTestId + kTestId;
Expand Down Expand Up @@ -1912,7 +1914,8 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest_GetContentCallback) {

EXPECT_EQ(HTTP_SUCCESS, result_code);
EXPECT_EQ(net::test_server::METHOD_GET, http_request_.method);
EXPECT_EQ(kTestDownloadPathPrefix + kTestId, http_request_.relative_url);
EXPECT_EQ(kTestDownloadPathPrefix + kTestId + "?" + kTestDownloadFileQuery,
http_request_.relative_url);
EXPECT_EQ(kDownloadedFilePath, temp_file);

const std::string expected_contents = kTestId + kTestId + kTestId;
Expand Down
13 changes: 2 additions & 11 deletions google_apis/drive/drive_api_url_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const char kDriveV2UploadNewFileUrl[] = "upload/drive/v2/files";
const char kDriveV2UploadExistingFileUrlPrefix[] = "upload/drive/v2/files/";
const char kDriveV2BatchUploadUrl[] = "upload/drive";
const char kDriveV2PermissionsUrlFormat[] = "drive/v2/files/%s/permissions";
const char kDriveV2DownloadUrlFormat[] = "host/%s";
const char kDriveV2DownloadUrlFormat[] = "drive/v2/files/%s?alt=media";
const char kDriveV2ThumbnailUrlFormat[] = "d/%s=w%d-h%d";
const char kDriveV2ThumbnailUrlWithCropFormat[] = "d/%s=w%d-h%d-c";

Expand All @@ -54,10 +54,8 @@ GURL AddMultipartUploadParam(const GURL& url) {
} // namespace

DriveApiUrlGenerator::DriveApiUrlGenerator(const GURL& base_url,
const GURL& base_download_url,
const GURL& base_thumbnail_url)
: base_url_(base_url),
base_download_url_(base_download_url),
base_thumbnail_url_(base_thumbnail_url) {
// Do nothing.
}
Expand All @@ -69,13 +67,6 @@ DriveApiUrlGenerator::~DriveApiUrlGenerator() {
const char DriveApiUrlGenerator::kBaseUrlForProduction[] =
"https://www.googleapis.com";

const char DriveApiUrlGenerator::kBaseDownloadUrlForProduction[] =
#if defined(GOOGLE_CHROME_BUILD) || defined(USE_OFFICIAL_GOOGLE_API_KEYS)
"https://www.googledrive.com/p/";
#else
"https://www.googledrive.com";
#endif

const char DriveApiUrlGenerator::kBaseThumbnailUrlForProduction[] =
"https://lh3.googleusercontent.com";

Expand Down Expand Up @@ -289,7 +280,7 @@ GURL DriveApiUrlGenerator::GetMultipartUploadExistingFileUrl(

GURL DriveApiUrlGenerator::GenerateDownloadFileUrl(
const std::string& resource_id) const {
return base_download_url_.Resolve(base::StringPrintf(
return base_url_.Resolve(base::StringPrintf(
kDriveV2DownloadUrlFormat, net::EscapePath(resource_id).c_str()));
}

Expand Down
4 changes: 0 additions & 4 deletions google_apis/drive/drive_api_url_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@ class DriveApiUrlGenerator {
// |base_url| is the path to the target drive api server.
// Note that this is an injecting point for a testing server.
DriveApiUrlGenerator(const GURL& base_url,
const GURL& base_download_url,
const GURL& base_thumbnail_url);
~DriveApiUrlGenerator();

// The base URL for communicating with the production drive api server.
static const char kBaseUrlForProduction[];

// The base URL for the file download server for production.
static const char kBaseDownloadUrlForProduction[];

// The base URL for the thumbnail download server for production.
static const char kBaseThumbnailUrlForProduction[];

Expand Down
12 changes: 6 additions & 6 deletions google_apis/drive/drive_api_url_generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ namespace {
// The URLs used for production may be different for Chromium OS and Chrome
// OS, so use testing base urls.
const char kBaseUrlForTesting[] = "https://www.example.com";
const char kBaseDownloadUrlForTesting[] = "https://download.example.com/p/";
const char kBaseThumbnailUrlForTesting[] = "https://thumbnail.example.com";
} // namespace

class DriveApiUrlGeneratorTest : public testing::Test {
public:
DriveApiUrlGeneratorTest()
: url_generator_(GURL(kBaseUrlForTesting),
GURL(kBaseDownloadUrlForTesting),
GURL(kBaseThumbnailUrlForTesting)) {}

protected:
Expand Down Expand Up @@ -362,10 +360,12 @@ TEST_F(DriveApiUrlGeneratorTest, GetMultipartUploadExistingFileUrl) {
}

TEST_F(DriveApiUrlGeneratorTest, GenerateDownloadFileUrl) {
EXPECT_EQ("https://download.example.com/p/host/resourceId",
url_generator_.GenerateDownloadFileUrl("resourceId").spec());
EXPECT_EQ("https://download.example.com/p/host/file%3AresourceId",
url_generator_.GenerateDownloadFileUrl("file:resourceId").spec());
EXPECT_EQ(
"https://www.example.com/drive/v2/files/resourceId?alt=media",
url_generator_.GenerateDownloadFileUrl("resourceId").spec());
EXPECT_EQ(
"https://www.example.com/drive/v2/files/file%3AresourceId?alt=media",
url_generator_.GenerateDownloadFileUrl("file:resourceId").spec());
}

TEST_F(DriveApiUrlGeneratorTest, GeneratePermissionsInsertUrl) {
Expand Down
1 change: 0 additions & 1 deletion google_apis/drive/files_list_request_runner_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class FilesListRequestRunnerTest : public testing::Test {
runner_.reset(new FilesListRequestRunner(
request_sender_.get(),
google_apis::DriveApiUrlGenerator(test_server_.base_url(),
test_server_.GetURL("/download/"),
test_server_.GetURL("/thumbnail/"))));
}

Expand Down

0 comments on commit 5ef444b

Please sign in to comment.