[search] Get rid of duplication in the downloader search.

map/framework.cpp

@@ -12,6 +12,7 @@
 #include "routing/route.hpp"
 #include "routing/routing_algorithm.hpp"
 
+#include "search/downloader_search_callback.hpp"
 #include "search/engine.hpp"
 #include "search/everywhere_search_params.hpp"
 #include "search/geometry_utils.hpp"
@@ -1093,22 +1094,6 @@ void Framework::UpdateUserViewportChanged()
   Search(params);
 }
 
-bool Framework::GetGroupCountryIdFromFeature(FeatureType const & ft, string & name) const
-{
-  int8_t langIndices[] = { StringUtf8Multilang::kEnglishCode,
-                           StringUtf8Multilang::kDefaultCode,
-                           StringUtf8Multilang::kInternationalCode };
-
-  for (auto const langIndex : langIndices)
-  {
-    if (!ft.GetName(langIndex, name))
-      continue;
-    if (Storage().IsCoutryIdCountryTreeInnerNode(name))
-      return true;
-  }
-  return false;
-}
-
 bool Framework::SearchEverywhere(search::EverywhereSearchParams const & params)
 {
   search::SearchParams p;
@@ -1159,56 +1144,8 @@ bool Framework::SearchInDownloader(DownloaderSearchParams const & params)
   p.SetMode(search::Mode::Downloader);
   p.SetSuggestsEnabled(false);
   p.SetForceSearch(true);
-  p.m_onResults = [this, params](search::Results const & results)
-  {
-    DownloaderSearchResults downloaderSearchResults;
-    for (auto const & result : results)
-    {
-      if (!result.HasPoint())
-        continue;
-
-      if (result.GetResultType() != search::Result::RESULT_LATLON)
-      {
-        FeatureID const & fid = result.GetFeatureID();
-        Index::FeaturesLoaderGuard loader(m_model.GetIndex(), fid.m_mwmId);
-        FeatureType ft;
-        if (!loader.GetFeatureByIndex(fid.m_index, ft))
-        {
-          LOG(LERROR, ("Feature can't be loaded:", fid));
-          continue;
-        }
-
-        ftypes::Type const type = ftypes::IsLocalityChecker::Instance().GetType(ft);
-
-        if (type == ftypes::COUNTRY || type == ftypes::STATE)
-        {
-          string groupFeatureName;
-          if (GetGroupCountryIdFromFeature(ft, groupFeatureName))
-          {
-            downloaderSearchResults.m_results.emplace_back(groupFeatureName,
-                                                           result.GetString() /* m_matchedName */);
-            continue;
-          }
-        }
-      }
-
-      auto const & mercator = result.GetFeatureCenter();
-      TCountryId const & countryId = CountryInfoGetter().GetRegionCountryId(mercator);
-      if (countryId == kInvalidCountryId)
-        continue;
-      downloaderSearchResults.m_results.emplace_back(countryId,
-                                                     result.GetString() /* m_matchedName */);
-    }
-    downloaderSearchResults.m_query = params.m_query;
-    downloaderSearchResults.m_endMarker = results.IsEndMarker();
-
-    if (params.m_onResults)
-    {
-      GetPlatform().RunOnGuiThread(
-          [params, downloaderSearchResults]() { params.m_onResults(downloaderSearchResults); });
-    }
-  };
-
+  p.m_onResults = search::DownloaderSearchCallback(m_model.GetIndex(), CountryInfoGetter(),
+                                                   params);
   return Search(p);
 }
 

map/framework.hpp

@@ -457,7 +457,6 @@ private:
 
   void OnUpdateGpsTrackPointsCallback(vector<pair<size_t, location::GpsTrackInfo>> && toAdd,
                                       pair<size_t, size_t> const & toRemove);
-  bool GetGroupCountryIdFromFeature(FeatureType const & ft, string & name) const;
 
 public:
   using TSearchRequest = search::QuerySaver::TSearchRequest;

search/downloader_search_callback.cpp

