[Feed] Implement LoadAllContentKeys and LoadAllJournals

Implement LoadAllContentKeys and LoadAllJournals, and remove
LoadAllJournalKeys since do not need it.

Bug:828938

Change-Id: I913d59ec25982d2f019dc0a788f20cc58e320af8
Reviewed-on: https://chromium-review.googlesource.com/1119234
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571687}

components/feed/core/feed_storage_database.cc

@@ -45,20 +45,20 @@ std::string FormatJournalKeyToStorageKey(const std::string& journal_key) {
   return kJournalStoragePrefix + journal_key;
 }
 
-// Check if the |storage_key| is for journal data.
-bool IsValidJournalKey(const std::string& storage_key) {
-  return base::StartsWith(storage_key, kJournalStoragePrefix,
+// Check if the |storage_key| is for content data.
+bool IsValidContentKey(const std::string& storage_key) {
+  return base::StartsWith(storage_key, kContentStoragePrefix,
                           base::CompareCase::SENSITIVE);
 }
 
-// Parse journal key from storage key. Return an empty string if |storage_key|
-// is not recognized as journal key. ex. content's storage key.
-std::string ParseJournalKey(const std::string& storage_key) {
-  if (!IsValidJournalKey(storage_key)) {
+// Parse content key from storage key. Return an empty string if |storage_key|
+// is not recognized as content key. ex. journal's storage key.
+std::string ParseContentKey(const std::string& storage_key) {
+  if (!IsValidContentKey(storage_key)) {
     return std::string();
   }
 
-  return storage_key.substr(strlen(kJournalStoragePrefix));
+  return storage_key.substr(strlen(kContentStoragePrefix));
 }
 
 bool DatabaseKeyFilter(const std::unordered_set<std::string>& key_set,
@@ -136,6 +136,14 @@ void FeedStorageDatabase::LoadContentByPrefix(const std::string& prefix,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void FeedStorageDatabase::LoadAllContentKeys(ContentKeyCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  storage_database_->LoadKeys(
+      base::BindOnce(&FeedStorageDatabase::OnLoadKeysForLoadAllContentKeys,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void FeedStorageDatabase::SaveContent(std::vector<KeyAndData> pairs,
                                       ConfirmationCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -204,11 +212,12 @@ void FeedStorageDatabase::LoadJournal(const std::string& key,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void FeedStorageDatabase::LoadAllJournalKeys(JournalLoadCallback callback) {
+void FeedStorageDatabase::LoadAllJournals(LoadAllJournalsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  storage_database_->LoadKeys(
-      base::BindOnce(&FeedStorageDatabase::OnLoadKeysForLoadAllJournalKeys,
+  storage_database_->LoadEntriesWithFilter(
+      base::BindRepeating(&DatabasePrefixFilter, kJournalStoragePrefix),
+      base::BindOnce(&FeedStorageDatabase::OnLoadEntriesForLoadAllJournals,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
@@ -293,6 +302,27 @@ void FeedStorageDatabase::OnLoadEntriesForLoadContent(
   std::move(callback).Run(std::move(results));
 }
 
+void FeedStorageDatabase::OnLoadKeysForLoadAllContentKeys(
+    ContentKeyCallback callback,
+    bool success,
+    std::unique_ptr<std::vector<std::string>> keys) {
+  std::vector<std::string> results;
+
+  if (!success || !keys) {
+    DVLOG_IF(1, !success) << "FeedStorageDatabase load content keys failed.";
+    std::move(callback).Run(std::move(results));
+    return;
+  }
+
+  // Filter out journal keys, only keep content keys.
+  for (const std::string& key : *keys) {
+    if (IsValidContentKey(key))
+      results.emplace_back(ParseContentKey(key));
+  }
+
+  std::move(callback).Run(std::move(results));
+}
+
 void FeedStorageDatabase::OnGetEntryForLoadJournal(
     JournalLoadCallback callback,
     bool success,
@@ -366,22 +396,27 @@ void FeedStorageDatabase::OnGetEntryForCopyJournal(
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void FeedStorageDatabase::OnLoadKeysForLoadAllJournalKeys(
-    JournalLoadCallback callback,
+void FeedStorageDatabase::OnLoadEntriesForLoadAllJournals(
+    LoadAllJournalsCallback callback,
     bool success,
-    std::unique_ptr<std::vector<std::string>> keys) {
-  std::vector<std::string> results;
+    std::unique_ptr<std::vector<FeedStorageProto>> entries) {
+  std::vector<std::vector<std::string>> results;
 
-  if (!success || !keys) {
-    DVLOG_IF(1, !success) << "FeedStorageDatabase load journal keys failed.";
+  if (!success || !entries) {
+    DVLOG_IF(1, !success) << "FeedStorageDatabase load journals failed.";
     std::move(callback).Run(std::move(results));
     return;
   }
 
-  // Filter out content keys, only keep journal keys.
-  for (const std::string& key : *keys) {
-    if (IsValidJournalKey(key))
-      results.emplace_back(ParseJournalKey(key));
+  for (const auto& entry : *entries) {
+    DCHECK(entry.has_key());
+    DCHECK_NE(entry.journal_data_size(), 0);
+
+    std::vector<std::string> journal;
+    for (int i = 0; i < entry.journal_data_size(); ++i) {
+      journal.emplace_back(entry.journal_data(i));
+    }
+    results.push_back(journal);
   }
 
   std::move(callback).Run(std::move(results));

components/feed/core/feed_storage_database.h

@@ -29,10 +29,17 @@ class FeedStorageDatabase {
   // loading data.
   using ContentLoadCallback = base::OnceCallback<void(std::vector<KeyAndData>)>;
 
+  // Returns the content keys as a vector when calling loading all content keys.
+  using ContentKeyCallback = base::OnceCallback<void(std::vector<std::string>)>;
+
   // Returns the journal data as a vector of strings when calling loading data.
   using JournalLoadCallback =
       base::OnceCallback<void(std::vector<std::string>)>;
 
+  // Returns a vector of journal data when calling loading all journals.
+  using LoadAllJournalsCallback =
+      base::OnceCallback<void(std::vector<std::vector<std::string>>)>;
+
   // Returns whether the commit operation succeeded.
   using ConfirmationCallback = base::OnceCallback<void(bool)>;
 
@@ -64,6 +71,9 @@ class FeedStorageDatabase {
   void LoadContentByPrefix(const std::string& prefix,
                            ContentLoadCallback callback);
 
+  // Loads all content keys in the storage, and passes them to |callback|.
+  void LoadAllContentKeys(ContentKeyCallback callback);
+
   // Inserts or updates the content data |pairs|, |callback| will be called when
   // the data are saved or if there is an error. The fields in |pairs| will be
   // std::move.
@@ -87,8 +97,8 @@ class FeedStorageDatabase {
   // Loads the journal data for the |key| and passes it to |callback|.
   void LoadJournal(const std::string& key, JournalLoadCallback callback);
 
-  // Loads all journal keys in the storage, and passes them to |callback|.
-  void LoadAllJournalKeys(JournalLoadCallback callback);
+  // Loads all journals in the storage, and passes them to |callback|.
+  void LoadAllJournals(LoadAllJournalsCallback callback);
 
   // Appends |entries| to a journal whose key is |key|, if there the journal do
   // not exist, create one. |callback| will be called when the data are saved or
@@ -119,6 +129,10 @@ class FeedStorageDatabase {
       ContentLoadCallback callback,
       bool success,
       std::unique_ptr<std::vector<FeedStorageProto>> content);
+  void OnLoadKeysForLoadAllContentKeys(
+      ContentKeyCallback callback,
+      bool success,
+      std::unique_ptr<std::vector<std::string>> keys);
   void OnGetEntryForLoadJournal(JournalLoadCallback callback,
                                 bool success,
                                 std::unique_ptr<FeedStorageProto> journal);
@@ -131,10 +145,10 @@ class FeedStorageDatabase {
                                 std::string to_key,
                                 bool success,
                                 std::unique_ptr<FeedStorageProto> journal);
-  void OnLoadKeysForLoadAllJournalKeys(
-      JournalLoadCallback callback,
+  void OnLoadEntriesForLoadAllJournals(
+      LoadAllJournalsCallback callback,
       bool success,
-      std::unique_ptr<std::vector<std::string>> keys);
+      std::unique_ptr<std::vector<FeedStorageProto>> entries);
   void OnStorageCommitted(ConfirmationCallback callback, bool success);
 
   State database_status_;

components/feed/core/feed_storage_database_unittest.cc

@@ -88,7 +88,10 @@ class FeedStorageDatabaseTest : public testing::Test {
 
   MOCK_METHOD1(OnContentEntriesReceived,
                void(std::vector<std::pair<std::string, std::string>>));
+  MOCK_METHOD1(OnContentKeyReceived, void(std::vector<std::string>));
   MOCK_METHOD1(OnJournalEntryReceived, void(std::vector<std::string>));
+  MOCK_METHOD1(OnJournalEntriesReceived,
+               void(std::vector<std::vector<std::string>>));
   MOCK_METHOD1(OnStorageCommitted, void(bool));
 
  private:
@@ -179,6 +182,29 @@ TEST_F(FeedStorageDatabaseTest, LoadContentsEntriesByPrefix) {
   storage_db()->LoadCallback(true);
 }
 
+TEST_F(FeedStorageDatabaseTest, LoadAllContentKeys) {
+  CreateDatabase(/*init_database=*/true);
+
+  // Store |kContentKey1|, |kContentKey2|, |kJournalKey1|, |kJournalKey2|,
+  // |kJournalKey3|.
+  InjectContentStorageProto(kContentKey1, kContentData1);
+  InjectContentStorageProto(kContentKey2, kContentData2);
+  InjectJournalStorageProto(kJournalKey1,
+                            {kJournalData1, kJournalData2, kJournalData3});
+  InjectJournalStorageProto(kJournalKey2, {kJournalData4, kJournalData5});
+  InjectJournalStorageProto(kJournalKey3, {kJournalData6});
+
+  EXPECT_CALL(*this, OnContentKeyReceived(_))
+      .WillOnce([](std::vector<std::string> results) {
+        ASSERT_EQ(results.size(), 2U);
+        EXPECT_EQ(results[0], kContentKey1);
+        EXPECT_EQ(results[1], kContentKey2);
+      });
+  db()->LoadAllContentKeys(base::BindOnce(
+      &FeedStorageDatabaseTest::OnContentKeyReceived, base::Unretained(this)));
+  storage_db()->LoadKeysCallback(true);
+}
+
 TEST_F(FeedStorageDatabaseTest, SaveContent) {
   CreateDatabase(/*init_database=*/true);
 
@@ -300,14 +326,14 @@ TEST_F(FeedStorageDatabaseTest, DeleteAllContent) {
   storage_db()->LoadCallback(true);
 
   // Make sure all journals are there.
-  EXPECT_CALL(*this, OnJournalEntryReceived(_))
-      .WillOnce([](std::vector<std::string> results) {
+  EXPECT_CALL(*this, OnJournalEntriesReceived(_))
+      .WillOnce([](std::vector<std::vector<std::string>> results) {
         ASSERT_EQ(results.size(), 3U);
       });
-  db()->LoadAllJournalKeys(
-      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+  db()->LoadAllJournals(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntriesReceived,
                      base::Unretained(this)));
-  storage_db()->LoadKeysCallback(true);
+  storage_db()->LoadCallback(true);
 }
 
 TEST_F(FeedStorageDatabaseTest, LoadJournalEntry) {
@@ -347,7 +373,7 @@ TEST_F(FeedStorageDatabaseTest, LoadNonExistingJournalEntry) {
   storage_db()->GetCallback(true);
 }
 
-TEST_F(FeedStorageDatabaseTest, LoadAllJournalKeys) {
+TEST_F(FeedStorageDatabaseTest, LoadAllJournals) {
   CreateDatabase(/*init_database=*/true);
 
   // Store |kContentKey1|, |kContentKey2|, |kJournalKey1|, |kJournalKey2|,
@@ -359,17 +385,20 @@ TEST_F(FeedStorageDatabaseTest, LoadAllJournalKeys) {
   InjectJournalStorageProto(kJournalKey2, {kJournalData4, kJournalData5});
   InjectJournalStorageProto(kJournalKey3, {kJournalData6});
 
-  EXPECT_CALL(*this, OnJournalEntryReceived(_))
-      .WillOnce([](std::vector<std::string> results) {
+  EXPECT_CALL(*this, OnJournalEntriesReceived(_))
+      .WillOnce([](std::vector<std::vector<std::string>> results) {
         ASSERT_EQ(results.size(), 3U);
-        EXPECT_EQ(results[0], kJournalKey1);
-        EXPECT_EQ(results[1], kJournalKey2);
-        EXPECT_EQ(results[2], kJournalKey3);
+        EXPECT_EQ(results[0][0], kJournalData1);
+        EXPECT_EQ(results[0][1], kJournalData2);
+        EXPECT_EQ(results[0][2], kJournalData3);
+        EXPECT_EQ(results[1][0], kJournalData4);
+        EXPECT_EQ(results[1][1], kJournalData5);
+        EXPECT_EQ(results[2][0], kJournalData6);
       });
-  db()->LoadAllJournalKeys(
-      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+  db()->LoadAllJournals(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntriesReceived,
                      base::Unretained(this)));
-  storage_db()->LoadKeysCallback(true);
+  storage_db()->LoadCallback(true);
 }
 
 TEST_F(FeedStorageDatabaseTest, AppendToJournal_WhenJournalExists) {
@@ -515,16 +544,18 @@ TEST_F(FeedStorageDatabaseTest, DeleteJournal) {
   storage_db()->UpdateCallback(true);
 
   // Make sure |kJournalKey2| got deleted.
-  EXPECT_CALL(*this, OnJournalEntryReceived(_))
-      .WillOnce([](std::vector<std::string> results) {
+  EXPECT_CALL(*this, OnJournalEntriesReceived(_))
+      .WillOnce([](std::vector<std::vector<std::string>> results) {
         ASSERT_EQ(results.size(), 2U);
-        EXPECT_EQ(results[0], kJournalKey1);
-        EXPECT_EQ(results[1], kJournalKey3);
+        EXPECT_EQ(results[0][0], kJournalData1);
+        EXPECT_EQ(results[0][1], kJournalData2);
+        EXPECT_EQ(results[0][2], kJournalData3);
+        EXPECT_EQ(results[1][0], kJournalData6);
       });
-  db()->LoadAllJournalKeys(
-      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+  db()->LoadAllJournals(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntriesReceived,
                      base::Unretained(this)));
-  storage_db()->LoadKeysCallback(true);
+  storage_db()->LoadCallback(true);
 }
 
 TEST_F(FeedStorageDatabaseTest, DeleteAllJournals) {
@@ -548,14 +579,14 @@ TEST_F(FeedStorageDatabaseTest, DeleteAllJournals) {
   storage_db()->UpdateCallback(true);
 
   // Make sure all journals got deleted.
-  EXPECT_CALL(*this, OnJournalEntryReceived(_))
-      .WillOnce([](std::vector<std::string> results) {
+  EXPECT_CALL(*this, OnJournalEntriesReceived(_))
+      .WillOnce([](std::vector<std::vector<std::string>> results) {
         ASSERT_EQ(results.size(), 0U);
       });
-  db()->LoadAllJournalKeys(
-      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntryReceived,
+  db()->LoadAllJournals(
+      base::BindOnce(&FeedStorageDatabaseTest::OnJournalEntriesReceived,
                      base::Unretained(this)));
-  storage_db()->LoadKeysCallback(true);
+  storage_db()->LoadCallback(true);
 
   // Make sure all content are still there.
   EXPECT_CALL(*this, OnContentEntriesReceived(_))