[partners_api] booking review fixes

14 files changed, 139 insertions, 70 deletions

diff --git a/base/CMakeLists.txt b/base/CMakeLists.txt
index 4bf65b9622..006e6003d8 100644
--- a/base/CMakeLists.txt
+++ b/base/CMakeLists.txt
@@ -88,6 +88,8 @@ set(
   timer.hpp
   uni_string_dfa.cpp
   uni_string_dfa.hpp
+  url_helpers.cpp
+  url_helpers.hpp
   visitor.hpp
   worker_thread.cpp
   worker_thread.hpp
diff --git a/base/base.pro b/base/base.pro
index 11c8a56e7e..db228f5130 100644
--- a/base/base.pro
+++ b/base/base.pro
@@ -36,6 +36,7 @@ SOURCES += \
     timegm.cpp \
     timer.cpp \
     uni_string_dfa.cpp \
+    url_helpers.cpp \
     worker_thread.cpp \
 
 HEADERS += \
@@ -100,6 +101,7 @@ HEADERS += \
     timegm.hpp \
     timer.hpp \
     uni_string_dfa.hpp \
+    url_helpers.hpp \
     visitor.hpp \
     waiter.hpp \
     worker_thread.hpp \
diff --git a/base/url_helpers.cpp b/base/url_helpers.cpp
new file mode 100644
index 0000000000..27090e3a53
--- /dev/null
+++ b/base/url_helpers.cpp
@@ -0,0 +1,35 @@
+#include "base/url_helpers.hpp"
+
+#include <sstream>
+
+using namespace std;
+
+namespace base
+{
+namespace url
+{
+string Make(string const & baseUrl, Params const & params)
+{
+  ostringstream os;
+  os << baseUrl;
+
+  bool firstParam = true;
+  for (auto const & param : params)
+  {
+    if (firstParam)
+    {
+      firstParam = false;
+      os << "?";
+    }
+    else
+    {
+      os << "&";
+    }
+
+    os << param.m_name << "=" << param.m_value;
+  }
+
+  return os.str();
+}
+}  // namespace url
+}  // namespace base
diff --git a/base/url_helpers.hpp b/base/url_helpers.hpp
new file mode 100644
index 0000000000..185d515ff6
--- /dev/null
+++ b/base/url_helpers.hpp
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <string>
+#include <utility>
+#include <vector>
+
+namespace base
+{
+namespace url
+{
+struct Param
+{
+  std::string m_name;
+  std::string m_value;
+};
+
+using Params = std::vector<Param>;
+
+// Make URL by using base url and vector of params.
+std::string Make(std::string const & baseUrl, Params const & params);
+}  // namespace url
+}  // namespace base
diff --git a/coding/coding.pro b/coding/coding.pro
index 12eca71250..5b0d2316ce 100644
--- a/coding/coding.pro
+++ b/coding/coding.pro
@@ -99,4 +99,4 @@ HEADERS += \
     writer.hpp \
     zip_creator.hpp \
     zip_reader.hpp \
-    zlib.hpp \
+    zlib.hpp
diff --git a/map/local_ads_manager.cpp b/map/local_ads_manager.cpp
index 70695be79e..cb9fb31689 100644
--- a/map/local_ads_manager.cpp
+++ b/map/local_ads_manager.cpp
@@ -25,6 +25,8 @@
 #include "coding/url_encode.hpp"
 #include "coding/zlib.hpp"
 
+#include "base/url_helpers.hpp"
+
 #include "3party/jansson/myjansson.hpp"
 
 #include "private.h"
@@ -37,6 +39,8 @@
 // is deflated JSON.
 #define TEMPORARY_LOCAL_ADS_JSON_SERIALIZATION
 
+using namespace base;
+
 namespace
 {
 std::array<char const * const, 5> const kMarketingParameters = {marketing::kFrom, marketing::kType, marketing::kName,
@@ -231,26 +235,18 @@ std::string MakeCampaignPageURL(FeatureID const & featureId)
   ss << kCampaignPageUrl << "/" << featureId.m_mwmId.GetInfo()->GetVersion() << "/"
      << UrlEncode(featureId.m_mwmId.GetInfo()->GetCountryName()) << "/" << featureId.m_index;
 
-  bool isFirstParam = true;
+  url::Params params;
+  params.reserve(kMarketingParameters.size());
   for (auto const & key : kMarketingParameters)
   {
     string value;
     if (!marketing::Settings::Get(key, value))
       continue;
 
-    if (isFirstParam)
-    {
-      ss << "?";
-      isFirstParam = false;
-    }
-    else
-    {
-      ss << "&";
-    }
-    ss << key << "=" << value;
+    params.push_back({key, value});
   }
 
-  return ss.str();
+  return url::Make(ss.str(), params);
 }
 }  // namespace
 
diff --git a/partners_api/booking_api.cpp b/partners_api/booking_api.cpp
index 57945fa4cc..0b26a51682 100644
--- a/partners_api/booking_api.cpp
+++ b/partners_api/booking_api.cpp
@@ -7,6 +7,7 @@
 #include "base/get_time.hpp"
 #include "base/logging.hpp"
 #include "base/thread.hpp"
+#include "base/url_helpers.hpp"
 
 #include <iostream>
 #include <sstream>
@@ -16,6 +17,7 @@
 
 #include "private.h"
 
+using namespace base;
 using namespace booking;
 using namespace booking::http;
 using namespace std;
@@ -31,20 +33,20 @@ string const kPhotoSmallUrl = "http://aff.bstatic.com/images/hotel/max300/";
 string const kSearchBaseUrl = "https://www.booking.com/search.html";
 string g_BookingUrlForTesting = "";
 
-string MakeApiUrlV1(string const & func, vector<pair<string, string>> const & params)
+string MakeApiUrlV1(string const & func, url::Params const & params)
 {
   if (!g_BookingUrlForTesting.empty())
-    return MakeApiUrl(g_BookingUrlForTesting, "." + func, params);
+    return url::Make(g_BookingUrlForTesting + "." + func, params);
 
-  return MakeApiUrl(kBookingApiBaseUrlV1, "." + func, params);
+  return url::Make(kBookingApiBaseUrlV1 + "." + func, params);
 }
 
-string MakeApiUrlV2(string const & func, vector<pair<string, string>> const & params)
+string MakeApiUrlV2(string const & func, url::Params const & params)
 {
   if (!g_BookingUrlForTesting.empty())
-    return MakeApiUrl(g_BookingUrlForTesting, "/" + func, params);
+    return url::Make(g_BookingUrlForTesting + "/" + func, params);
 
-  return MakeApiUrl(kBookingApiBaseUrlV2, "/" + func, params);
+  return url::Make(kBookingApiBaseUrlV2 + "/" + func, params);
 }
 
 void ClearHotelInfo(HotelInfo & info)
@@ -218,8 +220,6 @@ void FillHotelIds(string const & src, vector<uint64_t> & result)
   auto const resultsArray = json_object_get(root.get(), "result");
 
   auto const size = json_array_size(resultsArray);
-  if (size == 0)
-    return;
 
   result.resize(size);
   for (size_t i = 0; i < size; ++i)
diff --git a/partners_api/booking_api.hpp b/partners_api/booking_api.hpp
index e95fa871d4..5940952b98 100644
--- a/partners_api/booking_api.hpp
+++ b/partners_api/booking_api.hpp
@@ -69,7 +69,8 @@ public:
   std::string GetDescriptionUrl(std::string const & baseUrl) const;
   std::string GetHotelReviewsUrl(std::string const & hotelId, std::string const & baseUrl) const;
   std::string GetSearchUrl(std::string const & city, std::string const & name) const;
-  /// Real-time information methods (used for retriving rapidly changing information).
+
+  /// Real-time information methods (used for retrieving rapidly changing information).
   /// These methods send requests directly to Booking.
   void GetMinPrice(std::string const & hotelId, std::string const & currency, GetMinPriceCallback const & fn);
 
diff --git a/partners_api/booking_http.cpp b/partners_api/booking_http.cpp
index c33d07dc89..8a4a6c0d4d 100644
--- a/partners_api/booking_http.cpp
+++ b/partners_api/booking_http.cpp
@@ -9,6 +9,7 @@
 
 #include "private.h"
 
+using namespace base::url;
 using namespace platform;
 using namespace std;
 using namespace std::chrono;
@@ -24,44 +25,14 @@ bool RunSimpleHttpRequest(bool const needAuth, string const & url, string & resu
   if (needAuth)
     request.SetUserAndPassword(BOOKING_KEY, BOOKING_SECRET);
 
-  if (request.RunHttpRequest() && !request.WasRedirected() && request.ErrorCode() == 200)
-  {
-    result = request.ServerResponse();
-    return true;
-  }
-
-  return false;
+  return request.RunHttpRequest(result);
 }
 
-string MakeApiUrl(string const & baseUrl, string const & func, Params const & params)
-{
-  ASSERT_NOT_EQUAL(params.size(), 0, ());
-
-  ostringstream os;
-  os << baseUrl << func << "?";
-
-  bool firstParam = true;
-  for (auto const & param : params)
-  {
-    if (firstParam)
-    {
-      firstParam = false;
-    }
-    else
-    {
-      os << "&";
-    }
-    os << param.first << "=" << param.second;
-  }
-
-  return os.str();
-}
-
-std::string FormatTime(Time p)
+string FormatTime(Time p)
 {
   time_t t = duration_cast<seconds>(p.time_since_epoch()).count();
   ostringstream os;
-  os << put_time(std::gmtime(&t), "%Y-%m-%d");
+  os << put_time(gmtime(&t), "%Y-%m-%d");
   return os.str();
 }
 
@@ -77,12 +48,12 @@ Params AvailabilityParams::Get() const
     result.push_back({"room" + to_string(i + 1), m_rooms[i]});
 
   if (m_minReviewScore != 0.0)
-    result.push_back({"min_review_score", std::to_string(m_minReviewScore)});
+    result.push_back({"min_review_score", to_string(m_minReviewScore)});
 
   if (!m_stars.empty())
     result.push_back({"stars", strings::JoinStrings(m_stars, ',')});
 
   return result;
 }
