diff --git a/generator/generator_tests/check_mwms.cpp b/generator/generator_tests/check_mwms.cpp index 927e308aeb..10fab2cb9e 100644 --- a/generator/generator_tests/check_mwms.cpp +++ b/generator/generator_tests/check_mwms.cpp @@ -16,7 +16,8 @@ UNIT_TEST(CheckMWM_LoadAll) { Platform & platform = GetPlatform(); vector localFiles; - platform::FindAllLocalMapsInDirectory(platform.WritableDir(), 0 /* version */, localFiles); + platform::FindAllLocalMapsInDirectoryAndCleanup(platform.WritableDir(), 0 /* version */, + -1 /* latestVersion */, localFiles); model::FeaturesFetcher m; m.InitClassificator(); diff --git a/map/framework.cpp b/map/framework.cpp index e003c9a7ec..92b052c0fe 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -477,11 +477,6 @@ void Framework::RegisterAllMaps() ("Registering maps while map downloading leads to removing downloading maps from " "ActiveMapsListener::m_items.")); - auto const isCountryName = [this](string const & filename) - { - return m_storage.FindIndexByFile(filename).IsValid(); - }; - platform::CleanupMapsDirectory(m_storage.GetCurrentDataVersion(), isCountryName); m_storage.RegisterAllLocalMaps(); int minFormat = numeric_limits::max(); diff --git a/platform/local_country_file.hpp b/platform/local_country_file.hpp index 7d29731f07..7ead5f1d86 100644 --- a/platform/local_country_file.hpp +++ b/platform/local_country_file.hpp @@ -81,7 +81,8 @@ public: private: friend string DebugPrint(LocalCountryFile const &); friend void UnitTest_LocalCountryFile_DirectoryLookup(); - friend void FindAllLocalMaps(vector & localFiles); + friend void FindAllLocalMapsAndCleanup(int64_t latestVersion, + vector & localFiles); /// @note! If directory is empty, the file is stored in resources. /// In this case, the only valid params are m_countryFile and m_version. diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index 56525a9e0e..9943522f95 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -77,38 +77,44 @@ string GetSpecialFilesSearchScope() #endif // defined(OMIM_OS_ANDROID) } -void DeleteDownloaderFilesForAllCountries(string const & directory) +class StringsRegexpFilter { - static string const regexp = "\\.(downloading|resume|ready)[0-9]?$"; - Platform::FilesList files; - Platform::GetFilesByRegExp(directory, regexp, files); - for (auto const & file : files) - my::DeleteFileX(my::JoinFoldersToPath(directory, file)); +public: + StringsRegexpFilter(string const & regexp) { regexp::Create(regexp, m_regexp); } + + bool Matches(string const & s) const { return regexp::Matches(s, m_regexp); } + +private: + regexp::RegExpT m_regexp; +}; + +bool IsSpecialName(string const & name) { return name == "." || name == ".."; } + +bool IsDownloaderFile(string const & name) +{ + static StringsRegexpFilter filter(".*\\.(downloading|resume|ready)[0-9]?$"); + return filter.Matches(name); } -void DeleteIndexesForAbsentCountries(string const & directory, - int64_t version, - function const & isCountryName) +bool DirectoryHasIndexesOnly(string const & directory) { - vector files; - FindAllLocalMapsInDirectory(directory, version, files); - - unordered_set names; - for (auto const & file : files) - names.insert(file.GetCountryName()); - - Platform::FilesList subdirs; - Platform::GetFilesByType(directory, Platform::FILE_TYPE_DIRECTORY, subdirs); - for (auto const & subdir : subdirs) + Platform::TFilesWithType fwts; + Platform::GetFilesByType(directory, Platform::FILE_TYPE_REGULAR | Platform::FILE_TYPE_DIRECTORY, + fwts); + for (auto const & fwt : fwts) { - if (subdir == "." || subdir == "..") + auto const & name = fwt.first; + auto const & type = fwt.second; + if (type == Platform::FILE_TYPE_DIRECTORY) + { + if (!IsSpecialName(name)) + return false; continue; - if (!isCountryName(subdir) || names.count(subdir) != 0) - continue; - - LocalCountryFile absentCountry(directory, CountryFile(subdir), version); - CountryIndexes::DeleteFromDisk(absentCountry); + } + if (!CountryIndexes::IsIndexFile(name)) + return false; } + return true; } } // namespace @@ -124,94 +130,93 @@ void DeleteDownloaderFilesForCountry(CountryFile const & countryFile, int64_t ve } } -void CleanupMapsDirectory(int64_t latestVersion, - function const & isCountryName) -{ - Platform & platform = GetPlatform(); - - string const mapsDir = platform.WritableDir(); - - { - // Delete Brazil.mwm and Japan.mwm maps, because they was replaces with - // smaler regions after osrm routing implementation. - vector localFiles; - FindAllLocalMapsInDirectory(mapsDir, 0 /* version */, localFiles); - for (LocalCountryFile & localFile : localFiles) - { - string const & countryName = localFile.GetCountryFile().GetNameWithoutExt(); - if (countryName == "Japan" || countryName == "Brazil") - { - localFile.SyncWithDisk(); - localFile.DeleteFromDisk(MapOptions::MapWithCarRouting); - } - } - } - - // Try to delete empty folders. - Platform::FilesList subdirs; - platform.GetFilesByType(mapsDir, Platform::FILE_TYPE_DIRECTORY, subdirs); - for (string const & subdir : subdirs) - { - // No need to visit parent directory. - if (subdir == "..") - continue; - - int64_t version = 0; - if (subdir != "." && !ParseVersion(subdir, version)) - continue; - - string const subdirPath = my::JoinFoldersToPath(mapsDir, subdir); - - // It's OK to remove all temprorary files for maps older than app. - if (version != latestVersion) - DeleteDownloaderFilesForAllCountries(subdirPath); - - // It's OK to remove indexes for absent countries. - DeleteIndexesForAbsentCountries(subdirPath, version, isCountryName); - - // Remove subdirectory if it does not contain any files except "." and "..". - if (subdir != "." && Platform::IsDirectoryEmpty(subdirPath)) - { - Platform::EError const ret = Platform::RmDir(subdirPath); - ASSERT_EQUAL(Platform::ERR_OK, ret, - ("Can't remove empty directory:", subdirPath, "error:", ret)); - UNUSED_VALUE(ret); - } - } -} - -void FindAllLocalMapsInDirectory(string const & directory, int64_t version, - vector & localFiles) +void FindAllLocalMapsInDirectoryAndCleanup(string const & directory, int64_t version, + int64_t latestVersion, + vector & localFiles) { vector files; Platform & platform = GetPlatform(); - platform.GetFilesByRegExp(directory, ".*\\" DATA_FILE_EXTENSION "$", files); - for (string const & file : files) + Platform::TFilesWithType fwts; + platform.GetFilesByType(directory, Platform::FILE_TYPE_REGULAR | Platform::FILE_TYPE_DIRECTORY, + fwts); + + unordered_set names; + for (auto const & fwt : fwts) { + if (fwt.second != Platform::FILE_TYPE_REGULAR) + continue; + + string name = fwt.first; + + // Remove downloader files for old version directories. + if (IsDownloaderFile(name) && version < latestVersion) + { + my::DeleteFileX(my::JoinFoldersToPath(directory, name)); + continue; + } + + if (!strings::EndsWith(name, DATA_FILE_EXTENSION)) + continue; + // Remove DATA_FILE_EXTENSION and use base name as a country file name. - string name = file; my::GetNameWithoutExt(name); - localFiles.emplace_back(directory, CountryFile(name), version); + names.insert(name); + LocalCountryFile localFile(directory, CountryFile(name), version); + + // Delete Brazil.mwm and Japan.mwm maps, because they were + // replaced with smaller regions after osrm routing + // implementation. + if (name == "Japan" || name == "Brazil") + { + localFile.SyncWithDisk(); + localFile.DeleteFromDisk(MapOptions::MapWithCarRouting); + continue; + } + + localFiles.push_back(localFile); + } + + for (auto const & fwt : fwts) + { + if (fwt.second != Platform::FILE_TYPE_DIRECTORY) + continue; + + string name = fwt.first; + if (IsSpecialName(name)) + continue; + + if (names.count(name) == 0 && DirectoryHasIndexesOnly(my::JoinFoldersToPath(directory, name))) + { + // Directory which looks like a directory with indexes for absent country. It's OK to remove + // it. + LocalCountryFile absentCountry(directory, CountryFile(name), version); + CountryIndexes::DeleteFromDisk(absentCountry); + } } } -void FindAllLocalMaps(vector & localFiles) +void FindAllLocalMapsAndCleanup(int64_t latestVersion, vector & localFiles) { - localFiles.clear(); - Platform & platform = GetPlatform(); - string const directory = platform.WritableDir(); - FindAllLocalMapsInDirectory(directory, 0 /* version */, localFiles); + string const dir = platform.WritableDir(); + FindAllLocalMapsInDirectoryAndCleanup(dir, 0 /* version */, latestVersion, localFiles); - Platform::FilesList subdirs; - Platform::GetFilesByType(directory, Platform::FILE_TYPE_DIRECTORY, subdirs); - for (string const & subdir : subdirs) + Platform::TFilesWithType fwts; + Platform::GetFilesByType(dir, Platform::FILE_TYPE_DIRECTORY, fwts); + for (auto const & fwt : fwts) { + string const & subdir = fwt.first; int64_t version; - if (ParseVersion(subdir, version)) - FindAllLocalMapsInDirectory(my::JoinFoldersToPath(directory, subdir), version, localFiles); + if (!ParseVersion(subdir, version)) + continue; + + string const fullPath = my::JoinFoldersToPath(dir, subdir); + FindAllLocalMapsInDirectoryAndCleanup(fullPath, version, latestVersion, localFiles); + Platform::EError err = Platform::RmDir(fullPath); + if (err != Platform::ERR_OK && err != Platform::ERR_DIRECTORY_NOT_EMPTY) + LOG(LWARNING, ("Can't remove directory:", fullPath, err)); } // World and WorldCoasts can be stored in app bundle or in resources @@ -227,10 +232,12 @@ void FindAllLocalMaps(vector & localFiles) try { - ModelReaderPtr reader(platform.GetReader(file + DATA_FILE_EXTENSION, GetSpecialFilesSearchScope())); + ModelReaderPtr reader( + platform.GetReader(file + DATA_FILE_EXTENSION, GetSpecialFilesSearchScope())); // Assume that empty path means the resource file. - LocalCountryFile worldFile(string(), CountryFile(file), version::ReadVersionTimestamp(reader)); + LocalCountryFile worldFile(string(), CountryFile(file), + version::ReadVersionTimestamp(reader)); worldFile.m_files = MapOptions::Map; if (i != localFiles.end()) { @@ -251,6 +258,12 @@ void FindAllLocalMaps(vector & localFiles) } } +void CleanupMapsDirectory(int64_t latestVersion) +{ + vector localFiles; + FindAllLocalMapsAndCleanup(latestVersion, localFiles); +} + bool ParseVersion(string const & s, int64_t & version) { if (s.empty() || s.size() > kMaxTimestampLength) @@ -294,7 +307,10 @@ ModelReader * GetCountryReader(platform::LocalCountryFile const & file, MapOptio Platform & platform = GetPlatform(); // See LocalCountryFile comment for explanation. if (file.GetDirectory().empty()) - return platform.GetReader(file.GetCountryName() + DATA_FILE_EXTENSION, GetSpecialFilesSearchScope()); + { + return platform.GetReader(file.GetCountryName() + DATA_FILE_EXTENSION, + GetSpecialFilesSearchScope()); + } return platform.GetReader(file.GetPath(options), "f"); } @@ -358,6 +374,13 @@ void CountryIndexes::GetIndexesExts(vector & exts) exts.push_back(kOffsetsExt); } +// static +bool CountryIndexes::IsIndexFile(string const & file) +{ + return strings::EndsWith(file, kBitsExt) || strings::EndsWith(file, kNodesExt) || + strings::EndsWith(file, kOffsetsExt); +} + // static string CountryIndexes::IndexesDir(LocalCountryFile const & localFile) { diff --git a/platform/local_country_file_utils.hpp b/platform/local_country_file_utils.hpp index e6795ea063..f241cd3210 100644 --- a/platform/local_country_file_utils.hpp +++ b/platform/local_country_file_utils.hpp @@ -15,23 +15,19 @@ namespace platform // Removes all files downloader creates during downloading of a country. void DeleteDownloaderFilesForCountry(CountryFile const & countryFile, int64_t version); -// Removes partially downloaded maps, empty directories and old -// (format v1) maps. Also, removes old (splitted) Japan and Brazil -// maps. |version| must be set to the latest data version this app can -// work with. |isCountryName| predicate is used to detect and remove -// indexes for absent countries. It must return true for file names -// corresponding to a country. -void CleanupMapsDirectory(int64_t latestVersion, +void CleanupMapsDirectory(int64_t latestVersion, vector const & localFiles, function const & isCountryName); -// Finds all local map files in a directory. Version of these files is -// passed as an argument. -void FindAllLocalMapsInDirectory(string const & directory, int64_t version, - vector & localFiles); +// Finds all local map files in |directory|. Version of these files is +// passed as an argument. Also, performs cleanup described in comment +// for FindAllLocalMapsAndCleanup(). +void FindAllLocalMapsInDirectoryAndCleanup(string const & directory, int64_t version, + int64_t latestVersion, + vector & localFiles); -// Finds all local map files in resources and writable -// directory. Also, for Android, checks /Android/obb directory. -// Directories should have the following structure: +// Finds all local map files in resources and writable directory. For +// Android, checks /Android/obb directory. Subdirectories in the +// writable directory should have the following structure: // // dir/*.mwm -- map files, base name should correspond to countries.txt, // -- version is assumed to be zero (unknown). @@ -42,10 +38,17 @@ void FindAllLocalMapsInDirectory(string const & directory, int64_t version, // dir/[0-9]{1,18}/*.mwm.routing -- routing file for corresponding map files, // -- version is assumed to be the name of a directory. // -// We can't derive version of a map file from its header because -// currently header stores format version + individual mwm's file -// generation timestamp, not timestamp mentioned in countries.txt. -void FindAllLocalMaps(vector & localFiles); +// Also, this method performs cleanup described in a comment for +// CleanupMapsDirectory(). +void FindAllLocalMapsAndCleanup(int64_t latestVersion, vector & localFiles); + +// This method removes: +// * partially downloaded non-latest maps (with version less than |latestVersion|) +// * empty directories +// * old (format v1) maps +// * old (splitted) Japan and Brazil maps +// * indexes for absent countries +void CleanupMapsDirectory(int64_t latestVersion); // Tries to parse a version from a string of size not longer than 18 // symbols and representing an unsigned decimal number. Leading zeroes @@ -88,6 +91,9 @@ public: // Pushes to the exts's end possible index files extensions. static void GetIndexesExts(vector & exts); + // Returns true if |file| corresponds to an index file. + static bool IsIndexFile(string const & file); + private: friend void UnitTest_LocalCountryFile_CountryIndexes(); friend void UnitTest_LocalCountryFile_DoNotDeleteUserFiles(); diff --git a/platform/platform.cpp b/platform/platform.cpp index c34b28a1b9..fc9cc430d7 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -120,7 +120,8 @@ void Platform::GetFilesByExt(string const & directory, string const & ext, Files } // static -void Platform::GetFilesByType(string const & directory, unsigned typeMask, FilesList & outFiles) +void Platform::GetFilesByType(string const & directory, unsigned typeMask, + TFilesWithType & outFiles) { FilesList allFiles; GetFilesByRegExp(directory, ".*", allFiles); @@ -130,7 +131,7 @@ void Platform::GetFilesByType(string const & directory, unsigned typeMask, Files if (GetFileType(my::JoinFoldersToPath(directory, file), type) != ERR_OK) continue; if (typeMask & type) - outFiles.push_back(file); + outFiles.emplace_back(file, type); } } diff --git a/platform/platform.hpp b/platform/platform.hpp index 312196ac3b..d7e071dc38 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -49,6 +49,8 @@ public: CONNECTION_WWAN }; + using TFilesWithType = vector>; + protected: /// Usually read-only directory for application resources string m_resourcesDir; @@ -138,7 +140,8 @@ public: static void GetFilesByRegExp(string const & directory, string const & regexp, FilesList & outFiles); //@} - static void GetFilesByType(string const & directory, unsigned typeMask, FilesList & outFiles); + static void GetFilesByType(string const & directory, unsigned typeMask, + TFilesWithType & outFiles); static bool IsDirectoryEmpty(string const & directory); diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index b25ab73f20..87527a990f 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -157,25 +157,16 @@ UNIT_TEST(LocalCountryFile_CleanupMapFiles) LocalCountryFile irelandLocalFile(dir4.GetFullPath(), irelandFile, 4 /* version */); ScopedFile irelandMapFile(dir4, irelandFile, MapOptions::Map, "Ireland"); - // Check that FindAllLocalMaps() + // Check FindAllLocalMaps() vector localFiles; - FindAllLocalMaps(localFiles); - TEST(Contains(localFiles, japanLocalFile), (japanLocalFile, localFiles)); - TEST(Contains(localFiles, brazilLocalFile), (brazilLocalFile, localFiles)); + FindAllLocalMapsAndCleanup(-1 /* latestVersion */, localFiles); + TEST(!Contains(localFiles, japanLocalFile), (japanLocalFile, localFiles)); + TEST(!Contains(localFiles, brazilLocalFile), (brazilLocalFile, localFiles)); TEST(Contains(localFiles, irelandLocalFile), (irelandLocalFile, localFiles)); - CleanupMapsDirectory(0 /* latestVersion */, [](string const filename) - { - return filename == "Ireland" || filename == "Absent"; - }); - - japanLocalFile.SyncWithDisk(); - TEST_EQUAL(MapOptions::Nothing, japanLocalFile.GetFiles(), ()); TEST(!japanMapFile.Exists(), (japanMapFile)); japanMapFile.Reset(); - brazilLocalFile.SyncWithDisk(); - TEST_EQUAL(MapOptions::Nothing, brazilLocalFile.GetFiles(), ()); TEST(!brazilMapFile.Exists(), (brazilMapFile)); brazilMapFile.Reset(); @@ -215,11 +206,7 @@ UNIT_TEST(LocalCountryFile_CleanupPartiallyDownloadedFiles) {my::JoinFoldersToPath(latestDir.GetRelativePath(), "Russia_Southern.mwm.downloading"), "Southern Russia map"}}; - auto const isCountryName = [&](string const & /* filename */) - { - return false; - }; - CleanupMapsDirectory(101010 /* latestVersion */, isCountryName); + CleanupMapsDirectory(101010 /* latestVersion */); for (ScopedFile & file : toBeDeleted) { @@ -254,7 +241,8 @@ UNIT_TEST(LocalCountryFile_DirectoryLookup) "Netherlands-routing"); vector localFiles; - FindAllLocalMapsInDirectory(testDir.GetFullPath(), 150309, localFiles); + FindAllLocalMapsInDirectoryAndCleanup(testDir.GetFullPath(), 150309 /* version */, + -1 /* latestVersion */, localFiles); sort(localFiles.begin(), localFiles.end()); for (LocalCountryFile & localFile : localFiles) localFile.SyncWithDisk(); @@ -278,11 +266,11 @@ UNIT_TEST(LocalCountryFile_AllLocalFilesLookup) { CountryFile const italyFile("Italy"); - ScopedDir testDir("010101"); + ScopedDir testDir("10101"); ScopedFile testItalyMapFile(testDir, italyFile, MapOptions::Map, "Italy-map"); vector localFiles; - FindAllLocalMaps(localFiles); + FindAllLocalMapsAndCleanup(-1 /* latestVersion */, localFiles); multiset localFilesSet(localFiles.begin(), localFiles.end()); bool worldFound = false; diff --git a/platform/platform_tests/platform_test.cpp b/platform/platform_tests/platform_test.cpp index 851b48cf9c..14789475e6 100644 --- a/platform/platform_tests/platform_test.cpp +++ b/platform/platform_tests/platform_test.cpp @@ -22,9 +22,13 @@ char const * TEST_FILE_NAME = "some_temporary_unit_test_file.tmp"; void CheckFilesPresence(string const & baseDir, unsigned typeMask, initializer_list> const & files) { - Platform::FilesList filesList; - Platform::GetFilesByType(baseDir, typeMask, filesList); - multiset filesSet(filesList.begin(), filesList.end()); + Platform::TFilesWithType fwts; + Platform::GetFilesByType(baseDir, typeMask, fwts); + + multiset filesSet; + for (auto const & fwt : fwts) + filesSet.insert(fwt.first); + for (auto const & file : files) TEST_EQUAL(filesSet.count(file.first), file.second, (file.first, file.second)); } diff --git a/routing/routing_integration_tests/cross_section_tests.cpp b/routing/routing_integration_tests/cross_section_tests.cpp index 6a5b89dda0..50f17023ba 100644 --- a/routing/routing_integration_tests/cross_section_tests.cpp +++ b/routing/routing_integration_tests/cross_section_tests.cpp @@ -18,7 +18,7 @@ UNIT_TEST(CheckCrossSections) { static double constexpr kPointEquality = 0.01; vector localFiles; - platform::FindAllLocalMaps(localFiles); + platform::FindAllLocalMapsAndCleanup(-1 /* latestVersion */, localFiles); size_t ingoingErrors = 0; size_t outgoingErrors = 0; diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index d0a6d6f3cb..e90a9ada13 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -159,7 +159,7 @@ namespace integration pl.SetResourceDir(options.m_resourcePath); vector localFiles; - platform::FindAllLocalMaps(localFiles); + platform::FindAllLocalMapsAndCleanup(-1, localFiles); for (auto & file : localFiles) file.SyncWithDisk(); ASSERT(!localFiles.empty(), ()); diff --git a/storage/storage.cpp b/storage/storage.cpp index 5cadadce8a..f7d9794f49 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -111,7 +111,7 @@ void Storage::RegisterAllLocalMaps() m_localFilesForFakeCountries.clear(); vector localFiles; - FindAllLocalMaps(localFiles); + FindAllLocalMapsAndCleanup(GetCurrentDataVersion(), localFiles); auto compareByCountryAndVersion = [](LocalCountryFile const & lhs, LocalCountryFile const & rhs) { diff --git a/storage/storage.hpp b/storage/storage.hpp index 8016d375df..95a3baac62 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -6,6 +6,8 @@ #include "storage/queued_country.hpp" #include "storage/storage_defines.hpp" +#include "platform/local_country_file.hpp" + #include "std/function.hpp" #include "std/list.hpp" #include "std/set.hpp" @@ -14,7 +16,6 @@ #include "std/unique_ptr.hpp" #include "std/vector.hpp" - namespace storage { /// Can be used to store local maps and/or maps available for download @@ -48,7 +49,9 @@ private: using TLocalFilePtr = shared_ptr; map> m_localFiles; - // Our World.mwm and WorldCoasts.mwm are fake countries, together with any custom mwm in data folder. + + // Our World.mwm and WorldCoasts.mwm are fake countries, together with any custom mwm in data + // folder. map m_localFilesForFakeCountries; /// used to correctly calculate total country download progress with more than 1 file