review fixes

This commit is contained in:
Arsentiy Milchakov 2017-06-26 18:44:24 +03:00 committed by Yuri Gorshenin
parent 9613778802
commit 5dc1a2f7b6
10 changed files with 78 additions and 54 deletions

View file

@ -545,8 +545,8 @@ void Framework::EnableDownloadOn3g()
}
uint64_t Framework::RequestTaxiProducts(JNIEnv * env, jobject policy, ms::LatLon const & from,
ms::LatLon const & to,
taxi::SuccessfulCallback const & callback,
taxi::ErrorCallback const & errorCallback)
taxi::SuccessCallback const & onSuccess,
taxi::ErrorCallback const & onError)
{
auto const taxiEngine = m_work.GetTaxiEngine(ToNativeNetworkPolicy(env, policy));
if (!taxiEngine)

View file

@ -183,8 +183,8 @@ namespace android
void EnableDownloadOn3g();
uint64_t RequestTaxiProducts(JNIEnv * env, jobject policy, ms::LatLon const & from,
ms::LatLon const & to, taxi::SuccessfulCallback const & callback,
taxi::ErrorCallback const & errorCallback);
ms::LatLon const & to, taxi::SuccessCallback const & onSuccess,
taxi::ErrorCallback const & onError);
taxi::RideRequestLinks GetTaxiLinks(JNIEnv * env, jobject policy, std::string const & productId,
ms::LatLon const & from, ms::LatLon const & to);

View file

