From 1b74903e669c43af4d1f41928af8c7613122c852 Mon Sep 17 00:00:00 2001 From: Alex Zolotarev Date: Mon, 5 Dec 2011 18:09:34 +0300 Subject: [PATCH] - Split Platform::GetFileSize() to GetFileSizeByName() and GetFileSizeByFullPath() - Fixed country downloading for Android - Improved unit tests for chunks downloader --- generator/update_generator.cpp | 2 +- platform/platform.hpp | 16 +++++++---- platform/platform_android.cpp | 24 ++++++++++++---- platform/platform_ios.mm | 22 +++++++++++---- platform/platform_linux.cpp | 4 +-- platform/platform_mac.mm | 6 ++-- platform/platform_qt.cpp | 18 ++++++++++-- platform/platform_tests/downloader_test.cpp | 31 ++++++++++++++++++++- platform/platform_tests/platform_test.cpp | 21 +++++++++++--- platform/platform_win.cpp | 4 +-- storage/country.cpp | 2 +- storage/storage.cpp | 2 +- 12 files changed, 117 insertions(+), 35 deletions(-) diff --git a/generator/update_generator.cpp b/generator/update_generator.cpp index acf6a877f4..7b974c50c4 100644 --- a/generator/update_generator.cpp +++ b/generator/update_generator.cpp @@ -55,7 +55,7 @@ namespace update ++m_processedFiles; uint64_t size = 0; string const fname = c.Value().m_files[i].GetFileWithExt(); - if (!GetPlatform().GetFileSize(m_dataDir + fname, size)) + if (!GetPlatform().GetFileSizeByFullPath(m_dataDir + fname, size)) LOG(LERROR, ("File was not found:", fname)); CHECK_GREATER(size, 0, ("Zero file size?", fname)); c.Value().m_files[i].m_remoteSize = size; diff --git a/platform/platform.hpp b/platform/platform.hpp index 1b1b57aca8..8ea2e1bd79 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -19,14 +19,16 @@ protected: /// Used only on those platforms where needed PlatformImpl * m_impl; + static bool IsFileExistsByFullPath(string const & filePath); + /// Internal function to use files from writable dir if they override the same in the resources string ReadPathForFile(string const & file) const { string fullPath = m_writableDir + file; - if (!IsFileExists(fullPath)) + if (!IsFileExistsByFullPath(fullPath)) { fullPath = m_resourcesDir + file; - if (!IsFileExists(fullPath)) + if (!IsFileExistsByFullPath(fullPath)) MYTHROW(FileAbsentException, ("File doesn't exist", fullPath)); } return fullPath; @@ -56,11 +58,13 @@ public: /// @param directory directory path with slash at the end /// @param mask files extension to find, like ".map" etc /// @return number of files found in outFiles - void GetFilesInDir(string const & directory, string const & mask, FilesList & outFiles) const; + static void GetFilesInDir(string const & directory, string const & mask, FilesList & outFiles); /// @return false if file is not exist - bool GetFileSize(string const & file, uint64_t & size) const; - /// Simple file existing check - bool IsFileExists(string const & file) const; + /// @note Check files in Writable dir first, and in ReadDir if not exist in Writable dir + bool GetFileSizeByName(string const & fileName, uint64_t & size) const; + /// @return false if file is not exist + /// @note Try do not use in client production code + static bool GetFileSizeByFullPath(string const & filePath, uint64_t & size); //@} int CpuCores() const; diff --git a/platform/platform_android.cpp b/platform/platform_android.cpp index a5d1ab1b1e..1bd480fe10 100644 --- a/platform/platform_android.cpp +++ b/platform/platform_android.cpp @@ -14,15 +14,15 @@ Platform::~Platform() {} /// @warning doesn't work for files inside .apk (zip)!!! -bool Platform::IsFileExists(string const & file) const +bool Platform::IsFileExistsByFullPath(string const & filePath) { struct stat s; - return stat(file.c_str(), &s) == 0; + return stat(filePath.c_str(), &s) == 0; } ModelReader * Platform::GetReader(string const & file) const { - if (IsFileExists(m_writableDir + file)) + if (IsFileExistsByFullPath(m_writableDir + file)) return new FileReader(ReadPathForFile(file), 10, 12); else { @@ -31,7 +31,7 @@ ModelReader * Platform::GetReader(string const & file) const } } -void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & res) const +void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & res) { if (ZipFileReader::IsZip(directory)) { // Get files list inside zip file @@ -132,11 +132,11 @@ int Platform::VideoMemoryLimit() const return 10 * 1024 * 1024; } -bool Platform::GetFileSize(string const & file, uint64_t & size) const +bool Platform::GetFileSizeByName(string const & fileName, uint64_t & size) const { try { - size = ReaderPtr(GetReader(file)).Size(); + size = ReaderPtr(GetReader(fileName)).Size(); return true; } catch (RootException const &) @@ -145,6 +145,18 @@ bool Platform::GetFileSize(string const & file, uint64_t & size) const } } +/// @warning doesn't work for files inside .apk (zip)!!! +bool Platform::GetFileSizeByFullPath(string const & filePath, uint64_t & size) +{ + struct stat s; + if (stat(filePath.c_str(), &s) == 0) + { + size = s.st_size; + return true; + } + return false; +} + string Platform::UniqueClientId() const { return "@TODO"; diff --git a/platform/platform_ios.mm b/platform/platform_ios.mm index 3c9112a253..6edbf61c4d 100644 --- a/platform/platform_ios.mm +++ b/platform/platform_ios.mm @@ -91,13 +91,13 @@ Platform::~Platform() delete m_impl; } -bool Platform::IsFileExists(string const & file) const +bool Platform::IsFileExistsByFullPath(string const & filePath) { struct stat s; - return stat(file.c_str(), &s) == 0; + return stat(filePath.c_str(), &s) == 0; } -void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & res) const +void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & res) { DIR * dir; struct dirent * entry; @@ -133,10 +133,10 @@ void Platform::GetFilesInDir(string const & directory, string const & mask, File closedir(dir); } -bool Platform::GetFileSize(string const & file, uint64_t & size) const +bool Platform::GetFileSizeByFullPath(string const & filePath, uint64_t & size) { struct stat s; - if (stat(file.c_str(), &s) == 0) + if (stat(filePath.c_str(), &s) == 0) { size = s.st_size; return true; @@ -144,6 +144,18 @@ bool Platform::GetFileSize(string const & file, uint64_t & size) const return false; } +bool Platform::GetFileSizeByName(string const & fileName, uint64_t & size) const +{ + try + { + return GetFileSizeByFullPath(ReadPathForFile(fileName), size); + } + catch (std::exception const &) + { + return false; + } +} + void Platform::GetFontNames(FilesList & res) const { GetFilesInDir(ResourcesDir(), "*.ttf", res); diff --git a/platform/platform_linux.cpp b/platform/platform_linux.cpp index 4de81fe05a..54f21a49a9 100644 --- a/platform/platform_linux.cpp +++ b/platform/platform_linux.cpp @@ -50,10 +50,10 @@ Platform::~Platform() { } -bool Platform::IsFileExists(string const & file) const +bool Platform::IsFileExistsByFullPath(string const & filePath) { struct stat s; - return stat(file.c_str(), &s) == 0; + return stat(filePath.c_str(), &s) == 0; } int Platform::CpuCores() const diff --git a/platform/platform_mac.mm b/platform/platform_mac.mm index 1005472c29..ef945a132e 100644 --- a/platform/platform_mac.mm +++ b/platform/platform_mac.mm @@ -36,7 +36,7 @@ Platform::Platform() #ifndef OMIM_PRODUCTION // developers can have symlink to data folder char const * dataPath = "../../../../../data/"; - if (IsFileExists(m_resourcesDir + dataPath)) + if (IsFileExistsByFullPath(m_resourcesDir + dataPath)) m_writableDir = m_resourcesDir + dataPath; #endif @@ -59,10 +59,10 @@ Platform::~Platform() { } -bool Platform::IsFileExists(string const & file) const +bool Platform::IsFileExistsByFullPath(string const & filePath) { struct stat s; - return stat(file.c_str(), &s) == 0; + return stat(filePath.c_str(), &s) == 0; } int Platform::CpuCores() const diff --git a/platform/platform_qt.cpp b/platform/platform_qt.cpp index 81ce96c5bd..a09a94af43 100644 --- a/platform/platform_qt.cpp +++ b/platform/platform_qt.cpp @@ -17,14 +17,26 @@ ModelReader * Platform::GetReader(string const & file) const return new FileReader(ReadPathForFile(file), 10, 12); } -bool Platform::GetFileSize(string const & file, uint64_t & size) const +bool Platform::GetFileSizeByFullPath(string const & filePath, uint64_t & size) { - QFileInfo f(file.c_str()); + QFileInfo f(filePath.c_str()); size = static_cast(f.size()); return size != 0; } -void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & outFiles) const +bool Platform::GetFileSizeByName(string const & fileName, uint64_t & size) const +{ + try + { + return GetFileSizeByFullPath(ReadPathForFile(fileName), size); + } + catch (std::exception const &) + { + return false; + } +} + +void Platform::GetFilesInDir(string const & directory, string const & mask, FilesList & outFiles) { QDir dir(directory.c_str(), mask.c_str(), QDir::Unsorted, QDir::Files | QDir::Readable | QDir::Dirs | QDir::NoDotAndDotDot); diff --git a/platform/platform_tests/downloader_test.cpp b/platform/platform_tests/downloader_test.cpp index 2e56d401fb..89c2e4336c 100644 --- a/platform/platform_tests/downloader_test.cpp +++ b/platform/platform_tests/downloader_test.cpp @@ -306,6 +306,14 @@ UNIT_TEST(DownloadChunks) { string const FILENAME = "some_downloader_test_file"; + { // remove data from previously failed files + Platform::FilesList files; + Platform::GetFilesInDir(".", "*.resume", files); + Platform::GetFilesInDir(".", "*.downloading", files); + for (Platform::FilesList::iterator it = files.begin(); it != files.end(); ++it) + FileWriter::DeleteFileX(*it); + } + DownloadObserver observer; HttpRequest::CallbackT onFinish = bind(&DownloadObserver::OnDownloadFinish, &observer, _1); HttpRequest::CallbackT onProgress = bind(&DownloadObserver::OnDownloadProgress, &observer, _1); @@ -326,6 +334,8 @@ UNIT_TEST(DownloadChunks) TEST(ReadFileAsString(FILENAME, s), ()); TEST_EQUAL(s, "Test1", ()); FileWriter::DeleteFileX(FILENAME); + uint64_t size; + TEST(!Platform::GetFileSizeByFullPath(FILENAME + ".resume", size), ("No resume file on success")); } observer.Reset(); @@ -343,6 +353,9 @@ UNIT_TEST(DownloadChunks) QCoreApplication::exec(); observer.TestFailed(); FileWriter::DeleteFileX(FILENAME); + uint64_t size; + TEST(Platform::GetFileSizeByFullPath(FILENAME + ".resume", size), ("Resume file should present")); + FileWriter::DeleteFileX(FILENAME + ".resume"); } string const SHA256 = "EE6AE6A2A3619B2F4A397326BEC32583DE2196D9D575D66786CB3B6F9D04A633"; @@ -365,6 +378,8 @@ UNIT_TEST(DownloadChunks) TEST(ReadFileAsString(FILENAME, s), ()); TEST_EQUAL(sha2::digest256(s), SHA256, ()); FileWriter::DeleteFileX(FILENAME); + uint64_t size; + TEST(!Platform::GetFileSizeByFullPath(FILENAME + ".resume", size), ("No resume file on success")); } observer.Reset(); @@ -384,6 +399,8 @@ UNIT_TEST(DownloadChunks) TEST(ReadFileAsString(FILENAME, s), ()); TEST_EQUAL(sha2::digest256(s), SHA256, ()); FileWriter::DeleteFileX(FILENAME); + uint64_t size; + TEST(!Platform::GetFileSizeByFullPath(FILENAME + ".resume", size), ("No resume file on success")); } observer.Reset(); @@ -399,6 +416,9 @@ UNIT_TEST(DownloadChunks) QCoreApplication::exec(); observer.TestFailed(); FileWriter::DeleteFileX(FILENAME); + uint64_t size; + TEST(Platform::GetFileSizeByFullPath(FILENAME + ".resume", size), ("Resume file should present")); + FileWriter::DeleteFileX(FILENAME + ".resume"); } } @@ -439,6 +459,14 @@ UNIT_TEST(DownloadResumeChunks) string const RESUME_FILENAME = FILENAME + ".resume"; string const SHA256 = "EE6AE6A2A3619B2F4A397326BEC32583DE2196D9D575D66786CB3B6F9D04A633"; + { // remove data from previously failed files + Platform::FilesList files; + Platform::GetFilesInDir(".", "*.resume", files); + Platform::GetFilesInDir(".", "*.downloading", files); + for (Platform::FilesList::iterator it = files.begin(); it != files.end(); ++it) + FileWriter::DeleteFileX(*it); + } + vector urls; urls.push_back(TEST_URL_BIG_FILE); @@ -493,7 +521,8 @@ UNIT_TEST(DownloadResumeChunks) string s; TEST(ReadFileAsString(FILENAME, s), ()); TEST_EQUAL(sha2::digest256(s), SHA256, ()); - TEST(!GetPlatform().IsFileExists(RESUME_FILENAME), ()); + uint64_t size = 0; + TEST(!GetPlatform().GetFileSizeByFullPath(RESUME_FILENAME, size), ()); FileWriter::DeleteFileX(FILENAME); } } diff --git a/platform/platform_tests/platform_test.cpp b/platform/platform_tests/platform_test.cpp index cd91f51488..5257e18aec 100644 --- a/platform/platform_tests/platform_test.cpp +++ b/platform/platform_tests/platform_test.cpp @@ -77,27 +77,40 @@ UNIT_TEST(GetFilesInDir) UNIT_TEST(GetFileSize) { Platform & pl = GetPlatform(); + uint64_t size = 123141; + TEST(!pl.GetFileSizeByName("adsmngfuwrbfyfwe", size), ()); + TEST(!Platform::GetFileSizeByFullPath("adsmngfuwrbfyfwe", size), ()); { FileWriter testFile(TEST_FILE_NAME); testFile.Write("HOHOHO", 6); } - uint64_t size = 0; - pl.GetFileSize(TEST_FILE_NAME, size); + size = 0; + TEST(Platform::GetFileSizeByFullPath(TEST_FILE_NAME, size), ()); TEST_EQUAL(size, 6, ()); FileWriter::DeleteFileX(TEST_FILE_NAME); + + { + FileWriter testFile(pl.WritablePathForFile(TEST_FILE_NAME)); + testFile.Write("HOHOHO", 6); + } + size = 0; + TEST(pl.GetFileSizeByName(TEST_FILE_NAME, size), ()); + TEST_EQUAL(size, 6, ()); + + FileWriter::DeleteFileX(pl.WritablePathForFile(TEST_FILE_NAME)); } UNIT_TEST(CpuCores) { - int coresNum = GetPlatform().CpuCores(); + int const coresNum = GetPlatform().CpuCores(); TEST_GREATER(coresNum, 0, ()); TEST_LESS_OR_EQUAL(coresNum, 128, ()); } UNIT_TEST(VisualScale) { - double visualScale = GetPlatform().VisualScale(); + double const visualScale = GetPlatform().VisualScale(); TEST_GREATER_OR_EQUAL(visualScale, 1.0, ()); } diff --git a/platform/platform_win.cpp b/platform/platform_win.cpp index 4d454f22f2..cd5e394a7e 100644 --- a/platform/platform_win.cpp +++ b/platform/platform_win.cpp @@ -81,9 +81,9 @@ Platform::~Platform() { } -bool Platform::IsFileExists(string const & file) const +bool Platform::IsFileExistsByFullPath(string const & filePath) { - return ::GetFileAttributesA(file.c_str()) != INVALID_FILE_ATTRIBUTES; + return ::GetFileAttributesA(filePath.c_str()) != INVALID_FILE_ATTRIBUTES; } int Platform::CpuCores() const diff --git a/storage/country.cpp b/storage/country.cpp index b1790ffa2a..db33df642a 100644 --- a/storage/country.cpp +++ b/storage/country.cpp @@ -20,7 +20,7 @@ namespace storage bool IsFileDownloaded(CountryFile const & file) { uint64_t size = 0; - if (!GetPlatform().GetFileSize(GetPlatform().WritablePathForFile(file.GetFileWithExt()), size)) + if (!GetPlatform().GetFileSizeByName(file.GetFileWithExt(), size)) return false; return true;//tile.second == size; diff --git a/storage/storage.cpp b/storage/storage.cpp index a9d93ab158..e00d29bea5 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -121,7 +121,7 @@ namespace storage if (m_failedCountries.find(index) != m_failedCountries.end()) return EDownloadFailed; - LocalAndRemoteSizeT size = CountryByIndex(index).Size(); + LocalAndRemoteSizeT const size = CountryByIndex(index).Size(); if (size.first == size.second) { if (size.second == 0)