review fixes

5 files changed, 175 insertions, 83 deletions

diff --git a/partners_api/partners_api.pro b/partners_api/partners_api.pro
index 966eb00066..e49f090dae 100644
--- a/partners_api/partners_api.pro
+++ b/partners_api/partners_api.pro
@@ -15,4 +15,3 @@ SOURCES += \
 HEADERS += \
     booking_api.hpp \
     uber_api.hpp \
-
diff --git a/partners_api/partners_api_tests/partners_api_tests.pro b/partners_api/partners_api_tests/partners_api_tests.pro
index 45c70aa138..4904006a86 100644
--- a/partners_api/partners_api_tests/partners_api_tests.pro
+++ b/partners_api/partners_api_tests/partners_api_tests.pro
@@ -20,6 +20,10 @@ macx-* {
   LIBS *= "-framework IOKit" "-framework SystemConfiguration"
 }
 
+win*|linux* {
+  QT *= network
+}
+
 SOURCES += \
     $$ROOT_DIR/testing/testingmain.cpp \
     booking_tests.cpp \
diff --git a/partners_api/partners_api_tests/uber_tests.cpp b/partners_api/partners_api_tests/uber_tests.cpp
index 850f1333d7..5da08c22f7 100644
--- a/partners_api/partners_api_tests/uber_tests.cpp
+++ b/partners_api/partners_api_tests/uber_tests.cpp
@@ -4,18 +4,31 @@
 
 #include "geometry/latlon.hpp"
 
+#include "std/algorithm.hpp"
+#include "std/atomic.hpp"
+#include "std/mutex.hpp"
+
 #include "3party/jansson/myjansson.hpp"
 
+namespace
+{
+bool IsComplete(uber::Product const & product)
+{
+  return !product.m_productId.empty() && !product.m_name.empty() && !product.m_time.empty() &&
+         !product.m_price.empty();
+}
+}  // namespace
+
 UNIT_TEST(Uber_GetProducts)
 {
-  ms::LatLon pos(38.897724, -77.036531);
+  ms::LatLon const pos(38.897724, -77.036531);
 
   TEST(!uber::RawApi::GetProducts(pos).empty(), ());
 }
 
 UNIT_TEST(Uber_GetTimes)
 {
-  ms::LatLon pos(38.897724, -77.036531);
+  ms::LatLon const pos(38.897724, -77.036531);
 
   my::Json timeRoot(uber::RawApi::GetEstimatedTime(pos).c_str());
   auto const timesArray = json_object_get(timeRoot.get(), "times");
@@ -40,7 +53,7 @@ UNIT_TEST(Uber_GetTimes)
       TEST(false, (e.Msg()));
     }
 
-    string estimated = strings::to_string(estimatedTime);
+    string const estimated = strings::to_string(estimatedTime);
 
     TEST(!name.empty(), ());
     TEST(!estimated.empty(), ());
