[indexer] Removed indexer dependency on search.

editor/editor_config.hpp

@@ -2,6 +2,7 @@
 
 #include "indexer/feature_meta.hpp"
 
+#include "std/mutex.hpp"
 #include "std/shared_ptr.hpp"
 #include "std/string.hpp"
 #include "std/vector.hpp"
@@ -59,10 +60,24 @@ class EditorConfigWrapper
 public:
   EditorConfigWrapper() = default;
 
-  void Set(shared_ptr<EditorConfig> config) { atomic_store(&m_config, config); }
-  shared_ptr<EditorConfig const> Get() const { return atomic_load(&m_config); }
+  void Set(shared_ptr<EditorConfig> config)
+  {
+    lock_guard<mutex> lock(m_mu);
+    m_config = config;
+  }
+
+  shared_ptr<EditorConfig const> Get() const
+  {
+    lock_guard<mutex> lock(m_mu);
+    return m_config;
+  }
 
 private:
+  // It's possible to use atomic_{load|store} here instead of mutex,
+  // but seems that libstdc++4.9 doesn't support it. Need to rewrite
+  // this code as soon as libstdc++5 will be ready for lastest Debian
+  // release, or as soon as atomic_shared_ptr will be ready.
+  mutable mutex m_mu;
   shared_ptr<EditorConfig> m_config = make_shared<EditorConfig>();
 
   // Just in case someone tryes to pass EditorConfigWrapper by value instead of referense.

indexer/indexer_tests/osm_editor_test.cpp

@@ -2,7 +2,7 @@
 
 #include "indexer/indexer_tests/osm_editor_test.hpp"
 
-#include "search/reverse_geocoder.hpp"
+#include "search/editor_delegate.hpp"
 
 #include "indexer/classificator.hpp"
 #include "indexer/classificator_loader.hpp"
@@ -17,6 +17,8 @@
 
 #include "coding/file_name_utils.hpp"
 
+#include "std/unique_ptr.hpp"
+
 using namespace generator::tests_support;
 using namespace indexer::tests_support;
 
@@ -144,7 +146,7 @@ EditorTest::EditorTest()
     LOG(LERROR, ("Classificator read error: ", e.what()));
   }
 
-  indexer::tests_support::SetUpEditorForTesting(m_index);
+  indexer::tests_support::SetUpEditorForTesting(make_unique<search::EditorDelegate>(m_index));
 }
 
 EditorTest::~EditorTest()
@@ -232,7 +234,7 @@ void EditorTest::SetIndexTest()
     builder.Add(TestCafe({4.0, 4.0}, "London Cafe", "en"));
   });
 
-  auto const mwmId = editor.m_mwmIdByMapNameFn("GB");
+  auto const mwmId = editor.GetMwmIdByMapName("GB");
 
   TEST_EQUAL(gbMwmId, mwmId, ());
 
@@ -241,28 +243,28 @@ void EditorTest::SetIndexTest()
 
   ForEachCafeAtPoint(m_index, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft)
   {
-    auto const firstPtr = editor.m_getOriginalFeatureFn(ft.GetID());
+    auto const firstPtr = editor.GetOriginalFeature(ft.GetID());
     TEST(firstPtr, ());
     SetBuildingLevelsToOne(ft);
-    auto const secondPtr = editor.m_getOriginalFeatureFn(ft.GetID());
+    auto const secondPtr = editor.GetOriginalFeature(ft.GetID());
     TEST(secondPtr, ());
     TEST_EQUAL(firstPtr->GetID(), secondPtr->GetID(), ());
   });
 
   ForEachCafeAtPoint(m_index, m2::PointD(1.0, 1.0), [&editor](FeatureType & ft)
   {
-    TEST_EQUAL(editor.m_getOriginalFeatureStreetFn(ft), "Test street", ());
+    TEST_EQUAL(editor.GetOriginalFeatureStreet(ft), "Test street", ());
 
     EditFeature(ft, [](osm::EditableMapObject & emo)
     {
       osm::LocalizedStreet ls{"Some street", ""};
       emo.SetStreet(ls);
     });
-    TEST_EQUAL(editor.m_getOriginalFeatureStreetFn(ft), "Test street", ());
+    TEST_EQUAL(editor.GetOriginalFeatureStreet(ft), "Test street", ());
   });
 
   uint32_t counter = 0;
-  editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft)
+  editor.ForEachFeatureAtPoint([&counter](FeatureType & ft)
   {
     ++counter;
   }, {100.0, 100.0});
@@ -270,7 +272,7 @@ void EditorTest::SetIndexTest()
   TEST_EQUAL(counter, 0, ());
 
   counter = 0;
