gpesquero/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[booking] filter refactoring. Review fixes

Browse source
This commit is contained in:
Arsentiy Milchakov 2018-05-25 18:03:56 +03:00 • committed by Vlad Mihaylenko
parent acab617963
commit a5d305092d
16 changed files with 114 additions and 111 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
android/jni/com/mapswithme/maps/SearchEngine.cpp
View file

@ -489,17 +489,19 @@ void OnBookmarksSearchResults(search::BookmarksSearchParams::Results const & res
env->CallVoidMethod(g_javaListener, method, jResults.get(), static_cast<jlong>(timestamp));
}
void OnBookingFilterResults(std::shared_ptr<booking::ParamsBase> const & params,
void OnBookingFilterResults(std::shared_ptr<booking::ParamsBase> const & apiParams,
vector<FeatureID> const & featuresSorted)
{
// Ignore obsolete booking filter results.
if (!g_lastBookingFilterParams.Equals(*params))
if (!g_lastBookingFilterParams.Equals(*apiParams))
return;
JNIEnv * env = jni::GetEnv();
jni::TScopedLocalObjectArrayRef jResults(env,
usermark_helper::ToFeatureIdArray(env, featuresSorted));
env->CallVoidMethod(g_javaListener, g_onFilterAvailableHotelsId, jResults.get());
ASSERT(std::is_sorted(featuresSorted.cbegin(), featuresSorted.cend()), ());
JNIEnv * env = jni::GetEnv();
jni::TScopedLocalObjectArrayRef jResults(env,
usermark_helper::ToFeatureIdArray(env, featuresSorted));
env->CallVoidMethod(g_javaListener, g_onFilterAvailableHotelsId, jResults.get());
}
} // namespace
@ -571,8 +573,8 @@ extern "C"
params.m_onResults = bind(&OnResults, _1, _2, timestamp, false, hasPosition, lat, lon);
params.m_hotelsFilter = g_hotelsFilterBuilder.Build(env, hotelsFilter);
g_lastBookingFilterParams = g_bookingAvailabilityParamsBuilder.Build(env, bookingFilterParams);
params.m_bookingFilterParams.m_params =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
params.m_bookingFilterParams.m_apiParams =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
params.m_bookingFilterParams.m_callback = bind(&OnBookingFilterResults, _1, _2);
bool const searchStarted = g_framework->NativeFramework()->SearchEverywhere(params);
@ -590,8 +592,8 @@ extern "C"
vparams.m_inputLocale = jni::ToNativeString(env, lang);
vparams.m_hotelsFilter = g_hotelsFilterBuilder.Build(env, hotelsFilter);
g_lastBookingFilterParams = g_bookingAvailabilityParamsBuilder.Build(env, bookingFilterParams);
vparams.m_bookingFilterParams.m_params =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
vparams.m_bookingFilterParams.m_apiParams =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
vparams.m_bookingFilterParams.m_callback = bind(&OnBookingFilterResults, _1, _2);
// TODO (@alexzatsepin): set up vparams.m_onCompleted here and use
@ -606,8 +608,8 @@ extern "C"
eparams.m_onResults = bind(&OnResults, _1, _2, timestamp, isMapAndTable,
false /* hasPosition */, 0.0 /* lat */, 0.0 /* lon */);
eparams.m_hotelsFilter = vparams.m_hotelsFilter;
eparams.m_bookingFilterParams.m_params =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
eparams.m_bookingFilterParams.m_apiParams =
std::make_shared<booking::AvailabilityParams>(g_lastBookingFilterParams);
eparams.m_bookingFilterParams.m_callback = bind(&OnBookingFilterResults, _1, _2);
if (g_framework->NativeFramework()->SearchEverywhere(eparams))

4
iphone/Maps/Core/Search/MWMSearch.mm
View file

