Review fixes

5 files changed, 193 insertions, 344 deletions

diff --git a/ugc/serdes_json.hpp b/ugc/serdes_json.hpp
index 43151efd14..4a88860e4a 100644
--- a/ugc/serdes_json.hpp
+++ b/ugc/serdes_json.hpp
@@ -1,226 +1,180 @@
 #pragma once
 
-#include "ugc/serdes.hpp"
 #include "ugc/types.hpp"
 
-#include "coding/multilang_utf8_string.hpp"
-#include "coding/point_to_integer.hpp"
-#include "coding/reader.hpp"
-#include "coding/varint.hpp"
-#include "coding/write_to_sink.hpp"
-
 #include "base/exception.hpp"
 
 #include "3party/jansson/myjansson.hpp"
 
-#include <cmath>
 #include <cstdint>
-#include <iostream>
+#include <cstdlib>
 
 namespace ugc
 {
-  template <typename Sink>
-  class SerializerJson
+template <typename Sink>
+class SerializerJson
+{
+public:
+  SerializerJson(Sink & sink) : m_sink(sink) {}
+  ~SerializerJson()
   {
-  public:
-    SerializerJson(Sink & sink, HeaderV0 const & header) : m_sink(sink), m_header(header) {}
-
-    void operator()(bool const d, char const * name = nullptr)
-    {
-      ToJSONObject(*m_json, name, d);
-    }
-
-    void operator()(uint8_t const d, char const * name = nullptr)
-    {
-      ToJSONObject(*m_json, name, d);
-    }
-
-    void operator()(uint32_t const d, char const * name = nullptr)
-    {
-      ToJSONObject(*m_json, name, d);
-    }
-
-    void operator()(uint64_t const d, char const * name = nullptr)
-    {
-      ToJSONObject(*m_json, name, d);
-    }
-
-    void operator()(std::string const & s, char const * name = nullptr)
-    {
-      ToJSONObject(*m_json, name, s);
-    }
+    char * str = json_dumps(m_json.get(), 0);
+    std::string json(str);
+    std::free(static_cast<void *>(str));
+    m_sink.Write(json.data(), json.size());
+  }
+  void operator()(bool const d, char const * name = nullptr) { ToJSONObject(*m_json, name, d); }
+  void operator()(uint8_t const d, char const * name = nullptr) { ToJSONObject(*m_json, name, d); }
+  void operator()(uint32_t const d, char const * name = nullptr) { ToJSONObject(*m_json, name, d); }
+  void operator()(uint64_t const d, char const * name = nullptr) { ToJSONObject(*m_json, name, d); }
+  void operator()(std::string const & s, char const * name = nullptr)
+  {
+    ToJSONObject(*m_json, name, s);
+  }
 
-    void operator()(Time const & t, char const * name = nullptr)
-    {
-      (*this)(ToDaysSinceEpoch(t), name);
-    }
+  void operator()(Time const & t, char const * name = nullptr)
+  {
+    (*this)(ToDaysSinceEpoch(t), name);
+  }
 
-    void operator()(Sentiment sentiment, char const * name = nullptr)
+  void operator()(Sentiment sentiment, char const * name = nullptr)
+  {
+    switch (sentiment)
     {
-      switch (sentiment)
-      {
-        case Sentiment::Negative: return (*this)(false, name);
-        case Sentiment::Positive: return (*this)(true, name);
-      }
+    case Sentiment::Negative: return (*this)(false, name);
+    case Sentiment::Positive: return (*this)(true, name);
     }
+  }
 
-    template <typename T>
-    void operator()(vector<T> const & vs, char const * name = nullptr)
-    {
-      my::JSONPtr safe_json = std::move(m_json);
-      m_json = my::NewJSONArray();
+  template <typename T>
+  void operator()(vector<T> const & vs, char const * name = nullptr)
+  {
+    NewScopeWith(my::NewJSONArray(), name, [this, &vs] {
       for (auto const & v : vs)
         (*this)(v);
-      Update(std::move(safe_json), name);
-    }
+    });
+  }
 
-    template <typename R>
-    void operator()(R const & r, char const * name = nullptr)
-    {
-      my::JSONPtr safe_json = std::move(m_json);
-      m_json = my::NewJSONObject();
-      r.Visit(*this);
-      Update(std::move(safe_json), name);
-    }
+  template <typename R>
+  void operator()(R const & r, char const * name = nullptr)
+  {
+    NewScopeWith(my::NewJSONObject(), name, [this, &r] { r.Visit(*this); });
+  }
 
-    void VisitRating(float const f, char const * name = nullptr)
-    {
-      CHECK_GREATER_OR_EQUAL(f, 0.0, ());
-      auto const d = static_cast<double>(f);
-      ToJSONObject(*m_json, name, d);
-    }
+  void VisitRating(float const f, char const * name = nullptr)
+  {
+    CHECK_GREATER_OR_EQUAL(f, 0.0, ());
+    auto const d = static_cast<double>(f);
+    ToJSONObject(*m_json, name, d);
+  }
+
+private:
+  template <typename Fn>
+  void NewScopeWith(my::JSONPtr json_object, char const * name, Fn && fn)
+  {
+    my::JSONPtr safe_json = std::move(m_json);
+    m_json = std::move(json_object);
 
-    void Flush()
-    {
-      std::string json(json_dumps(m_json.get(), 0));
-      m_sink.Write(json.data(), json.size());
-    }
+    fn();
 
-  private:
+    if (safe_json && json_is_array(safe_json))
+      json_array_append_new(safe_json.get(), m_json.release());
+    else if (safe_json && json_is_object(safe_json))
+      json_object_set_new(safe_json.get(), name, m_json.release());
 
-    void Update(my::JSONPtr safe_json, char const * name = nullptr)
-    {
-      if (safe_json && json_is_array(safe_json))
-        json_array_append_new(safe_json.get(), m_json.release());
-      else if (safe_json && json_is_object(safe_json))
-        json_object_set_new(safe_json.get(), name, m_json.release());
+    if (safe_json)
+      m_json = std::move(safe_json);
+  }
 
-      if (safe_json)
-        m_json = std::move(safe_json);
-    }
+  my::JSONPtr m_json = nullptr;
+  Sink & m_sink;
+};
 
-    my::JSONPtr m_json = nullptr;
-    Sink & m_sink;
-    HeaderV0 const m_header;
-  };
+template <typename Source>
+class DeserializerJsonV0
+{
+public:
+  DECLARE_EXCEPTION(Exception, RootException);
 
-  template <typename Source>
-  class DeserializerJsonV0
+  DeserializerJsonV0(Source & source) : m_source(source)
   {
-  public:
-    DECLARE_EXCEPTION(Exception, RootException);
-
-    DeserializerJsonV0(Source & source, HeaderV0 const & header) : m_source(source), m_header(header)
-    {
-      std::string src;
-      src.resize(source.Size());
-      source.Read(static_cast<void *>(&src[0]), source.Size());
-      m_jsonObject.Attach(src.c_str());
-      m_json = m_jsonObject.get();
-    }
-
-    void operator()(bool & d, char const * name = nullptr)
-    {
-      FromJSONObject(m_json, name, d);
-    }
-
-    void operator()(uint8_t & d, char const * name = nullptr)
-    {
-      FromJSONObject(m_json, name, d);
-    }
-
-    void operator()(uint32_t & d, char const * name = nullptr)
-    {
-      FromJSONObject(m_json, name, d);
-    }
-
-    void operator()(uint64_t & d, char const * name = nullptr)
-    {
-      FromJSONObject(m_json, name, d);
-    }
+    std::string src(source.Size(), '\0');
+    source.Read(static_cast<void *>(&src[0]), source.Size());
+    m_jsonObject.ParseFrom(src.c_str());
+    m_json = m_jsonObject.get();
+  }
+
+  void operator()(bool & d, char const * name = nullptr) { FromJSONObject(m_json, name, d); }
+  void operator()(uint8_t & d, char const * name = nullptr) { FromJSONObject(m_json, name, d); }
+  void operator()(uint32_t & d, char const * name = nullptr) { FromJSONObject(m_json, name, d); }
+  void operator()(uint64_t & d, char const * name = nullptr) { FromJSONObject(m_json, name, d); }
+  void operator()(std::string & s, char const * name = nullptr) { FromJSONObject(m_json, name, s); }
+  void operator()(Time & t, char const * name = nullptr)
+  {
+    uint32_t d = 0;
+    FromJSONObject(m_json, name, d);
+    t = FromDaysSinceEpoch(d);
+  }
 
-    void operator()(std::string & s, char const * name = nullptr)
-    {
-      FromJSONObject(m_json, name, s);
-    }
+  void operator()(Sentiment & sentiment, char const * name = nullptr)
+  {
+    bool s = false;
+    FromJSONObject(m_json, name, s);
+    sentiment = s ? Sentiment::Positive : Sentiment::Negative;
+  }
 
-    void operator()(Time & t, char const * name = nullptr)
-    {
-      uint32_t d = 0;
-      FromJSONObject(m_json, name, d);
-      t = FromDaysSinceEpoch(d);
-    }
+  template <typename T>
+  void operator()(vector<T> & vs, char const * name = nullptr)
+  {
+    json_t * context = SaveContext(name);
 
-    void operator()(Sentiment & sentiment, char const * name = nullptr)
-    {
-      bool s = false;
-      FromJSONObject(m_json, name, s);
-      sentiment = s ? Sentiment::Positive : Sentiment::Negative;
-    }
+    if (!json_is_array(m_json))
+      MYTHROW(my::Json::Exception, ("The field", name, "must contain a json array."));
 
-    template <typename T>
-    void operator()(vector<T> & vs, char const * name = nullptr)
+    vs.resize(json_array_size(m_json));
+    for (size_t index = 0; index < vs.size(); ++index)
     {
-      json_t * context = SaveContext(name);
-
-      if (!json_is_array(m_json))
-        MYTHROW(my::Json::Exception, ("The field", name, "must contain a json array."));
-
-      vs.resize(json_array_size(m_json));
-      for (size_t index = 0; index < vs.size(); ++index)
-      {
-        json_t * context = SaveContext();
-        m_json = json_array_get(context, index);
-        (*this)(vs[index]);
-        RestoreContext(context);
-      }
-
+      json_t * context = SaveContext();
+      m_json = json_array_get(context, index);
+      (*this)(vs[index]);
       RestoreContext(context);
     }
 
-    template <typename R>
-    void operator()(R & r, char const * name = nullptr)
-    {
-      json_t * context = SaveContext(name);
-      r.Visit(*this);
-      RestoreContext(context);
-    }
+    RestoreContext(context);
+  }
 
-    void VisitRating(float & f, char const * name = nullptr)
-    {
-      double d = 0.0;
-      FromJSONObject(m_json, name, d);
-      f = static_cast<float>(d);
-    }
+  template <typename R>
+  void operator()(R & r, char const * name = nullptr)
+  {
+    json_t * context = SaveContext(name);
+    r.Visit(*this);
+    RestoreContext(context);
+  }
 
-  private:
-    json_t * SaveContext(char const * name = nullptr)
-    {
-      json_t * context = m_json;
-      if (name)
-        m_json = my::GetJSONObligatoryField(context, name);
-      return context;
-    }
+  void VisitRating(float & f, char const * name = nullptr)
+  {
+    double d = 0.0;
+    FromJSONObject(m_json, name, d);
+    f = static_cast<float>(d);
+  }
 
-    void RestoreContext(json_t * context)
-    {
-      if (context)
-        m_json = context;
-    }
+private:
+  json_t * SaveContext(char const * name = nullptr)
+  {
+    json_t * context = m_json;
+    if (name)
+      m_json = my::GetJSONObligatoryField(context, name);
+    return context;
+  }
 
-    my::Json m_jsonObject;
-    json_t * m_json = nullptr;
-    Source & m_source;
-    HeaderV0 const m_header;
-  };
+  void RestoreContext(json_t * context)
+  {
+    if (context)
+      m_json = context;
+  }
+
+  my::Json m_jsonObject;
+  json_t * m_json = nullptr;
+  Source & m_source;
+};
 }  // namespace ugc
