diff --git a/editor/changeset_wrapper.cpp b/editor/changeset_wrapper.cpp index 2b00a1e4d8..d151fca0b9 100644 --- a/editor/changeset_wrapper.cpp +++ b/editor/changeset_wrapper.cpp @@ -34,6 +34,30 @@ bool OsmFeatureHasTags(pugi::xml_node const & osmFt) return osmFt.child("tag"); } +vector const static kMainTags = {"amenity", "shop", "tourism", "historic", + "craft", "emergency", "barrier", "highway", + "office", "entrance", "building"}; + +string GetTypeForFeature(XMLFeature const & node) +{ + for (string const & key : kMainTags) + { + if (node.HasTag(key)) + { + string const value = node.GetTagValue(key); + if (value == "yes") + return key; + else if (key == "shop" || key == "office" || key == "building" || key == "entrance") + return value + " " + key; // "convenience shop" + else + return value; + } + } + + // Did not find any known tags. + return node.HasAnyTags() ? "unknown object" : "empty object"; +} + vector NaiveSample(vector const & source, size_t count) { count = min(count, source.size()); @@ -74,7 +98,8 @@ namespace osm { ChangesetWrapper::ChangesetWrapper(TKeySecret const & keySecret, ServerApi06::TKeyValueTags const & comments) noexcept - : m_changesetComments(comments), m_api(OsmOAuth::ServerAuth(keySecret)) + : m_changesetComments(comments), + m_api(OsmOAuth::ServerAuth(keySecret)) { } @@ -84,6 +109,8 @@ ChangesetWrapper::~ChangesetWrapper() { try { + m_changesetComments["comment"] = GetDescription(); + m_api.UpdateChangeSet(m_changesetId, m_changesetComments); m_api.CloseChangeSet(m_changesetId); } catch (std::exception const & ex) @@ -100,7 +127,9 @@ void ChangesetWrapper::LoadXmlFromOSM(ms::LatLon const & ll, pugi::xml_document MYTHROW(HttpErrorException, ("HTTP error", response, "with GetXmlFeaturesAtLatLon", ll)); if (pugi::status_ok != doc.load(response.second.c_str()).status) - MYTHROW(OsmXmlParseException, ("Can't parse OSM server response for GetXmlFeaturesAtLatLon request", response.second)); + MYTHROW( + OsmXmlParseException, + ("Can't parse OSM server response for GetXmlFeaturesAtLatLon request", response.second)); } void ChangesetWrapper::LoadXmlFromOSM(ms::LatLon const & min, ms::LatLon const & max, pugi::xml_document & doc) @@ -110,7 +139,8 @@ void ChangesetWrapper::LoadXmlFromOSM(ms::LatLon const & min, ms::LatLon const & MYTHROW(HttpErrorException, ("HTTP error", response, "with GetXmlFeaturesInRect", min, max)); if (pugi::status_ok != doc.load(response.second.c_str()).status) - MYTHROW(OsmXmlParseException, ("Can't parse OSM server response for GetXmlFeaturesInRect request", response.second)); + MYTHROW(OsmXmlParseException, + ("Can't parse OSM server response for GetXmlFeaturesInRect request", response.second)); } XMLFeature ChangesetWrapper::GetMatchingNodeFeatureFromOSM(m2::PointD const & center) @@ -171,8 +201,7 @@ XMLFeature ChangesetWrapper::GetMatchingAreaFeatureFromOSM(vector co stringstream sstr; bestWayOrRelation.print(sstr); LOG(LDEBUG, ("Relation is the best match", sstr.str())); - MYTHROW(RelationFeatureAreNotSupportedException, - ("Got relation as the best matching")); + MYTHROW(RelationFeatureAreNotSupportedException, ("Got relation as the best matching")); } if (!OsmFeatureHasTags(bestWayOrRelation)) @@ -204,6 +233,7 @@ void ChangesetWrapper::Create(XMLFeature node) node.SetAttribute("changeset", strings::to_string(m_changesetId)); // TODO(AlexZ): Think about storing/logging returned OSM ids. UNUSED_VALUE(m_api.CreateElement(node)); + m_created_types[GetTypeForFeature(node)]++; } void ChangesetWrapper::Modify(XMLFeature node) @@ -214,6 +244,7 @@ void ChangesetWrapper::Modify(XMLFeature node) // Changeset id should be updated for every OSM server commit. node.SetAttribute("changeset", strings::to_string(m_changesetId)); m_api.ModifyElement(node); + m_modified_types[GetTypeForFeature(node)]++; } void ChangesetWrapper::Delete(XMLFeature node) @@ -224,6 +255,80 @@ void ChangesetWrapper::Delete(XMLFeature node) // Changeset id should be updated for every OSM server commit. node.SetAttribute("changeset", strings::to_string(m_changesetId)); m_api.DeleteElement(node); + m_deleted_types[GetTypeForFeature(node)]++; } -} // namespace osm +string ChangesetWrapper::TypeCountToString(TTypeCount const & typeCount) +{ + if (typeCount.empty()) + return string(); + + // Convert map to vector and sort pairs by count, descending. + vector> items; + for (auto const & tc : typeCount) + items.push_back(tc); + + sort(items.begin(), items.end(), + [](pair const & a, pair const & b) + { + return a.second > b.second; + }); + + ostringstream ss; + auto const limit = min(3UL, items.size()); + for (auto i = 0; i < limit; ++i) + { + if (i > 0) + { + // Separator: "A and B" for two, "A, B, and C" for three or more. + if (limit > 2) + ss << ", "; + else + ss << " "; + if (i == limit - 1) + ss << "and "; + } + + auto & currentPair = items[i]; + // If we have more objects left, make the last one a list of these. + if (i == limit - 1 && limit < items.size()) + { + int count = 0; + for (auto j = i; j < items.size(); ++j) + count += items[j].second; + currentPair = {"other object", count}; + } + + // Format a count: "a shop" for single shop, "4 shops" for multiple. + if (currentPair.second == 1) + ss << "a "; + else + ss << currentPair.second << ' '; + ss << currentPair.first; + if (currentPair.second > 1) + ss << 's'; + } + return ss.str(); +} + +string ChangesetWrapper::GetDescription() const +{ + string result; + if (!m_created_types.empty()) + result = "Created " + TypeCountToString(m_created_types); + if (!m_modified_types.empty()) + { + if (!result.empty()) + result += "; "; + result += "Updated " + TypeCountToString(m_modified_types); + } + if (!m_deleted_types.empty()) + { + if (!result.empty()) + result += "; "; + result += "Deleted " + TypeCountToString(m_deleted_types); + } + return result; +} + +} // namespace osm diff --git a/editor/changeset_wrapper.hpp b/editor/changeset_wrapper.hpp index 375cd2b279..4081044e94 100644 --- a/editor/changeset_wrapper.hpp +++ b/editor/changeset_wrapper.hpp @@ -19,6 +19,8 @@ struct ClientToken; class ChangesetWrapper { + using TTypeCount = map; + public: DECLARE_EXCEPTION(ChangesetWrapperException, RootException); DECLARE_EXCEPTION(NetworkErrorException, ChangesetWrapperException); @@ -32,7 +34,8 @@ public: DECLARE_EXCEPTION(RelationFeatureAreNotSupportedException, ChangesetWrapperException); DECLARE_EXCEPTION(EmptyFeatureException, ChangesetWrapperException); - ChangesetWrapper(TKeySecret const & keySecret, ServerApi06::TKeyValueTags const & comments) noexcept; + ChangesetWrapper(TKeySecret const & keySecret, + ServerApi06::TKeyValueTags const & comments) noexcept; ~ChangesetWrapper(); /// Throws many exceptions from above list, plus including XMLNode's parsing ones. @@ -60,6 +63,12 @@ private: ServerApi06 m_api; static constexpr uint64_t kInvalidChangesetId = 0; uint64_t m_changesetId = kInvalidChangesetId; + + TTypeCount m_modified_types; + TTypeCount m_created_types; + TTypeCount m_deleted_types; + static string TypeCountToString(TTypeCount const & typeCount); + string GetDescription() const; }; -} // namespace osm +} // namespace osm diff --git a/editor/editor_tests/server_api_test.cpp b/editor/editor_tests/server_api_test.cpp index 20c4fa0110..e4b69e8122 100644 --- a/editor/editor_tests/server_api_test.cpp +++ b/editor/editor_tests/server_api_test.cpp @@ -96,6 +96,10 @@ UNIT_TEST(OSM_ServerAPI_ChangesetAndNode) // After modification, node version increases in ModifyElement. TEST_EQUAL(node.GetAttribute("version"), "2", ()); + // All tags must be specified, because there is no merging of old and new tags. + api.UpdateChangeSet(changeSetId, {{"created_by", "MAPS.ME Unit Test"}, + {"comment", "For test purposes only (updated)."}}); + // To retrieve created node, changeset should be closed first. // It is done here via Scope Guard. } diff --git a/editor/server_api.cpp b/editor/server_api.cpp index 4d511e6df2..f20ce26394 100644 --- a/editor/server_api.cpp +++ b/editor/server_api.cpp @@ -13,6 +13,21 @@ #include "3party/pugixml/src/pugixml.hpp" +namespace +{ +string KeyValueTagsToXML(osm::ServerApi06::TKeyValueTags const & kvTags) +{ + ostringstream stream; + stream << "\n" + "\n"; + for (auto const & tag : kvTags) + stream << " \n"; + stream << "\n" + "\n"; + return stream.str(); +} +} // namespace + namespace osm { @@ -26,15 +41,7 @@ uint64_t ServerApi06::CreateChangeSet(TKeyValueTags const & kvTags) const if (!m_auth.IsAuthorized()) MYTHROW(NotAuthorized, ("Not authorized.")); - ostringstream stream; - stream << "\n" - "\n"; - for (auto const & tag : kvTags) - stream << " \n"; - stream << "\n" - "\n"; - - OsmOAuth::Response const response = m_auth.Request("/changeset/create", "PUT", stream.str()); + OsmOAuth::Response const response = m_auth.Request("/changeset/create", "PUT", KeyValueTagsToXML(kvTags)); if (response.first != OsmOAuth::HTTP::OK) MYTHROW(CreateChangeSetHasFailed, ("CreateChangeSet request has failed:", response)); @@ -97,6 +104,13 @@ void ServerApi06::DeleteElement(editor::XMLFeature const & element) const MYTHROW(ErrorDeletingElement, ("Could not delete an element:", response)); } +void ServerApi06::UpdateChangeSet(uint64_t changesetId, TKeyValueTags const & kvTags) const +{ + OsmOAuth::Response const response = m_auth.Request("/changeset/" + strings::to_string(changesetId), "PUT", KeyValueTagsToXML(kvTags)); + if (response.first != OsmOAuth::HTTP::OK) + MYTHROW(UpdateChangeSetHasFailed, ("UpdateChangeSet request has failed:", response)); +} + void ServerApi06::CloseChangeSet(uint64_t changesetId) const { OsmOAuth::Response const response = m_auth.Request("/changeset/" + strings::to_string(changesetId) + "/close", "PUT"); diff --git a/editor/server_api.hpp b/editor/server_api.hpp index 1ff1b697a8..d5cfaa5380 100644 --- a/editor/server_api.hpp +++ b/editor/server_api.hpp @@ -34,6 +34,7 @@ public: DECLARE_EXCEPTION(NotAuthorized, ServerApi06Exception); DECLARE_EXCEPTION(CantParseServerResponse, ServerApi06Exception); DECLARE_EXCEPTION(CreateChangeSetHasFailed, ServerApi06Exception); + DECLARE_EXCEPTION(UpdateChangeSetHasFailed, ServerApi06Exception); DECLARE_EXCEPTION(CreateElementHasFailed, ServerApi06Exception); DECLARE_EXCEPTION(ModifiedElementHasNoIdAttribute, ServerApi06Exception); DECLARE_EXCEPTION(ModifyElementHasFailed, ServerApi06Exception); @@ -70,6 +71,7 @@ public: /// @param element should already have all attributes set, including "id", "version", "changeset". /// @returns true if element was successfully deleted (or was already deleted). void DeleteElement(editor::XMLFeature const & element) const; + void UpdateChangeSet(uint64_t changesetId, TKeyValueTags const & kvTags) const; void CloseChangeSet(uint64_t changesetId) const; /// @returns id of a created note. uint64_t CreateNote(ms::LatLon const & ll, string const & message) const; diff --git a/editor/xml_feature.cpp b/editor/xml_feature.cpp index 2367f036eb..c125725efb 100644 --- a/editor/xml_feature.cpp +++ b/editor/xml_feature.cpp @@ -323,6 +323,11 @@ void XMLFeature::SetUploadError(string const & error) SetAttribute(kUploadError, error); } +bool XMLFeature::HasAnyTags() const +{ + return m_document.child("tag"); +} + bool XMLFeature::HasTag(string const & key) const { return FindTag(m_document, key); diff --git a/editor/xml_feature.hpp b/editor/xml_feature.hpp index f6f09240fd..720ed81e68 100644 --- a/editor/xml_feature.hpp +++ b/editor/xml_feature.hpp @@ -140,6 +140,7 @@ public: void SetUploadError(string const & error); //@} + bool HasAnyTags() const; bool HasTag(string const & key) const; bool HasAttribute(string const & key) const; bool HasKey(string const & key) const;