Review fixes

This commit is contained in:
VladiMihaylenko 2017-11-20 16:19:34 +03:00 committed by Yuri Gorshenin
parent 941878cb4d
commit 68faef4794
8 changed files with 164 additions and 154 deletions

View file

@ -26,9 +26,9 @@ set(
bookmark.hpp
chart_generator.cpp
chart_generator.hpp
discovery_client_params.hpp
discovery_manager.cpp
discovery_manager.hpp
discovery/discovery_client_params.hpp
discovery/discovery_manager.cpp
discovery/discovery_manager.hpp
displacement_mode_manager.cpp
displacement_mode_manager.hpp
displayed_categories_modifiers.cpp

View file

@ -11,7 +11,7 @@ enum class ItemType
{
Viator,
Attractions,
Cafe,
Cafes,
Hotels,
LocalExperts
};

View file

@ -1,4 +1,4 @@
#include "map/discovery_manager.hpp"
#include "map/discovery/discovery_manager.hpp"
namespace
{
@ -8,7 +8,7 @@ std::string GetQuery(discovery::ItemType const type)
{
case discovery::ItemType::Hotels: return "hotel ";
case discovery::ItemType::Attractions: return "attraction ";
case discovery::ItemType::Cafe: return "cafe ";
case discovery::ItemType::Cafes: return "cafe ";
case discovery::ItemType::LocalExperts:
case discovery::ItemType::Viator: ASSERT(false, ()); return "";
}
@ -17,17 +17,13 @@ std::string GetQuery(discovery::ItemType const type)
namespace discovery
{
Manager::Manager(Index const & index, search::CityFinder * const cityFinder, APIs const & apis)
Manager::Manager(Index const & index, search::CityFinder & cityFinder, APIs const & apis)
: m_index(index)
, m_cityFinder(cityFinder)
, m_searchApi(apis.m_search)
, m_viatorApi(apis.m_viator)
, m_localsApi(apis.m_locals)
{
CHECK(m_cityFinder, ());
CHECK(m_searchApi, ());
CHECK(m_viatorApi, ());
CHECK(m_localsApi, ());
}
// static
@ -45,16 +41,17 @@ search::SearchParams Manager::GetSearchParams(Manager::Params const & params, It
std::string Manager::GetCityViatorId(m2::PointD const & point) const
{
auto const fid = m_cityFinder->GetCityFeatureID(point);
ASSERT_THREAD_CHECKER(m_threadChecker, ());
auto const fid = m_cityFinder.GetCityFeatureID(point);
if (!fid.IsValid())
return string();
return {};
Index::FeaturesLoaderGuard const guard(m_index, fid.m_mwmId);
FeatureType ft;
if (!guard.GetFeatureByIndex(fid.m_index, ft))
{
LOG(LERROR, ("Feature can't be loaded:", fid));
return string();
return {};
}
auto const & metadata = ft.GetMetadata();

View file

@ -0,0 +1,137 @@
#pragma once
#include "search/city_finder.hpp"
#include "map/discovery/discovery_client_params.hpp"
#include "map/search_api.hpp"
#include "partners_api/booking_api.hpp"
#include "partners_api/locals_api.hpp"
#include "partners_api/viator_api.hpp"
#include "platform/platform.hpp"
#include "indexer/index.hpp"
#include "geometry/point2d.hpp"
#include "geometry/rect2d.hpp"
#include "base/thread_checker.hpp"
#include <functional>
#include <string>
#include <vector>
namespace discovery
{
class Manager final
{
public:
struct APIs
{
APIs(SearchAPI & search, viator::Api const & viator, locals::Api & locals)
: m_search(search), m_viator(viator), m_locals(locals)
{
}
SearchAPI & m_search;
viator::Api const & m_viator;
locals::Api & m_locals;
};
struct Params
{
std::string m_curency;
std::string m_lang;
size_t m_itemsCount = 0;
m2::PointD m_viewportCenter;
m2::RectD m_viewport;
ItemTypes m_itemTypes;
};
using ErrorCalback = std::function<void(ItemType const type)>;
Manager(Index const & index, search::CityFinder & cityFinder, APIs const & apis);
template <typename ResultCallback>
void Discover(Params && params, ResultCallback const & onResult, ErrorCalback const & onError)
{
ASSERT_THREAD_CHECKER(m_threadChecker, ());
auto const & types = params.m_itemTypes;
ASSERT(!types.empty(), ("Types must contain at least one element."));
for (auto const type : types)
{
switch (type)
{
case ItemType::Viator:
{
std::string const sponsoredId = GetCityViatorId(params.m_viewportCenter);
if (sponsoredId.empty())
{
onError(type);
break;
}
m_viatorApi.GetTop5Products(
sponsoredId, params.m_curency,
[this, sponsoredId, onResult, type](std::string const & destId,
std::vector<viator::Product> const & products) {
ASSERT_THREAD_CHECKER(m_threadChecker, ());
if (destId == sponsoredId)
onResult(products, type);
});
break;
}
case ItemType::Attractions: // fallthrough
case ItemType::Cafes:
{
auto p = GetSearchParams(params, type);
p.m_onResults = [onResult, type](search::Results const & results) {
if (!results.IsEndMarker())
return;
GetPlatform().RunTask(Platform::Thread::Gui,
[onResult, type, results] { onResult(results, type); });
};
m_searchApi.GetEngine().Search(p);
break;
}
case ItemType::Hotels:
{
ASSERT(false, ("Discovering hotels isn't supported yet."));
break;
}
case ItemType::LocalExperts:
{
auto const latLon = MercatorBounds::ToLatLon(params.m_viewportCenter);
auto constexpr pageNumber = 1;
m_localsApi.GetLocals(
latLon.lat, latLon.lon, params.m_lang, params.m_itemsCount, pageNumber,
[this, onResult, type](uint64_t id, std::vector<locals::LocalExpert> const & locals,
size_t /* pageNumber */, size_t /* countPerPage */,
bool /* hasPreviousPage */, bool /* hasNextPage */) {
ASSERT_THREAD_CHECKER(m_threadChecker, ());
onResult(locals, type);
},
[this, onError, type](uint64_t id, int errorCode, std::string const & errorMessage) {
ASSERT_THREAD_CHECKER(m_threadChecker, ());
onError(type);
});
break;
}
}
}
}
private:
static search::SearchParams GetSearchParams(Params const & params, ItemType const type);
std::string GetCityViatorId(m2::PointD const & point) const;
Index const & m_index;
search::CityFinder & m_cityFinder;
SearchAPI & m_searchApi;
viator::Api const & m_viatorApi;
locals::Api & m_localsApi;
ThreadChecker m_threadChecker;
};
} // namespace discovery

View file

@ -1,125 +0,0 @@
#pragma once
#include "search/city_finder.hpp"
#include "map/discovery_client_params.hpp"
#include "map/search_api.hpp"
#include "partners_api/booking_api.hpp"
#include "partners_api/locals_api.hpp"
#include "partners_api/viator_api.hpp"
#include "indexer/index.hpp"
#include "geometry/point2d.hpp"
#include "geometry/rect2d.hpp"
#include <functional>
#include <string>
#include <vector>
namespace discovery
{
class Manager final
{
public:
struct APIs
{
APIs(SearchAPI * const search, viator::Api * const viator, locals::Api * const locals)
: m_search(search), m_viator(viator), m_locals(locals)
{
}
SearchAPI * const m_search;
viator::Api * const m_viator;
locals::Api * const m_locals;
};
Manager(Index const & index, search::CityFinder * const cityFinder, APIs const & apis);
struct Params
{
std::string m_curency;
std::string m_lang;
size_t m_itemsCount;
m2::PointD m_viewportCenter;
m2::RectD m_viewport;
ItemTypes m_itemTypes;
};
using ErrorCalback = std::function<void(ItemType const type)>;
template <typename ResultCallback>
void Discover(Params && params, ResultCallback const & result, ErrorCalback const & error)
{
auto const & types = params.m_itemTypes;
ASSERT(!types.empty(), ("Types must contain at least one element."));
for (auto const type : types)
{
switch (type)
{
case ItemType::Viator:
{
string const sponsoredId = GetCityViatorId(params.m_viewportCenter);
if (sponsoredId.empty())
{
error(type);
break;
}
m_viatorApi->GetTop5Products(
sponsoredId, params.m_curency,
[sponsoredId, result, type](string const & destId,
std::vector<viator::Product> const & products) {
if (destId == sponsoredId)
result(products, type);
});
break;
}
case ItemType::Attractions:
case ItemType::Cafe:
{
auto p = GetSearchParams(params, type);
p.m_onResults = [result, type](search::Results const & results) {
if (results.IsEndMarker())
result(results, type);
};
m_searchApi->GetEngine().Search(p);
break;
}
case ItemType::Hotels:
{
ASSERT(false, ("Discovering hotels isn't supported yet."));
break;
}
case ItemType::LocalExperts:
{
auto const latLon = MercatorBounds::ToLatLon(params.m_viewportCenter);
auto constexpr pageNumber = 1;
m_localsApi->GetLocals(
latLon.lat, latLon.lon, params.m_lang, params.m_itemsCount, pageNumber,
[result, type](uint64_t id, std::vector<locals::LocalExpert> const & locals,
size_t /* pageNumber */, size_t /* countPerPage */,
bool /* hasPreviousPage */,
bool /* hasNextPage */) { result(locals, type); },
[error, type](uint64_t id, int errorCode, std::string const & errorMessage) {
error(type);
});
break;
}
}
}
}
private:
static search::SearchParams GetSearchParams(Params const & params, ItemType const type);
std::string GetCityViatorId(m2::PointD const & point) const;
private:
Index const & m_index;
search::CityFinder * const m_cityFinder;
SearchAPI * const m_searchApi;
viator::Api * const m_viatorApi;
locals::Api * const m_localsApi;
};
} // namespace discovery

View file

@ -326,11 +326,11 @@ void Framework::Migrate(bool keepDownloaded)
OnDestroyGLContext();
}
m_selectedFeature = FeatureID();
m_discoveryManager.reset();
m_searchAPI.reset();
m_infoGetter.reset();
m_taxiEngine.reset();
m_cityFinder.reset();
m_discoveryManager.reset();
m_ugcApi.reset();
TCountriesVec existedCountries;
GetStorage().DeleteAllLocalMaps(&existedCountries);
@ -1353,9 +1353,9 @@ void Framework::InitDiscoveryManager()
CHECK(m_searchAPI.get(), ("InitDiscoveryManager() must be called after InitSearchApi()"));
CHECK(m_cityFinder.get(), ("InitDiscoveryManager() must be called after InitCityFinder()"));
discovery::Manager::APIs const apis(m_searchAPI.get(), m_viatorApi.get(), m_localsApi.get());
discovery::Manager::APIs const apis(*m_searchAPI.get(), *m_viatorApi.get(), *m_localsApi.get());
m_discoveryManager =
make_unique<discovery::Manager>(m_model.GetIndex(), m_cityFinder.get(), apis);
make_unique<discovery::Manager>(m_model.GetIndex(), *m_cityFinder.get(), apis);
}
void Framework::InitTransliteration()
@ -2533,11 +2533,11 @@ void Framework::EnableChoosePositionMode(bool enable, bool enableBounds, bool ap
discovery::Manager::Params Framework::GetDiscoveryParams(
discovery::ClientParams && clientParams) const
{
auto constexpr rectSide = 2000.0;
auto constexpr kRectSideM = 2000.0;
discovery::Manager::Params p;
auto const currentPosition = GetCurrentPosition();
p.m_viewportCenter = currentPosition ? *currentPosition : GetViewportCenter();
p.m_viewport = MercatorBounds::RectByCenterXYAndSizeInMeters(p.m_viewportCenter, rectSide);
p.m_viewport = MercatorBounds::RectByCenterXYAndSizeInMeters(p.m_viewportCenter, kRectSideM);
p.m_curency = clientParams.m_currency;
p.m_lang = clientParams.m_lang;
p.m_itemsCount = clientParams.m_itemsCount;

View file

@ -4,7 +4,7 @@
#include "map/booking_filter.hpp"
#include "map/bookmark.hpp"
#include "map/bookmark_manager.hpp"
#include "map/discovery_manager.hpp"
#include "map/discovery/discovery_manager.hpp"
#include "map/displacement_mode_manager.hpp"
#include "map/feature_vec_model.hpp"
#include "map/local_ads_manager.hpp"
@ -204,8 +204,6 @@ protected:
LocalAdsManager m_localAdsManager;
unique_ptr<discovery::Manager> m_discoveryManager;
User m_user;
booking::filter::Filter m_bookingFilter;
@ -243,7 +241,6 @@ public:
ugc::Api * GetUGCApi() { return m_ugcApi.get(); }
ugc::Api const * GetUGCApi() const { return m_ugcApi.get(); }
df::DrapeApi & GetDrapeApi() { return m_drapeApi; }
User & GetUser() { return m_user; }
@ -752,11 +749,11 @@ public:
public:
template <typename ResultCallback>
void Discover(discovery::ClientParams && params, ResultCallback const & result,
discovery::Manager::ErrorCalback const & error) const
void Discover(discovery::ClientParams && params, ResultCallback const & onResult,
discovery::Manager::ErrorCalback const & onError) const
{
CHECK(m_discoveryManager.get(), ());
m_discoveryManager->Discover(GetDiscoveryParams(move(params)), result, error);
m_discoveryManager->Discover(GetDiscoveryParams(move(params)), onResult, onError);
}
discovery::Manager::Params GetDiscoveryParams(discovery::ClientParams && clientParams) const;
@ -843,4 +840,8 @@ private:
public:
void FilterSearchResultsOnBooking(booking::filter::availability::Params const & params,
search::Results const & results, bool inViewport) override;
private:
// m_discoveryManager must be bellow m_searchApi, m_viatorApi, m_localsApi
unique_ptr<discovery::Manager> m_discoveryManager;
};

View file

@ -19,8 +19,8 @@ HEADERS += \
bookmark.hpp \
bookmark_manager.hpp \
chart_generator.hpp \
discovery_client_params.hpp \
discovery_manager.hpp \
discovery/discovery_client_params.hpp \
discovery/discovery_manager.hpp \
displacement_mode_manager.hpp \
displayed_categories_modifiers.hpp \
everywhere_search_params.hpp \
@ -61,7 +61,7 @@ SOURCES += \
bookmark.cpp \
bookmark_manager.cpp \
chart_generator.cpp \
discovery_manager.cpp \
discovery/discovery_manager.cpp \
displacement_mode_manager.cpp \
displayed_categories_modifiers.cpp \
feature_vec_model.cpp \