-  editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft)
+  editor.ForEachFeatureAtPoint([&counter](FeatureType & ft)
   {
     ++counter;
   }, {3.0, 3.0});
@@ -278,7 +280,7 @@ void EditorTest::SetIndexTest()
   TEST_EQUAL(counter, 1, ());
 
   counter = 0;
-  editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft)
+  editor.ForEachFeatureAtPoint([&counter](FeatureType & ft)
   {
     ++counter;
   }, {1.0, 1.0});
@@ -286,7 +288,7 @@ void EditorTest::SetIndexTest()
   TEST_EQUAL(counter, 2, ());
 
   counter = 0;
-  editor.m_forEachFeatureAtPointFn([&counter](FeatureType & ft)
+  editor.ForEachFeatureAtPoint([&counter](FeatureType & ft)
   {
     ++counter;
   }, {4.0, 4.0});

indexer/indexer_tests_support/helpers.cpp

@@ -2,16 +2,14 @@
 
 #include "editor/editor_storage.hpp"
 
-#include "std/unique_ptr.hpp"
-
 namespace indexer
 {
 namespace tests_support
 {
-void SetUpEditorForTesting(Index & index)
+void SetUpEditorForTesting(unique_ptr<osm::Editor::Delegate> delegate)
 {
   auto & editor = osm::Editor::Instance();
-  editor.SetIndex(index);
+  editor.SetDelegate(move(delegate));
   editor.SetStorageForTesting(make_unique<editor::InMemoryStorage>());
   editor.ClearAllLocalEdits();
 }

indexer/indexer_tests_support/helpers.hpp

@@ -3,6 +3,8 @@
 #include "indexer/editable_map_object.hpp"
 #include "indexer/osm_editor.hpp"
 
+#include "std/unique_ptr.hpp"
+
 #include "base/assert.hpp"
 
 class Index;
@@ -11,7 +13,7 @@ namespace indexer
 {
 namespace tests_support
 {
-void SetUpEditorForTesting(Index & index);
+void SetUpEditorForTesting(unique_ptr<osm::Editor::Delegate> delegate);
 
 template <typename TFn>
 void EditFeature(FeatureType const & ft, TFn && fn)

indexer/osm_editor.cpp

@@ -10,8 +10,6 @@
 #include "indexer/osm_editor.hpp"
 #include "indexer/index_helpers.hpp"
 
-#include "search/reverse_geocoder.hpp"
-
 #include "platform/local_country_file_utils.hpp"
 #include "platform/platform.hpp"
 #include "platform/preferred_languages.hpp"
@@ -140,7 +138,8 @@ Editor::Editor()
   : m_configLoader(m_config)
   , m_notes(editor::Notes::MakeNotes())
   , m_storage(make_unique<editor::LocalStorage>())
-{}
+{
+}
 
 Editor & Editor::Instance()
 {
@@ -148,47 +147,11 @@ Editor & Editor::Instance()
   return instance;
 }
 
-void Editor::SetIndex(Index const & index)
-{
-  m_mwmIdByMapNameFn = [&index](string const & name) -> MwmSet::MwmId
-  {
-    return index.GetMwmIdByCountryFile(platform::CountryFile(name));
-  };
-
-  m_getOriginalFeatureFn = [&index](FeatureID const & fid) -> unique_ptr<FeatureType>
-  {
-    unique_ptr<FeatureType> feature(new FeatureType());
-    Index::FeaturesLoaderGuard const guard(index, fid.m_mwmId);
-    if (!guard.GetOriginalFeatureByIndex(fid.m_index, *feature))
-      return nullptr;
-    feature->ParseEverything();
-    return feature;
-  };
-
-  m_getOriginalFeatureStreetFn = [&index](FeatureType & ft) -> string
-  {
-    search::ReverseGeocoder const coder(index);
-    auto const streets = coder.GetNearbyFeatureStreets(ft);
-    if (streets.second < streets.first.size())
-      return streets.first[streets.second].m_name;
-    return {};
-  };
-
-  // Due to floating points accuracy issues (geometry is saved in editor up to 7 digits
-  // after decimal point) some feature vertexes are threated as external to a given feature.
-  auto const toleranceInMeters = 1e-2;
-  m_forEachFeatureAtPointFn =
-      [&index, toleranceInMeters](TFeatureTypeFn && fn, m2::PointD const & mercator)
-  {
-    indexer::ForEachFeatureAtPoint(index, move(fn), mercator, toleranceInMeters);
-  };
-}
-
 void Editor::LoadMapEdits()
 {
-  if (!m_mwmIdByMapNameFn)
+  if (!m_delegate)
   {
-    LOG(LERROR, ("Can't load any map edits, MwmIdByNameAndVersionFn has not been set."));
+    LOG(LERROR, ("Can't load any map edits, delegate has not been set."));
     return;
   }
 
@@ -213,7 +176,7 @@ void Editor::LoadMapEdits()
   {
     string const mapName = mwm.attribute("name").as_string("");
     int64_t const mapVersion = mwm.attribute("version").as_llong(0);
-    MwmSet::MwmId const mwmId = m_mwmIdByMapNameFn(mapName);
+    MwmSet::MwmId const mwmId = GetMwmIdByMapName(mapName);
     // TODO(mgsergio, AlexZ): Is it normal to have isMwmIdAlive and mapVersion
     // NOT equal to mwmId.GetInfo()->GetVersion() at the same time?
     auto const needMigrateEdits = !mwmId.IsAlive() || mapVersion != mwmId.GetInfo()->GetVersion();
@@ -235,10 +198,15 @@ void Editor::LoadMapEdits()
             goto SECTION_END;
           }
 
+          TForEachFeaturesNearByFn forEach = [this](TFeatureTypeFn && fn,
+                                                    m2::PointD const & point) {
+            return ForEachFeatureAtPoint(move(fn), point);
+          };
+
           // TODO(mgsergio): Deleted features are not properly handled yet.
           auto const fid = needMigrateEdits
                                ? editor::MigrateFeatureIndex(
-                                     m_forEachFeatureAtPointFn, xml, section.first,
+                                     forEach, xml, section.first,
                                      [this, &mwmId] { return GenerateNewFeatureId(mwmId); })
                                : FeatureID(mwmId, xml.GetMWMFeatureIndex());
 
@@ -253,7 +221,7 @@ void Editor::LoadMapEdits()
           }
           else
           {
-            auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
+            auto const originalFeaturePtr = GetOriginalFeature(fid);
             if (!originalFeaturePtr)
             {
               LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
@@ -451,7 +419,7 @@ bool Editor::OriginalFeatureHasDefaultName(FeatureID const & fid) const
   if (IsCreatedFeature(fid))
     return false;
 
-  auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
+  auto const originalFeaturePtr = GetOriginalFeature(fid);
   if (!originalFeaturePtr)
   {
     LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
@@ -504,7 +472,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo)
   }
   else
   {
-    auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
+    auto const originalFeaturePtr = GetOriginalFeature(fid);
     if (!originalFeaturePtr)
     {
       LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
@@ -518,7 +486,7 @@ Editor::SaveResult Editor::SaveEditedFeature(EditableMapObject const & emo)
     fti.m_feature.ReplaceBy(emo);
     bool const sameAsInMWM =
         AreFeaturesEqualButStreet(fti.m_feature, *originalFeaturePtr) &&
-        emo.GetStreet().m_defaultName == m_getOriginalFeatureStreetFn(fti.m_feature);
+        emo.GetStreet().m_defaultName == GetOriginalFeatureStreet(fti.m_feature);
 
     if (featureStatus != FeatureStatus::Untouched)
     {
@@ -675,7 +643,7 @@ EditableProperties Editor::GetEditableProperties(FeatureType const & feature) co
   // Disable opening hours editing if opening hours cannot be parsed.
   if (GetFeatureStatus(feature.GetID()) != FeatureStatus::Created)
   {
-    auto const originalFeaturePtr = m_getOriginalFeatureFn(feature.GetID());
+    auto const originalFeaturePtr = GetOriginalFeature(feature.GetID());
     if (!originalFeaturePtr)
     {
       LOG(LERROR, ("A feature with id", feature.GetID(), "cannot be loaded."));
@@ -837,7 +805,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
                 feature.SetTagValue(kAddrStreetTag, fti.m_street);
               ourDebugFeatureString = DebugPrint(feature);
 
-              auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID());
+              auto const originalFeaturePtr = GetOriginalFeature(fti.m_feature.GetID());
               if (!originalFeaturePtr)
               {
                 LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded."));
@@ -866,7 +834,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
             break;
 
           case FeatureStatus::Deleted:
-            auto const originalFeaturePtr = m_getOriginalFeatureFn(fti.m_feature.GetID());
+            auto const originalFeaturePtr = GetOriginalFeature(fti.m_feature.GetID());
             if (!originalFeaturePtr)
             {
               LOG(LERROR, ("A feature with id", fti.m_feature.GetID(), "cannot be loaded."));
@@ -1129,7 +1097,7 @@ void Editor::MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status)
 {
   auto & fti = m_features[fid.m_mwmId][fid.m_index];
 
-  auto const originalFeaturePtr = m_getOriginalFeatureFn(fid);
+  auto const originalFeaturePtr = GetOriginalFeature(fid);
 
   if (!originalFeaturePtr)
   {
@@ -1143,6 +1111,46 @@ void Editor::MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status)
   fti.m_modificationTimestamp = time(nullptr);
 }
 
+MwmSet::MwmId Editor::GetMwmIdByMapName(string const & name)
+{
+  if (!m_delegate)
+  {
+    LOG(LERROR, ("Can't get mwm id by map name:", name, ", delegate is not set."));
+    return {};
+  }
+  return m_delegate->GetMwmIdByMapName(name);
+}
+
+unique_ptr<FeatureType> Editor::GetOriginalFeature(FeatureID const & fid) const
+{
+  if (!m_delegate)
+  {
+    LOG(LERROR, ("Can't get original feature by id:", fid, ", delegate is not set."));
+    return {};
+  }
+  return m_delegate->GetOriginalFeature(fid);
+}
+
+string Editor::GetOriginalFeatureStreet(FeatureType & ft) const
+{
+  if (!m_delegate)
+  {
+    LOG(LERROR, ("Can't get feature street, delegate is not set."));
+    return {};
+  }
+  return m_delegate->GetOriginalFeatureStreet(ft);
+}
+
+void Editor::ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const
+{
+  if (!m_delegate)
+  {
+    LOG(LERROR, ("Can't load features in point's vicinity, delegate is not set."));
+    return;
+  }
+  m_delegate->ForEachFeatureAtPoint(move(fn), point);
+}
+
 string DebugPrint(Editor::FeatureStatus fs)
 {
   switch (fs)

indexer/osm_editor.hpp

@@ -51,6 +51,16 @@ public:
   using TForEachFeaturesNearByFn =
       function<void(TFeatureTypeFn && /*fn*/, m2::PointD const & /*mercator*/)>;
 
+  struct Delegate
+  {
+    virtual ~Delegate() = default;
+
+    virtual MwmSet::MwmId GetMwmIdByMapName(string const & name) const = 0;
+    virtual unique_ptr<FeatureType> GetOriginalFeature(FeatureID const & fid) const = 0;
+    virtual string GetOriginalFeatureStreet(FeatureType & ft) const = 0;
+    virtual void ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const = 0;
+  };
+
   enum class UploadResult
   {
     Success,
@@ -72,8 +82,7 @@ public:
 
   static Editor & Instance();
 
-  // Reference to the index will be used in editor functors, it should not be temporary object.
-  void SetIndex(Index const & index);
+  inline void SetDelegate(unique_ptr<Delegate> delegate) { m_delegate = move(delegate); }
 
   inline void SetStorageForTesting(unique_ptr<editor::StorageBase> storage)
   {
@@ -225,20 +234,20 @@ private:
 
   void MarkFeatureWithStatus(FeatureID const & fid, FeatureStatus status);
 
+  // These methods are just checked wrappers around Delegate.
+  MwmSet::MwmId GetMwmIdByMapName(string const & name);
+  unique_ptr<FeatureType> GetOriginalFeature(FeatureID const & fid) const;
+  string GetOriginalFeatureStreet(FeatureType & ft) const;
+  void ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const;
+
   // TODO(AlexZ): Synchronize multithread access.
   /// Deleted, edited and created features.
   map<MwmSet::MwmId, map<uint32_t, FeatureTypeInfo>> m_features;
 
-  /// Get MwmId for each map, used in FeatureIDs and to check if edits are up-to-date.
-  TMwmIdByMapNameFn m_mwmIdByMapNameFn;
+  unique_ptr<Delegate> m_delegate;
+
   /// Invalidate map viewport after edits.
   TInvalidateFn m_invalidateFn;
-  /// Get FeatureType from mwm.
-  TFeatureLoaderFn m_getOriginalFeatureFn;
-  /// Get feature original street name or empty string.
-  TFeatureOriginalStreetFn m_getOriginalFeatureStreetFn;
-  /// Iterate over all features in some area that includes given point.
-  TForEachFeaturesNearByFn m_forEachFeatureAtPointFn;
 
   /// Contains information about what and how can be edited.
   editor::EditorConfigWrapper m_config;

map/framework.cpp

@@ -13,6 +13,7 @@
 #include "routing/routing_algorithm.hpp"
 
 #include "search/downloader_search_callback.hpp"
+#include "search/editor_delegate.hpp"
 #include "search/engine.hpp"
 #include "search/everywhere_search_params.hpp"
 #include "search/geometry_utils.hpp"
@@ -397,7 +398,7 @@ Framework::Framework()
 
   osm::Editor & editor = osm::Editor::Instance();
 
-  editor.SetIndex(m_model.GetIndex());
+  editor.SetDelegate(make_unique<search::EditorDelegate>(m_model.GetIndex()));
   editor.SetInvalidateFn([this](){ InvalidateRect(GetCurrentViewport()); });
   editor.LoadMapEdits();
 

search/editor_delegate.cpp

@@ -0,0 +1,44 @@
+#include "search/editor_delegate.hpp"
+
+#include "search/reverse_geocoder.hpp"
+
+#include "indexer/feature_decl.hpp"
+#include "indexer/index.hpp"
+#include "indexer/index.hpp"
+#include "indexer/index_helpers.hpp"
+
+namespace search
+{
+EditorDelegate::EditorDelegate(Index const & index) : m_index(index) {}
+
+MwmSet::MwmId EditorDelegate::GetMwmIdByMapName(string const & name) const
+{
+  return m_index.GetMwmIdByCountryFile(platform::CountryFile(name));
+}
+
+unique_ptr<FeatureType> EditorDelegate::GetOriginalFeature(FeatureID const & fid) const
+{
+  auto feature = make_unique<FeatureType>();
+  Index::FeaturesLoaderGuard const guard(m_index, fid.m_mwmId);
+  if (!guard.GetOriginalFeatureByIndex(fid.m_index, *feature))
+    return unique_ptr<FeatureType>();
+  feature->ParseEverything();
+  return feature;
+}
+
+string EditorDelegate::GetOriginalFeatureStreet(FeatureType & ft) const
+{
+  search::ReverseGeocoder const coder(m_index);
+  auto const streets = coder.GetNearbyFeatureStreets(ft);
+  if (streets.second < streets.first.size())
+    return streets.first[streets.second].m_name;
+  return {};
+}
+
+void EditorDelegate::ForEachFeatureAtPoint(osm::Editor::TFeatureTypeFn && fn,
+                                           m2::PointD const & point) const
+{
+  auto const kToleranceMeters = 1e-2;
+  indexer::ForEachFeatureAtPoint(m_index, move(fn), point, kToleranceMeters);
+}
+}  // namespace search

search/editor_delegate.hpp

@@ -0,0 +1,24 @@
+#pragma once
+
+#include "indexer/osm_editor.hpp"
+
+class Index;
+
+namespace search
+{
+class EditorDelegate : public osm::Editor::Delegate
+{
+public:
+  EditorDelegate(Index const & index);
+
+  // osm::Editor::Delegate overrides:
+  MwmSet::MwmId GetMwmIdByMapName(string const & name) const override;
+  unique_ptr<FeatureType> GetOriginalFeature(FeatureID const & fid) const override;
+  string GetOriginalFeatureStreet(FeatureType & ft) const override;
+  void ForEachFeatureAtPoint(osm::Editor::TFeatureTypeFn && fn,
+                             m2::PointD const & point) const override;
+
+private:
+  Index const & m_index;
+};
+}  // namespace search

search/search.pro

@@ -18,6 +18,7 @@ HEADERS += \
     displayed_categories.hpp \
     downloader_search_callback.hpp \
     dummy_rank_table.hpp \
+    editor_delegate.hpp \
     engine.hpp \
     everywhere_search_params.hpp \
     feature_offset_match.hpp \
@@ -83,6 +84,7 @@ SOURCES += \
     displayed_categories.cpp \
     downloader_search_callback.cpp \
     dummy_rank_table.cpp \
+    editor_delegate.cpp \
     engine.cpp \
     features_filter.cpp \
     features_layer.cpp \

search/search_integration_tests/helpers.cpp

@@ -1,5 +1,6 @@
 #include "search/search_integration_tests/helpers.hpp"
 
+#include "search/editor_delegate.hpp"
 #include "search/processor_factory.hpp"
 #include "search/search_tests_support/test_search_request.hpp"
 
@@ -27,7 +28,7 @@ SearchTest::SearchTest()
   , m_engine(make_unique<storage::CountryInfoGetterForTesting>(), make_unique<ProcessorFactory>(),
              Engine::Params())
 {
-  indexer::tests_support::SetUpEditorForTesting(m_engine);
+  indexer::tests_support::SetUpEditorForTesting(make_unique<EditorDelegate>(m_engine));
 }
 
 SearchTest::~SearchTest()