[Feed] Add delete all function for both content and journal

1. Add Delete all function for both content and jornal.
2. Update DeleteContentByPrefix to use
   leveldb_proto::UpdateEntriesWithRemoveFilter.
3. Fix FakeDB<T>::UpdateEntriesWithRemoveFilter the way to handle
   erase during iteration.

Bug:831633

Change-Id: I7ae4df74244b3c3ac5c0bf36ea2e2bdc7839d375
Reviewed-on: https://chromium-review.googlesource.com/1103939Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568224}

components/feed/core/feed_storage_database.cc

@@ -160,13 +160,13 @@ void FeedStorageDatabase::DeleteContent(
     ConfirmationCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::unordered_set<std::string> key_set;
+  auto content_to_delete = std::make_unique<std::vector<std::string>>();
   for (const auto& key : keys_to_delete) {
-    key_set.insert(FormatContentKeyToStorageKey(key));
+    content_to_delete->emplace_back(FormatContentKeyToStorageKey(key));
   }
-  storage_database_->LoadEntriesWithFilter(
-      base::BindRepeating(&DatabaseKeyFilter, std::move(key_set)),
-      base::BindOnce(&FeedStorageDatabase::OnLoadEntriesForDeleteContent,
+  storage_database_->UpdateEntries(
+      std::make_unique<StorageEntryVector>(), std::move(content_to_delete),
+      base::BindOnce(&FeedStorageDatabase::OnStorageCommitted,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
@@ -176,9 +176,21 @@ void FeedStorageDatabase::DeleteContentByPrefix(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   std::string key_prefix = FormatContentKeyToStorageKey(prefix_to_delete);
-  storage_database_->LoadEntriesWithFilter(
+  storage_database_->UpdateEntriesWithRemoveFilter(
+      std::make_unique<StorageEntryVector>(),
+      base::BindRepeating(&DatabasePrefixFilter, std::move(key_prefix)),
+      base::BindOnce(&FeedStorageDatabase::OnStorageCommitted,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void FeedStorageDatabase::DeleteAllContent(ConfirmationCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  std::string key_prefix = FormatContentKeyToStorageKey(std::string());
+  storage_database_->UpdateEntriesWithRemoveFilter(
+      std::make_unique<StorageEntryVector>(),
       base::BindRepeating(&DatabasePrefixFilter, std::move(key_prefix)),
-      base::BindOnce(&FeedStorageDatabase::OnLoadEntriesForDeleteContent,
+      base::BindOnce(&FeedStorageDatabase::OnStorageCommitted,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
@@ -237,6 +249,17 @@ void FeedStorageDatabase::DeleteJournal(const std::string& key,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void FeedStorageDatabase::DeleteAllJournals(ConfirmationCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  std::string key_prefix = FormatJournalKeyToStorageKey(std::string());
+  storage_database_->UpdateEntriesWithRemoveFilter(
+      std::make_unique<StorageEntryVector>(),
+      base::BindRepeating(&DatabasePrefixFilter, std::move(key_prefix)),
+      base::BindOnce(&FeedStorageDatabase::OnStorageCommitted,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void FeedStorageDatabase::OnDatabaseInitialized(bool success) {
   DCHECK_EQ(database_status_, UNINITIALIZED);
 
@@ -270,28 +293,6 @@ void FeedStorageDatabase::OnLoadEntriesForLoadContent(
   std::move(callback).Run(std::move(results));
 }
 
-void FeedStorageDatabase::OnLoadEntriesForDeleteContent(
-    ConfirmationCallback callback,
-    bool success,
-    std::unique_ptr<std::vector<FeedStorageProto>> content) {
-  if (!success || !content) {
-    DVLOG_IF(1, !success) << "FeedStorageDatabase load content failed.";
-    std::move(callback).Run(success);
-    return;
-  }
-
-  auto contents_to_delete = std::make_unique<std::vector<std::string>>();
-  for (const auto& proto : *content) {
-    DCHECK(proto.has_content_data());
-    contents_to_delete->push_back(FormatContentKeyToStorageKey(proto.key()));
-  }
-
-  storage_database_->UpdateEntries(
-      std::make_unique<StorageEntryVector>(), std::move(contents_to_delete),
-      base::BindOnce(&FeedStorageDatabase::OnStorageCommitted,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
-}
-
 void FeedStorageDatabase::OnGetEntryForLoadJournal(
     JournalLoadCallback callback,
     bool success,

components/feed/core/feed_storage_database.h

@@ -80,6 +80,10 @@ class FeedStorageDatabase {
   void DeleteContentByPrefix(const std::string& prefix_to_delete,
                              ConfirmationCallback callback);
 
+  // Delete all content, |callback| will be called when all content is deleted
+  // or if there is an error.
+  void DeleteAllContent(ConfirmationCallback callback);
+
   // Loads the journal data for the |key| and passes it to |callback|.
   void LoadJournal(const std::string& key, JournalLoadCallback callback);
 
@@ -104,6 +108,10 @@ class FeedStorageDatabase {
   // is deleted or if there is an error.
   void DeleteJournal(const std::string& key, ConfirmationCallback callback);
 
+  // Delete all journals, |callback| will be called when all journals are
+  // deleted or if there is an error.
+  void DeleteAllJournals(ConfirmationCallback callback);
+
  private:
   // Callback methods given to |storage_database_| for async responses.
   void OnDatabaseInitialized(bool success);
@@ -111,10 +119,6 @@ class FeedStorageDatabase {
       ContentLoadCallback callback,
       bool success,
       std::unique_ptr<std::vector<FeedStorageProto>> content);
-  void OnLoadEntriesForDeleteContent(
-      ConfirmationCallback callback,
-      bool success,
-      std::unique_ptr<std::vector<FeedStorageProto>> content);
   void OnGetEntryForLoadJournal(JournalLoadCallback callback,
                                 bool success,
                                 std::unique_ptr<FeedStorageProto> journal);

components/feed/core/feed_storage_database_unittest.cc

@@ -225,7 +225,6 @@ TEST_F(FeedStorageDatabaseTest, DeleteContent) {
       std::move(keys),
       base::BindOnce(&FeedStorageDatabaseTest::OnStorageCommitted,
                      base::Unretained(this)));
-  storage_db()->LoadCallback(true);
   storage_db()->UpdateCallback(true);
 
   // Make sure only |kContentKey2| got deleted.
@@ -255,7 +254,38 @@ TEST_F(FeedStorageDatabaseTest, DeleteContentByPrefix) {
       kContentKeyPrefix,
       base::BindOnce(&FeedStorageDatabaseTest::OnStorageCommitted,
                      base::Unretained(this)));
+  storage_db()->UpdateCallback(true);
+
+  // Make sure |kContentKey1| and |kContentKey2| got deleted.
+  EXPECT_CALL(*this, OnContentEntriesReceived(_))
+      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+        EXPECT_EQ(results.size(), 0U);
+      });
+  db()->LoadContent(
+      {kContentKey1, kContentKey2},
+      base::BindOnce(&FeedStorageDatabaseTest::OnContentEntriesReceived,
+                     base::Unretained(this)));
   storage_db()->LoadCallback(true);
+}
+
+TEST_F(FeedStorageDatabaseTest, DeleteAllContent) {
+  CreateDatabase(/*init_database=*/true);
+
+  // Store |kContentKey1| and |kContentKey2|.
+  InjectContentStorageProto(kContentKey1, kContentData1);
+  InjectContentStorageProto(kContentKey2, kContentData2);
+
+  // Store |kJournalKey1|, |kJournalKey2|, |kJournalKey3|.
+  InjectJournalStorageProto(kJournalKey1,
+                            {kJournalData1, kJournalData2, kJournalData3});
+  InjectJournalStorageProto(kJournalKey2, {kJournalData4, kJournalData5});
+  InjectJournalStorageProto(kJournalKey3, {kJournalData6});
+
+  // Delete all content, meaning |kContentKey1| and |kContentKey2| are expected
+  // to be deleted.
+  EXPECT_CALL(*this, OnStorageCommitted(true));
+  db()->DeleteAllContent(base::BindOnce(
+      &FeedStorageDatabaseTest::OnStorageCommitted, base::Unretained(this)));
   storage_db()->UpdateCallback(true);
 
   // Make sure |kContentKey1| and |kContentKey2| got deleted.
@@ -268,6 +298,16 @@ TEST_F(FeedStorageDatabaseTest, DeleteContentByPrefix) {
       base::BindOnce(&FeedStorageDatabaseTest::OnContentEntriesReceived,
                      base::Unretained(this)));
   storage_db()->LoadCallback(true);
+
+  // Make sure all journals are there.
+  EXPECT_CALL(*this, OnJournalEntryReceived(_))
+      .WillOnce([](std::vector<std::string> results) {
+        ASSERT_EQ(results.size(), 3U);
+      });
+  db()->LoadAllJournalKeys(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+                     base::Unretained(this)));
+  storage_db()->LoadKeysCallback(true);
 }
 
 TEST_F(FeedStorageDatabaseTest, LoadJournalEntry) {
@@ -487,4 +527,50 @@ TEST_F(FeedStorageDatabaseTest, DeleteJournal) {
   storage_db()->LoadKeysCallback(true);
 }
 
+TEST_F(FeedStorageDatabaseTest, DeleteAllJournals) {
+  CreateDatabase(/*init_database=*/true);
+
+  // Store |kContentKey1| and |kContentKey2|.
+  InjectContentStorageProto(kContentKey1, kContentData1);
+  InjectContentStorageProto(kContentKey2, kContentData2);
+
+  // Store |kJournalKey1|, |kJournalKey2|, |kJournalKey3|.
+  InjectJournalStorageProto(kJournalKey1,
+                            {kJournalData1, kJournalData2, kJournalData3});
+  InjectJournalStorageProto(kJournalKey2, {kJournalData4, kJournalData5});
+  InjectJournalStorageProto(kJournalKey3, {kJournalData6});
+
+  // Delete all journals, meaning |kJournalKey1|, |kJournalKey2| and
+  // |kJournalKey3| are expected to be deleted.
+  EXPECT_CALL(*this, OnStorageCommitted(true));
+  db()->DeleteAllJournals(base::BindOnce(
+      &FeedStorageDatabaseTest::OnStorageCommitted, base::Unretained(this)));
+  storage_db()->UpdateCallback(true);
+
+  // Make sure all journals got deleted.
+  EXPECT_CALL(*this, OnJournalEntryReceived(_))
+      .WillOnce([](std::vector<std::string> results) {
+        ASSERT_EQ(results.size(), 0U);
+      });
+  db()->LoadAllJournalKeys(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+                     base::Unretained(this)));
+  storage_db()->LoadKeysCallback(true);
+
+  // Make sure all content are still there.
+  EXPECT_CALL(*this, OnContentEntriesReceived(_))
+      .WillOnce([](std::vector<std::pair<std::string, std::string>> results) {
+        ASSERT_EQ(results.size(), 2U);
+        EXPECT_EQ(results[0].first, kContentKey1);
+        EXPECT_EQ(results[0].second, kContentData1);
+        EXPECT_EQ(results[1].first, kContentKey2);
+        EXPECT_EQ(results[1].second, kContentData2);
+      });
+  db()->LoadContent(
+      {kContentKey1, kContentKey2},
+      base::BindOnce(&FeedStorageDatabaseTest::OnContentEntriesReceived,
+                     base::Unretained(this)));
+  storage_db()->LoadCallback(true);
+}
+
 }  // namespace feed

components/leveldb_proto/testing/fake_db.h

@@ -135,7 +135,7 @@ void FakeDB<T>::UpdateEntriesWithRemoveFilter(
   auto it = db_->begin();
   while (it != db_->end()) {
     if (!delete_key_filter.is_null() && delete_key_filter.Run(it->first))
-      db_->erase(it->first);
+      db_->erase(it++);
     else
       ++it;
   }