diff --git a/ugc/ugc_tests/CMakeLists.txt b/ugc/ugc_tests/CMakeLists.txt
index 63638a70a8..333d762dac 100644
--- a/ugc/ugc_tests/CMakeLists.txt
+++ b/ugc/ugc_tests/CMakeLists.txt
@@ -3,19 +3,18 @@ project(ugc_tests)
 set(
   SRC
   serdes_tests.cpp
-  serdes_json_tests.cpp
 )
 
 omim_add_test(${PROJECT_NAME} ${SRC})
 omim_link_libraries(
   ${PROJECT_NAME}
   ugc
-  jansson
   indexer
   platform
   coding
   geometry
   base
+  jansson
   stats_client
   ${LIBZ}
 )
diff --git a/ugc/ugc_tests/serdes_json_test.cpp b/ugc/ugc_tests/serdes_json_test.cpp
deleted file mode 100644
index d4d16a4780..0000000000
--- a/ugc/ugc_tests/serdes_json_test.cpp
+++ /dev/null
@@ -1,143 +0,0 @@
-#include "testing/testing.hpp"
-
-#include "ugc/api.hpp"
-#include "ugc/serdes_json.hpp"
-#include "ugc/types.hpp"
-
-#include "coding/reader.hpp"
-#include "coding/writer.hpp"
-
-#include <cstdint>
-#include <vector>
-
-using namespace std;
-using namespace ugc;
-
-namespace
-{
-  using Buffer = vector<uint8_t>;
-  using Ser = SerializerJson<MemWriter<Buffer>>;
-  using Des = DeserializerJsonV0<ReaderSource<MemReader>>;
-
-  Rating GetTestRating()
-  {
-    vector<RatingRecord> records;
-    records.emplace_back("music" /* key */, 5.0 /* value */);
-    records.emplace_back("service" /* key */, 4.0 /* value */);
-
-    return Rating(records, 4.5 /* aggValue */);
-  }
-
-  MemWriter<Buffer> MakeSink(Buffer & buffer) { return MemWriter<Buffer>(buffer); }
-
-  ReaderSource<MemReader> MakeSource(Buffer const & buffer)
-  {
-    MemReader reader(buffer.data(), buffer.size());
-    return ReaderSource<MemReader>(reader);
-  }
-
-  UNIT_TEST(SerDes_Json_Rating)
-  {
-    auto expectedRating = GetTestRating();
-    TEST_EQUAL(expectedRating, expectedRating, ());
-
-    HeaderV0 header;
-
-    Buffer buffer;
-
-    {
-      auto sink = MakeSink(buffer);
-      Ser ser(sink, header);
-      ser(expectedRating);
-      ser.Flush();
-    }
-
-    Rating actualRating({} /* ratings */, {} /* aggValue */);
-
-    {
-      auto source = MakeSource(buffer);
-      Des des(source, header);
-      des(actualRating);
-    }
-
-    TEST_EQUAL(expectedRating, actualRating, ());
-  }
-
-  UNIT_TEST(SerDes_Json_Reviews)
-  {
-    auto expectedUGC = Api::MakeTestUGC1().m_reviews;
-    TEST_EQUAL(expectedUGC, expectedUGC, ());
-
-    HeaderV0 header;
-
-    Buffer buffer;
-
-    {
-      auto sink = MakeSink(buffer);
-      Ser ser(sink, header);
-      ser(expectedUGC);
-      ser.Flush();
-    }
-
-    std::vector<Review> actualUGC({} /* rating */, {} /* reviews */, {} /* attributes */);
-    {
-      auto source = MakeSource(buffer);
-      Des des(source, header);
-      des(actualUGC);
-    }
-
-    TEST_EQUAL(expectedUGC, actualUGC, ());
-  }
-
-  UNIT_TEST(SerDes_Json_Attributes)
-  {
-    auto expectedUGC = Api::MakeTestUGC1().m_attributes;
-    TEST_EQUAL(expectedUGC, expectedUGC, ());
-
-    HeaderV0 header;
-
-    Buffer buffer;
-
-    {
-      auto sink = MakeSink(buffer);
-      Ser ser(sink, header);
-      ser(expectedUGC);
-      ser.Flush();
-    }
-
-    std::vector<Attribute> actualUGC({} /* rating */, {} /* reviews */, {} /* attributes */);
-    {
-      auto source = MakeSource(buffer);
-      Des des(source, header);
-      des(actualUGC);
-    }
-
-    TEST_EQUAL(expectedUGC, actualUGC, ());
-  }
-
-  UNIT_TEST(SerDes_Json_UGC)
-  {
-    auto expectedUGC = Api::MakeTestUGC1();
-    TEST_EQUAL(expectedUGC, expectedUGC, ());
-
-    HeaderV0 header;
-
-    Buffer buffer;
-
-    {
-      auto sink = MakeSink(buffer);
-      Ser ser(sink, header);
-      ser(expectedUGC);
-      ser.Flush();
-    }
-
-    UGC actualUGC({} /* rating */, {} /* reviews */, {} /* attributes */);
-    {
-      auto source = MakeSource(buffer);
-      Des des(source, header);
-      des(actualUGC);
-    }
-
-    TEST_EQUAL(expectedUGC, actualUGC, ());
-  }
-}  // namespace
diff --git a/ugc/ugc_tests/serdes_tests.cpp b/ugc/ugc_tests/serdes_tests.cpp
index 79465ad522..8271ec8e98 100644
--- a/ugc/ugc_tests/serdes_tests.cpp
+++ b/ugc/ugc_tests/serdes_tests.cpp
@@ -2,6 +2,7 @@
 
 #include "ugc/api.hpp"
 #include "ugc/serdes.hpp"
