gdata: Fix a bug in GDataLevelDB where errors are not handled right

chrome/browser/chromeos/gdata/gdata_db_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/gdata/gdata_db.h"
 
 #include "base/string_number_conversions.h"
+#include "chrome/browser/chromeos/gdata/gdata.pb.h"
 #include "chrome/browser/chromeos/gdata/gdata_db_factory.h"
 #include "chrome/browser/chromeos/gdata/gdata_files.h"
+#include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "chrome/test/base/testing_profile.h"
 
 namespace gdata {
 namespace {
 
 class GDataDBTest : public testing::Test {
  public:
   GDataDBTest() {
   }
 
   virtual ~GDataDBTest() {
   }
 
  protected:
   // testing::Test implementation.
   virtual void SetUp() OVERRIDE;
 
   // Tests GDataDB::GetPath and GDataDB::ResourceId, ensuring that an entry
   // matching |source| does not exist.
   void TestGetNotFound(const GDataEntry& source);
 
   // Tests GDataDB::GetPath and GDataDB::ResourceId, ensuring that an entry
   // matching |source| exists.
   void TestGetFound(const GDataEntry& source);
 
+  // Tests GDataDB::GetPath and GDataDB::ResourceId, ensuring that an entry
+  // matching |source| is corrupt.
+  void TestGetCorrupt(const GDataEntry& source);
+
   // Initialize the database with the following entries:
   // drive/dir1
   // drive/dir2
   // drive/dir1/dir3
   // drive/dir1/file4
   // drive/dir1/file5
   // drive/dir2/file6
   // drive/dir2/file7
   // drive/dir2/file8
   // drive/dir1/dir3/file9
@@ skipping 45 matching lines @@
   entry.reset();
 
   status = gdata_db_->GetByResourceId(source.resource_id(), &entry);
   EXPECT_EQ(GDataDB::DB_OK, status);
   ASSERT_TRUE(entry.get());
   EXPECT_EQ(source.title(), entry->title());
   EXPECT_EQ(source.resource_id(), entry->resource_id());
   EXPECT_EQ(source.content_url(), entry->content_url());
 }
 
+void GDataDBTest::TestGetCorrupt(const GDataEntry& source) {
+  scoped_ptr<GDataEntry> entry;
+  GDataDB::Status status = gdata_db_->GetByPath(source.GetFilePath(), &entry);
+  EXPECT_EQ(GDataDB::DB_CORRUPTION, status);
+  EXPECT_FALSE(entry.get());
+
+  status = gdata_db_->GetByResourceId(source.resource_id(), &entry);
+  EXPECT_EQ(GDataDB::DB_CORRUPTION, status);
+  EXPECT_FALSE(entry.get());
+}
+
 void GDataDBTest::InitDB() {
   int sequence_id = 1;
   GDataRootDirectory root;
   GDataDirectory* dir1 = AddDirectory(&root, &root, sequence_id++);
   GDataDirectory* dir2 = AddDirectory(&root, &root, sequence_id++);
   GDataDirectory* dir3 = AddDirectory(dir1, &root, sequence_id++);
 
   AddFile(dir1, &root, sequence_id++);
   AddFile(dir1, &root, sequence_id++);
 
@@ skipping 68 matching lines @@
   GDataDirectory dir(&root, &root);
   dir.set_title("dir");
   dir.set_file_name("dir");
   dir.set_resource_id("dir_resource_id");
   dir.set_content_url(GURL("http://content/dir"));
   dir.set_upload_url(GURL("http://upload/dir"));
 
   TestGetNotFound(dir);
 
   GDataDB::Status status = gdata_db_->Put(dir);
-  EXPECT_EQ(GDataDB::DB_OK, status);
+  ASSERT_EQ(GDataDB::DB_OK, status);

[satorux1, 2012/07/10 17:10:17]

change it to ASSERT. We shouldn't go further if DB operation fails.

 
   TestGetFound(dir);
 
   scoped_ptr<GDataEntry> entry;
-  gdata_db_->GetByPath(dir.GetFilePath(), &entry);
+  status = gdata_db_->GetByPath(dir.GetFilePath(), &entry);
+  ASSERT_EQ(GDataDB::DB_OK, status);
   EXPECT_EQ(dir.upload_url(), entry->AsGDataDirectory()->upload_url());
   EXPECT_TRUE(entry->AsGDataDirectory()->file_info().is_directory);
 
   status = gdata_db_->DeleteByPath(dir.GetFilePath());
-  EXPECT_EQ(GDataDB::DB_OK, status);
+  ASSERT_EQ(GDataDB::DB_OK, status);
 
   TestGetNotFound(dir);
 
   GDataFile file(&dir, &root);
   file.set_title("file");
   file.set_file_name("file");
   file.set_resource_id("file_resource_id");
   file.set_content_url(GURL("http://content/dir/file"));
   file.set_file_md5("file_md5");
 
   TestGetNotFound(file);
 
   status = gdata_db_->Put(file);
-  EXPECT_EQ(GDataDB::DB_OK, status);
+  ASSERT_EQ(GDataDB::DB_OK, status);
 
   TestGetFound(file);
 
-  gdata_db_->GetByPath(file.GetFilePath(), &entry);
+  status = gdata_db_->GetByPath(file.GetFilePath(), &entry);
+  ASSERT_EQ(GDataDB::DB_OK, status);
   EXPECT_EQ(file.file_md5(), entry->AsGDataFile()->file_md5());
   EXPECT_FALSE(entry->AsGDataFile()->file_info().is_directory);
 
   status = gdata_db_->DeleteByPath(file.GetFilePath());
-  EXPECT_EQ(GDataDB::DB_OK, status);
+  ASSERT_EQ(GDataDB::DB_OK, status);
 
   TestGetNotFound(file);
 }
 
 TEST_F(GDataDBTest, IterTest) {
   InitDB();
 
   const char* dir1_children[] = {
     "drive/dir1",
     "drive/dir1/dir3",
@@ skipping 34 matching lines @@
     "drive/dir2",
     "drive/dir2/file6",
     "drive/dir2/file7",
     "drive/dir2/file8",
   };
   TestIter("", all_entries, arraysize(all_entries));
 
   TestIter("dir4", NULL, 0);
 }
 
+TEST_F(GDataDBTest, IncompatibleProtoTest) {
+  GDataRootDirectory root;
+  GDataFile file(&root, &root);
+  file.set_title("file");
+  file.set_file_name("file");
+  file.set_resource_id("file_resource_id");
+  file.set_content_url(GURL("http://content/dir/file"));
+  file.set_file_md5("file_md5");
+
+  // Add a file and check if it's found.
+  GDataDB::Status status = gdata_db_->Put(file);
+  ASSERT_EQ(GDataDB::DB_OK, status);
+  TestGetFound(file);
+  // Check if the iterator works too.
+  const char* all_entries[] = {
+    "drive/file",
+  };
+  TestIter("", all_entries, arraysize(all_entries));
+
+  // Tweak the file proto to simulate an incompatible proto in the DB.
+  GDataFileProto raw_proto;
+  file.ToProto(&raw_proto);
+  raw_proto.clear_upload_url();  // This will make FromProto() fail.
+  gdata_db_->PutRawForTesting("file_resource_id", raw_proto);
+
+  // Check if the corruption is detected.
+  TestGetCorrupt(file);
+  // We should no longer be able to find the file by iteration.
+  TestIter("", NULL, 0);
+}
+
 }  // namespace gdata