[Notification Scheduler]: Support multiple icon operations in Icon Store.

- In support of loading/adding/deleting a vector of icons with callbacks.
- Change Add API, removing key argument.
- Will follow-up unit test in next cl.

Bug: 963290
Change-Id: Ie0fddeea5a1dc74103fb5765a976bb3ad933e156
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1722407
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682107}

chrome/browser/notifications/scheduler/internal/icon_store.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/stl_util.h"
 #include "chrome/browser/notifications/scheduler/internal/icon_entry.h"
 #include "chrome/browser/notifications/scheduler/internal/proto_conversion.h"
 
@@ -24,6 +25,12 @@ void ProtoToData(notifications::proto::Icon* proto,
 }  // namespace leveldb_proto
 
 namespace notifications {
+namespace {
+bool HasKeyInDb(const std::vector<std::string>& key_dict,
+                const std::string& key) {
+  return base::Contains(key_dict, key);
+}
+}  // namespace
 
 using KeyEntryPair = std::pair<std::string, IconEntry>;
 using KeyEntryVector = std::vector<KeyEntryPair>;
@@ -41,32 +48,63 @@ void IconProtoDbStore::Init(InitCallback callback) {
                            std::move(callback)));
 }
 
-void IconProtoDbStore::Load(const std::string& key, LoadCallback callback) {
+void IconProtoDbStore::Load(const std::string& key, LoadIconCallback callback) {
   db_->GetEntry(
       key, base::BindOnce(&IconProtoDbStore::OnIconEntryLoaded,
                           weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void IconProtoDbStore::Add(const std::string& key,
-                           std::unique_ptr<IconEntry> entry,
+void IconProtoDbStore::LoadIcons(const std::vector<std::string>& keys,
+                                 LoadIconsCallback callback) {
+  db_->LoadEntriesWithFilter(
+      base::BindRepeating(&HasKeyInDb, keys),
+      base::BindOnce(&IconProtoDbStore::OnIconEntriesLoaded,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void IconProtoDbStore::Add(std::unique_ptr<IconEntry> entry,
                            UpdateCallback callback) {
   auto entries_to_save = std::make_unique<KeyEntryVector>();
   // TODO(xingliu): See if proto database can use
   // std::vector<std::unique_ptr<T>> to avoid copy T into std::pair. Currently
   // some code uses the copy constructor that force T to be copyable.
+  auto key = entry->uuid;
   entries_to_save->emplace_back(
-      std::make_pair(key, std::move(*entry.release())));
-  db_->UpdateEntries(std::move(entries_to_save), std::make_unique<KeyVector>(),
+      std::make_pair(std::move(key), std::move(*entry.release())));
+  db_->UpdateEntries(std::move(entries_to_save),
+                     std::make_unique<KeyVector>() /*keys_to_delete*/,
+                     std::move(callback));
+}
+
+void IconProtoDbStore::AddIcons(std::vector<std::unique_ptr<IconEntry>> entries,
+                                UpdateCallback callback) {
+  auto entries_to_save = std::make_unique<KeyEntryVector>();
+  for (size_t i = 0; i < entries.size(); i++) {
+    auto entry = std::move(entries[i]);
+    auto key = entry->uuid;
+    entries_to_save->emplace_back(std::move(key), std::move(*entry.release()));
+  }
+  db_->UpdateEntries(std::move(entries_to_save),
+                     std::make_unique<KeyVector>() /*keys_to_delete*/,
                      std::move(callback));
 }
 
 void IconProtoDbStore::Delete(const std::string& key, UpdateCallback callback) {
   auto keys_to_delete = std::make_unique<KeyVector>();
   keys_to_delete->emplace_back(key);
-  db_->UpdateEntries(std::make_unique<KeyEntryVector>(),
+  db_->UpdateEntries(std::make_unique<KeyEntryVector>() /*keys_to_save*/,
                      std::move(keys_to_delete), std::move(callback));
 }
 
+void IconProtoDbStore::DeleteIcons(const std::vector<std::string>& keys,
+                                   UpdateCallback callback) {
+  auto keys_to_delete = std::make_unique<KeyVector>();
+  for (size_t i = 0; i < keys.size(); i++)
+    keys_to_delete->emplace_back(keys[i]);
+
+  db_->UpdateEntries(std::make_unique<KeyEntryVector>() /*keys_to_save*/,
+                     std::move(keys_to_delete), std::move(callback));
+}
 void IconProtoDbStore::OnDbInitialized(
     InitCallback callback,
     leveldb_proto::Enums::InitStatus status) {
@@ -75,7 +113,7 @@ void IconProtoDbStore::OnDbInitialized(
 }
 
 void IconProtoDbStore::OnIconEntryLoaded(
-    LoadCallback callback,
+    LoadIconCallback callback,
     bool success,
     std::unique_ptr<IconEntry> icon_entry) {
   if (!success) {
@@ -86,4 +124,16 @@ void IconProtoDbStore::OnIconEntryLoaded(
   std::move(callback).Run(true, std::move(icon_entry));
 }
 
+void IconProtoDbStore::OnIconEntriesLoaded(
+    LoadIconsCallback callback,
+    bool success,
+    std::unique_ptr<std::vector<IconEntry>> icon_entries) {
+  if (!success) {
+    std::move(callback).Run(false, nullptr);
+    return;
+  }
+
+  std::move(callback).Run(true, std::move(icon_entries));
+}
+
 }  // namespace notifications

chrome/browser/notifications/scheduler/internal/icon_store.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ -31,8 +32,10 @@ namespace notifications {
 // be loaded into memory.
 class IconStore {
  public:
-  using LoadCallback =
+  using LoadIconCallback =
       base::OnceCallback<void(bool, std::unique_ptr<IconEntry>)>;
+  using LoadIconsCallback =
+      base::OnceCallback<void(bool, std::unique_ptr<std::vector<IconEntry>>)>;
   using InitCallback = base::OnceCallback<void(bool)>;
   using UpdateCallback = base::OnceCallback<void(bool)>;
 
@@ -40,16 +43,27 @@ class IconStore {
   virtual void Init(InitCallback callback) = 0;
 
   // Loads one icon.
-  virtual void Load(const std::string& key, LoadCallback callback) = 0;
+  virtual void Load(const std::string& key, LoadIconCallback callback) = 0;
+
+  // Loads multiple icons.
+  virtual void LoadIcons(const std::vector<std::string>& keys,
+                         LoadIconsCallback callback) = 0;
 
   // Adds one icon to storage.
-  virtual void Add(const std::string& key,
-                   std::unique_ptr<IconEntry> entry,
+  virtual void Add(std::unique_ptr<IconEntry> entry,
                    UpdateCallback callback) = 0;
 
+  // Adds multiple icons to storage.
+  virtual void AddIcons(std::vector<std::unique_ptr<IconEntry>> entries,
+                        UpdateCallback callback) = 0;
+
   // Deletes an icon.
   virtual void Delete(const std::string& key, UpdateCallback callback) = 0;
 
+  // Deletes multiple icons.
+  virtual void DeleteIcons(const std::vector<std::string>& keys,
+                           UpdateCallback callback) = 0;
+
   IconStore() = default;
   virtual ~IconStore() = default;
 
@@ -67,21 +81,31 @@ class IconProtoDbStore : public IconStore {
  private:
   // IconStore implementation.
   void Init(InitCallback callback) override;
-  void Load(const std::string& key, LoadCallback callback) override;
-  void Add(const std::string& key,
-           std::unique_ptr<IconEntry> entry,
-           UpdateCallback callback) override;
+  void Load(const std::string& key, LoadIconCallback callback) override;
+  void LoadIcons(const std::vector<std::string>& keys,
+                 LoadIconsCallback callback) override;
+  void Add(std::unique_ptr<IconEntry> entry, UpdateCallback callback) override;
+  void AddIcons(std::vector<std::unique_ptr<IconEntry>> entries,
+                UpdateCallback callback) override;
   void Delete(const std::string& key, UpdateCallback callback) override;
+  void DeleteIcons(const std::vector<std::string>& keys,
+                   UpdateCallback callback) override;
 
   // Called when the proto database is initialized.
   void OnDbInitialized(InitCallback callback,
                        leveldb_proto::Enums::InitStatus status);
 
   // Called when the icon is retrieved from the database.
-  void OnIconEntryLoaded(LoadCallback callback,
+  void OnIconEntryLoaded(LoadIconCallback callback,
                          bool success,
                          std::unique_ptr<IconEntry> icon_entry);
 
+  // Called when the icon is retrieved from the database.
+  void OnIconEntriesLoaded(
+      LoadIconsCallback callback,
+      bool success,
+      std::unique_ptr<std::vector<IconEntry>> icon_entries);
+
   // The proto database instance that persists data.
   std::unique_ptr<leveldb_proto::ProtoDatabase<proto::Icon, IconEntry>> db_;
 

chrome/browser/notifications/scheduler/internal/icon_store_unittest.cc

@@ -28,8 +28,6 @@
 namespace notifications {
 namespace {
 
-const char kEntryKey[] = "guid_key1";
-const char kEntryKey2[] = "guid_key2";
 const char kEntryId[] = "proto_id_1";
 const char kEntryId2[] = "proto_id_2";
 const char kEntryData[] = "data_1";
@@ -46,7 +44,7 @@ class IconStoreTest : public testing::Test {
     entry.data = kEntryData;
     proto::Icon proto;
     leveldb_proto::DataToProto(&entry, &proto);
-    db_entries_.emplace(kEntryKey, proto);
+    db_entries_.emplace(kEntryId, proto);
 
     auto db =
         std::make_unique<leveldb_proto::test::FakeDB<proto::Icon, IconEntry>>(
@@ -107,7 +105,7 @@ TEST_F(IconStoreTest, InitFailed) {
 
 TEST_F(IconStoreTest, LoadOne) {
   InitDb();
-  Load(kEntryKey);
+  Load(kEntryId);
   db()->GetCallback(true);
 
   // Verify data is loaded.
@@ -118,7 +116,7 @@ TEST_F(IconStoreTest, LoadOne) {
 
 TEST_F(IconStoreTest, LoadFailed) {
   InitDb();
-  Load(kEntryKey);
+  Load(kEntryId);
   db()->GetCallback(false);
 
   // Verify load failure.
@@ -133,11 +131,11 @@ TEST_F(IconStoreTest, MAYBE_Add) {
   new_entry->uuid = kEntryId2;
   new_entry->data = kEntryData;
 
-  store()->Add(kEntryKey2, std::move(new_entry), base::DoNothing());
+  store()->Add(std::move(new_entry), base::DoNothing());
   db()->UpdateCallback(true);
 
   // Verify the entry is added.
-  Load(kEntryKey2);
+  Load(kEntryId2);
   db()->GetCallback(true);
   EXPECT_TRUE(load_result());
   VerifyEntry(kEntryId2, kEntryData);
@@ -150,11 +148,11 @@ TEST_F(IconStoreTest, MAYBE_AddDuplicate) {
   new_entry->uuid = kEntryId;
   new_entry->data = kEntryData2;
 
-  store()->Add(kEntryKey, std::move(new_entry), base::DoNothing());
+  store()->Add(std::move(new_entry), base::DoNothing());
   db()->UpdateCallback(true);
 
   // Add a duplicate id is currently allowed, we just update the entry.
-  Load(kEntryKey);
+  Load(kEntryId);
   db()->GetCallback(true);
   EXPECT_TRUE(load_result());
   VerifyEntry(kEntryId, kEntryData2);
@@ -164,12 +162,12 @@ TEST_F(IconStoreTest, Delete) {
   InitDb();
 
   // Delete the only entry.
-  store()->Delete(kEntryKey,
+  store()->Delete(kEntryId,
                   base::BindOnce([](bool success) { EXPECT_TRUE(success); }));
   db()->UpdateCallback(true);
 
   // No entry can be loaded, move nullptr as result.
-  Load(kEntryKey);
+  Load(kEntryId);
   db()->GetCallback(true);
   EXPECT_FALSE(loaded_entry());
   EXPECT_TRUE(load_result());

chrome/browser/notifications/scheduler/internal/scheduled_notification_manager.cc

@@ -232,7 +232,7 @@ class ScheduledNotificationManagerImpl : public ScheduledNotificationManager {
     icon_entry->uuid = icon_uuid;
     icon_entry->data = std::move(encoded_data);
     icon_store_->Add(
-        icon_uuid, std::move(icon_entry),
+        std::move(icon_entry),
         base::BindOnce(&ScheduledNotificationManagerImpl::OnIconAdded,
                        weak_ptr_factory_.GetWeakPtr(), type, std::move(guid),
                        std::move(icon_uuid)));

chrome/browser/notifications/scheduler/internal/scheduled_notification_manager_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/notifications/scheduler/internal/scheduled_notification_manager.h"
 
+#include <utility>
+#include <vector>
+
 #include "base/bind.h"
 #include "base/guid.h"
 #include "base/run_loop.h"
@@ -66,12 +69,19 @@ class MockIconStore : public IconStore {
   MockIconStore() {}
 
   MOCK_METHOD1(Init, void(IconStore::InitCallback));
-  MOCK_METHOD2(Load, void(const std::string&, IconStore::LoadCallback));
-  MOCK_METHOD3(Add,
-               void(const std::string&,
-                    std::unique_ptr<IconEntry>,
+  MOCK_METHOD2(Load, void(const std::string&, IconStore::LoadIconCallback));
+  MOCK_METHOD2(LoadIcons,
+               void(const std::vector<std::string>&,
+                    IconStore::LoadIconsCallback));
+  MOCK_METHOD2(Add,
+               void(std::unique_ptr<IconEntry>, IconStore::UpdateCallback));
+  MOCK_METHOD2(AddIcons,
+               void(std::vector<std::unique_ptr<IconEntry>>,
                     IconStore::UpdateCallback));
   MOCK_METHOD2(Delete, void(const std::string&, IconStore::UpdateCallback));
+  MOCK_METHOD2(DeleteIcons,
+               void(const std::vector<std::string>&,
+                    IconStore::UpdateCallback));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockIconStore);