diff options
author | Yuri Gorshenin <y@maps.me> | 2015-07-01 16:55:50 +0300 |
---|---|---|
committer | Alex Zolotarev <alex@maps.me> | 2015-09-23 02:53:33 +0300 |
commit | 54e5990c7e4c65b0749c21ce4fe3866909a29668 (patch) | |
tree | ec9f80550c7402b7d5f577773353c0d7eb6fb0a7 /indexer | |
parent | 025f73ad85d489c4ea143b9d95dde4d308daf89b (diff) |
Revert "Merge pull request #1068 from maps/revert-929-abstract-country-file"
This reverts commit 66aac38c3004d261ee06a16f7e9db372f67614df, reversing
changes made to ad8708944070f6b32a17fbb944d3c493b3fb2a24.
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, 271 insertions, 380 deletions
diff --git a/indexer/index.cpp b/indexer/index.cpp index e27814dc74..12d0a02fe7 100644 --- a/indexer/index.cpp +++ b/indexer/index.cpp @@ -6,32 +6,27 @@ #include "coding/file_name_utils.hpp" #include "coding/internal/file_data.hpp" +using platform::CountryFile; +using platform::LocalCountryFile; ////////////////////////////////////////////////////////////////////////////////// // MwmValue implementation ////////////////////////////////////////////////////////////////////////////////// -MwmValue::MwmValue(string const & name) - : m_cont(GetPlatform().GetReader(name)) +MwmValue::MwmValue(LocalCountryFile const & localFile) + : m_cont(GetPlatform().GetCountryReader(localFile, TMapOptions::EMap)), + m_countryFile(localFile.GetCountryFile()) { 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(string const & name, MwmInfo & info) const +bool Index::GetVersion(LocalCountryFile const & localFile, MwmInfo & info) const { - MwmValue value(name); + MwmValue value(localFile); feature::DataHeader const & h = value.GetHeader(); if (!h.IsMWMSuitable()) @@ -47,9 +42,9 @@ bool Index::GetVersion(string const & name, MwmInfo & info) const return true; } -MwmSet::TMwmValueBasePtr Index::CreateValue(string const & name) const +MwmSet::TMwmValueBasePtr Index::CreateValue(LocalCountryFile const & localFile) const { - TMwmValueBasePtr p(new MwmValue(name)); + TMwmValueBasePtr p(new MwmValue(localFile)); ASSERT(static_cast<MwmValue &>(*p.get()).GetHeader().IsMWMSuitable(), ()); return p; } @@ -63,124 +58,23 @@ Index::~Index() Cleanup(); } -namespace +pair<MwmSet::MwmLock, bool> Index::RegisterMap(LocalCountryFile const & 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); + pair<MwmSet::MwmLock, bool> result = Register(localFile); + if (result.first.IsLocked() && result.second) + m_observers.ForEach(&Observer::OnMapRegistered, localFile); return result; } -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::DeregisterMap(CountryFile const & countryFile) { return Deregister(countryFile); } bool Index::AddObserver(Observer & observer) { return m_observers.Add(observer); } bool Index::RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); } -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) +void Index::OnMwmDeregistered(LocalCountryFile const & localFile) { - ClearCache(MwmId(info)); - ReplaceFileWithReady(info->m_fileName); - info->SetStatus(MwmInfo::STATUS_UP_TO_DATE); - m_observers.ForEach(&Observer::OnMapUpdated, info->m_fileName); + m_observers.ForEach(&Observer::OnMapDeregistered, localFile); } ////////////////////////////////////////////////////////////////////////////////// @@ -194,11 +88,11 @@ Index::FeaturesLoaderGuard::FeaturesLoaderGuard(Index const & parent, MwmId id) { } -string Index::FeaturesLoaderGuard::GetFileName() const +string Index::FeaturesLoaderGuard::GetCountryFileName() const { if (!m_lock.IsLocked()) return string(); - return m_lock.GetValue<MwmValue>()->GetFileName(); + return m_lock.GetValue<MwmValue>()->GetCountryFile().GetNameWithoutExt(); } bool Index::FeaturesLoaderGuard::IsWorld() const diff --git a/indexer/index.hpp b/indexer/index.hpp index c058a0f11b..20d9d75c5d 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -24,33 +24,25 @@ class MwmValue : public MwmSet::MwmValueBase public: FilesContainerR m_cont; IndexFactory m_factory; + platform::CountryFile const m_countryFile; - explicit MwmValue(string const & name); + explicit MwmValue(platform::LocalCountryFile const & localFile); inline feature::DataHeader const & GetHeader() const { return m_factory.GetHeader(); } inline version::MwmVersion const & GetMwmVersion() const { return m_factory.GetMwmVersion(); } - /// @return MWM file name without extension. - string GetFileName() const; + inline platform::CountryFile const & GetCountryFile() const { return m_countryFile; } }; class Index : public MwmSet { protected: // MwmSet overrides: - 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; + 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; 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 @@ -58,19 +50,14 @@ public: class Observer { public: - virtual ~Observer() {} - - /// Called when a map is registered for a first time. - virtual void OnMapRegistered(string const & file) {} + virtual ~Observer() = default; - /// Called when an update for a map is downloaded. - virtual void OnMapUpdateIsReady(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 applied. - virtual void OnMapUpdated(string const & file) {} - - /// Called when a map is deleted. - virtual void OnMapDeleted(string const & file) {} + /// Called when a map is deregistered and can not be used. + virtual void OnMapDeregistered(platform::LocalCountryFile const & localFile) {} }; Index(); @@ -78,37 +65,21 @@ public: /// Registers a new map. /// - /// \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. + /// \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. /// - /// \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); + /// \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); bool AddObserver(Observer & observer); @@ -280,7 +251,7 @@ public: FeaturesLoaderGuard(Index const & parent, MwmId id); inline MwmSet::MwmId GetId() const { return m_lock.GetId(); } - string GetFileName() const; + string GetCountryFileName() 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 7efeb27fb6..1ebfe9c068 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(fileName)); + UNUSED_VALUE(index.Register(platform::LocalCountryFile::MakeForTesting("build_index_test"))); // 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 dfea961b07..1e24aa5365 100644 --- a/indexer/indexer_tests/index_test.cpp +++ b/indexer/indexer_tests/index_test.cpp @@ -6,6 +6,8 @@ #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" @@ -16,6 +18,11 @@ #include "std/bind.hpp" #include "std/string.hpp" +using platform::CountryFile; +using platform::LocalCountryFile; + +namespace +{ void CheckedDeleteFile(string const & file) { if (Platform::IsFileExistsByFullPath(file)) @@ -25,57 +32,60 @@ void CheckedDeleteFile(string const & file) class Observer : public Index::Observer { public: - 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) + Observer() : m_numRegisteredCalls(0), m_numDeregisteredCalls(0) {} + + ~Observer() { CheckExpectations(); } + + void ExpectRegisteredMap(platform::LocalCountryFile const & localFile) { + m_expectedRegisteredMaps.push_back(localFile); } - // Index::Observer overrides: - void OnMapRegistered(string const & file) override + void ExpectDeregisteredMap(platform::LocalCountryFile const & localFile) { - CHECK_EQUAL(m_file, file, ()); - ++m_map_registered_calls; + m_expectedDeregisteredMaps.push_back(localFile); } - void OnMapUpdateIsReady(string const & file) override + void CheckExpectations() { - CHECK_EQUAL(m_file, file, ()); - ++m_map_update_is_ready_calls; + CHECK_EQUAL(m_numRegisteredCalls, m_expectedRegisteredMaps.size(), ()); + CHECK_EQUAL(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(), ()); } - void OnMapUpdated(string const & file) override + // Index::Observer overrides: + void OnMapRegistered(platform::LocalCountryFile const & localFile) override { - CHECK_EQUAL(m_file, file, ()); - ++m_map_updated_calls; + CHECK_LESS(m_numRegisteredCalls, m_expectedRegisteredMaps.size(), + ("Unexpected OnMapRegistered() call (", m_numRegisteredCalls, "): ", localFile)); + CHECK_EQUAL(m_expectedRegisteredMaps[m_numRegisteredCalls], localFile, (m_numRegisteredCalls)); + ++m_numRegisteredCalls; } - void OnMapDeleted(string const & file) override + void OnMapDeregistered(platform::LocalCountryFile const & localFile) override { - CHECK_EQUAL(m_file, file, ()); - ++m_map_deleted_calls; + CHECK_LESS(m_numDeregisteredCalls, m_expectedDeregisteredMaps.size(), + ("Unexpected OnMapDeregistered() call (", m_numDeregisteredCalls, "): ", localFile)); + CHECK_EQUAL(m_expectedDeregisteredMaps[m_numDeregisteredCalls], localFile, + (m_numDeregisteredCalls)); + ++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; } + inline size_t MapRegisteredCalls() const { return m_numRegisteredCalls; } + inline size_t MapDeregisteredCalls() const { return m_numDeregisteredCalls; } private: - 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; + size_t m_numRegisteredCalls; + size_t m_numDeregisteredCalls; + + vector<LocalCountryFile> m_expectedRegisteredMaps; + vector<LocalCountryFile> m_expectedDeregisteredMaps; }; +} // namespace UNIT_TEST(Index_Parse) { Index index; - UNUSED_VALUE(index.RegisterMap("minsk-pass" DATA_FILE_EXTENSION)); + UNUSED_VALUE(index.RegisterMap(platform::LocalCountryFile::MakeForTesting("minsk-pass"))); // Make sure that index is actually parsed. NoopFunctor fn; @@ -84,65 +94,68 @@ UNIT_TEST(Index_Parse) 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; + Platform & platform = GetPlatform(); + string const mapsDir = GetPlatform().WritableDir(); + CountryFile const countryFile("minsk-pass"); - TEST(my::CopyFileX(sourceMapPath, testMapPath), ()); - MY_SCOPE_GUARD(testMapGuard, bind(&CheckedDeleteFile, testMapPath)); + // 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 */); Index index; - Observer observer(testMapName); + Observer observer; index.AddObserver(observer); - TEST_EQUAL(0, observer.map_registered_calls(), ()); + TEST_EQUAL(0, observer.MapRegisteredCalls(), ()); + + MwmSet::MwmId localFileV1Id; // Checks that observers are triggered after map registration. { - pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(testMapName); + observer.ExpectRegisteredMap(localFileV1); + pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV1); TEST(p.first.IsLocked(), ()); TEST(p.second, ()); - TEST_EQUAL(1, observer.map_registered_calls(), ()); + observer.CheckExpectations(); + localFileV1Id = p.first.GetId(); } - // Checks that map can't registered twice and observers aren't - // triggered. + // Checks that map can't registered twice. { - pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(testMapName); + pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV1); TEST(p.first.IsLocked(), ()); TEST(!p.second, ()); - TEST_EQUAL(1, observer.map_registered_calls(), ()); + observer.CheckExpectations(); + TEST_EQUAL(localFileV1Id, p.first.GetId(), ()); } - TEST(my::CopyFileX(testMapPath, testMapUpdatePath), ()); - MY_SCOPE_GUARD(testMapUpdateGuard, bind(&CheckedDeleteFile, testMapUpdatePath)); - - // Checks that observers are notified when map is deleted. + // Checks that observers are notified when map is updated. + MwmSet::MwmId localFileV2Id; { - 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); + observer.ExpectRegisteredMap(localFileV2); + observer.ExpectDeregisteredMap(localFileV1); + pair<MwmSet::MwmLock, bool> const p = index.RegisterMap(localFileV2); TEST(p.first.IsLocked(), ()); - TEST_EQUAL(Index::UPDATE_STATUS_OK, p.second, ()); - TEST_EQUAL(1, observer.map_update_is_ready_calls(), ()); - TEST_EQUAL(1, observer.map_updated_calls(), ()); + TEST(p.second, ()); + observer.CheckExpectations(); + localFileV2Id = p.first.GetId(); + TEST_NOT_EQUAL(localFileV1Id, localFileV2Id, ()); } - // Tries to delete map in presence of active lock. Map should be - // marked "to be removed" but can't be deleted. + // 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. { - MwmSet::MwmLock const lock = index.GetMwmLockByFileName(testMapName); + MwmSet::MwmLock const lock = index.GetMwmLockByCountryFile(countryFile); TEST(lock.IsLocked(), ()); - TEST(!index.DeleteMap(testMapName), ()); - TEST_EQUAL(0, observer.map_deleted_calls(), ()); - } - - // Checks that observers are notified when all locks are destroyed. - TEST_EQUAL(1, observer.map_deleted_calls(), ()); + TEST(!index.DeregisterMap(countryFile), ()); + observer.CheckExpectations(); + observer.ExpectDeregisteredMap(localFileV2); + } + observer.CheckExpectations(); index.RemoveObserver(observer); } diff --git a/indexer/indexer_tests/mwm_set_test.cpp b/indexer/indexer_tests/mwm_set_test.cpp index 27fd148f6b..30b9adebe5 100644 --- a/indexer/indexer_tests/mwm_set_test.cpp +++ b/indexer/indexer_tests/mwm_set_test.cpp @@ -7,6 +7,11 @@ #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 @@ -16,16 +21,17 @@ class MwmValue : public MwmSet::MwmValueBase class TestMwmSet : public MwmSet { protected: - bool GetVersion(string const & path, MwmInfo & info) const override + // MwmSet overrides: + bool GetVersion(LocalCountryFile const & localFile, MwmInfo & info) const override { - int const n = path[0] - '0'; + int const n = localFile.GetCountryFile().GetNameWithoutExt()[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(string const &) const override + TMwmValueBasePtr CreateValue(LocalCountryFile const &) const override { return TMwmValueBasePtr(new MwmValue()); } @@ -34,23 +40,21 @@ public: ~TestMwmSet() { Cleanup(); } }; -void GetMwmsInfo(MwmSet const & mwmSet, - unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> & mwmsInfo) +void GetMwmsInfo(MwmSet const & mwmSet, TMwmsInfo & mwmsInfo) { vector<shared_ptr<MwmInfo>> mwmsInfoList; mwmSet.GetMwmsInfo(mwmsInfoList); mwmsInfo.clear(); for (shared_ptr<MwmInfo> const & info : mwmsInfoList) - mwmsInfo[info->GetFileName()] = info; + mwmsInfo[info->GetCountryName()] = info; } -void TestFilesPresence(unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> const & mwmsInfo, - initializer_list<MwmSet::TMwmFileName> const & expectedNames) +void TestFilesPresence(TMwmsInfo const & mwmsInfo, initializer_list<string> const & expectedNames) { TEST_EQUAL(expectedNames.size(), mwmsInfo.size(), ()); - for (MwmSet::TMwmFileName const & fileName : expectedNames) - TEST_EQUAL(1, mwmsInfo.count(fileName), (fileName)); + for (string const & countryFileName : expectedNames) + TEST_EQUAL(1, mwmsInfo.count(countryFileName), (countryFileName)); } } // namespace @@ -58,12 +62,12 @@ void TestFilesPresence(unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> UNIT_TEST(MwmSetSmokeTest) { TestMwmSet mwmSet; - unordered_map<MwmSet::TMwmFileName, shared_ptr<MwmInfo>> mwmsInfo; + TMwmsInfo mwmsInfo; - UNUSED_VALUE(mwmSet.Register("0")); - UNUSED_VALUE(mwmSet.Register("1")); - UNUSED_VALUE(mwmSet.Register("2")); - mwmSet.Deregister("1"); + 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")); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2"}); @@ -72,13 +76,13 @@ UNIT_TEST(MwmSetSmokeTest) TEST_EQUAL(mwmsInfo["0"]->m_maxScale, 0, ()); TEST(mwmsInfo["2"]->IsUpToDate(), ()); { - MwmSet::MwmLock const lock0 = mwmSet.GetMwmLockByFileName("0"); - MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByFileName("1"); + MwmSet::MwmLock const lock0 = mwmSet.GetMwmLockByCountryFile(CountryFile("0")); + MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByCountryFile(CountryFile("1")); TEST(lock0.IsLocked(), ()); TEST(!lock1.IsLocked(), ()); } - UNUSED_VALUE(mwmSet.Register("3")); + UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("3"))); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2", "3"}); @@ -91,10 +95,10 @@ UNIT_TEST(MwmSetSmokeTest) TEST_EQUAL(mwmsInfo["3"]->m_maxScale, 3, ()); { - MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByFileName("1"); + MwmSet::MwmLock const lock1 = mwmSet.GetMwmLockByCountryFile(CountryFile("1")); TEST(!lock1.IsLocked(), ()); - mwmSet.Deregister("3"); - UNUSED_VALUE(mwmSet.Register("4")); + mwmSet.Deregister(CountryFile("3")); + UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("4"))); } GetMwmsInfo(mwmSet, mwmsInfo); @@ -107,7 +111,7 @@ UNIT_TEST(MwmSetSmokeTest) TEST(mwmsInfo["4"]->IsUpToDate(), ()); TEST_EQUAL(mwmsInfo["4"]->m_maxScale, 4, ()); - UNUSED_VALUE(mwmSet.Register("5")); + UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("5"))); GetMwmsInfo(mwmSet, mwmsInfo); TestFilesPresence(mwmsInfo, {"0", "2", "4", "5"}); @@ -126,17 +130,17 @@ UNIT_TEST(MwmSetSmokeTest) UNIT_TEST(MwmSetIdTest) { TestMwmSet mwmSet; - TEST(mwmSet.Register("3").second, ()); + TEST(mwmSet.Register(LocalCountryFile::MakeForTesting("3")).second, ()); - MwmSet::MwmId const id0 = mwmSet.GetMwmLockByFileName("3").GetId(); - MwmSet::MwmId const id1 = mwmSet.GetMwmLockByFileName("3").GetId(); + MwmSet::MwmId const id0 = mwmSet.GetMwmLockByCountryFile(CountryFile("3")).GetId(); + MwmSet::MwmId const id1 = mwmSet.GetMwmLockByCountryFile(CountryFile("3")).GetId(); TEST(id0.IsAlive(), ()); TEST(id1.IsAlive(), ()); TEST_EQUAL(id0.GetInfo().get(), id1.GetInfo().get(), ()); - TEST_EQUAL(MwmInfo::STATUS_UP_TO_DATE, id0.GetInfo()->GetStatus(), ()); + TEST_EQUAL(MwmInfo::STATUS_REGISTERED, id0.GetInfo()->GetStatus(), ()); - TEST(mwmSet.Deregister("3"), ()); + TEST(mwmSet.Deregister(CountryFile("3")), ()); // Test that both id's are sour now. TEST(!id0.IsAlive(), ()); @@ -151,14 +155,15 @@ UNIT_TEST(MwmSetLockAndIdTest) MwmSet::MwmId id; { - pair<MwmSet::MwmLock, bool> const lockFlag = mwmSet.Register("4"); + pair<MwmSet::MwmLock, bool> const lockFlag = + mwmSet.Register(LocalCountryFile::MakeForTesting("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_UP_TO_DATE, lock.GetInfo()->GetStatus(), ()); + TEST_EQUAL(MwmInfo::STATUS_REGISTERED, lock.GetInfo()->GetStatus(), ()); - TEST(!mwmSet.Deregister("4"), ()); // It's not possible to remove mwm 4 right now. + TEST(!mwmSet.Deregister(CountryFile("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(); @@ -172,7 +177,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.GetMwmLockByFileName("4"); + MwmSet::MwmLock lock = mwmSet.GetMwmLockByCountryFile(CountryFile("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 c9005078ce..d6961ebdd0 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -9,6 +9,9 @@ #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 @@ -74,57 +77,64 @@ 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::GetMwmIdByFileNameImpl(TMwmFileName const & name) const +MwmSet::MwmId MwmSet::GetMwmIdByCountryFileImpl(CountryFile const & countryFile) const { + string const name = countryFile.GetNameWithoutExt(); ASSERT(!name.empty(), ()); auto const it = m_info.find(name); - if (it == m_info.cend()) + if (it == m_info.cend() || it->second.empty()) return MwmId(); - return MwmId(it->second); + return MwmId(it->second.back()); } -pair<MwmSet::MwmLock, bool> MwmSet::Register(TMwmFileName const & fileName) +pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile) { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByFileNameImpl(fileName); + CountryFile const & countryFile = localFile.GetCountryFile(); + MwmId const id = GetMwmIdByCountryFileImpl(countryFile); if (!id.IsAlive()) - return RegisterImpl(fileName); + return RegisterImpl(localFile); + shared_ptr<MwmInfo> info = id.GetInfo(); - 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); + + // 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); } -pair<MwmSet::MwmLock, bool> MwmSet::RegisterImpl(TMwmFileName const & fileName) +pair<MwmSet::MwmLock, bool> MwmSet::RegisterImpl(LocalCountryFile const & localFile) { shared_ptr<MwmInfo> info(new MwmInfo()); // This function can throw an exception for a bad mwm file. - if (!GetVersion(fileName, *info)) + if (!GetVersion(localFile, *info)) return make_pair(MwmLock(), false); - info->SetStatus(MwmInfo::STATUS_UP_TO_DATE); - info->m_fileName = fileName; - m_info[fileName] = info; + info->SetStatus(MwmInfo::STATUS_REGISTERED); + info->m_file = localFile; + string const name = localFile.GetCountryFile().GetNameWithoutExt(); + vector<shared_ptr<MwmInfo>> & infos = m_info[name]; + infos.push_back(info); return make_pair(GetLock(MwmId(info)), true); } @@ -133,26 +143,28 @@ 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); - m_info.erase(info->m_fileName); - OnMwmDeleted(info); + vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()]; + infos.erase(remove(infos.begin(), infos.end(), info), infos.end()); + OnMwmDeregistered(info->GetLocalFile()); return true; } info->SetStatus(MwmInfo::STATUS_MARKED_TO_DEREGISTER); return false; } -bool MwmSet::Deregister(TMwmFileName const & fileName) +bool MwmSet::Deregister(CountryFile const & countryFile) { lock_guard<mutex> lock(m_lock); - return DeregisterImpl(fileName); + return DeregisterImpl(countryFile); } -bool MwmSet::DeregisterImpl(TMwmFileName const & fileName) +bool MwmSet::DeregisterImpl(CountryFile const & countryFile) { - MwmId const id = GetMwmIdByFileNameImpl(fileName); + MwmId const id = GetMwmIdByCountryFileImpl(countryFile); if (!id.IsAlive()) return false; bool const deregistered = DeregisterImpl(id); @@ -164,21 +176,24 @@ void MwmSet::DeregisterAll() { lock_guard<mutex> lock(m_lock); - for (auto it = m_info.begin(); it != m_info.end();) + for (auto const & p : m_info) { - auto cur = it++; - DeregisterImpl(MwmId(cur->second)); + // 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)); } // Do not call ClearCache - it's under mutex lock. ClearCacheImpl(m_cache.begin(), m_cache.end()); } -bool MwmSet::IsLoaded(TMwmFileName const & fileName) const +bool MwmSet::IsLoaded(CountryFile const & countryFile) const { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByFileNameImpl(fileName + DATA_FILE_EXTENSION); + MwmId const id = GetMwmIdByCountryFileImpl(countryFile); return id.IsAlive() && id.GetInfo()->IsRegistered(); } @@ -188,7 +203,10 @@ void MwmSet::GetMwmsInfo(vector<shared_ptr<MwmInfo>> & info) const info.clear(); info.reserve(m_info.size()); for (auto const & p : m_info) - info.push_back(p.second); + { + if (!p.second.empty()) + info.push_back(p.second.back()); + } } MwmSet::TMwmValueBasePtr MwmSet::LockValue(MwmId const & id) @@ -216,7 +234,7 @@ MwmSet::TMwmValueBasePtr MwmSet::LockValueImpl(MwmId const & id) return result; } } - return CreateValue(info->m_fileName); + return CreateValue(info->GetLocalFile()); } void MwmSet::UnlockValue(MwmId const & id, TMwmValueBasePtr p) @@ -233,22 +251,10 @@ void MwmSet::UnlockValueImpl(MwmId const & id, TMwmValueBasePtr p) return; shared_ptr<MwmInfo> const & info = id.GetInfo(); - CHECK_GREATER(info->m_lockCount, 0, ()); + ASSERT_GREATER(info->m_lockCount, 0, ()); --info->m_lockCount; - 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->m_lockCount == 0 && info->GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER) + VERIFY(DeregisterImpl(id), ()); if (info->IsUpToDate()) { @@ -267,20 +273,20 @@ void MwmSet::ClearCache() ClearCacheImpl(m_cache.begin(), m_cache.end()); } -MwmSet::MwmId MwmSet::GetMwmIdByFileName(TMwmFileName const & fileName) const +MwmSet::MwmId MwmSet::GetMwmIdByCountryFile(CountryFile const & countryFile) const { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByFileNameImpl(fileName); - ASSERT(id.IsAlive(), ("Can't get an mwm's (", fileName, ") identifier.")); + MwmId const id = GetMwmIdByCountryFileImpl(countryFile); + ASSERT(id.IsAlive(), ("Can't get an mwm's (", countryFile.GetNameWithoutExt(), ") identifier.")); return id; } -MwmSet::MwmLock MwmSet::GetMwmLockByFileName(TMwmFileName const & fileName) +MwmSet::MwmLock MwmSet::GetMwmLockByCountryFile(CountryFile const & countryFile) { lock_guard<mutex> lock(m_lock); - MwmId const id = GetMwmIdByFileNameImpl(fileName); + MwmId const id = GetMwmIdByCountryFileImpl(countryFile); TMwmValueBasePtr value(nullptr); if (id.IsAlive()) value = LockValueImpl(id); diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp index 25682c16b0..dfc225f4c4 100644 --- a/indexer/mwm_set.hpp +++ b/indexer/mwm_set.hpp @@ -2,6 +2,9 @@ #include "indexer/mwm_version.hpp" +#include "platform/country_file.hpp" +#include "platform/local_country_file.hpp" + #include "geometry/rect2d.hpp" #include "base/macros.hpp" @@ -31,10 +34,9 @@ public: enum Status { - 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 + 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. }; MwmInfo(); @@ -46,31 +48,32 @@ public: inline Status GetStatus() const { return m_status; } + inline bool IsUpToDate() const { return IsRegistered(); } + inline bool IsRegistered() const { - return m_status == STATUS_UP_TO_DATE || m_status == STATUS_PENDING_UPDATE; + return m_status == STATUS_REGISTERED; } - inline bool IsUpToDate() const { return m_status == STATUS_UP_TO_DATE; } + inline platform::LocalCountryFile const & GetLocalFile() const { return m_file; } + + inline string GetCountryName() const { return m_file.GetCountryFile().GetNameWithoutExt(); } - inline string const & GetFileName() const { return m_fileName; } + inline int64_t GetVersion() const { return m_file.GetVersion(); } MwmTypeT GetType() const; private: inline void SetStatus(Status status) { m_status = status; } - string m_fileName; ///< Path to the mwm file. - Status m_status; ///< Current country status. - uint8_t m_lockCount; ///< Number of locks. + platform::LocalCountryFile m_file; ///< 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: @@ -94,9 +97,9 @@ public: friend ostream & operator<<(ostream & os, MwmId const & id) { if (id.m_info.get()) - os << "MwmId[" << id.m_info->GetFileName() << "]"; + os << "MwmId [" << id.m_info->GetCountryName() << "]"; else - os << "MwmId[invalid]"; + os << "MwmId [invalid]"; return os; } @@ -152,41 +155,43 @@ public: /// Registers a new map. /// - /// \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: + /// \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. /// - /// * 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 - //@{ + /// *NOTE* When a new version for the same country is registered, + /// all previous versions will be automatically deregistered. protected: - WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterImpl(TMwmFileName const & fileName); + WARN_UNUSED_RESULT pair<MwmLock, bool> RegisterImpl(platform::LocalCountryFile const & localFile); public: - WARN_UNUSED_RESULT pair<MwmLock, bool> Register(TMwmFileName const & fileName); + WARN_UNUSED_RESULT pair<MwmLock, bool> Register(platform::LocalCountryFile const & localFile); //@} /// @name Remove mwm. //@{ protected: - /// Deregisters a map from the set when it's possible. Note that an - /// underlying file is not deleted. + /// Deregisters a map from internal records. /// - /// @return true when the map was deregistered. + /// \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 DeregisterImpl(MwmId const & id); - bool DeregisterImpl(TMwmFileName const & ofileName); + bool DeregisterImpl(platform::CountryFile const & countryFile); //@} public: - bool Deregister(TMwmFileName const & fileName); + bool Deregister(platform::CountryFile const & countryFile); void DeregisterAll(); //@} - /// @param[in] file File name without extension. - bool IsLoaded(TMwmFileName const & fileName) const; + /// Returns true when country is registered and can be used. + bool IsLoaded(platform::CountryFile const & countryFile) const; /// Get ids of all mwms. Some of them may be with not active status. /// In that case, LockValue returns NULL. @@ -194,14 +199,14 @@ public: void ClearCache(); - MwmId GetMwmIdByFileName(TMwmFileName const & fileName) const; + MwmId GetMwmIdByCountryFile(platform::CountryFile const & countryFile) const; - MwmLock GetMwmLockByFileName(TMwmFileName const & fileName); + MwmLock GetMwmLockByCountryFile(platform::CountryFile const & countryFile); protected: /// @return True when file format version was successfully read to MwmInfo. - virtual bool GetVersion(TMwmFileName const & fileName, MwmInfo & info) const = 0; - virtual TMwmValueBasePtr CreateValue(string const & name) const = 0; + virtual bool GetVersion(platform::LocalCountryFile const & localFile, MwmInfo & info) const = 0; + virtual TMwmValueBasePtr CreateValue(platform::LocalCountryFile const & localFile) const = 0; void Cleanup(); @@ -226,7 +231,7 @@ protected: /// Find mwm with a given name. /// @precondition This function is always called under mutex m_lock. - MwmId GetMwmIdByFileNameImpl(TMwmFileName const & fileName) const; + MwmId GetMwmIdByCountryFileImpl(platform::CountryFile const & countryFile) const; /// @precondition This function is always called under mutex m_lock. WARN_UNUSED_RESULT inline MwmLock GetLock(MwmId const & id) @@ -236,12 +241,9 @@ protected: // This method is called under m_lock when mwm is removed from a // registry. - 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) {} + virtual void OnMwmDeregistered(platform::LocalCountryFile const & localFile) {} - TMwmInfoTable m_info; + map<string, vector<shared_ptr<MwmInfo>>> m_info; mutable mutex m_lock; }; |