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
R=kinaba@chromium.org

Review URL: https://codereview.chromium.org/32333002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230029 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 34 insertions, 39 deletions

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::ClearAll() {
 bool FileCache::Initialize() {
   AssertOnSequencedWorkerPool();
 
-  RenameCacheFilesToNewFormat();
+  if (!RenameCacheFilesToNewFormat())
+    return false;
 
   if (storage_->cache_file_scan_is_needed()) {
     CacheMap cache_map;
@@ -424,11 +426,8 @@ bool FileCache::CanonicalizeIDs(
   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 @@ bool FileCache::HasEnoughSpaceFor(int64 num_bytes,
   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 */);
-  }
-
-  // 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());
+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;
   }
+  return true;
 }
 
 }  // namespace internal
diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h
index 06b61ae..ba6aa1a 100644
--- a/chrome/browser/chromeos/drive/file_cache.h
+++ b/chrome/browser/chromeos/drive/file_cache.h
@@ -193,9 +193,9 @@ class FileCache {
   // 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 e4d98b5..d2c8810 100644
--- a/chrome/browser/chromeos/drive/file_cache_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc
@@ -738,8 +738,8 @@ class FileCacheTest : public testing::Test {
     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 @@ TEST_F(FileCacheTest, CanonicalizeIDs) {
   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 @@ TEST_F(FileCacheTest, CanonicalizeIDs) {
   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 @@ TEST_F(FileCacheTest, RenameCacheFilesToNewFormat) {
       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);
 }