[booking] review fixes + tests fix.

map/booking_availability_filter.cpp

@@ -67,6 +67,8 @@ void UpdateCache(HotelsMapping<T> const & hotelsMapping, booking::HotelsWithExtr
 {
   using availability::Cache;
 
+  cache.ReserveAdditional(hotelsMapping.size());
+
   for (auto & hotel : hotelsMapping)
   {
     if (hotel.GetStatus() != Cache::HotelStatus::Absent)
@@ -256,6 +258,8 @@ void AvailabilityFilter::ApplyFilter(search::Results const & results,
 void AvailabilityFilter::ApplyFilter(std::vector<FeatureID> const & featureIds,
                                      ParamsRawInternal const & filterParams)
 {
+  ASSERT(std::is_sorted(featureIds.cbegin(), featureIds.cend()), ());
+
   ApplyFilterInternal<FeatureID>(featureIds, filterParams);
 }
 

map/booking_filter_cache.cpp

@@ -18,7 +18,7 @@ bool IsExpired(Cache::Clock::time_point const & timestamp, size_t expiryPeriod)
 template <typename Item>
 bool IsExpired(Item const & item, size_t expiryPeriod)
 {
-  return Cache::Clock::now() > item.m_timestamp + seconds(expiryPeriod);
+  return IsExpired(item.m_timestamp, expiryPeriod);
 }
 
 template <typename MapType>
@@ -77,6 +77,12 @@ Cache::Info Cache::Get(std::string const & hotelId)
   return Info(HotelStatus::Absent);
 }
 
+void Cache::ReserveAdditional(size_t count)
+{
+  ReserveAdditional(m_unavailableHotels, count);
+  ReserveAdditional(m_availableHotels, count);
+}
+
 void Cache::InsertNotReady(std::string const & hotelId)
 {
   ASSERT(m_unavailableHotels.find(hotelId) == m_unavailableHotels.end(), ());
@@ -87,7 +93,7 @@ void Cache::InsertNotReady(std::string const & hotelId)
 
 void Cache::InsertUnavailable(std::string const & hotelId)
 {
-  RemoveOverly();
+  ReserveAdditional(m_unavailableHotels, 1);
 
   m_unavailableHotels[hotelId] = Clock::now();
   m_notReadyHotels.erase(hotelId);
@@ -96,7 +102,7 @@ void Cache::InsertUnavailable(std::string const & hotelId)
 
 void Cache::InsertAvailable(std::string const & hotelId, Extras && extras)
 {
-  RemoveOverly();
+  ReserveAdditional(m_availableHotels, 1);
 
   m_availableHotels[hotelId] = Item(std::move(extras));
   m_notReadyHotels.erase(hotelId);
@@ -122,13 +128,11 @@ void Cache::Clear()
   m_availableHotels.clear();
 }
 
-void Cache::RemoveOverly()
+template <typename Container>
+void Cache::ReserveAdditional(Container & container, size_t additionalCount)
 {
-  if (m_maxCount != 0 && m_unavailableHotels.size() >= m_maxCount)
-    m_unavailableHotels.clear();
-
-  if (m_maxCount != 0 && m_availableHotels.size() >= m_maxCount)
-    m_availableHotels.clear();
+  if (m_maxCount != 0 && (container.size() + additionalCount) > m_maxCount)
+    container.clear();
 }
 
 std::string DebugPrint(Cache::HotelStatus status)

map/booking_filter_cache.hpp

@@ -46,6 +46,10 @@ public:
   Cache(size_t maxCount, size_t expiryPeriodSeconds);
 
   Info Get(std::string const & hotelId);
+  // Removes items from available and unavailable
+  // in case of (current items count + |count|) >= |m_maxCount|.
+  void ReserveAdditional(size_t count);
+
   void InsertNotReady(std::string const & hotelId);
   void InsertUnavailable(std::string const & hotelId);
   void InsertAvailable(std::string const & hotelId, Extras && extras);
@@ -65,9 +69,9 @@ private:
 
   using HotelWithTimestampMap = std::map<std::string, Clock::time_point>;
   using HotelWithExtrasMap = std::map<std::string, Item>;
-  // In case when size >= |m_maxCount| removes items except those who have the status
-  // HotelStatus::NotReady.
-  void RemoveOverly();
+  // Removes items in case size >= |m_maxCount|.
+  template <typename Container>
+  void ReserveAdditional(Container & container, size_t additionalCount);
 
   HotelWithTimestampMap m_notReadyHotels;
   HotelWithTimestampMap m_unavailableHotels;

map/booking_utils.cpp

@@ -2,9 +2,15 @@
 
 #include "map/search_mark.hpp"
 
+#include "search/result.hpp"
+
 #include "indexer/feature_decl.hpp"
 
+#include "base/stl_helpers.hpp"
+
+#include <algorithm>
 #include <utility>
+#include <vector>
 
 namespace booking
 {
@@ -15,13 +21,15 @@ void Sort(search::Results && results, std::vector<Extras> && extras,
 {
   if (!extras.empty())
   {
+    CHECK_EQUAL(results.GetCount(), extras.size(), ());
+
     std::vector<std::pair<FeatureID, booking::Extras>> featuresWithExtras;
     featuresWithExtras.reserve(results.GetCount());
     for (size_t i = 0; i < results.GetCount(); ++i)
       featuresWithExtras.emplace_back(std::move(results[i].GetFeatureID()), std::move(extras[i]));
 
     std::sort(featuresWithExtras.begin(), featuresWithExtras.end(),
-              [](auto const & lhs, auto const & rhs) { return lhs.first < rhs.first; });
+              base::LessBy(&std::pair<FeatureID, booking::Extras>::first));
 
     sortedFeatures.reserve(featuresWithExtras.size());
     sortedExtras.reserve(featuresWithExtras.size());
@@ -33,6 +41,7 @@ void Sort(search::Results && results, std::vector<Extras> && extras,
   }
   else
   {
+    sortedFeatures.reserve(results.GetCount());
     for (auto const & r : results)
       sortedFeatures.push_back(r.GetFeatureID());
 

map/booking_utils.hpp

@@ -2,12 +2,6 @@
 
 #include "map/booking_filter_params.hpp"
 
-#include "search/result.hpp"
-
-#include <vector>
-
-struct FeatureID;
-
 class SearchMarks;
 
 namespace booking

map/search_api.cpp

@@ -387,6 +387,8 @@ void SearchAPI::FilterAllHotelsInViewport(m2::RectD const & viewport,
     return base::ControlFlow::Continue;
   });
 
+  ASSERT(std::is_sorted(featureIds.cbegin(), featureIds.cend()), ());
+
   if (featureIds.size() <= kMaxHotelFeatures)
     m_delegate.FilterHotels(filterTasks, move(featureIds));
 }

partners_api/booking_api.hpp

@@ -134,7 +134,7 @@ using BlockAvailabilityCallback =
     platform::SafeCallback<void(std::string const & hotelId, Blocks const & blocks)>;
 using GetHotelInfoCallback = platform::SafeCallback<void(HotelInfo const & hotelInfo)>;
 // NOTE: this callback will be called on the network thread.
-using GetHotelAvailabilityCallback = std::function<void(HotelsWithExtras hotels)>;
+using GetHotelAvailabilityCallback = std::function<void(HotelsWithExtras && hotels)>;
 
 /// Callbacks will be called in the same order as methods are called.
 class Api

partners_api/partners_api_tests/booking_tests.cpp

@@ -129,9 +129,9 @@ UNIT_CLASS_TEST(AsyncGuiThreadBooking, GetHotelAvailability)
   params.m_checkout = std::chrono::system_clock::now() + std::chrono::hours(24 * 7);
   params.m_stars = {"4"};
   Api api;
-  std::vector<std::string> result;
+  HotelsWithExtras result;
 
-  api.GetHotelAvailability(params, [&result](std::vector<std::string> const & r)
+  api.GetHotelAvailability(params, [&result](HotelsWithExtras && r)
   {
     result = r;
     testing::Notify();
@@ -139,8 +139,8 @@ UNIT_CLASS_TEST(AsyncGuiThreadBooking, GetHotelAvailability)
   testing::Wait();
 
   TEST_EQUAL(result.size(), 3, ());
-  TEST_EQUAL(result[0], "10623", ());
-  TEST_EQUAL(result[1], "10624", ());
-  TEST_EQUAL(result[2], "10625", ());
+  TEST(result.find("10623") != result.cend(), ());
+  TEST(result.find("10624") != result.cend(), ());
+  TEST(result.find("10625") != result.cend(), ());
 }
 }  // namespace