From 6c389a2e4cce6e5ea4f35212cb209d0c2022e749 Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Fri, 3 Nov 2017 17:50:08 +0300 Subject: [PATCH] New version of UGCUpdate --- ugc/serdes.hpp | 46 +++++++++- ugc/serdes_json.hpp | 18 ++++ ugc/storage.cpp | 126 +++++++++++++++------------ ugc/storage.hpp | 1 + ugc/types.hpp | 149 +++++++++++++++++++++++++++----- ugc/ugc_tests/serdes_tests.cpp | 22 +++++ ugc/ugc_tests/storage_tests.cpp | 25 +++++- ugc/ugc_tests/utils.cpp | 16 +++- ugc/ugc_tests/utils.hpp | 1 + 9 files changed, 321 insertions(+), 83 deletions(-) diff --git a/ugc/serdes.hpp b/ugc/serdes.hpp index 51f36976d8..9d9c74f8a8 100644 --- a/ugc/serdes.hpp +++ b/ugc/serdes.hpp @@ -12,6 +12,7 @@ #include #include +#include namespace ugc { @@ -21,7 +22,8 @@ DECLARE_EXCEPTION(BadBlob, SerDesException); enum class Version : uint8_t { V0, - Latest = V0 + V1, + Latest = V1 }; template @@ -50,6 +52,11 @@ public: WriteToSink(m_sink, index); } + void VisitLangs(std::vector const & indexes, char const * /* name */ = nullptr) + { + (*this)(indexes); + } + template void VisitVarUint(T const & t, char const * /* name */ = nullptr) { @@ -127,6 +134,11 @@ public: ReadPrimitiveFromSource(m_source, index); } + void VisitLangs(std::vector & indexes, char const * /* name */ = nullptr) + { + (*this)(indexes); + } + template void VisitVarUint(T & t, char const * /* name */ = nullptr) { @@ -177,20 +189,46 @@ private: Source & m_source; }; +// Version must be used only for testing. +// We should use only the latest version in production. template -void Serialize(Sink & sink, UGC const & ugc) +void Serialize(Sink & sink, UGC const & ugc, Version const version = Version::Latest) { - WriteToSink(sink, static_cast(Version::Latest)); + WriteToSink(sink, static_cast(version)); Serializer ser(sink); ser(ugc); } -template +template void Deserialize(Source & source, UGC & ugc) +{ + uint8_t version = 0; + ReadPrimitiveFromSource(source, version); + if (version == static_cast(Version::V0) || version == static_cast(Version::V1)) + { + DeserializerV0 des(source); + des(ugc); + return; + } + + MYTHROW(BadBlob, ("Unknown data version:", static_cast(version))); +} + +template +void Deserialize(Source & source, UGCUpdate & ugc) { uint8_t version = 0; ReadPrimitiveFromSource(source, version); if (version == static_cast(Version::V0)) + { + DeserializerV0 des(source); + v0::UGCUpdate old; + des(old); + ugc.BuildFrom(old); + return; + } + + if (version == static_cast(Version::V1)) { DeserializerV0 des(source); des(ugc); diff --git a/ugc/serdes_json.hpp b/ugc/serdes_json.hpp index 613e1dd1b1..306557a1c1 100644 --- a/ugc/serdes_json.hpp +++ b/ugc/serdes_json.hpp @@ -8,6 +8,7 @@ #include #include +#include namespace ugc { @@ -85,6 +86,15 @@ public: ToJSONObject(*m_json, name, StringUtf8Multilang::GetLangByCode(index)); } + void VisitLangs(std::vector const & indexes, char const * name = nullptr) + { + std::vector langs; + for (auto const index : indexes) + langs.emplace_back(StringUtf8Multilang::GetLangByCode(index)); + + ToJSONObject(*m_json, name, langs); + } + void VisitPoint(m2::PointD const & pt, char const * x = nullptr, char const * y = nullptr) { (*this)(pt.x, x); @@ -214,6 +224,14 @@ public: index = StringUtf8Multilang::GetLangIndex(lang); } + void VisitLangs(std::vector & indexes, char const * name = nullptr) + { + std::vector langs; + FromJSONObject(m_json, name, langs); + for (auto const & lang : langs) + indexes.emplace_back(StringUtf8Multilang::GetLangIndex(lang)); + } + void VisitPoint(m2::PointD & pt, char const * x = nullptr, char const * y = nullptr) { FromJSONObject(m_json, x, pt.x); diff --git a/ugc/storage.cpp b/ugc/storage.cpp index f253dd81d5..756678be54 100644 --- a/ugc/storage.cpp +++ b/ugc/storage.cpp @@ -72,6 +72,61 @@ string SerializeUGCIndex(vector const & indexes) unique_ptr buffer(json_dumps(array.get(), JSON_COMPACT | JSON_ENSURE_ASCII)); return string(buffer.get()); } + +template +Storage::SettingResult SetGenericUGCUpdate( + vector & indexes, size_t & numberOfDeleted, FeatureID const & id, + UGCUpdate const & ugc, + FeatureType const & featureType, + Version const version = Version::Latest) +{ + if (!ugc.IsValid()) + return Storage::SettingResult::InvalidUGC; + + auto const mercator = feature::GetCenter(featureType); + feature::TypesHolder th(featureType); + th.SortBySpec(); + auto const optMatchingType = ftraits::UGC::GetType(th); + CHECK(optMatchingType, ()); + auto const type = th.GetBestType(); + for (auto & index : indexes) + { + if (type == index.m_type && mercator == index.m_mercator && !index.m_deleted) + { + index.m_deleted = true; + ++numberOfDeleted; + break; + } + } + + Storage::UGCIndex index; + uint64_t offset; + if (!GetUGCFileSize(offset)) + offset = 0; + + index.m_mercator = mercator; + index.m_type = type; + index.m_matchingType = *optMatchingType; + index.m_mwmName = id.GetMwmName(); + index.m_dataVersion = id.GetMwmVersion(); + index.m_featureId = id.m_index; + index.m_offset = offset; + + auto const ugcFilePath = GetUGCFilePath(); + try + { + FileWriter w(ugcFilePath, FileWriter::Op::OP_APPEND); + Serialize(w, ugc, version); + indexes.emplace_back(move(index)); + } + catch (FileWriter::Exception const & exception) + { + LOG(LERROR, ("Exception while writing file:", ugcFilePath, "reason:", exception.what())); + return Storage::SettingResult::WritingError; + } + + return Storage::SettingResult::Success; +} } // namespace UGCUpdate Storage::GetUGCUpdate(FeatureID const & id) const @@ -105,7 +160,7 @@ UGCUpdate Storage::GetUGCUpdate(FeatureID const & id) const } catch (FileReader::Exception const & exception) { - LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.Msg())); + LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.what())); return {}; } @@ -118,54 +173,9 @@ UGCUpdate Storage::GetUGCUpdate(FeatureID const & id) const Storage::SettingResult Storage::SetUGCUpdate(FeatureID const & id, UGCUpdate const & ugc) { - if (!ugc.IsValid()) - return Storage::SettingResult::InvalidUGC; - auto const feature = GetFeature(id); - auto const mercator = feature::GetCenter(*feature); - feature::TypesHolder th(*feature); - th.SortBySpec(); - auto const optMatchingType = ftraits::UGC::GetType(th); - CHECK(optMatchingType, ()); - auto const type = th.GetBestType(); - for (auto & index : m_UGCIndexes) - { - if (type == index.m_type && mercator == index.m_mercator && !index.m_deleted) - { - index.m_deleted = true; - m_numberOfDeleted++; - break; - } - } - // TODO: Call Defragmentation(). - - UGCIndex index; - uint64_t offset; - if (!GetUGCFileSize(offset)) - offset = 0; - - index.m_mercator = mercator; - index.m_type = type; - index.m_matchingType = *optMatchingType; - index.m_mwmName = id.GetMwmName(); - index.m_dataVersion = id.GetMwmVersion(); - index.m_featureId = id.m_index; - index.m_offset = offset; - - auto const ugcFilePath = GetUGCFilePath(); - try - { - FileWriter w(ugcFilePath, FileWriter::Op::OP_APPEND); - Serialize(w, ugc); - m_UGCIndexes.emplace_back(move(index)); - } - catch (FileWriter::Exception const & exception) - { - LOG(LERROR, ("Exception while writing file:", ugcFilePath, "reason:", exception.Msg())); - return Storage::SettingResult::WritingError; - } - - return Storage::SettingResult::Success; + return SetGenericUGCUpdate(m_UGCIndexes, m_numberOfDeleted, id, ugc, + *feature); } void Storage::Load() @@ -179,7 +189,7 @@ void Storage::Load() } catch (FileReader::Exception const & exception) { - LOG(LWARNING, ("Exception while reading file:", indexFilePath, "reason:", exception.Msg())); + LOG(LWARNING, ("Exception while reading file:", indexFilePath, "reason:", exception.what())); return; } @@ -205,7 +215,7 @@ void Storage::SaveIndex() const } catch (FileWriter::Exception const & exception) { - LOG(LERROR, ("Exception while writing file:", indexFilePath, "reason:", exception.Msg())); + LOG(LERROR, ("Exception while writing file:", indexFilePath, "reason:", exception.what())); } } @@ -241,12 +251,12 @@ void Storage::Defragmentation() } catch (FileReader::Exception const & exception) { - LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.Msg())); + LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.what())); return; } catch (FileWriter::Exception const & exception) { - LOG(LERROR, ("Exception while writing file:", tmpUGCFilePath, "reason:", exception.Msg())); + LOG(LERROR, ("Exception while writing file:", tmpUGCFilePath, "reason:", exception.what())); return; } @@ -285,7 +295,7 @@ string Storage::GetUGCToSend() const } catch (FileReader::Exception const & exception) { - LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.Msg())); + LOG(LERROR, ("Exception while reading file:", ugcFilePath, "reason:", exception.what())); return string(); } @@ -371,4 +381,14 @@ unique_ptr Storage::GetFeature(FeatureID const & id) const CHECK(feature, ()); return feature; } + +// Testing +Storage::SettingResult Storage::SetUGCUpdateForTesting(FeatureID const & id, + v0::UGCUpdate const & ugc) +{ + auto const feature = GetFeature(id); + return SetGenericUGCUpdate(m_UGCIndexes, m_numberOfDeleted, id, ugc, + *feature, Version::V0); +} + } // namespace ugc diff --git a/ugc/storage.hpp b/ugc/storage.hpp index 3c9f484120..8eb4022fa6 100644 --- a/ugc/storage.hpp +++ b/ugc/storage.hpp @@ -60,6 +60,7 @@ public: /// Testing std::vector const & GetIndexesForTesting() const { return m_UGCIndexes; } size_t GetNumberOfDeletedForTesting() const { return m_numberOfDeleted; } + SettingResult SetUGCUpdateForTesting(FeatureID const & id, v0::UGCUpdate const & ugc); private: uint64_t UGCSizeAtIndex(size_t const indexPosition) const; diff --git a/ugc/types.hpp b/ugc/types.hpp index d2457e95b4..3817dc6b3f 100644 --- a/ugc/types.hpp +++ b/ugc/types.hpp @@ -140,6 +140,42 @@ inline std::string DebugPrint(Text const & text) return os.str(); } +struct KeyboardText +{ + KeyboardText() = default; + KeyboardText(std::string const & text, uint8_t const deviceLang, + std::vector const & keyboardLangs) + : m_text(text), m_deviceLang(deviceLang), m_keyboardLangs(keyboardLangs) + { + } + + DECLARE_VISITOR(visitor(m_text, "text"), visitor.VisitLang(m_deviceLang, "lang"), + visitor.VisitLangs(m_keyboardLangs, "kbd_langs")) + + bool operator==(KeyboardText const & rhs) const + { + return m_text == rhs.m_text && m_deviceLang == rhs.m_deviceLang && + m_keyboardLangs == rhs.m_keyboardLangs; + } + + std::string m_text; + uint8_t m_deviceLang = StringUtf8Multilang::kDefaultCode; + std::vector m_keyboardLangs; +}; + +inline std::string DebugPrint(KeyboardText const & text) +{ + std::ostringstream os; + os << "Keyboard text [ " << StringUtf8Multilang::GetLangByCode(text.m_deviceLang) << ": " + << text.m_text << " ]"; + os << "Keyboard locales [ "; + for (auto const index : text.m_keyboardLangs) + os << StringUtf8Multilang::GetLangByCode(index) << " "; + + os << " ]"; + return os.str(); +} + struct Review { using ReviewId = uint64_t; @@ -199,6 +235,84 @@ struct Attribute TranslationKey m_value{}; }; +struct ReviewFeedback +{ + ReviewFeedback() = default; + ReviewFeedback(Sentiment const sentiment, Time const & time) + : m_sentiment(sentiment), m_time(time) + { + } + + Sentiment m_sentiment{}; + Time m_time{}; +}; + +struct ReviewAbuse +{ + ReviewAbuse() = default; + ReviewAbuse(std::string const & reason, Time const & time) : m_reason(reason), m_time(time) {} + + std::string m_reason{}; + Time m_time{}; +}; + +namespace v0 +{ +struct UGCUpdate +{ + UGCUpdate() = default; + UGCUpdate(Ratings const & records, Text const & text, Time const & time) + : m_ratings(records), m_text(text), m_time(time) + { + } + + DECLARE_VISITOR(visitor(m_ratings, "ratings"), visitor(m_text, "text"), visitor(m_time, "date")) + + bool operator==(UGCUpdate const & rhs) const + { + return m_ratings == rhs.m_ratings && m_text == rhs.m_text && m_time == rhs.m_time; + } + + bool IsEmpty() const + { + return (m_ratings.empty() && m_text.m_text.empty()) || m_time == Time(); + } + + bool IsValid() const + { + bool const timeIsValid = m_time != Time(); + if (!m_text.m_text.empty()) + return timeIsValid; + + bool ratingIsValid = false; + for (auto const & r : m_ratings) + { + if (static_cast(r.m_value) > 0) + { + ratingIsValid = true; + break; + } + } + + return ratingIsValid && timeIsValid; + } + + Ratings m_ratings; + Text m_text; + Time m_time{}; +}; + +inline std::string DebugPrint(UGCUpdate const & ugcUpdate) +{ + std::ostringstream os; + os << "UGCUpdate [ "; + os << "records:" << ::DebugPrint(ugcUpdate.m_ratings) << ", "; + os << "text:" << DebugPrint(ugcUpdate.m_text) << ", "; + os << "days since epoch:" << ToDaysSinceEpoch(ugcUpdate.m_time) << " ]"; + return os.str(); +} +} // namespace v0 + struct UGC { UGC() = default; @@ -242,7 +356,7 @@ inline std::string DebugPrint(UGC const & ugc) struct UGCUpdate { UGCUpdate() = default; - UGCUpdate(Ratings const & records, Text const & text, Time const & time) + UGCUpdate(Ratings const & records, KeyboardText const & text, Time const & time) : m_ratings(records), m_text(text), m_time(time) { } @@ -254,9 +368,17 @@ struct UGCUpdate return m_ratings == rhs.m_ratings && m_text == rhs.m_text && m_time == rhs.m_time; } + void BuildFrom(v0::UGCUpdate const & update) + { + m_ratings = update.m_ratings; + m_text.m_text = update.m_text.m_text; + m_text.m_deviceLang = update.m_text.m_lang; + m_time = update.m_time; + } + bool IsEmpty() const { - return !((!m_ratings.empty() || !m_text.m_text.empty()) && m_time != Time()); + return (m_ratings.empty() && m_text.m_text.empty()) || m_time == Time(); } bool IsValid() const @@ -279,7 +401,7 @@ struct UGCUpdate } Ratings m_ratings; - Text m_text; + KeyboardText m_text; Time m_time{}; }; @@ -292,26 +414,5 @@ inline std::string DebugPrint(UGCUpdate const & ugcUpdate) os << "days since epoch:" << ToDaysSinceEpoch(ugcUpdate.m_time) << " ]"; return os.str(); } - -struct ReviewFeedback -{ - ReviewFeedback() = default; - ReviewFeedback(Sentiment const sentiment, Time const & time) - : m_sentiment(sentiment), m_time(time) - { - } - - Sentiment m_sentiment{}; - Time m_time{}; -}; - -struct ReviewAbuse -{ - ReviewAbuse() = default; - ReviewAbuse(std::string const & reason, Time const & time) : m_reason(reason), m_time(time) {} - - std::string m_reason{}; - Time m_time{}; -}; } // namespace ugc diff --git a/ugc/ugc_tests/serdes_tests.cpp b/ugc/ugc_tests/serdes_tests.cpp index b3f5c0154f..a6e844533d 100644 --- a/ugc/ugc_tests/serdes_tests.cpp +++ b/ugc/ugc_tests/serdes_tests.cpp @@ -149,4 +149,26 @@ UNIT_TEST(SerDes_UGCUpdate) TEST_EQUAL(expectedUGC, actualUGC, ()); } + +UNIT_TEST(SerDes_UGCUpdateDifferentVersions) +{ + auto const v0 = MakeTestUGCUpdateV0(Time(chrono::hours(24 * 10))); + TEST_EQUAL(v0, v0, ()); + + Buffer buffer; + { + auto sink = MakeSink(buffer); + Serialize(sink, v0, Version::V0); + } + + UGCUpdate actualUGC{}; + { + auto source = MakeSource(buffer); + Deserialize(source, actualUGC); + } + + UGCUpdate expectedUgc; + expectedUgc.BuildFrom(v0); + TEST_EQUAL(expectedUgc, actualUGC, ()); +} } // namespace diff --git a/ugc/ugc_tests/storage_tests.cpp b/ugc/ugc_tests/storage_tests.cpp index 76b5c5d5a7..2f49b94cac 100644 --- a/ugc/ugc_tests/storage_tests.cpp +++ b/ugc/ugc_tests/storage_tests.cpp @@ -338,7 +338,7 @@ UNIT_CLASS_TEST(StorageTest, InvalidUGC) TEST_EQUAL(storage.SetUGCUpdate(cafeId, first), Storage::SettingResult::InvalidUGC, ()); TEST(storage.GetIndexesForTesting().empty(), ()); TEST(storage.GetUGCToSend().empty(), ()); - first.m_text = Text("a", 1); + first.m_text = KeyboardText("a", 1, {2}); TEST_EQUAL(storage.SetUGCUpdate(cafeId, first), Storage::SettingResult::Success, ()); UGCUpdate second; second.m_time = chrono::system_clock::now(); @@ -347,3 +347,26 @@ UNIT_CLASS_TEST(StorageTest, InvalidUGC) second.m_ratings.emplace_back("b", 1); TEST_EQUAL(storage.SetUGCUpdate(cafeId, second), Storage::SettingResult::Success, ()); } + +UNIT_CLASS_TEST(StorageTest, DifferentUGCVersions) +{ + auto & builder = MwmBuilder::Builder(); + m2::PointD const firstPoint(1.0, 1.0); + m2::PointD const secondPoint(2.0, 2.0); + builder.Build({TestCafe(firstPoint), TestCafe(secondPoint)}); + + Storage storage(builder.GetIndex()); + storage.Load(); + + auto const firstId = builder.FeatureIdForCafeAtPoint(firstPoint); + auto const oldUGC = MakeTestUGCUpdateV0(Time(chrono::hours(24 * 10))); + TEST_EQUAL(storage.SetUGCUpdateForTesting(firstId, oldUGC), Storage::SettingResult::Success, ()); + auto const secondId = builder.FeatureIdForCafeAtPoint(secondPoint); + auto const newUGC = MakeTestUGCUpdate(Time(chrono::hours(24 * 5))); + TEST_EQUAL(storage.SetUGCUpdate(secondId, newUGC), Storage::SettingResult::Success, ()); + + TEST_EQUAL(newUGC, storage.GetUGCUpdate(secondId), ()); + UGCUpdate fromOld; + fromOld.BuildFrom(oldUGC); + TEST_EQUAL(fromOld, storage.GetUGCUpdate(firstId), ()); +} diff --git a/ugc/ugc_tests/utils.cpp b/ugc/ugc_tests/utils.cpp index 11b178df42..b246837f28 100644 --- a/ugc/ugc_tests/utils.cpp +++ b/ugc/ugc_tests/utils.cpp @@ -43,7 +43,7 @@ UGC MakeTestUGC2(Time now) return UGC(records, reviews, 5.0 /* rating */, 1 /* votes */); } -UGCUpdate MakeTestUGCUpdate(Time now) +v0::UGCUpdate MakeTestUGCUpdateV0(Time now) { Ratings records; records.emplace_back("food" /* key */, 4.0 /* value */); @@ -54,6 +54,20 @@ UGCUpdate MakeTestUGCUpdate(Time now) Time time{FromDaysAgo(now, 1)}; + return v0::UGCUpdate(records, text, time); +} + +UGCUpdate MakeTestUGCUpdate(Time now) +{ + Ratings records; + records.emplace_back("food" /* key */, 4.0 /* value */); + records.emplace_back("service" /* key */, 5.0 /* value */); + records.emplace_back("music" /* key */, 5.0 /* value */); + + KeyboardText text{"It's aways nice to visit this place", StringUtf8Multilang::kEnglishCode, {1}}; + + Time time{FromDaysAgo(now, 1)}; + return UGCUpdate(records, text, time); } } diff --git a/ugc/ugc_tests/utils.hpp b/ugc/ugc_tests/utils.hpp index cc4507bb8c..c28ab6b2f1 100644 --- a/ugc/ugc_tests/utils.hpp +++ b/ugc/ugc_tests/utils.hpp @@ -7,5 +7,6 @@ namespace ugc UGC MakeTestUGC1(Time now = Clock::now()); UGC MakeTestUGC2(Time now = Clock::now()); +v0::UGCUpdate MakeTestUGCUpdateV0(Time now = Clock::now()); UGCUpdate MakeTestUGCUpdate(Time now = Clock::now()); } // namespace ugc