From 232f82675a2fee9c6f8c69c57b257a5b2c61f01e Mon Sep 17 00:00:00 2001 From: Vlad Mihaylenko Date: Tue, 26 Sep 2017 17:20:02 +0300 Subject: [PATCH] [dnm] Refactored ugc structure (#7136) * Refactored ugc structure and connected it with UI. * Fixed iOS compilation. * [android] Fixed android build compilation * Round rating. --- android/jni/com/mapswithme/maps/Framework.cpp | 2 +- android/jni/com/mapswithme/maps/Sponsored.cpp | 5 +- android/jni/com/mapswithme/maps/ugc/UGC.cpp | 6 +- iphone/Maps/UI/PlacePage/MWMPlacePageData.mm | 35 ++------ .../Maps/UI/PlacePage/MWMPlacePageManager.mm | 8 +- .../Content/UGC/MWMUGCReviewVM.mm | 19 ++-- map/framework.cpp | 2 +- map/framework.hpp | 5 +- map/place_page_info.cpp | 71 +++++++++++---- map/place_page_info.hpp | 30 ++++++- ugc/api.cpp | 83 ++++++++---------- ugc/api.hpp | 6 +- ugc/serdes.hpp | 4 +- ugc/types.hpp | 86 +++++++++++-------- ugc/ugc_tests/serdes_binary_tests.cpp | 3 +- ugc/ugc_tests/serdes_tests.cpp | 34 +++++++- 16 files changed, 231 insertions(+), 168 deletions(-) diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index d26a33d059..f70bd4a492 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -585,7 +585,7 @@ void Framework::RequestViatorProducts(JNIEnv * env, jobject policy, std::string void Framework::RequestUGC(FeatureID const & fid, ugc::Api::UGCCallback const & ugcCallback) { - m_work.GetUGCApi().GetUGC(fid, ugcCallback); + m_work.GetUGCApi()->GetUGC(fid, ugcCallback); } uint64_t Framework::GetRentNearby(JNIEnv * env, jobject policy, ms::LatLon const & latlon, diff --git a/android/jni/com/mapswithme/maps/Sponsored.cpp b/android/jni/com/mapswithme/maps/Sponsored.cpp index be6a66261a..80f8e12f52 100644 --- a/android/jni/com/mapswithme/maps/Sponsored.cpp +++ b/android/jni/com/mapswithme/maps/Sponsored.cpp @@ -117,8 +117,11 @@ JNIEXPORT jobject JNICALL Java_com_mapswithme_maps_widget_placepage_Sponsored_na if (!ppInfo.IsSponsored()) return nullptr; + //TODO: consider using the raw value and impress directly. + std::string rating = place_page::rating::GetRatingFormatted(ppInfo.GetRatingRawValue()); + return env->NewObject(g_sponsoredClass, g_sponsoredClassConstructor, - jni::ToJavaString(env, ppInfo.GetRatingFormatted()), + jni::ToJavaString(env, rating), jni::ToJavaString(env, ppInfo.GetApproximatePricing()), jni::ToJavaString(env, ppInfo.GetSponsoredUrl()), jni::ToJavaString(env, ppInfo.GetSponsoredDescriptionUrl()), diff --git a/android/jni/com/mapswithme/maps/ugc/UGC.cpp b/android/jni/com/mapswithme/maps/ugc/UGC.cpp index aa0bd47eaf..4cb4fa78c7 100644 --- a/android/jni/com/mapswithme/maps/ugc/UGC.cpp +++ b/android/jni/com/mapswithme/maps/ugc/UGC.cpp @@ -67,10 +67,10 @@ public: private: jobject ToJavaUGC(JNIEnv * env, ugc::UGC const & ugc) { - jni::TScopedLocalObjectArrayRef ratings(env, ToJavaRatings(env, ugc.m_rating.m_ratings)); + jni::TScopedLocalObjectArrayRef ratings(env, ToJavaRatings(env, ugc.m_ratings)); jni::TScopedLocalObjectArrayRef reviews(env, ToJavaReviews(env, ugc.m_reviews)); - jobject result = env->NewObject(m_ugcClass, m_ugcCtor, ratings.get(), ugc.m_rating.m_aggValue, + jobject result = env->NewObject(m_ugcClass, m_ugcCtor, ratings.get(), ugc.m_aggRating, reviews.get()); ASSERT(result, ()); return result; @@ -159,7 +159,7 @@ JNIEXPORT void JNICALL Java_com_mapswithme_maps_ugc_UGC_requestUGC(JNIEnv * env, jobject featureId) { auto const fid = g_builder.Build(env, featureId); - g_framework->RequestUGC(fid, [&](ugc::UGC const & ugc) { + g_framework->RequestUGC(fid, [&](ugc::UGC const & ugc, ugc::UGCUpdate const & update) { JNIEnv * e = jni::GetEnv(); g_bridge.OnResult(e, ugc); }); diff --git a/iphone/Maps/UI/PlacePage/MWMPlacePageData.mm b/iphone/Maps/UI/PlacePage/MWMPlacePageData.mm index 261ba04bdf..7f1870c772 100644 --- a/iphone/Maps/UI/PlacePage/MWMPlacePageData.mm +++ b/iphone/Maps/UI/PlacePage/MWMPlacePageData.mm @@ -442,39 +442,17 @@ using namespace place_page; m_ugcRows.push_back(UGCRow::SelectImpression); auto & f = GetFramework(); - f.GetUGCApi().GetUGC([self featureId], [self](ugc::UGC const & ugc) { + f.GetUGCApi()->GetUGC(self.featureId, [self](ugc::UGC const & ugc, ugc::UGCUpdate const & update) + { auto const it = find(self->m_sections.begin(), self->m_sections.end(), Sections::UGC); ASSERT(it != self->m_sections.end(), ()); auto const position = static_cast(distance(self->m_sections.begin(), it)); auto length = 0UL; - - self->m_ugc = ugc; - auto constexpr maxNumberOfReviews = 3UL; - auto const size = ugc.m_reviews.size(); - if (size > maxNumberOfReviews) - { - self->m_ugcRows.insert(m_ugcRows.end(), maxNumberOfReviews, UGCRow::Comment); - length += maxNumberOfReviews; - self->m_ugcRows.emplace_back(UGCRow::ShowMore); - length++; - } - else - { - self->m_ugcRows.insert(m_ugcRows.end(), size, UGCRow::Comment); - length += size; - } + // TODO: process ugc. self.sectionsAreReadyCallback({position, length}, self, NO /* It's not a section */); }); - - // TODO: Complete static ugc with dynamic one. - f.GetUGCApi().GetUGCUpdate(self.featureId, [self](ugc::UGCUpdate const & ugc) { - self.reviewViewModel = static_cast *>( - [MWMUGCReviewVM fromUGC:ugc - featureId:self.featureId - name:self.title]); - }); } #pragma mark - Update bookmark status @@ -551,7 +529,12 @@ using namespace place_page; #pragma mark - Sponsored -- (NSString *)bookingRating { return self.isBooking ? @(m_info.GetRatingFormatted().c_str()) : nil; } +- (NSString *)bookingRating +{ + if (!self.isBooking) + return nil; + return @(rating::GetRatingFormatted(m_info.GetRatingRawValue()).c_str()); +} - (NSString *)bookingApproximatePricing { return self.isBooking ? @(m_info.GetApproximatePricing().c_str()) : nil; } - (NSURL *)sponsoredURL { diff --git a/iphone/Maps/UI/PlacePage/MWMPlacePageManager.mm b/iphone/Maps/UI/PlacePage/MWMPlacePageManager.mm index 707647c4bf..f5e0affb90 100644 --- a/iphone/Maps/UI/PlacePage/MWMPlacePageManager.mm +++ b/iphone/Maps/UI/PlacePage/MWMPlacePageManager.mm @@ -600,13 +600,7 @@ void logSponsoredEvent(MWMPlacePageData * data, NSString * eventName) - (void)reviewOn:(NSInteger)starNumber { - GetFramework().GetUGCApi().GetUGCUpdate( - [self.data featureId], [self, starNumber](ugc::UGCUpdate const & ugc) { - auto viewModel = self.data.reviewViewModel; - [viewModel setDefaultStarCount:starNumber]; - auto controller = [MWMUGCReviewController instanceFromViewModel:viewModel]; - [self.ownerViewController.navigationController pushViewController:controller animated:YES]; - }); + // TODO: Prepare ugc update. } #pragma mark - AvailableArea / PlacePageArea diff --git a/iphone/Maps/UI/PlacePage/PlacePageLayout/Content/UGC/MWMUGCReviewVM.mm b/iphone/Maps/UI/PlacePage/PlacePageLayout/Content/UGC/MWMUGCReviewVM.mm index 50cbdbe34a..a8cf52fbcd 100644 --- a/iphone/Maps/UI/PlacePage/PlacePageLayout/Content/UGC/MWMUGCReviewVM.mm +++ b/iphone/Maps/UI/PlacePage/PlacePageLayout/Content/UGC/MWMUGCReviewVM.mm @@ -34,31 +34,25 @@ - (void)config { - auto & records = m_ugc.m_ratings.m_ratings; -// Uncomment for testing. - records.emplace_back("Price", 0); - records.emplace_back("Place", 0); - - m_rows.insert(m_rows.begin(), records.size(), ugc_review::Row::Detail); - m_rows.emplace_back(ugc_review::Row::Message); + // TODO: Config controller with ugc. } - (void)setDefaultStarCount:(NSInteger)starCount { - for (auto & r : m_ugc.m_ratings.m_ratings) - r.m_value = starCount; + // TODO: Set stars count. } - (void)submit { - GetFramework().GetUGCApi().SetUGCUpdate(m_fid, m_ugc); + GetFramework().GetUGCApi()->SetUGCUpdate(m_fid, m_ugc); } - (NSInteger)numberOfRows { return m_rows.size(); } - (ugc::RatingRecord const &)recordForIndexPath:(NSIndexPath *)indexPath { - return m_ugc.m_ratings.m_ratings[indexPath.row]; + // TODO: Return rating record from ugc. + return ugc::RatingRecord(); } - (ugc_review::Row)rowForIndexPath:(NSIndexPath *)indexPath { return m_rows[indexPath.row]; } @@ -67,8 +61,7 @@ - (void)changeReviewRate:(NSInteger)rate atIndexPath:(NSIndexPath *)indexPath { - auto & record = m_ugc.m_ratings.m_ratings[indexPath.row]; - record.m_value = rate; + //TODO: Change review rate. } #pragma mark - MWMUGCTextReviewDelegate diff --git a/map/framework.cpp b/map/framework.cpp index 67a79d378a..151fe63715 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -861,7 +861,6 @@ void Framework::FillInfoFromFeatureType(FeatureType const & ft, place_page::Info buildingHolder.Assign(classif().GetTypeByPath({"building"})); info.SetLocalizedWifiString(m_stringsBundle.GetString("wifi")); - info.SetLocalizedRatingString(m_stringsBundle.GetString("place_page_booking_rating")); if (ftypes::IsAddressObjectChecker::Instance()(ft)) info.SetAddress(GetAddressInfoAtPoint(feature::GetCenter(ft)).FormatHouseAndStreet()); @@ -2494,6 +2493,7 @@ df::SelectionShape::ESelectedObject Framework::OnTapEventImpl(TapEvent const & t } outInfo.SetAdsEngine(m_adsEngine.get()); + outInfo.SetUGCApi(m_ugcApi.get()); UserMark const * mark = FindUserMarkInTapPosition(tapInfo); if (mark != nullptr) diff --git a/map/framework.hpp b/map/framework.hpp index 584ec28999..64be2fcf14 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -236,6 +236,9 @@ public: taxi::Engine * GetTaxiEngine(platform::NetworkPolicy const & policy); cian::Api * GetCianApi(platform::NetworkPolicy const & policy); locals::Api * GetLocalsApi(platform::NetworkPolicy const & policy); + ugc::Api * GetUGCApi() { return m_ugcApi.get(); } + ugc::Api const * GetUGCApi() const { return m_ugcApi.get(); } + df::DrapeApi & GetDrapeApi() { return m_drapeApi; } @@ -299,8 +302,6 @@ public: Index const & GetIndex() const { return m_model.GetIndex(); } - ugc::Api & GetUGCApi() { return *m_ugcApi; } - search::Engine & GetSearchEngine() { return *m_searchEngine; } search::Engine const & GetSearchEngine() const { return *m_searchEngine; } diff --git a/map/place_page_info.cpp b/map/place_page_info.cpp index 4037ce727c..07e89be729 100644 --- a/map/place_page_info.cpp +++ b/map/place_page_info.cpp @@ -11,12 +11,18 @@ #include "platform/preferred_languages.hpp" #include "platform/settings.hpp" +#include + namespace place_page { +namespace +{ +auto constexpr kTopRatingBound = 10.0f; +} // namespace + char const * const Info::kSubtitleSeparator = " • "; char const * const Info::kStarSymbol = "★"; char const * const Info::kMountainSymbol = "▲"; -char const * const Info::kEmptyRatingSymbol = "-"; char const * const Info::kPricingSymbol = "$"; char const * const kWheelchairSymbol = u8"\u267F"; @@ -196,23 +202,18 @@ string Info::GetFormattedCoordinate(bool isDMS) const : measurement_utils::FormatLatLonAsDMS(ll.lat, ll.lon, 2); } -string Info::GetRatingFormatted() const +float Info::GetRatingRawValue() const { - if (!IsSponsored()) - return string(); + if (!IsSponsored() && !ShouldShowUGC()) + return kIncorrectRating; - auto const r = GetMetadata().Get(feature::Metadata::FMD_RATING); - char const * rating = r.empty() ? kEmptyRatingSymbol : r.c_str(); - int const size = snprintf(nullptr, 0, m_localizedRatingString.c_str(), rating); - if (size < 0) - { - LOG(LERROR, ("Incorrect size for string:", m_localizedRatingString, ", rating:", rating)); - return string(); - } + // Only sponsored rating is stored in metadata. UGC rating will be stored in special section and will be ready soon. + auto const rating = GetMetadata().Get(feature::Metadata::FMD_RATING); + float raw; + if (!strings::to_float(rating, raw)) + return kIncorrectRating; - vector buf(size + 1); - snprintf(buf.data(), buf.size(), m_localizedRatingString.c_str(), rating); - return string(buf.begin(), buf.end()); + return raw; } string Info::GetApproximatePricing() const @@ -252,4 +253,44 @@ vector Info::GetBanners() const return m_adsEngine->GetBanners(m_types, m_topmostCountryIds, languages::GetCurrentNorm()); } + +namespace rating +{ +namespace +{ +std::string const kEmptyRatingSymbol = "-"; +} // namespace + +Impress GetImpress(float const rawRating) +{ + CHECK_LESS_OR_EQUAL(rawRating, kTopRatingBound, ()); + CHECK_GREATER_OR_EQUAL(rawRating, kIncorrectRating, ()); + + if (rawRating == kIncorrectRating) + return Impress::None; + if (rawRating < 0.2f * kTopRatingBound) + return Impress::Horrible; + if (rawRating < 0.4f * kTopRatingBound) + return Impress::Bad; + if (rawRating < 0.6f * kTopRatingBound) + return Impress::Normal; + if (rawRating < 0.8f * kTopRatingBound) + return Impress::Good; + + return Impress::Excellent; +} + +std::string GetRatingFormatted(float const rawRating) +{ + CHECK_LESS_OR_EQUAL(rawRating, kTopRatingBound, ()); + CHECK_GREATER_OR_EQUAL(rawRating, kIncorrectRating, ()); + + if (rawRating == kIncorrectRating) + return kEmptyRatingSymbol; + + std::ostringstream oss; + oss << std::setprecision(1) << rawRating; + return oss.str(); +} +} // namespace rating } // namespace place_page diff --git a/map/place_page_info.hpp b/map/place_page_info.hpp index 3d2201aaed..debe40310f 100644 --- a/map/place_page_info.hpp +++ b/map/place_page_info.hpp @@ -4,6 +4,8 @@ #include "partners_api/taxi_provider.hpp" +#include "ugc/api.hpp" + #include "map/routing_mark.hpp" #include "storage/index.hpp" @@ -52,13 +54,14 @@ enum class LocalsStatus Available }; +auto constexpr kIncorrectRating = -1.0f; + class Info : public osm::MapObject { public: static char const * const kSubtitleSeparator; static char const * const kStarSymbol; static char const * const kMountainSymbol; - static char const * const kEmptyRatingSymbol; static char const * const kPricingSymbol; /// Place traits @@ -75,6 +78,7 @@ public: /// UGC // TODO: Uncomment correct implementation. // TODO: UGC is disabled before UI isn't ready. + void SetUGCApi(ugc::Api * const api) { m_ugcApi = api; } bool ShouldShowUGC() const { return false; } //ftraits::UGC::IsUGCAvailable(m_types); } bool ShouldShowUGCRating() const { return false; } //ftraits::UGC::IsRatingAvailable(m_types); } bool ShouldShowUGCReviews() const { return false; } //ftraits::UGC::IsReviewsAvailable(m_types); } @@ -97,8 +101,8 @@ public: std::string const & GetAddress() const { return m_uiAddress; } /// @returns coordinate in DMS format if isDMS is true std::string GetFormattedCoordinate(bool isDMS) const; - /// @returns formatted rating string for booking object, or empty if it isn't booking object - std::string GetRatingFormatted() const; + /// @return rating raw value or kInvalidRating if there is no data. + float GetRatingRawValue() const; /// @returns string with |kPricingSymbol| signs or empty std::string if it isn't booking object std::string GetApproximatePricing() const; @@ -109,7 +113,6 @@ public: void SetIsMyPosition() { m_isMyPosition = true; } void SetCanEditOrAdd(bool canEditOrAdd) { m_canEditOrAdd = canEditOrAdd; } void SetLocalizedWifiString(std::string const & str) { m_localizedWifiString = str; } - void SetLocalizedRatingString(std::string const & str) { m_localizedRatingString = str; } /// Bookmark BookmarkAndCategory const & GetBookmarkAndCategory() const { return m_bac; } @@ -253,6 +256,9 @@ private: LocalAdsStatus m_localAdsStatus = LocalAdsStatus::NotAvailable; /// Ads source. ads::Engine * m_adsEngine = nullptr; + /// UGC + /// This is a non-owning pointer; + ugc::Api * m_ugcApi = nullptr; /// Sponsored type or None. SponsoredType m_sponsoredType = SponsoredType::None; @@ -268,4 +274,20 @@ private: std::string m_localsUrl; LocalsStatus m_localsStatus = LocalsStatus::NotAvailable; }; + +namespace rating +{ +enum Impress +{ + None, + Horrible, + Bad, + Normal, + Good, + Excellent +}; + +Impress GetImpress(float const rawRating); +std::string GetRatingFormatted(float const rawRating); +} // namespace rating } // namespace place_page diff --git a/ugc/api.cpp b/ugc/api.cpp index 98d85ea2dd..b2e6af9aa5 100644 --- a/ugc/api.cpp +++ b/ugc/api.cpp @@ -24,11 +24,6 @@ void Api::GetUGC(FeatureID const & id, UGCCallback callback) m_thread.Push([=] { GetUGCImpl(id, callback); }); } -void Api::GetUGCUpdate(FeatureID const & id, UGCUpdateCallback callback) -{ - m_thread.Push([=] { GetUGCUpdateImpl(id, callback); }); -} - void Api::SetUGCUpdate(FeatureID const & id, UGCUpdate const & ugc) { m_thread.Push([=] { SetUGCUpdate(id, ugc); }); @@ -37,73 +32,67 @@ void Api::SetUGCUpdate(FeatureID const & id, UGCUpdate const & ugc) // static UGC Api::MakeTestUGC1(Time now) { - Rating rating; - rating.m_ratings.emplace_back("food" /* key */, 4.0 /* value */); - rating.m_ratings.emplace_back("service" /* key */, 5.0 /* value */); - rating.m_ratings.emplace_back("music" /* key */, 5.0 /* value */); - rating.m_aggValue = 4.5; + 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 */); - vector reviews; + Reviews reviews; reviews.emplace_back(20 /* id */, Text("Damn good coffee", StringUtf8Multilang::kEnglishCode), Author(UID(987654321 /* hi */, 123456789 /* lo */), "Cole"), - 5.0 /* rating */, Sentiment::Positive, FromDaysAgo(now, 10)); - reviews.emplace_back(67812 /* id */, - Text("Clean place, reasonably priced", StringUtf8Multilang::kDefaultCode), - Author(UID(0 /* hi */, 315 /* lo */), "Cooper"), 5.0 /* rating */, - Sentiment::Positive, FromDaysAgo(now, 1)); + 5.0 /* rating */, FromDaysAgo(now, 10)); + reviews.emplace_back( + 67812 /* id */, Text("Clean place, reasonably priced", StringUtf8Multilang::kDefaultCode), + Author(UID(0 /* hi */, 315 /* lo */), "Cooper"), 5.0 /* rating */, FromDaysAgo(now, 1)); - vector attributes; - attributes.emplace_back("best-drink", "Coffee"); - - return UGC(rating, reviews, attributes); + return UGC(records, reviews, 4.5 /* rating */); } // static UGC Api::MakeTestUGC2(Time now) { - Rating rating; - rating.m_ratings.emplace_back("food" /* key */, 5.0 /* value */); - rating.m_ratings.emplace_back("service" /* key */, 5.0 /* value */); - rating.m_ratings.emplace_back("music" /* key */, 5.0 /* value */); - rating.m_aggValue = 5.0; + Ratings records; + records.emplace_back("food" /* key */, 5.0 /* value */); + records.emplace_back("service" /* key */, 5.0 /* value */); + records.emplace_back("music" /* key */, 5.0 /* value */); vector reviews; - reviews.emplace_back(119 /* id */, - Text("This pie's so good it is a crime", StringUtf8Multilang::kDefaultCode), - Author(UID(0 /* hi */, 315 /* lo */), "Cooper"), 5.0 /* rating */, - Sentiment::Positive, FromDaysAgo(now, 1)); + reviews.emplace_back( + 119 /* id */, Text("This pie's so good it is a crime", StringUtf8Multilang::kDefaultCode), + Author(UID(0 /* hi */, 315 /* lo */), "Cooper"), 5.0 /* rating */, FromDaysAgo(now, 1)); - vector attributes; - attributes.emplace_back("best-drink", "Coffee"); - attributes.emplace_back("best-meal", "Cherry Pie"); + return UGC(records, reviews, 5.0 /* rating */); +} - return UGC(rating, reviews, attributes); +// static +UGCUpdate Api::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 */); + + Text text{"It's aways nice to visit this place", StringUtf8Multilang::kEnglishCode}; + + Time time{FromDaysAgo(now, 1)}; + + return UGCUpdate(records, text, time); } void Api::GetUGCImpl(FeatureID const & id, UGCCallback callback) { // TODO (@y, @mgsergio): retrieve static UGC - UGC ugc(Rating({}, {}), {}, {}); + UGC ugc; + UGCUpdate update; if (!id.IsValid()) { - GetPlatform().RunOnGuiThread([ugc, callback] { callback(ugc); }); + GetPlatform().RunOnGuiThread([ugc, update, callback] { callback(ugc, update); }); return; } ugc = MakeTestUGC1(); - - GetPlatform().RunOnGuiThread([ugc, callback] { callback(ugc); }); -} - -void Api::GetUGCUpdateImpl(FeatureID const & /* id */, UGCUpdateCallback callback) -{ - // TODO (@y, @mgsergio): retrieve dynamic UGC - UGCUpdate ugc(Rating({}, {}), - Attribute({}, {}), - ReviewAbuse({}, {}), - ReviewFeedback({}, {})); - GetPlatform().RunOnGuiThread([ugc, callback] { callback(ugc); }); + GetPlatform().RunOnGuiThread([ugc, update, callback] { callback(ugc, update); }); } void Api::SetUGCUpdateImpl(FeatureID const & id, UGCUpdate const & ugc) diff --git a/ugc/api.hpp b/ugc/api.hpp index 67961893c2..d4e8c3af7a 100644 --- a/ugc/api.hpp +++ b/ugc/api.hpp @@ -15,22 +15,20 @@ namespace ugc class Api { public: - using UGCCallback = std::function; - using UGCUpdateCallback = std::function; + using UGCCallback = std::function; explicit Api(Index const & index, std::string const & filename); void GetUGC(FeatureID const & id, UGCCallback callback); - void GetUGCUpdate(FeatureID const & id, UGCUpdateCallback callback); void SetUGCUpdate(FeatureID const & id, UGCUpdate const & ugc); static UGC MakeTestUGC1(Time now = Clock::now()); static UGC MakeTestUGC2(Time now = Clock::now()); + static UGCUpdate MakeTestUGCUpdate(Time now = Clock::now()); private: void GetUGCImpl(FeatureID const & id, UGCCallback callback); - void GetUGCUpdateImpl(FeatureID const & id, UGCUpdateCallback callback); void SetUGCUpdateImpl(FeatureID const & id, UGCUpdate const & ugc); diff --git a/ugc/serdes.hpp b/ugc/serdes.hpp index 1e5a90807b..9f514988af 100644 --- a/ugc/serdes.hpp +++ b/ugc/serdes.hpp @@ -167,7 +167,7 @@ private: Source & m_source; }; -template +template void Serialize(Sink & sink, UGC const & ugc) { WriteToSink(sink, static_cast(Version::Latest)); @@ -175,7 +175,7 @@ void Serialize(Sink & sink, UGC const & ugc) ser(ugc); } -template +template void Deserialize(Source & source, UGC & ugc) { uint8_t version = 0; diff --git a/ugc/types.hpp b/ugc/types.hpp index 7afd1ae874..5e26c6cd73 100644 --- a/ugc/types.hpp +++ b/ugc/types.hpp @@ -108,6 +108,8 @@ struct RatingRecord float m_value{}; }; +using Ratings = std::vector; + struct Rating { Rating() = default; @@ -202,19 +204,18 @@ struct Review Review() = default; Review(ReviewId id, Text const & text, Author const & author, float const rating, - Sentiment const sentiment, Time const & time) - : m_text(text), m_author(author), m_rating(rating), m_sentiment(sentiment), m_time(time) + Time const & time) + : m_text(text), m_author(author), m_rating(rating), m_time(time) { } DECLARE_VISITOR(visitor(m_id, "id"), visitor(m_text, "text"), visitor(m_author, "author"), - visitor.VisitRating(m_rating, "rating"), visitor(m_sentiment, "sentiment"), - visitor(m_time, "time")) + visitor.VisitRating(m_rating, "rating"), visitor(m_time, "time")) bool operator==(Review const & rhs) const { return m_id == rhs.m_id && m_text == rhs.m_text && m_author == rhs.m_author && - m_rating == rhs.m_rating && m_sentiment == rhs.m_sentiment && m_time == rhs.m_time; + m_rating == rhs.m_rating && m_time == rhs.m_time; } friend std::string DebugPrint(Review const & review) @@ -225,7 +226,6 @@ struct Review os << "text:" << DebugPrint(review.m_text) << ", "; os << "author:" << DebugPrint(review.m_author) << ", "; os << "rating:" << review.m_rating << ", "; - os << "sentiment:" << DebugPrint(review.m_sentiment) << ", "; os << "days since epoch:" << ToDaysSinceEpoch(review.m_time) << " ]"; return os.str(); } @@ -233,14 +233,12 @@ struct Review ReviewId m_id{}; Text m_text{}; Author m_author{}; - // A rating of a review itself. It is accumulated from other users - // likes or dislikes. float m_rating{}; - // A positive/negative sentiment given to a place by an author. - Sentiment m_sentiment = Sentiment::Positive; Time m_time{}; }; +using Reviews = std::vector; + struct Attribute { Attribute() = default; @@ -270,34 +268,61 @@ struct Attribute struct UGC { UGC() = default; - UGC(Rating const & rating, std::vector const & reviews, - std::vector const & attributes) - : m_rating(rating), m_reviews(reviews), m_attributes(attributes) + UGC(Ratings const & records, Reviews const & reviews, float const rating) + : m_ratings(records), m_reviews(reviews), m_aggRating(rating) { } - DECLARE_VISITOR(visitor(m_rating, "rating"), visitor(m_reviews, "reviews"), - visitor(m_attributes, "attributes")) + DECLARE_VISITOR(visitor(m_ratings, "ratings"), visitor(m_reviews, "reviews"), + visitor.VisitRating(m_aggRating, "aggRating")) bool operator==(UGC const & rhs) const { - return m_rating == rhs.m_rating && m_reviews == rhs.m_reviews && - m_attributes == rhs.m_attributes; + return m_ratings == rhs.m_ratings && m_reviews == rhs.m_reviews; } friend std::string DebugPrint(UGC const & ugc) { std::ostringstream os; os << "UGC [ "; - os << "rating:" << DebugPrint(ugc.m_rating) << ", "; - os << "reviews:" << ::DebugPrint(ugc.m_reviews) << ", "; - os << "attributes:" << ::DebugPrint(ugc.m_attributes) << " ]"; + os << "records:" << ::DebugPrint(ugc.m_ratings) << ", "; + os << "reviews:" << ::DebugPrint(ugc.m_reviews) << " ]"; return os.str(); } - Rating m_rating{}; - std::vector m_reviews; - std::vector m_attributes; + Ratings m_ratings; + Reviews m_reviews; + float m_aggRating{}; +}; + +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, "time")) + + bool operator==(UGCUpdate const & rhs) const + { + return m_ratings == rhs.m_ratings && m_text == rhs.m_text && m_time == rhs.m_time; + } + + friend 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(); + } + + Ratings m_ratings; + Text m_text; + Time m_time{}; }; struct ReviewFeedback @@ -320,21 +345,6 @@ struct ReviewAbuse std::string m_reason{}; Time m_time{}; }; - -struct UGCUpdate -{ - UGCUpdate() = default; - UGCUpdate(Rating ratings, Attribute attribute, ReviewAbuse abuses, ReviewFeedback feedbacks) - : m_ratings(ratings), m_attribute(attribute), m_abuses(abuses), m_feedbacks(feedbacks) - { - } - - Rating m_ratings; - Attribute m_attribute; - - ReviewAbuse m_abuses; - ReviewFeedback m_feedbacks; -}; } // namespace ugc #undef DECLARE_VISITOR diff --git a/ugc/ugc_tests/serdes_binary_tests.cpp b/ugc/ugc_tests/serdes_binary_tests.cpp index b0266d873e..dd358fbdd6 100644 --- a/ugc/ugc_tests/serdes_binary_tests.cpp +++ b/ugc/ugc_tests/serdes_binary_tests.cpp @@ -16,8 +16,7 @@ namespace { vector const GetExpectedTranslationKeys() { - vector keys = { - {"food", "service", "music", "best-drink", "best-meal", "Coffee", "Cherry Pie"}}; + vector keys = {{"food", "service", "music"}}; sort(keys.begin(), keys.end()); return keys; } diff --git a/ugc/ugc_tests/serdes_tests.cpp b/ugc/ugc_tests/serdes_tests.cpp index 8271ec8e98..0bda71fbcc 100644 --- a/ugc/ugc_tests/serdes_tests.cpp +++ b/ugc/ugc_tests/serdes_tests.cpp @@ -84,9 +84,9 @@ UNIT_TEST(SerDes_Json_Reviews) MakeTest(expectedUGC); } -UNIT_TEST(SerDes_Json_Attributes) +UNIT_TEST(SerDes_Json_RatingRecords) { - auto expectedUGC = Api::MakeTestUGC1(Time(chrono::hours(24 * 100))).m_attributes; + auto expectedUGC = Api::MakeTestUGC1(Time(chrono::hours(24 * 100))).m_ratings; TEST_EQUAL(expectedUGC, expectedUGC, ()); MakeTest(expectedUGC); @@ -122,4 +122,34 @@ UNIT_TEST(SerDes_UGC) TEST_EQUAL(expectedUGC, actualUGC, ()); } + +UNIT_TEST(SerDes_Json_UGCUpdate) +{ + auto expectedUGC = Api::MakeTestUGCUpdate(Time(chrono::hours(24 * 200))); + TEST_EQUAL(expectedUGC, expectedUGC, ()); + + MakeTest(expectedUGC); +} + +UNIT_TEST(SerDes_UGCUpdate) +{ + // Time must be in whole days to prevent lose of precision during + // serialization/deserialization. + auto const expectedUGC = Api::MakeTestUGCUpdate(Time(chrono::hours(24 * 300))); + TEST_EQUAL(expectedUGC, expectedUGC, ()); + + Buffer buffer; + { + auto sink = MakeSink(buffer); + Serialize(sink, expectedUGC); + } + + UGCUpdate actualUGC{}; + { + auto source = MakeSource(buffer); + Deserialize(source, actualUGC); + } + + TEST_EQUAL(expectedUGC, actualUGC, ()); +} } // namespace