From 808f111a336569c7ff83cac5e257040edf6cd9d9 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Mon, 23 Oct 2017 14:37:28 +0300 Subject: [PATCH] [partners_api] booking review fixes --- base/CMakeLists.txt | 2 + base/base.pro | 2 + base/url_helpers.cpp | 35 +++++++++++++++ base/url_helpers.hpp | 22 ++++++++++ coding/coding.pro | 2 +- map/local_ads_manager.cpp | 20 ++++----- partners_api/booking_api.cpp | 16 +++---- partners_api/booking_api.hpp | 3 +- partners_api/booking_http.cpp | 43 +++---------------- partners_api/booking_http.hpp | 8 ++-- .../partners_api_tests/booking_tests.cpp | 24 +++++++---- platform/http_client.cpp | 19 ++++++++ platform/http_client.hpp | 5 +++ xcode/base/base.xcodeproj/project.pbxproj | 8 ++++ 14 files changed, 139 insertions(+), 70 deletions(-) create mode 100644 base/url_helpers.cpp create mode 100644 base/url_helpers.hpp 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 + +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 +#include +#include + +namespace base +{ +namespace url +{ +struct Param +{ + std::string m_name; + std::string m_value; +}; + +using Params = std::vector; + +// 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 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 #include @@ -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> 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> 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 & 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(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 #include #include @@ -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>; using Hotels = std::vector; using Rooms = std::vector; using Stars = std::vector; -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; + // 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 = ""; }; 39FD27401CC65B2800AFF551 /* libindexer.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libindexer.a; path = "../../../omim-xcode-build/Debug/libindexer.a"; sourceTree = ""; }; 39FD27421CC65B4800AFF551 /* libcoding.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libcoding.a; path = "../../../omim-xcode-build/Debug/libcoding.a"; sourceTree = ""; }; + 3D3731FC1F9A445400D2121B /* url_helpers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = url_helpers.cpp; sourceTree = ""; }; + 3D3731FD1F9A445500D2121B /* url_helpers.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = url_helpers.hpp; sourceTree = ""; }; 3D74EF0A1F8B902A0081202C /* move_to_front.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = move_to_front.hpp; sourceTree = ""; }; 3D74EF0B1F8B902B0081202C /* suffix_array.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = suffix_array.cpp; sourceTree = ""; }; 3D74EF0C1F8B902B0081202C /* visitor.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = visitor.hpp; sourceTree = ""; }; @@ -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 */,