diff --git a/editor/editor_tests/osm_editor_test.cpp b/editor/editor_tests/osm_editor_test.cpp index e89454fe0a..8595f10403 100644 --- a/editor/editor_tests/osm_editor_test.cpp +++ b/editor/editor_tests/osm_editor_test.cpp @@ -108,7 +108,7 @@ void GenerateUploadedFeature(MwmSet::MwmId const & mwmId, FeatureType ft; editor.GetEditedFeature(emo.GetID().m_mwmId, emo.GetID().m_index, ft); - editor::XMLFeature xf = ft.ToXML(true); + editor::XMLFeature xf = editor::ToXML(ft, true); xf.SetMWMFeatureIndex(ft.GetID().m_index); xf.SetModificationTime(time(nullptr)); xf.SetUploadTime(time(nullptr)); diff --git a/editor/editor_tests/xml_feature_test.cpp b/editor/editor_tests/xml_feature_test.cpp index 13ddab50a3..59a6be07e1 100644 --- a/editor/editor_tests/xml_feature_test.cpp +++ b/editor/editor_tests/xml_feature_test.cpp @@ -2,6 +2,9 @@ #include "editor/xml_feature.hpp" +#include "indexer/classificator_loader.hpp" +#include "indexer/feature.hpp" + #include "geometry/mercator.hpp" #include "base/timer.hpp" @@ -345,3 +348,33 @@ UNIT_TEST(XMLFeature_ApplyPatch) TEST_EQUAL(hasMainAndAltTag.GetTagValue("url"), "mapswithme.com", ()); } } + +UNIT_TEST(XMLFeature_FromXMLAndBackToXML) +{ + classificator::Load(); + + string const xmlNoTypeStr = R"( + + + + + + + )"; + + char const kTimestamp[] = "2015-11-27T21:13:32Z"; + + editor::XMLFeature xmlNoType(xmlNoTypeStr); + editor::XMLFeature xmlWithType = xmlNoType; + xmlWithType.SetTagValue("amenity", "atm"); + + FeatureType ft; + editor::FromXML(xmlWithType, ft); + auto fromFtWithType = editor::ToXML(ft, true); + fromFtWithType.SetAttribute("timestamp", kTimestamp); + TEST_EQUAL(fromFtWithType, xmlWithType, ()); + + auto fromFtWithoutType = editor::ToXML(ft, false); + fromFtWithoutType.SetAttribute("timestamp", kTimestamp); + TEST_EQUAL(fromFtWithoutType, xmlNoType, ()); +} diff --git a/editor/osm_auth_tests/CMakeLists.txt b/editor/osm_auth_tests/CMakeLists.txt index e7e4361934..09f3a72c49 100644 --- a/editor/osm_auth_tests/CMakeLists.txt +++ b/editor/osm_auth_tests/CMakeLists.txt @@ -11,12 +11,16 @@ omim_add_test(${PROJECT_NAME} ${SRC}) omim_link_libraries( ${PROJECT_NAME} editor + indexer platform platform_tests_support geometry coding base pugixml + protobuf + jansson + icu oauthcpp stats_client ${LIBZ} diff --git a/editor/osm_editor.cpp b/editor/osm_editor.cpp index 1b4c47759d..ac53bfc93c 100644 --- a/editor/osm_editor.cpp +++ b/editor/osm_editor.cpp @@ -267,7 +267,7 @@ bool Editor::Save() const { FeatureTypeInfo const & fti = index.second; // TODO: Do we really need to serialize deleted features in full details? Looks like mwm ID and meta fields are enough. - XMLFeature xf = fti.m_feature.ToXML(true /*type serializing helps during migration*/); + XMLFeature xf = editor::ToXML(fti.m_feature, true /*type serializing helps during migration*/); xf.SetMWMFeatureIndex(index.first); if (!fti.m_street.empty()) xf.SetTagValue(kAddrStreetTag, fti.m_street); @@ -687,7 +687,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset case FeatureStatus::Obsolete: continue; // Obsolete features will be deleted by OSMers. case FeatureStatus::Created: { - XMLFeature feature = fti.m_feature.ToXML(true); + XMLFeature feature = editor::ToXML(fti.m_feature, true); if (!fti.m_street.empty()) feature.SetTagValue(kAddrStreetTag, fti.m_street); ourDebugFeatureString = DebugPrint(feature); @@ -736,7 +736,7 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset { // Do not serialize feature's type to avoid breaking OSM data. // TODO: Implement correct types matching when we support modifying existing feature types. - XMLFeature feature = fti.m_feature.ToXML(false); + XMLFeature feature = editor::ToXML(fti.m_feature, false); if (!fti.m_street.empty()) feature.SetTagValue(kAddrStreetTag, fti.m_street); ourDebugFeatureString = DebugPrint(feature); @@ -866,7 +866,7 @@ bool Editor::FillFeatureInfo(FeatureStatus status, XMLFeature const & xml, Featu { if (status == FeatureStatus::Created) { - fti.m_feature.FromXML(xml); + editor::FromXML(xml, fti.m_feature); } else { @@ -879,7 +879,7 @@ bool Editor::FillFeatureInfo(FeatureStatus status, XMLFeature const & xml, Featu } fti.m_feature = *originalFeaturePtr; - fti.m_feature.ApplyPatch(xml); + editor::ApplyPatch(xml, fti.m_feature); } fti.m_feature.SetID(fid); diff --git a/editor/xml_feature.cpp b/editor/xml_feature.cpp index 220d27c140..8ea1188845 100644 --- a/editor/xml_feature.cpp +++ b/editor/xml_feature.cpp @@ -1,5 +1,8 @@ #include "editor/xml_feature.hpp" +#include "indexer/classificator.hpp" +#include "indexer/feature.hpp" + #include "base/macros.hpp" #include "base/string_utils.hpp" #include "base/timer.hpp" @@ -412,6 +415,131 @@ XMLFeature::Type XMLFeature::StringToType(string const & type) return Type::Unknown; } +void ApplyPatch(XMLFeature const & xml, FeatureType & feature) +{ + xml.ForEachName([&feature](string const & lang, string const & name) { + feature.GetParams().name.AddString(lang, name); + }); + + string const house = xml.GetHouse(); + if (!house.empty()) + feature.GetParams().house.Set(house); + + xml.ForEachTag([&feature](string const & k, string const & v) { + if (!feature.UpdateMetadataValue(k, v)) + LOG(LWARNING, ("Patch feature has unknown tags", k, v)); + }); + + // If types count are changed here, in ApplyPatch, new number of types should be passed + // instead of GetTypesCount(). + // So we call UpdateHeader for recalc header and update parsed parts. + feature.UpdateHeader(true /* commonParsed */, true /* metadataParsed */); +} + +XMLFeature ToXML(FeatureType const & fromFeature, bool serializeType) +{ + bool const isPoint = fromFeature.GetFeatureType() == feature::GEOM_POINT; + XMLFeature toFeature(isPoint ? XMLFeature::Type::Node : XMLFeature::Type::Way); + + if (isPoint) + { + toFeature.SetCenter(fromFeature.GetCenter()); + } + else + { + auto const & triangles = fromFeature.GetTriangesAsPoints(FeatureType::BEST_GEOMETRY); + toFeature.SetGeometry(begin(triangles), end(triangles)); + } + + fromFeature.ForEachName( + [&toFeature](uint8_t const & lang, string const & name) { toFeature.SetName(lang, name); }); + + string const house = fromFeature.GetHouseNumber(); + if (!house.empty()) + toFeature.SetHouse(house); + + if (serializeType) + { + feature::TypesHolder th(fromFeature); + // TODO(mgsergio): Use correct sorting instead of SortBySpec based on the config. + th.SortBySpec(); + // TODO(mgsergio): Either improve "OSM"-compatible serialization for more complex types, + // or save all our types directly, to restore and reuse them in migration of modified features. + for (uint32_t const type : th) + { + string const strType = classif().GetReadableObjectName(type); + strings::SimpleTokenizer iter(strType, "-"); + string const k = *iter; + if (++iter) + { + // First (main) type is always stored as "k=amenity v=restaurant". + // Any other "k=amenity v=atm" is replaced by "k=atm v=yes". + if (toFeature.GetTagValue(k).empty()) + toFeature.SetTagValue(k, *iter); + else + toFeature.SetTagValue(*iter, "yes"); + } + else + { + // We're editing building, generic craft, shop, office, amenity etc. + // Skip it's serialization. + // TODO(mgsergio): Correcly serialize all types back and forth. + LOG(LDEBUG, ("Skipping type serialization:", k)); + } + } + } + + fromFeature.ForEachMetadataItem(true /* skipSponsored */, + [&toFeature](string const & tag, string const & value) { + toFeature.SetTagValue(tag, value); + }); + + return toFeature; +} + +bool FromXML(XMLFeature const & xml, FeatureType & feature) +{ + ASSERT_EQUAL(XMLFeature::Type::Node, xml.GetType(), + ("At the moment only new nodes (points) can can be created.")); + feature.SetCenter(xml.GetMercatorCenter()); + xml.ForEachName([&feature](string const & lang, string const & name) { + feature.GetParams().name.AddString(lang, name); + }); + + string const house = xml.GetHouse(); + if (!house.empty()) + feature.GetParams().house.Set(house); + + uint32_t typesCount = 0; + uint32_t types[feature::kMaxTypesCount]; + xml.ForEachTag([&feature, &types, &typesCount](string const & k, string const & v) { + if (feature.UpdateMetadataValue(k, v)) + return; + + // Simple heuristics. It works if all our supported types for + // new features at data/editor.config + // are of one or two levels nesting (currently it's true). + Classificator & cl = classif(); + uint32_t type = cl.GetTypeByPathSafe({k, v}); + if (type == 0) + type = cl.GetTypeByPathSafe({k}); // building etc. + if (type == 0) + type = cl.GetTypeByPathSafe({"amenity", k}); // atm=yes, toilet=yes etc. + + if (type && typesCount >= feature::kMaxTypesCount) + LOG(LERROR, ("Can't add type:", k, v, ". Types limit exceeded.")); + else if (type) + types[typesCount++] = type; + else + LOG(LWARNING, ("Can't load/parse type:", k, v)); + }); + + feature.SetTypes(types, typesCount); + feature.UpdateHeader(true /* commonParsed */, true /* metadataParsed */); + + return typesCount > 0; +} + string DebugPrint(XMLFeature const & feature) { ostringstream ost; diff --git a/editor/xml_feature.hpp b/editor/xml_feature.hpp index 92840c3b93..086a5fa397 100644 --- a/editor/xml_feature.hpp +++ b/editor/xml_feature.hpp @@ -13,6 +13,8 @@ #include "3party/pugixml/src/pugixml.hpp" +class FeatureType; + namespace editor { DECLARE_EXCEPTION(XMLFeatureError, RootException); @@ -171,6 +173,19 @@ private: pugi::xml_document m_document; }; +/// Rewrites all but geometry and types. +/// Should be applied to existing features only (in mwm files). +void ApplyPatch(XMLFeature const & xml, FeatureType & feature); + +/// @param serializeType if false, types are not serialized. +/// Useful for applying modifications to existing OSM features, to avoid issues when someone +/// has changed a type in OSM, but our users uploaded invalid outdated type after modifying feature. +XMLFeature ToXML(FeatureType const & feature, bool serializeType); + +/// Creates new feature, including geometry and types. +/// @Note: only nodes (points) are supported at the moment. +bool FromXML(XMLFeature const & xml, FeatureType & feature); + string DebugPrint(XMLFeature const & feature); string DebugPrint(XMLFeature::Type const type); } // namespace editor diff --git a/indexer/feature.cpp b/indexer/feature.cpp index 059b2136e7..49ba919a59 100644 --- a/indexer/feature.cpp +++ b/indexer/feature.cpp @@ -17,9 +17,10 @@ #include "base/range_iterator.hpp" #include "base/stl_helpers.hpp" -#include "std/algorithm.hpp" +#include using namespace feature; +using namespace std; /////////////////////////////////////////////////////////////////////////////////////////////////// // FeatureBase implementation @@ -35,39 +36,6 @@ void FeatureBase::Deserialize(feature::LoaderBase * pLoader, TBuffer buffer) m_header = m_pLoader->GetHeader(); } -void FeatureType::ApplyPatch(editor::XMLFeature const & xml) -{ - xml.ForEachName([this](string const & lang, string const & name) - { - m_params.name.AddString(lang, name); - }); - - string const house = xml.GetHouse(); - if (!house.empty()) - m_params.house.Set(house); - - // TODO(mgsergio): - // m_params.ref = - // m_params.layer = - // m_params.rank = - m_commonParsed = true; - - xml.ForEachTag([this](string const & k, string const & v) - { - Metadata::EType mdType; - if (Metadata::TypeFromString(k, mdType)) - m_metadata.Set(mdType, v); - else - LOG(LWARNING, ("Patching feature has unknown tags")); - }); - m_metadataParsed = true; - - // If types count are changed here, in ApplyPatch, new number of types should be passed - // instead of GetTypesCount(). - m_header = CalculateHeader(GetTypesCount(), Header() & HEADER_GEOTYPE_MASK, m_params); - m_header2Parsed = true; -} - void FeatureType::ReplaceBy(osm::EditableMapObject const & emo) { uint8_t geoType; @@ -106,136 +74,6 @@ void FeatureType::ReplaceBy(osm::EditableMapObject const & emo) m_id = emo.GetID(); } -editor::XMLFeature FeatureType::ToXML(bool serializeType) const -{ - editor::XMLFeature feature(GetFeatureType() == feature::GEOM_POINT - ? editor::XMLFeature::Type::Node - : editor::XMLFeature::Type::Way); - - if (GetFeatureType() == feature::GEOM_POINT) - { - feature.SetCenter(GetCenter()); - } - else - { - ParseTriangles(BEST_GEOMETRY); - feature.SetGeometry(begin(m_triangles), end(m_triangles)); - } - - ForEachName([&feature](uint8_t const & lang, string const & name) - { - feature.SetName(lang, name); - }); - - string const house = GetHouseNumber(); - if (!house.empty()) - feature.SetHouse(house); - - // TODO(mgsergio): - // feature.m_params.ref = - // feature.m_params.layer = - // feature.m_params.rank = - - if (serializeType) - { - feature::TypesHolder th(*this); - // TODO(mgsergio): Use correct sorting instead of SortBySpec based on the config. - th.SortBySpec(); - // TODO(mgsergio): Either improve "OSM"-compatible serialization for more complex types, - // or save all our types directly, to restore and reuse them in migration of modified features. - for (uint32_t const type : th) - { - string const strType = classif().GetReadableObjectName(type); - strings::SimpleTokenizer iter(strType, "-"); - string const k = *iter; - if (++iter) - { - // First (main) type is always stored as "k=amenity v=restaurant". - // Any other "k=amenity v=atm" is replaced by "k=atm v=yes". - if (feature.GetTagValue(k).empty()) - feature.SetTagValue(k, *iter); - else - feature.SetTagValue(*iter, "yes"); - } - else - { - // We're editing building, generic craft, shop, office, amenity etc. - // Skip it's serialization. - // TODO(mgsergio): Correcly serialize all types back and forth. - LOG(LDEBUG, ("Skipping type serialization:", k)); - } - } - } - - for (auto const type : m_metadata.GetPresentTypes()) - { - if (m_metadata.IsSponsoredType(static_cast(type))) - continue; - auto const attributeName = DebugPrint(static_cast(type)); - feature.SetTagValue(attributeName, m_metadata.Get(type)); - } - - return feature; -} - -bool FeatureType::FromXML(editor::XMLFeature const & xml) -{ - ASSERT_EQUAL(editor::XMLFeature::Type::Node, xml.GetType(), - ("At the moment only new nodes (points) can can be created.")); - m_center = xml.GetMercatorCenter(); - m_limitRect.Add(m_center); - m_pointsParsed = m_trianglesParsed = true; - - xml.ForEachName([this](string const & lang, string const & name) - { - m_params.name.AddString(lang, name); - }); - - string const house = xml.GetHouse(); - if (!house.empty()) - m_params.house.Set(house); - - // TODO(mgsergio): - // m_params.ref = - // m_params.layer = - // m_params.rank = - m_commonParsed = true; - - uint32_t typesCount = 0; - xml.ForEachTag([this, &typesCount](string const & k, string const & v) - { - Metadata::EType mdType; - if (Metadata::TypeFromString(k, mdType)) - { - m_metadata.Set(mdType, v); - } - else - { - // Simple heuristics. It works if all our supported types for new features at data/editor.config - // are of one or two levels nesting (currently it's true). - Classificator & cl = classif(); - uint32_t type = cl.GetTypeByPathSafe({k, v}); - if (type == 0) - type = cl.GetTypeByPathSafe({k}); // building etc. - if (type == 0) - type = cl.GetTypeByPathSafe({"amenity", k}); // atm=yes, toilet=yes etc. - - if (type) - m_types[typesCount++] = type; - else - LOG(LWARNING, ("Can't load/parse type:", k, v)); - } - }); - m_metadataParsed = true; - m_typesParsed = true; - - EHeaderTypeMask const geomType = house.empty() && !m_params.ref.empty() ? HEADER_GEOM_POINT : HEADER_GEOM_POINT_EX; - m_header = CalculateHeader(typesCount, geomType, m_params); - m_header2Parsed = true; - - return typesCount > 0; -} - void FeatureBase::ParseTypes() const { if (!m_typesParsed) @@ -266,6 +104,18 @@ feature::EGeomType FeatureBase::GetFeatureType() const } } +void FeatureBase::SetTypes(uint32_t const (&types)[feature::kMaxTypesCount], uint32_t count) +{ + ASSERT_GREATER_OR_EQUAL(count, 1, ("Must be one type at least.")); + ASSERT_LESS(count, feature::kMaxTypesCount, ("Too many types for feature")); + if (count >= feature::kMaxTypesCount) + count = feature::kMaxTypesCount - 1; + fill(begin(m_types), end(m_types), 0); + copy_n(begin(types), count, begin(m_types)); + auto value = static_cast((count - 1) & feature::HEADER_TYPE_MASK); + m_header = (m_header & (~feature::HEADER_TYPE_MASK)) | value; +} + string FeatureBase::DebugString() const { ParseCommon(); @@ -391,6 +241,53 @@ void FeatureType::SetMetadata(feature::Metadata const & newMetadata) m_metadata = newMetadata; } +void FeatureType::UpdateHeader(bool commonParsed, bool metadataParsed) +{ + feature::EHeaderTypeMask geomType = + static_cast(Header() & feature::HEADER_GEOTYPE_MASK); + if (!geomType) + { + geomType = m_params.house.IsEmpty() && !m_params.ref.empty() ? feature::HEADER_GEOM_POINT + : feature::HEADER_GEOM_POINT_EX; + } + + m_header = feature::CalculateHeader(GetTypesCount(), geomType, m_params); + m_header2Parsed = true; + m_typesParsed = true; + + m_commonParsed = commonParsed; + m_metadataParsed = metadataParsed; +} + +bool FeatureType::UpdateMetadataValue(string const & key, string const & value) +{ + feature::Metadata::EType mdType; + if (!feature::Metadata::TypeFromString(key, mdType)) + return false; + m_metadata.Set(mdType, value); + return true; +} + +void FeatureType::ForEachMetadataItem( + bool skipSponsored, function const & fn) const +{ + for (auto const type : m_metadata.GetPresentTypes()) + { + if (skipSponsored && m_metadata.IsSponsoredType(static_cast(type))) + continue; + auto const attributeName = ToString(static_cast(type)); + fn(attributeName, m_metadata.Get(type)); + } +} + +void FeatureType::SetCenter(m2::PointD const & pt) +{ + ASSERT_EQUAL(GetFeatureType(), GEOM_POINT, ("Only for point feature.")); + m_center = pt; + m_limitRect.Add(m_center); + m_pointsParsed = m_trianglesParsed = true; +} + namespace { template diff --git a/indexer/feature.hpp b/indexer/feature.hpp index 2160fc3377..c69e0762ef 100644 --- a/indexer/feature.hpp +++ b/indexer/feature.hpp @@ -50,6 +50,7 @@ public: //@} feature::EGeomType GetFeatureType() const; + FeatureParamsBase & GetParams() {return m_params;} inline uint8_t GetTypesCount() const { @@ -123,6 +124,8 @@ public: f(m_types[i]); } + void SetTypes(uint32_t const (&types)[feature::kMaxTypesCount], uint32_t count); + protected: /// @name Need for FeatureBuilder. //@{ @@ -166,32 +169,25 @@ public: /// @name Editor methods. //@{ - /// Rewrites all but geometry and types. - /// Should be applied to existing features only (in mwm files). - void ApplyPatch(editor::XMLFeature const & xml); /// Apply changes from UI for edited or newly created features. /// Replaces all FeatureType's components. void ReplaceBy(osm::EditableMapObject const & ef); - /// @param serializeType if false, types are not serialized. - /// Useful for applying modifications to existing OSM features, to avoid ussues when someone - /// has changed a type in OSM, but our users uploaded invalid outdated type after modifying feature. - editor::XMLFeature ToXML(bool serializeType) const; - /// Creates new feature, including geometry and types. - /// @Note: only nodes (points) are supported at the moment. - bool FromXML(editor::XMLFeature const & xml); + StringUtf8Multilang const & GetNames() const; + void SetNames(StringUtf8Multilang const & newNames); + void SetMetadata(feature::Metadata const & newMetadata); + + void UpdateHeader(bool commonParsed, bool metadataParsed); + bool UpdateMetadataValue(string const & key, string const & value); + void ForEachMetadataItem(bool skipSponsored, + function const & fn) const; + + void SetCenter(m2::PointD const &pt); //@} inline void SetID(FeatureID const & id) { m_id = id; } inline FeatureID const & GetID() const { return m_id; } - /// @name Editor functions. - //@{ - StringUtf8Multilang const & GetNames() const; - void SetNames(StringUtf8Multilang const & newNames); - void SetMetadata(feature::Metadata const & newMetadata); - //@} - /// @name Parse functions. Do simple dispatching to m_pLoader. //@{ /// Super-method to call all possible Parse* methods. diff --git a/indexer/feature_meta.cpp b/indexer/feature_meta.cpp index 6b521895ac..2ed8f1bf33 100644 --- a/indexer/feature_meta.cpp +++ b/indexer/feature_meta.cpp @@ -165,7 +165,7 @@ void RegionData::AddPublicHoliday(int8_t month, int8_t offset) } // namespace feature // Warning: exact osm tag keys should be returned for valid enum values. -string DebugPrint(feature::Metadata::EType type) +string ToString(feature::Metadata::EType type) { using feature::Metadata; switch (type) diff --git a/indexer/feature_meta.hpp b/indexer/feature_meta.hpp index 443a0f4704..820f4b27f5 100644 --- a/indexer/feature_meta.hpp +++ b/indexer/feature_meta.hpp @@ -226,4 +226,5 @@ public: } // namespace feature // Prints types in osm-friendly format. -string DebugPrint(feature::Metadata::EType type); +string ToString(feature::Metadata::EType type); +inline string DebugPrint(feature::Metadata::EType type) { return ToString(type); }