diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index 324299aab0..5a344c2e0e 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -542,12 +542,12 @@ namespace android void Framework::AddLocalMaps() { - m_work.AddMaps(); + m_work.RegisterAllMaps(); } void Framework::RemoveLocalMaps() { - m_work.RemoveMaps(); + m_work.DeregisterAllMaps(); } void Framework::GetMapsWithoutSearch(vector & out) const diff --git a/base/base.pro b/base/base.pro index c8c2287523..317ea6467d 100644 --- a/base/base.pro +++ b/base/base.pro @@ -59,6 +59,7 @@ HEADERS += \ mru_cache.hpp \ mutex.hpp \ object_tracker.hpp \ + observer_list.hpp \ pseudo_random.hpp \ regexp.hpp \ resource_pool.hpp \ diff --git a/base/base_tests/base_tests.pro b/base/base_tests/base_tests.pro index b1e8ae8a89..6b2bcb923a 100644 --- a/base/base_tests/base_tests.pro +++ b/base/base_tests/base_tests.pro @@ -28,6 +28,7 @@ SOURCES += \ matrix_test.cpp \ mem_trie_test.cpp \ mru_cache_test.cpp \ + observer_list_test.cpp \ regexp_test.cpp \ rolling_hash_test.cpp \ scheduled_task_test.cpp \ diff --git a/base/base_tests/observer_list_test.cpp b/base/base_tests/observer_list_test.cpp new file mode 100644 index 0000000000..6e3a4ba48f --- /dev/null +++ b/base/base_tests/observer_list_test.cpp @@ -0,0 +1,68 @@ +#include "../../testing/testing.hpp" + +#include "../observer_list.hpp" + +#include "../../std/string.hpp" + +namespace +{ +struct Observer +{ + Observer() : status(0) {} + + void OnOperationCompleted(string const & message, int status) + { + this->message = message; + this->status = status; + } + + string message; + int status; +}; +} // namespace + +UNIT_TEST(ObserverList_Basic) +{ + Observer observer0; + Observer observer1; + Observer observer2; + + my::ObserverList observers; + + // Register all observers in a list. Also check that it's not + // possible to add the same observer twice. + TEST(observers.Add(observer0), ()); + TEST(!observers.Add(observer0), ()); + + TEST(observers.Add(observer1), ()); + TEST(!observers.Add(observer1), ()); + + TEST(observers.Add(observer2), ()); + TEST(!observers.Add(observer2), ()); + + TEST(!observers.Add(observer1), ()); + TEST(!observers.Add(observer2), ()); + + string const message = "HTTP OK"; + observers.ForEach(&Observer::OnOperationCompleted, message, 204); + + // Check observers internal state. + TEST_EQUAL(message, observer0.message, ()); + TEST_EQUAL(204, observer0.status, ()); + + TEST_EQUAL(message, observer1.message, ()); + TEST_EQUAL(204, observer1.status, ()); + + TEST_EQUAL(message, observer2.message, ()); + TEST_EQUAL(204, observer2.status, ()); + + // Remove all observers from a list. + TEST(observers.Remove(observer0), ()); + TEST(!observers.Remove(observer0), ()); + + TEST(observers.Remove(observer1), ()); + TEST(!observers.Remove(observer1), ()); + + TEST(observers.Remove(observer2), ()); + TEST(!observers.Remove(observer2), ()); +} diff --git a/base/observer_list.hpp b/base/observer_list.hpp new file mode 100644 index 0000000000..915c78f1d8 --- /dev/null +++ b/base/observer_list.hpp @@ -0,0 +1,47 @@ +#pragma once + +#include "../std/algorithm.hpp" +#include "../std/mutex.hpp" +#include "../std/vector.hpp" + +namespace my +{ +/// This class represents a thread-safe observers list. It allows to +/// add/remove observers as well as trigger them all. +template +class ObserverList +{ +public: + bool Add(TObserver & observer) + { + lock_guard lock(m_observersLock); + auto it = find(m_observers.begin(), m_observers.end(), &observer); + if (it != m_observers.end()) + return false; + m_observers.push_back(&observer); + return true; + } + + bool Remove(TObserver & observer) + { + lock_guard lock(m_observersLock); + auto it = find(m_observers.begin(), m_observers.end(), &observer); + if (it == m_observers.end()) + return false; + m_observers.erase(it); + return true; + } + + template + void ForEach(F fn, Args const &... args) + { + lock_guard lock(m_observersLock); + for (TObserver * observer : m_observers) + (observer->*fn)(args...); + } + +private: + vector m_observers; + mutex m_observersLock; +}; +} // namespace my diff --git a/coding/internal/file_data.cpp b/coding/internal/file_data.cpp index f3c136e938..08b24afe3e 100644 --- a/coding/internal/file_data.cpp +++ b/coding/internal/file_data.cpp @@ -7,10 +7,11 @@ #include "../../base/exception.hpp" #include "../../base/logging.hpp" -#include "../../std/target_os.hpp" -#include "../../std/fstream.hpp" -#include "../../std/exception.hpp" #include "../../std/cerrno.hpp" +#include "../../std/cstring.hpp" +#include "../../std/exception.hpp" +#include "../../std/fstream.hpp" +#include "../../std/target_os.hpp" #ifdef OMIM_OS_WINDOWS #include @@ -221,23 +222,24 @@ bool GetFileSize(string const & fName, uint64_t & sz) namespace { - bool CheckRemoveResult(int res, string const & fName) - { - if (0 != res) - { - // additional check if file really was removed correctly - uint64_t dummy; - if (GetFileSize(fName, dummy)) - { - LOG(LERROR, ("File exists but can't be deleted. Sharing violation?", fName)); - } +bool CheckFileOperationResult(int res, string const & fName) +{ + if (!res) + return true; +#if !defined(OMIM_OS_TIZEN) + LOG(LWARNING, ("File operation error for file:", fName, "-", strerror(errno))); +#endif - return false; - } - else - return true; + // additional check if file really was removed correctly + uint64_t dummy; + if (GetFileSize(fName, dummy)) + { + LOG(LERROR, ("File exists but can't be deleted. Sharing violation?", fName)); } + + return false; } +} // namespace bool DeleteFileX(string const & fName) { @@ -249,7 +251,7 @@ bool DeleteFileX(string const & fName) res = remove(fName.c_str()); #endif - return CheckRemoveResult(res, fName); + return CheckFileOperationResult(res, fName); } bool RenameFileX(string const & fOld, string const & fNew) @@ -262,7 +264,7 @@ bool RenameFileX(string const & fOld, string const & fNew) res = rename(fOld.c_str(), fNew.c_str()); #endif - return CheckRemoveResult(res, fOld); + return CheckFileOperationResult(res, fOld); } bool CopyFileX(string const & fOld, string const & fNew) diff --git a/drape_head/drape_surface.cpp b/drape_head/drape_surface.cpp index 712f7a54ae..8a41da185b 100644 --- a/drape_head/drape_surface.cpp +++ b/drape_head/drape_surface.cpp @@ -32,7 +32,7 @@ DrapeSurface::DrapeSurface() Platform & pl = GetPlatform(); pl.GetFilesByExt(pl.WritableDir(), DATA_FILE_EXTENSION, maps); - for_each(maps.begin(), maps.end(), bind(&model::FeaturesFetcher::AddMap, &m_model, _1)); + for_each(maps.begin(), maps.end(), bind(&model::FeaturesFetcher::RegisterMap, &m_model, _1)); ///} /// m_navigator.LoadState(); diff --git a/generator/generator_tests/check_mwms.cpp b/generator/generator_tests/check_mwms.cpp index 8596407013..45a3f417e6 100644 --- a/generator/generator_tests/check_mwms.cpp +++ b/generator/generator_tests/check_mwms.cpp @@ -23,7 +23,7 @@ UNIT_TEST(CheckMWM_LoadAll) { try { - m.AddMap(s); + m.RegisterMap(s); } catch (RootException const & ex) { diff --git a/generator/routing_generator.cpp b/generator/routing_generator.cpp index 6a17bbc426..56e38e40ca 100644 --- a/generator/routing_generator.cpp +++ b/generator/routing_generator.cpp @@ -226,7 +226,7 @@ void BuildRoutingIndex(string const & baseDir, string const & countryName, strin string const mwmFile = baseDir + countryName + DATA_FILE_EXTENSION; Index index; m2::RectD rect; - if (!index.Add(mwmFile, rect)) + if (!index.Register(mwmFile, rect)) { LOG(LCRITICAL, ("MWM file not found")); return; diff --git a/indexer/index.cpp b/indexer/index.cpp index 2d65e1286e..9116265a11 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -3,6 +3,8 @@ #include "../platform/platform.hpp" +#include "../base/logging.hpp" + #include "../coding/file_name_utils.hpp" #include "../coding/internal/file_data.hpp" @@ -76,6 +78,8 @@ Index::~Index() namespace { +// Deletes map file denoted by @path and all temporary files related +// to it. void DeleteMapFiles(string const & path, bool deleteReady) { (void)my::DeleteFileX(path); @@ -91,15 +95,17 @@ namespace return GetPlatform().WritablePathForFile(fileName); } + // Deletes all files related to @fileName and renames + // @fileName.READY_FILE_EXTENSION to @fileName. void ReplaceFileWithReady(string const & fileName) { string const path = GetFullPath(fileName); - DeleteMapFiles(path, false); - CHECK ( my::RenameFileX(path + READY_FILE_EXTENSION, path), (path) ); + DeleteMapFiles(path, false /* deleteReady */); + CHECK(my::RenameFileX(path + READY_FILE_EXTENSION, path), (path)); } } -int Index::AddMap(string const & fileName, m2::RectD & rect) +int Index::RegisterMap(string const & fileName, m2::RectD & rect) { if (GetPlatform().IsFileExistsByFullPath(GetFullPath(fileName + READY_FILE_EXTENSION))) { @@ -116,61 +122,80 @@ int Index::AddMap(string const & fileName, m2::RectD & rect) return ret; } } - else - return Add(fileName, rect); + + lock_guard lock(m_lock); + int rv = Register(fileName, rect); + if (rv >= 0) + m_observers.ForEach(&Observer::OnMapRegistered, fileName); + return rv; } bool Index::DeleteMap(string const & fileName) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); - if (!RemoveImpl(fileName)) + if (!DeregisterImpl(fileName)) return false; DeleteMapFiles(GetFullPath(fileName), true); + m_observers.ForEach(&Observer::OnMapDeleted, fileName); return true; } +bool Index::AddObserver(Observer & observer) +{ + return m_observers.Add(observer); +} + +bool Index::RemoveObserver(Observer & observer) +{ + return m_observers.Remove(observer); +} + int Index::UpdateMap(string const & fileName, m2::RectD & rect) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); MwmId const id = GetIdByName(fileName); if (id != INVALID_MWM_ID && m_info[id].m_lockCount > 0) { - m_info[id].m_status = MwmInfo::STATUS_UPDATE; + m_info[id].SetStatus(MwmInfo::STATUS_PENDING_UPDATE); return -2; } ReplaceFileWithReady(fileName); - return AddImpl(fileName, rect); + int rv = RegisterImpl(fileName, rect); + if (rv >= 0) + m_observers.ForEach(&Observer::OnMapUpdated, fileName); + return rv; } void Index::UpdateMwmInfo(MwmId id) { - switch (m_info[id].m_status) + switch (m_info[id].GetStatus()) { - case MwmInfo::STATUS_TO_REMOVE: - if (m_info[id].m_lockCount == 0) - { - DeleteMapFiles(m_name[id], true); + case MwmInfo::STATUS_MARKED_TO_DEREGISTER: + if (m_info[id].m_lockCount == 0) + { + string const name = m_name[id]; + DeleteMapFiles(name, true); + CHECK(DeregisterImpl(id), ()); + m_observers.ForEach(&Observer::OnMapDeleted, name); + } + break; - CHECK(RemoveImpl(id), ()); - } - break; + case MwmInfo::STATUS_PENDING_UPDATE: + if (m_info[id].m_lockCount == 0) + { + ClearCache(id); + ReplaceFileWithReady(m_name[id]); + m_info[id].SetStatus(MwmInfo::STATUS_UP_TO_DATE); + m_observers.ForEach(&Observer::OnMapUpdated, m_name[id]); + } + break; - case MwmInfo::STATUS_UPDATE: - if (m_info[id].m_lockCount == 0) - { - ClearCache(id); - - ReplaceFileWithReady(m_name[id]); - - m_info[id].m_status = MwmInfo::STATUS_ACTIVE; - } - break; + default: + break; } } diff --git a/indexer/index.hpp b/indexer/index.hpp index 000883f0d2..3bcb03d52c 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -1,19 +1,20 @@ #pragma once #include "cell_id.hpp" #include "data_factory.hpp" -#include "mwm_set.hpp" #include "feature_covering.hpp" #include "features_vector.hpp" +#include "mwm_set.hpp" #include "scale_index.hpp" #include "../coding/file_container.hpp" #include "../defines.hpp" -#include "../std/vector.hpp" -#include "../std/unordered_set.hpp" -#include "../std/algorithm.hpp" +#include "../base/observer_list.hpp" +#include "../std/algorithm.hpp" +#include "../std/unordered_set.hpp" +#include "../std/vector.hpp" class MwmValue : public MwmSet::MwmValueBase { @@ -39,6 +40,16 @@ protected: virtual void UpdateMwmInfo(MwmId id); public: + class Observer + { + public: + virtual ~Observer() {} + + virtual void OnMapRegistered(string const & file) {} + virtual void OnMapUpdated(string const & file) {} + virtual void OnMapDeleted(string const & file) {} + }; + Index(); ~Index(); @@ -59,14 +70,34 @@ public: string GetFileName() const; }; - /// @return mwm format version or -1 if file isn't suitable (greater version). - int AddMap(string const & fileName, m2::RectD & rect); - /// @return mwm format version or - /// -1 if file isn't suitable (greater version). - /// -2 if file is busy now (delayed update). + /// Registers new map. + /// + /// \return mwm format version or -1 if file isn't suitable (greater + /// version). + int RegisterMap(string const & fileName, m2::RectD & rect); + + /// Replaces map file corresponding to fileName with a new one, when + /// it's possible - no clients of the map file. Otherwise, update + /// will be delayed. + /// + /// \return mwm format version or -1 if file isn't suitable (greater + /// version). -2 if file is busy now (delayed update). int UpdateMap(string const & fileName, m2::RectD & rect); + + /// Deletes map both from file system and internal tables, also, + /// deletes all files related to the map. If map was successfully + /// deleted, notifies observers. + /// + /// \param fileName A fileName denoting the map to be deleted, may + /// be a full path or a short path relative to + /// executable's directories. + //// \return True if map was successfully deleted. bool DeleteMap(string const & fileName); + bool AddObserver(Observer & observer); + + bool RemoveObserver(Observer & observer); + private: template @@ -245,9 +276,8 @@ public: MwmId GetMwmIdByName(string const & name) const { Index * p = const_cast(this); + lock_guard lock(p->m_lock); - threads::MutexGuard guard(p->m_lock); - UNUSED_VALUE(guard); ASSERT(p->GetIdByName(name) != INVALID_MWM_ID, ("Can't get mwm identifier")); return p->GetIdByName(name); } @@ -351,4 +381,6 @@ private: f(lock, cov, scale); } } + + my::ObserverList m_observers; }; diff --git a/indexer/indexer_tests/index_builder_test.cpp b/indexer/indexer_tests/index_builder_test.cpp index af5683f3fb..887518bde9 100644 --- a/indexer/indexer_tests/index_builder_test.cpp +++ b/indexer/indexer_tests/index_builder_test.cpp @@ -58,7 +58,7 @@ UNIT_TEST(BuildIndexTest) { // Check that index actually works. Index index; - index.Add(fileName); + index.Register(fileName); // Make sure that index is actually parsed. NoopFunctor fn; diff --git a/indexer/indexer_tests/index_test.cpp b/indexer/indexer_tests/index_test.cpp index a29af92fc5..eeec1fe9e7 100644 --- a/indexer/indexer_tests/index_test.cpp +++ b/indexer/indexer_tests/index_test.cpp @@ -1,18 +1,97 @@ #include "../../testing/testing.hpp" #include "../index.hpp" +#include "../../coding/file_name_utils.hpp" +#include "../../coding/internal/file_data.hpp" + +#include "../../platform/platform.hpp" + +#include "../../base/logging.hpp" #include "../../base/macros.hpp" +#include "../../base/scope_guard.hpp" #include "../../base/stl_add.hpp" +#include "../../std/bind.hpp" #include "../../std/string.hpp" +void CheckedDeleteFile(string const & file) +{ + if (Platform::IsFileExistsByFullPath(file)) + CHECK(my::DeleteFileX(file), ("Can't remove file:", file)); +} -UNIT_TEST(IndexParseTest) +class Observer : public Index::Observer +{ +public: + Observer() : m_map_registered_calls(0), m_map_updated_calls(0), m_map_deleted_calls(0) {} + + // Index::Observer overrides: + void OnMapRegistered(string const & file) override { ++m_map_registered_calls; } + void OnMapUpdated(string const & file) override { ++m_map_updated_calls; } + void OnMapDeleted(string const & file) override { ++m_map_deleted_calls; } + + int map_registered_calls() const { return m_map_registered_calls; } + int map_updated_calls() const { return m_map_updated_calls; } + int map_deleted_calls() const { return m_map_deleted_calls; } + +private: + int m_map_registered_calls; + int m_map_updated_calls; + int m_map_deleted_calls; +}; + +UNIT_TEST(Index_Parse) { Index index; - index.Add("minsk-pass" DATA_FILE_EXTENSION); + + m2::RectD dummyRect; + index.RegisterMap("minsk-pass" DATA_FILE_EXTENSION, dummyRect); // Make sure that index is actually parsed. NoopFunctor fn; index.ForEachInScale(fn, 15); } + +UNIT_TEST(Index_MwmStatusNotifications) +{ + string const resourcesDir = GetPlatform().ResourcesDir(); + string const sourceMapName = "minsk-pass" DATA_FILE_EXTENSION; + string const sourceMapPath = my::JoinFoldersToPath(resourcesDir, sourceMapName); + string const testMapName = "minsk-pass-copy" DATA_FILE_EXTENSION; + string const testMapPath = my::JoinFoldersToPath(resourcesDir, testMapName); + string const testMapUpdatePath = testMapPath + READY_FILE_EXTENSION; + + TEST(my::CopyFileX(sourceMapPath, testMapPath), ()); + MY_SCOPE_GUARD(testMapGuard, bind(&CheckedDeleteFile, testMapPath)); + + Index index; + Observer observer; + index.AddObserver(observer); + + TEST_EQUAL(0, observer.map_registered_calls(), ()); + m2::RectD dummyRect; + + // Check that observers are triggered after map registration. + TEST_LESS_OR_EQUAL(0, index.RegisterMap(testMapPath, dummyRect), ()); + TEST_EQUAL(1, observer.map_registered_calls(), ()); + + // Check that map can't registered twice and observers aren't + // triggered. + TEST_EQUAL(-1, index.RegisterMap(testMapPath, dummyRect), ()); + TEST_EQUAL(1, observer.map_registered_calls(), ()); + + TEST(my::CopyFileX(testMapPath, testMapUpdatePath), ()); + MY_SCOPE_GUARD(testMapUpdateGuard, bind(&CheckedDeleteFile, testMapUpdatePath)); + + // Check that observers are notified when map is deleted. + TEST_EQUAL(0, observer.map_updated_calls(), ()); + TEST_LESS_OR_EQUAL(0, index.UpdateMap(testMapName, dummyRect), ()); + TEST_EQUAL(1, observer.map_updated_calls(), ()); + + // Check that observers are notified when map is deleted. + TEST_EQUAL(0, observer.map_deleted_calls(), ()); + TEST(index.DeleteMap(testMapName), ()); + TEST_EQUAL(1, observer.map_deleted_calls(), ()); + + index.RemoveObserver(observer); +} diff --git a/indexer/indexer_tests/mwm_set_test.cpp b/indexer/indexer_tests/mwm_set_test.cpp index c630952b1c..12eb87a7a3 100644 --- a/indexer/indexer_tests/mwm_set_test.cpp +++ b/indexer/indexer_tests/mwm_set_test.cpp @@ -37,16 +37,16 @@ UNIT_TEST(MwmSetSmokeTest) TestMwmSet mwmSet; vector info; - mwmSet.Add("0"); - mwmSet.Add("1"); - mwmSet.Add("2"); - mwmSet.Remove("1"); + mwmSet.Register("0"); + mwmSet.Register("1"); + mwmSet.Register("2"); + mwmSet.Deregister("1"); mwmSet.GetMwmInfo(info); TEST_EQUAL(info.size(), 3, ()); - TEST(info[0].IsActive(), ()); + TEST(info[0].IsUpToDate(), ()); TEST_EQUAL(info[0].m_maxScale, 0, ()); - TEST(!info[1].IsActive(), ()); - TEST(info[2].IsActive(), ()); + TEST(!info[1].IsUpToDate(), ()); + TEST(info[2].IsUpToDate(), ()); TEST_EQUAL(info[2].m_maxScale, 2, ()); { MwmSet::MwmLock lock0(mwmSet, 0); @@ -55,41 +55,41 @@ UNIT_TEST(MwmSetSmokeTest) TEST(lock1.GetValue() == NULL, ()); } - mwmSet.Add("3"); + mwmSet.Register("3"); mwmSet.GetMwmInfo(info); TEST_EQUAL(info.size(), 3, ()); - TEST(info[0].IsActive(), ()); + TEST(info[0].IsUpToDate(), ()); TEST_EQUAL(info[0].m_maxScale, 0, ()); - TEST(info[1].IsActive(), ()); + TEST(info[1].IsUpToDate(), ()); TEST_EQUAL(info[1].m_maxScale, 3, ()); - TEST(info[2].IsActive(), ()); + TEST(info[2].IsUpToDate(), ()); TEST_EQUAL(info[2].m_maxScale, 2, ()); { MwmSet::MwmLock lock(mwmSet, 1); TEST(lock.GetValue() != NULL, ()); - mwmSet.Remove("3"); - mwmSet.Add("4"); + mwmSet.Deregister("3"); + mwmSet.Register("4"); } mwmSet.GetMwmInfo(info); TEST_EQUAL(info.size(), 4, ()); - TEST(info[0].IsActive(), ()); + TEST(info[0].IsUpToDate(), ()); TEST_EQUAL(info[0].m_maxScale, 0, ()); - TEST(!info[1].IsActive(), ()); - TEST(info[2].IsActive(), ()); + TEST(!info[1].IsUpToDate(), ()); + TEST(info[2].IsUpToDate(), ()); TEST_EQUAL(info[2].m_maxScale, 2, ()); - TEST(info[3].IsActive(), ()); + TEST(info[3].IsUpToDate(), ()); TEST_EQUAL(info[3].m_maxScale, 4, ()); - mwmSet.Add("5"); + mwmSet.Register("5"); mwmSet.GetMwmInfo(info); TEST_EQUAL(info.size(), 4, ()); - TEST(info[0].IsActive(), ()); + TEST(info[0].IsUpToDate(), ()); TEST_EQUAL(info[0].m_maxScale, 0, ()); - TEST(info[1].IsActive(), ()); + TEST(info[1].IsUpToDate(), ()); TEST_EQUAL(info[1].m_maxScale, 5, ()); - TEST(info[2].IsActive(), ()); + TEST(info[2].IsUpToDate(), ()); TEST_EQUAL(info[2].m_maxScale, 2, ()); - TEST(info[3].IsActive(), ()); + TEST(info[3].IsUpToDate(), ()); TEST_EQUAL(info[3].m_maxScale, 4, ()); } diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp index f38056f311..46d9bea017 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -9,11 +9,10 @@ #include "../std/algorithm.hpp" - -MwmInfo::MwmInfo() : m_lockCount(0), m_status(STATUS_REMOVED) +MwmInfo::MwmInfo() : m_lockCount(0), m_status(STATUS_DEREGISTERED) { - // Important: STATUS_REMOVED - is the default value. - // Apply STATUS_ACTIVE before adding to maps container. + // Important: STATUS_DEREGISTERED - is the default value. + // Apply STATUS_UP_TO_DATE before adding to maps container. } MwmInfo::MwmTypeT MwmInfo::GetType() const @@ -24,7 +23,6 @@ MwmInfo::MwmTypeT MwmInfo::GetType() const return COASTS; } - MwmSet::MwmLock::MwmLock(MwmSet & mwmSet, MwmId mwmId) : m_mwmSet(mwmSet), m_id(mwmId), m_pValue(mwmSet.LockValue(mwmId)) { @@ -50,15 +48,14 @@ MwmSet::~MwmSet() void MwmSet::Cleanup() { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); ClearCacheImpl(m_cache.begin(), m_cache.end()); #ifdef DEBUG for (MwmId i = 0; i < m_info.size(); ++i) { - if (m_info[i].IsActive()) + if (m_info[i].IsUpToDate()) { ASSERT_EQUAL(m_info[i].m_lockCount, 0, (i, m_name[i])); ASSERT_NOT_EQUAL(m_name[i], string(), (i)); @@ -69,8 +66,8 @@ void MwmSet::Cleanup() void MwmSet::UpdateMwmInfo(MwmId id) { - if (m_info[id].m_status == MwmInfo::STATUS_TO_REMOVE) - (void)RemoveImpl(id); + if (m_info[id].GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER) + (void)DeregisterImpl(id); } MwmSet::MwmId MwmSet::GetFreeId() @@ -78,7 +75,7 @@ MwmSet::MwmId MwmSet::GetFreeId() MwmId const size = m_info.size(); for (MwmId i = 0; i < size; ++i) { - if (m_info[i].m_status == MwmInfo::STATUS_REMOVED) + if (m_info[i].GetStatus() == MwmInfo::STATUS_DEREGISTERED) return i; } @@ -97,7 +94,7 @@ MwmSet::MwmId MwmSet::GetIdByName(string const & name) if (m_name[i] == name) { - ASSERT_NOT_EQUAL ( m_info[i].m_status, MwmInfo::STATUS_REMOVED, () ); + ASSERT_NOT_EQUAL(m_info[i].GetStatus(), MwmInfo::STATUS_DEREGISTERED, ()); return i; } } @@ -105,26 +102,25 @@ MwmSet::MwmId MwmSet::GetIdByName(string const & name) return INVALID_MWM_ID; } -int MwmSet::Add(string const & fileName, m2::RectD & rect) +int MwmSet::Register(string const & fileName, m2::RectD & rect) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); MwmId const id = GetIdByName(fileName); if (id != INVALID_MWM_ID) { - if (m_info[id].IsExist()) + if (m_info[id].IsRegistered()) LOG(LWARNING, ("Trying to add already added map", fileName)); else - m_info[id].m_status = MwmInfo::STATUS_ACTIVE; + m_info[id].SetStatus(MwmInfo::STATUS_UP_TO_DATE); return -1; } - return AddImpl(fileName, rect); + return RegisterImpl(fileName, rect); } -int MwmSet::AddImpl(string const & fileName, m2::RectD & rect) +int MwmSet::RegisterImpl(string const & fileName, m2::RectD & rect) { // this function can throw an exception for bad mwm file MwmInfo info; @@ -132,7 +128,7 @@ int MwmSet::AddImpl(string const & fileName, m2::RectD & rect) if (version == -1) return -1; - info.m_status = MwmInfo::STATUS_ACTIVE; + info.SetStatus(MwmInfo::STATUS_UP_TO_DATE); MwmId const id = GetFreeId(); m_name[id] = fileName; @@ -143,37 +139,35 @@ int MwmSet::AddImpl(string const & fileName, m2::RectD & rect) return version; } -bool MwmSet::RemoveImpl(MwmId id) +bool MwmSet::DeregisterImpl(MwmId id) { if (m_info[id].m_lockCount == 0) { m_name[id].clear(); - m_info[id].m_status = MwmInfo::STATUS_REMOVED; + m_info[id].SetStatus(MwmInfo::STATUS_DEREGISTERED); return true; } else { - m_info[id].m_status = MwmInfo::STATUS_TO_REMOVE; + m_info[id].SetStatus(MwmInfo::STATUS_MARKED_TO_DEREGISTER); return false; } } -void MwmSet::Remove(string const & fileName) +void MwmSet::Deregister(string const & fileName) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); - - (void)RemoveImpl(fileName); + lock_guard lock(m_lock); + (void)DeregisterImpl(fileName); } -bool MwmSet::RemoveImpl(string const & fileName) +bool MwmSet::DeregisterImpl(string const & fileName) { bool ret = true; MwmId const id = GetIdByName(fileName); if (id != INVALID_MWM_ID) { - ret = RemoveImpl(id); + ret = DeregisterImpl(id); ClearCache(id); } @@ -181,13 +175,12 @@ bool MwmSet::RemoveImpl(string const & fileName) return ret; } -void MwmSet::RemoveAll() +void MwmSet::DeregisterAll() { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); for (MwmId i = 0; i < m_info.size(); ++i) - (void)RemoveImpl(i); + (void)DeregisterImpl(i); // do not call ClearCache - it's under mutex lock ClearCacheImpl(m_cache.begin(), m_cache.end()); @@ -196,20 +189,16 @@ void MwmSet::RemoveAll() bool MwmSet::IsLoaded(string const & file) const { MwmSet * p = const_cast(this); - - threads::MutexGuard mutexGuard(p->m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(p->m_lock); MwmId const id = p->GetIdByName(file + DATA_FILE_EXTENSION); - return (id != INVALID_MWM_ID && m_info[id].IsExist()); + return (id != INVALID_MWM_ID && m_info[id].IsRegistered()); } void MwmSet::GetMwmInfo(vector & info) const { MwmSet * p = const_cast(this); - - threads::MutexGuard mutexGuard(p->m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(p->m_lock); for (MwmId i = 0; i < m_info.size(); ++i) p->UpdateMwmInfo(i); @@ -219,15 +208,14 @@ void MwmSet::GetMwmInfo(vector & info) const MwmSet::MwmValueBase * MwmSet::LockValue(MwmId id) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); ASSERT_LESS(id, m_info.size(), ()); if (id >= m_info.size()) return NULL; UpdateMwmInfo(id); - if (!m_info[id].IsActive()) + if (!m_info[id].IsUpToDate()) return NULL; ++m_info[id].m_lockCount; @@ -247,8 +235,7 @@ MwmSet::MwmValueBase * MwmSet::LockValue(MwmId id) void MwmSet::UnlockValue(MwmId id, MwmValueBase * p) { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); ASSERT(p, (id)); ASSERT_LESS(id, m_info.size(), ()); @@ -260,7 +247,7 @@ void MwmSet::UnlockValue(MwmId id, MwmValueBase * p) --m_info[id].m_lockCount; UpdateMwmInfo(id); - if (m_info[id].IsActive()) + if (m_info[id].IsUpToDate()) { m_cache.push_back(make_pair(id, p)); if (m_cache.size() > m_cacheSize) @@ -276,8 +263,7 @@ void MwmSet::UnlockValue(MwmId id, MwmValueBase * p) void MwmSet::ClearCache() { - threads::MutexGuard mutexGuard(m_lock); - UNUSED_VALUE(mutexGuard); + lock_guard lock(m_lock); ClearCacheImpl(m_cache.begin(), m_cache.end()); } diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp index 4799dcc378..f3597c7f5b 100644 --- a/indexer/mwm_set.hpp +++ b/indexer/mwm_set.hpp @@ -1,43 +1,54 @@ #pragma once #include "../geometry/rect2d.hpp" -#include "../base/mutex.hpp" - #include "../std/deque.hpp" +#include "../std/mutex.hpp" #include "../std/string.hpp" #include "../std/utility.hpp" #include "../std/vector.hpp" - /// Information about stored mwm. class MwmInfo { public: - MwmInfo(); - - m2::RectD m_limitRect; ///< Limit rect of mwm. - uint8_t m_minScale; ///< Min zoom level of mwm. - uint8_t m_maxScale; ///< Max zoom level of mwm. - - inline bool IsExist() const + enum MwmTypeT { - return (m_status == STATUS_ACTIVE || m_status == STATUS_UPDATE); - } - inline bool IsActive() const { return (m_status == STATUS_ACTIVE); } - - enum MwmTypeT { COUNTRY, WORLD, COASTS }; - MwmTypeT GetType() const; + COUNTRY, + WORLD, + COASTS + }; enum Status { - STATUS_ACTIVE, - STATUS_TO_REMOVE, - STATUS_REMOVED, - STATUS_UPDATE + STATUS_UP_TO_DATE, ///< Mwm is registered and up-to-date + STATUS_MARKED_TO_DEREGISTER, ///< Mwm is marked to be deregistered as soon as possible + STATUS_DEREGISTERED, ///< Mwm is deregistered + STATUS_PENDING_UPDATE ///< Mwm is registered but there're a pending update to it }; - uint8_t m_lockCount; ///< Number of locks. - uint8_t m_status; ///< Current country status. + MwmInfo(); + + m2::RectD m_limitRect; ///< Limit rect of mwm. + uint8_t m_minScale; ///< Min zoom level of mwm. + uint8_t m_maxScale; ///< Max zoom level of mwm. + + inline bool IsRegistered() const + { + return m_status == STATUS_UP_TO_DATE || m_status == STATUS_PENDING_UPDATE; + } + + inline bool IsUpToDate() const { return m_status == STATUS_UP_TO_DATE; } + + MwmTypeT GetType() const; + + inline void SetStatus(Status status) { m_status = status; } + + inline Status GetStatus() const { return m_status; } + + uint8_t m_lockCount; ///< Number of locks. + +private: + Status m_status; ///< Current country status. }; class MwmSet @@ -70,37 +81,39 @@ public: MwmValueBase * m_pValue; }; - /// Add new map. + /// Registers new map in the set. /// @param[in] fileName File name (without full path) of country. /// @param[out] rect Limit rect of country. /// @return Map format version or -1 if not added (already exists). //@{ protected: - int AddImpl(string const & fileName, m2::RectD & rect); + int RegisterImpl(string const & fileName, m2::RectD & rect); public: - int Add(string const & fileName, m2::RectD & rect); + int Register(string const & fileName, m2::RectD & rect); //@} /// Used in unit tests only. - inline void Add(string const & fileName) + inline void Register(string const & fileName) { m2::RectD dummy; - CHECK(Add(fileName, dummy), ()); + CHECK(Register(fileName, dummy), ()); } /// @name Remove mwm. //@{ protected: + /// Deregisters map from the set when it' possible. Note that + /// underlying file is not deleted. /// @return true - map is free; false - map is busy //@{ - bool RemoveImpl(MwmId id); - bool RemoveImpl(string const & fileName); + bool DeregisterImpl(MwmId id); + bool DeregisterImpl(string const & fileName); //@} public: - void Remove(string const & fileName); - void RemoveAll(); + void Deregister(string const & fileName); + void DeregisterAll(); //@} /// @param[in] file File name without extension. @@ -152,6 +165,8 @@ protected: virtual void UpdateMwmInfo(MwmId id); vector m_info; + vector m_name; - threads::Mutex m_lock; + + mutex m_lock; }; diff --git a/map/benchmark_engine.cpp b/map/benchmark_engine.cpp index 50598c3493..511035d11f 100644 --- a/map/benchmark_engine.cpp +++ b/map/benchmark_engine.cpp @@ -125,14 +125,14 @@ struct MapsCollector void BenchmarkEngine::PrepareMaps() { - // remove all previously added maps in framework constructor - m_framework->RemoveMaps(); + // Deregister all previously registered maps in framework constructor. + m_framework->DeregisterAllMaps(); // add only maps needed for benchmarks MapsCollector collector; ForEachBenchmarkRecord(collector); for_each(collector.m_maps.begin(), collector.m_maps.end(), - bind(&Framework::AddMap, m_framework, _1)); + bind(&Framework::RegisterMap, m_framework, _1)); } BenchmarkEngine::BenchmarkEngine(Framework * fw) diff --git a/map/benchmark_tool/features_loading.cpp b/map/benchmark_tool/features_loading.cpp index b607d1e199..0f56adcc04 100644 --- a/map/benchmark_tool/features_loading.cpp +++ b/map/benchmark_tool/features_loading.cpp @@ -109,7 +109,7 @@ void RunFeaturesLoadingBenchmark(string const & file, pair scaleR, All return; model::FeaturesFetcher src; - src.AddMap(file); + src.RegisterMap(file); RunBenchmark(src, header.GetBounds(), scaleR, res); } diff --git a/map/feature_vec_model.cpp b/map/feature_vec_model.cpp index c4397282bd..5acb0699a3 100644 --- a/map/feature_vec_model.cpp +++ b/map/feature_vec_model.cpp @@ -32,14 +32,14 @@ void FeaturesFetcher::InitClassificator() } } -int FeaturesFetcher::AddMap(string const & file) +int FeaturesFetcher::RegisterMap(string const & file) { int version = -1; try { m2::RectD r; - version = m_multiIndex.AddMap(file, r); + version = m_multiIndex.RegisterMap(file, r); if (version != -1) m_rect.Add(r); @@ -54,10 +54,9 @@ int FeaturesFetcher::AddMap(string const & file) return version; } -void FeaturesFetcher::RemoveMap(string const & file) -{ - m_multiIndex.Remove(file); -} +void FeaturesFetcher::DeregisterMap(string const & file) { m_multiIndex.Deregister(file); } + +void FeaturesFetcher::DeregisterAllMaps() { m_multiIndex.DeregisterAll(); } bool FeaturesFetcher::DeleteMap(string const & file) { @@ -66,12 +65,7 @@ bool FeaturesFetcher::DeleteMap(string const & file) bool FeaturesFetcher::UpdateMap(string const & file, m2::RectD & rect) { - return (m_multiIndex.UpdateMap(file, rect) >= 0); -} - -void FeaturesFetcher::RemoveAll() -{ - m_multiIndex.RemoveAll(); + return m_multiIndex.UpdateMap(file, rect) >= 0; } //void FeaturesFetcher::Clean() diff --git a/map/feature_vec_model.hpp b/map/feature_vec_model.hpp index f41b8a8440..b786ced872 100644 --- a/map/feature_vec_model.hpp +++ b/map/feature_vec_model.hpp @@ -8,7 +8,6 @@ #include "../coding/reader.hpp" #include "../coding/buffer_reader.hpp" - namespace model { //#define USE_BUFFER_READER @@ -33,11 +32,24 @@ namespace model /// @param[in] file Name of mwm file with extension. //@{ /// @return MWM format version for file or -1 if error and map was not added - int AddMap(string const & file); - void RemoveMap(string const & file); - void RemoveAll(); + int RegisterMap(string const & file); + /// Deregisters map denoted by file from internal records. + void DeregisterMap(string const & file); + + /// Deregisters all registered maps. + void DeregisterAllMaps(); + + /// Deletes all files related to map denoted by file. + /// + /// \return True if map was successfully deleted. bool DeleteMap(string const & file); + + /// Tries to update map denoted by file. If map is used right now, + /// update will be delayed. + /// + /// \return True when map was successfully updated, false when update + /// was delayed or when update's version is not good. bool UpdateMap(string const & file, m2::RectD & rect); //@} diff --git a/map/framework.cpp b/map/framework.cpp index 21351013a8..551b2730b1 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -84,17 +84,17 @@ namespace static const int BM_TOUCH_PIXEL_INCREASE = 20; } -int Framework::AddMap(string const & file) +int Framework::RegisterMap(string const & file) { LOG(LINFO, ("Loading map:", file)); - int const version = m_model.AddMap(file); + int const version = m_model.RegisterMap(file); if (version == feature::DataHeader::v1) { // Now we do force delete of old (April 2011) maps. LOG(LINFO, ("Deleting old map:", file)); - RemoveMap(file); + DeregisterMap(file); VERIFY(my::DeleteFileX(GetPlatform().WritablePathForFile(file)), ()); return -1; @@ -103,10 +103,7 @@ int Framework::AddMap(string const & file) return version; } -void Framework::RemoveMap(string const & file) -{ - m_model.RemoveMap(file); -} +void Framework::DeregisterMap(string const & file) { m_model.DeregisterMap(file); } void Framework::OnLocationError(TLocationError /*error*/) { @@ -262,7 +259,7 @@ Framework::Framework() LOG(LDEBUG, ("Search engine initialized")); #ifndef OMIM_OS_ANDROID - AddMaps(); + RegisterAllMaps(); #endif LOG(LDEBUG, ("Maps initialized")); @@ -401,7 +398,7 @@ void Framework::UpdateAfterDownload(string const & fileName, TMapOptions opt) DeleteCountryIndexes(countryName); } -void Framework::AddMaps() +void Framework::RegisterAllMaps() { //ASSERT(m_model.IsEmpty(), ()); @@ -409,9 +406,9 @@ void Framework::AddMaps() vector maps; GetMaps(maps); - for_each(maps.begin(), maps.end(), [&] (string const & file) + for_each(maps.begin(), maps.end(), [&](string const & file) { - int const v = AddMap(file); + int const v = RegisterMap(file); if (v != -1 && v < minVersion) minVersion = v; }); @@ -421,10 +418,10 @@ void Framework::AddMaps() GetSearchEngine()->SupportOldFormat(minVersion < feature::DataHeader::v3); } -void Framework::RemoveMaps() +void Framework::DeregisterAllMaps() { m_countryTree.Clear(); - m_model.RemoveAll(); + m_model.DeregisterAllMaps(); } void Framework::LoadBookmarks() diff --git a/map/framework.hpp b/map/framework.hpp index d75d17b98f..503e82ef9f 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -148,7 +148,7 @@ protected: void ClearAllCaches(); - void RemoveMap(string const & file); + void DeregisterMap(string const & file); /// Deletes user calculated indexes on country updates void DeleteCountryIndexes(string const & mwmName); @@ -162,13 +162,20 @@ public: /// @param[out] maps File names without path. void GetMaps(vector & maps) const; - void AddMaps(); - void RemoveMaps(); + /// Registers all local map files in internal indexes. + void RegisterAllMaps(); + /// Deregisters all registered map files. + void DeregisterAllMaps(); + + /// Registers local map file in internal indexes. + /// /// @return Inner mwm data version from header or -1 in case of error. - int AddMap(string const & file); + int RegisterMap(string const & file); //@} + /// Deletes all disk files corresponding to country. + /// /// @name This functions is used by Downloader UI. //@{ /// options - flags that signal about parts of map that must be deleted diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index 4cf5fa6b53..ee7cd28d8e 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -397,8 +397,8 @@ UNIT_TEST(Bookmarks_AddressInfo) { // Maps added in constructor (we need minsk-pass.mwm only) Framework fm; - fm.RemoveMaps(); - fm.AddMap("minsk-pass.mwm"); + fm.DeregisterAllMaps(); + fm.RegisterMap("minsk-pass.mwm"); fm.OnSize(800, 600); diff --git a/map/mwm_tests/multithread_mwm_test.cpp b/map/mwm_tests/multithread_mwm_test.cpp index d4b0b4c609..ea5b9706e5 100644 --- a/map/mwm_tests/multithread_mwm_test.cpp +++ b/map/mwm_tests/multithread_mwm_test.cpp @@ -62,7 +62,7 @@ namespace { SourceT src; src.InitClassificator(); - src.AddMap(file + DATA_FILE_EXTENSION); + src.RegisterMap(file + DATA_FILE_EXTENSION); // Check that country rect is valid and not infinity. m2::RectD const r = src.GetWorldRect(); diff --git a/map/mwm_tests/mwm_foreach_test.cpp b/map/mwm_tests/mwm_foreach_test.cpp index 84520cb665..15c2544299 100644 --- a/map/mwm_tests/mwm_foreach_test.cpp +++ b/map/mwm_tests/mwm_foreach_test.cpp @@ -250,7 +250,7 @@ void RunTest(string const & file) { model::FeaturesFetcher src1; src1.InitClassificator(); - src1.AddMap(file); + src1.RegisterMap(file); vector rects; rects.push_back(src1.GetWorldRect()); diff --git a/map/mwm_tests/mwm_index_test.cpp b/map/mwm_tests/mwm_index_test.cpp index 0d1c1ed093..f162d4c39b 100644 --- a/map/mwm_tests/mwm_index_test.cpp +++ b/map/mwm_tests/mwm_index_test.cpp @@ -39,7 +39,7 @@ public: bool RunTest(string const & fileName, int lowS, int highS) { model::FeaturesFetcher src; - if (src.AddMap(fileName) == -1) + if (src.RegisterMap(fileName) == -1) return false; CheckNonEmptyGeometry doCheck; diff --git a/search/search_tests/house_detector_tests.cpp b/search/search_tests/house_detector_tests.cpp index 7c7e5f54a5..689a79d140 100644 --- a/search/search_tests/house_detector_tests.cpp +++ b/search/search_tests/house_detector_tests.cpp @@ -184,7 +184,7 @@ UNIT_TEST(HS_StreetsMerge) Index index; m2::RectD rect; - TEST(index.Add("minsk-pass.mwm", rect), ()); + TEST(index.Register("minsk-pass.mwm", rect), ()); { search::HouseDetector houser(&index); @@ -272,7 +272,7 @@ UNIT_TEST(HS_FindHouseSmoke) Index index; m2::RectD rect; - index.Add("minsk-pass.mwm", rect); + index.Register("minsk-pass.mwm", rect); { vector streetName(1, "Московская улица"); @@ -374,7 +374,7 @@ UNIT_TEST(HS_MWMSearch) Index index; m2::RectD rect; - if (!index.Add(country + ".mwm", rect)) + if (!index.Register(country + ".mwm", rect)) { LOG(LWARNING, ("MWM file not found")); return; diff --git a/search/search_tests/locality_finder_test.cpp b/search/search_tests/locality_finder_test.cpp index 61b6aba122..10de3fd69e 100644 --- a/search/search_tests/locality_finder_test.cpp +++ b/search/search_tests/locality_finder_test.cpp @@ -33,7 +33,7 @@ UNIT_TEST(LocalityFinder) { Index index; m2::RectD rect; - TEST(index.Add("World.mwm", rect), ()); + TEST(index.Register("World.mwm", rect), ()); search::LocalityFinder finder(&index); finder.SetLanguage(StringUtf8Multilang::GetLangIndex("en"));