drive: Use UploadNewFile from EntryUpdatePerformer
Add UploadNewFileOptions argument to JobScheduler::UploadNewFile().
Pass ChangeListLoader* to SyncClient and EntryUpdatePerformer to lock the loader while creating new files.
Upload an empty file if the locally created file doesn't have a cache file.
BUG=260539
TEST=unit_tests
[email protected]
Review URL: https://codereview.chromium.org/148233006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247660 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/drive/file_system.cc b/chrome/browser/chromeos/drive/file_system.cc
index d2957c8..24af2b3 100644
--- a/chrome/browser/chromeos/drive/file_system.cc
+++ b/chrome/browser/chromeos/drive/file_system.cc
@@ -307,19 +307,20 @@
cache_,
temporary_file_directory_));
- sync_client_.reset(new internal::SyncClient(blocking_task_runner_.get(),
- observer,
- scheduler_,
- resource_metadata_,
- cache_,
- temporary_file_directory_));
-
change_list_loader_.reset(new internal::ChangeListLoader(
blocking_task_runner_.get(),
resource_metadata_,
scheduler_,
drive_service_));
change_list_loader_->AddObserver(this);
+
+ sync_client_.reset(new internal::SyncClient(blocking_task_runner_.get(),
+ observer,
+ scheduler_,
+ resource_metadata_,
+ cache_,
+ change_list_loader_.get(),
+ temporary_file_directory_));
}
void FileSystem::CheckForUpdates() {
diff --git a/chrome/browser/chromeos/drive/file_system.h b/chrome/browser/chromeos/drive/file_system.h
index 28f608a..9d13730 100644
--- a/chrome/browser/chromeos/drive/file_system.h
+++ b/chrome/browser/chromeos/drive/file_system.h
@@ -255,11 +255,11 @@
// Error of the last update check.
FileError last_update_check_error_;
- scoped_ptr<internal::SyncClient> sync_client_;
-
// The loader is used to load the change lists.
scoped_ptr<internal::ChangeListLoader> change_list_loader_;
+ scoped_ptr<internal::SyncClient> sync_client_;
+
ObserverList<FileSystemObserver> observers_;
scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
diff --git a/chrome/browser/chromeos/drive/file_system/operation_test_base.h b/chrome/browser/chromeos/drive/file_system/operation_test_base.h
index 4707f2b..e47c85d 100644
--- a/chrome/browser/chromeos/drive/file_system/operation_test_base.h
+++ b/chrome/browser/chromeos/drive/file_system/operation_test_base.h
@@ -117,6 +117,9 @@
return fake_free_disk_space_getter_.get();
}
internal::FileCache* cache() { return cache_.get(); }
+ internal::ChangeListLoader* change_list_loader() {
+ return change_list_loader_.get();
+ }
private:
content::TestBrowserThreadBundle thread_bundle_;
diff --git a/chrome/browser/chromeos/drive/job_scheduler.cc b/chrome/browser/chromeos/drive/job_scheduler.cc
index 68c666fd..3163d565 100644
--- a/chrome/browser/chromeos/drive/job_scheduler.cc
+++ b/chrome/browser/chromeos/drive/job_scheduler.cc
@@ -73,6 +73,7 @@
base::FilePath local_file_path;
std::string title;
std::string content_type;
+ drive::DriveUploader::UploadNewFileOptions options;
UploadCompletionCallback callback;
google_apis::ProgressCallback progress_callback;
};
@@ -85,7 +86,7 @@
params.local_file_path,
params.title,
params.content_type,
- DriveUploader::UploadNewFileOptions(),
+ params.options,
params.callback,
params.progress_callback);
}
@@ -618,6 +619,7 @@
const base::FilePath& local_file_path,
const std::string& title,
const std::string& content_type,
+ const drive::DriveUploader::UploadNewFileOptions& options,
const ClientContext& context,
const google_apis::GetResourceEntryCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -631,6 +633,7 @@
params.local_file_path = local_file_path;
params.title = title;
params.content_type = content_type;
+ params.options = options;
ResumeUploadParams resume_params;
resume_params.local_file_path = params.local_file_path;
diff --git a/chrome/browser/chromeos/drive/job_scheduler.h b/chrome/browser/chromeos/drive/job_scheduler.h
index 3d807e48..81cf0030 100644
--- a/chrome/browser/chromeos/drive/job_scheduler.h
+++ b/chrome/browser/chromeos/drive/job_scheduler.h
@@ -175,6 +175,7 @@
const base::FilePath& local_file_path,
const std::string& title,
const std::string& content_type,
+ const drive::DriveUploader::UploadNewFileOptions& options,
const ClientContext& context,
const google_apis::GetResourceEntryCallback& callback);
diff --git a/chrome/browser/chromeos/drive/job_scheduler_unittest.cc b/chrome/browser/chromeos/drive/job_scheduler_unittest.cc
index fb58dced..c1b123c 100644
--- a/chrome/browser/chromeos/drive/job_scheduler_unittest.cc
+++ b/chrome/browser/chromeos/drive/job_scheduler_unittest.cc
@@ -971,6 +971,7 @@
path,
"dummy title",
"plain/plain",
+ DriveUploader::UploadNewFileOptions(),
ClientContext(BACKGROUND),
google_apis::test_util::CreateCopyResultCallback(&upload_error, &entry));
base::RunLoop().RunUntilIdle();
@@ -1003,6 +1004,7 @@
upload_path,
"dummy title 1",
"text/plain",
+ DriveUploader::UploadNewFileOptions(),
ClientContext(BACKGROUND),
google_apis::test_util::CreateCopyResultCallback(&error1, &entry));
@@ -1019,6 +1021,7 @@
upload_path,
"dummy title 2",
"text/plain",
+ DriveUploader::UploadNewFileOptions(),
ClientContext(BACKGROUND),
google_apis::test_util::CreateCopyResultCallback(&error2, &entry));
@@ -1051,6 +1054,7 @@
upload_path,
"dummy title 1",
"text/plain",
+ DriveUploader::UploadNewFileOptions(),
ClientContext(USER_INITIATED),
google_apis::test_util::CreateCopyResultCallback(&error1, &entry));
@@ -1068,6 +1072,7 @@
upload_path,
"dummy title 2",
"text/plain",
+ DriveUploader::UploadNewFileOptions(),
ClientContext(USER_INITIATED),
google_apis::test_util::CreateCopyResultCallback(&error2, &entry));
diff --git a/chrome/browser/chromeos/drive/sync/entry_update_performer.cc b/chrome/browser/chromeos/drive/sync/entry_update_performer.cc
index 22357d9..e34367d 100644
--- a/chrome/browser/chromeos/drive/sync/entry_update_performer.cc
+++ b/chrome/browser/chromeos/drive/sync/entry_update_performer.cc
@@ -4,6 +4,9 @@
#include "chrome/browser/chromeos/drive/sync/entry_update_performer.h"
+#include "base/callback_helpers.h"
+#include "base/file_util.h"
+#include "chrome/browser/chromeos/drive/change_list_loader.h"
#include "chrome/browser/chromeos/drive/drive.pb.h"
#include "chrome/browser/chromeos/drive/file_cache.h"
#include "chrome/browser/chromeos/drive/file_system_util.h"
@@ -50,11 +53,23 @@
if (local_state->drive_file_path.empty())
return FILE_ERROR_NOT_FOUND;
- // Check if content update is needed or not.
FileCacheEntry cache_entry;
- if (cache->GetCacheEntry(local_id, &cache_entry) &&
- cache_entry.is_dirty() &&
- !cache->IsOpenedForWrite(local_id)) {
+ cache->GetCacheEntry(local_id, &cache_entry);
+ if (!cache_entry.is_present() && local_state->entry.resource_id().empty()) {
+ // Locally created file with no cache file, store an empty file.
+ base::FilePath empty_file;
+ if (!base::CreateTemporaryFile(&empty_file))
+ return FILE_ERROR_FAILED;
+ error = cache->Store(local_id, std::string(), empty_file,
+ FileCache::FILE_OPERATION_MOVE);
+ if (error != FILE_ERROR_OK)
+ return error;
+ if (!cache->GetCacheEntry(local_id, &cache_entry))
+ return FILE_ERROR_NOT_FOUND;
+ }
+
+ // Check if content update is needed or not.
+ if (cache_entry.is_dirty() && !cache->IsOpenedForWrite(local_id)) {
// Update cache entry's MD5 if needed.
if (cache_entry.md5().empty()) {
error = cache->UpdateMd5(local_id);
@@ -114,6 +129,7 @@
}
if (!entry.file_info().is_directory())
entry.mutable_file_specific_info()->set_md5(resource_entry->file_md5());
+ entry.set_resource_id(resource_entry->resource_id());
error = metadata->RefreshEntry(entry);
if (error != FILE_ERROR_OK)
return error;
@@ -136,11 +152,13 @@
file_system::OperationObserver* observer,
JobScheduler* scheduler,
ResourceMetadata* metadata,
- FileCache* cache)
+ FileCache* cache,
+ ChangeListLoader* change_list_loader)
: blocking_task_runner_(blocking_task_runner),
scheduler_(scheduler),
metadata_(metadata),
cache_(cache),
+ change_list_loader_(change_list_loader),
remove_performer_(new RemovePerformer(blocking_task_runner,
observer,
scheduler,
@@ -200,23 +218,44 @@
// Perform content update.
if (local_state->should_content_update) {
- drive::DriveUploader::UploadExistingFileOptions options;
- options.title = local_state->entry.title();
- options.parent_resource_id = local_state->parent_entry.resource_id();
- options.modified_date = last_modified;
- options.last_viewed_by_me_date = last_accessed;
- scheduler_->UploadExistingFile(
- local_state->entry.resource_id(),
- local_state->drive_file_path,
- local_state->cache_file_path,
- local_state->entry.file_specific_info().content_mime_type(),
- options,
- context,
- base::Bind(&EntryUpdatePerformer::UpdateEntryAfterUpdateResource,
- weak_ptr_factory_.GetWeakPtr(),
- context,
- callback,
- local_state->entry.local_id()));
+ if (local_state->entry.resource_id().empty()) {
+ drive::DriveUploader::UploadNewFileOptions options;
+ options.modified_date = last_modified;
+ options.last_viewed_by_me_date = last_accessed;
+ scheduler_->UploadNewFile(
+ local_state->parent_entry.resource_id(),
+ local_state->drive_file_path,
+ local_state->cache_file_path,
+ local_state->entry.title(),
+ local_state->entry.file_specific_info().content_mime_type(),
+ options,
+ context,
+ base::Bind(&EntryUpdatePerformer::UpdateEntryAfterUpdateResource,
+ weak_ptr_factory_.GetWeakPtr(),
+ context,
+ callback,
+ local_state->entry.local_id(),
+ base::Passed(change_list_loader_->GetLock())));
+ } else {
+ drive::DriveUploader::UploadExistingFileOptions options;
+ options.title = local_state->entry.title();
+ options.parent_resource_id = local_state->parent_entry.resource_id();
+ options.modified_date = last_modified;
+ options.last_viewed_by_me_date = last_accessed;
+ scheduler_->UploadExistingFile(
+ local_state->entry.resource_id(),
+ local_state->drive_file_path,
+ local_state->cache_file_path,
+ local_state->entry.file_specific_info().content_mime_type(),
+ options,
+ context,
+ base::Bind(&EntryUpdatePerformer::UpdateEntryAfterUpdateResource,
+ weak_ptr_factory_.GetWeakPtr(),
+ context,
+ callback,
+ local_state->entry.local_id(),
+ base::Passed(scoped_ptr<base::ScopedClosureRunner>())));
+ }
return;
}
@@ -233,13 +272,15 @@
context,
base::Bind(&EntryUpdatePerformer::UpdateEntryAfterUpdateResource,
weak_ptr_factory_.GetWeakPtr(),
- context, callback, local_state->entry.local_id()));
+ context, callback, local_state->entry.local_id(),
+ base::Passed(scoped_ptr<base::ScopedClosureRunner>())));
}
void EntryUpdatePerformer::UpdateEntryAfterUpdateResource(
const ClientContext& context,
const FileOperationCallback& callback,
const std::string& local_id,
+ scoped_ptr<base::ScopedClosureRunner> change_list_loader_lock,
google_apis::GDataErrorCode status,
scoped_ptr<google_apis::ResourceEntry> resource_entry) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
diff --git a/chrome/browser/chromeos/drive/sync/entry_update_performer.h b/chrome/browser/chromeos/drive/sync/entry_update_performer.h
index b09a877b..a4a2735 100644
--- a/chrome/browser/chromeos/drive/sync/entry_update_performer.h
+++ b/chrome/browser/chromeos/drive/sync/entry_update_performer.h
@@ -13,6 +13,7 @@
#include "google_apis/drive/gdata_errorcode.h"
namespace base {
+class ScopedClosureRunner;
class SequencedTaskRunner;
} // namespace base
@@ -32,6 +33,7 @@
namespace internal {
+class ChangeListLoader;
class EntryRevertPerformer;
class FileCache;
class RemovePerformer;
@@ -44,7 +46,8 @@
file_system::OperationObserver* observer,
JobScheduler* scheduler,
ResourceMetadata* metadata,
- FileCache* cache);
+ FileCache* cache,
+ ChangeListLoader* change_list_loader);
~EntryUpdatePerformer();
// Requests the server to update the metadata of the entry specified by
@@ -69,6 +72,7 @@
const ClientContext& context,
const FileOperationCallback& callback,
const std::string& local_id,
+ scoped_ptr<base::ScopedClosureRunner> change_list_loader_lock,
google_apis::GDataErrorCode status,
scoped_ptr<google_apis::ResourceEntry> resource_entry);
@@ -76,6 +80,7 @@
JobScheduler* scheduler_;
ResourceMetadata* metadata_;
FileCache* cache_;
+ ChangeListLoader* change_list_loader_;
scoped_ptr<RemovePerformer> remove_performer_;
scoped_ptr<EntryRevertPerformer> entry_revert_performer_;
diff --git a/chrome/browser/chromeos/drive/sync/entry_update_performer_unittest.cc b/chrome/browser/chromeos/drive/sync/entry_update_performer_unittest.cc
index ceb69926..8e4bcca 100644
--- a/chrome/browser/chromeos/drive/sync/entry_update_performer_unittest.cc
+++ b/chrome/browser/chromeos/drive/sync/entry_update_performer_unittest.cc
@@ -25,12 +25,13 @@
class EntryUpdatePerformerTest : public file_system::OperationTestBase {
protected:
virtual void SetUp() OVERRIDE {
- OperationTestBase::SetUp();
- performer_.reset(new EntryUpdatePerformer(blocking_task_runner(),
- observer(),
- scheduler(),
- metadata(),
- cache()));
+ OperationTestBase::SetUp();
+ performer_.reset(new EntryUpdatePerformer(blocking_task_runner(),
+ observer(),
+ scheduler(),
+ metadata(),
+ cache(),
+ change_list_loader()));
}
// Stores |content| to the cache and mark it as dirty.
@@ -360,5 +361,61 @@
EXPECT_FALSE(cache_entry.is_dirty());
}
+TEST_F(EntryUpdatePerformerTest, UpdateEntry_UploadNewFile) {
+ // Create a new file locally.
+ const base::FilePath kFilePath(FILE_PATH_LITERAL("drive/root/New File.txt"));
+
+ ResourceEntry parent;
+ EXPECT_EQ(FILE_ERROR_OK, GetLocalResourceEntry(kFilePath.DirName(), &parent));
+
+ ResourceEntry entry;
+ entry.set_parent_local_id(parent.local_id());
+ entry.set_title(kFilePath.BaseName().AsUTF8Unsafe());
+ entry.mutable_file_specific_info()->set_content_mime_type("text/plain");
+ entry.set_metadata_edit_state(ResourceEntry::DIRTY);
+
+ FileError error = FILE_ERROR_FAILED;
+ std::string local_id;
+ base::PostTaskAndReplyWithResult(
+ blocking_task_runner(),
+ FROM_HERE,
+ base::Bind(&internal::ResourceMetadata::AddEntry,
+ base::Unretained(metadata()),
+ entry,
+ &local_id),
+ google_apis::test_util::CreateCopyResultCallback(&error));
+ test_util::RunBlockingPoolTask();
+ EXPECT_EQ(FILE_ERROR_OK, error);
+
+ // Update. This should result in creating a new file on the server.
+ error = FILE_ERROR_FAILED;
+ performer_->UpdateEntry(
+ local_id,
+ ClientContext(USER_INITIATED),
+ google_apis::test_util::CreateCopyResultCallback(&error));
+ test_util::RunBlockingPoolTask();
+ EXPECT_EQ(FILE_ERROR_OK, error);
+
+ // The entry got a resource ID.
+ EXPECT_EQ(FILE_ERROR_OK, GetLocalResourceEntry(kFilePath, &entry));
+ EXPECT_FALSE(entry.resource_id().empty());
+ EXPECT_EQ(ResourceEntry::CLEAN, entry.metadata_edit_state());
+
+ // Make sure that the cache is no longer dirty.
+ bool success = false;
+ FileCacheEntry cache_entry;
+ base::PostTaskAndReplyWithResult(
+ blocking_task_runner(),
+ FROM_HERE,
+ base::Bind(&FileCache::GetCacheEntry,
+ base::Unretained(cache()),
+ local_id,
+ &cache_entry),
+ google_apis::test_util::CreateCopyResultCallback(&success));
+ test_util::RunBlockingPoolTask();
+ EXPECT_TRUE(success);
+ EXPECT_FALSE(cache_entry.is_dirty());
+}
+
} // namespace internal
} // namespace drive
diff --git a/chrome/browser/chromeos/drive/sync_client.cc b/chrome/browser/chromeos/drive/sync_client.cc
index a679f3f8..a8bddb6 100644
--- a/chrome/browser/chromeos/drive/sync_client.cc
+++ b/chrome/browser/chromeos/drive/sync_client.cc
@@ -136,6 +136,7 @@
JobScheduler* scheduler,
ResourceMetadata* metadata,
FileCache* cache,
+ ChangeListLoader* change_list_loader,
const base::FilePath& temporary_file_directory)
: blocking_task_runner_(blocking_task_runner),
operation_observer_(observer),
@@ -152,7 +153,8 @@
observer,
scheduler,
metadata,
- cache)),
+ cache,
+ change_list_loader)),
delay_(base::TimeDelta::FromSeconds(kDelaySeconds)),
long_delay_(base::TimeDelta::FromSeconds(kLongDelaySeconds)),
weak_ptr_factory_(this) {
diff --git a/chrome/browser/chromeos/drive/sync_client.h b/chrome/browser/chromeos/drive/sync_client.h
index 677ff1a..5f7f811 100644
--- a/chrome/browser/chromeos/drive/sync_client.h
+++ b/chrome/browser/chromeos/drive/sync_client.h
@@ -33,6 +33,7 @@
namespace internal {
+class ChangeListLoader;
class EntryUpdatePerformer;
class FileCache;
class ResourceMetadata;
@@ -50,6 +51,7 @@
JobScheduler* scheduler,
ResourceMetadata* metadata,
FileCache* cache,
+ ChangeListLoader* change_list_loader,
const base::FilePath& temporary_file_directory);
virtual ~SyncClient();
diff --git a/chrome/browser/chromeos/drive/sync_client_unittest.cc b/chrome/browser/chromeos/drive/sync_client_unittest.cc
index 77da5459..4698b96 100644
--- a/chrome/browser/chromeos/drive/sync_client_unittest.cc
+++ b/chrome/browser/chromeos/drive/sync_client_unittest.cc
@@ -135,6 +135,11 @@
NULL /* free_disk_space_getter */));
ASSERT_TRUE(cache_->Initialize());
+ change_list_loader_.reset(new ChangeListLoader(
+ base::MessageLoopProxy::current().get(),
+ metadata_.get(),
+ scheduler_.get(),
+ drive_service_.get()));
ASSERT_NO_FATAL_FAILURE(SetUpTestData());
sync_client_.reset(new SyncClient(base::MessageLoopProxy::current().get(),
@@ -142,6 +147,7 @@
scheduler_.get(),
metadata_.get(),
cache_.get(),
+ change_list_loader_.get(),
temp_dir_.path()));
// Disable delaying so that DoSyncLoop() starts immediately.
@@ -184,12 +190,7 @@
// Load data from the service to the metadata.
FileError error = FILE_ERROR_FAILED;
- internal::ChangeListLoader change_list_loader(
- base::MessageLoopProxy::current().get(),
- metadata_.get(),
- scheduler_.get(),
- drive_service_.get());
- change_list_loader.LoadForTesting(
+ change_list_loader_->LoadForTesting(
google_apis::test_util::CreateCopyResultCallback(&error));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(FILE_ERROR_OK, error);
@@ -256,6 +257,7 @@
test_util::DestroyHelperForTests> metadata_storage_;
scoped_ptr<ResourceMetadata, test_util::DestroyHelperForTests> metadata_;
scoped_ptr<FileCache, test_util::DestroyHelperForTests> cache_;
+ scoped_ptr<ChangeListLoader> change_list_loader_;
scoped_ptr<SyncClient> sync_client_;
std::map<std::string, std::string> resource_ids_; // Name-to-id map.
