From 66c409971d994c60dcbef704c6d58799613658fb Mon Sep 17 00:00:00 2001 From: Viktor Date: Sat, 4 Jul 2015 12:26:46 +0300 Subject: [PATCH] Review fixes. --- coding/coding_tests/file_utils_test.cpp | 20 +++++++++++------ coding/file_name_utils.cpp | 24 ++++----------------- coding/file_name_utils.hpp | 8 +++---- generator/generator_tool/generator_tool.cpp | 2 +- integration_tests/osrm_test_tools.cpp | 6 ++---- platform/platform.cpp | 18 ++++++++++++---- platform/platform.hpp | 6 +++--- 7 files changed, 41 insertions(+), 43 deletions(-) diff --git a/coding/coding_tests/file_utils_test.cpp b/coding/coding_tests/file_utils_test.cpp index c742fc2284..4f28330246 100644 --- a/coding/coding_tests/file_utils_test.cpp +++ b/coding/coding_tests/file_utils_test.cpp @@ -39,11 +39,12 @@ UNIT_TEST(FileName_Smoke) TEST_EQUAL(name, "test", ()); } -UNIT_TEST(FileName_GetDirectory) -{ // TODO (@gorshenin): implement a Clean() method to clean file path // (remove redundant separators, correctly collapse dots, dot-dots, etc.). -#if !defined(OMIM_OS_WINDOWS) +#ifndef OMIM_OS_WINDOWS + +UNIT_TEST(FileName_GetDirectory) +{ TEST_EQUAL("/tmp", my::GetDirectory("/tmp/evil\\file"), ()); TEST_EQUAL(".", my::GetDirectory("evil\\file"), ()); @@ -57,12 +58,10 @@ UNIT_TEST(FileName_GetDirectory) TEST_EQUAL(".", my::GetDirectory("somefile.txt"), ()); TEST_EQUAL(".", my::GetDirectory("somefile"), ()); -#endif // !defined(OMIM_OS_WINDOWS) } UNIT_TEST(FilePath_Slash) { -#ifndef OMIM_OS_WINDOWS TEST_EQUAL("/", my::AddSlashIfNeeded(""), ()); TEST_EQUAL("/", my::AddSlashIfNeeded("/"), ()); TEST_EQUAL("./", my::AddSlashIfNeeded("."), ()); @@ -72,5 +71,14 @@ UNIT_TEST(FilePath_Slash) TEST_EQUAL("/data/", my::AddSlashIfNeeded("/data/"), ()); TEST_EQUAL("../../data/", my::AddSlashIfNeeded("../../data"), ()); TEST_EQUAL("../../data/", my::AddSlashIfNeeded("../../data/"), ()); -#endif } + +UNIT_TEST(FilePath_Join) +{ + TEST_EQUAL("omim/strings.txt", my::JoinFoldersToPath("omim", "strings.txt"), ()); + TEST_EQUAL("omim/strings.txt", my::JoinFoldersToPath("omim/", "strings.txt"), ()); + TEST_EQUAL("../../omim/strings.txt", my::JoinFoldersToPath({"..", "..", "omim"}, "strings.txt"), ()); + TEST_EQUAL("../../omim/strings.txt", my::JoinFoldersToPath({"../", "..", "omim/"}, "strings.txt"), ()); +} + +#endif // OMIM_OS_WINDOWS diff --git a/coding/file_name_utils.cpp b/coding/file_name_utils.cpp index b2e44146f0..b872365a14 100644 --- a/coding/file_name_utils.cpp +++ b/coding/file_name_utils.cpp @@ -49,30 +49,14 @@ string GetNativeSeparator() string JoinFoldersToPath(const string & folder, const string & file) { - return folder + GetNativeSeparator() + file; + return my::AddSlashIfNeeded(folder) + file; } -string JoinFoldersToPath(const string & folder1, const string & folder2, const string & file) +string JoinFoldersToPath(initializer_list const & folders, const string & file) { - string nativeSeparator = GetNativeSeparator(); - return folder1 + nativeSeparator + folder2 + nativeSeparator + file; -} - -string JoinFoldersToPath(const string & folder1, const string & folder2, const string & folder3, const string & file) -{ - string nativeSeparator = GetNativeSeparator(); - return folder1 + nativeSeparator + folder2 + nativeSeparator + folder3 + nativeSeparator + file; -} - -string JoinFoldersToPath(const vector & folders, const string & file) -{ - if (folders.empty()) - return file; - - string nativeSeparator = GetNativeSeparator(); string result; - for (size_t i = 0; i < folders.size(); ++i) - result = result + folders[i] + nativeSeparator; + for (string const & s : folders) + result += AddSlashIfNeeded(s); result += file; return result; diff --git a/coding/file_name_utils.hpp b/coding/file_name_utils.hpp index f738588acd..76629660c9 100644 --- a/coding/file_name_utils.hpp +++ b/coding/file_name_utils.hpp @@ -1,7 +1,7 @@ #pragma once +#include "std/initializer_list.hpp" #include "std/string.hpp" -#include "std/vector.hpp" namespace my { @@ -23,10 +23,8 @@ namespace my /// Create full path from some folder using native folders separator string JoinFoldersToPath(const string & folder, const string & file); - string JoinFoldersToPath(const string & folder1, const string & folder2, const string & file); - string JoinFoldersToPath(const string & folder1, const string & folder2, const string & folder3, const string & file); - string JoinFoldersToPath(const vector & folders, const string & file); + string JoinFoldersToPath(initializer_list const & folders, const string & file); - /// Add terminating slash, if it's not exist to the folder path string. + /// Add the terminating slash to the folder path string if it's not already there. string AddSlashIfNeeded(string const & path); } diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 3f4f04116d..f6763843dc 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -80,7 +80,7 @@ int main(int argc, char ** argv) Platform & pl = GetPlatform(); if (!FLAGS_user_resource_path.empty()) - pl.SetResourceDir(my::AddSlashIfNeeded(FLAGS_user_resource_path)); + pl.SetResourceDir(FLAGS_user_resource_path); string const path = FLAGS_data_path.empty() ? pl.WritableDir() : my::AddSlashIfNeeded(FLAGS_data_path); diff --git a/integration_tests/osrm_test_tools.cpp b/integration_tests/osrm_test_tools.cpp index 64d15f4d78..0550d1d0e7 100644 --- a/integration_tests/osrm_test_tools.cpp +++ b/integration_tests/osrm_test_tools.cpp @@ -18,8 +18,6 @@ #include "geometry/distance_on_sphere.hpp" -#include "coding/file_name_utils.hpp" - #include @@ -128,9 +126,9 @@ namespace integration Platform & pl = GetPlatform(); CommandLineOptions const & options = GetTestingOptions(); if (options.m_dataPath) - pl.SetWritableDirForTests(my::AddSlashIfNeeded(options.m_dataPath)); + pl.SetWritableDirForTests(options.m_dataPath); if (options.m_resourcePath) - pl.SetResourceDir(my::AddSlashIfNeeded(options.m_resourcePath)); + pl.SetResourceDir(options.m_resourcePath); vector localFiles; platform::FindAllLocalMaps(localFiles); diff --git a/platform/platform.cpp b/platform/platform.cpp index 78dcdbde9a..194ba444ca 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -2,9 +2,9 @@ #include "platform/local_country_file.hpp" -#include "coding/sha2.hpp" #include "coding/base64.hpp" #include "coding/file_name_utils.hpp" +#include "coding/sha2.hpp" #include "coding/writer.hpp" #include "base/logging.hpp" @@ -135,12 +135,12 @@ string Platform::DeviceName() const string Platform::WritablePathForCountryIndexes(string const & mwmName) const { - string dir = WritableDir() + mwmName + my::GetNativeSeparator(); + string const dir = WritableDir() + mwmName + my::GetNativeSeparator(); if (!IsFileExistsByFullPath(dir)) + { if (MkDir(dir) != Platform::ERR_OK) - { MYTHROW(Writer::CreateDirException, ("Can't create directory:", dir)); - } + } return dir; } @@ -149,6 +149,16 @@ string Platform::GetIndexFileName(string const & mwmName, string const & extensi return GetPlatform().WritablePathForCountryIndexes(mwmName) + mwmName + extension; } +void Platform::SetWritableDirForTests(string const & path) +{ + m_writableDir = my::AddSlashIfNeeded(path); +} + +void Platform::SetResourceDir(string const & path) +{ + m_resourcesDir = my::AddSlashIfNeeded(path); +} + ModelReader * Platform::GetCountryReader(platform::LocalCountryFile const & file, TMapOptions options) const { diff --git a/platform/platform.hpp b/platform/platform.hpp index 930f488bba..f224a6b68d 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -88,7 +88,7 @@ public: /// @return always the same writable dir for current user with slash at the end string WritableDir() const { return m_writableDir; } /// Set writable dir — use for testing and linux stuff only - void SetWritableDirForTests(string const & path) { m_writableDir = path; } + void SetWritableDirForTests(string const & path); /// @return full path to file in user's writable directory string WritablePathForFile(string const & file) const { return WritableDir() + file; } /// @return full path to indexes directory for country file. @@ -101,7 +101,7 @@ public: string ResourcesDir() const { return m_resourcesDir; } /// @note! This function is used in generator_tool and unit tests. /// Client app should not replace default resource dir. - void SetResourceDir(string const & path) { m_resourcesDir = path; } + void SetResourceDir(string const & path); /// Creates directory at filesystem EError MkDir(string const & dirName) const; @@ -116,7 +116,7 @@ public: /// @return full path to file in the temporary directory string TmpPathForFile(string const & file) const { return TmpDir() + file; } - /// @return full path to file where stored data for unit tests. + /// @return full path to the file where data for unit tests is stored. string TestsDataPathForFile(string const & file) const { return ReadPathForFile(file); } /// @return path for directory in the persistent memory, can be the same