From 9fd65dac7a2a2737bfb1cf0188b8195f9148143a Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Thu, 26 Sep 2019 17:52:58 +0300 Subject: [PATCH] [platform] LocalCountryFile::GetFiles is removed --- .../srtm_coverage_checker.cpp | 2 +- generator/utils.cpp | 5 ++-- map/framework.cpp | 2 +- platform/local_country_file.cpp | 23 ++++++++++------ platform/local_country_file.hpp | 15 +++++++---- .../local_country_file_tests.cpp | 8 +++--- .../get_altitude_test.cpp | 2 +- storage/storage.cpp | 2 +- storage/storage_tests/storage_tests.cpp | 26 ++++++++++--------- 9 files changed, 48 insertions(+), 37 deletions(-) diff --git a/generator/srtm_coverage_checker/srtm_coverage_checker.cpp b/generator/srtm_coverage_checker/srtm_coverage_checker.cpp index fac50dec1c..322ed2cd06 100644 --- a/generator/srtm_coverage_checker/srtm_coverage_checker.cpp +++ b/generator/srtm_coverage_checker/srtm_coverage_checker.cpp @@ -59,7 +59,7 @@ int main(int argc, char * argv[]) for (auto & file : localFiles) { file.SyncWithDisk(); - if (file.GetFiles() != MapOptions::MapWithCarRouting) + if (!file.OnDisk(MapOptions::MapWithCarRouting)) { LOG(LINFO, ("Warning! Routing file not found for:", file.GetCountryName())); continue; diff --git a/generator/utils.cpp b/generator/utils.cpp index 3bf1017e13..34aef7aa08 100644 --- a/generator/utils.cpp +++ b/generator/utils.cpp @@ -23,9 +23,8 @@ SingleMwmDataSource::SingleMwmDataSource(std::string const & mwmPath) { m_countryFile = platform::LocalCountryFile::MakeTemporary(mwmPath); m_countryFile.SyncWithDisk(); - CHECK_EQUAL( - m_countryFile.GetFiles(), MapOptions::MapWithCarRouting, - ("No correct mwm corresponding to local country file:", m_countryFile, ". Path:", mwmPath)); + CHECK(m_countryFile.OnDisk(MapOptions::MapWithCarRouting), + ("No correct mwm corresponding to local country file:", m_countryFile, ". Path:", mwmPath)); auto const result = m_dataSource.Register(m_countryFile); CHECK_EQUAL(result.second, MwmSet::RegResult::Success, ()); diff --git a/map/framework.cpp b/map/framework.cpp index 989842b58c..8937ec779f 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -676,7 +676,7 @@ void Framework::OnCountryFileDownloaded(storage::CountryId const & countryId, m2::RectD rect = MercatorBounds::FullRect(); - if (localFile && HasOptions(localFile->GetFiles(), MapOptions::Map)) + if (localFile && localFile->OnDisk(MapOptions::Map)) { // Add downloaded map. auto p = m_featuresFetcher.RegisterMap(*localFile); diff --git a/platform/local_country_file.cpp b/platform/local_country_file.cpp index 1c56b4b871..35dee27efe 100644 --- a/platform/local_country_file.cpp +++ b/platform/local_country_file.cpp @@ -15,8 +15,16 @@ using namespace std; namespace platform { +namespace +{ +void SetOptions(boost::optional & destination, MapOptions value) +{ + destination = destination ? SetOptions(destination.get(), value) : value; +} +} // namespace + LocalCountryFile::LocalCountryFile() - : m_version(0), m_files(MapOptions::Nothing), m_mapSize(0), m_routingSize(0) + : m_version(0), m_mapSize(0), m_routingSize(0) { } @@ -25,7 +33,6 @@ LocalCountryFile::LocalCountryFile(string const & directory, CountryFile const & : m_directory(directory), m_countryFile(countryFile), m_version(version), - m_files(MapOptions::Nothing), m_mapSize(0), m_routingSize(0) { @@ -33,30 +40,30 @@ LocalCountryFile::LocalCountryFile(string const & directory, CountryFile const & void LocalCountryFile::SyncWithDisk() { - m_files = MapOptions::Nothing; + m_files = {}; m_mapSize = 0; m_routingSize = 0; Platform & platform = GetPlatform(); if (platform.GetFileSizeByFullPath(GetPath(MapOptions::Diff), m_mapSize)) { - m_files = SetOptions(m_files, MapOptions::Diff); + SetOptions(m_files, MapOptions::Diff); return; } if (platform.GetFileSizeByFullPath(GetPath(MapOptions::Map), m_mapSize)) - m_files = SetOptions(m_files, MapOptions::Map); + SetOptions(m_files, MapOptions::Map); if (version::IsSingleMwm(GetVersion())) { if (m_files == MapOptions::Map) - m_files = SetOptions(m_files, MapOptions::CarRouting); + SetOptions(m_files, MapOptions::CarRouting); return; } string const routingPath = GetPath(MapOptions::CarRouting); if (platform.GetFileSizeByFullPath(routingPath, m_routingSize)) - m_files = SetOptions(m_files, MapOptions::CarRouting); + SetOptions(m_files, MapOptions::CarRouting); } void LocalCountryFile::DeleteFromDisk(MapOptions files) const @@ -140,7 +147,7 @@ string DebugPrint(LocalCountryFile const & file) { ostringstream os; os << "LocalCountryFile [" << file.m_directory << ", " << DebugPrint(file.m_countryFile) << ", " - << file.m_version << ", " << DebugPrint(file.m_files) << "]"; + << file.m_version << ", " << (file.m_files ? DebugPrint(file.m_files.get()) : "No files") << "]"; return os.str(); } } // namespace platform diff --git a/platform/local_country_file.hpp b/platform/local_country_file.hpp index e290efd484..702deb6c1e 100644 --- a/platform/local_country_file.hpp +++ b/platform/local_country_file.hpp @@ -7,6 +7,8 @@ #include #include +#include + namespace platform { // This class represents a path to disk files corresponding to some @@ -55,15 +57,18 @@ public: // SyncWithDisk() is called. uint64_t GetSize(MapOptions filesMask) const; - // Returns a mask of all known country files. Return value may be - // empty until SyncWithDisk() is called. - MapOptions GetFiles() const { return m_files; } + // Returns true when some files are found during SyncWithDisk. + // Return value may be empty until SyncWithDisk() is called. + bool HasFiles() const { return m_files.is_initialized(); } // Checks whether files specified in filesMask are on disk. Return // value will be false until SyncWithDisk() is called. bool OnDisk(MapOptions filesMask) const { - return (static_cast(m_files) & static_cast(filesMask)) == + if (!m_files) + return false; + + return (static_cast(m_files.get()) & static_cast(filesMask)) == static_cast(filesMask); } @@ -100,7 +105,7 @@ private: CountryFile m_countryFile; int64_t m_version; - MapOptions m_files; + boost::optional m_files; /// Size of file which contains map section in bytes. It's mwm file in any case. uint64_t m_mapSize; diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index c62c3dce26..103e289e94 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -77,12 +77,10 @@ UNIT_TEST(LocalCountryFile_Smoke) localFile.GetPath(MapOptions::CarRouting), ()); // Not synced with disk yet. - TEST_EQUAL(MapOptions::Nothing, localFile.GetFiles(), ()); - - // Any statement is true about elements of an empty set. - TEST(localFile.OnDisk(MapOptions::Nothing), ()); + TEST(!localFile.HasFiles(), ()); TEST(!localFile.OnDisk(MapOptions::Map), ()); + TEST(!localFile.OnDisk(MapOptions::Diff), ()); TEST(!localFile.OnDisk(MapOptions::CarRouting), ()); TEST(!localFile.OnDisk(MapOptions::MapWithCarRouting), ()); @@ -186,7 +184,7 @@ UNIT_TEST(LocalCountryFile_CleanupMapFiles) brazilMapFile.Reset(); irelandLocalFile.SyncWithDisk(); - TEST_EQUAL(MapOptions::Map, irelandLocalFile.GetFiles(), ()); + TEST(irelandLocalFile.OnDisk(MapOptions::Map), ()); irelandLocalFile.DeleteFromDisk(MapOptions::Map); TEST(!irelandMapFile.Exists(), (irelandMapFile)); irelandMapFile.Reset(); diff --git a/routing/routing_integration_tests/get_altitude_test.cpp b/routing/routing_integration_tests/get_altitude_test.cpp index 021d9c8dfe..6753c33b1d 100644 --- a/routing/routing_integration_tests/get_altitude_test.cpp +++ b/routing/routing_integration_tests/get_altitude_test.cpp @@ -49,7 +49,7 @@ void TestAltitudeOfAllMwmFeatures(string const & countryId, TAltitude const alti LocalCountryFile const country = GetLocalCountryFileByCountryId(CountryFile(countryId)); TEST_NOT_EQUAL(country, LocalCountryFile(), ()); - TEST_NOT_EQUAL(country.GetFiles(), MapOptions::Nothing, (country)); + TEST(country.HasFiles(), (country)); pair const regResult = dataSource.RegisterMap(country); TEST_EQUAL(regResult.second, MwmSet::RegResult::Success, ()); diff --git a/storage/storage.cpp b/storage/storage.cpp index abb2599605..80d15afe36 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -1220,7 +1220,7 @@ void Storage::DeleteCountryFiles(CountryId const & countryId, MapOptions opt, bo { DeleteFromDiskWithIndexes(*localFile, opt); localFile->SyncWithDisk(); - if (localFile->GetFiles() == MapOptions::Nothing) + if (!localFile->HasFiles()) localFile.reset(); } auto isNull = [](LocalFilePtr const & localFile) { return !localFile; }; diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 09e2ff3326..b00c83f639 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -49,6 +49,8 @@ #include #include +#include + #include "defines.hpp" using namespace platform::tests_support; @@ -543,7 +545,7 @@ LocalFilePtr CreateDummyMapFile(CountryFile const & countryFile, int64_t version writer.Write(zeroes.data(), zeroes.size()); } localFile->SyncWithDisk(); - TEST_EQUAL(MapOptions::Map, localFile->GetFiles(), ()); + TEST(localFile->OnDisk(MapOptions::Map), ()); TEST_EQUAL(size, localFile->GetSize(MapOptions::Map), ()); return localFile; } @@ -697,10 +699,10 @@ UNIT_TEST(StorageTest_DeleteTwoVersionsOfTheSameCountry) storage.DeleteCountry(countryId, MapOptions::Map); localFileV1->SyncWithDisk(); - TEST_EQUAL(MapOptions::Nothing, localFileV1->GetFiles(), ()); + TEST(!localFileV1->HasFiles(), ()); localFileV2->SyncWithDisk(); - TEST_EQUAL(MapOptions::Nothing, localFileV2->GetFiles(), ()); + TEST(!localFileV1->HasFiles(), ()); TEST_EQUAL(Status::ENotDownloaded, storage.CountryStatusEx(countryId), ()); } @@ -742,11 +744,11 @@ UNIT_TEST(StorageTest_DownloadMapAndRoutingSeparately) LocalFilePtr localFileA = storage.GetLatestLocalFile(countryId); TEST(localFileA.get(), ()); - TEST_EQUAL(MapOptions::Map, localFileA->GetFiles(), ()); + TEST(localFileA->OnDisk(MapOptions::Map), ()); MwmSet::MwmId id = mwmSet.GetMwmIdByCountryFile(countryFile); TEST(id.IsAlive(), ()); - TEST_EQUAL(MapOptions::Map, id.GetInfo()->GetLocalFile().GetFiles(), ()); + TEST(id.GetInfo()->GetLocalFile().OnDisk(MapOptions::Map), ()); // Download routing file in addition to exising map file. { @@ -759,10 +761,10 @@ UNIT_TEST(StorageTest_DownloadMapAndRoutingSeparately) LocalFilePtr localFileB = storage.GetLatestLocalFile(countryId); TEST(localFileB.get(), ()); TEST_EQUAL(localFileA.get(), localFileB.get(), (*localFileA, *localFileB)); - TEST_EQUAL(MapOptions::MapWithCarRouting, localFileB->GetFiles(), ()); + TEST(localFileB->OnDisk(MapOptions::MapWithCarRouting), ()); TEST(id.IsAlive(), ()); - TEST_EQUAL(MapOptions::MapWithCarRouting, id.GetInfo()->GetLocalFile().GetFiles(), ()); + TEST(id.GetInfo()->GetLocalFile().OnDisk(MapOptions::MapWithCarRouting), ()); // Delete routing file and check status update. { @@ -772,10 +774,10 @@ UNIT_TEST(StorageTest_DownloadMapAndRoutingSeparately) LocalFilePtr localFileC = storage.GetLatestLocalFile(countryId); TEST(localFileC.get(), ()); TEST_EQUAL(localFileB.get(), localFileC.get(), (*localFileB, *localFileC)); - TEST_EQUAL(MapOptions::Map, localFileC->GetFiles(), ()); + TEST(localFileC->OnDisk(MapOptions::Map), ()); TEST(id.IsAlive(), ()); - TEST_EQUAL(MapOptions::Map, id.GetInfo()->GetLocalFile().GetFiles(), ()); + TEST(id.GetInfo()->GetLocalFile().OnDisk(MapOptions::Map), ()); // Delete map file and check status update. { @@ -786,7 +788,7 @@ UNIT_TEST(StorageTest_DownloadMapAndRoutingSeparately) // Framework should notify MwmSet about deletion of a map file. // As there're no framework, there should not be any changes in MwmInfo. TEST(id.IsAlive(), ()); - TEST_EQUAL(MapOptions::Map, id.GetInfo()->GetLocalFile().GetFiles(), ()); + TEST(id.GetInfo()->GetLocalFile().OnDisk(MapOptions::Map), ()); } UNIT_CLASS_TEST(StorageTest, DeletePendingCountry) @@ -921,7 +923,7 @@ UNIT_CLASS_TEST(StorageTest, DeleteCountry) tests_support::ScopedFile map("Wonderland.mwm", ScopedFile::Mode::Create); LocalCountryFile file = LocalCountryFile::MakeForTesting("Wonderland", version::FOR_TESTING_SINGLE_MWM1); - TEST_EQUAL(MapOptions::MapWithCarRouting, file.GetFiles(), ()); + TEST(file.OnDisk(MapOptions::MapWithCarRouting), ()); CountryIndexes::PreparePlaceOnDisk(file); string const bitsPath = CountryIndexes::GetPath(file, CountryIndexes::Index::Bits); @@ -947,7 +949,7 @@ UNIT_CLASS_TEST(TwoComponentStorageTest, DeleteCountry) tests_support::ScopedFile map("Wonderland.mwm", ScopedFile::Mode::Create); LocalCountryFile file = LocalCountryFile::MakeForTesting("Wonderland", version::FOR_TESTING_TWO_COMPONENT_MWM1); - TEST_EQUAL(MapOptions::Map, file.GetFiles(), ()); + TEST(file.OnDisk(MapOptions::Map), ()); CountryIndexes::PreparePlaceOnDisk(file); string const bitsPath = CountryIndexes::GetPath(file, CountryIndexes::Index::Bits);