[Upboarding]: ProtoConversion and unittest for QueryTileStore.

- Implemented proto_conversion for query_tile_entry.
- Added unit test for proto_conversion and query_tile_store.
- Fixed some lint/nits.

Bug: 1060805c
Change-Id: Ife1245aa6749ad2e534351c8b66a7e819adf1d75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2103656
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750880}

chrome/browser/upboarding/query_tiles/BUILD.gn

@@ -115,3 +115,11 @@ if (is_android) {
     ]
   }
 }
+
+source_set("unit_tests") {
+  testonly = true
+
+  sources = []
+
+  deps = [ "//chrome/browser/upboarding/query_tiles/internal:unit_tests" ]
+}

chrome/browser/upboarding/query_tiles/internal/BUILD.gn

@@ -11,6 +11,8 @@ source_set("internal") {
   sources = [
     "image_loader.cc",
     "image_loader.h",
+    "proto_conversion.cc",
+    "proto_conversion.h",
     "query_tile_store.cc",
     "query_tile_store.h",
     "store.h",
@@ -25,3 +27,24 @@ source_set("internal") {
     "//url",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "proto_conversion_unittest.cc",
+    "query_tile_store_unittest.cc",
+  ]
+
+  deps = [
+    "//base",
+    "//chrome/browser/upboarding/query_tiles:public",
+    "//chrome/browser/upboarding/query_tiles/internal",
+    "//chrome/browser/upboarding/query_tiles/proto",
+    "//components/leveldb_proto",
+    "//components/leveldb_proto:test_support",
+    "//skia",
+    "//testing/gmock",
+    "//testing/gtest",
+    "//url",
+  ]
+}

chrome/browser/upboarding/query_tiles/internal/proto_conversion.cc

+// Copyright 2020 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 "chrome/browser/upboarding/query_tiles/internal/proto_conversion.h"
+
+#include "base/strings/utf_string_conversions.h"
+
+namespace upboarding {
+
+void QueryTileEntryToProto(
+    upboarding::QueryTileEntry* entry,
+    upboarding::query_tiles::proto::QueryTileEntry* proto) {
+  proto->set_id(entry->id);
+  proto->set_query_text(entry->query_text);
+  proto->set_display_text(entry->display_text);
+  proto->set_accessibility_text(entry->accessibility_text);
+
+  // Set ImageMetadatas.
+  for (auto& image : entry->image_metadatas) {
+    auto* data = proto->add_image_metadatas();
+    data->set_id(image.id);
+    data->set_url(image.url.spec());
+  }
+
+  // Set Ids of children.
+  for (const auto& child : entry->children) {
+    proto->add_children(child);
+  }
+}
+
+void QueryTileEntryFromProto(
+    upboarding::query_tiles::proto::QueryTileEntry* proto,
+    upboarding::QueryTileEntry* entry) {
+  entry->id = proto->id();
+  entry->query_text = proto->query_text();
+  entry->display_text = proto->display_text();
+  entry->accessibility_text = proto->accessibility_text();
+
+  for (const auto& image_md : proto->image_metadatas()) {
+    entry->image_metadatas.emplace_back(image_md.id(), GURL(image_md.url()));
+  }
+
+  for (const auto& child : proto->children()) {
+    entry->children.emplace(child);
+  }
+}
+
+}  // namespace upboarding

chrome/browser/upboarding/query_tiles/internal/proto_conversion.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_UPBOARDING_QUERY_TILES_INTERNAL_PROTO_CONVERSION_H_
+#define CHROME_BROWSER_UPBOARDING_QUERY_TILES_INTERNAL_PROTO_CONVERSION_H_
+
+#include "chrome/browser/upboarding/query_tiles/proto/query_tile_entry.pb.h"
+#include "chrome/browser/upboarding/query_tiles/query_tile_entry.h"
+
+namespace upboarding {
+
+// Converts a QueryTileEntry to proto.
+void QueryTileEntryToProto(
+    upboarding::QueryTileEntry* entry,
+    upboarding::query_tiles::proto::QueryTileEntry* proto);
+
+// Converts a proto to QueryTileEntry.
+void QueryTileEntryFromProto(
+    upboarding::query_tiles::proto::QueryTileEntry* proto,
+    upboarding::QueryTileEntry* entry);
+
+}  // namespace upboarding
+
+#endif  // CHROME_BROWSER_UPBOARDING_QUERY_TILES_INTERNAL_PROTO_CONVERSION_H_

chrome/browser/upboarding/query_tiles/internal/proto_conversion_unittest.cc

+// Copyright 2020 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 "chrome/browser/upboarding/query_tiles/internal/proto_conversion.h"
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace upboarding {
+namespace {
+
+void TestQueryTileEntryConversion(QueryTileEntry* expected) {
+  DCHECK(expected);
+  upboarding::query_tiles::proto::QueryTileEntry proto;
+  QueryTileEntry actual;
+  QueryTileEntryToProto(expected, &proto);
+  QueryTileEntryFromProto(&proto, &actual);
+  EXPECT_EQ(*expected, actual);
+}
+
+TEST(ProtoConversionTest, QueryTileEntryConversion) {
+  QueryTileEntry entry;
+  entry.id = "test-guid-001";
+  entry.query_text = "test query str";
+  entry.display_text = "test display text";
+  entry.accessibility_text = "read this test display text";
+  entry.image_metadatas.emplace_back("image-test-id-1",
+                                     GURL("www.example.com"));
+  entry.image_metadatas.emplace_back("image-test-id-2",
+                                     GURL("www.fakeurl.com"));
+  entry.children.emplace("test-guid-002");
+  entry.children.emplace("test-guid-003");
+  entry.children.emplace("test-guid-004");
+  TestQueryTileEntryConversion(&entry);
+}
+
+}  // namespace
+
+}  // namespace upboarding

chrome/browser/upboarding/query_tiles/internal/query_tile_store.cc

@@ -2,20 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/upboarding/query_tiles/internal/query_tile_store.h"
+
 #include <utility>
 
-#include "chrome/browser/upboarding/query_tiles/internal/query_tile_store.h"
+#include "chrome/browser/upboarding/query_tiles/internal/proto_conversion.h"
 
 namespace leveldb_proto {
 
 void DataToProto(upboarding::QueryTileEntry* data,
                  upboarding::query_tiles::proto::QueryTileEntry* proto) {
-  NOTIMPLEMENTED();
+  QueryTileEntryToProto(data, proto);
 }
 
 void ProtoToData(upboarding::query_tiles::proto::QueryTileEntry* proto,
                  upboarding::QueryTileEntry* data) {
-  NOTIMPLEMENTED();
+  QueryTileEntryFromProto(proto, data);
 }
 
 }  // namespace leveldb_proto
@@ -52,7 +54,7 @@ void QueryTileStore::Delete(const std::string& key, DeleteCallback callback) {
 void QueryTileStore::OnDbInitialized(LoadCallback callback,
                                      leveldb_proto::Enums::InitStatus status) {
   if (status != leveldb_proto::Enums::InitStatus::kOK) {
-    std::move(callback).Run(false, {});
+    std::move(callback).Run(false, KeysAndEntries());
     return;
   }
 

chrome/browser/upboarding/query_tiles/internal/query_tile_store.h

@@ -39,7 +39,7 @@ class QueryTileStore : public Store<QueryTileEntry> {
   ~QueryTileStore() override;
 
   QueryTileStore(const QueryTileStore& other) = delete;
-  QueryTileStore& operator=(QueryTileStore& other) = delete;
+  QueryTileStore& operator=(const QueryTileStore& other) = delete;
 
  private:
   using KeyEntryVector = std::vector<std::pair<std::string, QueryTileEntry>>;
@@ -49,7 +49,7 @@ class QueryTileStore : public Store<QueryTileEntry> {
   // Store<QueryTileEntry> implementation.
   void InitAndLoad(LoadCallback callback) override;
   void Update(const std::string& key,
-              const QueryTileEntry& node,
+              const QueryTileEntry& entry,
               UpdateCallback callback) override;
   void Delete(const std::string& key, DeleteCallback callback) override;
 

chrome/browser/upboarding/query_tiles/internal/query_tile_store_unittest.cc

+// Copyright 2020 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 "chrome/browser/upboarding/query_tiles/internal/query_tile_store.h"
+
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/test/task_environment.h"
+#include "chrome/browser/upboarding/query_tiles/internal/proto_conversion.h"
+#include "components/leveldb_proto/public/proto_database.h"
+#include "components/leveldb_proto/testing/fake_db.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using leveldb_proto::test::FakeDB;
+using InitStatus = leveldb_proto::Enums::InitStatus;
+
+namespace upboarding {
+namespace {
+
+class QueryTileStoreTest : public testing::Test {
+ public:
+  using QueryTileEntryProto = query_tiles::proto::QueryTileEntry;
+  using EntriesMap = std::map<std::string, std::unique_ptr<QueryTileEntry>>;
+  using ProtoMap = std::map<std::string, QueryTileEntryProto>;
+  using KeysAndEntries = std::map<std::string, QueryTileEntry>;
+  using TestEntries = std::vector<QueryTileEntry>;
+
+  QueryTileStoreTest() : load_result_(false), db_(nullptr) {}
+  ~QueryTileStoreTest() override = default;
+
+  void SetUp() override {}
+
+  QueryTileStoreTest(const QueryTileStoreTest& other) = delete;
+  QueryTileStoreTest& operator=(const QueryTileStoreTest& other) = delete;
+
+ protected:
+  void Init(TestEntries input, InitStatus status) {
+    CreateTestDbEntries(std::move(input));
+    auto db = std::make_unique<FakeDB<QueryTileEntryProto, QueryTileEntry>>(
+        &db_entries_);
+    db_ = db.get();
+    store_ = std::make_unique<QueryTileStore>(std::move(db));
+    store_->InitAndLoad(base::BindOnce(&QueryTileStoreTest::OnEntriesLoaded,
+                                       base::Unretained(this)));
+    db_->InitStatusCallback(status);
+  }
+
+  void OnEntriesLoaded(bool success, EntriesMap loaded_entries) {
+    load_result_ = success;
+    in_memory_entries_ = std::move(loaded_entries);
+  }
+
+  void CreateTestDbEntries(TestEntries input) {
+    for (auto entry : input) {
+      QueryTileEntryProto proto;
+      upboarding::QueryTileEntryToProto(&entry, &proto);
+      db_entries_.emplace(entry.id, proto);
+    }
+  }
+
+  // Verifies the entries in the db is |expected|.
+  void VerifyDataInDb(std::unique_ptr<KeysAndEntries> expected) {
+    db_->LoadKeysAndEntries(
+        base::BindOnce(&QueryTileStoreTest::OnVerifyDataInDb,
+                       base::Unretained(this), std::move(expected)));
+    db_->LoadCallback(true);
+  }
+
+  void OnVerifyDataInDb(std::unique_ptr<KeysAndEntries> expected,
+                        bool success,
+                        std::unique_ptr<KeysAndEntries> loaded_entries) {
+    EXPECT_TRUE(success);
+    DCHECK(expected);
+    DCHECK(loaded_entries);
+    EXPECT_EQ(*expected.get(), *loaded_entries.get());
+  }
+
+  bool load_result() const { return load_result_; }
+  const EntriesMap& in_memory_entries() const { return in_memory_entries_; }
+  FakeDB<QueryTileEntryProto, QueryTileEntry>* db() { return db_; }
+  Store<QueryTileEntry>* store() { return store_.get(); }
+
+ private:
+  base::test::TaskEnvironment task_environment_;
+  bool load_result_;
+  EntriesMap in_memory_entries_;
+  ProtoMap db_entries_;
+  FakeDB<QueryTileEntryProto, QueryTileEntry>* db_;
+  std::unique_ptr<Store<QueryTileEntry>> store_;
+};
+
+// Test Initializing and loading an empty database .
+TEST_F(QueryTileStoreTest, InitSuccessEmptyDb) {
+  auto test_data = TestEntries();
+  Init(test_data, InitStatus::kOK);
+  db()->LoadCallback(true);
+  EXPECT_EQ(load_result(), true);
+  EXPECT_TRUE(in_memory_entries().empty());
+}
+
+// Test Initializing and loading a non-empty database.
+TEST_F(QueryTileStoreTest, InitSuccessWithData) {
+  auto test_data = TestEntries();
+  auto test_entry = QueryTileEntry();
+  test_entry.id = "test-id";
+  test_data.emplace_back(test_entry);
+  Init(test_data, InitStatus::kOK);
+  db()->LoadCallback(true);
+  EXPECT_EQ(load_result(), true);
+  EXPECT_EQ(in_memory_entries().size(), 1u);
+  auto actual = in_memory_entries().begin();
+  EXPECT_EQ(actual->first, test_entry.id);
+  EXPECT_EQ(*(actual->second.get()), test_entry);
+}
+
+// Test Initializing and loading a non-empty database failed.
+TEST_F(QueryTileStoreTest, InitFailedWithData) {
+  auto test_data = TestEntries();
+  auto test_entry = QueryTileEntry();
+  test_entry.id = "test-id";
+  test_data.emplace_back(test_entry);
+  Init(test_data, InitStatus::kOK);
+  db()->LoadCallback(false);
+  EXPECT_EQ(load_result(), false);
+  EXPECT_TRUE(in_memory_entries().empty());
+}
+
+// Test adding and updating.
+TEST_F(QueryTileStoreTest, AddAndUpdateDataSuccess) {
+  auto test_data = TestEntries();
+  Init(test_data, InitStatus::kOK);
+  db()->LoadCallback(true);
+  EXPECT_EQ(load_result(), true);
+  EXPECT_TRUE(in_memory_entries().empty());
+
+  // Add an entry failed.
+  auto test_entry_1 = QueryTileEntry();
+  test_entry_1.id = "test_entry_id_1";
+  test_entry_1.display_text = "test_entry_test_display_text";
+  store()->Update(test_entry_1.id, test_entry_1,
+                  base::BindOnce([](bool success) { EXPECT_FALSE(success); }));
+  db()->UpdateCallback(false);
+
+  // Add an entry successfully.
+  store()->Update(test_entry_1.id, test_entry_1,
+                  base::BindOnce([](bool success) { EXPECT_TRUE(success); }));
+  db()->UpdateCallback(true);
+
+  auto expected = std::make_unique<KeysAndEntries>();
+  expected->emplace(test_entry_1.id, test_entry_1);
+  VerifyDataInDb(std::move(expected));
+
+  // Update an existing entry successfully.
+  test_entry_1.display_text = "test_entry_test_display_text_updated";
+  store()->Update(test_entry_1.id, test_entry_1,
+                  base::BindOnce([](bool success) { EXPECT_TRUE(success); }));
+  db()->UpdateCallback(true);
+
+  expected = std::make_unique<KeysAndEntries>();
+  expected->emplace(test_entry_1.id, test_entry_1);
+  VerifyDataInDb(std::move(expected));
+}
+
+// Test Deleting from db.
+TEST_F(QueryTileStoreTest, DeleteSuccess) {
+  auto test_data = TestEntries();
+  auto test_entry = QueryTileEntry();
+  test_entry.id = "test-id";
+  test_data.emplace_back(test_entry);
+  Init(test_data, InitStatus::kOK);
+  db()->LoadCallback(true);
+  EXPECT_EQ(load_result(), true);
+  EXPECT_EQ(in_memory_entries().size(), 1u);
+  auto actual = in_memory_entries().begin();
+  EXPECT_EQ(actual->first, test_entry.id);
+  EXPECT_EQ(*(actual->second.get()), test_entry);
+
+  store()->Delete(test_entry.id,
+                  base::BindOnce([](bool success) { EXPECT_TRUE(success); }));
+  db()->UpdateCallback(true);
+  // No entry is expected in db.
+  auto expected = std::make_unique<KeysAndEntries>();
+  VerifyDataInDb(std::move(expected));
+}
+
+}  // namespace
+}  // namespace upboarding

chrome/browser/upboarding/query_tiles/proto/query_tile_entry.proto

@@ -31,7 +31,7 @@ message QueryTileEntry {
   string display_text = 3;
 
   // Counterpart of |display_text| for accessibility purposes.
-  string accessiblity_text = 4;
+  string accessibility_text = 4;
 
   // A list of image metadatas.
   repeated ImageMetadata image_metadatas = 5;

chrome/browser/upboarding/query_tiles/query_tile_entry.cc

@@ -29,8 +29,7 @@ bool QueryTileEntry::operator==(const QueryTileEntry& other) const {
   return id == other.id && query_text == other.query_text &&
          display_text == other.display_text &&
          accessibility_text == other.accessibility_text &&
-         image_metadatas.size() == other.image_metadatas.size() &&
-         children.size() == other.children.size();
+         image_metadatas == other.image_metadatas && children == other.children;
 }
 
 }  // namespace upboarding

chrome/test/BUILD.gn

@@ -3651,6 +3651,7 @@ test("unit_tests") {
     "//chrome/browser/notifications:unit_tests",
     "//chrome/browser/paint_preview/services:unit_tests",
     "//chrome/browser/payments:unittests",
+    "//chrome/browser/upboarding/query_tiles:unit_tests",
     "//chrome/browser/updates/announcement_notification:unit_tests",
     "//chrome/common:test_support",
     "//chrome/common/media_router:test_support",