diff --git a/editor/editor_tests/user_stats_test.cpp b/editor/editor_tests/user_stats_test.cpp index db68bb7712..d1e43b1ff1 100644 --- a/editor/editor_tests/user_stats_test.cpp +++ b/editor/editor_tests/user_stats_test.cpp @@ -2,19 +2,56 @@ #include "editor/user_stats.hpp" +#include "platform/platform_tests_support/writable_dir_changer.hpp" + namespace editor { namespace { -UNIT_TEST(UserStats_Smoke) +auto constexpr kEditorTestDir = "editor-tests"; +auto constexpr kUserName = "Vladimir BI"; + +UNIT_TEST(UserStatsLoader_Smoke) { - // This user made only two changes and the possibility of further changes is very low. - UserStats userStats("Vladimir BI"); - TEST(userStats.GetUpdateStatus(), ()); - TEST(userStats.IsChangesCountInitialized(), ()); - TEST(userStats.IsRankInitialized(), ()); - TEST_EQUAL(userStats.GetChangesCount(), 2, ()); - TEST_GREATER_OR_EQUAL(userStats.GetRank(), 5800, ()); + WritableDirChanger wdc(kEditorTestDir, WritableDirChanger::SettingsDirPolicy::UseWritableDir); + + { + UserStatsLoader statsLoader; + TEST(!statsLoader.GetStats(kUserName), ()); + } + + { + // This user made only two changes and the possibility of further changes is very low. + UserStatsLoader statsLoader; + + statsLoader.Update(kUserName); + auto const userStats = statsLoader.GetStats(kUserName); + + TEST(userStats, ()); + int32_t rank, changesCount; + TEST(userStats.GetRank(rank), ()); + TEST(userStats.GetChangesCount(changesCount), ()); + + TEST_GREATER_OR_EQUAL(rank, 5800, ()); + TEST_EQUAL(changesCount, 2, ()); + } + + // This test checks if user stats info was stored in setting. + // NOTE: there Update function is not called. + { + UserStatsLoader statsLoader; + + TEST_EQUAL(statsLoader.GetUserName(), kUserName, ()); + auto const userStats = statsLoader.GetStats(kUserName); + + TEST(userStats, ()); + int32_t rank, changesCount; + TEST(userStats.GetRank(rank), ()); + TEST(userStats.GetChangesCount(changesCount), ()); + + TEST_GREATER_OR_EQUAL(rank, 5800, ()); + TEST_EQUAL(changesCount, 2, ()); + } } } // namespace } // namespace editor diff --git a/editor/user_stats.cpp b/editor/user_stats.cpp index 76788e0e75..629f4543a9 100644 --- a/editor/user_stats.cpp +++ b/editor/user_stats.cpp @@ -1,42 +1,94 @@ #include "editor/user_stats.hpp" +#include "platform/platform.hpp" +#include "platform/settings.hpp" + #include "coding/url_encode.hpp" #include "base/logging.hpp" +#include "base/timer.hpp" + +#include "std/thread.hpp" #include "3party/Alohalytics/src/http_client.h" #include "3party/pugixml/src/pugixml.hpp" using TRequest = alohalytics::HTTPClientPlatformWrapper; - namespace { string const kUserStatsUrl = "http://py.osmz.ru/mmwatch/user?format=xml"; -auto constexpr kUninitialized = -1; +int32_t constexpr kUninitialized = -1; + +auto constexpr kSettingsUserName = "LastLoggedUser"; +auto constexpr kSettingsRating = "UserEditorRating"; +auto constexpr kSettingsChangesCount = "UserEditorChangesCount"; +auto constexpr kSettingsLastUpdate = "UserEditorLastUpdate"; + +auto constexpr kSecondsInHour = 60 * 60; } // namespace namespace editor { -UserStats::UserStats(string const & userName) - : m_userName(userName), m_changesCount(kUninitialized), m_rank(kUninitialized) +// UserStat ---------------------------------------------------------------------------------------- + +UserStats::UserStats() + : m_changesCount(kUninitialized), m_rank(kUninitialized) + , m_updateTime(my::SecondsSinceEpochToTimeT(0)), m_valid(false) { - m_updateStatus = Update(); } -bool UserStats::IsChangesCountInitialized() const +UserStats::UserStats(time_t const updateTime, uint32_t const rating, + uint32_t const changesCount, string const & levelUpFeat) + : m_changesCount(changesCount), m_rank(rating) + , m_updateTime(updateTime), m_levelUpRequiredFeat(levelUpFeat) + , m_valid(true) { - return m_changesCount != kUninitialized; } -bool UserStats::IsRankInitialized() const +bool UserStats::GetChangesCount(int32_t & changesCount) const { - return m_rank != kUninitialized; + if (m_changesCount == kUninitialized) + return false; + changesCount = m_changesCount; + return true; } -bool UserStats::Update() +bool UserStats::GetRank(int32_t & rank) const { - auto const url = kUserStatsUrl + "&name=" + UrlEncode(m_userName); + if (m_rank == kUninitialized) + return false; + rank = m_rank; + return true; +} + +bool UserStats::GetLevelUpRequiredFeat(string & levelUpFeat) +{ + if (m_levelUpRequiredFeat.empty()) + return false; + levelUpFeat = m_levelUpRequiredFeat; + return true; +} + +// UserStatsLoader --------------------------------------------------------------------------------- + +UserStatsLoader::UserStatsLoader() + : m_lastUpdate(my::SecondsSinceEpochToTimeT(0)) +{ + if (!LoadFromSettings()) + LOG(LINFO, ("There is no cached user stats info in settings")); + else + LOG(LINFO, ("User stats info was loaded successfully")); +} + +bool UserStatsLoader::Update(string const & userName) +{ + { + lock_guard g(m_mutex); + m_userName = userName; + } + + auto const url = kUserStatsUrl + "&name=" + UrlEncode(userName); TRequest request(url); if (!request.RunHTTPRequest()) @@ -60,9 +112,103 @@ bool UserStats::Update() return false; } - m_changesCount = document.select_node("mmwatch/edits/@value").attribute().as_int(kUninitialized); - m_rank = document.select_node("mmwatch/rank/@value").attribute().as_int(kUninitialized); + auto changesCount = document.select_node("mmwatch/edits/@value").attribute().as_int(-1); + auto rank = document.select_node("mmwatch/rank/@value").attribute().as_int(-1); + auto levelUpFeat = document.select_node("mmwatch/levelUpFeat/@value").attribute().as_string(); + + lock_guard g(m_mutex); + if (m_userName != userName) + return false; + + m_lastUpdate = time(nullptr); + m_userStats = UserStats(m_lastUpdate, rank, changesCount, levelUpFeat); + SaveToSettings(); return true; } + +void UserStatsLoader::Update(string const & userName, TOnUpdateCallback fn) +{ + auto nothingToUpdate = false; + { + lock_guard g(m_mutex); + nothingToUpdate = m_userStats && m_userName == userName && m_userStats && + difftime(m_lastUpdate, time(nullptr)) < kSecondsInHour; + } + + if (nothingToUpdate) + { + GetPlatform().RunOnGuiThread(fn); + return; + } + + thread([this, userName, fn] { + if (Update(userName)) + GetPlatform().RunOnGuiThread(fn); + }).detach(); +} + +void UserStatsLoader::DropStats(string const & userName) +{ + lock_guard g(m_mutex); + if (m_userName != userName) + return; + m_userStats = {}; + DropSettings(); +} + +UserStats UserStatsLoader::GetStats(string const & userName) const +{ + lock_guard g(m_mutex); + if (m_userName == userName) + return m_userStats; + return {}; +} + +string UserStatsLoader::GetUserName() const +{ + lock_guard g(m_mutex); + return m_userName; +} + +bool UserStatsLoader::LoadFromSettings() +{ + uint32_t rating, changesCount; + uint64_t lastUpdate; + if (!settings::Get(kSettingsUserName, m_userName) || + !settings::Get(kSettingsChangesCount, changesCount) || + !settings::Get(kSettingsRating, rating) || + !settings::Get(kSettingsLastUpdate, lastUpdate)) + { + return false; + } + + m_lastUpdate = my::SecondsSinceEpochToTimeT(lastUpdate); + m_userStats = UserStats(m_lastUpdate, rating, changesCount, ""); + return true; +} + +void UserStatsLoader::SaveToSettings() +{ + if (!m_userStats) + return; + + settings::Set(kSettingsUserName, m_userName); + int32_t rank; + if (m_userStats.GetRank(rank)) + settings::Set(kSettingsRating, rank); + int32_t changesCount; + if (m_userStats.GetChangesCount(changesCount)) + settings::Set(kSettingsChangesCount, changesCount); + settings::Set(kSettingsLastUpdate, my::TimeTToSecondsSinceEpoch(m_lastUpdate)); + // Do not save m_requiredLevelUpFeat for it becomes obsolete very fast. +} + +void UserStatsLoader::DropSettings() +{ + settings::Delete(kSettingsUserName); + settings::Delete(kSettingsRating); + settings::Delete(kSettingsChangesCount); + settings::Delete(kSettingsLastUpdate); +} } // namespace editor diff --git a/editor/user_stats.hpp b/editor/user_stats.hpp index 38f714bdcd..200330ec23 100644 --- a/editor/user_stats.hpp +++ b/editor/user_stats.hpp @@ -1,6 +1,9 @@ #pragma once #include "std/cstdint.hpp" +#include "std/ctime.hpp" +#include "std/function.hpp" +#include "std/mutex.hpp" #include "std/string.hpp" namespace editor @@ -8,23 +11,64 @@ namespace editor class UserStats { public: - explicit UserStats(string const & userName); + UserStats(); + UserStats(time_t const updateTime, uint32_t const rating, + uint32_t const changesCount, string const & levelUpFeat); - bool IsChangesCountInitialized() const; - bool IsRankInitialized() const; + bool IsValid() const { return m_valid; } - int32_t GetChangesCount() const { return m_changesCount; } - int32_t GetRank() const { return m_rank; } + operator bool() const { return IsValid(); } - bool GetUpdateStatus() const { return m_updateStatus; } - bool Update(); + bool GetChangesCount(int32_t & changesCount) const; + bool GetRank(int32_t & rank) const; + bool GetLevelUpRequiredFeat(string & levelUpFeat); + + time_t GetLastUpdate() const { return m_updateTime; } private: - string m_userName; int32_t m_changesCount; int32_t m_rank; + time_t m_updateTime; + /// A very doubtful field representing what a user must commit to have a better rank. + string m_levelUpRequiredFeat; + bool m_valid; +}; - /// True if last update was successful. - bool m_updateStatus; +class UserStatsLoader +{ +public: + using TOnUpdateCallback = function; + + UserStatsLoader(); + + /// Synchronously sends request to the server. Updates stats and returns true on success. + bool Update(string const & userName); + + /// Launch the update process if stats are too old. + /// The process posts fn to a gui thread on success. + void Update(string const & userName, TOnUpdateCallback fn); + + /// Resets internal state and removes records from settings. + void DropStats(string const & userName); + + /// Atomically returns stats if userName is still actual. + UserStats GetStats(string const & userName) const; + + /// Debug only. + string GetUserName() const; + +private: + /// Not thread-safe, but called only in constructor. + bool LoadFromSettings(); + /// Not thread-safe, use synchonization. + void SaveToSettings(); + void DropSettings(); + + string m_userName; + + time_t m_lastUpdate; + mutable mutex m_mutex; + + UserStats m_userStats; }; } // namespace editor diff --git a/map/framework.cpp b/map/framework.cpp index c5253230b3..2fd2cacd9a 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -412,6 +412,8 @@ Framework::Framework() editor.LoadMapEdits(); m_model.GetIndex().AddObserver(editor); + + LOG(LINFO, ("Editor initialized")); } Framework::~Framework() @@ -2900,12 +2902,3 @@ bool Framework::RollBackChanges(FeatureID const & fid) } return rolledBack; } - -bool Framework::UpdateUserStats(string const & userName) -{ - auto userStats = make_unique(userName); - if (!userStats->GetUpdateStatus()) - return false; - m_userStats = move(userStats); - return true; -} diff --git a/map/framework.hpp b/map/framework.hpp index 1fd41a1d7f..1e8a4c2dea 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -43,7 +43,6 @@ #include "base/thread_checker.hpp" #include "std/list.hpp" -#include "std/shared_ptr.hpp" #include "std/target_os.hpp" #include "std/unique_ptr.hpp" #include "std/vector.hpp" @@ -675,15 +674,24 @@ private: public: //@{ - //User statistics. - editor::UserStats const * GetUserStats() const { return m_userStats.get(); } - /// Sends a synchronous request to the server and updates user's stats. - /// @returns true on success. - bool UpdateUserStats(string const & userName); - void DropUserStats() { m_userStats = nullptr; } + // User statistics. + + editor::UserStats GetUserStats(string const & userName) const + { + return m_userStatsLoader.GetStats(userName); + } + + // Reads user stats from server or gets it from cache calls |fn| on success. + void UpdateUserStats(string const & userName, + editor::UserStatsLoader::TOnUpdateCallback fn) + { + m_userStatsLoader.Update(userName, fn); + } + + void DropUserStats(string const & userName) { m_userStatsLoader.DropStats(userName); } private: - unique_ptr m_userStats; + editor::UserStatsLoader m_userStatsLoader; //@} DECLARE_THREAD_CHECKER(m_threadChecker); diff --git a/omim.pro b/omim.pro index ffca2e6d7b..8fcce7b7af 100644 --- a/omim.pro +++ b/omim.pro @@ -202,7 +202,7 @@ SUBDIRS = 3party base coding geometry editor indexer routing search SUBDIRS *= generator_tests editor_tests.subdir = editor/editor_tests - editor_tests.depends = 3party base coding geometry editor + editor_tests.depends = 3party base coding geometry platform editor SUBDIRS *= editor_tests SUBDIRS *= qt_tstfrm diff --git a/platform/platform.cpp b/platform/platform.cpp index 339ff20b35..2e70593165 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -87,6 +87,11 @@ bool Platform::RmDirRecursively(string const & dirName) return res; } +void Platform::SetSettingsDirForTests(string const & path) +{ + m_settingsDir = my::AddSlashIfNeeded(path); +} + string Platform::ReadPathForFile(string const & file, string searchScope) const { if (searchScope.empty()) diff --git a/platform/platform.hpp b/platform/platform.hpp index 45770be135..a8247f799b 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -126,6 +126,8 @@ public: /// @return path for directory in the persistent memory, can be the same /// as WritableDir, but on some platforms it's different string SettingsDir() const { return m_settingsDir; } + /// Set settings dir — use for testing. + void SetSettingsDirForTests(string const & path); /// @return full path to file in the settings directory string SettingsPathForFile(string const & file) const { return SettingsDir() + file; } diff --git a/platform/platform_tests_support/writable_dir_changer.cpp b/platform/platform_tests_support/writable_dir_changer.cpp index a61859bd72..f63799403f 100644 --- a/platform/platform_tests_support/writable_dir_changer.cpp +++ b/platform/platform_tests_support/writable_dir_changer.cpp @@ -8,15 +8,18 @@ #include "coding/file_name_utils.hpp" #include "coding/internal/file_data.hpp" -WritableDirChanger::WritableDirChanger(string const & testDir) +WritableDirChanger::WritableDirChanger(string const & testDir, SettingsDirPolicy settingsDirPolicy) : m_writableDirBeforeTest(GetPlatform().WritableDir()) , m_testDirFullPath(m_writableDirBeforeTest + testDir) + , m_settingsDirPolicy(settingsDirPolicy) { Platform & platform = GetPlatform(); platform.RmDirRecursively(m_testDirFullPath); TEST(!platform.IsFileExistsByFullPath(m_testDirFullPath), ()); TEST_EQUAL(Platform::ERR_OK, platform.MkDir(m_testDirFullPath), ()); platform.SetWritableDirForTests(m_testDirFullPath); + if (m_settingsDirPolicy == SettingsDirPolicy::UseWritableDir) + platform.SetSettingsDirForTests(m_testDirFullPath); settings::Clear(); } @@ -26,5 +29,7 @@ WritableDirChanger::~WritableDirChanger() Platform & platform = GetPlatform(); string const writableDirForTest = platform.WritableDir(); platform.SetWritableDirForTests(m_writableDirBeforeTest); + if (m_settingsDirPolicy == SettingsDirPolicy::UseWritableDir) + platform.SetSettingsDirForTests(m_writableDirBeforeTest); platform.RmDirRecursively(writableDirForTest); } diff --git a/platform/platform_tests_support/writable_dir_changer.hpp b/platform/platform_tests_support/writable_dir_changer.hpp index e0e17ddc03..d00b3d47e8 100644 --- a/platform/platform_tests_support/writable_dir_changer.hpp +++ b/platform/platform_tests_support/writable_dir_changer.hpp @@ -3,10 +3,17 @@ class WritableDirChanger { public: - WritableDirChanger(string const & testDir); + enum class SettingsDirPolicy + { + UseDefault, UseWritableDir + }; + + WritableDirChanger(string const & testDir, + SettingsDirPolicy settingsDirPolicy = SettingsDirPolicy::UseDefault); ~WritableDirChanger(); private: string const m_writableDirBeforeTest; string const m_testDirFullPath; + SettingsDirPolicy m_settingsDirPolicy; };