Store the next notification id in the notification database.

content/browser/notifications/notification_database_unittest.cc

 // Copyright 2015 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 "content/browser/notifications/notification_database.h"
 
 #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 {
 
 class NotificationDatabaseTest : public ::testing::Test {
  protected:
   // Creates a new NotificationDatabase instance in memory.
   NotificationDatabase* CreateDatabaseInMemory() {
     return new NotificationDatabase(base::FilePath());
   }
 
   // Creates a new NotificationDatabase instance in |path|.
   NotificationDatabase* CreateDatabaseOnFileSystem(
       const base::FilePath& path) {
     return new NotificationDatabase(path);
   }
 
   // Returns if |database| has been opened.
   bool IsDatabaseOpen(NotificationDatabase* database) {
     return database->IsOpen();
   }
 
   // Returns if |database| is an in-memory only database.
   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) {
   scoped_ptr<NotificationDatabase> database(CreateDatabaseInMemory());
 
   // Should return false because the database does not exist in memory.
   EXPECT_EQ(NotificationDatabase::STATUS_ERROR_NOT_FOUND,
             database->Open(false /* create_if_missing */));
 
   // Should return true, indicating that the database could be created.
@@ skipping 50 matching lines @@
   ASSERT_EQ(NotificationDatabase::STATUS_OK, database->Destroy());
   EXPECT_FALSE(IsDatabaseOpen(database.get()));
 
   // Try to re-open the database (but not re-create it). This should fail as
   // the files associated with the database should have been blown away.
   database.reset(CreateDatabaseOnFileSystem(database_dir.path()));
   EXPECT_EQ(NotificationDatabase::STATUS_ERROR_NOT_FOUND,
             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