[editor] remove error messages from CS comment and enforce OSM 255 char length limit (#919)

Signed-off-by: map-per <map-per@gmx.de>

libs/base/string_utils.hpp

@@ -173,6 +173,29 @@ inline constexpr bool IsASCIISpace(T c)
 
 bool IsASCIILatin(UniChar c);
 
+/// Escape characters not allowed in XML
+template <typename T>
+std::string EscapeForXML(const T & in)
+{
+  std::string result;
+  result.reserve(in.size());
+
+  for (char c : in)
+  {
+    switch (c)
+    {
+      case '&':  result.append("&amp;");  break;
+      case '<':  result.append("&lt;");   break;
+      case '>':  result.append("&gt;");   break;
+      case '"':  result.append("&quot;"); break;
+      case '\'': result.append("&apos;"); break;
+      default:   result.append(1, c);     break;
+    }
+  }
+
+  return result;
+}
+
 inline std::string DebugPrint(UniString const & s)
 {
   return ToUtf8(s);

libs/editor/changeset_wrapper.cpp

@@ -115,7 +115,7 @@ ChangesetWrapper::~ChangesetWrapper()
   {
     try
     {
-      m_changesetComments["comment"] = GetDescription();
+      AddChangesetTag("comment", GetDescription());
       m_api.UpdateChangeSet(m_changesetId, m_changesetComments);
       m_api.CloseChangeSet(m_changesetId);
     }
@@ -240,7 +240,25 @@ void ChangesetWrapper::Modify(editor::XMLFeature node)
 
 void ChangesetWrapper::AddChangesetTag(std::string key, std::string value)
 {
-  m_changesetComments.emplace(std::move(key), std::move(value));
+  value = strings::EscapeForXML(value);
+
+  //OSM has a length limit of 255 characters
+  if (value.length() > kMaximumOsmChars)
+  {
+    LOG(LWARNING, ("value is too long for OSM 255 char limit: ", value));
+    value = value.substr(0, kMaximumOsmChars - 3).append("...");
+  }
+
+  m_changesetComments.insert_or_assign(std::move(key), std::move(value));
+}
+
+void ChangesetWrapper::AddToChangesetKeyList(std::string key, std::string value)
+{
+  auto it = m_changesetComments.find(key);
+  if (it == m_changesetComments.end())
+    AddChangesetTag(std::move(key), std::move(value));
+  else
+    AddChangesetTag(std::move(key), it->second + "; " + value);
 }
 
 void ChangesetWrapper::Delete(editor::XMLFeature node)
@@ -338,17 +356,6 @@ std::string ChangesetWrapper::GetDescription() const
       result.append("; ");
     result.append("Deleted ").append(TypeCountToString(m_deleted_types));
   }
-  if (!m_error.empty())
-  {
-    if (!result.empty())
-      result.append("; ");
-    result.append(m_error);
-  }
   return result;
 }
-
-void ChangesetWrapper::SetErrorDescription(std::string const & error)
-{
-  m_error = error;
-}
 }  // namespace osm

libs/editor/changeset_wrapper.hpp

@@ -52,8 +52,8 @@ public:
   /// Add a tag to the changeset
   void AddChangesetTag(std::string key, std::string value);
 
-  /// Allows to see exception details in OSM changesets for easier debugging.
+  /// Add item to ';' separated list for a changeset key
-  void SetErrorDescription(std::string const & error);
+  void AddToChangesetKeyList(std::string key, std::string value);
 
 private:
   /// Unfortunately, pugi can't return xml_documents from methods.
@@ -65,11 +65,11 @@ private:
   ServerApi06 m_api;
   static constexpr uint64_t kInvalidChangesetId = 0;
   uint64_t m_changesetId = kInvalidChangesetId;
+  static constexpr int kMaximumOsmChars = 255;
 
   TypeCount m_modified_types;
   TypeCount m_created_types;
   TypeCount m_deleted_types;
-  std::string m_error;
   static std::string TypeCountToString(TypeCount const & typeCount);
   std::string GetDescription() const;
 };

libs/editor/osm_editor.cpp

@@ -829,7 +829,7 @@ void Editor::UploadChanges(string const & oauthToken, ChangesetTags tags, Finish
           uploadInfo.m_uploadError = ex.Msg();
           ++errorsCount;
           LOG(LWARNING, (ex.what()));
-          changeset.SetErrorDescription(ex.Msg());
+          changeset.AddToChangesetKeyList("upload_attempt_error", kDeletedFromOSMServer);
         }
         catch (ChangesetWrapper::EmptyFeatureException const & ex)
         {
@@ -837,7 +837,7 @@ void Editor::UploadChanges(string const & oauthToken, ChangesetTags tags, Finish
           uploadInfo.m_uploadError = ex.Msg();
           ++errorsCount;
           LOG(LWARNING, (ex.what()));
-          changeset.SetErrorDescription(ex.Msg());
+          changeset.AddToChangesetKeyList("upload_attempt_error", kMatchedFeatureIsEmpty);
         }
         catch (RootException const & ex)
         {
@@ -845,7 +845,7 @@ void Editor::UploadChanges(string const & oauthToken, ChangesetTags tags, Finish
           uploadInfo.m_uploadError = ex.Msg();
           ++errorsCount;
           LOG(LWARNING, (ex.what()));
-          changeset.SetErrorDescription(ex.Msg());
+          changeset.AddToChangesetKeyList("upload_attempt_error", ex.Msg());
         }
         // TODO(AlexZ): Use timestamp from the server.
         uploadInfo.m_uploadAttemptTimestamp = time(nullptr);