@ -102,10 +102,10 @@ using Observers = NSHashTable<Observer>;
};
m_everywhereParams.m_bookingFilterParams.m_callback =
[self](shared_ptr<booking::ParamsBase> const & params,
[self](shared_ptr<booking::ParamsBase> const & apiParams,
std::vector<FeatureID> const & sortedFeatures) {
auto const & p = self->m_everywhereParams.m_bookingFilterParams;
if (p.m_params->IsEmpty() || !p.m_params->Equals(*params))
if (p.m_apiParams->IsEmpty() || !p.m_apiParams->Equals(*apiParams))
return;
self->m_bookingAvailableFeatureIDs = sortedFeatures;
[self onSearchResultsUpdated];

71
map/booking_availability_filter.cpp
View file

@ -58,29 +58,29 @@ void FillResults(HotelToResults && hotelToResults, std::vector<std::string> cons
{
switch (hotelToResult.m_cacheStatus)
{
case Cache::HotelStatus::Unavailable: continue;
case Cache::HotelStatus::Available:
{
case Cache::HotelStatus::Unavailable: continue;
case Cache::HotelStatus::Available:
{
results.AddResult(std::move(hotelToResult.m_result));
continue;
}
case Cache::HotelStatus::NotReady:
{
auto hotelStatus = cache.Get(hotelToResult.m_hotelId);
CHECK_NOT_EQUAL(hotelStatus, Cache::HotelStatus::Absent, ());
if (hotelStatus == Cache::HotelStatus::Available)
results.AddResult(std::move(hotelToResult.m_result));
continue;
}
case Cache::HotelStatus::NotReady:
{
auto hotelStatus = cache.Get(hotelToResult.m_hotelId);
CHECK_NOT_EQUAL(hotelStatus, Cache::HotelStatus::Absent, ());
if (hotelStatus == Cache::HotelStatus::Available)
results.AddResult(std::move(hotelToResult.m_result));
continue;
}
case Cache::HotelStatus::Absent:
{
if (std::binary_search(hotelIds.cbegin(), hotelIds.cend(), hotelToResult.m_hotelId))
results.AddResult(std::move(hotelToResult.m_result));
continue;
}
case Cache::HotelStatus::Absent:
{
if (std::binary_search(hotelIds.cbegin(), hotelIds.cend(), hotelToResult.m_hotelId))
results.AddResult(std::move(hotelToResult.m_result));
continue;
}
continue;
}
}
}
}
@ -148,24 +148,25 @@ namespace filter
{
AvailabilityFilter::AvailabilityFilter(Delegate const & d) : FilterBase(d) {}
void AvailabilityFilter::ApplyFilter(search::Results const & results, ParamsInternal const & params)
void AvailabilityFilter::ApplyFilter(search::Results const & results,
ParamsInternal const & filterParams)
{
ASSERT(params.m_params, ());
ASSERT(filterParams.m_apiParams, ());
auto const & p = *params.m_params;
auto const & cb = params.m_callback;
auto const & p = *filterParams.m_apiParams;
auto const & cb = filterParams.m_callback;
UpdateParams(p);
m_params.m_hotelIds.clear();
m_apiParams.m_hotelIds.clear();
HotelToResults hotelToResults;
PrepareData(GetDelegate().GetIndex(), results, hotelToResults, *m_cache, m_params);
PrepareData(GetDelegate().GetIndex(), results, hotelToResults, *m_cache, m_apiParams);
if (m_params.m_hotelIds.empty())
if (m_apiParams.m_hotelIds.empty())
{
search::Results result;
FillResults(std::move(hotelToResults), {}, *m_cache, result);
FillResults(std::move(hotelToResults), {} /* hotelIds */, *m_cache, result);
cb(result);
return;
@ -188,21 +189,21 @@ void AvailabilityFilter::ApplyFilter(search::Results const & results, ParamsInte
});
};
GetDelegate().GetApi().GetHotelAvailability(m_params, apiCallback);
GetDelegate().GetApi().GetHotelAvailability(m_apiParams, apiCallback);
m_cache->RemoveOutdated();
}
void AvailabilityFilter::UpdateParams(ParamsBase const & params)
void AvailabilityFilter::UpdateParams(ParamsBase const & apiParams)
{
if (m_params.Equals(params))
if (m_apiParams.Equals(apiParams))
return;
m_params.Set(params);
m_apiParams.Set(apiParams);
m_cache = std::make_shared<availability::Cache>();
}
void AvailabilityFilter::GetFeaturesFromCache(search::Results const & results,
std::vector<FeatureID> & sortedResults)
std::vector<FeatureID> & sortedResults)
{
std::vector<FeatureID> features;
@ -222,8 +223,8 @@ void AvailabilityFilter::GetFeaturesFromCache(search::Results const & results,
{
if (mwmId != featureId.m_mwmId)
{
guard = my::make_unique<Index::FeaturesLoaderGuard>(GetDelegate().GetIndex(),
featureId.m_mwmId);
guard =
my::make_unique<Index::FeaturesLoaderGuard>(GetDelegate().GetIndex(), featureId.m_mwmId);
mwmId = featureId.m_mwmId;
}

6
map/booking_availability_filter.hpp
View file

@ -21,17 +21,17 @@ class AvailabilityFilter : public FilterBase
{
public:
explicit AvailabilityFilter(Delegate const & d);
void ApplyFilter(search::Results const & results, ParamsInternal const & params) override;
void ApplyFilter(search::Results const & results, ParamsInternal const & filterParams) override;
void GetFeaturesFromCache(search::Results const & results,
std::vector<FeatureID> & sortedResults) override;
void UpdateParams(ParamsBase const & params) override;
void UpdateParams(ParamsBase const & apiParams) override;
private:
using CachePtr = std::shared_ptr<availability::Cache>;
CachePtr m_cache = std::make_shared<availability::Cache>();
AvailabilityParams m_params;
AvailabilityParams m_apiParams;
};
} // namespace filter
} // namespace booking

10
map/booking_filter.hpp
View file

@ -35,15 +35,13 @@ public:
explicit FilterBase(Delegate const & d) : m_delegate(d) {}
virtual ~FilterBase() = default;
virtual void ApplyFilter(search::Results const & results, ParamsInternal const & params) = 0;
virtual void ApplyFilter(search::Results const & results,
ParamsInternal const & filterParams) = 0;
virtual void GetFeaturesFromCache(search::Results const & results,
std::vector<FeatureID> & sortedResults) = 0;
virtual void UpdateParams(ParamsBase const & params) = 0;
virtual void UpdateParams(ParamsBase const & apiParams) = 0;
Delegate const & GetDelegate() const
{
return m_delegate;
}
Delegate const & GetDelegate() const { return m_delegate; }
private:
Delegate const & m_delegate;

8
map/booking_filter_params.hpp
View file

@ -29,15 +29,15 @@ template <typename R>
struct ParamsImpl
{
ParamsImpl() = default;
ParamsImpl(std::shared_ptr<ParamsBase> params, R const & cb)
: m_params(move(params))
ParamsImpl(std::shared_ptr<ParamsBase> apiParams, R const & cb)
: m_apiParams(apiParams)
, m_callback(cb)
{
}
bool IsEmpty() const { return !m_params || m_params->IsEmpty(); }
bool IsEmpty() const { return !m_apiParams || m_apiParams->IsEmpty(); }
std::shared_ptr<ParamsBase> m_params;
std::shared_ptr<ParamsBase> m_apiParams;
R m_callback;
};

30
map/booking_filter_processor.cpp
View file

@ -25,31 +25,33 @@ void FilterProcessor::ApplyFilters(search::Results const & results,
// Run provided filters consecutively.
for (size_t i = tasks.size() - 1; i > 0; --i)
{
auto const & cb = tasks[i - 1].m_params.m_callback;
auto const & cb = tasks[i - 1].m_filterParams.m_callback;
tasks[i - 1].m_params.m_callback =
[this, cb, nextTask = std::move(tasks[i])](search::Results const & results) mutable
tasks[i - 1].m_filterParams.m_callback =
[ this, cb, nextTask = std::move(tasks[i]) ](search::Results const & results) mutable
{
cb(results);
// Run the next filter with obtained results from the previous one.
// Post different task on the file thread to increase granularity.
GetPlatform().RunTask(Platform::Thread::File, [this, results, nextTask = std::move(nextTask)]()
{
m_filters.at(nextTask.m_type)->ApplyFilter(results, nextTask.m_params);
m_filters.at(nextTask.m_type)->ApplyFilter(results, nextTask.m_filterParams);
});
};
}
// Run first filter.
m_filters.at(tasks.front().m_type)->ApplyFilter(results, tasks.front().m_params);
m_filters.at(tasks.front().m_type)->ApplyFilter(results, tasks.front().m_filterParams);
});
}
void FilterProcessor::OnParamsUpdated(Type const type, std::shared_ptr<ParamsBase> const & params)
void FilterProcessor::OnParamsUpdated(Type const type,
std::shared_ptr<ParamsBase> const & apiParams)
{
GetPlatform().RunTask(Platform::Thread::File, [this, type, params = std::move(params)]() mutable
{
m_filters.at(type)->UpdateParams(*params);
});
GetPlatform().RunTask(Platform::Thread::File,
[this, type, apiParams = std::move(apiParams)]() mutable
{
m_filters.at(type)->UpdateParams(*apiParams);
});
}
void FilterProcessor::GetFeaturesFromCache(Type const type, search::Results const & results,
@ -57,9 +59,11 @@ void FilterProcessor::GetFeaturesFromCache(Type const type, search::Results cons
{
GetPlatform().RunTask(Platform::Thread::File, [this, type, results, callback]()
{
std::vector<FeatureID> resultSorted;
m_filters.at(type)->GetFeaturesFromCache(results, resultSorted);
callback(resultSorted);
std::vector<FeatureID> featuresSorted;
m_filters.at(type)->GetFeaturesFromCache(results, featuresSorted);
ASSERT(std::is_sorted(featuresSorted.begin(), featuresSorted.end()), ());
callback(featuresSorted);
});
}

9
map/booking_filter_processor.hpp
View file

@ -33,17 +33,18 @@ enum class Type
struct FilterTask
{
FilterTask(Type const type, ParamsInternal && params)
FilterTask(Type const type, ParamsInternal && filterParams)
: m_type(type)
, m_params(std::move(params))
, m_filterParams(std::move(filterParams))
{
}
Type const m_type;
ParamsInternal m_params;
ParamsInternal m_filterParams;
};
using FillSearchMarksCallback = platform::SafeCallback<void(std::vector<FeatureID> availableHotelsSorted)>;
using FillSearchMarksCallback =
platform::SafeCallback<void(std::vector<FeatureID> availableHotelsSorted)>;
class FilterProcessor : public FilterBase::Delegate
{

16
map/framework.cpp
View file

@ -3151,8 +3151,8 @@ void Framework::ShowViewportSearchResults(bool clear, search::Results::ConstIter
FillSearchResultsMarks(clear, results.begin(), results.end(), postProcessing);
};
m_bookingFilterProcessor.GetFeaturesFromCache(booking::filter::Type::Availability,
results, fillCallback);
m_bookingFilterProcessor.GetFeaturesFromCache(booking::filter::Type::Availability, results,
fillCallback);
}
void Framework::ClearViewportSearchResults()
@ -3397,17 +3397,17 @@ ugc::Reviews Framework::FilterUGCReviews(ugc::Reviews const & reviews) const
return result;
}
void Framework::FilterSearchResultsOnBooking(booking::filter::Params const & params,
void Framework::FilterSearchResultsOnBooking(booking::filter::Params const & filterParams,
search::Results const & results, bool inViewport)
{
using namespace booking::filter;
auto const & p = params.m_params;
auto const & cb = params.m_callback;
auto const & apiParams = filterParams.m_apiParams;
auto const & cb = filterParams.m_callback;
ParamsInternal paramsInternal
{
p,
[this, p, cb, inViewport](search::Results const & results)
apiParams,
[this, apiParams, cb, inViewport](search::Results const & results)
{
if (results.GetCount() == 0)
return;
@ -3428,7 +3428,7 @@ void Framework::FilterSearchResultsOnBooking(booking::filter::Params const & par
});
}
cb(p, features);
cb(apiParams, features);
}
};

5
map/framework.hpp
View file

@ -841,9 +841,10 @@ private:
ugc::Reviews FilterUGCReviews(ugc::Reviews const & reviews) const;
public:
void FilterSearchResultsOnBooking(booking::filter::Params const & params,
void FilterSearchResultsOnBooking(booking::filter::Params const & filterParams,
search::Results const & results, bool inViewport) override;
void OnBookingAvailabilityParamsUpdate(std::shared_ptr<booking::ParamsBase> const & params) override;
void OnBookingAvailabilityParamsUpdate(
std::shared_ptr<booking::ParamsBase> const & params) override;
private:
// m_discoveryManager must be bellow m_searchApi, m_viatorApi, m_localsApi

23
map/map_tests/booking_filter_test.cpp
View file

@ -22,19 +22,13 @@ using namespace generator::tests_support;
namespace
{
class TestMwmEnvironment : public indexer::tests_support::TestWithCustomMwms
, public FilterBase::Delegate
class TestMwmEnvironment : public indexer::tests_support::TestWithCustomMwms,
public FilterBase::Delegate
{
public:
Index const & GetIndex() const override
{
return m_index;
}
Index const & GetIndex() const override { return m_index; }
booking::Api const & GetApi() const override
{
return m_api;
}
booking::Api const & GetApi() const override { return m_api; }
protected:
TestMwmEnvironment()
@ -93,16 +87,15 @@ UNIT_CLASS_TEST(TestMwmEnvironment, BookingFilter_AvailabilitySmoke)
expectedResults.AddResult(std::move(copy));
},
rect, scales::GetUpperScale());
ParamsInternal params;
ParamsInternal filterParams;
search::Results filteredResults;
params.m_params = make_shared<booking::AvailabilityParams>();
params.m_callback = [&filteredResults](search::Results const & results)
{
filterParams.m_apiParams = make_shared<booking::AvailabilityParams>();
filterParams.m_callback = [&filteredResults](search::Results const & results) {
filteredResults = results;
testing::Notify();
};
filter.ApplyFilter(results, params);
filter.ApplyFilter(results, filterParams);
testing::Wait();

4
map/search_api.cpp
View file

@ -181,7 +181,7 @@ bool SearchAPI::SearchEverywhere(EverywhereSearchParams const & params)
});
if (m_sponsoredMode == SponsoredMode::Booking)
m_delegate.OnBookingAvailabilityParamsUpdate(params.m_bookingFilterParams.m_params);
m_delegate.OnBookingAvailabilityParamsUpdate(params.m_bookingFilterParams.m_apiParams);
return Search(p, true /* forceSearch */);
}
@ -220,7 +220,7 @@ bool SearchAPI::SearchInViewport(ViewportSearchParams const & params)
});
if (m_sponsoredMode == SponsoredMode::Booking)
m_delegate.OnBookingAvailabilityParamsUpdate(params.m_bookingFilterParams.m_params);
m_delegate.OnBookingAvailabilityParamsUpdate(params.m_bookingFilterParams.m_apiParams);
return Search(p, false /* forceSearch */);
}