-}
-}
+}  // namespace http
+}  // namespace booking
diff --git a/partners_api/booking_http.hpp b/partners_api/booking_http.hpp
index 0941c8fa47..d7eaa556a0 100644
--- a/partners_api/booking_http.hpp
+++ b/partners_api/booking_http.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include "base/url_helpers.hpp"
+
 #include <chrono>
 #include <string>
 #include <vector>
@@ -11,19 +13,17 @@ namespace http
 bool RunSimpleHttpRequest(bool const needAuth, std::string const & url, std::string & result);
 
 using Time = std::chrono::system_clock::time_point;
-using Params = std::vector<std::pair<std::string, std::string>>;
 using Hotels = std::vector<std::string>;
 using Rooms = std::vector<std::string>;
 using Stars = std::vector<std::string>;
 
-std::string MakeApiUrl(std::string const & baseUrl, std::string const & func, Params const & params);
 std::string FormatTime(Time p);
 
 /// Params for checking availability of hotels.
 /// [m_hotelIds], [m_checkin], [m_checkout], [m_rooms] are required.
 struct AvailabilityParams
 {
-  Params Get() const;
+  base::url::Params Get() const;
 
   /// Limit the result list to the specified hotels where they have availability for the
   /// specified guests and dates.
@@ -33,7 +33,7 @@ struct AvailabilityParams
   /// The departure date. Must be later than [m_checkin]. Must be between 1 and 30 days after
   /// [m_checkin]. Must be within 360 days in the future and in the format yyyy-mm-dd.
   Time m_checkout;
-  /// Each room is s comma separated array of guests to stay in this room where "A" represents an
+  /// Each room is comma-separated array of guests to stay in this room where "A" represents an
   /// adult and an integer represents a child. eg room1=A,A,4 would be a room with 2 adults and 1
   /// four year-old child. Child age numbers are 0..17.
   Rooms m_rooms;
diff --git a/partners_api/partners_api_tests/booking_tests.cpp b/partners_api/partners_api_tests/booking_tests.cpp
index e09b444e6c..43cc4d8bc9 100644
--- a/partners_api/partners_api_tests/booking_tests.cpp
+++ b/partners_api/partners_api_tests/booking_tests.cpp
@@ -14,6 +14,20 @@ using namespace booking::http;
 
 namespace
 {
+class AsyncGuiThreadBooking : public AsyncGuiThread
+{
+public:
+  AsyncGuiThreadBooking()
+  {
+    SetBookingUrlForTesting("http://localhost:34568/booking/min_price");
+  }
+
+  ~AsyncGuiThreadBooking() override
+  {
+    SetBookingUrlForTesting("");
+  }
+};
+
 UNIT_TEST(Booking_GetHotelAvailability)
 {
   string const kHotelId = "98251";  // Booking hotel id for testing.
@@ -44,11 +58,8 @@ UNIT_TEST(Booking_HotelAvailability)
   LOG(LINFO, (result));
 }
 
-UNIT_CLASS_TEST(AsyncGuiThread, Booking_GetMinPrice)
+UNIT_CLASS_TEST(AsyncGuiThreadBooking, Booking_GetMinPrice)
 {
-  SetBookingUrlForTesting("http://localhost:34568/booking/min_price");
-  MY_SCOPE_GUARD(cleanup, []() { SetBookingUrlForTesting(""); });
-
   string const kHotelId = "0";  // Internal hotel id for testing.
   Api api;
   {
@@ -129,11 +140,8 @@ UNIT_CLASS_TEST(AsyncGuiThread, GetHotelInfo)
   TEST_EQUAL(info.m_reviews.size(), 4, ());
 }
 
-UNIT_CLASS_TEST(AsyncGuiThread, GetHotelAvailability)
+UNIT_CLASS_TEST(AsyncGuiThreadBooking, GetHotelAvailability)
 {
-  SetBookingUrlForTesting("http://localhost:34568/booking/min_price");
-  MY_SCOPE_GUARD(cleanup, []() { SetBookingUrlForTesting(""); });
-
   AvailabilityParams params;
   params.m_hotelIds = {"77615", "10623"};
   params.m_rooms = {"A,A"};
diff --git a/platform/http_client.cpp b/platform/http_client.cpp
index 7688a7438c..a7b3a03641 100644
--- a/platform/http_client.cpp
+++ b/platform/http_client.cpp
@@ -17,6 +17,25 @@ HttpClient::HttpClient(string const & url) : m_urlRequested(url)
 #endif
 }
 
+bool HttpClient::RunHttpRequest(string & response, SuccessChecker checker /* = nullptr */)
+{
+  static auto const simpleChecker = [](HttpClient const & request)
+  {
+    return request.ErrorCode() == 200;
+  };
+
+  if (checker == nullptr)
+    checker = simpleChecker;
+
+  if (RunHttpRequest() && checker(*this))
+  {
+    response = ServerResponse();
+    return true;
+  }
+
+  return false;
+}
+
 HttpClient & HttpClient::SetUrlRequested(string const & url)
 {
   m_urlRequested = url;
diff --git a/platform/http_client.hpp b/platform/http_client.hpp
index d21ef6a880..37f6096d90 100644
--- a/platform/http_client.hpp
+++ b/platform/http_client.hpp
@@ -44,6 +44,11 @@ public:
   // @note Implementations should transparently support all needed HTTP redirects.
   // Implemented for each platform.
   bool RunHttpRequest();
+  using SuccessChecker = std::function<bool(HttpClient const & request)>;
+  // Returns true and copy of server response into [response] in case when RunHttpRequest() and
+  // [checker] return true. When [checker] is equal to nullptr then default checker will be used.
+  // Check by default: ErrorCode() == 200
+  bool RunHttpRequest(string & response, SuccessChecker checker = nullptr);
 
   // Shared methods for all platforms, implemented at http_client.cpp
   HttpClient & SetDebugMode(bool debug_mode);
diff --git a/xcode/base/base.xcodeproj/project.pbxproj b/xcode/base/base.xcodeproj/project.pbxproj
index db1e82cbff..3b58f82080 100644
--- a/xcode/base/base.xcodeproj/project.pbxproj
+++ b/xcode/base/base.xcodeproj/project.pbxproj
@@ -44,6 +44,8 @@
 		39FD27381CC65AD000AFF551 /* timegm_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 39FD26E21CC65A0E00AFF551 /* timegm_test.cpp */; };
 		39FD27391CC65AD000AFF551 /* timer_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 39FD26E31CC65A0E00AFF551 /* timer_test.cpp */; };
 		39FD273B1CC65B1000AFF551 /* libbase.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 675341771A3F57BF00A0A8C3 /* libbase.a */; };
+		3D3731FE1F9A445500D2121B /* url_helpers.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D3731FC1F9A445400D2121B /* url_helpers.cpp */; };
+		3D3731FF1F9A445500D2121B /* url_helpers.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D3731FD1F9A445500D2121B /* url_helpers.hpp */; };
 		3D74EF101F8B902C0081202C /* move_to_front.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D74EF0A1F8B902A0081202C /* move_to_front.hpp */; };
 		3D74EF111F8B902C0081202C /* suffix_array.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3D74EF0B1F8B902B0081202C /* suffix_array.cpp */; };
 		3D74EF121F8B902C0081202C /* visitor.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3D74EF0C1F8B902B0081202C /* visitor.hpp */; };
