From 9a8c58ee092ee108f5b360c55fdce34611221d11 Mon Sep 17 00:00:00 2001 From: "r.kuznetsov" Date: Tue, 31 Jul 2018 21:09:14 +0300 Subject: [PATCH] Review fixes --- android/jni/com/mapswithme/platform/HttpUserAgent.cpp | 2 +- editor/user_stats.cpp | 2 +- local_ads/statistics.cpp | 2 +- map/cloud.cpp | 2 +- map/local_ads_manager.cpp | 2 +- map/user.cpp | 2 +- partners_api/locals_api.cpp | 2 +- platform/http_client.cpp | 5 ----- platform/http_client.hpp | 3 --- platform/http_user_agent.cpp | 7 +------ platform/http_user_agent.hpp | 5 +++-- platform/platform.hpp | 1 + platform/remote_file.cpp | 2 +- routing/online_cross_fetcher.cpp | 2 +- storage/diff_scheme/diff_scheme_checker.cpp | 2 +- traffic/traffic_info.cpp | 4 ++-- 16 files changed, 17 insertions(+), 28 deletions(-) diff --git a/android/jni/com/mapswithme/platform/HttpUserAgent.cpp b/android/jni/com/mapswithme/platform/HttpUserAgent.cpp index efe28bf40f..e2a5e22fc8 100644 --- a/android/jni/com/mapswithme/platform/HttpUserAgent.cpp +++ b/android/jni/com/mapswithme/platform/HttpUserAgent.cpp @@ -7,4 +7,4 @@ std::string HttpUserAgent::ExtractAppVersion() const //TODO(@alexzatsepin): implement app version extraction. return {}; } -} // platform \ No newline at end of file +} // platform diff --git a/editor/user_stats.cpp b/editor/user_stats.cpp index 66e3b3c6fa..b645fe78b5 100644 --- a/editor/user_stats.cpp +++ b/editor/user_stats.cpp @@ -90,7 +90,7 @@ bool UserStatsLoader::Update(string const & userName) auto const url = kUserStatsUrl + "&name=" + UrlEncode(userName); platform::HttpClient request(url); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (!request.RunHttpRequest()) { diff --git a/local_ads/statistics.cpp b/local_ads/statistics.cpp index 95039eba7b..41ce9d9ae6 100644 --- a/local_ads/statistics.cpp +++ b/local_ads/statistics.cpp @@ -423,7 +423,7 @@ void Statistics::SendFileWithMetadata(MetadataKey && metadataKey, Metadata && me #endif request.SetBodyData(std::string(bytes.begin(), bytes.end()), contentType, "POST", contentEncoding); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (request.RunHttpRequest() && request.ErrorCode() == 200) { GetPlatform().RunTask(Platform::Thread::File, [this, metadataKey = std::move(metadataKey), diff --git a/map/cloud.cpp b/map/cloud.cpp index 45a13d3991..f84e593688 100644 --- a/map/cloud.cpp +++ b/map/cloud.cpp @@ -231,7 +231,7 @@ Cloud::RequestResult CloudRequestWithResult(std::string const & url, request.SetTimeout(kRequestTimeoutInSec); request.SetRawHeader("Accept", kApplicationJson); request.SetRawHeader("Authorization", BuildAuthenticationToken(accessToken)); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); request.SetBodyData(SerializeToJson(RequestDataType(args...)), kApplicationJson); if (request.RunHttpRequest() && !request.WasRedirected()) diff --git a/map/local_ads_manager.cpp b/map/local_ads_manager.cpp index 65e9cca8e1..a500b22848 100644 --- a/map/local_ads_manager.cpp +++ b/map/local_ads_manager.cpp @@ -414,7 +414,7 @@ bool LocalAdsManager::DownloadCampaign(MwmSet::MwmId const & mwmId, std::vector< platform::HttpClient request(url); request.SetTimeout(5); // timeout in seconds - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); bool const success = request.RunHttpRequest() && request.ErrorCode() == 200; std::lock_guard lock(m_campaignsMutex); diff --git a/map/user.cpp b/map/user.cpp index fad56d1167..2ecd56ac03 100644 --- a/map/user.cpp +++ b/map/user.cpp @@ -565,7 +565,7 @@ void User::RequestImpl(std::string const & url, BuildRequestHandler const & onBu platform::HttpClient request(url); request.SetRawHeader("Accept", kApplicationJson); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (onBuildRequest) onBuildRequest(request); diff --git a/partners_api/locals_api.cpp b/partners_api/locals_api.cpp index c9bb3b32b2..0733b0052e 100644 --- a/partners_api/locals_api.cpp +++ b/partners_api/locals_api.cpp @@ -84,7 +84,7 @@ bool RawApi::Get(double lat, double lon, std::string const & lang, size_t result platform::HttpClient request(ostream.str()); request.SetHttpMethod("GET"); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (request.RunHttpRequest()) { result = request.ServerResponse(); diff --git a/platform/http_client.cpp b/platform/http_client.cpp index 4561eeb5f9..a7b3a03641 100644 --- a/platform/http_client.cpp +++ b/platform/http_client.cpp @@ -95,11 +95,6 @@ void HttpClient::SetTimeout(double timeoutSec) m_timeoutSec = timeoutSec; } -void HttpClient::SetUserAgent(HttpUserAgent const & userAgent) -{ - m_headers.emplace(userAgent.Key(), userAgent.Value()); -} - string const & HttpClient::UrlRequested() const { return m_urlRequested; diff --git a/platform/http_client.hpp b/platform/http_client.hpp index 7273f37753..37f6096d90 100644 --- a/platform/http_client.hpp +++ b/platform/http_client.hpp @@ -23,8 +23,6 @@ SOFTWARE. *******************************************************************************/ #pragma once -#include "platform/http_user_agent.hpp" - #include "base/macros.hpp" #include "std/string.hpp" @@ -85,7 +83,6 @@ public: HttpClient & SetHandleRedirects(bool handle_redirects); HttpClient & SetRawHeader(string const & key, string const & value); void SetTimeout(double timeoutSec); - void SetUserAgent(HttpUserAgent const & userAgent); string const & UrlRequested() const; // @returns empty string in the case of error diff --git a/platform/http_user_agent.cpp b/platform/http_user_agent.cpp index c3651a6f5c..304df85d06 100644 --- a/platform/http_user_agent.cpp +++ b/platform/http_user_agent.cpp @@ -9,12 +9,7 @@ HttpUserAgent::HttpUserAgent() m_appVersion = ExtractAppVersion(); } -std::string HttpUserAgent::Key() const -{ - return "User-Agent"; -} - -std::string HttpUserAgent::Value() const +std::string HttpUserAgent::Get() const { std::stringstream ss; ss << "MAPS.ME/"; diff --git a/platform/http_user_agent.hpp b/platform/http_user_agent.hpp index 842bd98b1b..8636edcc0b 100644 --- a/platform/http_user_agent.hpp +++ b/platform/http_user_agent.hpp @@ -8,8 +8,9 @@ class HttpUserAgent { public: HttpUserAgent(); - std::string Key() const; - std::string Value() const; + std::string Get() const; + + operator std::string() const { return Get(); } private: std::string ExtractAppVersion() const; diff --git a/platform/platform.hpp b/platform/platform.hpp index 47aa3b9906..56d81818fd 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -284,6 +284,7 @@ public: MarketingService & GetMarketingService() { return m_marketingService; } platform::SecureStorage & GetSecureStorage() { return m_secureStorage; } platform::HttpUserAgent & GetAppUserAgent() { return m_appUserAgent; } + platform::HttpUserAgent const & GetAppUserAgent() const { return m_appUserAgent; } /// \brief Placing an executable object |task| on a queue of |thread|. Then the object will be /// executed on |thread|. diff --git a/platform/remote_file.cpp b/platform/remote_file.cpp index 961917af25..a79a3ecdf1 100644 --- a/platform/remote_file.cpp +++ b/platform/remote_file.cpp @@ -26,7 +26,7 @@ RemoteFile::Result RemoteFile::Download(std::string const & filePath) const platform::HttpClient request(m_url); request.SetTimeout(kRequestTimeoutInSec); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (request.RunHttpRequest()) { if (!m_allowRedirection && request.WasRedirected()) diff --git a/routing/online_cross_fetcher.cpp b/routing/online_cross_fetcher.cpp index 3ad70a74ca..17efb9bbe9 100644 --- a/routing/online_cross_fetcher.cpp +++ b/routing/online_cross_fetcher.cpp @@ -77,7 +77,7 @@ void OnlineCrossFetcher::Do() string const url = GenerateOnlineRequest(m_serverURL, MercatorBounds::ToLatLon(pointFrom), MercatorBounds::ToLatLon(pointTo)); platform::HttpClient request(url); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); LOG(LINFO, ("Check mwms by URL: ", url)); if (request.RunHttpRequest() && request.ErrorCode() == 200 && !request.WasRedirected()) diff --git a/storage/diff_scheme/diff_scheme_checker.cpp b/storage/diff_scheme/diff_scheme_checker.cpp index 420f72be74..0751dea5b0 100644 --- a/storage/diff_scheme/diff_scheme_checker.cpp +++ b/storage/diff_scheme/diff_scheme_checker.cpp @@ -119,7 +119,7 @@ NameDiffInfoMap Checker::Check(LocalMapsInfo const & info) ASSERT(!body.empty(), ()); request.SetBodyData(body, "application/json"); request.SetTimeout(kTimeoutInSeconds); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); NameDiffInfoMap diffs; if (request.RunHttpRequest() && !request.WasRedirected() && request.ErrorCode() == 200) { diff --git a/traffic/traffic_info.cpp b/traffic/traffic_info.cpp index 95a9f73368..48a6b26264 100644 --- a/traffic/traffic_info.cpp +++ b/traffic/traffic_info.cpp @@ -41,7 +41,7 @@ namespace bool ReadRemoteFile(string const & url, vector & contents, int & errorCode) { platform::HttpClient request(url); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); if (!request.RunHttpRequest()) { errorCode = request.ErrorCode(); @@ -453,7 +453,7 @@ TrafficInfo::ServerDataStatus TrafficInfo::ReceiveTrafficValues(string & etag, v platform::HttpClient request(url); request.LoadHeaders(true); - request.SetUserAgent(GetPlatform().GetAppUserAgent()); + request.SetRawHeader("User-Agent", GetPlatform().GetAppUserAgent()); request.SetRawHeader("If-None-Match", etag); if (!request.RunHttpRequest() || request.ErrorCode() != 200)