diff options
author | Yuri Gorshenin <y@maps.me> | 2015-06-17 15:15:27 +0300 |
---|---|---|
committer | Alex Zolotarev <alex@maps.me> | 2015-09-23 02:53:26 +0300 |
commit | e0470160313b3ed0bc396ffec72b16fded6b8520 (patch) | |
tree | ae8e0b0ad75acdac4a5a06039e9ebc99ae7ab521 | |
parent | 598e71858b63ef4e587efd932db41474f8f516a0 (diff) |
Review fixes.
-rw-r--r-- | android/jni/com/mapswithme/maps/Framework.cpp | 29 | ||||
-rw-r--r-- | generator/routing_generator.cpp | 3 | ||||
-rw-r--r-- | indexer/index.hpp | 4 | ||||
-rw-r--r-- | indexer/indexer_tests/mwm_set_test.cpp | 9 | ||||
-rw-r--r-- | indexer/mwm_set.cpp | 10 | ||||
-rw-r--r-- | indexer/mwm_set.hpp | 10 | ||||
-rw-r--r-- | integration_tests/osrm_test_tools.cpp | 6 | ||||
-rw-r--r-- | map/active_maps_layout.hpp | 3 | ||||
-rw-r--r-- | map/benchmark_engine.cpp | 1 | ||||
-rw-r--r-- | map/feature_vec_model.cpp | 10 | ||||
-rw-r--r-- | map/feature_vec_model.hpp | 4 | ||||
-rw-r--r-- | map/framework.cpp | 8 | ||||
-rw-r--r-- | pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp | 14 | ||||
-rw-r--r-- | platform/local_country_file.cpp | 3 | ||||
-rw-r--r-- | platform/local_country_file_utils.cpp | 1 | ||||
-rw-r--r-- | platform/platform.cpp | 1 | ||||
-rw-r--r-- | platform/platform_tests/local_country_file_tests.cpp | 9 | ||||
-rw-r--r-- | routing/osrm_router.cpp | 2 | ||||
-rw-r--r-- | routing/osrm_router.hpp | 2 | ||||
-rw-r--r-- | routing/routing_mapping.cpp | 2 | ||||
-rw-r--r-- | storage/storage.cpp | 27 | ||||
-rw-r--r-- | storage/storage.hpp | 2 | ||||
-rw-r--r-- | storage/storage_tests/storage_tests.cpp | 28 |
23 files changed, 99 insertions, 89 deletions
diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index 3187676797..6e9f751f5d 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -558,7 +558,7 @@ namespace android void Framework::GetMapsWithoutSearch(vector<string> & out) const { - ASSERT ( out.empty(), () ); + ASSERT(out.empty(), ()); ::Platform const & pl = GetPlatform(); @@ -569,20 +569,21 @@ namespace android { CountryFile const countryFile = localFile.GetCountryFile(); // skip World and WorldCoast - if (countryFile.GetNameWithoutExt() != WORLD_FILE_NAME && - countryFile.GetNameWithoutExt() != WORLD_COASTS_FILE_NAME) + if (countryFile.GetNameWithoutExt() == WORLD_FILE_NAME || + countryFile.GetNameWithoutExt() == WORLD_COASTS_FILE_NAME) { - try - { - FilesContainerR cont(pl.GetCountryReader(localFile, TMapOptions::EMap)); - if (!cont.IsExist(SEARCH_INDEX_FILE_TAG)) - out.push_back(countryFile.GetNameWithoutExt()); - } - catch (RootException const & ex) - { - // sdcard can contain dummy _*.mwm files. Supress this errors. - LOG(LWARNING, ("Bad mwm file:", countryFile.GetNameWithoutExt(), "Error:", ex.Msg())); - } + continue; + } + try + { + FilesContainerR cont(pl.GetCountryReader(localFile, TMapOptions::EMap)); + if (!cont.IsExist(SEARCH_INDEX_FILE_TAG)) + out.push_back(countryFile.GetNameWithoutExt()); + } + catch (RootException const & ex) + { + // sdcard can contain dummy _*.mwm files. Suppress these errors. + LOG(LWARNING, ("Bad mwm file:", countryFile.GetNameWithoutExt(), "Error:", ex.Msg())); } } } diff --git a/generator/routing_generator.cpp b/generator/routing_generator.cpp index 169e09f2ad..2f37a187a3 100644 --- a/generator/routing_generator.cpp +++ b/generator/routing_generator.cpp @@ -227,7 +227,10 @@ void BuildRoutingIndex(string const & baseDir, string const & countryName, strin classificator::Load(); CountryFile countryFile(countryName); + + // Correct mwm version doesn't matter here - we just need access to mwm files via Index. LocalCountryFile localFile(baseDir, countryFile, 0 /* version */); + localFile.SyncWithDisk(); Index index; pair<MwmSet::MwmLock, bool> const p = index.Register(localFile); if (!p.second) diff --git a/indexer/index.hpp b/indexer/index.hpp index ba5e031cf4..20d9d75c5d 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -66,9 +66,9 @@ public: /// Registers a new map. /// /// \return A pair of an MwmLock and a flag. There are three cases: - /// * the map is newer than the newset registered - returns + /// * the map is newer than the newest registered - returns /// active lock and set flag - /// * the map is older than the newset registered - returns inactive lock and + /// * 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. diff --git a/indexer/indexer_tests/mwm_set_test.cpp b/indexer/indexer_tests/mwm_set_test.cpp index cc62d5d44e..30b9adebe5 100644 --- a/indexer/indexer_tests/mwm_set_test.cpp +++ b/indexer/indexer_tests/mwm_set_test.cpp @@ -10,6 +10,8 @@ using platform::CountryFile; using platform::LocalCountryFile; +using TMwmsInfo = unordered_map<string, shared_ptr<MwmInfo>>; + namespace { class MwmValue : public MwmSet::MwmValueBase @@ -38,7 +40,7 @@ public: ~TestMwmSet() { Cleanup(); } }; -void GetMwmsInfo(MwmSet const & mwmSet, unordered_map<string, shared_ptr<MwmInfo>> & mwmsInfo) +void GetMwmsInfo(MwmSet const & mwmSet, TMwmsInfo & mwmsInfo) { vector<shared_ptr<MwmInfo>> mwmsInfoList; mwmSet.GetMwmsInfo(mwmsInfoList); @@ -48,8 +50,7 @@ void GetMwmsInfo(MwmSet const & mwmSet, unordered_map<string, shared_ptr<MwmInfo mwmsInfo[info->GetCountryName()] = info; } -void TestFilesPresence(unordered_map<string, shared_ptr<MwmInfo>> const & mwmsInfo, - initializer_list<string> const & expectedNames) +void TestFilesPresence(TMwmsInfo const & mwmsInfo, initializer_list<string> const & expectedNames) { TEST_EQUAL(expectedNames.size(), mwmsInfo.size(), ()); for (string const & countryFileName : expectedNames) @@ -61,7 +62,7 @@ void TestFilesPresence(unordered_map<string, shared_ptr<MwmInfo>> const & mwmsIn UNIT_TEST(MwmSetSmokeTest) { TestMwmSet mwmSet; - unordered_map<string, shared_ptr<MwmInfo>> mwmsInfo; + TMwmsInfo mwmsInfo; UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("0"))); UNUSED_VALUE(mwmSet.Register(LocalCountryFile::MakeForTesting("1"))); diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp index a360a4181b..d6961ebdd0 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -95,8 +95,6 @@ pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile) lock_guard<mutex> lock(m_lock); CountryFile const & countryFile = localFile.GetCountryFile(); - string const name = countryFile.GetNameWithoutExt(); - MwmId const id = GetMwmIdByCountryFileImpl(countryFile); if (!id.IsAlive()) return RegisterImpl(localFile); @@ -110,6 +108,7 @@ pair<MwmSet::MwmLock, bool> MwmSet::Register(LocalCountryFile const & localFile) return RegisterImpl(localFile); } + string const name = countryFile.GetNameWithoutExt(); // Update the status of the mwm with the same version. if (info->GetVersion() == localFile.GetVersion()) { @@ -144,12 +143,11 @@ bool MwmSet::DeregisterImpl(MwmId const & id) if (!id.IsAlive()) return false; shared_ptr<MwmInfo> const & info = id.GetInfo(); - string const name = info->GetCountryName(); if (info->m_lockCount == 0) { info->SetStatus(MwmInfo::STATUS_DEREGISTERED); - vector<shared_ptr<MwmInfo>> & infos = m_info[name]; + vector<shared_ptr<MwmInfo>> & infos = m_info[info->GetCountryName()]; infos.erase(remove(infos.begin(), infos.end(), info), infos.end()); OnMwmDeregistered(info->GetLocalFile()); return true; @@ -253,10 +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 && info->GetStatus() == MwmInfo::STATUS_MARKED_TO_DEREGISTER) - CHECK(DeregisterImpl(id), ()); + VERIFY(DeregisterImpl(id), ()); if (info->IsUpToDate()) { diff --git a/indexer/mwm_set.hpp b/indexer/mwm_set.hpp index 9365e2b81e..dfc225f4c4 100644 --- a/indexer/mwm_set.hpp +++ b/indexer/mwm_set.hpp @@ -35,8 +35,8 @@ 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_MARKED_TO_DEREGISTER, ///< Mwm is marked to be deregistered as soon as possible. + STATUS_DEREGISTERED, ///< Mwm is deregistered. }; MwmInfo(); @@ -156,9 +156,9 @@ public: /// Registers a new map. /// /// \return A pair of an MwmLock and a flag. There are three cases: - /// * the map is newer than the newset registered - returns - /// active lock and set flag - /// * the map is older than the newset registered - returns inactive lock and + /// * 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. diff --git a/integration_tests/osrm_test_tools.cpp b/integration_tests/osrm_test_tools.cpp index 72acd2e3d6..4bf6cd94b8 100644 --- a/integration_tests/osrm_test_tools.cpp +++ b/integration_tests/osrm_test_tools.cpp @@ -37,8 +37,7 @@ namespace namespace integration { -shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher( - vector<LocalCountryFile> const & localFiles) + shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher(vector<LocalCountryFile> const & localFiles) { size_t const maxOpenFileNumber = 1024; ChangeMaxNumberOfOpenFiles(maxOpenFileNumber); @@ -47,11 +46,10 @@ shared_ptr<model::FeaturesFetcher> CreateFeaturesFetcher( for (LocalCountryFile const & localFile : localFiles) { - LOG(LINFO, ("Registering mwm:", localFile.GetPath(TMapOptions::EMap))); pair<MwmSet::MwmLock, bool> result = featuresFetcher->RegisterMap(localFile); if (!result.second) { - ASSERT(false, ()); + ASSERT(false, ("Can't register", localFile)); return nullptr; } } diff --git a/map/active_maps_layout.hpp b/map/active_maps_layout.hpp index e4e7cbb87d..331d17296b 100644 --- a/map/active_maps_layout.hpp +++ b/map/active_maps_layout.hpp @@ -3,9 +3,8 @@ #include "storage/index.hpp" #include "storage/storage_defines.hpp" -#include "platform/country_file.hpp" - #include "platform/country_defines.hpp" +#include "platform/country_file.hpp" #include "base/buffer_vector.hpp" diff --git a/map/benchmark_engine.cpp b/map/benchmark_engine.cpp index 3e2a6bcffe..208cd1bb94 100644 --- a/map/benchmark_engine.cpp +++ b/map/benchmark_engine.cpp @@ -133,7 +133,6 @@ void BenchmarkEngine::PrepareMaps() // add only maps needed for benchmarks MapsCollector collector; ForEachBenchmarkRecord(collector); - string const directory = GetPlatform().WritableDir(); for (string const & map : collector.m_maps) { LOG(LINFO, ("Looking for:", map)); diff --git a/map/feature_vec_model.cpp b/map/feature_vec_model.cpp index e1ba82c165..ca5f1f4ba0 100644 --- a/map/feature_vec_model.cpp +++ b/map/feature_vec_model.cpp @@ -18,9 +18,15 @@ using platform::LocalCountryFile; namespace model { -FeaturesFetcher::FeaturesFetcher() { m_multiIndex.AddObserver(*this); } +FeaturesFetcher::FeaturesFetcher() +{ + m_multiIndex.AddObserver(*this); +} -FeaturesFetcher::~FeaturesFetcher() { m_multiIndex.RemoveObserver(*this); } +FeaturesFetcher::~FeaturesFetcher() +{ + m_multiIndex.RemoveObserver(*this); +} // While reading any files (classificator or mwm), there are 2 types of possible exceptions: // Reader::Exception, FileAbsentException. diff --git a/map/feature_vec_model.hpp b/map/feature_vec_model.hpp index 39ddabd690..9698d5a745 100644 --- a/map/feature_vec_model.hpp +++ b/map/feature_vec_model.hpp @@ -48,9 +48,9 @@ class FeaturesFetcher : public Index::Observer /// Registers a new map. /// /// \return A pair of an MwmLock and a flag. There are three cases: - /// * the map is newer than the newset registered - returns + /// * the map is newer than the newest registered - returns /// active lock and set flag - /// * the map is older than the newset registered - returns inactive lock and + /// * 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. diff --git a/map/framework.cpp b/map/framework.cpp index 9bbc86ad94..e4083f8004 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -372,9 +372,11 @@ void Framework::DeleteCountry(storage::TIndex const & index, TMapOptions opt) { InvalidateRect(GetCountryBounds(countryFile.GetNameWithoutExt()), true /* doForceUpdate */); } - } + // TODO (@ldragunov, @gorshenin): rewrite routing session to use MwmLocks. Thus, + // it won' be needed to reset it after maps update. m_routingSession.Reset(); return; + } case TMapOptions::ECarRouting: m_routingSession.Reset(); m_storage.DeleteCountry(index, opt); @@ -421,6 +423,8 @@ void Framework::ShowCountry(TIndex const & index) void Framework::UpdateAfterDownload(LocalCountryFile const & localFile) { + // TODO (@ldragunov, @gorshenin): rewrite routing session to use MwmLocks. Thus, + // it won' be needed to reset it after maps update. m_routingSession.Reset(); if (!HasOptions(localFile.GetFiles(), TMapOptions::EMap)) @@ -455,7 +459,7 @@ void Framework::RegisterAllMaps() for (CountryFile const & countryFile : maps) { shared_ptr<LocalCountryFile> localFile = m_storage.GetLatestLocalFile(countryFile); - if (!localFile.get()) + if (!localFile) continue; pair<MwmSet::MwmLock, bool> const p = RegisterMap(*localFile); if (!p.second) diff --git a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp index 572388e540..fe9e956d73 100644 --- a/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp +++ b/pedestrian_routing_benchmarks/pedestrian_routing_benchmarks.cpp @@ -71,7 +71,7 @@ m2::PointD GetPointOnEdge(routing::Edge & e, double posAlong) void GetNearestPedestrianEdges(Index & index, m2::PointD const & pt, vector<pair<routing::Edge, m2::PointD>> & edges) { - MwmSet::MwmId const id = index.GetMwmIdByFileName(MAP_FILE); + MwmSet::MwmId const id = index.GetMwmIdByCountryFile(CountryFile(MAP_NAME)); TEST(id.IsAlive(), ()); routing::PedestrianModel const vehicleModel; @@ -109,7 +109,7 @@ void TestRouter(routing::IRouter & router, m2::PointD const & startPos, m2::Poin void TestRouters(Index const & index, m2::PointD const & startPos, m2::PointD const & finalPos) { - auto const countryFileFn = [](m2::PointD const & /* point */){ return MAP_FILE; }; + auto const countryFileFn = [](m2::PointD const & /* point */) { return MAP_NAME; }; // find route by A*-bidirectional algorithm routing::Route routeFoundByAstarBidirectional(""); @@ -129,10 +129,10 @@ void TestTwoPointsOnFeature(m2::PointD const & startPos, m2::PointD const & fina { classificator::Load(); - Index index; - CountryFile countryFile(MAP_NAME); LocalCountryFile localFile = LocalCountryFile::MakeForTesting(MAP_NAME); + + Index index; UNUSED_VALUE(index.RegisterMap(localFile)); TEST(index.IsLoaded(countryFile), ()); MwmSet::MwmId const id = index.GetMwmIdByCountryFile(countryFile); @@ -146,8 +146,10 @@ void TestTwoPointsOnFeature(m2::PointD const & startPos, m2::PointD const & fina GetNearestPedestrianEdges(index, finalPos, finalEdges); TEST(!finalEdges.empty(), ()); - m2::PointD const startPosOnFeature = GetPointOnEdge(startEdges.front().first, 0.0 /* the start point of the feature */ ); - m2::PointD const finalPosOnFeature = GetPointOnEdge(finalEdges.front().first, 1.0 /* the end point of the feature */ ); + m2::PointD const startPosOnFeature = + GetPointOnEdge(startEdges.front().first, 0.0 /* the start point of the feature */); + m2::PointD const finalPosOnFeature = + GetPointOnEdge(finalEdges.front().first, 1.0 /* the end point of the feature */); TestRouters(index, startPosOnFeature, finalPosOnFeature); } diff --git a/platform/local_country_file.cpp b/platform/local_country_file.cpp index 4cf20444ce..2ccf776392 100644 --- a/platform/local_country_file.cpp +++ b/platform/local_country_file.cpp @@ -49,10 +49,7 @@ void LocalCountryFile::DeleteFromDisk(TMapOptions files) const if (OnDisk(file) && HasOptions(files, file)) { if (!my::DeleteFileX(GetPath(file))) - { LOG(LERROR, (file, "from", *this, "wasn't deleted from disk.")); - ASSERT(false, ()); - } } } } diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index 4c2ca4993a..b5ded8a11c 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -189,6 +189,7 @@ shared_ptr<LocalCountryFile> PreparePlaceForCountryFiles(CountryFile const & cou return make_shared<LocalCountryFile>(directory, countryFile, version); } default: + LOG(LERROR, ("Can't prepare place for", countryFile, "(", version, ") :", ret)); return shared_ptr<LocalCountryFile>(); } } diff --git a/platform/platform.cpp b/platform/platform.cpp index d7b04686cf..bfffb65a7c 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -153,6 +153,5 @@ string Platform::GetIndexFileName(string const & mwmName, string const & extensi ModelReader * Platform::GetCountryReader(platform::LocalCountryFile const & file, TMapOptions options) const { - LOG(LINFO, ("Getting country reader for:", file, options)); return GetReader(file.GetPath(options), "f"); } diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index 12aace8803..46981142f6 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -60,19 +60,20 @@ public: if (m_reset) return; - Platform::EError ret = Platform::RmDir(GetFullPath()); + string const fullPath = GetFullPath(); + Platform::EError ret = Platform::RmDir(fullPath); switch (ret) { case Platform::ERR_OK: break; case Platform::ERR_FILE_DOES_NOT_EXIST: - LOG(LWARNING, (GetFullPath(), "was deleted before destruction of ScopedTestDir.")); + LOG(LWARNING, (fullPath, "was deleted before destruction of ScopedTestDir.")); break; case Platform::ERR_DIRECTORY_NOT_EMPTY: - LOG(LWARNING, ("There are files in", GetFullPath())); + LOG(LWARNING, ("There are files in", fullPath)); break; default: - LOG(LWARNING, ("Platform::RmDir() error for", GetFullPath(), ":", ret)); + LOG(LWARNING, ("Platform::RmDir() error for", fullPath, ":", ret)); break; } } diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 4f1f9dea72..e52a2bc6c8 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -352,7 +352,7 @@ public: OsrmRouter::OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn, TCountryLocalFileFn const & countryLocalFileFn, - RoutingVisualizerFn routingVisualization) + TRoutingVisualizerFn routingVisualization) : m_pIndex(index), m_indexManager(countryFileFn, countryLocalFileFn, index), m_routingVisualization(routingVisualization) diff --git a/routing/osrm_router.hpp b/routing/osrm_router.hpp index e10a4a756b..813fffd1cd 100644 --- a/routing/osrm_router.hpp +++ b/routing/osrm_router.hpp @@ -38,7 +38,7 @@ public: OsrmRouter(Index const * index, TCountryFileFn const & countryFileFn, TCountryLocalFileFn const & countryLocalFileFn, - RoutingVisualizerFn routingVisualization = nullptr); + TRoutingVisualizerFn routingVisualization = nullptr); virtual string GetName() const; diff --git a/routing/routing_mapping.cpp b/routing/routing_mapping.cpp index c760cf1921..a7c6f3bf97 100644 --- a/routing/routing_mapping.cpp +++ b/routing/routing_mapping.cpp @@ -143,7 +143,7 @@ TRoutingMappingPtr RoutingIndexManager::GetMappingByName(string const & mapName) { shared_ptr<platform::LocalCountryFile> localFile = m_countryLocalFileFn(mapName); // Return invalid mapping when file does not exist. - if (!localFile.get()) + if (!localFile) return RoutingMapping::MakeInvalid(platform::CountryFile(mapName)); // Check if we have already loaded this file. diff --git a/storage/storage.cpp b/storage/storage.cpp index 8994f873cd..b9f7e75f58 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -169,16 +169,17 @@ CountriesContainerT const & NodeFromIndex(CountriesContainerT const & root, TInd // complex logic to avoid [] out_of_bounds exceptions if (index.m_group == TIndex::INVALID || index.m_group >= static_cast<int>(root.SiblingsCount())) return root; - else + if (index.m_country == TIndex::INVALID || + index.m_country >= static_cast<int>(root[index.m_group].SiblingsCount())) + { + return root[index.m_group]; + } + if (index.m_region == TIndex::INVALID || + index.m_region >= static_cast<int>(root[index.m_group][index.m_country].SiblingsCount())) { - if (index.m_country == TIndex::INVALID || - index.m_country >= static_cast<int>(root[index.m_group].SiblingsCount())) - return root[index.m_group]; - if (index.m_region == TIndex::INVALID || - index.m_region >= static_cast<int>(root[index.m_group][index.m_country].SiblingsCount())) - return root[index.m_group][index.m_country]; - return root[index.m_group][index.m_country][index.m_region]; + return root[index.m_group][index.m_country]; } + return root[index.m_group][index.m_country][index.m_region]; } Country const & Storage::CountryByIndex(TIndex const & index) const @@ -265,7 +266,7 @@ shared_ptr<LocalCountryFile> Storage::GetLatestLocalFile(TIndex const & index) c TStatus Storage::CountryStatus(TIndex const & index) const { - // first, check if we already downloading this country or have in in the queue + // Check if we already downloading this country or have it in the queue if (IsCountryInQueue(index)) { if (IsCountryFirstInQueue(index)) @@ -274,7 +275,7 @@ TStatus Storage::CountryStatus(TIndex const & index) const return TStatus::EInQueue; } - // second, check if this country has failed while downloading + // Check if this country has failed while downloading. if (m_failedCountries.count(index) > 0) return TStatus::EDownloadFailed; @@ -770,7 +771,7 @@ shared_ptr<LocalCountryFile> Storage::GetLocalFile(TIndex const & index, int64_t void Storage::RegisterCountryFiles(shared_ptr<LocalCountryFile> localFile) { - ASSERT(localFile.get(), ()); + CHECK(localFile.get(), ()); localFile->SyncWithDisk(); TIndex const index = FindIndexByFile(localFile->GetCountryFile().GetNameWithoutExt()); @@ -784,7 +785,7 @@ void Storage::RegisterCountryFiles(shared_ptr<LocalCountryFile> localFile) void Storage::RegisterCountryFiles(TIndex const & index, string const & directory, int64_t version) { shared_ptr<LocalCountryFile> localFile = GetLocalFile(index, version); - if (localFile.get() != nullptr) + if (localFile) return; CountryFile const & countryFile = GetCountryFile(index); @@ -848,7 +849,7 @@ bool Storage::DeleteCountryFilesFromDownloader(TIndex const & index, TMapOptions void Storage::KickDownloaderAfterDeletionOfCountryFiles(TIndex const & index) { - // Do nothing when there're no counties to download or when downloader is busy. + // Do nothing when there're no countries to download or when downloader is busy. if (m_queue.empty() || !m_downloader->IsIdle()) return; if (IsCountryFirstInQueue(index)) diff --git a/storage/storage.hpp b/storage/storage.hpp index c080986061..0e3b1f1ccb 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -38,7 +38,7 @@ class Storage /// RunOnUIThread. If not, at least use a syncronization object. TQueue m_queue; - /// stores countries which download has failed recently + /// stores countries whose download has failed recently typedef set<TIndex> TCountriesSet; TCountriesSet m_failedCountries; diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 55eefbf7d9..9bc206024c 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -48,22 +48,22 @@ public: m_slot = m_storage.Subscribe( bind(&CountryDownloaderChecker::OnCountryStatusChanged, this, _1), bind(&CountryDownloaderChecker::OnCountryDownloadingProgress, this, _1, _2)); - CHECK(m_index.IsValid(), (m_countryFile)); - CHECK(!m_transitionList.empty(), (m_countryFile)); + TEST(m_index.IsValid(), (m_countryFile)); + TEST(!m_transitionList.empty(), (m_countryFile)); } void StartDownload() { - CHECK_EQUAL(0, m_currStatus, (m_countryFile)); - CHECK_LESS(m_currStatus, m_transitionList.size(), (m_countryFile)); - CHECK_EQUAL(m_transitionList[m_currStatus], m_storage.CountryStatusEx(m_index), + TEST_EQUAL(0, m_currStatus, (m_countryFile)); + TEST_LESS(m_currStatus, m_transitionList.size(), (m_countryFile)); + TEST_EQUAL(m_transitionList[m_currStatus], m_storage.CountryStatusEx(m_index), (m_countryFile)); m_storage.DownloadCountry(m_index, m_files); } virtual ~CountryDownloaderChecker() { - CHECK_EQUAL(m_currStatus + 1, m_transitionList.size(), (m_countryFile)); + TEST_EQUAL(m_currStatus + 1, m_transitionList.size(), (m_countryFile)); m_storage.Unsubscribe(m_slot); } @@ -76,8 +76,8 @@ private: TStatus const nextStatus = m_storage.CountryStatusEx(m_index); LOG(LINFO, (m_countryFile, "status transition: from", m_transitionList[m_currStatus], "to", nextStatus)); - CHECK_LESS(m_currStatus + 1, m_transitionList.size(), (m_countryFile)); - CHECK_EQUAL(nextStatus, m_transitionList[m_currStatus + 1], (m_countryFile)); + TEST_LESS(m_currStatus + 1, m_transitionList.size(), (m_countryFile)); + TEST_EQUAL(nextStatus, m_transitionList[m_currStatus + 1], (m_countryFile)); ++m_currStatus; if (m_transitionList[m_currStatus] == TStatus::EDownloading) { @@ -93,12 +93,12 @@ private: LOG(LINFO, (m_countryFile, "downloading progress:", progress)); - CHECK_GREATER(progress.first, m_bytesDownloaded, (m_countryFile)); + TEST_GREATER(progress.first, m_bytesDownloaded, (m_countryFile)); m_bytesDownloaded = progress.first; - CHECK_LESS_OR_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, (m_countryFile)); + TEST_LESS_OR_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, (m_countryFile)); LocalAndRemoteSizeT localAndRemoteSize = m_storage.CountrySizeInBytes(m_index, m_files); - CHECK_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, (m_countryFile)); + TEST_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, (m_countryFile)); } Storage & m_storage; @@ -167,15 +167,15 @@ shared_ptr<LocalCountryFile> CreateDummyMapFile(CountryFile const & countryFile, { shared_ptr<LocalCountryFile> localFile = platform::PreparePlaceForCountryFiles(countryFile, version); - CHECK(localFile.get(), ("Can't prepare place for", countryFile, "(version ", version, ")")); + TEST(localFile.get(), ("Can't prepare place for", countryFile, "(version ", version, ")")); { string const zeroes(size, '\0'); FileWriter writer(localFile->GetPath(TMapOptions::EMap)); writer.Write(zeroes.data(), zeroes.size()); } localFile->SyncWithDisk(); - CHECK_EQUAL(TMapOptions::EMap, localFile->GetFiles(), ()); - CHECK_EQUAL(size, localFile->GetSize(TMapOptions::EMap), ()); + TEST_EQUAL(TMapOptions::EMap, localFile->GetFiles(), ()); + TEST_EQUAL(size, localFile->GetSize(TMapOptions::EMap), ()); return localFile; } |