pastk/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[editor] load map edits refactoring

Browse source
This commit is contained in:
Arsentiy Milchakov 2017-12-07 11:19:31 +03:00 • committed by mgsergio
parent cd0aeff6a0
commit 5f19f86e31
8 changed files with 186 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
indexer/edits_migration.cpp
View file

@ -12,7 +12,7 @@
namespace editor
{
FeatureID MigrateNodeFeatureIndex(osm::Editor::TForEachFeaturesNearByFn & forEach,
FeatureID MigrateNodeFeatureIndex(osm::Editor::ForEachFeaturesNearByFn & forEach,
XMLFeature const & xml,
osm::Editor::FeatureStatus const featureStatus,
TGenerateIDFn const & generateID)
@ -44,7 +44,7 @@ FeatureID MigrateNodeFeatureIndex(osm::Editor::TForEachFeaturesNearByFn & forEac
}
FeatureID MigrateWayFeatureIndex(
osm::Editor::TForEachFeaturesNearByFn & forEach, XMLFeature const & xml,
osm::Editor::ForEachFeaturesNearByFn & forEach, XMLFeature const & xml,
osm::Editor::FeatureStatus const /* Unused for now (we don't create/delete area features)*/,
TGenerateIDFn const & /*Unused for the same reason*/)
{
@ -108,7 +108,7 @@ FeatureID MigrateWayFeatureIndex(
return feature->GetID();
}
FeatureID MigrateFeatureIndex(osm::Editor::TForEachFeaturesNearByFn & forEach,
FeatureID MigrateFeatureIndex(osm::Editor::ForEachFeaturesNearByFn & forEach,
XMLFeature const & xml,
osm::Editor::FeatureStatus const featureStatus,
TGenerateIDFn const & generateID)

2
indexer/edits_migration.hpp
View file

@ -17,7 +17,7 @@ using TGenerateIDFn = function<FeatureID()>;
/// Tries to match xml feature with one on a new mwm and retruns FeatrueID
/// of a found feature, thows MigrationError if migration fails.
FeatureID MigrateFeatureIndex(osm::Editor::TForEachFeaturesNearByFn & forEach,
FeatureID MigrateFeatureIndex(osm::Editor::ForEachFeaturesNearByFn & forEach,
XMLFeature const & xml,
osm::Editor::FeatureStatus const featureStatus,
TGenerateIDFn const & generateID);

14
indexer/indexer_tests/osm_editor_test.cpp
View file

@ -438,7 +438,7 @@ void EditorTest::IsFeatureUploadedTest()
pugi::xml_document doc;
GenerateUploadedFeature(mwmId, emo, doc);
editor.m_storage->Save(doc);
editor.LoadMapEdits();
editor.LoadEdits();
TEST(editor.IsFeatureUploaded(emo.GetID().m_mwmId, emo.GetID().m_index), ());
}
@ -756,7 +756,7 @@ void EditorTest::GetStatsTest()
pugi::xml_document doc;
GenerateUploadedFeature(mwmId, emo, doc);
editor.m_storage->Save(doc);
editor.LoadMapEdits();
editor.LoadEdits();
stats = editor.GetStats();
TEST_EQUAL(stats.m_edits.size(), 1, ());
@ -931,7 +931,7 @@ void EditorTest::LoadMapEditsTest()
features.emplace_back(emo.GetID());
editor.Save();
editor.LoadMapEdits();
editor.LoadEdits();
auto const fillLoaded = [&editor](vector<FeatureID> & loadedFeatures)
{
@ -962,7 +962,7 @@ void EditorTest::LoadMapEditsTest()
builder.Add(TestCafe(m2::PointD(6.0, 6.0), "Moscow Cafe4", "en"));
}, 1);
editor.LoadMapEdits();
editor.LoadEdits();
fillLoaded(loadedFeatures);
TEST_EQUAL(features.size(), loadedFeatures.size(), ());
@ -972,7 +972,7 @@ void EditorTest::LoadMapEditsTest()
TEST_EQUAL(editor.m_features.size(), 2, ());
editor.LoadMapEdits();
editor.LoadEdits();
fillLoaded(loadedFeatures);
TEST_EQUAL(editor.m_features.size(), 1, ());
@ -984,7 +984,7 @@ void EditorTest::LoadMapEditsTest()
pugi::xml_document doc;
GenerateUploadedFeature(gbMwmId, gbEmo, doc);
editor.m_storage->Save(doc);
editor.LoadMapEdits();
editor.LoadEdits();
fillLoaded(loadedFeatures);
TEST_EQUAL(editor.m_features.size(), 1, ());
@ -1001,7 +1001,7 @@ void EditorTest::LoadMapEditsTest()
newGbMwmId.GetInfo()->m_version.SetSecondsSinceEpoch(time(nullptr) + 1);
editor.LoadMapEdits();
editor.LoadEdits();
TEST(editor.m_features.empty(), ());
}

254
indexer/osm_editor.cpp
View file

@ -66,6 +66,48 @@ constexpr char const * kRelationsAreNotSupported = "Relations are not supported
constexpr char const * kNeedsRetry = "Needs Retry";
constexpr char const * kWrongMatch = "Matched feature has no tags";
struct XmlSection
{
osm::Editor::FeatureStatus m_status;
std::string m_sectionName;
};
array<XmlSection, 4> const kXmlSections =
{{
{osm::Editor::FeatureStatus::Deleted, kDeleteSection},
{osm::Editor::FeatureStatus::Modified, kModifySection},
{osm::Editor::FeatureStatus::Obsolete, kObsoleteSection},
{osm::Editor::FeatureStatus::Created, kCreateSection}
}};
struct LogHelper
{
LogHelper(MwmSet::MwmId const & mwmId) : m_mwmId(mwmId) {}
~LogHelper()
{
LOG(LINFO, ("For", m_mwmId, ". Was loaded", m_modified, "modified,", m_created, "created,",
m_deleted, "deleted and", m_obsolete, "obsolete features."));
}
void Increment(osm::Editor::FeatureStatus status)
{
switch (status)
{
case osm::Editor::FeatureStatus::Deleted: ++m_deleted; break;
case osm::Editor::FeatureStatus::Modified: ++m_modified; break;
case osm::Editor::FeatureStatus::Obsolete: ++m_obsolete; break;
case osm::Editor::FeatureStatus::Created: ++m_created; break;
case osm::Editor::FeatureStatus::Untouched: ASSERT(false, ());
}
}
int m_deleted = 0;
int m_obsolete = 0;
int m_modified = 0;
int m_created = 0;
MwmSet::MwmId const & m_mwmId;
};
bool NeedsUpload(string const & uploadStatus)
{
return uploadStatus != kUploaded &&
@ -152,7 +194,7 @@ void Editor::SetDefaultStorage()
m_storage = make_unique<editor::LocalStorage>();
}
void Editor::LoadMapEdits()
void Editor::LoadEdits()
{
if (!m_delegate)
{
@ -164,121 +206,35 @@ void Editor::LoadMapEdits()
if (!m_storage->Load(doc))
return;
array<pair<FeatureStatus, char const *>, 4> const sections =
{{
{FeatureStatus::Deleted, kDeleteSection},
{FeatureStatus::Modified, kModifySection},
{FeatureStatus::Obsolete, kObsoleteSection},
{FeatureStatus::Created, kCreateSection}
}};
int deleted = 0, obsolete = 0, modified = 0, created = 0;
bool needRewriteEdits = false;
// TODO(mgsergio): synchronize access to m_features.
m_features.clear();
for (xml_node mwm : doc.child(kXmlRootNode).children(kXmlMwmNode))
for (auto const & mwm : doc.child(kXmlRootNode).children(kXmlMwmNode))
{
string const mapName = mwm.attribute("name").as_string("");
int64_t const mapVersion = mwm.attribute("version").as_llong(0);
MwmSet::MwmId const mwmId = GetMwmIdByMapName(mapName);
auto const mwmId = GetMwmIdByMapName(mapName);
// TODO(mgsergio): A map could be renamed, we'll treat it as deleted.
if (!mwmId.IsAlive())
{
LOG(LINFO, ("Mwm", mapName, "was deleted"));
needRewriteEdits = true;
continue;
}
// 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();
needRewriteEdits |= needMigrateEdits;
auto const needMigrateEdits = mapVersion != mwmId.GetInfo()->GetVersion();
needRewriteEdits = needRewriteEdits || needMigrateEdits;
for (auto const & section : sections)
{
for (auto const nodeOrWay : mwm.child(section.second).select_nodes("node|way"))
{
try
{
XMLFeature const xml(nodeOrWay.node());
// TODO(mgsergio): A map could be renamed, we'll treat it as deleted.
// The right thing to do is to try to migrate all changes anyway.
if (!mwmId.IsAlive())
{
LOG(LINFO, ("Mwm", mapName, "was deleted"));
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(
forEach, xml, section.first,
[this, &mwmId] { return GenerateNewFeatureId(mwmId); })
: FeatureID(mwmId, xml.GetMWMFeatureIndex());
// Remove obsolete changes during migration.
if (needMigrateEdits && IsObsolete(xml, fid))
continue;
FeatureTypeInfo fti;
if (section.first == FeatureStatus::Created)
{
fti.m_feature.FromXML(xml);
}
else
{
auto const originalFeaturePtr = GetOriginalFeature(fid);
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
goto SECTION_END;
}
fti.m_feature = *originalFeaturePtr;
fti.m_feature.ApplyPatch(xml);
}
fti.m_feature.SetID(fid);
fti.m_street = xml.GetTagValue(kAddrStreetTag);
fti.m_modificationTimestamp = xml.GetModificationTime();
ASSERT_NOT_EQUAL(my::INVALID_TIME_STAMP, fti.m_modificationTimestamp, ());
fti.m_uploadAttemptTimestamp = xml.GetUploadTime();
fti.m_uploadStatus = xml.GetUploadStatus();
fti.m_uploadError = xml.GetUploadError();
fti.m_status = section.first;
switch (section.first)
{
case FeatureStatus::Deleted: ++deleted; break;
case FeatureStatus::Modified: ++modified; break;
case FeatureStatus::Obsolete: ++obsolete; break;
case FeatureStatus::Created: ++created; break;
case FeatureStatus::Untouched: ASSERT(false, ()); continue;
}
// Insert initialized structure at the end: exceptions are possible in above code.
m_features[fid.m_mwmId].emplace(fid.m_index, move(fti));
}
catch (editor::XMLFeatureError const & ex)
{
ostringstream s;
nodeOrWay.node().print(s, " ");
LOG(LERROR, (ex.what(), "Can't create XMLFeature in section", section.second, s.str()));
}
catch (editor::MigrationError const & ex)
{
LOG(LWARNING, (ex.Msg(), "mwmId =", mwmId, XMLFeature(nodeOrWay.node())));
}
} // for nodes
} // for sections
SECTION_END:
;
LoadMwmEdits(mwm, mwmId, needMigrateEdits);
} // for mwms
// Save edits with new indexes and mwm version to avoid another migration on next startup.
if (needRewriteEdits)
Save();
LOG(LINFO, ("Loaded", modified, "modified,",
created, "created,", deleted, "deleted and", obsolete, "obsolete features."));
}
bool Editor::Save() const
@ -930,6 +886,40 @@ void Editor::SaveUploadedInformation(FeatureTypeInfo const & fromUploader)
Save();
}
bool Editor::FillFeatureInfo(FeatureStatus status, XMLFeature const & xml, FeatureID const & fid,
FeatureTypeInfo & fti) const
{
if (status == FeatureStatus::Created)
{
fti.m_feature.FromXML(xml);
}
else
{
auto const originalFeaturePtr = GetOriginalFeature(fid);
if (!originalFeaturePtr)
{
LOG(LERROR, ("A feature with id", fid, "cannot be loaded."));
alohalytics::LogEvent("Editor_MissingFeature_Error");
return false;
}
fti.m_feature = *originalFeaturePtr;
fti.m_feature.ApplyPatch(xml);
}
fti.m_feature.SetID(fid);
fti.m_street = xml.GetTagValue(kAddrStreetTag);
fti.m_modificationTimestamp = xml.GetModificationTime();
ASSERT_NOT_EQUAL(my::INVALID_TIME_STAMP, fti.m_modificationTimestamp, ());
fti.m_uploadAttemptTimestamp = xml.GetUploadTime();
fti.m_uploadStatus = xml.GetUploadStatus();
fti.m_uploadError = xml.GetUploadError();
fti.m_status = status;
return true;
}
// Macros is used to avoid code duplication.
#define GET_FEATURE_TYPE_INFO_BODY \
do \
@ -1037,7 +1027,7 @@ NewFeatureCategories Editor::GetNewFeatureCategories() const
return NewFeatureCategories(*(m_config.Get()));
}
FeatureID Editor::GenerateNewFeatureId(MwmSet::MwmId const & id)
FeatureID Editor::GenerateNewFeatureId(MwmSet::MwmId const & id) const
{
DECLARE_AND_ASSERT_THREAD_CHECKER("GenerateNewFeatureId is single-threaded.");
// TODO(vng): Double-check if new feature indexes should uninterruptedly continue after existing indexes in mwm file.
@ -1176,7 +1166,7 @@ string Editor::GetOriginalFeatureStreet(FeatureType & ft) const
return m_delegate->GetOriginalFeatureStreet(ft);
}
void Editor::ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & point) const
void Editor::ForEachFeatureAtPoint(FeatureTypeFn && fn, m2::PointD const & point) const
{
if (!m_delegate)
{
@ -1186,6 +1176,64 @@ void Editor::ForEachFeatureAtPoint(TFeatureTypeFn && fn, m2::PointD const & poin
m_delegate->ForEachFeatureAtPoint(move(fn), point);
}
FeatureID Editor::GetFeatureIdByXmlFeature(XMLFeature const & xml, MwmSet::MwmId const & mwmId,
FeatureStatus status, bool needMigrate) const
{
ForEachFeaturesNearByFn forEach = [this](FeatureTypeFn && fn, m2::PointD const & point)
{
return ForEachFeatureAtPoint(move(fn), point);
};
// TODO(mgsergio): Deleted features are not properly handled yet.
if (needMigrate)
{
return editor::MigrateFeatureIndex(forEach, xml, status,
[this, &mwmId] { return GenerateNewFeatureId(mwmId); });
}
return FeatureID(mwmId, xml.GetMWMFeatureIndex());
}
void Editor::LoadMwmEdits(xml_node const & mwm, MwmSet::MwmId const & mwmId, bool needMigrate)
{
LogHelper logHelper(mwmId);
for (auto const & section : kXmlSections)
{
for (auto const & nodeOrWay : mwm.child(section.m_sectionName.c_str()).select_nodes("node|way"))
{
try
{
XMLFeature const xml(nodeOrWay.node());
auto const fid = GetFeatureIdByXmlFeature(xml, mwmId, section.m_status, needMigrate);
// Remove obsolete changes during migration.
if (needMigrate && IsObsolete(xml, fid))
continue;
FeatureTypeInfo fti;
if (!FillFeatureInfo(section.m_status, xml, fid, fti))
continue;
logHelper.Increment(section.m_status);
m_features[fid.m_mwmId].emplace(fid.m_index, move(fti));
}
catch (editor::XMLFeatureError const & ex)
{
ostringstream s;
nodeOrWay.node().print(s, " ");
LOG(LERROR, (ex.what(), "mwmId =", mwmId, "in section", section.m_sectionName, s.str()));
}
catch (editor::MigrationError const & ex)
{
LOG(LWARNING, (ex.what(), "mwmId =", mwmId, XMLFeature(nodeOrWay.node())));
}
}
}
}
string DebugPrint(Editor::FeatureStatus fs)
{
switch (fs)

38
indexer/osm_editor.hpp
View file

@ -28,8 +28,12 @@ namespace editor
namespace testing
{
class EditorTest;
} // namespace testing
} // namespace editor
}
}
namespace editor
{
class XMLFeature;
}
class Index;
@ -42,14 +46,9 @@ class Editor final : public MwmSet::Observer
Editor();
public:
using TFeatureTypeFn = function<void(FeatureType &)>; // Mimics Framework::TFeatureTypeFn.
using TMwmIdByMapNameFn = function<MwmSet::MwmId(string const & /*map*/)>;
using TInvalidateFn = function<void()>;
using TFeatureLoaderFn = function<unique_ptr<FeatureType> (FeatureID const & /*fid*/)>;
using TFeatureOriginalStreetFn = function<string(FeatureType & /*ft*/)>;
using TForEachFeaturesNearByFn =
function<void(TFeatureTypeFn && /*fn*/, m2::PointD const & /*mercator*/)>;
using FeatureTypeFn = function<void(FeatureType & ft)>;
using InvalidateFn = function<void()>;
using ForEachFeaturesNearByFn = function<void(FeatureTypeFn && fn, m2::PointD const & mercator)>;
struct Delegate
{
@ -58,7 +57,7 @@ public:
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;
virtual void ForEachFeatureAtPoint(FeatureTypeFn && fn, m2::PointD const & point) const = 0;
};
enum class UploadResult
@ -91,16 +90,16 @@ public:
void SetDefaultStorage();
void SetInvalidateFn(TInvalidateFn const & fn) { m_invalidateFn = fn; }
void SetInvalidateFn(InvalidateFn const & fn) { m_invalidateFn = fn; }
void LoadMapEdits();
void LoadEdits();
/// Resets editor to initial state: no any edits or created/deleted features.
void ClearAllLocalEdits();
void OnMapUpdated(platform::LocalCountryFile const &,
platform::LocalCountryFile const &) override
{
LoadMapEdits();
LoadEdits();
}
void OnMapDeregistered(platform::LocalCountryFile const & localFile) override;
@ -215,7 +214,7 @@ private:
bool MarkFeatureAsObsolete(FeatureID const & fid);
bool RemoveFeature(FeatureID const & fid);
FeatureID GenerateNewFeatureId(MwmSet::MwmId const & id);
FeatureID GenerateNewFeatureId(MwmSet::MwmId const & id) const;
EditableProperties GetEditablePropertiesForTypes(feature::TypesHolder const & types) const;
struct FeatureTypeInfo
@ -231,6 +230,8 @@ private:
string m_uploadStatus;
string m_uploadError;
};
bool FillFeatureInfo(FeatureStatus status, editor::XMLFeature const & xml, FeatureID const & fid,
FeatureTypeInfo & fti) const;
/// @returns pointer to m_features[id][index] if exists, nullptr otherwise.
FeatureTypeInfo const * GetFeatureTypeInfo(MwmSet::MwmId const & mwmId, uint32_t index) const;
FeatureTypeInfo * GetFeatureTypeInfo(MwmSet::MwmId const & mwmId, uint32_t index);
@ -242,7 +243,10 @@ private:
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;
void ForEachFeatureAtPoint(FeatureTypeFn && fn, m2::PointD const & point) const;
FeatureID GetFeatureIdByXmlFeature(editor::XMLFeature const & xml, MwmSet::MwmId const & mwmId,
FeatureStatus status, bool needMigrate) const;
void LoadMwmEdits(pugi::xml_node const & mwm, MwmSet::MwmId const & mwmId, bool needMigrate);
// TODO(AlexZ): Synchronize multithread access.
/// Deleted, edited and created features.
@ -251,7 +255,7 @@ private:
unique_ptr<Delegate> m_delegate;
/// Invalidate map viewport after edits.
TInvalidateFn m_invalidateFn;
InvalidateFn m_invalidateFn;
/// Contains information about what and how can be edited.
editor::EditorConfigWrapper m_config;

2
map/framework.cpp
View file

@ -475,7 +475,7 @@ Framework::Framework(FrameworkParams const & params)
editor.SetDelegate(make_unique<search::EditorDelegate>(m_model.GetIndex()));
editor.SetInvalidateFn([this](){ InvalidateRect(GetCurrentViewport()); });
editor.LoadMapEdits();
editor.LoadEdits();
m_model.GetIndex().AddObserver(editor);

2
search/editor_delegate.cpp
View file

@ -34,7 +34,7 @@ string EditorDelegate::GetOriginalFeatureStreet(FeatureType & ft) const
return {};
}
void EditorDelegate::ForEachFeatureAtPoint(osm::Editor::TFeatureTypeFn && fn,
void EditorDelegate::ForEachFeatureAtPoint(osm::Editor::FeatureTypeFn && fn,
m2::PointD const & point) const
{
auto const kToleranceMeters = 1e-2;

2
search/editor_delegate.hpp
View file

@ -15,7 +15,7 @@ public:
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,
void ForEachFeatureAtPoint(osm::Editor::FeatureTypeFn && fn,
m2::PointD const & point) const override;
private: