From 14274b554310cfaec9daddebb73553a8c7bf6349 Mon Sep 17 00:00:00 2001 From: Ilya Zverev Date: Wed, 23 Mar 2016 18:45:03 +0300 Subject: [PATCH] [editor] Changeset comments --- editor/changeset_wrapper.cpp | 106 ++++++++++++++++++++++++++++++++--- editor/changeset_wrapper.hpp | 13 ++++- editor/server_api.cpp | 32 ++++++++--- editor/server_api.hpp | 1 + 4 files changed, 133 insertions(+), 19 deletions(-) diff --git a/editor/changeset_wrapper.cpp b/editor/changeset_wrapper.cpp index 73e73e91ed..a426d38cc9 100644 --- a/editor/changeset_wrapper.cpp +++ b/editor/changeset_wrapper.cpp @@ -29,9 +29,34 @@ m2::RectD GetBoundingRect(vector const & geometry) return rect; } -bool OsmFeatureHasTags(pugi::xml_node const & osmFt) +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) { - return osmFt.child("tag"); + for (string const & key : kMainTags) + { + if (node.HasTag(key)) + { + string 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. + bool foundTags = false; + node.ForEachTag([&foundTags](string const & k, string const & v) + { + foundTags = true; + }); + return foundTags ? "unknown object" : "empty object"; } vector NaiveSample(vector const & source, size_t count) @@ -74,7 +99,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 +110,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 +128,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(m2::RectD const & rect, pugi::xml_document & doc) @@ -110,7 +140,8 @@ void ChangesetWrapper::LoadXmlFromOSM(m2::RectD const & rect, pugi::xml_document MYTHROW(HttpErrorException, ("HTTP error", response, "with GetXmlFeaturesInRect", rect)); 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 +202,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 +234,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 +245,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) @@ -226,4 +258,62 @@ void ChangesetWrapper::Delete(XMLFeature node) m_api.DeleteElement(node); } -} // namespace osm +string ChangesetWrapper::TypeCountToString(TTypeCount const & typeCount) const +{ + if (typeCount.empty()) + return string(); + if (typeCount.size() > 3) + { + // TODO(zverik): Instead return top 2 + "and XX other features". + int count = 0; + for (auto const & tc : typeCount) + count += tc.second; + return strings::to_string(count) + " objects"; + } + + // TODO(zverik): Reverse sort by count + vector> items; + for (auto const & tc : typeCount) + items.push_back(tc); + + ostringstream ss; + for (auto i = 0; i < items.size(); ++i) + { + if (i > 0) + { + // Separator: "A and B" for two, "A, B, and C" for three or more. + if (items.size() > 2) + ss << ", "; + else + ss << " "; + if (i == items.size() - 1) + ss << "and "; + } + + // Format a count: "a shop" for single shop, "4 shops" for multiple. + if (items[i].second == 1) + ss << "a "; + else + ss << items[i].second << ' '; + ss << items[i].first; + if (items[i].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); + } + return result; +} + +} // namespace osm diff --git a/editor/changeset_wrapper.hpp b/editor/changeset_wrapper.hpp index 3cc09f251d..c1d0d5583c 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; + + // No m_deleted_types here, since we do not delete features. + TTypeCount m_modified_types; + TTypeCount m_created_types; + string TypeCountToString(TTypeCount const & typeCount) const; + string GetDescription() const; }; -} // namespace osm +} // namespace osm diff --git a/editor/server_api.cpp b/editor/server_api.cpp index fad349603c..513fb04a02 100644 --- a/editor/server_api.cpp +++ b/editor/server_api.cpp @@ -12,6 +12,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 { @@ -25,15 +40,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)); @@ -96,6 +103,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(CreateChangeSetHasFailed, ("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 e69983b5a5..57027d0462 100644 --- a/editor/server_api.hpp +++ b/editor/server_api.hpp @@ -70,6 +70,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;