@@ -0,0 +1,101 @@
+#include "search/downloader_search_callback.hpp"
+
+#include "search/result.hpp"
+
+#include "storage/country_info_getter.hpp"
+#include "storage/storage.hpp"
+
+#include "indexer/index.hpp"
+
+#include "base/string_utils.hpp"
+
+namespace
+{
+bool GetGroupCountryIdFromFeature(FeatureType const & ft, string & name)
+{
+  int8_t langIndices[] = {StringUtf8Multilang::kEnglishCode, StringUtf8Multilang::kDefaultCode,
+                          StringUtf8Multilang::kInternationalCode};
+
+  for (auto const langIndex : langIndices)
+  {
+    if (!ft.GetName(langIndex, name))
+      continue;
+    if (storage::Storage().IsCoutryIdCountryTreeInnerNode(name))
+      return true;
+  }
+  return false;
+}
+}  // namespace
+
+namespace search
+{
+DownloaderSearchCallback::DownloaderSearchCallback(Index const & index,
+                                                   storage::CountryInfoGetter const & infoGetter,
+                                                   storage::DownloaderSearchParams params)
+  : m_index(index), m_infoGetter(infoGetter), m_params(move(params))
+{
+}
+
+void DownloaderSearchCallback::operator()(search::Results const & results)
+{
+  storage::DownloaderSearchResults downloaderSearchResults;
+
+  for (auto const & result : results)
+  {
+    if (!result.HasPoint())
+      continue;
+
+    if (result.GetResultType() != search::Result::RESULT_LATLON)
+    {
+      FeatureID const & fid = result.GetFeatureID();
+      Index::FeaturesLoaderGuard loader(m_index, fid.m_mwmId);
+      FeatureType ft;
+      if (!loader.GetFeatureByIndex(fid.m_index, ft))
+      {
+        LOG(LERROR, ("Feature can't be loaded:", fid));
+        continue;
+      }
+
+      ftypes::Type const type = ftypes::IsLocalityChecker::Instance().GetType(ft);
+
+      if (type == ftypes::COUNTRY || type == ftypes::STATE)
+      {
+        string groupFeatureName;
+        if (GetGroupCountryIdFromFeature(ft, groupFeatureName))
+        {
+          storage::DownloaderSearchResult downloaderResult(groupFeatureName,
+                                                           result.GetString() /* m_matchedName */);
+          if (m_uniqueResults.find(downloaderResult) == m_uniqueResults.end())
+          {
+            m_uniqueResults.insert(downloaderResult);
+            downloaderSearchResults.m_results.push_back(downloaderResult);
+          }
+          continue;
+        }
+      }
+    }
+
+    auto const & mercator = result.GetFeatureCenter();
+    storage::TCountryId const & countryId = m_infoGetter.GetRegionCountryId(mercator);
+    if (countryId == storage::kInvalidCountryId)
+      continue;
+
+    storage::DownloaderSearchResult downloaderResult(countryId,
+                                                     result.GetString() /* m_matchedName */);
+    if (m_uniqueResults.find(downloaderResult) == m_uniqueResults.end())
+    {
+      m_uniqueResults.insert(downloaderResult);
+      downloaderSearchResults.m_results.push_back(downloaderResult);
+    }
+  }
+
+  downloaderSearchResults.m_query = m_params.m_query;
+  downloaderSearchResults.m_endMarker = results.IsEndMarker();
+
+  if (m_params.m_onResults)
+  {
+    GetPlatform().RunOnGuiThread(
+        [this, downloaderSearchResults]() { m_params.m_onResults(downloaderSearchResults); });
+  }
+}
+}  // namespace search

search/downloader_search_callback.hpp

@@ -0,0 +1,42 @@
+#pragma once
+
+#include "storage/downloader_search_params.hpp"
+
+#include "std/set.hpp"
+#include "std/string.hpp"
+
+class Index;
+
+namespace storage
+{
+class CountryInfoGetter;
+}  // namespace storage
+
+// todo(@m)
+// add tests
+
+namespace search
+{
+class Results;
+
+// An on-results callback that should be used for the search in downloader.
+//
+// *NOTE* the class is NOT thread safe.
+class DownloaderSearchCallback
+{
+public:
+  using TOnResults = storage::DownloaderSearchParams::TOnResults;
+
+  DownloaderSearchCallback(Index const & index, storage::CountryInfoGetter const & infoGetter,
+                           storage::DownloaderSearchParams params);
+
+  void operator()(search::Results const & results);
+
+private:
+  set<storage::DownloaderSearchResult> m_uniqueResults;
+
+  Index const & m_index;
+  storage::CountryInfoGetter const & m_infoGetter;
+  storage::DownloaderSearchParams m_params;
+};
+}  // namespace search

search/search.pro

@@ -15,6 +15,7 @@ HEADERS += \
     cbv.hpp \
     common.hpp \
     displayed_categories.hpp \
+    downloader_search_callback.hpp \
     dummy_rank_table.hpp \
     engine.hpp \
     everywhere_search_params.hpp \
@@ -79,6 +80,7 @@ SOURCES += \
     approximate_string_match.cpp \
     cbv.cpp \
     displayed_categories.cpp \
+    downloader_search_callback.cpp \
     dummy_rank_table.cpp \
     engine.cpp \
     features_filter.cpp \

storage/downloader_search_params.hpp

@@ -15,6 +15,18 @@ struct DownloaderSearchResult
   {
   }
 
+  bool operator==(DownloaderSearchResult const & rhs) const
+  {
+    return m_countryId == rhs.m_countryId && m_matchedName == rhs.m_matchedName;
+  }
+
+  bool operator<(DownloaderSearchResult const & rhs) const
+  {
+    if (m_countryId != rhs.m_countryId)
+      return m_countryId < rhs.m_countryId;
+    return m_matchedName < rhs.m_matchedName;
+  }
+
   TCountryId m_countryId;
   /// \brief |m_matchedName| is a name of found feature in case of searching in World.mwm
   /// and is a local name of mwm (group or leaf) in case of searching in country tree.