From d730ce9466a6eef3a64c2688ce023550181d171b Mon Sep 17 00:00:00 2001 From: Alexander Borsuk Date: Sun, 10 Nov 2024 16:28:45 +0100 Subject: [PATCH 01/13] Minor warning fixes Signed-off-by: Alexander Borsuk --- drape_frontend/read_manager.cpp | 2 +- editor/editor_tests/osm_editor_test.cpp | 2 +- geometry/geometry_tests/bounding_box_tests.cpp | 11 ++++------- geometry/geometry_tests/diamond_box_tests.cpp | 13 +++++-------- geometry/geometry_tests/robust_test.cpp | 7 +++---- search/ranking_utils.hpp | 2 +- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/drape_frontend/read_manager.cpp b/drape_frontend/read_manager.cpp index 49e1d255aa..44b6fac7ad 100755 --- a/drape_frontend/read_manager.cpp +++ b/drape_frontend/read_manager.cpp @@ -49,11 +49,11 @@ ReadManager::ReadManager(ref_ptr commutator, MapDataProvider , m_trafficEnabled(trafficEnabled) , m_isolinesEnabled(isolinesEnabled) , m_modeChanged(false) + , m_mapLangIndex(StringUtf8Multilang::kDefaultCode) , m_tasksPool(64, ReadMWMTaskFactory(m_model)) , m_counter(0) , m_generationCounter(0) , m_userMarksGenerationCounter(0) - , m_mapLangIndex(StringUtf8Multilang::kDefaultCode) { Start(); } diff --git a/editor/editor_tests/osm_editor_test.cpp b/editor/editor_tests/osm_editor_test.cpp index 3a22748220..fb82cc84de 100644 --- a/editor/editor_tests/osm_editor_test.cpp +++ b/editor/editor_tests/osm_editor_test.cpp @@ -246,7 +246,7 @@ void EditorTest::GetFeatureTypeInfoTest() auto const featuresAfter = editor.m_features.Get(); auto const fti = editor.GetFeatureTypeInfo(*featuresAfter, ft.GetID().m_mwmId, ft.GetID().m_index); - TEST_NOT_EQUAL(fti, 0, ()); + TEST_NOT_EQUAL(fti, nullptr, ()); TEST_EQUAL(fti->m_object.GetID(), ft.GetID(), ()); }); } diff --git a/geometry/geometry_tests/bounding_box_tests.cpp b/geometry/geometry_tests/bounding_box_tests.cpp index feb18f001c..994068fef8 100644 --- a/geometry/geometry_tests/bounding_box_tests.cpp +++ b/geometry/geometry_tests/bounding_box_tests.cpp @@ -1,23 +1,20 @@ #include "testing/testing.hpp" #include "geometry/bounding_box.hpp" -#include "geometry/point2d.hpp" -using namespace m2; - -namespace +namespace bounding_box_tests { UNIT_TEST(BoundingBox_Smoke) { { - BoundingBox bbox; + m2::BoundingBox bbox; TEST(!bbox.HasPoint(0, 0), ()); TEST(!bbox.HasPoint(-1, 1), ()); } { - BoundingBox bbox; + m2::BoundingBox bbox; bbox.Add(0, 0); TEST(bbox.HasPoint(0, 0), ()); @@ -33,4 +30,4 @@ UNIT_TEST(BoundingBox_Smoke) TEST(bbox.HasPoint(0.5, 0.5), ()); } } -} // namespace +} // namespace bounding_box_tests \ No newline at end of file diff --git a/geometry/geometry_tests/diamond_box_tests.cpp b/geometry/geometry_tests/diamond_box_tests.cpp index a3ed68f0e4..4c0b16aa3e 100644 --- a/geometry/geometry_tests/diamond_box_tests.cpp +++ b/geometry/geometry_tests/diamond_box_tests.cpp @@ -1,21 +1,18 @@ #include "testing/testing.hpp" #include "geometry/diamond_box.hpp" -#include "geometry/point2d.hpp" -using namespace m2; - -namespace +namespace diamond_box_tests { UNIT_TEST(DiamondBox_Smoke) { { - DiamondBox dbox; + m2::DiamondBox dbox; TEST(!dbox.HasPoint(0, 0), ()); } { - DiamondBox dbox; + m2::DiamondBox dbox; dbox.Add(0, 0); TEST(dbox.HasPoint(0, 0), ()); TEST(!dbox.HasPoint(0, 1), ()); @@ -32,7 +29,7 @@ UNIT_TEST(DiamondBox_Smoke) } { - DiamondBox dbox; + m2::DiamondBox dbox; dbox.Add(0, 1); dbox.Add(0, -1); @@ -50,4 +47,4 @@ UNIT_TEST(DiamondBox_Smoke) TEST(!dbox.HasPoint(-0.51, -0.51), ()); } } -} // namespace +} // namespace diamond_box_tests diff --git a/geometry/geometry_tests/robust_test.cpp b/geometry/geometry_tests/robust_test.cpp index c04be35f79..aaf1db368e 100644 --- a/geometry/geometry_tests/robust_test.cpp +++ b/geometry/geometry_tests/robust_test.cpp @@ -6,10 +6,9 @@ #include -using namespace m2::robust; - -namespace +namespace robust_test { +using namespace m2::robust; using P = m2::PointD; template @@ -29,7 +28,6 @@ bool InsideTriangle(P const & p, P const ps[]) { return IsPointInsideTriangle(p, ps[0], ps[1], ps[2]); } -} // namespace UNIT_TEST(OrientedS_Smoke) { @@ -170,3 +168,4 @@ UNIT_TEST(PolygonSelfIntersections_TangentSmoke) CheckSelfIntersections(&arr[0], arr + ARRAY_SIZE(arr), false); } } +} // namespace robust_test \ No newline at end of file diff --git a/search/ranking_utils.hpp b/search/ranking_utils.hpp index 81adbf0952..069d3f3c4e 100644 --- a/search/ranking_utils.hpp +++ b/search/ranking_utils.hpp @@ -165,7 +165,7 @@ struct NameScores m_errorsMade = ErrorsMade::Min(m_errorsMade, rhs.m_errorsMade); } - bool operator==(NameScores const & rhs) + bool operator==(NameScores const & rhs) const { return m_nameScore == rhs.m_nameScore && m_errorsMade == rhs.m_errorsMade && m_isAltOrOldName == rhs.m_isAltOrOldName && m_matchedLength == rhs.m_matchedLength; -- 2.45.3 From 7568b882b438f864b43015a0f02ea031dc85d78d Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 21 Nov 2024 13:53:17 -0300 Subject: [PATCH 02/13] Print settings.ini to the log. Signed-off-by: Viktor Govako --- platform/string_storage_base.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/platform/string_storage_base.cpp b/platform/string_storage_base.cpp index 0f2f4a4df4..b8951e9baf 100644 --- a/platform/string_storage_base.cpp +++ b/platform/string_storage_base.cpp @@ -38,7 +38,10 @@ StringStorageBase::StringStorageBase(std::string const & path) : m_path(path) std::string key = line.substr(0, delimPos); std::string value = line.substr(delimPos + 1); if (!key.empty() && !value.empty()) + { + LOG(LINFO, (key, ":", value)); VERIFY(m_values.emplace(std::move(key), std::move(value)).second, ()); + } } } catch (RootException const & ex) -- 2.45.3 From 457288fab6184dab913dccf6e4016e9217e467ca Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 21 Nov 2024 13:53:36 -0300 Subject: [PATCH 03/13] Minor code style fixes. Signed-off-by: Viktor Govako --- .../app/organicmaps/search/SearchResult.java | 10 ++++++---- search/highlighting.cpp | 20 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/android/app/src/main/java/app/organicmaps/search/SearchResult.java b/android/app/src/main/java/app/organicmaps/search/SearchResult.java index d2be945e8d..f903188ece 100644 --- a/android/app/src/main/java/app/organicmaps/search/SearchResult.java +++ b/android/app/src/main/java/app/organicmaps/search/SearchResult.java @@ -122,12 +122,14 @@ public class SearchResult return title; } - public void formatText(SpannableStringBuilder builder, int[] ranges) { - if (ranges != null) { + public void formatText(SpannableStringBuilder builder, int[] ranges) + { + if (ranges != null) + { final int size = ranges.length / 2; int index = 0; - - for (int i = 0; i < size; i++) { + for (int i = 0; i < size; i++) + { final int start = ranges[index++]; final int len = ranges[index++]; diff --git a/search/highlighting.cpp b/search/highlighting.cpp index 2469304cb6..f876b97e13 100644 --- a/search/highlighting.cpp +++ b/search/highlighting.cpp @@ -1,7 +1,7 @@ #include "search/highlighting.hpp" -using namespace std; - +namespace search +{ namespace { // Makes continuous range for tokens and prefix. @@ -42,8 +42,7 @@ public: }; } // namespace -namespace search -{ + void HighlightResult(QueryTokens const & tokens, strings::UniString const & prefix, Result & res) { using Iter = QueryTokens::const_iterator; @@ -52,15 +51,16 @@ void HighlightResult(QueryTokens const & tokens, strings::UniString const & pref CombinedIter beg(tokens.begin(), tokens.end(), prefix.empty() ? nullptr : &prefix); CombinedIter end(tokens.end() /* cur */, tokens.end() /* end */, nullptr); - // Highlight Title - SearchStringTokensIntersectionRanges(res.GetString(), beg, end, - [&](pair const &range) { + // Highlight Title + SearchStringTokensIntersectionRanges(res.GetString(), beg, end, + [&](std::pair const & range) + { res.AddHighlightRange(range); - }); + }); // Highlight description. - SearchStringTokensIntersectionRanges(res.GetAddress(), beg, end, - [&](pair const &range) + SearchStringTokensIntersectionRanges(res.GetAddress(), beg, end, + [&](std::pair const & range) { res.AddDescHighlightRange(range); }); -- 2.45.3 From 8e885b84378124b3e05d146905dc620a6e5286e4 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 21 Nov 2024 13:54:07 -0300 Subject: [PATCH 04/13] [android] Fixed bug with search history order. Signed-off-by: Viktor Govako --- .../app/src/main/java/app/organicmaps/search/SearchRecents.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/app/src/main/java/app/organicmaps/search/SearchRecents.java b/android/app/src/main/java/app/organicmaps/search/SearchRecents.java index 6591df5f9d..1b115613ff 100644 --- a/android/app/src/main/java/app/organicmaps/search/SearchRecents.java +++ b/android/app/src/main/java/app/organicmaps/search/SearchRecents.java @@ -39,7 +39,7 @@ public final class SearchRecents public static boolean add(@NonNull String query, @NonNull Context context) { - if (TextUtils.isEmpty(query) || sRecents.contains(query)) + if (TextUtils.isEmpty(query)) return false; nativeAdd(Language.getKeyboardLocale(context), query); -- 2.45.3 From 326d1d5e5fe5987e3509d6cd7e48c239598a23a2 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 21 Nov 2024 13:54:26 -0300 Subject: [PATCH 05/13] [android] Avoid intermediate List. Signed-off-by: Viktor Govako --- .../app/src/main/cpp/app/organicmaps/SearchRecents.cpp | 9 ++------- .../main/java/app/organicmaps/search/SearchRecents.java | 8 ++------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/android/app/src/main/cpp/app/organicmaps/SearchRecents.cpp b/android/app/src/main/cpp/app/organicmaps/SearchRecents.cpp index e7827ffeed..9d5e9f818a 100644 --- a/android/app/src/main/cpp/app/organicmaps/SearchRecents.cpp +++ b/android/app/src/main/cpp/app/organicmaps/SearchRecents.cpp @@ -16,17 +16,12 @@ extern "C" if (items.empty()) return; - auto const & pairBuilder = jni::PairBuilder::Instance(env); auto const listAddMethod = jni::ListBuilder::Instance(env).m_add; for (SearchRequest const & item : items) { - using SLR = jni::TScopedLocalRef; - SLR pair(env, pairBuilder.Create(env, SLR(env, jni::ToJavaString(env, item.first)), - SLR(env, jni::ToJavaString(env, item.second)))); - ASSERT(pair.get(), (jni::DescribeException())); - - env->CallBooleanMethod(result, listAddMethod, pair.get()); + jni::TScopedLocalRef str(env, jni::ToJavaString(env, item.second)); + env->CallBooleanMethod(result, listAddMethod, str.get()); } } diff --git a/android/app/src/main/java/app/organicmaps/search/SearchRecents.java b/android/app/src/main/java/app/organicmaps/search/SearchRecents.java index 1b115613ff..54029e8683 100644 --- a/android/app/src/main/java/app/organicmaps/search/SearchRecents.java +++ b/android/app/src/main/java/app/organicmaps/search/SearchRecents.java @@ -19,12 +19,8 @@ public final class SearchRecents public static void refresh() { - final List> pairs = new ArrayList<>(); - nativeGetList(pairs); sRecents.clear(); - - for (Pair pair : pairs) - sRecents.add(pair.second); + nativeGetList(sRecents); } public static int getSize() @@ -53,7 +49,7 @@ public final class SearchRecents sRecents.clear(); } - private static native void nativeGetList(List> result); + private static native void nativeGetList(List result); private static native void nativeAdd(String locale, String query); private static native void nativeClear(); } -- 2.45.3 From 85805018673adfca7a84ac635d012d38d544e0e5 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Thu, 21 Nov 2024 22:07:38 -0300 Subject: [PATCH 06/13] Better check in Storage::IsNodeDownloaded. Signed-off-by: Viktor Govako --- storage/storage.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/storage/storage.cpp b/storage/storage.cpp index 6c8d12a895..7c25ffa9de 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -1267,12 +1267,9 @@ bool Storage::IsNodeDownloaded(CountryId const & countryId) const { CHECK_THREAD_CHECKER(m_threadChecker, ()); - for (auto const & localeMap : m_localFiles) - { - if (countryId == localeMap.first) - return true; - } - return false; + auto const it = m_localFiles.find(countryId); + /// @todo IDK what is the logic here, but other functions also check on empty list. + return (it != m_localFiles.end() && !it->second.empty()); } bool Storage::HasLatestVersion(CountryId const & countryId) const @@ -1283,7 +1280,7 @@ bool Storage::HasLatestVersion(CountryId const & countryId) const bool Storage::IsAllowedToEditVersion(CountryId const & countryId) const { auto const status = CountryStatusEx(countryId); - switch (status) + switch (status) { case Status::OnDisk: return true; case Status::OnDiskOutOfDate: @@ -1292,7 +1289,7 @@ bool Storage::IsAllowedToEditVersion(CountryId const & countryId) const ASSERT(localFile, ("Local file shouldn't be nullptr.")); auto const currentVersionTime = base::YYMMDDToSecondsSinceEpoch(static_cast(m_currentVersion)); auto const localVersionTime = base::YYMMDDToSecondsSinceEpoch(static_cast(localFile->GetVersion())); - return currentVersionTime - localVersionTime < kMaxSecondsTillLastVersionUpdate && + return currentVersionTime - localVersionTime < kMaxSecondsTillLastVersionUpdate && base::SecondsSinceEpoch() - localVersionTime < kMaxSecondsTillNoEdits; } default: return false; @@ -1350,10 +1347,11 @@ void Storage::DeleteNode(CountryId const & countryId) if (!node) return; - auto deleteAction = [this](CountryTree::Node const & descendantNode) { - bool onDisk = m_localFiles.find(descendantNode.Value().Name()) != m_localFiles.end(); + auto const deleteAction = [this](CountryTree::Node const & descendantNode) + { + bool const onDisk = m_localFiles.find(descendantNode.Value().Name()) != m_localFiles.end(); if (descendantNode.ChildrenCount() == 0 && onDisk) - this->DeleteCountry(descendantNode.Value().Name(), MapFileType::Map); + DeleteCountry(descendantNode.Value().Name(), MapFileType::Map); }; node->ForEachInSubtree(deleteAction); } @@ -1784,7 +1782,8 @@ Progress Storage::CalculateProgress(CountriesVec const & descendants) const void Storage::UpdateNode(CountryId const & countryId) { - ForEachInSubtree(countryId, [this](CountryId const & descendantId, bool groupNode) { + ForEachInSubtree(countryId, [this](CountryId const & descendantId, bool groupNode) + { if (!groupNode && m_localFiles.find(descendantId) != m_localFiles.end()) DownloadNode(descendantId, true /* isUpdate */); }); -- 2.45.3 From 545caccbec9cf824cc8268f96380b3fc71d50f1b Mon Sep 17 00:00:00 2001 From: Andrew Shkrob Date: Sat, 23 Nov 2024 23:22:29 +0100 Subject: [PATCH 07/13] [xcode] Revert swift version setting for CoreApi Signed-off-by: Andrew Shkrob --- iphone/CoreApi/CoreApi.xcodeproj/project.pbxproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/iphone/CoreApi/CoreApi.xcodeproj/project.pbxproj b/iphone/CoreApi/CoreApi.xcodeproj/project.pbxproj index 7d28f25775..459a51a4c3 100644 --- a/iphone/CoreApi/CoreApi.xcodeproj/project.pbxproj +++ b/iphone/CoreApi/CoreApi.xcodeproj/project.pbxproj @@ -702,8 +702,6 @@ PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)"; PROVISIONING_PROFILE_SPECIFIER = ""; SKIP_INSTALL = YES; - SWIFT_OPTIMIZATION_LEVEL = "-Onone"; - SWIFT_VERSION = 6.0; }; name = Debug; }; @@ -729,7 +727,6 @@ PRODUCT_NAME = "$(TARGET_NAME:c99extidentifier)"; PROVISIONING_PROFILE_SPECIFIER = ""; SKIP_INSTALL = YES; - SWIFT_VERSION = 6.0; }; name = Release; }; -- 2.45.3 From 88a9ea7f4f3971bb9c2339573708d1278e8494ed Mon Sep 17 00:00:00 2001 From: Aquatica <81624781+Aquathing@users.noreply.github.com> Date: Sun, 24 Nov 2024 11:35:58 +0100 Subject: [PATCH 08/13] Add aliases for Police (#9674) (#9693) * Add aliases for Police in Italy * Remove useless "Posto di Polizia" Signed-off-by: Aquatica <81624781+Aquathing@users.noreply.github.com> Co-authored-by: Konstantin Pastbin --- data/categories.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/categories.txt b/data/categories.txt index 12f3f2b9e8..582a1cd2fc 100644 --- a/data/categories.txt +++ b/data/categories.txt @@ -9618,7 +9618,7 @@ cs:4bezpečnost de:Polizeiwache|Polizei fr:Poste de police|Commissariat hi:3थाना -it:Posto di polizia +it:6Commissariato|5Questura|4Caserma|4Guardia|5Carabinieri ja:1屯所|ポリス|交番|お巡りさん|おまわりさん|通報 ko:보안대 pl:Policja|komisariat|posterunek -- 2.45.3 From 26864888a3fbd67a4f524751b8324f7d98b96481 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sat, 23 Nov 2024 15:02:13 -0300 Subject: [PATCH 09/13] [drape] Fixed arrow drawing for dense polyline. Signed-off-by: Viktor Govako --- drape_frontend/route_shape.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drape_frontend/route_shape.cpp b/drape_frontend/route_shape.cpp index 4c61d20e22..67026fb671 100644 --- a/drape_frontend/route_shape.cpp +++ b/drape_frontend/route_shape.cpp @@ -80,7 +80,7 @@ std::vector CalculatePoints(m2::PolylineD const & polyline, for (size_t i = 0; i + 1 < path.size(); i++) { double const dist = (path[i + 1] - path[i]).Length(); - if (fabs(dist) < 1e-5) + if (dist == 0) continue; double const l = len + dist; -- 2.45.3 From 2b62ca9cb9cd626efcd23f7466816b6aafe3d238 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sat, 23 Nov 2024 15:03:10 -0300 Subject: [PATCH 10/13] [drape] Minor code prettify. Signed-off-by: Viktor Govako --- drape_frontend/route_shape.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/drape_frontend/route_shape.cpp b/drape_frontend/route_shape.cpp index 67026fb671..1e7d502d23 100644 --- a/drape_frontend/route_shape.cpp +++ b/drape_frontend/route_shape.cpp @@ -62,11 +62,10 @@ void GetArrowTextureRegion(ref_ptr textures, textures->GetSymbolRegion("route-arrow", region); } -std::vector CalculatePoints(m2::PolylineD const & polyline, - double start, double end) +void CalculatePoints(m2::PolylineD const & polyline, double start, double end, + std::vector & result) { - std::vector result; - result.reserve(polyline.GetSize() / 4); + result.clear(); auto addIfNotExist = [&result](m2::PointD const & pnt) { @@ -74,12 +73,15 @@ std::vector CalculatePoints(m2::PolylineD const & polyline, result.push_back(pnt); }; - std::vector const & path = polyline.GetPoints(); + auto const & path = polyline.GetPoints(); double len = 0; bool started = false; - for (size_t i = 0; i + 1 < path.size(); i++) + for (size_t i = 1; i < path.size(); ++i) { - double const dist = (path[i + 1] - path[i]).Length(); + auto const & p1 = path[i - 1]; + auto const & p2 = path[i]; + auto const vec = p2 - p1; + double const dist = vec.Length(); if (dist == 0) continue; @@ -87,7 +89,7 @@ std::vector CalculatePoints(m2::PolylineD const & polyline, if (!started && start >= len && start <= l) { double const k = (start - len) / dist; - addIfNotExist(path[i] + (path[i + 1] - path[i]) * k); + addIfNotExist(p1 + vec * k); started = true; } if (!started) @@ -99,16 +101,15 @@ std::vector CalculatePoints(m2::PolylineD const & polyline, if (end >= len && end <= l) { double const k = (end - len) / dist; - addIfNotExist(path[i] + (path[i + 1] - path[i]) * k); + addIfNotExist(p1 + vec * k); break; } else { - addIfNotExist(path[i + 1]); + addIfNotExist(p2); } len = l; } - return result; } float SideByNormal(glsl::vec2 const & normal, bool isLeft) @@ -516,13 +517,14 @@ void RouteShape::CacheRouteArrows(ref_ptr context, // Generate arrow geometry. auto depth = static_cast(baseDepthIndex * rs::kDepthPerSubroute) + rs::kArrowsDepth; float const depthStep = (rs::kArrowsDepth - rs::kRouteDepth) / (1 + borders.size()); + + std::vector points; + points.reserve(polyline.GetSize() / 4); for (ArrowBorders const & b : borders) { depth -= depthStep; - std::vector points = rs::CalculatePoints(polyline, b.m_startDistance, b.m_endDistance); - ASSERT_LESS_OR_EQUAL(points.size(), polyline.GetSize(), ()); - PrepareArrowGeometry(points, routeArrowsData.m_pivot, region.GetTexRect(), depthStep, - depth, geometryData); + rs::CalculatePoints(polyline, b.m_startDistance, b.m_endDistance, points); + PrepareArrowGeometry(points, routeArrowsData.m_pivot, region.GetTexRect(), depthStep, depth, geometryData); } geometryData.m_boundingBox.Scale(kBoundingBoxScale); -- 2.45.3 From 45bb2be3140d82d088109337e8306e081a9437be Mon Sep 17 00:00:00 2001 From: Kiryl Kaveryn Date: Tue, 19 Nov 2024 14:17:57 +0400 Subject: [PATCH 11/13] [platform] add ProductsConfig 1. fetch and parse ProductsConfig json 2. save it to the separate file "products_settings.json" 3. small servers_list.cpp refactoring 4. add unit tests for servers config and products config 5 .add products fetching to the framework Signed-off-by: Kiryl Kaveryn Signed-off-by: Kiryl Kaveryn --- defines.hpp | 2 +- iphone/Maps/Core/Settings/MWMSettings.mm | 2 +- map/framework.cpp | 82 ++++++++++++ map/framework.hpp | 18 +++ platform/CMakeLists.txt | 2 + platform/platform_tests/CMakeLists.txt | 2 + platform/platform_tests/meta_config_tests.cpp | 110 ++++++++++++++++ platform/platform_tests/products_tests.cpp | 107 ++++++++++++++++ platform/products.cpp | 118 ++++++++++++++++++ platform/products.hpp | 64 ++++++++++ platform/servers_list.cpp | 15 ++- platform/servers_list.hpp | 1 + platform/settings.cpp | 25 ++-- platform/settings.hpp | 29 +---- storage/map_files_downloader.cpp | 13 +- .../platform.xcodeproj/project.pbxproj | 16 +++ 16 files changed, 561 insertions(+), 45 deletions(-) create mode 100644 platform/platform_tests/meta_config_tests.cpp create mode 100644 platform/platform_tests/products_tests.cpp create mode 100644 platform/products.cpp create mode 100644 platform/products.hpp diff --git a/defines.hpp b/defines.hpp index 3a682fc5df..081f461a8b 100644 --- a/defines.hpp +++ b/defines.hpp @@ -78,7 +78,7 @@ #define WORLD_COASTS_FILE_NAME "WorldCoasts" #define SETTINGS_FILE_NAME "settings.ini" -#define MARKETING_SETTINGS_FILE_NAME "marketing_settings.ini" +#define PRODUCTS_SETTINGS_FILE_NAME "products_settings.json" #define SEARCH_CATEGORIES_FILE_NAME "categories.txt" #define SEARCH_CUISINE_CATEGORIES_FILE_NAME "categories_cuisines.txt" diff --git a/iphone/Maps/Core/Settings/MWMSettings.mm b/iphone/Maps/Core/Settings/MWMSettings.mm index 28bc2b835c..4b9710ae9e 100644 --- a/iphone/Maps/Core/Settings/MWMSettings.mm +++ b/iphone/Maps/Core/Settings/MWMSettings.mm @@ -149,7 +149,7 @@ NSString * const kUDFileLoggingEnabledKey = @"FileLoggingEnabledKey"; + (NSString *)donateUrl { std::string url; - return settings::Get("DonateUrl", url) ? @(url.c_str()) : nil; + return settings::Get(settings::kDonateUrl, url) ? @(url.c_str()) : nil; } + (BOOL)isNY diff --git a/map/framework.cpp b/map/framework.cpp index dc68173bbb..0d88d376bc 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -102,6 +102,10 @@ std::string_view constexpr kTranslitMode = "TransliterationMode"; std::string_view constexpr kPreferredGraphicsAPI = "PreferredGraphicsAPI"; std::string_view constexpr kShowDebugInfo = "DebugInfo"; std::string_view constexpr kScreenViewport = "ScreenClipRect"; +std::string_view constexpr kPlacePageProductsPopupCloseTime = "PlacePageProductsPopupCloseTime"; +std::string_view constexpr kPlacePageProductSelectionTime = "PlacePageProductSelectionTime"; +std::string_view constexpr kPlacePageSelectedProduct = "PlacePageSelectedProduct"; +std::string_view constexpr kPlacePageProductRemindMeLaterTime = "PlacePageProductRemindMeLaterTime"; auto constexpr kLargeFontsScaleFactor = 1.6; size_t constexpr kMaxTrafficCacheSizeBytes = 64 /* Mb */ * 1024 * 1024; @@ -3313,3 +3317,81 @@ void Framework::OnPowerSchemeChanged(power_management::Scheme const actualScheme if (actualScheme == power_management::Scheme::EconomyMaximum && GetTrafficManager().IsEnabled()) GetTrafficManager().SetEnabled(false); } + +bool Framework::ShouldShowProducts() const +{ + auto const connectionStatus = Platform::ConnectionStatus(); + if (connectionStatus == Platform::EConnectionType::CONNECTION_NONE) + return false; + + std::string donateUrl; + if (!settings::Get(settings::kDonateUrl, donateUrl)) // donation is disabled + return false; + + if (!m_usageStats.IsLoyalUser()) + return false; + + if (!storage::IsPointCoveredByDownloadedMaps(GetCurrentPlacePageInfo().GetMercator(), m_storage, *m_infoGetter)) + return false; + + #ifdef DEBUG + uint32_t constexpr kPopupCloseTimeout = 10; + uint32_t constexpr kProductSelectTimeout = 20; + uint32_t constexpr kRemindMeLaterTimeout = 5; + #else + uint32_t constexpr kPopupCloseTimeout = 60 * 60 * 24 * 14; // 14 days + uint32_t constexpr kProductSelectTimeout = 60 * 60 * 24 * 180; // 180 days + uint32_t constexpr kRemindMeLaterTimeout = 60 * 60 * 24 * 3; // 3 days + #endif + + uint64_t popupCloseTime; + if (!settings::Get(kPlacePageProductsPopupCloseTime, popupCloseTime)) + popupCloseTime = 0; // popup was never shown + uint64_t productSelectTime; + if (!settings::Get(kPlacePageProductSelectionTime, productSelectTime)) + productSelectTime = 0; // product was never selected + uint64_t remindMeLaterTime; + if (!settings::Get(kPlacePageProductRemindMeLaterTime, remindMeLaterTime)) + remindMeLaterTime = 0; // remind me later was never pressed + + auto const now = base::SecondsSinceEpoch(); + bool const closePopupTimeoutExpired = popupCloseTime + kPopupCloseTimeout < now; + bool const productSelectTimeoutExpired = productSelectTime + kProductSelectTimeout < now; + bool const remindMeLaterTimeoutExpired = remindMeLaterTime + kRemindMeLaterTimeout < now; + if (closePopupTimeoutExpired && productSelectTimeoutExpired && remindMeLaterTimeoutExpired) + return true; + + return false; +} + +std::optional Framework::GetProductsConfiguration() const +{ + if (!ShouldShowProducts()) + return nullopt; + return products::GetProductsConfiguration(); +} + +void Framework::DidCloseProductsPopup(ProductsPopupCloseReason reason) const +{ + auto const now = base::SecondsSinceEpoch(); + settings::Set(kPlacePageProductsPopupCloseTime, now); + switch (reason) + { + case ProductsPopupCloseReason::Close: + break; + case ProductsPopupCloseReason::RemindLater: + settings::Set(kPlacePageProductRemindMeLaterTime, now); + break; + case ProductsPopupCloseReason::AlreadyDonated: // the already donated behaviour is the same as donate + case ProductsPopupCloseReason::SelectProduct: + settings::Set(kPlacePageProductSelectionTime, now); + break; + } + UNUSED_VALUE(reason); +} + +void Framework::DidSelectProduct(products::ProductsConfig::Product const & product) const +{ + settings::Set(kPlacePageSelectedProduct, product.GetTitle()); +} + diff --git a/map/framework.hpp b/map/framework.hpp index e99fbbb855..1b4e3bd895 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -47,6 +47,7 @@ #include "platform/location.hpp" #include "platform/platform.hpp" #include "platform/distance.hpp" +#include "platform/products.hpp" #include "routing/router.hpp" @@ -755,4 +756,21 @@ public: // PowerManager::Subscriber override. void OnPowerFacilityChanged(power_management::Facility const facility, bool enabled) override; void OnPowerSchemeChanged(power_management::Scheme const actualScheme) override; + +private: + bool ShouldShowProducts() const; + +public: + std::optional GetProductsConfiguration() const; + + enum class ProductsPopupCloseReason + { + Close, + SelectProduct, + AlreadyDonated, + RemindLater + }; + + void DidCloseProductsPopup(ProductsPopupCloseReason reason) const; + void DidSelectProduct(products::ProductsConfig::Product const & product) const; }; diff --git a/platform/CMakeLists.txt b/platform/CMakeLists.txt index fbad0d52c0..0e4f40becd 100644 --- a/platform/CMakeLists.txt +++ b/platform/CMakeLists.txt @@ -53,6 +53,8 @@ set(SRC secure_storage.hpp servers_list.cpp servers_list.hpp + products.cpp + products.hpp settings.cpp settings.hpp socket.hpp diff --git a/platform/platform_tests/CMakeLists.txt b/platform/platform_tests/CMakeLists.txt index d2be24a22f..b3cb32c5f8 100644 --- a/platform/platform_tests/CMakeLists.txt +++ b/platform/platform_tests/CMakeLists.txt @@ -14,6 +14,8 @@ set(SRC location_test.cpp measurement_tests.cpp platform_test.cpp + meta_config_tests.cpp + products_tests.cpp utm_mgrs_utils_tests.cpp ) diff --git a/platform/platform_tests/meta_config_tests.cpp b/platform/platform_tests/meta_config_tests.cpp new file mode 100644 index 0000000000..3597a33476 --- /dev/null +++ b/platform/platform_tests/meta_config_tests.cpp @@ -0,0 +1,110 @@ +#include "testing/testing.hpp" + +#include "platform/servers_list.hpp" + +#include "platform/products.hpp" + +#include "cppjansson/cppjansson.hpp" + +using namespace downloader; + +UNIT_TEST(MetaConfig_JSONParser_OldFormat) +{ + std::string oldFormatJson = R"(["http://url1", "http://url2", "http://url3"])"; + auto result = ParseMetaConfig(oldFormatJson); + TEST(result.has_value(), ()); + TEST_EQUAL(result->m_serversList.size(), 3, ()); + TEST_EQUAL(result->m_serversList[0], "http://url1", ()); + TEST_EQUAL(result->m_serversList[1], "http://url2", ()); + TEST_EQUAL(result->m_serversList[2], "http://url3", ()); + TEST(result->m_settings.empty(), ()); + TEST(result->m_productsConfig.empty(), ()); +} + +UNIT_TEST(MetaConfig_JSONParser_InvalidJSON) +{ + std::string invalidJson = R"({"servers": ["http://url1", "http://url2")"; + auto result = ParseMetaConfig(invalidJson); + TEST(!result.has_value(), ()); +} + +UNIT_TEST(MetaConfig_JSONParser_EmptyServersList) +{ + std::string emptyServersJson = R"({"servers": []})"; + auto result = ParseMetaConfig(emptyServersJson); + TEST(!result.has_value(), ()); +} + +UNIT_TEST(MetaConfig_JSONParser_NewFormatWithoutProducts) +{ + std::string newFormatJson = R"({ + "servers": ["http://url1", "http://url2"], + "settings": { + "key1": "value1", + "key2": "value2" + } + })"; + auto result = ParseMetaConfig(newFormatJson); + TEST(result.has_value(), ()); + TEST_EQUAL(result->m_serversList.size(), 2, ()); + TEST_EQUAL(result->m_serversList[0], "http://url1", ()); + TEST_EQUAL(result->m_serversList[1], "http://url2", ()); + TEST_EQUAL(result->m_settings.size(), 2, ()); + TEST_EQUAL(result->m_settings["key1"], "value1", ()); + TEST_EQUAL(result->m_settings["key2"], "value2", ()); + TEST(result->m_productsConfig.empty(), ()); +} + +UNIT_TEST(MetaConfig_JSONParser_NewFormatWithProducts) +{ + std::string newFormatJson = R"({ + "servers": ["http://url1", "http://url2"], + "settings": { + "key1": "value1", + "key2": "value2" + }, + "productsConfig": { + "placePagePrompt": "prompt1", + "aboutScreenPrompt": "prompt2", + "products": [ + { + "title": "Product 1", + "link": "http://product1" + }, + { + "title": "Product 2", + "link": "http://product2" + } + ] + } + })"; + + auto result = ParseMetaConfig(newFormatJson); + TEST(result.has_value(), ()); + TEST_EQUAL(result->m_serversList.size(), 2, ()); + TEST_EQUAL(result->m_serversList[0], "http://url1", ()); + TEST_EQUAL(result->m_serversList[1], "http://url2", ()); + TEST_EQUAL(result->m_settings.size(), 2, ()); + TEST_EQUAL(result->m_settings["key1"], "value1", ()); + TEST_EQUAL(result->m_settings["key2"], "value2", ()); + + TEST(!result->m_productsConfig.empty(), ()); + auto const productsConfigResult = products::ProductsConfig::Parse(result->m_productsConfig); + TEST(productsConfigResult.has_value(), ()); + auto const productsConfig = productsConfigResult.value(); + TEST_EQUAL(productsConfig.GetPlacePagePrompt(), "prompt1", ()); + TEST(productsConfig.HasProducts(), ()); + auto const products = productsConfig.GetProducts(); + TEST_EQUAL(products.size(), 2, ()); +} + +UNIT_TEST(MetaConfig_JSONParser_MissingServersKey) +{ + std::string missingServersJson = R"({ + "settings": { + "key1": "value1" + } + })"; + auto result = ParseMetaConfig(missingServersJson); + TEST(!result.has_value(), ("JSON shouldn't be parsed without 'servers' key")); +} diff --git a/platform/platform_tests/products_tests.cpp b/platform/platform_tests/products_tests.cpp new file mode 100644 index 0000000000..25382932e8 --- /dev/null +++ b/platform/platform_tests/products_tests.cpp @@ -0,0 +1,107 @@ +#include "testing/testing.hpp" + +#include "platform/products.hpp" + +#include "cppjansson/cppjansson.hpp" + +using namespace products; + +UNIT_TEST(ProductsConfig_ValidConfig) +{ + std::string jsonStr = R"({ + "placePagePrompt": "prompt1", + "products": [ + { + "title": "Product 1", + "link": "http://product1" + }, + { + "title": "Product 2", + "link": "http://product2" + } + ] + })"; + + auto const result = ProductsConfig::Parse(jsonStr); + TEST(result.has_value(), ()); + auto const productsConfig = result.value(); + TEST_EQUAL(productsConfig.GetPlacePagePrompt(), "prompt1", ()); + + auto const products = productsConfig.GetProducts(); + TEST_EQUAL(products.size(), 2, ()); + TEST_EQUAL(products[0].GetTitle(), "Product 1", ()); + TEST_EQUAL(products[0].GetLink(), "http://product1", ()); + TEST_EQUAL(products[1].GetTitle(), "Product 2", ()); + TEST_EQUAL(products[1].GetLink(), "http://product2", ()); +} + +UNIT_TEST(ProductsConfig_EmptyPrompts) +{ + std::string jsonStr = R"({ + "aboutScreenPrompt": "", + "products": [ + { + "title": "Product 1", + "link": "http://product1" + }, + { + "title": "Product 2", + "link": "http://product2" + } + ] + })"; + + auto const result = ProductsConfig::Parse(jsonStr); + TEST(result.has_value(), ()); + auto const productsConfig = result.value(); + TEST_EQUAL(productsConfig.GetPlacePagePrompt(), "", ()); + TEST_EQUAL(productsConfig.GetProducts().size(), 2, ()); +} + +UNIT_TEST(ProductsConfig_InvalidProduct) +{ + std::string jsonStr = R"({ + "placePagePrompt": "prompt1", + "products": [ + { + "title": "Product 1" + }, + { + "title": "Product 2", + "link": "http://product2" + } + ] + })"; + + auto const result = ProductsConfig::Parse(jsonStr); + TEST(result.has_value(), ()); + auto const productsConfig = result.value(); + TEST_EQUAL(productsConfig.GetPlacePagePrompt(), "prompt1", ()); + + auto const products = productsConfig.GetProducts(); + TEST_EQUAL(products.size(), 1, ()); + TEST_EQUAL(products[0].GetTitle(), "Product 2", ()); + TEST_EQUAL(products[0].GetLink(), "http://product2", ()); +} + +UNIT_TEST(ProductsConfig_EmptyProducts) +{ + std::string jsonStr = R"({ + "placePagePrompt": "prompt1", + "products": [] + })"; + + auto const result = ProductsConfig::Parse(jsonStr); + TEST(!result.has_value(), ()); +} + +UNIT_TEST(ProductsConfig_MissedProductsField) +{ + std::string jsonStr = R"({ + "placePagePrompt": "prompt1" + })"; + + auto const result = ProductsConfig::Parse(jsonStr); + TEST(!result.has_value(), ()); +} + diff --git a/platform/products.cpp b/platform/products.cpp new file mode 100644 index 0000000000..7c651cbd2f --- /dev/null +++ b/platform/products.cpp @@ -0,0 +1,118 @@ +#include "platform/products.hpp" +#include "platform/platform.hpp" + +#include "base/logging.hpp" +#include "base/assert.hpp" +#include "base/string_utils.hpp" + +#include "defines.hpp" + +#include "coding/file_writer.hpp" + +#include "cppjansson/cppjansson.hpp" + +namespace products { + +char const kPlacePagePrompt[] = "placePagePrompt"; +char const kProducts[] = "products"; +char const kProductTitle[] = "title"; +char const kProductLink[] = "link"; + +std::string GetProductsFilePath() +{ + return GetPlatform().SettingsPathForFile(PRODUCTS_SETTINGS_FILE_NAME); +} + +ProductsSettings::ProductsSettings() +{ + std::lock_guard guard(m_mutex); + auto const path = GetProductsFilePath(); + if (Platform::IsFileExistsByFullPath(path)) + { + try + { + std::string outValue; + auto dataReader = GetPlatform().GetReader(path); + dataReader->ReadAsString(outValue); + m_productsConfig = ProductsConfig::Parse(outValue); + } + catch (std::exception const & ex) + { + LOG(LERROR, ("Error reading ProductsConfig file.", ex.what())); + } + } + LOG(LWARNING, ("ProductsConfig file not found.")); +} + +ProductsSettings & ProductsSettings::Instance() +{ + static ProductsSettings instance; + return instance; +} + +std::optional ProductsSettings::Get() +{ + std::lock_guard guard(m_mutex); + return m_productsConfig; +} + +void ProductsSettings::Update(std::string const & jsonStr) +{ + std::lock_guard guard(m_mutex); + if (jsonStr.empty()) + FileWriter::DeleteFileX(GetProductsFilePath()); + else + { + try + { + FileWriter file(GetProductsFilePath()); + file.Write(jsonStr.data(), jsonStr.size()); + m_productsConfig = ProductsConfig::Parse(jsonStr); + } + catch (std::exception const & ex) + { + LOG(LERROR, ("Error writing ProductsConfig file.", ex.what())); + } + } +} + +std::optional ProductsConfig::Parse(std::string const & jsonStr) +{ + const base::Json root(jsonStr.c_str()); + auto const json = root.get(); + auto const productsObj = json_object_get(json, kProducts); + if (!json_is_object(json) || !productsObj || !json_is_array(productsObj)) + { + LOG(LWARNING, ("Failed to parse ProductsConfig:", jsonStr)); + return std::nullopt; + } + + ProductsConfig config; + auto const placePagePrompt = json_object_get(json, kPlacePagePrompt); + if (placePagePrompt && json_is_string(placePagePrompt)) + config.m_placePagePrompt = json_string_value(placePagePrompt); + + for (size_t i = 0; i < json_array_size(productsObj); ++i) + { + json_t * product = json_array_get(productsObj, i); + if (!product || !json_is_object(product)) + { + LOG(LWARNING, ("Failed to parse Product:", jsonStr)); + continue; + } + json_t * title = json_object_get(product, kProductTitle); + json_t * link = json_object_get(product, kProductLink); + if (title && link && json_is_string(title) && json_is_string(link)) + config.m_products.push_back({json_string_value(title), json_string_value(link)}); + else + LOG(LWARNING, ("Failed to parse Product:", jsonStr)); + } + if (config.m_products.empty()) + { + LOG(LWARNING, ("Products list is empty")); + return std::nullopt; + } + return config; +} + +} // namespace products diff --git a/platform/products.hpp b/platform/products.hpp new file mode 100644 index 0000000000..f7829f25d9 --- /dev/null +++ b/platform/products.hpp @@ -0,0 +1,64 @@ +#pragma once + +#include +#include +#include +#include + +namespace products { + +struct ProductsConfig +{ + struct Product + { + private: + std::string m_title; + std::string m_link; + + public: + Product(std::string const & title, std::string const & link) + : m_title(title), m_link(link) + {} + + std::string const & GetTitle() const { return m_title; } + std::string const & GetLink() const { return m_link; } + }; + +private: + std::string m_placePagePrompt; + std::vector m_products; + +public: + std::string const GetPlacePagePrompt() const { return m_placePagePrompt; } + std::vector const & GetProducts() const { return m_products; } + bool HasProducts() const { return !m_products.empty(); } + + static std::optional Parse(std::string const & jsonStr); +}; + +class ProductsSettings +{ +private: + ProductsSettings(); + + std::optional m_productsConfig; + mutable std::mutex m_mutex; + +public: + static ProductsSettings & Instance(); + + void Update(std::string const & jsonStr); + std::optional Get(); +}; + +inline void Update(std::string const & jsonStr) +{ + ProductsSettings::Instance().Update(jsonStr); +} + +inline std::optional GetProductsConfiguration() +{ + return ProductsSettings::Instance().Get(); +} + +} // namespace products diff --git a/platform/servers_list.cpp b/platform/servers_list.cpp index 03bf6a2037..d71ba5275e 100644 --- a/platform/servers_list.cpp +++ b/platform/servers_list.cpp @@ -10,8 +10,13 @@ namespace downloader { + std::optional ParseMetaConfig(std::string const & jsonStr) { + char const kSettings[] = "settings"; + char const kServers[] = "servers"; + char const kProductsConfig[] = "productsConfig"; + MetaConfig outMetaConfig; try { @@ -28,7 +33,7 @@ std::optional ParseMetaConfig(std::string const & jsonStr) // } // } - json_t * settings = json_object_get(root.get(), "settings"); + json_t * settings = json_object_get(root.get(), kSettings); const char * key; const json_t * value; json_object_foreach(settings, key, value) @@ -38,7 +43,13 @@ std::optional ParseMetaConfig(std::string const & jsonStr) outMetaConfig.m_settings[key] = valueStr; } - servers = json_object_get(root.get(), "servers"); + servers = json_object_get(root.get(), kServers); + + auto const productsConfig = json_object_get(root.get(), kProductsConfig); + if (productsConfig) + outMetaConfig.m_productsConfig = json_dumps(productsConfig, JSON_ENCODE_ANY); + else + LOG(LINFO, ("No ProductsConfig in meta configuration")); } else { diff --git a/platform/servers_list.hpp b/platform/servers_list.hpp index c6f605c4df..7887c02e72 100644 --- a/platform/servers_list.hpp +++ b/platform/servers_list.hpp @@ -14,6 +14,7 @@ struct MetaConfig ServersList m_serversList; using SettingsMap = std::map; SettingsMap m_settings; + std::string m_productsConfig; }; std::optional ParseMetaConfig(std::string const & jsonStr); diff --git a/platform/settings.cpp b/platform/settings.cpp index 802fd64afe..91deb8a664 100644 --- a/platform/settings.cpp +++ b/platform/settings.cpp @@ -23,6 +23,7 @@ using namespace std; std::string_view kMeasurementUnits = "Units"; std::string_view kMapLanguageCode = "MapLanguageCode"; std::string_view kDeveloperMode = "DeveloperMode"; +std::string_view kDonateUrl = "DonateUrl"; StringStorage::StringStorage() : StringStorageBase(GetPlatform().SettingsPathForFile(SETTINGS_FILE_NAME)) {} @@ -425,18 +426,16 @@ void UsageStats::EnterBackground() m_ss.SetValue(m_sessions, ToString(m_sessionsCount)); } -} // namespace settings - -/* -namespace marketing +bool UsageStats::IsLoyalUser() const { -Settings::Settings() : platform::StringStorageBase(GetPlatform().SettingsPathForFile(MARKETING_SETTINGS_FILE_NAME)) {} - -// static -Settings & Settings::Instance() -{ - static Settings instance; - return instance; + #ifdef DEBUG + uint32_t constexpr kMinTotalForegroundTimeout = 30; + uint32_t constexpr kMinSessionsCount = 3; + #else + uint32_t constexpr kMinTotalForegroundTimeout = 60 * 15; // 15 min + uint32_t constexpr kMinSessionsCount = 5; + #endif + return m_sessionsCount >= kMinSessionsCount && m_totalForegroundTime >= kMinTotalForegroundTimeout; } -} // namespace marketing -*/ + +} // namespace settings diff --git a/platform/settings.hpp b/platform/settings.hpp index 18aa97edcd..9a0e0457ac 100644 --- a/platform/settings.hpp +++ b/platform/settings.hpp @@ -12,6 +12,7 @@ namespace settings extern std::string_view kMeasurementUnits; extern std::string_view kDeveloperMode; extern std::string_view kMapLanguageCode; +extern std::string_view kDonateUrl; template bool FromString(std::string const & str, T & outValue); @@ -76,32 +77,8 @@ public: void EnterForeground(); void EnterBackground(); + + bool IsLoyalUser() const; }; } // namespace settings - -/* -namespace marketing -{ -class Settings : public platform::StringStorageBase -{ -public: - template - static void Set(std::string const & key, Value const & value) - { - Instance().SetValue(key, settings::ToString(value)); - } - - template - [[nodiscard]] static bool Get(std::string const & key, Value & outValue) - { - std::string strVal; - return Instance().GetValue(key, strVal) && settings::FromString(strVal, outValue); - } - -private: - static Settings & Instance(); - Settings(); -}; -} // namespace marketing -*/ diff --git a/storage/map_files_downloader.cpp b/storage/map_files_downloader.cpp index 9c29fdd11a..9dd98843e4 100644 --- a/storage/map_files_downloader.cpp +++ b/storage/map_files_downloader.cpp @@ -7,6 +7,8 @@ #include "platform/platform.hpp" #include "platform/servers_list.hpp" #include "platform/settings.hpp" +#include "platform/products.hpp" +#include "platform/locale.hpp" #include "coding/url.hpp" @@ -49,7 +51,7 @@ void MapFilesDownloader::RunMetaConfigAsync(std::function && callback) { m_serversList = metaConfig.m_serversList; settings::Update(metaConfig.m_settings); - + products::Update(metaConfig.m_productsConfig); callback(); // Reset flag to invoke servers list downloading next time if current request has failed. @@ -149,6 +151,12 @@ std::vector MapFilesDownloader::MakeUrlList(std::string const & rel return urls; } +std::string GetAcceptLanguage() +{ + auto const locale = platform::GetCurrentLocale(); + return locale.m_language + "-" + locale.m_country; +} + // static MetaConfig MapFilesDownloader::LoadMetaConfig() { @@ -160,7 +168,8 @@ MetaConfig MapFilesDownloader::LoadMetaConfig() { platform::HttpClient request(metaServerUrl); request.SetRawHeader("X-OM-DataVersion", std::to_string(m_dataVersion)); - request.SetRawHeader("X-OM-AppVersion", GetPlatform().Version()); + request.SetRawHeader("X-OM-AppVersion", pl.Version()); + request.SetRawHeader("Accept-Language", GetAcceptLanguage()); request.SetTimeout(10.0); // timeout in seconds request.RunHttpRequest(httpResult); } diff --git a/xcode/platform/platform.xcodeproj/project.pbxproj b/xcode/platform/platform.xcodeproj/project.pbxproj index 1b1fa72e9b..d59e90274d 100644 --- a/xcode/platform/platform.xcodeproj/project.pbxproj +++ b/xcode/platform/platform.xcodeproj/project.pbxproj @@ -114,6 +114,10 @@ D5B191CF2386C7E4009CD0D6 /* http_uploader_background.hpp in Headers */ = {isa = PBXBuildFile; fileRef = D5B191CE2386C7E4009CD0D6 /* http_uploader_background.hpp */; }; EB60B4DC204C130300E4953B /* network_policy_ios.mm in Sources */ = {isa = PBXBuildFile; fileRef = EB60B4DB204C130300E4953B /* network_policy_ios.mm */; }; EB60B4DE204C175700E4953B /* network_policy_ios.h in Headers */ = {isa = PBXBuildFile; fileRef = EB60B4DD204C175700E4953B /* network_policy_ios.h */; }; + ED49D7402CEE438E004AF27E /* products.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ED49D73F2CEE438E004AF27E /* products.cpp */; }; + ED49D7412CEE438E004AF27E /* products.hpp in Headers */ = {isa = PBXBuildFile; fileRef = ED49D73E2CEE438E004AF27E /* products.hpp */; }; + ED49D7442CEE43A4004AF27E /* meta_config_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ED49D7422CEE43A4004AF27E /* meta_config_tests.cpp */; }; + ED49D7452CEE43A4004AF27E /* products_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ED49D7432CEE43A4004AF27E /* products_tests.cpp */; }; ED965B252CD8F72E0049E39E /* duration.hpp in Headers */ = {isa = PBXBuildFile; fileRef = ED965B242CD8F72A0049E39E /* duration.hpp */; }; ED965B272CD8F7810049E39E /* duration.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ED965B262CD8F77D0049E39E /* duration.cpp */; }; ED965B472CDA52DB0049E39E /* duration_tests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = ED965B462CDA4EC00049E39E /* duration_tests.cpp */; }; @@ -256,6 +260,10 @@ D5B191CE2386C7E4009CD0D6 /* http_uploader_background.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = http_uploader_background.hpp; sourceTree = ""; }; EB60B4DB204C130300E4953B /* network_policy_ios.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = network_policy_ios.mm; sourceTree = ""; }; EB60B4DD204C175700E4953B /* network_policy_ios.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = network_policy_ios.h; sourceTree = ""; }; + ED49D73E2CEE438E004AF27E /* products.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = products.hpp; sourceTree = ""; }; + ED49D73F2CEE438E004AF27E /* products.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = products.cpp; sourceTree = ""; }; + ED49D7422CEE43A4004AF27E /* meta_config_tests.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = meta_config_tests.cpp; sourceTree = ""; }; + ED49D7432CEE43A4004AF27E /* products_tests.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = products_tests.cpp; sourceTree = ""; }; ED965B242CD8F72A0049E39E /* duration.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = duration.hpp; sourceTree = ""; }; ED965B262CD8F77D0049E39E /* duration.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = duration.cpp; sourceTree = ""; }; ED965B462CDA4EC00049E39E /* duration_tests.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = duration_tests.cpp; sourceTree = ""; }; @@ -317,6 +325,8 @@ 675340DE1C58C405002CF0D9 /* platform_tests */ = { isa = PBXGroup; children = ( + ED49D7422CEE43A4004AF27E /* meta_config_tests.cpp */, + ED49D7432CEE43A4004AF27E /* products_tests.cpp */, ED965B462CDA4EC00049E39E /* duration_tests.cpp */, 168EFCC12A30EB7400F71EE8 /* distance_tests.cpp */, 675341001C58C4C9002CF0D9 /* apk_test.cpp */, @@ -466,6 +476,8 @@ 675343A71A3F5D5A00A0A8C3 /* servers_list.hpp */, 675343A81A3F5D5A00A0A8C3 /* settings.cpp */, 675343A91A3F5D5A00A0A8C3 /* settings.hpp */, + ED49D73E2CEE438E004AF27E /* products.hpp */, + ED49D73F2CEE438E004AF27E /* products.cpp */, 34C624BB1DABCCD100510300 /* socket_apple.mm */, 34C624BC1DABCCD100510300 /* socket.hpp */, F6DF73561EC9EAE700D8BA0B /* string_storage_base.cpp */, @@ -544,6 +556,7 @@ 675343CD1A3F5D5A00A0A8C3 /* platform.hpp in Headers */, 6741250F1B4C00CC00A3E828 /* local_country_file.hpp in Headers */, 3D15ACE2214A707900F725D5 /* localization.hpp in Headers */, + ED49D7412CEE438E004AF27E /* products.hpp in Headers */, D5B191CF2386C7E4009CD0D6 /* http_uploader_background.hpp in Headers */, 1669C8422A30DCD200530AD1 /* distance.hpp in Headers */, 3DE8B98F1DEC3115000E6083 /* network_policy.hpp in Headers */, @@ -699,6 +712,7 @@ ED965B272CD8F7810049E39E /* duration.cpp in Sources */, EB60B4DC204C130300E4953B /* network_policy_ios.mm in Sources */, 6741250E1B4C00CC00A3E828 /* local_country_file.cpp in Sources */, + ED49D7402CEE438E004AF27E /* products.cpp in Sources */, 670E8C761BB318AB00094197 /* platform_ios.mm in Sources */, 67A2526A1BB40E100063F8A8 /* platform_qt.cpp in Sources */, 56EB1EDE1C6B6E6C0022D831 /* mwm_traits.cpp in Sources */, @@ -737,6 +751,8 @@ buildActionMask = 2147483647; files = ( 6783389E1C6DE59200FD6263 /* platform_test.cpp in Sources */, + ED49D7442CEE43A4004AF27E /* meta_config_tests.cpp in Sources */, + ED49D7452CEE43A4004AF27E /* products_tests.cpp in Sources */, 678338961C6DE59200FD6263 /* apk_test.cpp in Sources */, 678338991C6DE59200FD6263 /* jansson_test.cpp in Sources */, ED965B482CDA575B0049E39E /* duration.cpp in Sources */, -- 2.45.3 From 060de361c7c5349672ffd4b8acbf9acb157eaa0d Mon Sep 17 00:00:00 2001 From: Kiryl Kaveryn Date: Mon, 25 Nov 2024 16:52:01 +0400 Subject: [PATCH 12/13] [platform] fix timeouts and close reason Signed-off-by: Kiryl Kaveryn --- map/framework.cpp | 70 +++++++++++++++++++------------------------ map/framework.hpp | 7 +++-- platform/settings.cpp | 2 +- 3 files changed, 36 insertions(+), 43 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 0d88d376bc..61c3f13e68 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -103,9 +103,8 @@ std::string_view constexpr kPreferredGraphicsAPI = "PreferredGraphicsAPI"; std::string_view constexpr kShowDebugInfo = "DebugInfo"; std::string_view constexpr kScreenViewport = "ScreenClipRect"; std::string_view constexpr kPlacePageProductsPopupCloseTime = "PlacePageProductsPopupCloseTime"; -std::string_view constexpr kPlacePageProductSelectionTime = "PlacePageProductSelectionTime"; +std::string_view constexpr kPlacePageProductsPopupCloseReason = "PlacePageProductsPopupCloseReason"; std::string_view constexpr kPlacePageSelectedProduct = "PlacePageSelectedProduct"; -std::string_view constexpr kPlacePageProductRemindMeLaterTime = "PlacePageProductRemindMeLaterTime"; auto constexpr kLargeFontsScaleFactor = 1.6; size_t constexpr kMaxTrafficCacheSizeBytes = 64 /* Mb */ * 1024 * 1024; @@ -3334,31 +3333,16 @@ bool Framework::ShouldShowProducts() const if (!storage::IsPointCoveredByDownloadedMaps(GetCurrentPlacePageInfo().GetMercator(), m_storage, *m_infoGetter)) return false; - #ifdef DEBUG - uint32_t constexpr kPopupCloseTimeout = 10; - uint32_t constexpr kProductSelectTimeout = 20; - uint32_t constexpr kRemindMeLaterTimeout = 5; - #else - uint32_t constexpr kPopupCloseTimeout = 60 * 60 * 24 * 14; // 14 days - uint32_t constexpr kProductSelectTimeout = 60 * 60 * 24 * 180; // 180 days - uint32_t constexpr kRemindMeLaterTimeout = 60 * 60 * 24 * 3; // 3 days - #endif - uint64_t popupCloseTime; - if (!settings::Get(kPlacePageProductsPopupCloseTime, popupCloseTime)) - popupCloseTime = 0; // popup was never shown - uint64_t productSelectTime; - if (!settings::Get(kPlacePageProductSelectionTime, productSelectTime)) - productSelectTime = 0; // product was never selected - uint64_t remindMeLaterTime; - if (!settings::Get(kPlacePageProductRemindMeLaterTime, remindMeLaterTime)) - remindMeLaterTime = 0; // remind me later was never pressed + uint64_t productCloseReason; + if (!settings::Get(kPlacePageProductsPopupCloseTime, popupCloseTime) || + !settings::Get(kPlacePageProductsPopupCloseReason, productCloseReason)) + return true; // The popup was never closed. auto const now = base::SecondsSinceEpoch(); - bool const closePopupTimeoutExpired = popupCloseTime + kPopupCloseTimeout < now; - bool const productSelectTimeoutExpired = productSelectTime + kProductSelectTimeout < now; - bool const remindMeLaterTimeoutExpired = remindMeLaterTime + kRemindMeLaterTimeout < now; - if (closePopupTimeoutExpired && productSelectTimeoutExpired && remindMeLaterTimeoutExpired) + auto const timeout = GetTimeoutForReason(static_cast(productCloseReason)); + bool const timeoutExpired = popupCloseTime + timeout < now; + if (timeoutExpired) return true; return false; @@ -3373,21 +3357,8 @@ std::optional Framework::GetProductsConfiguration() co void Framework::DidCloseProductsPopup(ProductsPopupCloseReason reason) const { - auto const now = base::SecondsSinceEpoch(); - settings::Set(kPlacePageProductsPopupCloseTime, now); - switch (reason) - { - case ProductsPopupCloseReason::Close: - break; - case ProductsPopupCloseReason::RemindLater: - settings::Set(kPlacePageProductRemindMeLaterTime, now); - break; - case ProductsPopupCloseReason::AlreadyDonated: // the already donated behaviour is the same as donate - case ProductsPopupCloseReason::SelectProduct: - settings::Set(kPlacePageProductSelectionTime, now); - break; - } - UNUSED_VALUE(reason); + settings::Set(kPlacePageProductsPopupCloseTime, base::SecondsSinceEpoch()); + settings::Set(kPlacePageProductsPopupCloseReason, static_cast(reason)); } void Framework::DidSelectProduct(products::ProductsConfig::Product const & product) const @@ -3395,3 +3366,24 @@ void Framework::DidSelectProduct(products::ProductsConfig::Product const & produ settings::Set(kPlacePageSelectedProduct, product.GetTitle()); } +uint32_t Framework::GetTimeoutForReason(ProductsPopupCloseReason reason) const +{ + #ifdef DEBUG + uint32_t constexpr kPopupCloseTimeout = 10; + uint32_t constexpr kProductSelectTimeout = 20; + uint32_t constexpr kRemindMeLaterTimeout = 5; + #else + uint32_t constexpr kPopupCloseTimeout = 60 * 60 * 24 * 60; // 60 days + uint32_t constexpr kProductSelectTimeout = 60 * 60 * 24 * 180; // 180 days + uint32_t constexpr kRemindMeLaterTimeout = 60 * 60 * 24 * 3; // 3 days + #endif + switch (reason) + { + case ProductsPopupCloseReason::Close: return kPopupCloseTimeout; + case ProductsPopupCloseReason::RemindLater: return kRemindMeLaterTimeout; + case ProductsPopupCloseReason::AlreadyDonated: return kProductSelectTimeout; + case ProductsPopupCloseReason::SelectProduct: return kProductSelectTimeout; + } + UNUSED_VALUE(reason); +} + diff --git a/map/framework.hpp b/map/framework.hpp index 1b4e3bd895..28f14871a1 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -757,9 +757,6 @@ public: void OnPowerFacilityChanged(power_management::Facility const facility, bool enabled) override; void OnPowerSchemeChanged(power_management::Scheme const actualScheme) override; -private: - bool ShouldShowProducts() const; - public: std::optional GetProductsConfiguration() const; @@ -773,4 +770,8 @@ public: void DidCloseProductsPopup(ProductsPopupCloseReason reason) const; void DidSelectProduct(products::ProductsConfig::Product const & product) const; + +private: + bool ShouldShowProducts() const; + uint32_t GetTimeoutForReason(ProductsPopupCloseReason reason) const; }; diff --git a/platform/settings.cpp b/platform/settings.cpp index 91deb8a664..371b49bcba 100644 --- a/platform/settings.cpp +++ b/platform/settings.cpp @@ -432,7 +432,7 @@ bool UsageStats::IsLoyalUser() const uint32_t constexpr kMinTotalForegroundTimeout = 30; uint32_t constexpr kMinSessionsCount = 3; #else - uint32_t constexpr kMinTotalForegroundTimeout = 60 * 15; // 15 min + uint32_t constexpr kMinTotalForegroundTimeout = 60 * 30; // 30 min uint32_t constexpr kMinSessionsCount = 5; #endif return m_sessionsCount >= kMinSessionsCount && m_totalForegroundTime >= kMinTotalForegroundTimeout; -- 2.45.3 From 91b5cb8cd7953d53393ac92984a7e5872b86a229 Mon Sep 17 00:00:00 2001 From: Kiryl Kaveryn Date: Mon, 25 Nov 2024 18:05:29 +0400 Subject: [PATCH 13/13] [platform] review fixes Signed-off-by: Kiryl Kaveryn --- map/framework.cpp | 38 +++++++++++++++++++++++++++++++++----- map/framework.hpp | 2 ++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 61c3f13e68..782c7ce216 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -106,6 +106,11 @@ std::string_view constexpr kPlacePageProductsPopupCloseTime = "PlacePageProducts std::string_view constexpr kPlacePageProductsPopupCloseReason = "PlacePageProductsPopupCloseReason"; std::string_view constexpr kPlacePageSelectedProduct = "PlacePageSelectedProduct"; +std::string_view constexpr kProductsPopupCloseReasonCloseStr = "close"; +std::string_view constexpr kProductsPopupCloseReasonRemindLaterStr = "remind_later"; +std::string_view constexpr kProductsPopupCloseReasonAlreadyDonatedStr = "already_donated"; +std::string_view constexpr kProductsPopupCloseReasonSelectProductStr = "select_product"; + auto constexpr kLargeFontsScaleFactor = 1.6; size_t constexpr kMaxTrafficCacheSizeBytes = 64 /* Mb */ * 1024 * 1024; @@ -3334,13 +3339,13 @@ bool Framework::ShouldShowProducts() const return false; uint64_t popupCloseTime; - uint64_t productCloseReason; + std::string productCloseReason; if (!settings::Get(kPlacePageProductsPopupCloseTime, popupCloseTime) || !settings::Get(kPlacePageProductsPopupCloseReason, productCloseReason)) return true; // The popup was never closed. auto const now = base::SecondsSinceEpoch(); - auto const timeout = GetTimeoutForReason(static_cast(productCloseReason)); + auto const timeout = GetTimeoutForReason(FromString(productCloseReason)); bool const timeoutExpired = popupCloseTime + timeout < now; if (timeoutExpired) return true; @@ -3358,7 +3363,7 @@ std::optional Framework::GetProductsConfiguration() co void Framework::DidCloseProductsPopup(ProductsPopupCloseReason reason) const { settings::Set(kPlacePageProductsPopupCloseTime, base::SecondsSinceEpoch()); - settings::Set(kPlacePageProductsPopupCloseReason, static_cast(reason)); + settings::Set(kPlacePageProductsPopupCloseReason, std::string(ToString(reason))); } void Framework::DidSelectProduct(products::ProductsConfig::Product const & product) const @@ -3373,7 +3378,7 @@ uint32_t Framework::GetTimeoutForReason(ProductsPopupCloseReason reason) const uint32_t constexpr kProductSelectTimeout = 20; uint32_t constexpr kRemindMeLaterTimeout = 5; #else - uint32_t constexpr kPopupCloseTimeout = 60 * 60 * 24 * 60; // 60 days + uint32_t constexpr kPopupCloseTimeout = 60 * 60 * 24 * 30; // 30 days uint32_t constexpr kProductSelectTimeout = 60 * 60 * 24 * 180; // 180 days uint32_t constexpr kRemindMeLaterTimeout = 60 * 60 * 24 * 3; // 3 days #endif @@ -3384,6 +3389,29 @@ uint32_t Framework::GetTimeoutForReason(ProductsPopupCloseReason reason) const case ProductsPopupCloseReason::AlreadyDonated: return kProductSelectTimeout; case ProductsPopupCloseReason::SelectProduct: return kProductSelectTimeout; } - UNUSED_VALUE(reason); + ASSERT(false, ("Unknown reason")); + return kPopupCloseTimeout; } +std::string_view Framework::ToString(ProductsPopupCloseReason reason) const +{ + switch (reason) + { + case ProductsPopupCloseReason::Close: return kProductsPopupCloseReasonCloseStr; + case ProductsPopupCloseReason::RemindLater: return kProductsPopupCloseReasonRemindLaterStr; + case ProductsPopupCloseReason::AlreadyDonated: return kProductsPopupCloseReasonAlreadyDonatedStr; + case ProductsPopupCloseReason::SelectProduct: return kProductsPopupCloseReasonSelectProductStr; + } + ASSERT(false, ("Unknown reason")); + return kProductsPopupCloseReasonCloseStr; +} + +Framework::ProductsPopupCloseReason Framework::FromString(std::string const & str) const +{ + if (str == kProductsPopupCloseReasonCloseStr) return ProductsPopupCloseReason::Close; + if (str == kProductsPopupCloseReasonRemindLaterStr) return ProductsPopupCloseReason::RemindLater; + if (str == kProductsPopupCloseReasonAlreadyDonatedStr) return ProductsPopupCloseReason::AlreadyDonated; + if (str == kProductsPopupCloseReasonSelectProductStr) return ProductsPopupCloseReason::SelectProduct; + ASSERT(false, ("Incorrect reason string:", str)); + return ProductsPopupCloseReason::Close; +} diff --git a/map/framework.hpp b/map/framework.hpp index 28f14871a1..4276bab57b 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -774,4 +774,6 @@ public: private: bool ShouldShowProducts() const; uint32_t GetTimeoutForReason(ProductsPopupCloseReason reason) const; + std::string_view ToString(ProductsPopupCloseReason reason) const; + ProductsPopupCloseReason FromString(std::string const & str) const; }; -- 2.45.3