7
map/search_api.hpp
View file

@ -1,7 +1,7 @@
#pragma once
#include "map/bookmark.hpp"
#include "map/booking_filter_params.hpp"
#include "map/bookmark.hpp"
#include "search/downloader_search_callback.hpp"
#include "search/engine.hpp"
@ -80,7 +80,10 @@ public:
{
}
virtual void OnBookingAvailabilityParamsUpdate(std::shared_ptr<booking::ParamsBase> const & params) {}
virtual void OnBookingAvailabilityParamsUpdate(
std::shared_ptr<booking::ParamsBase> const & params)
{
}
virtual search::ProductInfo GetProductInfo(search::Result const & result) const { return {}; };
};

3
partners_api/booking_api.hpp
View file

@ -74,7 +74,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;
std::string ApplyAvailabilityParams(std::string const & url, AvailabilityParams const & params) const;
std::string ApplyAvailabilityParams(std::string const & url,
AvailabilityParams const & params) const;
/// Real-time information methods (used for retrieving rapidly changing information).
/// These methods send requests directly to Booking.

2
partners_api/booking_availability_params.hpp
View file

@ -13,7 +13,7 @@ namespace booking
{
/// Params for checking availability of hotels.
/// [m_hotelIds], [m_checkin], [m_checkout], [m_rooms] are required.
struct AvailabilityParams : ParamsBase
struct AvailabilityParams : public ParamsBase
{
struct Room
{

1
partners_api/booking_params_base.hpp
View file

@ -3,7 +3,6 @@
#include "base/macros.hpp"
#include <exception>
#include <type_traits>
namespace booking
{