Add database recovery for FileSystemDirectoryDatabase.

webkit/fileapi/file_system_directory_database_unittest.cc

Index: webkit/fileapi/file_system_directory_database_unittest.cc
diff --git a/webkit/fileapi/file_system_directory_database_unittest.cc b/webkit/fileapi/file_system_directory_database_unittest.cc
index 2f78449d451a6a74c88e0941a25761bfa7909056..e7429456caba6114f66af41a64ef0306a357001f 100644
--- a/webkit/fileapi/file_system_directory_database_unittest.cc
+++ b/webkit/fileapi/file_system_directory_database_unittest.cc
@@ -6,14 +6,24 @@
 
 #include <math.h>
 
+#include "base/file_util.h"
+#include "base/platform_file.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/fileapi/file_system_util.h"
+#include "webkit/fileapi/file_system_database_test_helper.h"
+
+#define FPL(x) FILE_PATH_LITERAL(x)
 
 namespace fileapi {
 
+namespace {
+const FilePath::CharType kDirectoryDatabaseName[] = FPL("Paths");
+}
+
 class FileSystemDirectoryDatabaseTest : public testing::Test {
  public:
   typedef FileSystemDirectoryDatabase::FileId FileId;
@@ -29,10 +39,14 @@ class FileSystemDirectoryDatabaseTest : public testing::Test {
   }
 
   void InitDatabase() {
-    // First reset() is to avoid multiple database instance for single
+    // Call CloseDatabes() to avoid multiple database instance for single

[ericu, 2012/04/03 01:52:11]

Typo: CloseDatabase.

[tzik, 2012/04/04 06:50:39]

Done.

     // directory at once.
+    CloseDatabase();
+    db_.reset(new FileSystemDirectoryDatabase(path()));
+  }
+
+  void CloseDatabase() {
     db_.reset();
-    db_.reset(new FileSystemDirectoryDatabase(base_.path()));
   }
 
   bool AddFileInfo(FileId parent_id, const FilePath::StringType& name) {
@@ -43,6 +57,64 @@ class FileSystemDirectoryDatabaseTest : public testing::Test {
     return db_->AddFileInfo(info, &file_id);
   }
 
+  void CreateDirectory(FileId parent_id,
+                       const FilePath::StringType& name,
+                       FileId* file_id) {
+    FileId file_id_alt;

[ericu, 2012/04/03 01:52:11]

[here and below]

Mostly we tend to do this by just having a local temp FileId, then storing back
to *file_id if it's non-NULL.  It's not a style rule or anything, but it's
standard enough that doing it this way stands out and looks weird.

We usually only do the reference trick if the copy is expensive, which it's not
here.

[tzik, 2012/04/04 06:50:39]

Done.

+    if (!file_id)
+      file_id = &file_id_alt;
+
+    FileInfo info;
+    info.parent_id = parent_id;
+    info.name = name;
+    ASSERT_TRUE(db_->AddFileInfo(info, file_id));
+  }
+
+  void CreateFile(FileId parent_id,
+                  const FilePath::StringType& name,
+                  const FilePath::StringType& data_path,
+                  FileId* file_id) {
+    FileId file_id_alt;
+    if (!file_id)
+      file_id = &file_id_alt;
+
+    FileInfo info;
+    info.parent_id = parent_id;
+    info.name = name;
+    info.data_path = FilePath(data_path);
+    ASSERT_TRUE(db_->AddFileInfo(info, file_id));
+
+    FilePath local_path = path().Append(data_path);
+    if (!file_util::DirectoryExists(local_path.DirName()))
+      ASSERT_TRUE(file_util::CreateDirectory(local_path.DirName()));
+
+    bool created;
+    base::PlatformFileError error;
+    base::PlatformFile file = base::CreatePlatformFile(
+        local_path,
+        base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE,
+        &created, &error);
+    ASSERT_EQ(base::PLATFORM_FILE_OK, error);
+    ASSERT_TRUE(created);
+    ASSERT_TRUE(base::ClosePlatformFile(file));
+  }
+
+  void ClearDatabaseAndDirectory() {
+    db_.reset();
+    ASSERT_TRUE(file_util::Delete(path(), true /* recursive */));
+    ASSERT_TRUE(file_util::CreateDirectory(path()));
+    db_.reset(new FileSystemDirectoryDatabase(path()));
+  }
+
+  bool RepairDatabase() {
+    return db()->RepairDatabase(
+        FilePathToString(path().Append(kDirectoryDatabaseName)));
+  }
+
+  const FilePath& path() {
+    return base_.path();
+  }
+
  protected:
   // Common temp base for nondestructive uses.
   ScopedTempDir base_;
@@ -418,4 +490,99 @@ TEST_F(FileSystemDirectoryDatabaseTest, TestGetNextInteger) {
   EXPECT_EQ(4, next);
 }
 
+TEST_F(FileSystemDirectoryDatabaseTest, TestConsistencyCheck_Empty) {
+  EXPECT_TRUE(db()->IsFileSystemConsistent());
+
+  int64 next;
+  EXPECT_TRUE(db()->GetNextInteger(&next));
+  EXPECT_EQ(0, next);
+  EXPECT_TRUE(db()->IsFileSystemConsistent());
+}
+
+TEST_F(FileSystemDirectoryDatabaseTest, TestConsistencyCheck_Consistent) {
+  FileId dir_id;
+  FileId file_id;

[ericu, 2012/04/03 01:52:11]

file_id can go away.

[tzik, 2012/04/04 06:50:39]

Done.

+  CreateFile(0, FPL("foo"), FPL("hoge"), NULL);
+  CreateDirectory(0, FPL("bar"), &dir_id);
+  CreateFile(dir_id, FPL("baz"), FPL("fuga"), &file_id);
+  CreateFile(dir_id, FPL("fizz"), FPL("buzz"), NULL);
+
+  EXPECT_TRUE(db()->IsFileSystemConsistent());
+}
+
+TEST_F(FileSystemDirectoryDatabaseTest,
+       TestConsistencyCheck_BackingMultiEntry) {
+  const FilePath::CharType kBackingFileName[] = FPL("the celeb");
+  CreateFile(0, FPL("foo"), kBackingFileName, NULL);
+  ASSERT_TRUE(file_util::Delete(path().Append(kBackingFileName), false));
+  CreateFile(0, FPL("bar"), kBackingFileName, NULL);
+
+  EXPECT_FALSE(db()->IsFileSystemConsistent());
+}
+
+TEST_F(FileSystemDirectoryDatabaseTest, TestConsistencyCheck_FileLost) {
+  const FilePath::CharType kBackingFileName[] = FPL("hoge");
+  CreateFile(0, FPL("foo"), kBackingFileName, NULL);
+  ASSERT_TRUE(file_util::Delete(path().Append(kBackingFileName), false));
+
+  EXPECT_FALSE(db()->IsFileSystemConsistent());
+}
+
+TEST_F(FileSystemDirectoryDatabaseTest, TestConsistencyCheck_OrphanFile) {
+  CreateFile(0, FPL("foo"), FPL("hoge"), NULL);
+
+  bool created;
+  base::PlatformFileError error;
+  base::PlatformFile file = base::CreatePlatformFile(
+      path().Append("Orphan File"),
+      base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_WRITE,
+      &created, &error);
+  ASSERT_EQ(base::PLATFORM_FILE_OK, error);
+  ASSERT_TRUE(created);
+  ASSERT_TRUE(base::ClosePlatformFile(file));
+
+  EXPECT_FALSE(db()->IsFileSystemConsistent());
+}

[ericu, 2012/04/03 01:52:11]

Please add a test for a directory loop that includes the root and for one that
does not [which can be as simple as a single directory that is its own parent].

[tzik, 2012/04/04 06:50:39]

Done.

+
+TEST_F(FileSystemDirectoryDatabaseTest, TestRepairDatabase_Success) {
+  FilePath::StringType kFileName = FPL("bar");
+
+  FileId file_id_prev;
+  CreateFile(0, FPL("foo"), FPL("hoge"), NULL);
+  CreateFile(0, kFileName, FPL("fuga"), &file_id_prev);
+
+  const FilePath kDatabaseDirectory = path().Append(kDirectoryDatabaseName);
+  CloseDatabase();
+  CorruptDatabase(kDatabaseDirectory, leveldb::kDescriptorFile,
+                  0, std::numeric_limits<size_t>::max());
+  InitDatabase();
+  EXPECT_FALSE(db()->IsFileSystemConsistent());
+
+  FileId file_id;
+  EXPECT_TRUE(db()->GetChildWithName(0, kFileName, &file_id));
+  EXPECT_EQ(file_id_prev, file_id);
+
+  EXPECT_TRUE(db()->IsFileSystemConsistent());
+}
+
+TEST_F(FileSystemDirectoryDatabaseTest, TestRepairDatabase_Failure) {
+  FilePath::StringType kFileName = FPL("bar");
+
+  CreateFile(0, FPL("foo"), FPL("hoge"), NULL);
+  CreateFile(0, kFileName, FPL("fuga"), NULL);
+
+  const FilePath kDatabaseDirectory = path().Append(kDirectoryDatabaseName);
+  CloseDatabase();
+  CorruptDatabase(kDatabaseDirectory, leveldb::kDescriptorFile,
+                  0, std::numeric_limits<size_t>::max());
+  CorruptDatabase(kDatabaseDirectory, leveldb::kLogFile,
+                  -1, 1);
+  InitDatabase();
+  EXPECT_FALSE(db()->IsFileSystemConsistent());
+
+  FileId file_id;
+  EXPECT_FALSE(db()->GetChildWithName(0, kFileName, &file_id));
+  EXPECT_TRUE(db()->IsFileSystemConsistent());
+}
+
 }  // namespace fileapi