From 25aaa05cefa9ecdd4ce6e7893786b4bf0eec5366 Mon Sep 17 00:00:00 2001 From: vng Date: Fri, 25 Sep 2015 16:22:48 +0300 Subject: [mwm set] Handle file system exceptions when building feature offsets index. --- indexer/features_offsets_table.cpp | 2 +- .../indexer_tests/features_offsets_table_test.cpp | 2 +- indexer/mwm_set.cpp | 14 ++++++- map/map_tests/map_tests.pro | 1 + map/map_tests/mwm_set_test.cpp | 49 ++++++++++++++++++++++ platform/local_country_file_utils.cpp | 11 +++-- platform/local_country_file_utils.hpp | 8 ++-- platform/platform.hpp | 2 +- .../platform_tests/local_country_file_tests.cpp | 6 +-- routing/osrm2feature_map.cpp | 1 + storage/storage_tests/storage_tests.cpp | 2 +- 11 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 map/map_tests/mwm_set_test.cpp diff --git a/indexer/features_offsets_table.cpp b/indexer/features_offsets_table.cpp index 2850a73cde..2b6d9e19fe 100644 --- a/indexer/features_offsets_table.cpp +++ b/indexer/features_offsets_table.cpp @@ -71,7 +71,7 @@ namespace feature { LOG(LINFO, ("Creating features offset table file", storePath)); - VERIFY(CountryIndexes::PreparePlaceOnDisk(localFile), ()); + CountryIndexes::PreparePlaceOnDisk(localFile); Builder builder; FeaturesVector::ForEachOffset(cont.GetReader(DATA_FILE_TAG), [&builder] (uint32_t offset) diff --git a/indexer/indexer_tests/features_offsets_table_test.cpp b/indexer/indexer_tests/features_offsets_table_test.cpp index d08cca7486..f995cd673c 100644 --- a/indexer/indexer_tests/features_offsets_table_test.cpp +++ b/indexer/indexer_tests/features_offsets_table_test.cpp @@ -97,7 +97,7 @@ namespace feature FilesContainerR baseContainer(pl.GetReader("minsk-pass" DATA_FILE_EXTENSION)); LocalCountryFile localFile = LocalCountryFile::MakeForTesting(testFileName); - TEST(CountryIndexes::PreparePlaceOnDisk(localFile), ()); + CountryIndexes::PreparePlaceOnDisk(localFile); string const indexFile = CountryIndexes::GetPath(localFile, CountryIndexes::Index::Offsets); FileWriter::DeleteFileX(indexFile); diff --git a/indexer/mwm_set.cpp b/indexer/mwm_set.cpp index 71d8bcef0e..1079ba19ff 100644 --- a/indexer/mwm_set.cpp +++ b/indexer/mwm_set.cpp @@ -4,6 +4,7 @@ #include "defines.hpp" #include "base/assert.hpp" +#include "base/exception.hpp" #include "base/logging.hpp" #include "base/stl_add.hpp" @@ -214,7 +215,18 @@ unique_ptr MwmSet::LockValueImpl(MwmId const & id) } } - return CreateValue(*info); + try + { + return CreateValue(*info); + } + catch (exception const & ex) + { + LOG(LERROR, ("Can't create MWMValue for", info->GetCountryName(), "Reason", ex.what())); + + --info->m_numRefs; + DeregisterImpl(id); + return nullptr; + } } void MwmSet::UnlockValue(MwmId const & id, unique_ptr && p) diff --git a/map/map_tests/map_tests.pro b/map/map_tests/map_tests.pro index 17df03d4dc..50d789ea1a 100644 --- a/map/map_tests/map_tests.pro +++ b/map/map_tests/map_tests.pro @@ -36,6 +36,7 @@ SOURCES += \ kmz_unarchive_test.cpp \ mwm_url_tests.cpp \ navigator_test.cpp \ + mwm_set_test.cpp \ !linux* { SOURCES += working_time_tests.cpp \ diff --git a/map/map_tests/mwm_set_test.cpp b/map/map_tests/mwm_set_test.cpp new file mode 100644 index 0000000000..3ea80b0536 --- /dev/null +++ b/map/map_tests/mwm_set_test.cpp @@ -0,0 +1,49 @@ +#include "testing/testing.hpp" + +#include "indexer/index.hpp" + +#include "platform/local_country_file_utils.hpp" +#include "platform/platform.hpp" + +#ifndef OMIM_OS_WINDOWS +#include +#endif + + +using namespace platform; +using namespace my; + +#ifndef OMIM_OS_WINDOWS +UNIT_TEST(MwmSet_FileSystemErrors) +{ + string const dir = GetPlatform().WritableDir(); + + CountryFile file("minsk-pass"); + LocalCountryFile localFile(dir, file, 0); + TEST(CountryIndexes::DeleteFromDisk(localFile), ()); + + LogLevel oldLevel = g_LogAbortLevel; + g_LogAbortLevel = LCRITICAL; + + // Remove writable permission. + int const readOnlyMode = S_IRUSR | S_IRGRP | S_IROTH | S_IXUSR | S_IXGRP | S_IXOTH; + TEST_EQUAL(chmod(dir.c_str(), readOnlyMode), 0, ()); + + Index index; + auto p = index.RegisterMap(localFile); + TEST_EQUAL(p.second, Index::RegResult::Success, ()); + + TEST(index.GetMwmIdByCountryFile(file) != Index::MwmId(), ()); + + TEST(!index.GetMwmHandleById(p.first).IsAlive(), ()); + + vector> infos; + index.GetMwmsInfo(infos); + TEST(infos.empty(), ()); + + // Restore writable permission. + TEST_EQUAL(chmod(dir.c_str(), readOnlyMode | S_IWUSR), 0, ()); + + g_LogAbortLevel = oldLevel; +} +#endif diff --git a/platform/local_country_file_utils.cpp b/platform/local_country_file_utils.cpp index 6519274861..19bcc28718 100644 --- a/platform/local_country_file_utils.cpp +++ b/platform/local_country_file_utils.cpp @@ -239,9 +239,11 @@ ModelReader * GetCountryReader(platform::LocalCountryFile const & file, MapOptio } // static -bool CountryIndexes::PreparePlaceOnDisk(LocalCountryFile const & localFile) +void CountryIndexes::PreparePlaceOnDisk(LocalCountryFile const & localFile) { - return MkDirChecked(IndexesDir(localFile)); + string const dir = IndexesDir(localFile); + if (!MkDirChecked(dir)) + MYTHROW(FileSystemException, ("Can't create directory", dir)); } // static @@ -302,6 +304,8 @@ string CountryIndexes::IndexesDir(LocalCountryFile const & localFile) string dir = localFile.GetDirectory(); CountryFile const & file = localFile.GetCountryFile(); + /// @todo It's a temporary code until we will put fIndex->fOffset into mwm container. + /// IndexesDir should not throw any exceptions. if (dir.empty()) { // Local file is stored in resources. Need to prepare index folder in the writable directory. @@ -309,7 +313,8 @@ string CountryIndexes::IndexesDir(LocalCountryFile const & localFile) ASSERT_GREATER(version, 0, ()); dir = my::JoinFoldersToPath(GetPlatform().WritableDir(), strings::to_string(version)); - VERIFY(MkDirChecked(dir), ()); + if (!MkDirChecked(dir)) + MYTHROW(FileSystemException, ("Can't create directory", dir)); } return my::JoinFoldersToPath(dir, file.GetNameWithoutExt()); diff --git a/platform/local_country_file_utils.hpp b/platform/local_country_file_utils.hpp index 660dcd65c3..739a9d35ba 100644 --- a/platform/local_country_file_utils.hpp +++ b/platform/local_country_file_utils.hpp @@ -62,10 +62,10 @@ public: Offsets }; - // Prepares (if necessary) directory for country indexes. Local file - // should point to existing local country files. Returns true on - // success, false otherwise. - static bool PreparePlaceOnDisk(LocalCountryFile const & localFile); + /// Prepares (if necessary) directory for country indexes. Local file + /// should point to existing local country files. + /// @throw FileSystemException if any file system error occured. + static void PreparePlaceOnDisk(LocalCountryFile const & localFile); // Removes country indexes from disk including containing directory. static bool DeleteFromDisk(LocalCountryFile const & localFile); diff --git a/platform/platform.hpp b/platform/platform.hpp index 16d369d27b..be5e8e4db1 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -15,7 +15,7 @@ #include "defines.hpp" DECLARE_EXCEPTION(FileAbsentException, RootException); -DECLARE_EXCEPTION(NotImplementedException, RootException); +DECLARE_EXCEPTION(FileSystemException, RootException); namespace platform { diff --git a/platform/platform_tests/local_country_file_tests.cpp b/platform/platform_tests/local_country_file_tests.cpp index c98dd28009..478bfd35dd 100644 --- a/platform/platform_tests/local_country_file_tests.cpp +++ b/platform/platform_tests/local_country_file_tests.cpp @@ -313,8 +313,7 @@ UNIT_TEST(LocalCountryFile_CountryIndexes) TEST_EQUAL( my::JoinFoldersToPath(germanyLocalFile.GetDirectory(), germanyFile.GetNameWithoutExt()), CountryIndexes::IndexesDir(germanyLocalFile), ()); - TEST(CountryIndexes::PreparePlaceOnDisk(germanyLocalFile), - ("Can't prepare place for:", germanyLocalFile)); + CountryIndexes::PreparePlaceOnDisk(germanyLocalFile); string const bitsPath = CountryIndexes::GetPath(germanyLocalFile, CountryIndexes::Index::Bits); TEST(!Platform::IsFileExistsByFullPath(bitsPath), (bitsPath)); @@ -344,9 +343,8 @@ UNIT_TEST(LocalCountryFile_DoNotDeleteUserFiles) CountryFile germanyFile("Germany"); LocalCountryFile germanyLocalFile(testDir.GetFullPath(), germanyFile, 101010 /* version */); + CountryIndexes::PreparePlaceOnDisk(germanyLocalFile); - TEST(CountryIndexes::PreparePlaceOnDisk(germanyLocalFile), - ("Can't prepare place for:", germanyLocalFile)); string const userFilePath = my::JoinFoldersToPath(CountryIndexes::IndexesDir(germanyLocalFile), "user-data.txt"); { diff --git a/routing/osrm2feature_map.cpp b/routing/osrm2feature_map.cpp index 9ecc62fd10..beb889908c 100644 --- a/routing/osrm2feature_map.cpp +++ b/routing/osrm2feature_map.cpp @@ -388,6 +388,7 @@ void OsrmFtSegBackwardIndex::Construct(OsrmFtSegMapping & mapping, uint32_t maxN if (m_oldFormat) LOG(LINFO, ("Using old format index for", localFile.GetCountryName())); + CountryIndexes::PreparePlaceOnDisk(localFile); string const bitsFileName = CountryIndexes::GetPath(localFile, CountryIndexes::Index::Bits); string const nodesFileName = CountryIndexes::GetPath(localFile, CountryIndexes::Index::Nodes); diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 42cd6b6ee4..733aa7c452 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -598,7 +598,7 @@ UNIT_TEST(StorageTest_DeleteCountry) LocalCountryFile file = LocalCountryFile::MakeForTesting("Wonderland"); TEST_EQUAL(MapOptions::MapWithCarRouting, file.GetFiles(), ()); - TEST(CountryIndexes::PreparePlaceOnDisk(file), ()); + CountryIndexes::PreparePlaceOnDisk(file); string const bitsPath = CountryIndexes::GetPath(file, CountryIndexes::Index::Bits); { FileWriter writer(bitsPath); -- cgit v1.2.3