From bab07f07102fde4695411d2f03937220204deecf Mon Sep 17 00:00:00 2001 From: vng Date: Thu, 11 Oct 2012 19:28:04 +0300 Subject: [PATCH] Set bookmarks file name equal with category name. Fix some unit tests for this case. --- map/bookmark.cpp | 40 ++++++++++++++--------- map/map_tests/bookmarks_test.cpp | 56 +++++++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 24 deletions(-) diff --git a/map/bookmark.cpp b/map/bookmark.cpp index 6b269a8b91..b88233c4ce 100644 --- a/map/bookmark.cpp +++ b/map/bookmark.cpp @@ -4,6 +4,7 @@ #include "../coding/file_reader.hpp" #include "../coding/parse_xml.hpp" // LoadFromKML +#include "../coding/internal/file_data.hpp" #include "../platform/platform.hpp" @@ -349,48 +350,57 @@ void BookmarkCategory::SaveToKML(ostream & s) s << kmlFooter; } -static bool IsValidCharForPath(strings::UniChar c) +static bool IsBadCharForPath(strings::UniChar const & c) { static strings::UniChar const illegalChars[] = {':', '/', '\\', '<', '>', '\"', '|', '?', '*'}; + for (size_t i = 0; i < ARRAY_SIZE(illegalChars); ++i) if (c < ' ' || illegalChars[i] == c) - return false; - return true; + return true; + + return false; } string BookmarkCategory::GenerateUniqueFileName(const string & path, string const & name) { // Remove not allowed symbols strings::UniString uniName = strings::MakeUniString(name); - size_t const charCount = uniName.size(); - strings::UniString uniFixedName; - for (size_t i = 0; i < charCount; ++i) - if (IsValidCharForPath(uniName[i])) - uniFixedName.push_back(uniName[i]); + strings::UniString::iterator iEnd = remove_if(uniName.begin(), uniName.end(), &IsBadCharForPath); + if (iEnd != uniName.end()) + { + // buffer_vector doesn't have erase function - call resize here (it's even better in this case). + uniName.resize(distance(uniName.begin(), iEnd)); + } - string const fixedName = uniFixedName.empty() ? "Bookmarks" : strings::ToUtf8(uniFixedName); + string const utfName = uniName.empty() ? "Bookmarks" : strings::ToUtf8(uniName); size_t counter = 1; string suffix; - while (Platform::IsFileExistsByFullPath(path + fixedName + suffix)) + while (Platform::IsFileExistsByFullPath(path + utfName + suffix)) suffix = strings::to_string(counter++); - return path + fixedName + suffix + ".kml"; + return (path + utfName + suffix + ".kml"); } bool BookmarkCategory::SaveToKMLFile() { - if (m_file.empty()) - m_file = GenerateUniqueFileName(GetPlatform().WritableDir(), m_name); + // always assign new file name according to category name + string fName = GenerateUniqueFileName(GetPlatform().WritableDir(), m_name); + m_file.swap(fName); try { - // @TODO On Windows UTF-8 file names are not supported. + /// @todo On Windows UTF-8 file names are not supported. ofstream fileToSave(m_file.c_str()); SaveToKML(fileToSave); + + // delete old file + if (!fName.empty() && fName != m_file) + VERIFY ( my::DeleteFileX(fName), (fName, m_file)); + + return true; } catch (std::exception const & e) { LOG(LWARNING, ("Can't save bookmarks category", m_name, "to file", m_file)); return false; } - return true; } diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index 90c3864dae..bd9e77b6b6 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -2,6 +2,10 @@ #include "../../map/framework.hpp" +#include "../../platform/platform.hpp" + +#include "../../coding/internal/file_data.hpp" + #include "../../std/fstream.hpp" @@ -125,9 +129,9 @@ UNIT_TEST(Bookmarks_ImportKML) UNIT_TEST(Bookmarks_ExportKML) { char const * BOOKMARKS_FILE_NAME = "UnitTestBookmarks.kml"; + BookmarkCategory cat("Default"); cat.LoadFromKML(new MemReader(kmlString, strlen(kmlString))); - CheckBookmarks(cat); TEST_EQUAL(cat.IsVisible(), false, ()); @@ -141,24 +145,25 @@ UNIT_TEST(Bookmarks_ExportKML) } cat.ClearBookmarks(); - TEST_EQUAL(cat.GetBookmarksCount(), 0, ()); cat.LoadFromKML(new FileReader(BOOKMARKS_FILE_NAME)); - CheckBookmarks(cat); TEST_EQUAL(cat.IsVisible(), true, ()); - BookmarkCategory * cat2 = BookmarkCategory::CreateFromKMLFile(BOOKMARKS_FILE_NAME); + scoped_ptr cat2(BookmarkCategory::CreateFromKMLFile(BOOKMARKS_FILE_NAME)); CheckBookmarks(*cat2); - FileWriter::DeleteFileX(BOOKMARKS_FILE_NAME); cat2->SaveToKMLFile(); - delete cat2; + // old file should be deleted if we save bookmarks with new category name + uint64_t dummy; + TEST(!my::GetFileSize(BOOKMARKS_FILE_NAME, dummy), ()); - cat2 = BookmarkCategory::CreateFromKMLFile(BOOKMARKS_FILE_NAME); + // MapName is the tag in test kml data. + string const catFileName = GetPlatform().WritableDir() + "MapName.kml"; + cat2.reset(BookmarkCategory::CreateFromKMLFile(catFileName)); CheckBookmarks(*cat2); - FileWriter::DeleteFileX(BOOKMARKS_FILE_NAME); + TEST(my::DeleteFileX(catFileName), ()); } UNIT_TEST(Bookmarks_Getting) @@ -230,6 +235,30 @@ UNIT_TEST(Bookmarks_AddressInfo) TEST(info.m_types[0] == "cafe" || info.m_types[0] == "кафе", (info.m_types[0])); } +UNIT_TEST(Bookmarks_IllegalFileName) +{ + char const * arrIllegal[] = { "?", "?|", "\"x", "|x:", "x<>y", "xy*"}; + char const * arrLegal[] = { "", "", "x", "x", "xy", "xy"}; + + for (size_t i = 0; i < ARRAY_SIZE(arrIllegal); ++i) + { + string name = BookmarkCategory::GenerateUniqueFileName("", arrIllegal[i]); + + // remove extension + TEST_GREATER(name.size(), 4, ()); + name.erase(name.end() - 4, name.end()); + + if (strlen(arrLegal[i]) == 0) + { + TEST_EQUAL("Bookmarks", name, ()); + } + else + { + TEST_EQUAL(arrLegal[i], name, ()); + } + } +} + UNIT_TEST(Bookmarks_UniqueFileName) { string const FILENAME = "SomeUniqueFileName"; @@ -300,5 +329,14 @@ UNIT_TEST(Bookmarks_AddingMoving) pBm = fm.GetBmCategory(res.first)->GetBookmark(res.second); TEST_EQUAL(pBm->GetName(), "name3", ()); TEST_EQUAL(pBm->GetType(), "placemark-green", ()); - +} + +// This test should always be the last to delete temporary files from previous tests. +UNIT_TEST(Bookmarks_Finalizing) +{ + string const path = GetPlatform().WritableDir(); + char const * arrFiles[] = { "cat1", "cat2", "cat3" }; + + for (size_t i = 0; i < ARRAY_SIZE(arrFiles); ++i) + FileWriter::DeleteFileX(path + arrFiles[i] + ".kml"); }