From 2eb10e2583a1a26d5cebe019bf986d6f88f79211 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Sun, 17 Apr 2022 09:42:25 +0300 Subject: [PATCH] [platform] Fixed bug with temporary string in platform::GetFilePathByUrl. Signed-off-by: Viktor Govako --- platform/downloader_utils.cpp | 8 +++---- platform/local_country_file_utils.cpp | 14 ++++-------- platform/local_country_file_utils.hpp | 22 ++++++++++++------- .../platform_tests/downloader_utils_tests.cpp | 16 +++++++++----- .../local_country_file_tests.cpp | 18 +++++---------- storage/storage_tests/storage_tests.cpp | 3 +-- 6 files changed, 38 insertions(+), 43 deletions(-) diff --git a/platform/downloader_utils.cpp b/platform/downloader_utils.cpp index 1d559ff5f2..972780a119 100644 --- a/platform/downloader_utils.cpp +++ b/platform/downloader_utils.cpp @@ -71,12 +71,12 @@ std::string GetFilePathByUrl(std::string const & url) uint64_t dataVersion = 0; CHECK(strings::to_uint(urlComponents[1], dataVersion), ()); - auto const countryComponents = strings::Tokenize(url::UrlDecode(urlComponents.back()), "."); - CHECK_EQUAL(countryComponents.size(), 2, ()); + std::string mwmFile = url::UrlDecode(urlComponents.back()); + // remove extension + mwmFile = mwmFile.substr(0, mwmFile.find('.')); auto const fileType = urlComponents[0] == kDiffsPath ? MapFileType::Diff : MapFileType::Map; - - return platform::GetFileDownloadPath(dataVersion, platform::CountryFile(std::string(countryComponents[0])), fileType); + return platform::GetFileDownloadPath(dataVersion, mwmFile, fileType); } } // namespace downloader diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index 739d8ba443..32cf860620 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -23,10 +23,10 @@ #include "defines.hpp" -using namespace std; - namespace platform { +using namespace std; + namespace { char const kBitsExt[] = ".bftsegbits"; @@ -321,15 +321,9 @@ shared_ptr PreparePlaceForCountryFiles(int64_t version, string return make_shared(directory, countryFile, version); } -string GetFileDownloadPath(int64_t version, CountryFile const & countryFile, MapFileType type) +string GetFileDownloadPath(int64_t version, string const & dataDir, string const & mwmName, MapFileType type) { - return GetFileDownloadPath(version, string(), countryFile, type); -} - -string GetFileDownloadPath(int64_t version, string const & dataDir, CountryFile const & countryFile, - MapFileType type) -{ - string const readyFile = GetFileName(countryFile.GetName(), type) + READY_FILE_EXTENSION; + string const readyFile = GetFileName(mwmName, type) + READY_FILE_EXTENSION; string const dir = GetDataDirFullPath(dataDir); if (version == 0) return base::JoinPath(dir, readyFile); diff --git a/platform/local_country_file_utils.hpp b/platform/local_country_file_utils.hpp index 9a8a7f6dd8..3ba8b3147a 100644 --- a/platform/local_country_file_utils.hpp +++ b/platform/local_country_file_utils.hpp @@ -69,11 +69,21 @@ std::shared_ptr PreparePlaceForCountryFiles(int64_t version, C std::shared_ptr PreparePlaceForCountryFiles(int64_t version, std::string const & dataDir, CountryFile const & countryFile); -// Note. The function assumes the maps are located in writable dir/|dataDir|/|version| directory. -// If |dataDir| is empty (or is not set) the function assumes that maps are in writable dir. -std::string GetFileDownloadPath(int64_t version, CountryFile const & countryFile, MapFileType type); +/// @note The function assumes the maps are located in writable dir/|dataDir|/|version| directory. +/// If |dataDir| is empty (or is not set) the function assumes that maps are in writable dir. +/// @{ std::string GetFileDownloadPath(int64_t version, std::string const & dataDir, - CountryFile const & countryFile, MapFileType type); + std::string const & mwmName, MapFileType type); +inline std::string GetFileDownloadPath(int64_t version, std::string const & dataDir, + CountryFile const & countryFile, MapFileType type) +{ + return GetFileDownloadPath(version, dataDir, countryFile.GetName(), type); +} +inline std::string GetFileDownloadPath(int64_t version, std::string const & mwmName, MapFileType type) +{ + return GetFileDownloadPath(version, {}, mwmName, type); +} +/// @} std::unique_ptr GetCountryReader(LocalCountryFile const & file, MapFileType type); @@ -107,10 +117,6 @@ public: // Returns true if |file| corresponds to an index file. static bool IsIndexFile(std::string const & file); -private: - friend void UnitTest_LocalCountryFile_CountryIndexes(); - friend void UnitTest_LocalCountryFile_DoNotDeleteUserFiles(); - static std::string IndexesDir(LocalCountryFile const & localFile); }; diff --git a/platform/platform_tests/downloader_utils_tests.cpp b/platform/platform_tests/downloader_utils_tests.cpp index 82e59a8d31..2e930b27b4 100644 --- a/platform/platform_tests/downloader_utils_tests.cpp +++ b/platform/platform_tests/downloader_utils_tests.cpp @@ -3,8 +3,11 @@ #include "platform/downloader_utils.hpp" #include "platform/local_country_file_utils.hpp" #include "platform/mwm_version.hpp" +#include "platform/platform.hpp" -UNIT_TEST(UrlConversionTest) +#include "base/file_name_utils.hpp" + +UNIT_TEST(Downloader_GetFilePathByUrl) { { std::string const mwmName = "Luna"; @@ -13,8 +16,7 @@ UNIT_TEST(UrlConversionTest) int64_t const diffVersion = 0; MapFileType const fileType = MapFileType::Map; - auto const path = - platform::GetFileDownloadPath(dataVersion, platform::CountryFile(mwmName), fileType); + auto const path = platform::GetFileDownloadPath(dataVersion, mwmName, fileType); auto const url = downloader::GetFileDownloadUrl(fileName, dataVersion, diffVersion); auto const resultPath = downloader::GetFilePathByUrl(url); @@ -28,17 +30,19 @@ UNIT_TEST(UrlConversionTest) int64_t const diffVersion = version::FOR_TESTING_MWM1; MapFileType const fileType = MapFileType::Diff; - auto const path = - platform::GetFileDownloadPath(dataVersion, platform::CountryFile(mwmName), fileType); + auto const path = platform::GetFileDownloadPath(dataVersion, mwmName, fileType); auto const url = downloader::GetFileDownloadUrl(fileName, dataVersion, diffVersion); auto const resultPath = downloader::GetFilePathByUrl(url); TEST_EQUAL(path, resultPath, ()); } + + TEST_EQUAL(downloader::GetFilePathByUrl("/maps/220314/Belarus_Brest Region.mwm"), + base::JoinPath(GetPlatform().WritableDir(), "220314/Belarus_Brest Region.mwm.ready"), ()); } -UNIT_TEST(IsUrlSupportedTest) +UNIT_TEST(Downloader_IsUrlSupported) { std::string const mwmName = "Luna"; diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index 44264194d9..58e5ad4c68 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -23,20 +23,12 @@ #include #include +namespace local_country_file_tests +{ +using namespace platform; using namespace platform::tests_support; using namespace std; -namespace platform -{ -namespace -{ -template -bool Contains(vector const & v, T const & t) -{ - return find(v.begin(), v.end(), t) != v.end(); -} -} // namespace - // Checks that all unsigned numbers less than 10 ^ 18 can be parsed as // a timestamp. UNIT_TEST(LocalCountryFile_ParseVersion) @@ -141,7 +133,7 @@ UNIT_TEST(LocalCountryFile_CleanupMapFiles) // Check FindAllLocalMaps() vector localFiles; FindAllLocalMapsAndCleanup(4 /* latestVersion */, localFiles); - TEST(Contains(localFiles, irelandLocalFile), (irelandLocalFile, localFiles)); + TEST(base::IsExist(localFiles, irelandLocalFile), (irelandLocalFile, localFiles)); irelandLocalFile.SyncWithDisk(); TEST(irelandLocalFile.OnDisk(MapFileType::Map), ()); @@ -355,4 +347,4 @@ UNIT_TEST(LocalCountryFile_MakeTemporary) TEST_EQUAL(file.GetPath(MapFileType::Map), path, ()); } -} // namespace platform +} // local_country_file_tests diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 01c7ab1cd6..1bdfa0c68f 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -675,8 +675,7 @@ UNIT_TEST(StorageTest_FailedDownloading) } // File wasn't downloaded, but temprorary downloader files must exist. - string const downloadPath = - GetFileDownloadPath(storage.GetCurrentDataVersion(), countryFile, MapFileType::Map); + string const downloadPath = GetFileDownloadPath(storage.GetCurrentDataVersion(), countryFile.GetName(), MapFileType::Map); TEST(!Platform::IsFileExistsByFullPath(downloadPath), ()); TEST(Platform::IsFileExistsByFullPath(downloadPath + DOWNLOADING_FILE_EXTENSION), ()); TEST(Platform::IsFileExistsByFullPath(downloadPath + RESUME_FILE_EXTENSION), ());