@ -903,8 +903,8 @@ public class RoutingController
mUberPlanning = true;
Uber.nativeRequestUberProducts(NetworkPolicy.newInstance(true), mStartPoint.getLat(),
mStartPoint.getLon(),
Uber.nativeRequestUberProducts(NetworkPolicy.newInstance(true /* canUse */),
mStartPoint.getLat(), mStartPoint.getLon(),
mEndPoint.getLat(), mEndPoint.getLon());
if (mContainer != null)
mContainer.updateBuildProgress(0, mLastRouterType);
@ -916,8 +916,9 @@ public class RoutingController
if (mStartPoint == null || mEndPoint == null)
return null;
return Uber.nativeGetUberLinks(NetworkPolicy.newInstance(true), productId, mStartPoint.getLat(),
mStartPoint.getLon(), mEndPoint.getLat(), mEndPoint.getLon());
return Uber.nativeGetUberLinks(NetworkPolicy.newInstance(true /* canUse */), productId,
mStartPoint.getLat(), mStartPoint.getLon(), mEndPoint.getLat(),
mEndPoint.getLon());
}
/**

View file

@ -1,7 +1,7 @@
namespace taxi
{
struct Product;
} // namespace taxi;
}
@interface MWMTaxiPreviewCell : UICollectionViewCell

View file

@ -487,6 +487,7 @@ Framework::~Framework()
booking::Api * Framework::GetBookingApi(platform::NetworkPolicy const & policy)
{
ASSERT(m_bookingApi, ());
if (policy.CanUse())
return m_bookingApi.get();
@ -495,6 +496,7 @@ booking::Api * Framework::GetBookingApi(platform::NetworkPolicy const & policy)
booking::Api const * Framework::GetBookingApi(platform::NetworkPolicy const & policy) const
{
ASSERT(m_bookingApi, ());
if (policy.CanUse())
return m_bookingApi.get();
@ -503,6 +505,7 @@ booking::Api const * Framework::GetBookingApi(platform::NetworkPolicy const & po
taxi::Engine * Framework::GetTaxiEngine(platform::NetworkPolicy const & policy)
{
ASSERT(m_taxiEngine, ());
if (policy.CanUse())
return m_taxiEngine.get();
@ -511,6 +514,7 @@ taxi::Engine * Framework::GetTaxiEngine(platform::NetworkPolicy const & policy)
viator::Api * Framework::GetViatorApi(platform::NetworkPolicy const & policy)
{
ASSERT(m_viatorApi, ());
if (policy.CanUse())
return m_viatorApi.get();

View file

@ -57,40 +57,57 @@ taxi::ProvidersContainer GetProvidersSynchronous(ms::LatLon const & from, ms::La
return providers;
}
void CompareProviders(taxi::ProvidersContainer const & providersContainer,
taxi::ProvidersContainer const & synchronousProviders)
bool CompareProviders(taxi::ProvidersContainer const & lhs, taxi::ProvidersContainer const & rhs)
{
TEST_EQUAL(synchronousProviders.size(), providersContainer.size(), ());
TEST_EQUAL(rhs.size(), lhs.size(), ());
for (auto const & sp : synchronousProviders)
auto const Match = [](taxi::Provider const & lhs, taxi::Provider const & rhs) -> bool {
if (lhs.GetType() != rhs.GetType())
return false;
auto const & lps = lhs.GetProducts();
auto const & rps = rhs.GetProducts();
TEST_EQUAL(lps.size(), rps.size(), ());
for (auto const & lp : lps)
{
auto const it = std::find_if(rps.cbegin(), rps.cend(), [&lp](taxi::Product const & rp) {
return lp.m_productId == rp.m_productId && lp.m_name == rp.m_name &&
lp.m_price == rp.m_price;
});
if (it == rps.cend())
return false;
}
return true;
};
auto const m = lhs.size();
vector<bool> used(m);
// TODO (@y) Change it to algorithm, based on bipartite graphs.
for (auto const & rItem : rhs)
{
auto const it = std::find_if(
providersContainer.cbegin(), providersContainer.cend(), [&sp](taxi::Provider const & p) {
if (p.GetType() != sp.GetType())
return false;
bool isMatched = false;
for (size_t i = 0; i < m; ++i)
{
if (used[i])
continue;
auto const & spp = sp.GetProducts();
auto const & pp = p.GetProducts();
if (Match(rItem, lhs[i]))
{
used[i] = true;
isMatched = true;
break;
}
}
TEST_EQUAL(spp.size(), pp.size(), ());
for (auto const & sprod : spp)
{
auto const prodIt =
std::find_if(pp.cbegin(), pp.cend(), [&sprod](taxi::Product const & prod) {
return sprod.m_productId == prod.m_productId && sprod.m_name == prod.m_name &&
sprod.m_price == prod.m_price;
});
if (prodIt == pp.cend())
return false;
}
return true;
});
TEST(it != providersContainer.cend(), ());
if (!isMatched)
return false;
}
return true;
}
UNIT_TEST(TaxiEngine_ResultMaker)
@ -222,8 +239,8 @@ UNIT_TEST(TaxiEngine_Smoke)
taxi::ProvidersContainer const synchronousProviders = GetProvidersSynchronous(from, to, kTesturl);
{
taxi::Engine engine({{taxi::Provider::Type::Uber, kTesturl},
{taxi::Provider::Type::Yandex, kTesturl}});
taxi::Engine engine(
{{taxi::Provider::Type::Uber, kTesturl}, {taxi::Provider::Type::Yandex, kTesturl}});
{
lock_guard<mutex> lock(resultsMutex);
reqId = engine.GetAvailableProducts(
@ -251,6 +268,6 @@ UNIT_TEST(TaxiEngine_Smoke)
testing::RunEventLoop();
CompareProviders(providersContainer, synchronousProviders);
TEST(CompareProviders(providersContainer, synchronousProviders), ());
}
} // namespace

View file

@ -192,7 +192,7 @@ UNIT_TEST(Uber_GetRideRequestLinks)
ms::LatLon const from(55.796918, 37.537859);
ms::LatLon const to(55.758213, 37.616093);
auto const links = api.GetRideRequestLinks("", from, to);
auto const links = api.GetRideRequestLinks("" /* productId */, from, to);
TEST(!links.m_deepLink.empty(), ());
TEST(!links.m_universalLink.empty(), ());

View file

