From b2d1f1cd3eb58740c7205c5a9d59eb2d10ad1413 Mon Sep 17 00:00:00 2001
From: Maxim Pimenov <m@maps.me>
Date: Thu, 29 Nov 2018 18:42:02 +0300
Subject: [PATCH] Review fixes.

---
 geocoder/beam.hpp                          |  2 +-
 geocoder/geocoder.cpp                      | 10 ++++++----
 geocoder/geocoder.hpp                      |  9 +++++++--
 geocoder/geocoder_tests/geocoder_tests.cpp | 11 +++++++++++
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/geocoder/beam.hpp b/geocoder/beam.hpp
index d0728889f1..04fbf29cdc 100644
--- a/geocoder/beam.hpp
+++ b/geocoder/beam.hpp
@@ -31,7 +31,7 @@ public:
 
   // O(log(n) + n) for |n| entries.
   // O(|m_capacity|) in the worst case.
-  void Add(Key const & key, Value value)
+  void Add(Key const & key, Value const & value)
   {
     if (PREDICT_FALSE(m_capacity == 0))
       return;
diff --git a/geocoder/geocoder.cpp b/geocoder/geocoder.cpp
index e268eedbd9..112f59c625 100644
--- a/geocoder/geocoder.cpp
+++ b/geocoder/geocoder.cpp
@@ -142,9 +142,10 @@ bool Geocoder::Context::IsTokenUsed(size_t id) const
 
 bool Geocoder::Context::AllTokensUsed() const { return m_numUsedTokens == m_tokens.size(); }
 
-void Geocoder::Context::AddResult(base::GeoObjectId const & osmId, double certainty, Type type)
+void Geocoder::Context::AddResult(base::GeoObjectId const & osmId, double certainty, Type type,
+                                  bool allTokensUsed)
 {
-  m_beam.Add(BeamKey(osmId, type), certainty);
+  m_beam.Add(BeamKey(osmId, type, allTokensUsed), certainty);
 }
 
 void Geocoder::Context::FillResults(vector<Result> & results) const
@@ -158,8 +159,9 @@ void Geocoder::Context::FillResults(vector<Result> & results) const
     if (!seen.insert(e.m_key.m_osmId).second)
       continue;
 
-    if (m_surelyGotHouseNumber && e.m_key.m_type != Type::Building)
+    if (m_surelyGotHouseNumber && e.m_key.m_type != Type::Building && !e.m_key.m_allTokensUsed)
       continue;
+
     results.emplace_back(e.m_key.m_osmId, e.m_value /* certainty */);
   }
 
@@ -247,7 +249,7 @@ void Geocoder::Go(Context & ctx, Type type) const
 
       for (auto const * e : curLayer.m_entries)
       {
-        ctx.AddResult(e->m_osmId, certainty, type);
+        ctx.AddResult(e->m_osmId, certainty, type, ctx.AllTokensUsed());
       }
 
       ctx.GetLayers().emplace_back(move(curLayer));
diff --git a/geocoder/geocoder.hpp b/geocoder/geocoder.hpp
index 0aaa29cf4d..131569a44e 100644
--- a/geocoder/geocoder.hpp
+++ b/geocoder/geocoder.hpp
@@ -50,10 +50,14 @@ public:
   public:
     struct BeamKey
     {
-      BeamKey(base::GeoObjectId osmId, Type type) : m_osmId(osmId), m_type(type) {}
+      BeamKey(base::GeoObjectId osmId, Type type, bool allTokensUsed)
+        : m_osmId(osmId), m_type(type), m_allTokensUsed(allTokensUsed)
+      {
+      }
 
       base::GeoObjectId m_osmId;
       Type m_type;
+      bool m_allTokensUsed;
     };
 
     Context(std::string const & query);
@@ -74,7 +78,8 @@ public:
     // Returns true iff all tokens are used.
     bool AllTokensUsed() const;
 
-    void AddResult(base::GeoObjectId const & osmId, double certainty, Type type);
+    void AddResult(base::GeoObjectId const & osmId, double certainty, Type type,
+                   bool allTokensUsed);
 
     void FillResults(std::vector<Result> & results) const;
 
diff --git a/geocoder/geocoder_tests/geocoder_tests.cpp b/geocoder/geocoder_tests/geocoder_tests.cpp
index 455f9c6279..d193ed2026 100644
--- a/geocoder/geocoder_tests/geocoder_tests.cpp
+++ b/geocoder/geocoder_tests/geocoder_tests.cpp
@@ -93,6 +93,10 @@ UNIT_TEST(Geocoder_OnlyBuildings)
 
 31 {"properties": {"address": {"street": "Bad", "locality": "Some Locality"}}}
 32 {"properties": {"address": {"building": "10", "street": "Bad", "locality": "Some Locality"}}}
+
+40 {"properties": {"address": {"street": "MaybeNumbered", "locality": "Some Locality"}}}
+41 {"properties": {"address": {"street": "MaybeNumbered-3", "locality": "Some Locality"}}}
+42 {"properties": {"address": {"building": "3", "street": "MaybeNumbered", "locality": "Some Locality"}}}
 )#";
 
   ScopedFile const regionsJsonFile("regions.jsonl", kData);
@@ -115,5 +119,12 @@ UNIT_TEST(Geocoder_OnlyBuildings)
   // Another possible resolution would be to return just "Good Street" (relaxed matching)
   // but at the time of writing the goal is to either have an exact match or no match at all.
   TestGeocoder(geocoder, "some locality good 10", {});
+
+  // Sometimes we may still emit a non-building.
+  // In this case it happens because all query tokens are used.
+  base::GeoObjectId const numberedStreet(41);
+  base::GeoObjectId const houseOnANonNumberedStreet(42);
+  TestGeocoder(geocoder, "some locality maybenumbered 3",
+               {{numberedStreet, 1.0}, {houseOnANonNumberedStreet, 0.8875}});
 }
 }  // namespace geocoder