Review fixes.

4 files changed, 73 insertions, 36 deletions

diff --git a/traffic/speed_groups.cpp b/traffic/speed_groups.cpp
index d32abde9c2..926be45bfd 100644
--- a/traffic/speed_groups.cpp
+++ b/traffic/speed_groups.cpp
@@ -11,11 +11,10 @@ uint32_t const kSpeedGroupThresholdPercentage[] = {8, 16, 33, 58, 83, 100, 100,
 SpeedGroup GetSpeedGroupByPercentage(double p)
 {
   p = my::clamp(p, 0.0, 100.0);
-  uint32_t const pu = static_cast<uint32_t>(p);
   SpeedGroup res = SpeedGroup::Unknown;
   for (int i = static_cast<int>(SpeedGroup::Count) - 1; i >= 0; --i)
   {
-    if (pu <= kSpeedGroupThresholdPercentage[i])
+    if (p <= kSpeedGroupThresholdPercentage[i])
       res = static_cast<SpeedGroup>(i);
   }
   return res;
diff --git a/traffic/traffic_info.cpp b/traffic/traffic_info.cpp
index 8383c74858..f09d4bc5a6 100644
--- a/traffic/traffic_info.cpp
+++ b/traffic/traffic_info.cpp
@@ -125,26 +125,19 @@ TrafficInfo TrafficInfo::BuildForTesting(Coloring && coloring)
   return info;
 }
 
+void TrafficInfo::SetTrafficKeysForTesting(vector<RoadSegmentId> const & keys)
+{
+  m_keys = keys;
+  m_availability = Availability::IsAvailable;
+}
+
 bool TrafficInfo::ReceiveTrafficData()
 {
   vector<SpeedGroup> values;
   if (!ReceiveTrafficValues(values))
     return false;
 
-  if (m_keys.size() != values.size())
-  {
-    LOG(LWARNING,
-        ("The number of received traffic values does not correspond to the number of keys:",
-         m_keys.size(), "keys", values.size(), "values."));
-    m_availability = Availability::NoData;
-    m_coloring.clear();
-    return false;
-  }
-
-  for (size_t i = 0; i < m_keys.size(); ++i)
-    m_coloring.emplace(m_keys[i], values[i]);
-
-  return true;
+  return UpdateTrafficData(values);
 }
 
 SpeedGroup TrafficInfo::GetSpeedGroup(RoadSegmentId const & id) const
@@ -195,6 +188,7 @@ void CombineColorings(vector<TrafficInfo::RoadSegmentId> const & keys,
     else
     {
       result[key] = it->second;
+      ASSERT_GREATER(numUnexpectedKeys, 0, ());
       --numUnexpectedKeys;
       ++numKnown;
     }
@@ -249,7 +243,7 @@ void TrafficInfo::SerializeTrafficKeys(vector<RoadSegmentId> const & keys, vecto
     uint32_t prevFid = 0;
     for (auto const & fid : fids)
     {
-      uint32_t const fidDiff = fid - prevFid;
+      uint64_t const fidDiff = static_cast<uint64_t>(fid - prevFid);
       bool ok = coding::GammaCoder::Encode(bitWriter, fidDiff + 1);
       ASSERT(ok, ());
       prevFid = fid;
@@ -262,7 +256,7 @@ void TrafficInfo::SerializeTrafficKeys(vector<RoadSegmentId> const & keys, vecto
     }
 
     for (auto const & val : oneWay)
-      bitWriter.Write(val ? 1 : 0, 1);
+      bitWriter.Write(val ? 1 : 0, 1 /* numBits */);
   }
 }
 
@@ -324,10 +318,13 @@ void TrafficInfo::SerializeTrafficValues(vector<SpeedGroup> const & values,
   WriteVarUint(memWriter, values.size());
   {
     BitWriter<decltype(memWriter)> bitWriter(memWriter);
+    auto const numSpeedGroups = static_cast<uint8_t>(SpeedGroup::Count);
+    static_assert(numSpeedGroups <= 8, "A speed group's value may not fit into 3 bits");
     for (auto const & v : values)
     {
-      // SpeedGroup's values fit into 3 bits.
-      bitWriter.Write(static_cast<uint8_t>(v), 3);
+      uint8_t const u = static_cast<uint8_t>(v);
+      CHECK_LESS(u, numSpeedGroups, ());
+      bitWriter.Write(u, 3);
     }
   }
 
@@ -448,6 +445,25 @@ bool TrafficInfo::ReceiveTrafficValues(vector<SpeedGroup> & values)
   return true;
 }
 
+bool TrafficInfo::UpdateTrafficData(vector<SpeedGroup> const & values)
+{
+  m_coloring.clear();
+
+  if (m_keys.size() != values.size())
+  {
+    LOG(LWARNING,
+        ("The number of received traffic values does not correspond to the number of keys:",
+         m_keys.size(), "keys", values.size(), "values."));
+    m_availability = Availability::NoData;
+    return false;
+  }
+
+  for (size_t i = 0; i < m_keys.size(); ++i)
+    m_coloring.emplace(m_keys[i], values[i]);
+
+  return true;
+}
+
 string DebugPrint(TrafficInfo::RoadSegmentId const & id)
 {
   string const dir =
diff --git a/traffic/traffic_info.hpp b/traffic/traffic_info.hpp
index b4bab86a5b..0673076a61 100644
--- a/traffic/traffic_info.hpp
+++ b/traffic/traffic_info.hpp
@@ -72,6 +72,7 @@ public:
   TrafficInfo(MwmSet::MwmId const & mwmId, int64_t currentDataVersion);
 
   static TrafficInfo BuildForTesting(Coloring && coloring);
+  void SetTrafficKeysForTesting(vector<RoadSegmentId> const & keys);
 
   // Fetches the latest traffic data from the server and updates the coloring.
   // Construct the url by passing an MwmId.
@@ -108,6 +109,8 @@ public:
   static void DeserializeTrafficValues(vector<uint8_t> const & data, vector<SpeedGroup> & result);
 
 private:
+  friend void UnitTest_TrafficInfo_UpdateTrafficData();
+
   // todo(@m) A temporary method. Remove it once the keys are added
   // to the generator and the data is regenerated.
   bool ReceiveTrafficKeys();
@@ -118,6 +121,9 @@ private:
   // Otherwise, returns false and does not change m_coloring.
   bool ReceiveTrafficValues(vector<SpeedGroup> & values);
 
+  // Updates the coloring and changes the availability status if needed.
+  bool UpdateTrafficData(vector<SpeedGroup> const & values);
+
   // The mapping from feature segments to speed groups (see speed_groups.hpp).
   Coloring m_coloring;
 
diff --git a/traffic/traffic_tests/traffic_info_test.cpp b/traffic/traffic_tests/traffic_info_test.cpp
index 0d96bf9db1..45f962c65a 100644
--- a/traffic/traffic_tests/traffic_info_test.cpp
+++ b/traffic/traffic_tests/traffic_info_test.cpp
@@ -6,14 +6,11 @@
 #include "platform/local_country_file.hpp"
 #include "platform/platform_tests_support/writable_dir_changer.hpp"
 
-#include "indexer/classificator_loader.hpp"
 #include "indexer/mwm_set.hpp"
 
-#include "geometry/mercator.hpp"
-#include "geometry/rect2d.hpp"
-
 #include "std/algorithm.hpp"
-#include "std/random.hpp"
+#include "std/cstdint.hpp"
+#include "std/vector.hpp"
 
 namespace traffic
 {
@@ -21,15 +18,6 @@ namespace
 {
 string const & kMapTestDir = "traffic-test";
 
-SpeedGroup GetSpeedGroup(TrafficInfo::Coloring const & coloring,
-                         TrafficInfo::RoadSegmentId const & fid)
-{
-  auto const it = coloring.find(fid);
-  if (it == coloring.cend())
-    return SpeedGroup::Unknown;
-  return it->second;
-}
-
 class TestMwmSet : public MwmSet
 {
 protected:
@@ -105,8 +93,8 @@ UNIT_TEST(TrafficInfo_Serialization)
     vector<TrafficInfo::RoadSegmentId> deserializedKeys;
     TrafficInfo::DeserializeTrafficKeys(buf, deserializedKeys);
 
-    ASSERT(is_sorted(keys.begin(), keys.end()), ());
-    ASSERT(is_sorted(deserializedKeys.begin(), deserializedKeys.end()), ());
+    TEST(is_sorted(keys.begin(), keys.end()), ());
+    TEST(is_sorted(deserializedKeys.begin(), deserializedKeys.end()), ());
     TEST_EQUAL(keys, deserializedKeys, ());
   }
 
@@ -119,4 +107,32 @@ UNIT_TEST(TrafficInfo_Serialization)
     TEST_EQUAL(values, deserializedValues, ());
   }
 }
+
+UNIT_TEST(TrafficInfo_UpdateTrafficData)
+{
+  vector<TrafficInfo::RoadSegmentId> const keys = {
+      TrafficInfo::RoadSegmentId(0, 0, 0),
+
+      TrafficInfo::RoadSegmentId(1, 0, 0), TrafficInfo::RoadSegmentId(1, 0, 1),
+  };
+
+  vector<SpeedGroup> const values1 = {
+      SpeedGroup::G1, SpeedGroup::G2, SpeedGroup::G3,
+  };
+
+  vector<SpeedGroup> const values2 = {
+      SpeedGroup::G4, SpeedGroup::G5, SpeedGroup::Unknown,
+  };
+
+  TrafficInfo info;
+  info.SetTrafficKeysForTesting(keys);
+
+  TEST(info.UpdateTrafficData(values1), ());
+  for (size_t i = 0; i < keys.size(); ++i)
+    TEST_EQUAL(info.GetSpeedGroup(keys[i]), values1[i], ());
+
+  TEST(info.UpdateTrafficData(values2), ());
+  for (size_t i = 0; i < keys.size(); ++i)
+    TEST_EQUAL(info.GetSpeedGroup(keys[i]), values2[i], ());
+}
 }  // namespace traffic