+#include "ugc/serdes_json.hpp"
 #include "ugc/types.hpp"
 
 #include "coding/reader.hpp"
@@ -16,8 +17,11 @@ using namespace ugc;
 namespace
 {
 using Buffer = vector<uint8_t>;
-using Ser = Serializer<MemWriter<Buffer>>;
-using Des = DeserializerV0<ReaderSource<MemReader>>;
+using ToBin = Serializer<MemWriter<Buffer>>;
+using FromBin = DeserializerV0<ReaderSource<MemReader>>;
+using ToJson = SerializerJson<MemWriter<Buffer>>;
+using FromJson = DeserializerJsonV0<ReaderSource<MemReader>>;
+
 
 Rating GetTestRating()
 {
@@ -36,28 +40,64 @@ ReaderSource<MemReader> MakeSource(Buffer const & buffer)
   return ReaderSource<MemReader>(reader);
 }
 
-UNIT_TEST(SerDes_Rating)
+template<typename Object, typename Serializator, typename Deserializator>
+void MakeTest(Object const & src)
 {
-  auto const expectedRating = GetTestRating();
-  TEST_EQUAL(expectedRating, expectedRating, ());
-
   Buffer buffer;
+  Object trg;
 
   {
     auto sink = MakeSink(buffer);
-    Ser ser(sink);
-    ser(expectedRating);
+    Serializator ser(sink);
+    ser(src);
   }
 
-  Rating actualRating({} /* ratings */, {} /* aggValue */);
-
   {
     auto source = MakeSource(buffer);
-    Des des(source);
-    des(actualRating);
+    Deserializator des(source);
+    des(trg);
   }
+  TEST_EQUAL(src, trg, ());
+}
+
+UNIT_TEST(SerDes_Rating)
+{
+  auto const expectedRating = GetTestRating();
+  TEST_EQUAL(expectedRating, expectedRating, ());
+
+  MakeTest<Rating, ToBin, FromBin>(expectedRating);
+}
+
+UNIT_TEST(SerDes_Json_Rating)
+{
+  auto const expectedRating = GetTestRating();
+  TEST_EQUAL(expectedRating, expectedRating, ());
+
+  MakeTest<Rating, ToJson, FromJson>(expectedRating);
+}
+
+UNIT_TEST(SerDes_Json_Reviews)
+{
+  auto expectedUGC = Api::MakeTestUGC1(Time(chrono::hours(24 * 100))).m_reviews;
+  TEST_EQUAL(expectedUGC, expectedUGC, ());
+
+  MakeTest<decltype(expectedUGC), ToJson, FromJson>(expectedUGC);
+}
+
+UNIT_TEST(SerDes_Json_Attributes)
+{
+  auto expectedUGC = Api::MakeTestUGC1(Time(chrono::hours(24 * 100))).m_attributes;
+  TEST_EQUAL(expectedUGC, expectedUGC, ());
+
+  MakeTest<decltype(expectedUGC), ToJson, FromJson>(expectedUGC);
+}
+
+UNIT_TEST(SerDes_Json_UGC)
+{
+  auto expectedUGC = Api::MakeTestUGC1(Time(chrono::hours(24 * 100)));
+  TEST_EQUAL(expectedUGC, expectedUGC, ());
 
-  TEST_EQUAL(expectedRating, actualRating, ());
+  MakeTest<decltype(expectedUGC), ToJson, FromJson>(expectedUGC);
 }
 
 UNIT_TEST(SerDes_UGC)
diff --git a/ugc/ugc_tests/ugc_tests.pro b/ugc/ugc_tests/ugc_tests.pro
index 2537430a24..4cc55a6895 100644
--- a/ugc/ugc_tests/ugc_tests.pro
+++ b/ugc/ugc_tests/ugc_tests.pro
@@ -6,7 +6,7 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 ROOT_DIR = ../..
-DEPENDENCIES = ugc jansson indexer platform coding geometry base stats_client
+DEPENDENCIES = ugc indexer platform coding geometry base jansson stats_client
 
 macx-* {
   LIBS *= "-framework IOKit" "-framework SystemConfiguration"
@@ -19,4 +19,3 @@ QT *= core
 SOURCES += \
   ../../testing/testingmain.cpp \
   serdes_tests.cpp \
-  serdes_json_tests.cpp \