From f04fb3ae6a3ce9d64b42251b52700dcc25598af5 Mon Sep 17 00:00:00 2001
From: Maxim Pimenov <m@maps.me>
Date: Thu, 25 Jul 2019 16:48:41 +0300
Subject: [PATCH] [search] [bookmarks] Disabled indexing of bookmark
 descriptions by default.

---
 search/bookmarks/data.hpp                     | 12 +++++--
 search/bookmarks/processor.cpp                | 13 +++++++-
 search/bookmarks/processor.hpp                |  7 ++++
 .../bookmarks_processor_tests.cpp             | 33 +++++++++++++++++++
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/search/bookmarks/data.hpp b/search/bookmarks/data.hpp
index 10811b779c..c4ef8250dc 100644
--- a/search/bookmarks/data.hpp
+++ b/search/bookmarks/data.hpp
@@ -26,19 +26,27 @@ struct Data
 
   Data(kml::BookmarkData const & bookmarkData)
     : m_name(kml::GetDefaultStr(bookmarkData.m_name))
-    // todo(@m) Do not add descriptions to index in Version 0.
     , m_description(kml::GetDefaultStr(bookmarkData.m_description))
   {
   }
 
   template <typename Fn>
-  void ForEachToken(Fn && fn) const
+  void ForEachNameToken(Fn && fn) const
   {
     auto withDefaultLang = [&](strings::UniString const & token) {
       fn(StringUtf8Multilang::kDefaultCode, token);
     };
 
     ForEachNormalizedToken(m_name, withDefaultLang);
+  }
+
+  template <typename Fn>
+  void ForEachDescriptionToken(Fn && fn) const
+  {
+    auto withDefaultLang = [&](strings::UniString const & token) {
+      fn(StringUtf8Multilang::kDefaultCode, token);
+    };
+
     ForEachNormalizedToken(m_description, withDefaultLang);
   }
 
diff --git a/search/bookmarks/processor.cpp b/search/bookmarks/processor.cpp
index 9a2deb8d7e..ba9a890aeb 100644
--- a/search/bookmarks/processor.cpp
+++ b/search/bookmarks/processor.cpp
@@ -78,14 +78,25 @@ Processor::Processor(Emitter & emitter, base::Cancellable const & cancellable)
 {
 }
 
+void Processor::EnableIndexingOfDescriptions(bool enable)
+{
+  m_indexDescriptions = enable;
+}
+
 void Processor::Add(Id const & id, Doc const & doc)
 {
   ASSERT_EQUAL(m_docs.count(id), 0, ());
 
   DocVec::Builder builder;
-  doc.ForEachToken(
+  doc.ForEachNameToken(
       [&](int8_t /* lang */, strings::UniString const & token) { builder.Add(token); });
 
+  if (m_indexDescriptions)
+  {
+    doc.ForEachDescriptionToken(
+        [&](int8_t /* lang */, strings::UniString const & token) { builder.Add(token); });
+  }
+
   DocVec const docVec(builder);
 
   m_index.Add(id, DocVecWrapper(docVec));
diff --git a/search/bookmarks/processor.hpp b/search/bookmarks/processor.hpp
index 40b1872ec5..2c9a49bae3 100644
--- a/search/bookmarks/processor.hpp
+++ b/search/bookmarks/processor.hpp
@@ -42,6 +42,11 @@ public:
   Processor(Emitter & emitter, base::Cancellable const & cancellable);
   ~Processor() override = default;
 
+  // By default, only bookmark names are indexed. This method
+  // should be used to enable or disable indexing bookmarks
+  // by their descriptions.
+  void EnableIndexingOfDescriptions(bool enable);
+
   void Add(Id const & id, Doc const & doc);
   void Erase(Id const & id);
 
@@ -76,6 +81,8 @@ private:
   Index m_index;
   std::unordered_map<Id, DocVec> m_docs;
 
+  bool m_indexDescriptions = false;
+
   // Currently a bookmark can belong to at most one group
   // but in the future it is possible for a single bookmark to be
   // attached to multiple groups.
diff --git a/search/search_tests/bookmarks_processor_tests.cpp b/search/search_tests/bookmarks_processor_tests.cpp
index 45b5e2d11f..5b8d52d9d7 100644
--- a/search/search_tests/bookmarks_processor_tests.cpp
+++ b/search/search_tests/bookmarks_processor_tests.cpp
@@ -25,6 +25,8 @@ class BookmarksProcessorTest
 public:
   BookmarksProcessorTest() : m_processor(m_emitter, m_cancellable) {}
 
+  Processor & GetProcessor() { return m_processor; }
+
   void Add(Id const & id, Doc const & doc) { m_processor.Add(id, doc); }
 
   void Erase(Id const & id) { m_processor.Erase(id); }
@@ -71,6 +73,8 @@ protected:
 
 UNIT_CLASS_TEST(BookmarksProcessorTest, Smoke)
 {
+  GetProcessor().EnableIndexingOfDescriptions(true);
+
   Add(10, {"Double R Diner" /* name */,
            "They've got a cherry pie there that'll kill ya!" /* description */});
 
@@ -94,4 +98,33 @@ UNIT_CLASS_TEST(BookmarksProcessorTest, Smoke)
   AttachToGroup(20, 0);
   TEST_EQUAL(Search("place", GroupId{0}), Ids({20, 18}), ());
 }
+
+UNIT_CLASS_TEST(BookmarksProcessorTest, IndexDescriptions)
+{
+  GetProcessor().EnableIndexingOfDescriptions(true);
+
+  Add(10, {"Double R Diner" /* name */,
+           "They've got a cherry pie there that'll kill ya!" /* description */});
+  TEST_EQUAL(Search("diner"), Ids({10}), ());
+  TEST_EQUAL(Search("cherry pie"), Ids({10}), ());
+
+  Erase(10);
+  TEST_EQUAL(Search("diner"), Ids(), ());
+  TEST_EQUAL(Search("cherry pie"), Ids{}, ());
+
+  GetProcessor().EnableIndexingOfDescriptions(false);
+  Add(10, {"Double R Diner" /* name */,
+           "They've got a cherry pie there that'll kill ya!" /* description */});
+  TEST_EQUAL(Search("diner"), Ids({10}), ());
+  TEST_EQUAL(Search("cherry pie"), Ids(), ());
+
+  // Already indexed results don't change.
+  GetProcessor().EnableIndexingOfDescriptions(true);
+  TEST_EQUAL(Search("diner"), Ids({10}), ());
+  TEST_EQUAL(Search("cherry pie"), Ids(), ());
+
+  Erase(10);
+  TEST_EQUAL(Search("diner"), Ids(), ());
+  TEST_EQUAL(Search("cherry pie"), Ids{}, ());
+}
 }  // namespace