From 31fdc5bc2343a04086e4a265b59ec6400540d9b1 Mon Sep 17 00:00:00 2001
From: Maxim Pimenov <m@maps.me>
Date: Fri, 17 Nov 2017 12:48:26 +0300
Subject: [PATCH] [search] Flattened the call graph of UpdateResults and
 Finish.

Previously, Processor called Emitter directly to set
the end marker, but all other calls to Emitter were made
through the Geocoder-Preranker-Ranker pipeline.

Now all calls must be made through the pipeline, which
at least makes it easier to update the state in case
of cancellation (which is only done in PreRanker's m_currEmit currently).

Also, removed an unneeded call of UpdateResults in Processor:
all UpdateResults calls from Processor must now be made indirectly,
either via the normal search process in Geocoder, or via calling
Finish.
---
 search/geocoder.cpp   |  7 +++++--
 search/geocoder.hpp   | 13 +++++++++++++
 search/pre_ranker.cpp |  8 ++++++++
 search/pre_ranker.hpp |  5 +++--
 search/processor.cpp  |  4 +---
 search/ranker.cpp     | 11 ++++++++++-
 search/ranker.hpp     |  2 ++
 7 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/search/geocoder.cpp b/search/geocoder.cpp
index 5ad19b3c98..4506072ead 100644
--- a/search/geocoder.cpp
+++ b/search/geocoder.cpp
@@ -433,6 +433,11 @@ void Geocoder::GoInViewport()
   GoImpl(infos, true /* inViewport */);
 }
 
+void Geocoder::Finish(bool cancelled)
+{
+  m_preRanker.Finish(cancelled);
+}
+
 void Geocoder::ClearCaches()
 {
   m_pivotRectsCache.Clear();
@@ -568,8 +573,6 @@ void Geocoder::GoImpl(vector<shared_ptr<MwmInfo>> & infos, bool inViewport)
 
     // Iterates through all alive mwms and performs geocoding.
     ForEachCountry(infos, processCountry);
-
-    m_preRanker.UpdateResults(true /* lastUpdate */);
   }
   catch (CancelException & e)
   {
diff --git a/search/geocoder.hpp b/search/geocoder.hpp
index 982037ae3e..dbb77dcd44 100644
--- a/search/geocoder.hpp
+++ b/search/geocoder.hpp
@@ -101,6 +101,19 @@ public:
   void GoEverywhere();
   void GoInViewport();
 
+  // Ends geocoding and informs the following stages
+  // of the pipeline (PreRanker).
+  // This method must be called from the previous stage
+  // of the pipeline (the Processor).
+  // If |cancelled| is true, the reason for calling Finish must
+  // be the cancellation of processing the search request, otherwise
+  // the reason must be the normal exit from GoEverywhere of GoInViewport.
+  //
+  // *NOTE* The caller assumes that a call to this method will never
+  // result in search::CancelException even if the shutdown takes
+  // noticeable time.
+  void Finish(bool cancelled);
+
   void ClearCaches();
 
 private:
diff --git a/search/pre_ranker.cpp b/search/pre_ranker.cpp
index d08b22d3cd..9d234a3331 100644
--- a/search/pre_ranker.cpp
+++ b/search/pre_ranker.cpp
@@ -54,6 +54,14 @@ void PreRanker::Init(Params const & params)
   m_currEmit.clear();
 }
 
+void PreRanker::Finish(bool cancelled)
+{
+  if (!cancelled)
+    UpdateResults(true /* lastUpdate */);
+
+  m_ranker.Finish(cancelled);
+}
+
 void PreRanker::FillMissingFieldsInPreResults()
 {
   MwmSet::MwmId mwmId;
diff --git a/search/pre_ranker.hpp b/search/pre_ranker.hpp
index edcf972d1c..4734e62a0b 100644
--- a/search/pre_ranker.hpp
+++ b/search/pre_ranker.hpp
@@ -54,6 +54,8 @@ public:
 
   void Init(Params const & params);
 
+  void Finish(bool cancelled);
+
   template <typename... TArgs>
   void Emplace(TArgs &&... args)
   {
@@ -68,8 +70,7 @@ public:
   void Filter(bool viewportSearch);
 
   // Emit a new batch of results up the pipeline (i.e. to ranker).
-  // Use lastUpdate in geocoder to indicate that
-  // no more results will be added.
+  // Use |lastUpdate| to indicate that no more results will be added.
   void UpdateResults(bool lastUpdate);
 
   inline size_t Size() const { return m_results.size(); }
diff --git a/search/processor.cpp b/search/processor.cpp
index 4288aa1533..cfa7e50d88 100644
--- a/search/processor.cpp
+++ b/search/processor.cpp
@@ -410,8 +410,6 @@ void Processor::Search(SearchParams const & params)
 
       m_geocoder.GoEverywhere();
     }
-
-    m_ranker.UpdateResults(true /* lastUpdate */);
   }
   catch (CancelException const &)
   {
@@ -422,7 +420,7 @@ void Processor::Search(SearchParams const & params)
     SendStatistics(params, viewport, m_emitter.GetResults());
 
   // Emit finish marker to client.
-  m_emitter.Finish(IsCancelled());
+  m_geocoder.Finish(IsCancelled());
 }
 
 void Processor::SearchCoordinates()
diff --git a/search/ranker.cpp b/search/ranker.cpp
index e99420b16e..8820fe0322 100644
--- a/search/ranker.cpp
+++ b/search/ranker.cpp
@@ -342,6 +342,12 @@ void Ranker::Init(Params const & params, Geocoder::Params const & geocoderParams
   m_tentativeResults.clear();
 }
 
+void Ranker::Finish(bool cancelled)
+{
+  // The results have been updated by PreRanker.
+  m_emitter.Finish(cancelled);
+}
+
 Result Ranker::MakeResult(RankerResult const & rankerResult, bool needAddress,
                           bool needHighlighting) const
 {
@@ -470,7 +476,10 @@ void Ranker::UpdateResults(bool lastUpdate)
   m_preRankerResults.clear();
 
   BailIfCancelled();
-  m_emitter.Emit();
+
+  // The last update must be handled by a call to Finish().
+  if (!lastUpdate)
+    m_emitter.Emit();
 }
 
 void Ranker::ClearCaches() { m_localities.ClearCache(); }
diff --git a/search/ranker.hpp b/search/ranker.hpp
index 82ab379ea8..cb24c4c6da 100644
--- a/search/ranker.hpp
+++ b/search/ranker.hpp
@@ -87,6 +87,8 @@ public:
 
   void Init(Params const & params, Geocoder::Params const & geocoderParams);
 
+  void Finish(bool cancelled);
+
   // Makes the final result that is shown to the user from a ranker's result.
   // |needAddress| and |needHighlighting| enable filling of optional fields
   // that may take a considerable amount of time to compute.