@@ -170,6 +172,8 @@
 		39FD273D1CC65B1000AFF551 /* libplatform.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libplatform.a; path = "../../../omim-xcode-build/Debug/libplatform.a"; sourceTree = "<group>"; };
 		39FD27401CC65B2800AFF551 /* libindexer.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libindexer.a; path = "../../../omim-xcode-build/Debug/libindexer.a"; sourceTree = "<group>"; };
 		39FD27421CC65B4800AFF551 /* libcoding.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libcoding.a; path = "../../../omim-xcode-build/Debug/libcoding.a"; sourceTree = "<group>"; };
+		3D3731FC1F9A445400D2121B /* url_helpers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = url_helpers.cpp; sourceTree = "<group>"; };
+		3D3731FD1F9A445500D2121B /* url_helpers.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = url_helpers.hpp; sourceTree = "<group>"; };
 		3D74EF0A1F8B902A0081202C /* move_to_front.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = move_to_front.hpp; sourceTree = "<group>"; };
 		3D74EF0B1F8B902B0081202C /* suffix_array.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = suffix_array.cpp; sourceTree = "<group>"; };
 		3D74EF0C1F8B902B0081202C /* visitor.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = visitor.hpp; sourceTree = "<group>"; };
@@ -337,6 +341,8 @@
 		675341791A3F57BF00A0A8C3 /* base */ = {
 			isa = PBXGroup;
 			children = (
+				3D3731FC1F9A445400D2121B /* url_helpers.cpp */,
+				3D3731FD1F9A445500D2121B /* url_helpers.hpp */,
 				3D74EF0E1F8B902B0081202C /* bwt.cpp */,
 				3D74EF0F1F8B902C0081202C /* bwt.hpp */,
 				3D74EF0D1F8B902B0081202C /* move_to_front.cpp */,
@@ -478,6 +484,7 @@
 				675342001A3F57E400A0A8C3 /* string_format.hpp in Headers */,
 				675341F41A3F57E400A0A8C3 /* scope_guard.hpp in Headers */,
 				672DD4BF1E0425600078E13C /* collection_cast.hpp in Headers */,
+				3D3731FF1F9A445500D2121B /* url_helpers.hpp in Headers */,
 				675342071A3F57E400A0A8C3 /* thread_pool.hpp in Headers */,
 				3446C6711DDCA96300146687 /* dfa_helpers.hpp in Headers */,
 				6753420C1A3F57E400A0A8C3 /* threaded_list.hpp in Headers */,
@@ -652,6 +659,7 @@
 				675341DA1A3F57E400A0A8C3 /* exception.cpp in Sources */,
 				3D74EF111F8B902C0081202C /* suffix_array.cpp in Sources */,
 				F6F8E3C81EF846CE00F2DE8F /* worker_thread.cpp in Sources */,
+				3D3731FE1F9A445500D2121B /* url_helpers.cpp in Sources */,
 				675341F91A3F57E400A0A8C3 /* src_point.cpp in Sources */,
 				675342031A3F57E400A0A8C3 /* strings_bundle.cpp in Sources */,
 				3D74EF141F8B902C0081202C /* bwt.cpp in Sources */,