drive: Make FileCache::RenameCacheFilesToNewFormat responsible to canonicalize file name
Move file name canonicalization responsibility from CanonicalizeIDs to RenameCacheFilesToNewFormat
CanonicalizeIDs will be deleted soon when cache entry ID canonicalization code moves to ResourceMetadataStorage.
BUG=309597
TEST=unit_tests
[email protected]
Review URL: https://codereview.chromium.org/32333002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230029 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/drive/file_cache.cc b/chrome/browser/chromeos/drive/file_cache.cc
index 48c5b6e..ca614c32 100644
--- a/chrome/browser/chromeos/drive/file_cache.cc
+++ b/chrome/browser/chromeos/drive/file_cache.cc
@@ -16,6 +16,7 @@
#include "chrome/browser/chromeos/drive/drive.pb.h"
#include "chrome/browser/chromeos/drive/file_system_util.h"
#include "chrome/browser/chromeos/drive/resource_metadata_storage.h"
+#include "chrome/browser/drive/drive_api_util.h"
#include "chromeos/chromeos_constants.h"
#include "content/public/browser/browser_thread.h"
@@ -391,7 +392,8 @@
bool FileCache::Initialize() {
AssertOnSequencedWorkerPool();
- RenameCacheFilesToNewFormat();
+ if (!RenameCacheFilesToNewFormat())
+ return false;
if (storage_->cache_file_scan_is_needed()) {
CacheMap cache_map;
@@ -424,11 +426,8 @@
for (; !it->IsAtEnd(); it->Advance()) {
const std::string id_canonicalized = id_canonicalizer.Run(it->GetID());
if (id_canonicalized != it->GetID()) {
- // Replace the existing entry and rename the file when needed.
- const base::FilePath path_old = GetCacheFilePath(it->GetID());
- const base::FilePath path_new = GetCacheFilePath(id_canonicalized);
+ // Replace the existing entry.
if (!storage_->RemoveCacheEntry(it->GetID()) ||
- (base::PathExists(path_old) && !base::Move(path_old, path_new)) ||
!storage_->PutCacheEntry(id_canonicalized, it->GetValue()))
return false;
}
@@ -525,27 +524,25 @@
return (free_space >= num_bytes);
}
-void FileCache::RenameCacheFilesToNewFormat() {
- // First, remove all files with multiple extensions just in case.
- {
- base::FileEnumerator enumerator(cache_file_directory_,
- false, // not recursive
- base::FileEnumerator::FILES,
- "*.*.*");
- for (base::FilePath current = enumerator.Next(); !current.empty();
- current = enumerator.Next())
- base::DeleteFile(current, false /* recursive */);
+bool FileCache::RenameCacheFilesToNewFormat() {
+ base::FileEnumerator enumerator(cache_file_directory_,
+ false, // not recursive
+ base::FileEnumerator::FILES);
+ for (base::FilePath current = enumerator.Next(); !current.empty();
+ current = enumerator.Next()) {
+ base::FilePath new_path = current.RemoveExtension();
+ if (!new_path.Extension().empty()) {
+ // Delete files with multiple extensions.
+ if (!base::DeleteFile(current, false /* recursive */))
+ return false;
+ continue;
+ }
+ const std::string& id = GetIdFromPath(new_path);
+ new_path = GetCacheFilePath(util::CanonicalizeResourceId(id));
+ if (new_path != current && !base::Move(current, new_path))
+ return false;
}
-
- // Rename files.
- {
- base::FileEnumerator enumerator(cache_file_directory_,
- false, // not recursive
- base::FileEnumerator::FILES);
- for (base::FilePath current = enumerator.Next(); !current.empty();
- current = enumerator.Next())
- base::Move(current, current.RemoveExtension());
- }
+ return true;
}
} // namespace internal
diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h
index 06b61ae8..ba6aa1a 100644
--- a/chrome/browser/chromeos/drive/file_cache.h
+++ b/chrome/browser/chromeos/drive/file_cache.h
@@ -193,9 +193,9 @@
// bytes, while keeping kMinFreeSpace bytes on the disk.
bool HasEnoughSpaceFor(int64 num_bytes, const base::FilePath& path);
- // Renames cache files from old "id.md5" format to the new format.
+ // Renames cache files from old "prefix:id.md5" format to the new format.
// TODO(hashimoto): Remove this method at some point.
- void RenameCacheFilesToNewFormat();
+ bool RenameCacheFilesToNewFormat();
const base::FilePath cache_file_directory_;
diff --git a/chrome/browser/chromeos/drive/file_cache_unittest.cc b/chrome/browser/chromeos/drive/file_cache_unittest.cc
index e4d98b5d..d2c8810 100644
--- a/chrome/browser/chromeos/drive/file_cache_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc
@@ -738,8 +738,8 @@
ASSERT_TRUE(cache_->Initialize());
}
- static void RenameCacheFilesToNewFormat(FileCache* cache) {
- cache->RenameCacheFilesToNewFormat();
+ static bool RenameCacheFilesToNewFormat(FileCache* cache) {
+ return cache->RenameCacheFilesToNewFormat();
}
content::TestBrowserThreadBundle thread_bundle_;
@@ -909,7 +909,6 @@
EXPECT_TRUE(file_util::CreateTemporaryFileInDir(temp_dir_.path(), &file));
EXPECT_EQ(FILE_ERROR_OK,
cache_->Store(id, md5, file, FileCache::FILE_OPERATION_COPY));
- EXPECT_TRUE(base::PathExists(file_directory.AppendASCII(id)));
// Canonicalize IDs.
EXPECT_TRUE(cache_->CanonicalizeIDs(id_canonicalizer));
@@ -918,16 +917,15 @@
FileCacheEntry entry;
EXPECT_FALSE(cache_->GetCacheEntry(id, &entry));
EXPECT_TRUE(cache_->GetCacheEntry(canonicalized_id, &entry));
- EXPECT_TRUE(base::PathExists(file_directory.AppendASCII(canonicalized_id)));
}
TEST_F(FileCacheTest, RenameCacheFilesToNewFormat) {
const base::FilePath file_directory =
temp_dir_.path().AppendASCII(kCacheFileDirectory);
- // File with an old style "<ID>.<MD5>" name.
+ // File with an old style "<prefix>:<ID>.<MD5>" name.
ASSERT_TRUE(google_apis::test_util::WriteStringToFile(
- file_directory.AppendASCII("id_koo.md5"), "koo"));
+ file_directory.AppendASCII("file:id_koo.md5"), "koo"));
// File with multiple extensions should be removed.
ASSERT_TRUE(google_apis::test_util::WriteStringToFile(
@@ -936,27 +934,27 @@
file_directory.AppendASCII("id_kyu.md5"), "kyu"));
// Rename and verify the result.
- RenameCacheFilesToNewFormat(cache_.get());
+ EXPECT_TRUE(RenameCacheFilesToNewFormat(cache_.get()));
std::string contents;
EXPECT_TRUE(base::ReadFileToString(file_directory.AppendASCII("id_koo"),
- &contents));
+ &contents));
EXPECT_EQ("koo", contents);
contents.clear();
EXPECT_TRUE(base::ReadFileToString(file_directory.AppendASCII("id_kyu"),
- &contents));
+ &contents));
EXPECT_EQ("kyu", contents);
// Rename again.
- RenameCacheFilesToNewFormat(cache_.get());
+ EXPECT_TRUE(RenameCacheFilesToNewFormat(cache_.get()));
// Files with new style names are not affected.
contents.clear();
EXPECT_TRUE(base::ReadFileToString(file_directory.AppendASCII("id_koo"),
- &contents));
+ &contents));
EXPECT_EQ("koo", contents);
contents.clear();
EXPECT_TRUE(base::ReadFileToString(file_directory.AppendASCII("id_kyu"),
- &contents));
+ &contents));
EXPECT_EQ("kyu", contents);
}