@ -24,7 +24,7 @@ namespace taxi
{
// ResultMaker ------------------------------------------------------------------------------------
void ResultMaker::Reset(uint64_t requestId, size_t requestsCount,
SuccessfulCallback const & successCallback,
SuccessCallback const & successCallback,
ErrorCallback const & errorCallback)
{
ASSERT(successCallback, ());
@ -77,7 +77,7 @@ void ResultMaker::ProcessError(uint64_t requestId, Provider::Type type, ErrorCod
void ResultMaker::MakeResult(uint64_t requestId) const
{
SuccessfulCallback successCallback;
SuccessCallback successCallback;
ErrorCallback errorCallback;
ProvidersContainer providers;
ErrorsContainer errors;
@ -119,7 +119,7 @@ Engine::Engine(std::vector<ProviderUrl> urls /* = {} */)
/// Requests list of available products. Returns request identificator immediately.
uint64_t Engine::GetAvailableProducts(ms::LatLon const & from, ms::LatLon const & to,
storage::TCountriesVec const & countryIds,
SuccessfulCallback const & successFn,
SuccessCallback const & successFn,
ErrorCallback const & errorFn)
{
ASSERT(successFn, ());
@ -133,7 +133,7 @@ uint64_t Engine::GetAvailableProducts(ms::LatLon const & from, ms::LatLon const
{
auto type = api.m_type;
if (IsAllCountriesDisabled(type, countryIds) || !IsAnyCountryEnabled(type, countryIds))
if (AreAllCountriesDisabled(type, countryIds) || !IsAnyCountryEnabled(type, countryIds))
{
maker->DecrementRequestCount(reqId);
maker->MakeResult(reqId);
@ -169,8 +169,8 @@ RideRequestLinks Engine::GetRideRequestLinks(Provider::Type type, std::string co
return it->m_api->GetRideRequestLinks(productId, from, to);
}
bool Engine::IsAllCountriesDisabled(Provider::Type type,
storage::TCountriesVec const & countryIds) const
bool Engine::AreAllCountriesDisabled(Provider::Type type,
storage::TCountriesVec const & countryIds) const
{
auto const it =
FindByProviderType(type, m_disabledCountries.cbegin(), m_disabledCountries.cend());

View file

@ -11,7 +11,7 @@
namespace taxi
{
using SuccessfulCallback =
using SuccessCallback =
std::function<void(ProvidersContainer const & products, uint64_t const requestId)>;
using ErrorCallback = std::function<void(ErrorsContainer const & errors, uint64_t const requestId)>;
@ -22,7 +22,7 @@ using ErrorCallback = std::function<void(ErrorsContainer const & errors, uint64_
class ResultMaker
{
public:
void Reset(uint64_t requestId, size_t requestsCount, SuccessfulCallback const & successCallback,
void Reset(uint64_t requestId, size_t requestsCount, SuccessCallback const & successCallback,
ErrorCallback const & errorCallback);
/// Reduces number of requests outstanding.
void DecrementRequestCount(uint64_t requestId);
@ -39,9 +39,11 @@ private:
mutable std::mutex m_mutex;
uint64_t m_requestId = 0;
SuccessfulCallback m_successCallback;
SuccessCallback m_successCallback;
ErrorCallback m_errorCallback;
// The number of the number of unfinished requests to the taxi providers.
// The maximum possible amount of requests is equal to the number of supported taxi providers.
int8_t m_requestsCount = 0;
ErrorsContainer m_errors;
ProvidersContainer m_providers;
@ -61,15 +63,15 @@ public:
/// Requests list of available products. Returns request identificator immediately.
uint64_t GetAvailableProducts(ms::LatLon const & from, ms::LatLon const & to,
storage::TCountriesVec const & countryIds,
SuccessfulCallback const & successFn,
ErrorCallback const & errorFn);
SuccessCallback const & successFn, ErrorCallback const & errorFn);
/// Returns link which allows you to launch some taxi app.
RideRequestLinks GetRideRequestLinks(Provider::Type type, std::string const & productId,
ms::LatLon const & from, ms::LatLon const & to) const;
private:
bool IsAllCountriesDisabled(Provider::Type type, storage::TCountriesVec const & countryIds) const;
bool AreAllCountriesDisabled(Provider::Type type,
storage::TCountriesVec const & countryIds) const;
bool IsAnyCountryEnabled(Provider::Type type, storage::TCountriesVec const & countryIds) const;
template <typename ApiType>

View file

@ -91,6 +91,6 @@ inline std::string DebugPrint(ErrorCode code)
inline std::string DebugPrint(ProviderError error)
{
return "[" + DebugPrint(error.m_type) + ", " + DebugPrint(error.m_code) + "]";
return "ProviderError [" + DebugPrint(error.m_type) + ", " + DebugPrint(error.m_code) + "]";
}
} // namespace taxi