diff options
author | gorshenin <y.gorshenin@corp.mail.ru> | 2015-07-01 16:48:58 +0300 |
---|---|---|
committer | Alex Zolotarev <alex@maps.me> | 2015-09-23 02:53:27 +0300 |
commit | a7bcdb5ae8b8e75c1a3806b00b19f3b80c72c5b3 (patch) | |
tree | e24caedef110b579fcedc15d29c62cc43e3f6d9c /indexer | |
parent | e0470160313b3ed0bc396ffec72b16fded6b8520 (diff) |
Revert "[storage, framework, index] Storage redesign. Plain strings are replaced to CountryFile and LocalCountryFile."
Diffstat (limited to 'indexer')
-rw-r--r-- | indexer/index.cpp | 142 | ||||
-rw-r--r-- | indexer/index.hpp | 83 | ||||
-rw-r--r-- | indexer/indexer_tests/index_builder_test.cpp | 2 | ||||
-rw-r--r-- | indexer/indexer_tests/index_test.cpp | 141 | ||||
-rw-r--r-- | indexer/indexer_tests/mwm_set_test.cpp | 67 | ||||
-rw-r--r-- | indexer/mwm_set.cpp | 130 | ||||
-rw-r--r-- | indexer/mwm_set.hpp | 86 |
7 files changed, 380 insertions, 271 deletions
diff --git a/indexer/index.cpp b/indexer/index.cpp index 12d0a02fe7..e27814dc74 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -6,27 +6,32 @@ #include "coding/file_name_utils.hpp" #include "coding/internal/file_data.hpp" -using platform::CountryFile; -using platform::LocalCountryFile; ////////////////////////////////////////////////////////////////////////////////// // MwmValue implementation ////////////////////////////////////////////////////////////////////////////////// -MwmValue::MwmValue(LocalCountryFile const & localFile) - : m_cont(GetPlatform().GetCountryReader(localFile, TMapOptions::EMap)), - m_countryFile(localFile.GetCountryFile()) +MwmValue::MwmValue(string const & name) + : m_cont(GetPlatform().GetReader(name)) { m_factory.Load(m_cont); } +string MwmValue::GetFileName() const +{ + string s = m_cont.GetFileName(); + my::GetNameFromFullPath(s); + my::GetNameWithoutExt(s); + return s; +} + ////////////////////////////////////////////////////////////////////////////////// // Index implementation ////////////////////////////////////////////////////////////////////////////////// -bool Index::GetVersion(LocalCountryFile const & localFile, MwmInfo & info) const +bool Index::GetVersion(string const & name, MwmInfo & info) const { - MwmValue value(localFile); + MwmValue value(name); feature::DataHeader const & h = value.GetHeader(); if (!h.IsMWMSuitable()) @@ -42,9 +47,9 @@ bool Index::GetVersion(LocalCountryFile const & localFile, MwmInfo & info) const return true; } -MwmSet::TMwmValueBasePtr Index::CreateValue(LocalCountryFile const & localFile) const +MwmSet::TMwmValueBasePtr Index::CreateValue(string const & name) const { - TMwmValueBasePtr p(new MwmValue(localFile)); + TMwmValueBasePtr p(new MwmValue(name)); ASSERT(static_cast<MwmValue &>(*p.get()).GetHeader().IsMWMSuitable(), ()); return p; } @@ -58,23 +63,124 @@ Index::~Index() Cleanup(); } -pair<MwmSet::MwmLock, bool> Index::RegisterMap(LocalCountryFile const & localFile) +namespace { - pair<MwmSet::MwmLock, bool> result = Register(localFile); - if (result.first.IsLocked() && result.second) - m_observers.ForEach(&Observer::OnMapRegistered, localFile); + // Deletes map file denoted by @path and all temporary files related + // to it. + void DeleteMapFiles(string const & path, bool deleteReady) + { + (void)my::DeleteFileX(path); + (void)my::DeleteFileX(path + RESUME_FILE_EXTENSION); + (void)my::DeleteFileX(path + DOWNLOADING_FILE_EXTENSION); + + if (deleteReady) + (void)my::DeleteFileX(path + READY_FILE_EXTENSION); + } + + string GetFullPath(string const & fileName) + { + 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 /* deleteReady */); + CHECK(my::RenameFileX(path + READY_FILE_EXTENSION, path), (path)); + } +} + +pair<MwmSet::MwmLock, bool> Index::RegisterMap(string const & fileName) +{ + if (GetPlatform().IsFileExistsByFullPath(GetFullPath(fileName + READY_FILE_EXTENSION))) + { + pair<MwmSet::MwmLock, UpdateStatus> updateResult = UpdateMap(fileName); + switch (updateResult.second) + { + case UPDATE_STATUS_OK: + return make_pair(move(updateResult.first), true); + case UPDATE_STATUS_BAD_FILE: + return make_pair(move(updateResult.first), false); + case UPDATE_STATUS_UPDATE_DELAYED: + // Not dangerous, but it's strange when adding existing maps. + ASSERT(false, ()); + return make_pair(move(updateResult.first), true); + } + } + + pair<MwmSet::MwmLock, bool> result = Register(fileName); + if (result.second) + m_observers.ForEach(&Observer::OnMapRegistered, fileName); return result; } -bool Index::DeregisterMap(CountryFile const & countryFile) { return Deregister(countryFile); } +bool Index::DeleteMap(string const & fileName) +{ + { + lock_guard<mutex> lock(m_lock); + + if (!DeregisterImpl(fileName)) + return false; + + DeleteMapFiles(GetFullPath(fileName), true /* deleteReady */); + } + m_observers.ForEach(&Observer::OnMapDeleted, fileName); + return true; +} bool Index::AddObserver(Observer & observer) { return m_observers.Add(observer); } bool Index::RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); } -void Index::OnMwmDeregistered(LocalCountryFile const & localFile) +pair<MwmSet::MwmLock, Index::UpdateStatus> Index::UpdateMap(string const & fileName) +{ + pair<MwmSet::MwmLock, UpdateStatus> result; + result.second = UPDATE_STATUS_BAD_FILE; + + { + lock_guard<mutex> lock(m_lock); + + MwmId const id = GetMwmIdByFileNameImpl(fileName); + shared_ptr<MwmInfo> info = id.GetInfo(); + if (id.IsAlive() && info->m_lockCount > 0) + { + info->SetStatus(MwmInfo::STATUS_PENDING_UPDATE); + result.first = GetLock(id); + result.second = UPDATE_STATUS_UPDATE_DELAYED; + } + else + { + ReplaceFileWithReady(fileName); + pair<MwmSet::MwmLock, bool> registerResult = RegisterImpl(fileName); + if (registerResult.second) + { + result.first = move(registerResult.first); + result.second = UPDATE_STATUS_OK; + } + } + } + if (result.second != UPDATE_STATUS_BAD_FILE) + m_observers.ForEach(&Observer::OnMapUpdateIsReady, fileName); + if (result.second == UPDATE_STATUS_OK) + m_observers.ForEach(&Observer::OnMapUpdated, fileName); + return result; +} + +void Index::OnMwmDeleted(shared_ptr<MwmInfo> const & info) +{ + string const & fileName = info->m_fileName; + DeleteMapFiles(fileName, true /* deleteReady */); + m_observers.ForEach(&Observer::OnMapDeleted, fileName); +} + +void Index::OnMwmReadyForUpdate(shared_ptr<MwmInfo> const & info) { - m_observers.ForEach(&Observer::OnMapDeregistered, localFile); + ClearCache(MwmId(info)); + ReplaceFileWithReady(info->m_fileName); + info->SetStatus(MwmInfo::STATUS_UP_TO_DATE); + m_observers.ForEach(&Observer::OnMapUpdated, info->m_fileName); } ////////////////////////////////////////////////////////////////////////////////// @@ -88,11 +194,11 @@ Index::FeaturesLoaderGuard::FeaturesLoaderGuard(Index const & parent, MwmId id) { } -string Index::FeaturesLoaderGuard::GetCountryFileName() const +string Index::FeaturesLoaderGuard::GetFileName() const { if (!m_lock.IsLocked()) return string(); - return m_lock.GetValue<MwmValue>()->GetCountryFile().GetNameWithoutExt(); + return m_lock.GetValue<MwmValue>()->GetFileName(); } bool Index::FeaturesLoaderGuard::IsWorld() const diff --git a/indexer/index.hpp b/indexer/index.hpp index 20d9d75c5d..c058a0f11b 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -24,25 +24,33 @@ class MwmValue : public MwmSet::MwmValueBase public: FilesContainerR m_cont; IndexFactory m_factory; - platform::CountryFile const m_countryFile; - explicit MwmValue(platform::LocalCountryFile const & localFile); + explicit MwmValue(string const & name); inline feature::DataHeader const & GetHeader() const { return m_factory.GetHeader(); } inline version::MwmVersion const & GetMwmVersion() const { return m_factory.GetMwmVersion(); } - inline platform::CountryFile const & GetCountryFile() const { return m_countryFile; } + /// @return MWM file name without extension. + string GetFileName() const; }; class Index : public MwmSet { protected: // MwmSet overrides: - bool GetVersion(platform::LocalCountryFile const & localFile, MwmInfo & info) const override; - TMwmValueBasePtr CreateValue(platform::LocalCountryFile const & localFile) const override; - void OnMwmDeregistered(platform::LocalCountryFile const & localFile) override; + bool GetVersion(string const & name, MwmInfo & info) const override; + TMwmValueBasePtr CreateValue(string const & name) const override; + void OnMwmDeleted(shared_ptr<MwmInfo> const & info) override; + void OnMwmReadyForUpdate(shared_ptr<MwmInfo> const & info) override; public: + enum UpdateStatus + { + UPDATE_STATUS_OK, + UPDATE_STATUS_BAD_FILE, + UPDATE_STATUS_UPDATE_DELAYED + }; + /// An Observer interface to MwmSet. Note that these functions can /// be called from *ANY* thread because most signals are sent when /// some thread releases its MwmLock, so overrides must be as fast @@ -50,14 +58,19 @@ public: class Observer { public: - virtual ~Observer() = default; + virtual ~Observer() {} + + /// Called when a map is registered for a first time. + virtual void OnMapRegistered(string const & file) {} - /// Called when a map is registered for a first time and can be - /// used. - virtual void OnMapRegistered(platform::LocalCountryFile const & localFile) {} + /// Called when an update for a map is downloaded. + virtual void OnMapUpdateIsReady(string const & file) {} - /// Called when a map is deregistered and can not be used. - virtual void OnMapDeregistered(platform::LocalCountryFile const & localFile) {} + /// Called when an update for a map is applied. + virtual void OnMapUpdated(string const & file) {} + + /// Called when a map is deleted. + virtual void OnMapDeleted(string const & file) {} }; Index(); @@ -65,21 +78,37 @@ public: /// Registers a new map. /// - /// \return A pair of an MwmLock and a flag. There are three cases: - /// * the map is newer than the newest registered - returns - /// active lock and set flag - /// * the map is older than the newest registered - returns inactive lock and - /// unset flag. - /// * the version of the map equals to the version of the newest registered - - /// returns active lock and unset flag. - WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterMap(platform::LocalCountryFile const & localFile); - - /// Deregisters a map from internal records. + /// \return A pair of an MwmLock and a flag. MwmLock is locked iff the + /// map with fileName was created or already exists. Flag + /// is set when the map was registered for a first + /// time. Thus, there are three main cases: + /// + /// * the map already exists - returns active lock and unset flag + /// * the map was already registered - returns active lock and set flag + /// * the map can't be registered - returns inactive lock and unset flag + WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterMap(string const & fileName); + + /// Replaces a 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 * the map file have been updated - returns active lock and + /// UPDATE_STATUS_OK + /// * update is delayed because the map is busy - returns active lock and + /// UPDATE_STATUS_UPDATE_DELAYED + /// * the file isn't suitable for update - returns inactive lock and + /// UPDATE_STATUS_BAD_FILE + WARN_UNUSED_RESULT pair<MwmLock, UpdateStatus> UpdateMap(string const & fileName); + + /// Deletes a map both from the file system and internal tables, also, + /// deletes all files related to the map. If the map was successfully + /// deleted, notifies observers. /// - /// \param countryFile A countryFile denoting a map to be deregistered. - /// \return True if the map was successfully deregistered. If map is locked - /// now, returns false. - bool DeregisterMap(platform::CountryFile const & countryFile); + /// \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 the map was successfully deleted. + bool DeleteMap(string const & fileName); bool AddObserver(Observer & observer); @@ -251,7 +280,7 @@ public: FeaturesLoaderGuard(Index const & parent, MwmId id); inline MwmSet::MwmId GetId() const { return m_lock.GetId(); } - string GetCountryFileName() const; + string GetFileName() const; bool IsWorld() const; void GetFeature(uint32_t offset, FeatureType & ft); diff --git a/indexer/indexer_tests/index_builder_test.cpp b/indexer/indexer_tests/index_builder_test.cpp index 1ebfe9c068..7efeb27fb6 100644 --- a/indexer/indexer_tests/index_builder_test.cpp +++ b/indexer/indexer_tests/index_builder_test.cpp @@ -56,7 +56,7 @@ UNIT_TEST(BuildIndexTest) { // Check that index actually works. Index index; - UNUSED_VALUE(index.Register(platform::LocalCountryFile::MakeForTesting("build_index_test"))); + UNUSED_VALUE(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 1e24aa5365..dfea961b07 100644 --- a/indexer/indexer_tests/index_test.cpp +++ b/indexer/indexer_tests/index_test.cpp @@ -6,8 +6,6 @@ #include "coding/file_name_utils.hpp" #include "coding/internal/file_data.hpp" -#include "platform/country_file.hpp" -#include "platform/local_country_file.hpp" #include "platform/platform.hpp" #include "base/logging.hpp" @@ -18,11 +16,6 @@ #include "std/bind.hpp" #include "std/string.hpp" -using platform::CountryFile; -using platform::LocalCountryFile; - -namespace -{ void CheckedDeleteFile(string const & file) { if (Platform::IsFileExistsByFullPath(file)) @@ -32,60 +25,57 @@ void CheckedDeleteFile(string const & file) class Observer : public Index::Observer { public: - Observer() : m_numRegisteredCalls(0), m_numDeregisteredCalls(0) {} - - ~Observer() { CheckExpectations(); } - - void ExpectRegisteredMap(platform::LocalCountryFile const & localFile) + Observer(string const & file) + : m_file(file), + m_map_registered_calls(0), + m_map_update_is_ready_calls(0), + m_map_updated_calls(0), + m_map_deleted_calls(0) { - m_expectedRegisteredMaps.push_back(localFile); } - void ExpectDeregisteredMap(platform::LocalCountryFile const & localFile) + // Index::Observer overrides: + void OnMapRegistered(string const & file) override { - m_expectedDeregisteredMaps.push_back(localFile); + CHECK_EQUAL(m_file, file, ()); + ++m_map_registered_calls; } - void CheckExpectations() + void OnMapUpdateIsReady(string const & file) override { - CHECK_EQUAL(m_numRegisteredCalls, m_expectedRegisteredMaps.size(), ()); - CHECK_EQUAL(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(), ()); + CHECK_EQUAL(m_file, file, ()); + ++m_map_update_is_ready_calls; } - // Index::Observer overrides: - void OnMapRegistered(platform::LocalCountryFile const & localFile) override + void OnMapUpdated(string const & file) override { - CHECK_LESS(m_numRegisteredCalls, m_expectedRegisteredMaps.size(), - ("Unexpected OnMapRegistered() call (", m_numRegisteredCalls, "): ", localFile)); - CHECK_EQUAL(m_expectedRegisteredMaps[m_numRegisteredCalls], localFile, (m_numRegisteredCalls)); - ++m_numRegisteredCalls; + CHECK_EQUAL(m_file, file, ()); + ++m_map_updated_calls; } - void OnMapDeregistered(platform::LocalCountryFile const & localFile) override + void OnMapDeleted(string const & file) override { - CHECK_LESS(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(), - ("Unexpected OnMapDeregistered() call (", m_numDeregisteredCalls, "): ", localFile)); - CHECK_EQUAL(m_expectedDeregisteredMaps[m_numDeregisteredCalls], localFile, - (m_numDeregisteredCalls)); - ++m_numDeregisteredCalls; + CHECK_EQUAL(m_file, file, ()); + ++m_map_deleted_calls; } - inline size_t MapRegisteredCalls() const { return m_numRegisteredCalls; } - inline size_t MapDeregisteredCalls() const { return m_numDeregisteredCalls; } + int map_registered_calls() const { return m_map_registered_calls; } + int map_update_is_ready_calls() const { return m_map_update_is_ready_calls; } + int map_updated_calls() const { return m_map_updated_calls; } + int map_deleted_calls() const { return m_map_deleted_calls; } private: - size_t m_numRegisteredCalls; - size_t m_numDeregisteredCalls; - - vector<LocalCountryFile> m_expectedRegisteredMaps; - vector<LocalCountryFile> m_expectedDeregisteredMaps; + string const m_file; + int m_map_registered_calls; + int m_map_update_is_ready_calls; + int m_map_updated_calls; + int m_map_deleted_calls; }; -} // namespace UNIT_TEST(Index_Parse) { Index index; - UNUSED_VALUE(index.RegisterMap(platform::LocalCountryFile::MakeForTesting("minsk-pass"))); + UNUSED_VALUE(index.RegisterMap("minsk-pass" DATA_FILE_EXTENSION)); // Make sure that index is actually parsed. NoopFunctor fn; @@ -94,68 +84,65 @@ UNIT_TEST(Index_Parse) UNIT_TEST(Index_MwmStatusNotifications) { - Platform & platform = GetPlatform(); - string const mapsDir = GetPlatform().WritableDir(); - CountryFile const countryFile("minsk-pass"); + 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; - // These two classes point to the same file, but will be considered - // by Index as distinct files because versions are artificially set - // to different numbers. - LocalCountryFile const localFileV1(mapsDir, countryFile, 1 /* version */); - LocalCountryFile const localFileV2(mapsDir, countryFile, 2 /* version */); + TEST(my::CopyFileX(sourceMapPath, testMapPath), ()); + MY_SCOPE_GUARD(testMapGuard, bind(&CheckedDeleteFile, testMapPath)); Index index; - Observer observer; + Observer observer(testMapName); index.AddObserver(observer); - TEST_EQUAL(0, observer.MapRegisteredCalls(), ()); - - MwmSet::MwmId localFileV1Id; + TEST_EQUAL(0, observer.map_registered_calls(), ()); // Checks that observers are triggered after map registration. { - observer.ExpectRegisteredMap(localFileV1); - pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV1); + pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(testMapName); TEST(p.first.IsLocked(), ()); TEST(p.second, ()); - observer.CheckExpectations(); - localFileV1Id = p.first.GetId(); + TEST_EQUAL(1, observer.map_registered_calls(), ()); } - // Checks that map can't registered twice. + // Checks that map can't registered twice and observers aren't + // triggered. { - pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV1); + pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(testMapName); TEST(p.first.IsLocked(), ()); TEST(!p.second, ()); - observer.CheckExpectations(); - TEST_EQUAL(localFileV1Id, p.first.GetId(), ()); + TEST_EQUAL(1, observer.map_registered_calls(), ()); } - // Checks that observers are notified when map is updated. - MwmSet::MwmId localFileV2Id; + TEST(my::CopyFileX(testMapPath, testMapUpdatePath), ()); + MY_SCOPE_GUARD(testMapUpdateGuard, bind(&CheckedDeleteFile, testMapUpdatePath)); + + // Checks that observers are notified when map is deleted. { - observer.ExpectRegisteredMap(localFileV2); - observer.ExpectDeregisteredMap(localFileV1); - pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV2); + TEST_EQUAL(0, observer.map_update_is_ready_calls(), ()); + TEST_EQUAL(0, observer.map_updated_calls(), ()); + pair<MwmSet::MwmLock, Index::UpdateStatus> const p = index.UpdateMap(testMapName); TEST(p.first.IsLocked(), ()); - TEST(p.second, ()); - observer.CheckExpectations(); - localFileV2Id = p.first.GetId(); - TEST_NOT_EQUAL(localFileV1Id, localFileV2Id, ()); + TEST_EQUAL(Index::UPDATE_STATUS_OK, p.second, ()); + TEST_EQUAL(1, observer.map_update_is_ready_calls(), ()); + TEST_EQUAL(1, observer.map_updated_calls(), ()); } - // Tries to deregister a map in presence of an active lock. Map - // should be marked "to be removed" but can't be deregistered. After - // leaving the inner block the map should be deregistered. + // Tries to delete map in presence of active lock. Map should be + // marked "to be removed" but can't be deleted. { - MwmSet::MwmLock const lock = index.GetMwmLockByCountryFile(countryFile); + MwmSet::MwmLock const lock = index.GetMwmLockByFileName(testMapName); TEST(lock.IsLocked(), ()); - TEST(!index.DeregisterMap(countryFile), ()); - observer.CheckExpectations(); - - observer.ExpectDeregisteredMap(localFileV2); + TEST(!index.DeleteMap(testMapName), ()); + TEST_EQUAL(0, observer.map_deleted_calls(), ()); } - observer.CheckExpectations(); + + // Checks that observers are notified when all locks are destroyed. + 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 30b9adebe5..27fd148f6b 100644 --- a/indexer/indexer_tests/mwm_set_test.cpp +++ b/indexer/indexer_tests/mwm_set_test.cpp @@ -7,11 +7,6 @@ #include "std/initializer_list.hpp" #include "std/unordered_map.hpp" -using platform::CountryFile; -using platform::LocalCountryFile; - -using TMwmsInfo = unordered_map<string, shared_ptr<MwmInfo>>; - namespace { class MwmValue : public MwmSet::MwmValueBase @@ -21,17 +16,16 @@ class MwmValue : public MwmSet::MwmValueBase class TestMwmSet : public MwmSet { protected: - // MwmSet overrides: - bool GetVersion(LocalCountryFile const & localFile, MwmInfo & info) const override + bool GetVersion(string const & path, MwmInfo & info) const override { - int const n = localFile.GetCountryFile().GetNameWithoutExt()[0] - '0'; + int const n = path[0] - '0'; info.m_maxScale = n; info.m_limitRect = m2::RectD(0, 0, 1, 1); info.m_version.format = version::lastFormat; return true; } - TMwmValueBasePtr CreateValue(LocalCountryFile const &) const override + TMwmValueBasePtr CreateValue(string const &) const override { return TMwmValueBasePtr(new MwmValue()); } @@ -40,21 +34,23 @@ public: ~TestMwmSet() { Cleanup(); } }; -void GetMwmsInfo(MwmSet const & mwmSet, TMwmsInfo & mwmsInfo) +void GetMwmsInfo(MwmSet const & mwmSet, + unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> & mwmsInfo) { vector<shared_ptr<MwmInfo>> mwmsInfoList; mwmSet.GetMwmsInfo(mwmsInfoList); mwmsInfo.clear(); for (shared_ptr<MwmInfo> const & info : mwmsInfoList) - mwmsInfo[info->GetCountryName()] = info; + mwmsInfo[info->GetFileName()] = info; } -void TestFilesPresence(TMwmsInfo const & mwmsInfo, initializer_list<string> const & expectedNames) +void TestFilesPresence(unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> const & mwmsInfo, + initializer_list<MwmSet::TMwmFileName> const & expectedNames) { TEST_EQUAL(expectedNames.size(), mwmsInfo.size(), ()); - for (string const & countryFileName : expectedNames) - TEST_EQUAL(1, mwmsInfo.count(countryFileName), (countryFileName)); + for (MwmSet::TMwmFileName const & fileName : expectedNames) + TEST_EQUAL(1, mwmsInfo.count(fileName), (fileName)); } } // namespace @@ -62,12 +58,12 @@ void TestFilesPresence(TMwmsInfo const & mwmsInfo, initializer_list<string> cons UNIT_TEST(MwmSetSmokeTest) { TestMwmSet mwmSet; - TMwmsInfo mwmsInfo; + unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> mwmsInfo; - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("0"))); - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("1"))); - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("2"))); - mwmSet.Deregister(CountryFile("1")); + UNUSED_VALUE(mwmSet.Register("0")); + UNUSED_VALUE(mwmSet.Register("1")); + UNUSED_VALUE(mwmSet.Register("2")); + mwmSet.Deregister("1"); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2"}); @@ -76,13 +72,13 @@ UNIT_TEST(MwmSetSmokeTest) TEST_EQUAL(mwmsInfo["0"]->m_maxScale, 0, ()); TEST(mwmsInfo["2"]->IsUpToDate(), ()); { - MwmSet::MwmLock const lock0 = mwmSet.GetMwmLockByCountryFile(CountryFile("0")); - MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByCountryFile(CountryFile("1")); + MwmSet::MwmLock const lock0 = mwmSet.GetMwmLockByFileName("0"); + MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByFileName("1"); TEST(lock0.IsLocked(), ()); TEST(!lock1.IsLocked(), ()); } - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("3"))); + UNUSED_VALUE(mwmSet.Register("3")); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2", "3"}); @@ -95,10 +91,10 @@ UNIT_TEST(MwmSetSmokeTest) TEST_EQUAL(mwmsInfo["3"]->m_maxScale, 3, ()); { - MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByCountryFile(CountryFile("1")); + MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByFileName("1"); TEST(!lock1.IsLocked(), ()); - mwmSet.Deregister(CountryFile("3")); - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("4"))); + mwmSet.Deregister("3"); + UNUSED_VALUE(mwmSet.Register("4")); } GetMwmsInfo(mwmSet, mwmsInfo); @@ -111,7 +107,7 @@ UNIT_TEST(MwmSetSmokeTest) TEST(mwmsInfo["4"]->IsUpToDate(), ()); TEST_EQUAL(mwmsInfo["4"]->m_maxScale, 4, ()); - UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("5"))); + UNUSED_VALUE(mwmSet.Register("5")); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2", "4", "5"}); @@ -130,17 +126,17 @@ UNIT_TEST(MwmSetSmokeTest) UNIT_TEST(MwmSetIdTest) { TestMwmSet mwmSet; - TEST(mwmSet.Register(LocalCountryFile::MakeForTesting("3")).second, ()); + TEST(mwmSet.Register("3").second, ()); - MwmSet::MwmId const id0 = mwmSet.GetMwmLockByCountryFile(CountryFile("3")).GetId(); - MwmSet::MwmId const id1 = mwmSet.GetMwmLockByCountryFile(CountryFile("3")).GetId(); + MwmSet::MwmId const id0 = mwmSet.GetMwmLockByFileName("3").GetId(); + MwmSet::MwmId const id1 = mwmSet.GetMwmLockByFileName("3").GetId(); TEST(id0.IsAlive(), ()); TEST(id1.IsAlive(), ()); TEST_EQUAL(id0.GetInfo().get(), id1.GetInfo().get(), ()); - TEST_EQUAL(MwmInfo::STATUS_REGISTERED, id0.GetInfo()->GetStatus(), ()); + TEST_EQUAL(MwmInfo::STATUS_UP_TO_DATE, id0.GetInfo()->GetStatus(), ()); - TEST(mwmSet.Deregister(CountryFile("3")), ()); + TEST(mwmSet.Deregister("3"), ()); // Test that both id's are sour now. TEST(!id0.IsAlive(), ()); @@ -155,15 +151,14 @@ UNIT_TEST(MwmSetLockAndIdTest) MwmSet::MwmId id; { - pair<MwmSet::MwmLock, bool> const lockFlag = - mwmSet.Register(LocalCountryFile::MakeForTesting("4")); + pair<MwmSet::MwmLock, bool> const lockFlag = mwmSet.Register("4"); MwmSet::MwmLock const & lock = lockFlag.first; bool const success = lockFlag.second; TEST(lock.IsLocked(), ()); TEST(success, ("Can't register test mwm 4")); - TEST_EQUAL(MwmInfo::STATUS_REGISTERED, lock.GetInfo()->GetStatus(), ()); + TEST_EQUAL(MwmInfo::STATUS_UP_TO_DATE, lock.GetInfo()->GetStatus(), ()); - TEST(!mwmSet.Deregister(CountryFile("4")), ()); // It's not possible to remove mwm 4 right now. + TEST(!mwmSet.Deregister("4"), ()); // It's not possible to remove mwm 4 right now. TEST(lock.IsLocked(), ()); TEST_EQUAL(MwmInfo::STATUS_MARKED_TO_DEREGISTER, lock.GetInfo()->GetStatus(), ()); id = lock.GetId(); @@ -177,7 +172,7 @@ UNIT_TEST(MwmSetLockAndIdTest) // It is not possible to lock mwm 4 because it is already deleted, // and it is not possible to get to it's info from mwmSet. - MwmSet::MwmLock lock = mwmSet.GetMwmLockByCountryFile(CountryFile("4")); + MwmSet::MwmLock lock = mwmSet.GetMwmLockByFileName("4"); TEST(!lock.IsLocked(), ()); TEST(!lock.GetId().IsAlive(), ()); TEST(!lock.GetId().GetInfo().get(), ()); diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp index d6961ebdd0..c9005078ce 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -9,9 +9,6 @@ #include "std/algorithm.hpp" -using platform::CountryFile; -using platform::LocalCountryFile; - MwmInfo::MwmInfo() : m_minScale(0), m_maxScale(0), m_status(STATUS_DEREGISTERED), m_lockCount(0) {} MwmInfo::MwmTypeT MwmInfo::GetType() const @@ -77,64 +74,57 @@ MwmSet::~MwmSet() void MwmSet::Cleanup() { lock_guard<mutex> lock(m_lock); + ClearCacheImpl(m_cache.begin(), m_cache.end()); + +#ifdef DEBUG + for (auto const & p : m_info) + { + MwmInfo const & info = *p.second; + if (info.IsUpToDate()) + { + ASSERT_EQUAL(info.m_lockCount, 0, (info.m_fileName)); + ASSERT(!info.m_fileName.empty(), ()); + } + } +#endif } -MwmSet::MwmId MwmSet::GetMwmIdByCountryFileImpl(CountryFile const & countryFile) const +MwmSet::MwmId MwmSet::GetMwmIdByFileNameImpl(TMwmFileName const & name) const { - string const name = countryFile.GetNameWithoutExt(); ASSERT(!name.empty(), ()); auto const it = m_info.find(name); - if (it == m_info.cend() || it->second.empty()) + if (it == m_info.cend()) return MwmId(); - return MwmId(it->second.back()); + return MwmId(it->second); } -pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile) +pair<MwmSet::MwmLock, bool> MwmSet::Register(TMwmFileName const & fileName) { lock_guard<mutex> lock(m_lock); - CountryFile const & countryFile = localFile.GetCountryFile(); - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); + MwmId const id = GetMwmIdByFileNameImpl(fileName); if (!id.IsAlive()) - return RegisterImpl(localFile); - + return RegisterImpl(fileName); shared_ptr<MwmInfo> info = id.GetInfo(); - - // Deregister old mwm for the country. - if (info->GetVersion() < localFile.GetVersion()) - { - DeregisterImpl(id); - return RegisterImpl(localFile); - } - - string const name = countryFile.GetNameWithoutExt(); - // Update the status of the mwm with the same version. - if (info->GetVersion() == localFile.GetVersion()) - { - LOG(LWARNING, ("Trying to add already registered mwm:", name)); - info->SetStatus(MwmInfo::STATUS_REGISTERED); - return make_pair(GetLock(id), false); - } - - LOG(LWARNING, ("Trying to add too old (", localFile.GetVersion(), ") mwm (", name, - "), current version:", info->GetVersion())); - return make_pair(MwmLock(), false); + if (info->IsRegistered()) + LOG(LWARNING, ("Trying to add already registered mwm:", fileName)); + else + info->SetStatus(MwmInfo::STATUS_UP_TO_DATE); + return make_pair(GetLock(id), false); } -pair<MwmSet::MwmLock, bool> MwmSet::RegisterImpl(LocalCountryFile const & localFile) +pair<MwmSet::MwmLock, bool> MwmSet::RegisterImpl(TMwmFileName const & fileName) { shared_ptr<MwmInfo> info(new MwmInfo()); // This function can throw an exception for a bad mwm file. - if (!GetVersion(localFile, *info)) + if (!GetVersion(fileName, *info)) return make_pair(MwmLock(), false); - info->SetStatus(MwmInfo::STATUS_REGISTERED); - info->m_file = localFile; - string const name = localFile.GetCountryFile().GetNameWithoutExt(); + info->SetStatus(MwmInfo::STATUS_UP_TO_DATE); + info->m_fileName = fileName; + m_info[fileName] = info; - vector<shared_ptr<MwmInfo>> & infos = m_info[name]; - infos.push_back(info); return make_pair(GetLock(MwmId(info)), true); } @@ -143,28 +133,26 @@ bool MwmSet::DeregisterImpl(MwmId const & id) if (!id.IsAlive()) return false; shared_ptr<MwmInfo> const & info = id.GetInfo(); - if (info->m_lockCount == 0) { info->SetStatus(MwmInfo::STATUS_DEREGISTERED); - vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()]; - infos.erase(remove(infos.begin(), infos.end(), info), infos.end()); - OnMwmDeregistered(info->GetLocalFile()); + m_info.erase(info->m_fileName); + OnMwmDeleted(info); return true; } info->SetStatus(MwmInfo::STATUS_MARKED_TO_DEREGISTER); return false; } -bool MwmSet::Deregister(CountryFile const & countryFile) +bool MwmSet::Deregister(TMwmFileName const & fileName) { lock_guard<mutex> lock(m_lock); - return DeregisterImpl(countryFile); + return DeregisterImpl(fileName); } -bool MwmSet::DeregisterImpl(CountryFile const & countryFile) +bool MwmSet::DeregisterImpl(TMwmFileName const & fileName) { - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); + MwmId const id = GetMwmIdByFileNameImpl(fileName); if (!id.IsAlive()) return false; bool const deregistered = DeregisterImpl(id); @@ -176,24 +164,21 @@ void MwmSet::DeregisterAll() { lock_guard<mutex> lock(m_lock); - for (auto const & p : m_info) + for (auto it = m_info.begin(); it != m_info.end();) { - // Vector of shared pointers is copied here because an original - // vector will be modified by the body of the cycle. - vector<shared_ptr<MwmInfo>> infos = p.second; - for (shared_ptr<MwmInfo> info : infos) - DeregisterImpl(MwmId(info)); + auto cur = it++; + DeregisterImpl(MwmId(cur->second)); } // Do not call ClearCache - it's under mutex lock. ClearCacheImpl(m_cache.begin(), m_cache.end()); } -bool MwmSet::IsLoaded(CountryFile const & countryFile) const +bool MwmSet::IsLoaded(TMwmFileName const & fileName) const { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); + MwmId const id = GetMwmIdByFileNameImpl(fileName + DATA_FILE_EXTENSION); return id.IsAlive() && id.GetInfo()->IsRegistered(); } @@ -203,10 +188,7 @@ void MwmSet::GetMwmsInfo(vector<shared_ptr<MwmInfo>> & info) const info.clear(); info.reserve(m_info.size()); for (auto const & p : m_info) - { - if (!p.second.empty()) - info.push_back(p.second.back()); - } + info.push_back(p.second); } MwmSet::TMwmValueBasePtr MwmSet::LockValue(MwmId const & id) @@ -234,7 +216,7 @@ MwmSet::TMwmValueBasePtr MwmSet::LockValueImpl(MwmId const & id) return result; } } - return CreateValue(info->GetLocalFile()); + return CreateValue(info->m_fileName); } void MwmSet::UnlockValue(MwmId const & id, TMwmValueBasePtr p) @@ -251,10 +233,22 @@ void MwmSet::UnlockValueImpl(MwmId const & id, TMwmValueBasePtr p) return; shared_ptr<MwmInfo> const & info = id.GetInfo(); - ASSERT_GREATER(info->m_lockCount, 0, ()); + CHECK_GREATER(info->m_lockCount, 0, ()); --info->m_lockCount; - if (info->m_lockCount == 0 && info->GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER) - VERIFY(DeregisterImpl(id), ()); + if (info->m_lockCount == 0) + { + switch (info->GetStatus()) + { + case MwmInfo::STATUS_MARKED_TO_DEREGISTER: + CHECK(DeregisterImpl(id), ()); + break; + case MwmInfo::STATUS_PENDING_UPDATE: + OnMwmReadyForUpdate(info); + break; + default: + break; + } + } if (info->IsUpToDate()) { @@ -273,20 +267,20 @@ void MwmSet::ClearCache() ClearCacheImpl(m_cache.begin(), m_cache.end()); } -MwmSet::MwmId MwmSet::GetMwmIdByCountryFile(CountryFile const & countryFile) const +MwmSet::MwmId MwmSet::GetMwmIdByFileName(TMwmFileName const & fileName) const { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); - ASSERT(id.IsAlive(), ("Can't get an mwm's (", countryFile.GetNameWithoutExt(), ") identifier.")); + MwmId const id = GetMwmIdByFileNameImpl(fileName); + ASSERT(id.IsAlive(), ("Can't get an mwm's (", fileName, ") identifier.")); return id; } -MwmSet::MwmLock MwmSet::GetMwmLockByCountryFile(CountryFile const & countryFile) +MwmSet::MwmLock MwmSet::GetMwmLockByFileName(TMwmFileName const & fileName) { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); + MwmId const id = GetMwmIdByFileNameImpl(fileName); TMwmValueBasePtr value(nullptr); if (id.IsAlive()) value = LockValueImpl(id); diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp index dfc225f4c4..25682c16b0 100644 --- a/indexer/mwm_set.hpp +++ b/indexer/mwm_set.hpp @@ -2,9 +2,6 @@ #include "indexer/mwm_version.hpp" -#include "platform/country_file.hpp" -#include "platform/local_country_file.hpp" - #include "geometry/rect2d.hpp" #include "base/macros.hpp" @@ -34,9 +31,10 @@ public: enum Status { - STATUS_REGISTERED, ///< 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_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 }; MwmInfo(); @@ -48,32 +46,31 @@ public: inline Status GetStatus() const { return m_status; } - inline bool IsUpToDate() const { return IsRegistered(); } - inline bool IsRegistered() const { - return m_status == STATUS_REGISTERED; + return m_status == STATUS_UP_TO_DATE || m_status == STATUS_PENDING_UPDATE; } - inline platform::LocalCountryFile const & GetLocalFile() const { return m_file; } - - inline string GetCountryName() const { return m_file.GetCountryFile().GetNameWithoutExt(); } + inline bool IsUpToDate() const { return m_status == STATUS_UP_TO_DATE; } - inline int64_t GetVersion() const { return m_file.GetVersion(); } + inline string const & GetFileName() const { return m_fileName; } MwmTypeT GetType() const; private: inline void SetStatus(Status status) { m_status = status; } - platform::LocalCountryFile m_file; ///< Path to the mwm file. - Status m_status; ///< Current country status. - uint8_t m_lockCount; ///< Number of locks. + string m_fileName; ///< Path to the mwm file. + Status m_status; ///< Current country status. + uint8_t m_lockCount; ///< Number of locks. }; class MwmSet { public: + using TMwmFileName = string; + using TMwmInfoTable = map<TMwmFileName, shared_ptr<MwmInfo>>; + struct MwmId { public: @@ -97,9 +94,9 @@ public: friend ostream & operator<<(ostream & os, MwmId const & id) { if (id.m_info.get()) - os << "MwmId [" << id.m_info->GetCountryName() << "]"; + os << "MwmId[" << id.m_info->GetFileName() << "]"; else - os << "MwmId [invalid]"; + os << "MwmId[invalid]"; return os; } @@ -155,43 +152,41 @@ public: /// Registers a new map. /// - /// \return A pair of an MwmLock and a flag. There are three cases: - /// * the map is newer than the newest registered - returns - /// active lock and set flag. - /// * the map is older than the newest registered - returns inactive lock and - /// unset flag. - /// * the version of the map equals to the version of the newest registered - - /// returns active lock and unset flag. + /// \return A pair of an MwmLock and a flag. MwmLock is locked iff the + /// map with fileName was created or already exists. Flag + /// is set when the map was registered for a first + /// time. Thus, there are three main cases: /// - /// *NOTE* When a new version for the same country is registered, - /// all previous versions will be automatically deregistered. + /// * the map already exists - returns active lock and unset flag + /// * the map was already registered - returns active lock and set flag + /// * the map can't be registered - returns inactive lock and unset flag + //@{ protected: - WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterImpl(platform::LocalCountryFile const & localFile); + WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterImpl(TMwmFileName const & fileName); public: - WARN_UNUSED_RESULT pair<MwmLock, bool> Register(platform::LocalCountryFile const & localFile); + WARN_UNUSED_RESULT pair<MwmLock, bool> Register(TMwmFileName const & fileName); //@} /// @name Remove mwm. //@{ protected: - /// Deregisters a map from internal records. + /// Deregisters a map from the set when it's possible. Note that an + /// underlying file is not deleted. /// - /// \param countryFile A countryFile denoting a map to be deregistered. - /// \return True if the map was successfully deregistered. If map is locked - /// now, returns false. + /// @return true when the map was deregistered. //@{ bool DeregisterImpl(MwmId const & id); - bool DeregisterImpl(platform::CountryFile const & countryFile); + bool DeregisterImpl(TMwmFileName const & ofileName); //@} public: - bool Deregister(platform::CountryFile const & countryFile); + bool Deregister(TMwmFileName const & fileName); void DeregisterAll(); //@} - /// Returns true when country is registered and can be used. - bool IsLoaded(platform::CountryFile const & countryFile) const; + /// @param[in] file File name without extension. + bool IsLoaded(TMwmFileName const & fileName) const; /// Get ids of all mwms. Some of them may be with not active status. /// In that case, LockValue returns NULL. @@ -199,14 +194,14 @@ public: void ClearCache(); - MwmId GetMwmIdByCountryFile(platform::CountryFile const & countryFile) const; + MwmId GetMwmIdByFileName(TMwmFileName const & fileName) const; - MwmLock GetMwmLockByCountryFile(platform::CountryFile const & countryFile); + MwmLock GetMwmLockByFileName(TMwmFileName const & fileName); protected: /// @return True when file format version was successfully read to MwmInfo. - virtual bool GetVersion(platform::LocalCountryFile const & localFile, MwmInfo & info) const = 0; - virtual TMwmValueBasePtr CreateValue(platform::LocalCountryFile const & localFile) const = 0; + virtual bool GetVersion(TMwmFileName const & fileName, MwmInfo & info) const = 0; + virtual TMwmValueBasePtr CreateValue(string const & name) const = 0; void Cleanup(); @@ -231,7 +226,7 @@ protected: /// Find mwm with a given name. /// @precondition This function is always called under mutex m_lock. - MwmId GetMwmIdByCountryFileImpl(platform::CountryFile const & countryFile) const; + MwmId GetMwmIdByFileNameImpl(TMwmFileName const & fileName) const; /// @precondition This function is always called under mutex m_lock. WARN_UNUSED_RESULT inline MwmLock GetLock(MwmId const & id) @@ -241,9 +236,12 @@ protected: // This method is called under m_lock when mwm is removed from a // registry. - virtual void OnMwmDeregistered(platform::LocalCountryFile const & localFile) {} + virtual void OnMwmDeleted(shared_ptr<MwmInfo> const & info) {} + + // This method is called under m_lock when mwm is ready for update. + virtual void OnMwmReadyForUpdate(shared_ptr<MwmInfo> const & info) {} - map<string, vector<shared_ptr<MwmInfo>>> m_info; + TMwmInfoTable m_info; mutable mutex m_lock; }; |