@@ -49,8 +62,8 @@ UNIT_TEST(Uber_GetTimes)
 
 UNIT_TEST(Uber_GetPrices)
 {
-  ms::LatLon from(38.897724, -77.036531);
-  ms::LatLon to(38.862416, -76.883316);
+  ms::LatLon const from(38.897724, -77.036531);
+  ms::LatLon const to(38.862416, -76.883316);
 
   my::Json priceRoot(uber::RawApi::GetEstimatedPrice(from, to).c_str());
   auto const pricesArray = json_object_get(priceRoot.get(), "prices");
@@ -95,36 +108,114 @@ UNIT_TEST(Uber_GetPrices)
 UNIT_TEST(Uber_ProductMaker)
 {
   size_t reqId = 1;
-  ms::LatLon from(38.897724, -77.036531);
-  ms::LatLon to(38.862416, -76.883316);
+  ms::LatLon const from(38.897724, -77.036531);
+  ms::LatLon const to(38.862416, -76.883316);
+
+  size_t returnedId = 0;
+  vector<uber::Product> returnedProducts;
 
   uber::ProductMaker maker;
 
   maker.Reset(reqId);
   maker.SetTimes(reqId, uber::RawApi::GetEstimatedTime(from));
   maker.SetPrices(reqId, uber::RawApi::GetEstimatedPrice(from, to));
-  maker.MakeProducts(reqId, [reqId](vector<uber::Product> const & products, size_t const requestId)
+  maker.MakeProducts(reqId, [&returnedId, &returnedProducts]
+                            (vector<uber::Product> const & products, size_t const requestId)
   {
-    TEST(!products.empty(), ());
-    TEST_EQUAL(requestId, reqId, ());
-
-    for (auto const & product : products)
-    {
-      TEST(!product.m_productId.empty() &&
-           !product.m_name.empty() &&
-           !product.m_time.empty() &&
-           !product.m_price.empty(),());
-    }
+    returnedId = requestId;
+    returnedProducts = products;
   });
 
+  TEST(!returnedProducts.empty(), ());
+  TEST_EQUAL(returnedId, reqId, ());
+
+  for (auto const & product : returnedProducts)
+    TEST(IsComplete(product),());
+
   ++reqId;
 
   maker.Reset(reqId);
   maker.SetTimes(reqId, uber::RawApi::GetEstimatedTime(from));
   maker.SetPrices(reqId, uber::RawApi::GetEstimatedPrice(from, to));
 
-  maker.MakeProducts(reqId + 1, [reqId](vector<uber::Product> const & products, size_t const requestId)
+  maker.MakeProducts(reqId + 1, [](vector<uber::Product> const & products, size_t const requestId)
   {
     TEST(false, ());
   });
 }
+
+UNIT_TEST(Uber_Smoke)
+{
+  // Used to synchronize access into GetAvailableProducts callback method.
+  mutex resultsMutex;
+  size_t reqId = 1;
+  vector<uber::Product> productsContainer;
+  ms::LatLon const from(38.897724, -77.036531);
+  ms::LatLon const to(38.862416, -76.883316);
+
+  auto const standardCallback =
+      [&reqId, &productsContainer, &resultsMutex](vector<uber::Product> const & products, size_t const requestId)
+  {
+    lock_guard<mutex> lock(resultsMutex);
+
+    if (reqId == requestId)
+      productsContainer = products;
+  };
+
+  auto const lastCallback =
+      [&standardCallback](vector<uber::Product> const & products, size_t const requestId)
+  {
+    standardCallback(products, requestId);
+    testing::StopEventLoop();
+  };
+
+  uber::ProductMaker maker;
+
+  maker.Reset(reqId);
+  maker.SetTimes(reqId, uber::RawApi::GetEstimatedTime(from));
+  maker.SetPrices(reqId, uber::RawApi::GetEstimatedPrice(from, to));
+  maker.MakeProducts(reqId, standardCallback);
+
+  reqId = 0;
+
+  auto const synchronousProducts = productsContainer;
+  productsContainer.clear();
+
+  uber::Api uberApi;
+
+  {
+    lock_guard<mutex> lock(resultsMutex);
+    reqId = uberApi.GetAvailableProducts(ms::LatLon(55.753960, 37.624513),
+                                         ms::LatLon(55.765866, 37.661270), standardCallback);
+  }
+  {
+    lock_guard<mutex> lock(resultsMutex);
+    reqId = uberApi.GetAvailableProducts(ms::LatLon(59.922445, 30.367201),
+                                         ms::LatLon(59.943675, 30.361123), standardCallback);
+  }
+  {
+    lock_guard<mutex> lock(resultsMutex);
+    reqId = uberApi.GetAvailableProducts(ms::LatLon(52.509621, 13.450067),
+                                         ms::LatLon(52.510811, 13.409490), standardCallback);
+  }
+  {
+    lock_guard<mutex> lock(resultsMutex);
+    reqId = uberApi.GetAvailableProducts(from, to, lastCallback);
+  }
+
+  testing::RunEventLoop();
+
+  TEST_EQUAL(synchronousProducts.size(), productsContainer.size(), ());
+
+  for (auto const & product : synchronousProducts)
+  {
+    auto const it = find_if(
+        productsContainer.begin(), productsContainer.end(), [&product](uber::Product const & item)
+        {
+          return product.m_productId == item.m_productId && product.m_name == item.m_name &&
+                 product.m_price == item.m_price;
+        });
+
+    TEST(it != productsContainer.end(), ());
+  }
+}
diff --git a/partners_api/uber_api.cpp b/partners_api/uber_api.cpp
index 22e1b6388d..668a910742 100644
--- a/partners_api/uber_api.cpp
+++ b/partners_api/uber_api.cpp
@@ -1,16 +1,12 @@
-#include "uber_api.hpp"
+#include "partners_api/uber_api.hpp"
 
 #include "platform/http_client.hpp"
 
 #include "geometry/latlon.hpp"
 
-#include "base/assert.hpp"
 #include "base/logging.hpp"
 #include "base/thread.hpp"
 
-#include "std/chrono.hpp"
-#include "std/future.hpp"
-
 #include "3party/jansson/myjansson.hpp"
 
 #include "private.h"
@@ -29,21 +25,11 @@ string RunSimpleHttpRequest(string const & url)
   return {};
 }
 
-/// Feature should refers to a shared state.
-template <typename T>
-bool WaitForFeature(future<T> const & f, uint32_t waitMillisec, atomic<bool> const & runFlag)
-{
-  future_status status = future_status::deferred;
-  while (runFlag && status != future_status::ready)
-  {
-    status = f.wait_for(milliseconds(waitMillisec));
-  }
-
-  return runFlag;
-}
-
 bool CheckUberAnswer(json_t const * answer)
 {
+  if (answer == nullptr)
+    return false;
+
   // Uber products are not available at this point.
   if (!json_is_array(answer))
     return false;
@@ -54,6 +40,11 @@ bool CheckUberAnswer(json_t const * answer)
   return true;
 }
 
+bool IsIncomplete(uber::Product const & p)
+{
+  return p.m_name.empty() || p.m_productId.empty() || p.m_time.empty() || p.m_price.empty();
+}
+
 void FillProducts(json_t const * time, json_t const * price, vector<uber::Product> & products)
 {
   // Fill data from time.
@@ -66,7 +57,7 @@ void FillProducts(json_t const * time, json_t const * price, vector<uber::Produc
     my::FromJSONObject(item, "display_name", product.m_name);
     my::FromJSONObject(item, "estimate", estimatedTime);
     product.m_time = strings::to_string(estimatedTime);
-    products.push_back(product);
+    products.push_back(move(product));
   }
 
   // Fill data from price.
@@ -77,9 +68,11 @@ void FillProducts(json_t const * time, json_t const * price, vector<uber::Produc
     auto const item = json_array_get(price, i);
 
     my::FromJSONObject(item, "display_name", name);
-    auto const it = find_if(products.begin(), products.end(), [&name](uber::Product const & product){
+    auto const it = find_if(products.begin(), products.end(), [&name](uber::Product const & product)
+    {
       return product.m_name == name;
     });
+
     if (it == products.end())
       continue;
 
@@ -92,10 +85,28 @@ void FillProducts(json_t const * time, json_t const * price, vector<uber::Produc
       it->m_currency = json_string_value(currency);
   }
 
-  products.erase(remove_if(products.begin(), products.end(), [](uber::Product const & p){
-                   return p.m_name.empty() || p.m_productId.empty() || p.m_time.empty() ||
-                          p.m_price.empty();
-                 }), products.end());
+  products.erase(remove_if(products.begin(), products.end(), IsIncomplete), products.end());
+}
+
+void MakeFromJson(char const * times, char const * prices, vector<uber::Product> & products)
+{
+  products.clear();
+  try
+  {
+    my::Json timesRoot(times);
+    my::Json pricesRoot(prices);
+    auto const timesArray = json_object_get(timesRoot.get(), "times");
+    auto const pricesArray = json_object_get(pricesRoot.get(), "prices");
+    if (CheckUberAnswer(timesArray) && CheckUberAnswer(pricesArray))
+    {
+      FillProducts(timesArray, pricesArray, products);
+    }
+  }
+  catch (my::Json::Exception const & e)
+  {
+    LOG(LERROR, (e.Msg()));
+    products.clear();
+  }
 }
 }  // namespace
 
@@ -163,45 +174,25 @@ void ProductMaker::SetPrices(size_t const requestId, string const & prices)
 
 void ProductMaker::MakeProducts(size_t const requestId, ProductsCallback const & fn)
 {
-  lock_guard<mutex> lock(m_mutex);
-
-  if (requestId != m_requestId || !m_times || !m_prices)
-    return;
-
   vector<uber::Product> products;
-
-  if (m_times->empty() || m_prices->empty())
   {
-    LOG(LWARNING, ("Time or price is empty, time:", *m_times, "; price:", *m_prices));
-    fn(products, m_requestId);
-    return;
-  }
+    lock_guard<mutex> lock(m_mutex);
 
-  try
-  {
-    my::Json timesRoot(m_times->c_str());
-    my::Json pricesRoot(m_prices->c_str());
-    auto const timesArray = json_object_get(timesRoot.get(), "times");
-    auto const pricesArray = json_object_get(pricesRoot.get(), "prices");
-    if (CheckUberAnswer(timesArray) && CheckUberAnswer(pricesArray))
-    {
-      FillProducts(timesArray, pricesArray, products);
-    }
-  }
-  catch (my::Json::Exception const & e)
-  {
-    LOG(LERROR, (e.Msg()));
-    products.clear();
-  }
+    if (requestId != m_requestId || !m_times || !m_prices)
+      return;
 
+    if (!m_times->empty() && !m_prices->empty())
+      MakeFromJson(m_times->c_str(), m_prices->c_str(), products);
+    else
+      LOG(LWARNING, ("Time or price is empty, time:", *m_times, "; price:", *m_prices));
+  }
   fn(products, requestId);
 }
 
 size_t Api::GetAvailableProducts(ms::LatLon const & from, ms::LatLon const & to,
-                                ProductsCallback const & fn)
+                                 ProductsCallback const & fn)
 {
-  static size_t requestId = 0;
-  size_t reqId = ++requestId;
+  size_t const reqId = ++m_requestId;
 
   m_maker->Reset(reqId);
 
@@ -221,14 +212,14 @@ size_t Api::GetAvailableProducts(ms::LatLon const & from, ms::LatLon const & to,
 }
 
 // static
-string Api::GetRideRequestLink(string const & productId, ms::LatLon const & from,
-                               ms::LatLon const & to)
+RideRequestLinks Api::GetRideRequestLinks(string const & productId, ms::LatLon const & from,
+                                          ms::LatLon const & to)
 {
   stringstream url;
-  url << "uber://?client_id=" << UBER_CLIENT_ID << "&action=setPickup&product_id=" << productId
+  url << "?client_id=" << UBER_CLIENT_ID << "&action=setPickup&product_id=" << productId
       << "&pickup[latitude]=" << from.lat << "&pickup[longitude]=" << from.lon
       << "&dropoff[latitude]=" << to.lat << "&dropoff[longitude]=" << to.lon;
 
-  return url.str();
+  return {"uber://" + url.str(), "https://m.uber.com/ul" + url.str()};
 }
 }  // namespace uber
diff --git a/partners_api/uber_api.hpp b/partners_api/uber_api.hpp
index faf5fc5292..9aed7dacff 100644
--- a/partners_api/uber_api.hpp
+++ b/partners_api/uber_api.hpp
@@ -10,12 +10,12 @@
 namespace ms
 {
 class LatLon;
-}  // namespace ms
+}
 
 namespace downloader
 {
 class HttpRequest;
-}  // namespace downloader
+}
 
 namespace uber
 {
@@ -62,12 +62,18 @@ public:
   void MakeProducts(size_t const requestId, ProductsCallback const & fn);
 
 private:
-  size_t m_requestId;
+  size_t m_requestId = 0;
   unique_ptr<string> m_times;
   unique_ptr<string> m_prices;
   mutex m_mutex;
 };
 
+struct RideRequestLinks
+{
+  string m_deepLink;
+  string m_universalLink;
+};
+
 class Api
 {
 public:
@@ -76,10 +82,11 @@ public:
                               ProductsCallback const & fn);
 
   /// Returns link which allows you to launch the Uber app.
-  static string GetRideRequestLink(string const & productId, ms::LatLon const & from,
-                                   ms::LatLon const & to);
+  static RideRequestLinks GetRideRequestLinks(string const & productId, ms::LatLon const & from,
+                                              ms::LatLon const & to);
 
 private:
   shared_ptr<ProductMaker> m_maker = make_shared<ProductMaker>();
+  size_t m_requestId = 0;
 };
 }  // namespace uber