From fed98f83bb07745261f668a105a6cee7f1c2ffa0 Mon Sep 17 00:00:00 2001 From: "hashimoto@chromium.org" Date: Fri, 28 Mar 2014 17:20:55 +0000 Subject: [PATCH] drive: Make FileResource copyable To implement FakeDriveService without GData WAPI classes. BUG=334544 TEST=unit_tests Review URL: https://codereview.chromium.org/216433002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260182 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/drive/drive_api_util.cc | 16 ++--- .../browser/drive/drive_api_util_unittest.cc | 8 +-- chrome/browser/drive/fake_drive_service.cc | 66 +++++++++---------- .../drive_backend/drive_backend_util.cc | 4 +- .../metadata_database_unittest.cc | 9 +-- .../sync_engine_initializer_unittest.cc | 2 +- google_apis/drive/drive_api_parser.cc | 28 +++++++- google_apis/drive/drive_api_parser.h | 20 ++---- .../drive/drive_api_parser_unittest.cc | 10 +-- 9 files changed, 83 insertions(+), 80 deletions(-) diff --git a/chrome/browser/drive/drive_api_util.cc b/chrome/browser/drive/drive_api_util.cc index ecadaad97dc4ae..cf8076694abd9d 100644 --- a/chrome/browser/drive/drive_api_util.cc +++ b/chrome/browser/drive/drive_api_util.cc @@ -343,21 +343,20 @@ scoped_ptr ConvertResourceEntryToFileResource( image_media_metadata->set_height(entry.image_height()); image_media_metadata->set_rotation(entry.image_rotation()); - ScopedVector parents; + std::vector* parents = file->mutable_parents(); for (size_t i = 0; i < entry.links().size(); ++i) { using google_apis::Link; const Link& link = *entry.links()[i]; switch (link.type()) { case Link::LINK_PARENT: { - scoped_ptr parent( - new google_apis::ParentReference); - parent->set_parent_link(link.href()); + google_apis::ParentReference parent; + parent.set_parent_link(link.href()); std::string file_id = drive::util::ExtractResourceIdFromUrl(link.href()); - parent->set_file_id(file_id); - parent->set_is_root(file_id == kWapiRootDirectoryResourceId); - parents.push_back(parent.release()); + parent.set_file_id(file_id); + parent.set_is_root(file_id == kWapiRootDirectoryResourceId); + parents->push_back(parent); break; } case Link::LINK_ALTERNATE: @@ -367,7 +366,6 @@ scoped_ptr ConvertResourceEntryToFileResource( break; } } - file->set_parents(parents.Pass()); file->set_modified_date(entry.updated_time()); file->set_last_viewed_by_me_date(entry.last_viewed_time()); @@ -449,7 +447,7 @@ ConvertFileResourceToResourceEntry( for (size_t i = 0; i < file_resource.parents().size(); ++i) { google_apis::Link* link = new google_apis::Link; link->set_type(google_apis::Link::LINK_PARENT); - link->set_href(file_resource.parents()[i]->parent_link()); + link->set_href(file_resource.parents()[i].parent_link()); links.push_back(link); } if (!file_resource.alternate_link().is_empty()) { diff --git a/chrome/browser/drive/drive_api_util_unittest.cc b/chrome/browser/drive/drive_api_util_unittest.cc index afb4a131694e6c..af405f332c83b4 100644 --- a/chrome/browser/drive/drive_api_util_unittest.cc +++ b/chrome/browser/drive/drive_api_util_unittest.cc @@ -202,13 +202,11 @@ TEST(FileSystemUtilTest, ConvertFileResourceToResource_Parents) { expected_links.push_back(GURL("http://server/id2")); expected_links.push_back(GURL("http://server/id3")); - ScopedVector parents; for (size_t i = 0; i < expected_links.size(); ++i) { - google_apis::ParentReference* parent = new google_apis::ParentReference; - parent->set_parent_link(expected_links[i]); - parents.push_back(parent); + google_apis::ParentReference parent; + parent.set_parent_link(expected_links[i]); + file_resource.mutable_parents()->push_back(parent); } - file_resource.set_parents(parents.Pass()); scoped_ptr entry( ConvertFileResourceToResourceEntry(file_resource)); diff --git a/chrome/browser/drive/fake_drive_service.cc b/chrome/browser/drive/fake_drive_service.cc index 2c147c0f5bfe02..393172776ffb22 100644 --- a/chrome/browser/drive/fake_drive_service.cc +++ b/chrome/browser/drive/fake_drive_service.cc @@ -724,12 +724,8 @@ CancelCallback FakeDriveService::CopyResource( scoped_ptr copied_entry(new EntryInfo); copied_entry->content_data = entry->content_data; copied_entry->share_url = entry->share_url; - - // TODO(hashimoto): Implement a proper way to copy FileResource. - scoped_ptr copied_resource_entry = - util::ConvertChangeResourceToResourceEntry(entry->change_resource); copied_entry->change_resource.set_file( - util::ConvertResourceEntryToFileResource(*copied_resource_entry)); + make_scoped_ptr(new FileResource(*entry->change_resource.file()))); ChangeResource* new_change = &copied_entry->change_resource; FileResource* new_file = new_change->mutable_file(); @@ -738,13 +734,13 @@ CancelCallback FakeDriveService::CopyResource( new_file->set_file_id(new_resource_id); new_file->set_title(new_title); - scoped_ptr parent(new ParentReference); - parent->set_file_id(parent_resource_id); - parent->set_parent_link(GetFakeLinkUrl(parent_resource_id)); - parent->set_is_root(parent_resource_id == GetRootResourceId()); - ScopedVector parents; - parents.push_back(parent.release()); - new_file->set_parents(parents.Pass()); + ParentReference parent; + parent.set_file_id(parent_resource_id); + parent.set_parent_link(GetFakeLinkUrl(parent_resource_id)); + parent.set_is_root(parent_resource_id == GetRootResourceId()); + std::vector parents; + parents.push_back(parent); + *new_file->mutable_parents() = parents; if (!last_modified.is_null()) new_file->set_modified_date(last_modified); @@ -797,14 +793,14 @@ CancelCallback FakeDriveService::UpdateResource( // Set parent if necessary. if (!parent_resource_id.empty()) { - scoped_ptr parent(new ParentReference); - parent->set_file_id(parent_resource_id); - parent->set_parent_link(GetFakeLinkUrl(parent_resource_id)); - parent->set_is_root(parent_resource_id == GetRootResourceId()); - - ScopedVector parents; - parents.push_back(parent.release()); - file->set_parents(parents.Pass()); + ParentReference parent; + parent.set_file_id(parent_resource_id); + parent.set_parent_link(GetFakeLinkUrl(parent_resource_id)); + parent.set_is_root(parent_resource_id == GetRootResourceId()); + + std::vector parents; + parents.push_back(parent); + *file->mutable_parents() = parents; } if (!last_modified.is_null()) @@ -863,11 +859,11 @@ CancelCallback FakeDriveService::AddResourceToDirectory( // structure. That is, each resource can have multiple parent. // We mimic the behavior here; AddResourceToDirectoy just adds // one more parent, not overwriting old ones. - scoped_ptr parent(new ParentReference); - parent->set_file_id(parent_resource_id); - parent->set_parent_link(GetFakeLinkUrl(parent_resource_id)); - parent->set_is_root(parent_resource_id == GetRootResourceId()); - change->mutable_file()->mutable_parents()->push_back(parent.release()); + ParentReference parent; + parent.set_file_id(parent_resource_id); + parent.set_parent_link(GetFakeLinkUrl(parent_resource_id)); + parent.set_is_root(parent_resource_id == GetRootResourceId()); + change->mutable_file()->mutable_parents()->push_back(parent); AddNewChangestamp(change); base::MessageLoop::current()->PostTask( @@ -897,9 +893,9 @@ CancelCallback FakeDriveService::RemoveResourceFromDirectory( if (entry) { ChangeResource* change = &entry->change_resource; FileResource* file = change->mutable_file(); - ScopedVector* parents = file->mutable_parents(); + std::vector* parents = file->mutable_parents(); for (size_t i = 0; i < parents->size(); ++i) { - if ((*parents)[i]->file_id() == parent_resource_id) { + if ((*parents)[i].file_id() == parent_resource_id) { parents->erase(parents->begin() + i); AddNewChangestamp(change); base::MessageLoop::current()->PostTask( @@ -1422,16 +1418,16 @@ const FakeDriveService::EntryInfo* FakeDriveService::AddNewEntry( new_file->set_mime_type(content_type); // Set parents. - scoped_ptr parent(new ParentReference); + ParentReference parent; if (parent_resource_id.empty()) - parent->set_file_id(GetRootResourceId()); + parent.set_file_id(GetRootResourceId()); else - parent->set_file_id(parent_resource_id); - parent->set_parent_link(GetFakeLinkUrl(parent->file_id())); - parent->set_is_root(parent->file_id() == GetRootResourceId()); - ScopedVector parents; - parents.push_back(parent.release()); - new_file->set_parents(parents.Pass()); + parent.set_file_id(parent_resource_id); + parent.set_parent_link(GetFakeLinkUrl(parent.file_id())); + parent.set_is_root(parent.file_id() == GetRootResourceId()); + std::vector parents; + parents.push_back(parent); + *new_file->mutable_parents() = parents; new_entry->share_url = net::AppendOrReplaceQueryParameter( share_url_base_, "name", title); diff --git a/chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc b/chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc index aee5b4b772f90e..a53b9566770bd8 100644 --- a/chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc +++ b/chrome/browser/sync_file_system/drive_backend/drive_backend_util.cc @@ -68,11 +68,11 @@ void PopulateFileDetailsByFileResource( const google_apis::FileResource& file_resource, FileDetails* details) { details->clear_parent_folder_ids(); - for (ScopedVector::const_iterator itr = + for (std::vector::const_iterator itr = file_resource.parents().begin(); itr != file_resource.parents().end(); ++itr) { - details->add_parent_folder_ids((*itr)->file_id()); + details->add_parent_folder_ids(itr->file_id()); } details->set_title(file_resource.title()); diff --git a/chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc b/chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc index 81d919a67fdaa4..df703d1acc267e 100644 --- a/chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc @@ -376,16 +376,13 @@ class MetadataDatabaseTest : public testing::Test { const FileMetadata& file) { scoped_ptr file_resource( new google_apis::FileResource); - ScopedVector parents; for (int i = 0; i < file.details().parent_folder_ids_size(); ++i) { - scoped_ptr parent( - new google_apis::ParentReference); - parent->set_file_id(file.details().parent_folder_ids(i)); - parents.push_back(parent.release()); + google_apis::ParentReference parent; + parent.set_file_id(file.details().parent_folder_ids(i)); + file_resource->mutable_parents()->push_back(parent); } file_resource->set_file_id(file.file_id()); - file_resource->set_parents(parents.Pass()); file_resource->set_title(file.details().title()); if (file.details().file_kind() == FILE_KIND_FOLDER) file_resource->set_mime_type("application/vnd.google-apps.folder"); diff --git a/chrome/browser/sync_file_system/drive_backend/sync_engine_initializer_unittest.cc b/chrome/browser/sync_file_system/drive_backend/sync_engine_initializer_unittest.cc index 5d1c4d82b34814..54974776b11b9d 100644 --- a/chrome/browser/sync_file_system/drive_backend/sync_engine_initializer_unittest.cc +++ b/chrome/browser/sync_file_system/drive_backend/sync_engine_initializer_unittest.cc @@ -135,7 +135,7 @@ class SyncEngineInitializerTest : public testing::Test { for (size_t i = 0; i < sync_root->parents().size(); ++i) { google_apis::GDataErrorCode error = google_apis::GDATA_OTHER_ERROR; fake_drive_service_.RemoveResourceFromDirectory( - sync_root->parents()[i]->file_id(), + sync_root->parents()[i].file_id(), sync_root->file_id(), CreateResultReceiver(&error)); base::RunLoop().RunUntilIdle(); diff --git a/google_apis/drive/drive_api_parser.cc b/google_apis/drive/drive_api_parser.cc index 1eefb8bc4fb06e..fff3d9d4301bb2 100644 --- a/google_apis/drive/drive_api_parser.cc +++ b/google_apis/drive/drive_api_parser.cc @@ -38,6 +38,28 @@ bool GetGURLFromString(const base::StringPiece& url_string, GURL* result) { return true; } +// Converts |value| to |result|. +bool GetParentsFromValue(const base::Value* value, + std::vector* result) { + DCHECK(value); + DCHECK(result); + + const base::ListValue* list_value = NULL; + if (!value->GetAsList(&list_value)) + return false; + + base::JSONValueConverter converter; + result->resize(list_value->GetSize()); + for (size_t i = 0; i < list_value->GetSize(); ++i) { + const base::Value* parent_value = NULL; + if (!list_value->Get(i, &parent_value) || + !converter.Convert(*parent_value, &(*result)[i])) + return false; + } + + return true; +} + // Converts |value| to |result|. The key of |value| is app_id, and its value // is URL to open the resource on the web app. bool GetOpenWithLinksFromDictionaryValue( @@ -469,8 +491,10 @@ void FileResource::RegisterJSONConverter( converter->RegisterCustomField(kAlternateLink, &FileResource::alternate_link_, GetGURLFromString); - converter->RegisterRepeatedMessage(kParents, - &FileResource::parents_); + converter->RegisterCustomValueField >( + kParents, + &FileResource::parents_, + GetParentsFromValue); converter->RegisterCustomValueField >( kOpenWithLinks, &FileResource::open_with_links_, diff --git a/google_apis/drive/drive_api_parser.h b/google_apis/drive/drive_api_parser.h index e51b864cefb96b..daa2b4bdb64558 100644 --- a/google_apis/drive/drive_api_parser.h +++ b/google_apis/drive/drive_api_parser.h @@ -365,8 +365,6 @@ class ParentReference { void set_is_root(bool is_root) { is_root_ = is_root; } private: - friend class base::internal::RepeatedMessageConverter; - // Parses and initializes data members from content of |value|. // Return false if parsing fails. bool Parse(const base::Value& value); @@ -374,8 +372,6 @@ class ParentReference { std::string file_id_; GURL parent_link_; bool is_root_; - - DISALLOW_COPY_AND_ASSIGN(ParentReference); }; // FileLabels represents labels for file or folder. @@ -422,8 +418,6 @@ class FileLabels { bool trashed_; bool restricted_; bool viewed_; - - DISALLOW_COPY_AND_ASSIGN(FileLabels); }; // ImageMediaMetadata represents image metadata for a file. @@ -462,8 +456,6 @@ class ImageMediaMetadata { int width_; int height_; int rotation_; - - DISALLOW_COPY_AND_ASSIGN(ImageMediaMetadata); }; @@ -549,8 +541,7 @@ class FileResource { const GURL& alternate_link() const { return alternate_link_; } // Returns parent references (directories) of this file. - const ScopedVector& parents() const { return parents_; } - ScopedVector* mutable_parents() { return &parents_; } + const std::vector& parents() const { return parents_; } // Returns the list of links to open the resource with a web app. const std::vector& open_with_links() const { @@ -605,8 +596,9 @@ class FileResource { void set_alternate_link(const GURL& alternate_link) { alternate_link_ = alternate_link; } - void set_parents(ScopedVector parents) { - parents_ = parents.Pass(); + std::vector* mutable_parents() { return &parents_; } + std::vector* mutable_open_with_links() { + return &open_with_links_; } private: @@ -634,10 +626,8 @@ class FileResource { std::string md5_checksum_; int64 file_size_; GURL alternate_link_; - ScopedVector parents_; + std::vector parents_; std::vector open_with_links_; - - DISALLOW_COPY_AND_ASSIGN(FileResource); }; // FileList represents a collection of files and folders. diff --git a/google_apis/drive/drive_api_parser_unittest.cc b/google_apis/drive/drive_api_parser_unittest.cc index fd9b0c39bc480c..89f4c01fd4767e 100644 --- a/google_apis/drive/drive_api_parser_unittest.cc +++ b/google_apis/drive/drive_api_parser_unittest.cc @@ -169,11 +169,11 @@ TEST(DriveAPIParserTest, FileListParser) { EXPECT_EQ(modified_time, file1.modified_by_me_date()); ASSERT_EQ(1U, file1.parents().size()); - EXPECT_EQ("0B4v7G8yEYAWHYW1OcExsUVZLABC", file1.parents()[0]->file_id()); + EXPECT_EQ("0B4v7G8yEYAWHYW1OcExsUVZLABC", file1.parents()[0].file_id()); EXPECT_EQ(GURL("https://www.googleapis.com/drive/v2/files/" "0B4v7G8yEYAWHYW1OcExsUVZLABC"), - file1.parents()[0]->parent_link()); - EXPECT_FALSE(file1.parents()[0]->is_root()); + file1.parents()[0].parent_link()); + EXPECT_FALSE(file1.parents()[0].is_root()); EXPECT_EQ("ext", file1.file_extension()); EXPECT_EQ("d41d8cd98f00b204e9800998ecf8427e", file1.md5_checksum()); @@ -223,8 +223,8 @@ TEST(DriveAPIParserTest, FileListParser) { EXPECT_FALSE(file3.shared()); ASSERT_EQ(1U, file3.parents().size()); - EXPECT_EQ("0AIv7G8yEYAWHUk9ABC", file3.parents()[0]->file_id()); - EXPECT_TRUE(file3.parents()[0]->is_root()); + EXPECT_EQ("0AIv7G8yEYAWHUk9ABC", file3.parents()[0].file_id()); + EXPECT_TRUE(file3.parents()[0].is_root()); EXPECT_EQ(0U, file3.open_with_links().size()); }