Store the next notification id in the notification database.

content/browser/notifications/notification_database_unittest.cc

Index: content/browser/notifications/notification_database_unittest.cc
diff --git a/content/browser/notifications/notification_database_unittest.cc b/content/browser/notifications/notification_database_unittest.cc
index 69853a0334fd921a87d7f8e9433540b22ab7be69..72e5f968603a3aa13e1eef151a8bcdbbaf14a66f 100644
--- a/content/browser/notifications/notification_database_unittest.cc
+++ b/content/browser/notifications/notification_database_unittest.cc
@@ -6,6 +6,8 @@
 
 #include "base/files/scoped_temp_dir.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
+#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
 namespace content {
 
@@ -31,6 +33,42 @@ class NotificationDatabaseTest : public ::testing::Test {
   bool IsInMemoryDatabase(NotificationDatabase* database) {
     return database->IsInMemoryDatabase();
   }
+
+  // Writes a LevelDB key-value pair directly to the LevelDB backing the
+  // notification database in |database|. Returns whether the pair could
+  // be written to the database successfully.
+  bool WriteLevelDBKeyValuePair(NotificationDatabase* database,
+                                const std::string& key,
+                                const std::string& value) {
+    leveldb::Status status = database->db()->Put(leveldb::WriteOptions(),
+                                                 key, value);
+    if (status.ok())

[Bernhard Bauer, 2015/03/11 22:02:38]

Change the return type to void and ASSERT? You can append the status to the
ASSERT stream as well.

(You're going to have to change the call site to use ASSERT_NO_FATAL_FAILURE.)

[Peter Beverloo, 2015/03/11 22:33:33]

ASSERT_NO_FATAL_FAILURE! Awesome! Changed :-).

+      return true;
+
+    LOG(ERROR) << "Unable to write to the database: " << status.ToString();
+    return false;
+  }
+
+  // Increments the next available notification id. |current_notification_id|
+  // must be the most recently used notification id. Returns whether the new
+  // value was written successfully.
+  //
+  // TODO(peter): Stop doing this manually when writing notification data will
+  // do this for us, except for tests verifying corruption behavior.
+  bool IncrementNextNotificationId(NotificationDatabase* database,
+                                   int64_t current_notification_id) {
+    leveldb::WriteBatch batch;
+    database->IncrementNextNotificationId(&batch, current_notification_id);
+
+    leveldb::Status status = database->db()->Write(leveldb::WriteOptions(),
+                                                   &batch);
+    if (status.ok())
+      return true;
+
+    LOG(ERROR) << "Unable to increment the next notification id: "
+        << status.ToString();
+    return false;
+  }
 };
 
 TEST_F(NotificationDatabaseTest, OpenCloseMemory) {
@@ -101,4 +139,63 @@ TEST_F(NotificationDatabaseTest, DestroyDatabase) {
             database->Open(false /* create_if_missing */));
 }
 
+TEST_F(NotificationDatabaseTest, GetNextNotificationIdIncrements) {
+  base::ScopedTempDir database_dir;
+  ASSERT_TRUE(database_dir.CreateUniqueTempDir());
+
+  scoped_ptr<NotificationDatabase> database(
+      CreateDatabaseOnFileSystem(database_dir.path()));
+
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->Open(true /* create_if_missing */));
+
+  int64_t notification_id = 0;
+
+  // Verify that getting two ids on the same database instance results in
+  // incrementing values. Notification ids will start at 1.
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->GetNextNotificationId(&notification_id));
+  EXPECT_EQ(1, notification_id);
+
+  EXPECT_TRUE(IncrementNextNotificationId(database.get(), notification_id));
+
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->GetNextNotificationId(&notification_id));
+  EXPECT_EQ(2, notification_id);
+
+  EXPECT_TRUE(IncrementNextNotificationId(database.get(), notification_id));
+
+  database.reset(CreateDatabaseOnFileSystem(database_dir.path()));
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->Open(false /* create_if_missing */));
+
+  // Verify that the next notification id was stored in the database, and
+  // continues where we expect it to be, even after closing and opening it.
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->GetNextNotificationId(&notification_id));
+  EXPECT_EQ(3, notification_id);
+}
+
+TEST_F(NotificationDatabaseTest, GetNextNotificationIdCorruption) {
+  scoped_ptr<NotificationDatabase> database(CreateDatabaseInMemory());
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->Open(true /* create_if_missing */));
+
+  int64_t notification_id = 0;
+
+  ASSERT_EQ(NotificationDatabase::STATUS_OK,
+            database->GetNextNotificationId(&notification_id));
+  EXPECT_EQ(1, notification_id);
+
+  // Deliberately write an invalid value as the next notification id to the

[Bernhard Bauer, 2015/03/11 22:02:38]

Nit: "ID" in uppercase.

[Peter Beverloo, 2015/03/11 22:33:33]

I've got it in lowercase everywhere in NotificationDatabase, as other reviewers
(in entirely separate patches) preferred lowercase.

Since lowercase is consistent with the rest of the notification code, I'd prefer
to keep it this way.

+  // database, which should cause GetNextNotificationId to realize that
+  // something is wrong with the data it's reading.
+  ASSERT_TRUE(WriteLevelDBKeyValuePair(database.get(),
+                                       "NEXT_NOTIFICATION_ID",
+                                       "-42"));
+
+  EXPECT_EQ(NotificationDatabase::STATUS_ERROR_CORRUPTED,
+            database->GetNextNotificationId(&notification_id));
+}
+
 }  // namespace content