map-per/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

Review fixes.

Browse source
This commit is contained in:
Yuri Gorshenin 2015-06-16 12:58:49 +03:00 • committed by Alex Zolotarev
parent b7b36b75e5
commit c9256b0909
9 changed files with 211 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
storage/map_files_downloader.hpp
View file

@ -2,8 +2,8 @@
#include "std/function.hpp"
#include "std/string.hpp"
#include "std/vector.hpp"
#include "std/utility.hpp"
#include "std/vector.hpp"
namespace storage
{
@ -12,6 +12,7 @@ namespace storage
class MapFilesDownloader
{
public:
// Denotes bytes downloaded and total number of bytes.
using TProgress = pair<int64_t, int64_t>;
using TFileDownloadedCallback = function<void(bool success, TProgress const & progress)>;
@ -20,12 +21,12 @@ public:
virtual ~MapFilesDownloader() = default;
/// Asynchroniously receives a list of all servers that can be asked
/// Asynchronously receives a list of all servers that can be asked
/// for a map file and invokes callback on the original thread.
virtual void GetServersList(string const & mapFileName,
TServersListCallback const & callback) = 0;
/// Asynchroniously downloads a map file, periodically invokes
/// Asynchronously downloads a map file, periodically invokes
/// onProgress callback and finally invokes onDownloaded
/// callback. Both callbacks will be invoked on the original thread.
virtual void DownloadMapFile(vector<string> const & urls, string const & path, int64_t size,

1
storage/storage.cpp
View file

@ -174,7 +174,6 @@ namespace storage
return m_pFile->GetFileWithExt(TMapOptions::EMap);
}
Storage::Storage() : m_downloader(new HttpMapFilesDownloader()), m_currentSlotId(0)
{
LoadCountriesFile(false);

13
storage/storage.hpp
View file

@ -98,13 +98,20 @@ namespace storage
void ReportProgress(TIndex const & index, pair<int64_t, int64_t> const & p);
/// @name
//@{
/// Called on the main thread by MapFilesDownloader when list of
/// suitable servers is received.
void OnServerListDownloaded(vector<string> const & urls);
/// Called on the main thread by MapFilesDownloader when
/// downloading of a map file succeeds/fails.
void OnMapDownloadFinished(bool success, MapFilesDownloader::TProgress const & progress);
/// Periodically called on the main thread by MapFilesDownloader
/// during the downloading process.
void OnMapDownloadProgress(MapFilesDownloader::TProgress const & progress);
/// Initiates downloading of the next file from the queue.
void DownloadNextFile(QueuedCountry const & cnt);
//@}
public:
Storage();

30
storage/storage_tests/fake_map_files_downloader.cpp
View file

@ -1,11 +1,14 @@
#include "storage/storage_tests/fake_map_files_downloader.hpp"
#include "storage/storage_tests/message_loop.hpp"
#include "storage/storage_tests/task_runner.hpp"
#include "coding/file_writer.hpp"
#include "base/assert.hpp"
#include "base/scope_guard.hpp"
#include "std/algorithm.hpp"
#include "std/bind.hpp"
namespace storage
{
@ -14,24 +17,21 @@ namespace
int64_t const kBlockSize = 1024 * 1024;
} // namespace
FakeMapFilesDownloader::FakeMapFilesDownloader(MessageLoop & loop)
: m_progress(make_pair(0, 0)), m_idle(true), m_loop(loop)
FakeMapFilesDownloader::FakeMapFilesDownloader(TaskRunner & taskRunner)
: m_progress(make_pair(0, 0)), m_idle(true), m_taskRunner(taskRunner)
{
m_servers.push_back("http://test-url/");
}
FakeMapFilesDownloader::~FakeMapFilesDownloader()
{
CHECK(m_checker.CalledOnOriginalThread(), ());
}
FakeMapFilesDownloader::~FakeMapFilesDownloader() { CHECK(m_checker.CalledOnOriginalThread(), ()); }
void FakeMapFilesDownloader::GetServersList(string const & mapFileName,
TServersListCallback const & callback)
{
CHECK(m_checker.CalledOnOriginalThread(), ());
m_idle = false;
MY_SCOPE_GUARD(resetIdle, [this]() { m_idle = true; });
m_loop.PostTask(bind(callback, m_servers));
MY_SCOPE_GUARD(resetIdle, bind(&FakeMapFilesDownloader::Reset, this));
m_taskRunner.PostTask(bind(callback, m_servers));
}
void FakeMapFilesDownloader::DownloadMapFile(vector<string> const & urls, string const & path,
@ -44,7 +44,7 @@ void FakeMapFilesDownloader::DownloadMapFile(vector<string> const & urls, string
m_progress.first = 0;
m_progress.second = size;
m_idle = false;
MY_SCOPE_GUARD(resetIdle, [this]() { m_idle = true; });
MY_SCOPE_GUARD(resetIdle, bind(&FakeMapFilesDownloader::Reset, this));
{
FileWriter writer(path);
@ -54,10 +54,10 @@ void FakeMapFilesDownloader::DownloadMapFile(vector<string> const & urls, string
writer.Write(kZeroes.data(), blockSize);
size -= blockSize;
m_progress.first += blockSize;
m_loop.PostTask(bind(onProgress, m_progress));
m_taskRunner.PostTask(bind(onProgress, m_progress));
}
}
m_loop.PostTask(bind(onDownloaded, true /* success */, m_progress));
m_taskRunner.PostTask(bind(onDownloaded, true /* success */, m_progress));
}
MapFilesDownloader::TProgress FakeMapFilesDownloader::GetDownloadingProgress()
@ -65,12 +65,14 @@ MapFilesDownloader::TProgress FakeMapFilesDownloader::GetDownloadingProgress()
return m_progress;
}
bool FakeMapFilesDownloader::IsIdle() {
bool FakeMapFilesDownloader::IsIdle()
{
CHECK(m_checker.CalledOnOriginalThread(), ());
return m_idle;
}
void FakeMapFilesDownloader::Reset() {
void FakeMapFilesDownloader::Reset()
{
CHECK(m_checker.CalledOnOriginalThread(), ());
m_idle = true;
}

10
storage/storage_tests/fake_map_files_downloader.hpp
View file

@ -5,19 +5,19 @@
namespace storage
{
class MessageLoop;
class TaskRunner;
// This class can be used in tests to mimic a real downloader. It
// always returns a single URL for map files downloading and when
// asked for a file, creates a file with zero-bytes content on a disk.
// Because all callbacks must be invoked asynchroniously, it needs a
// Because all callbacks must be invoked asynchronously, it needs a
// single-thread message loop runner to run callbacks.
//
// *NOTE*, this class is not thread-safe.
class FakeMapFilesDownloader : public MapFilesDownloader
{
public:
FakeMapFilesDownloader(MessageLoop & loop);
FakeMapFilesDownloader(TaskRunner & taskRunner);
virtual ~FakeMapFilesDownloader();
// MapFilesDownloader overrides:
@ -29,12 +29,12 @@ public:
bool IsIdle() override;
void Reset() override;
private:
private:
vector<string> m_servers;
TProgress m_progress;
bool m_idle;
MessageLoop & m_loop;
TaskRunner & m_taskRunner;
ThreadChecker m_checker;
};
} // namespace storage

220
storage/storage_tests/storage_tests.cpp
View file

@ -3,7 +3,7 @@
#include "storage/storage.hpp"
#include "storage/storage_defines.hpp"
#include "storage/storage_tests/fake_map_files_downloader.hpp"
#include "storage/storage_tests/message_loop.hpp"
#include "storage/storage_tests/task_runner.hpp"
#include "platform/platform.hpp"
@ -22,33 +22,45 @@ using namespace storage;
namespace
{
// This class checks steps Storage::DownloadMap() performs to download a map.
class CountryDownloaderChecker
{
public:
CountryDownloaderChecker(Storage & storage, string const & countryFileName)
CountryDownloaderChecker(Storage & storage, string const & countryFileName, TMapOptions files)
: m_storage(storage),
m_index(m_storage.FindIndexByFile(countryFileName)),
m_files(files),
m_lastStatus(TStatus::ENotDownloaded),
m_bytesDownloaded(0),
m_totalBytesToDownload(-1)
m_totalBytesToDownload(0),
m_slot(0)
{
CHECK(m_index.IsValid(), ());
CHECK_EQUAL(m_lastStatus, m_storage.CountryStatusEx(m_index), ());
m_slot = m_storage.Subscribe(
bind(&CountryDownloaderChecker::OnCountryStatusChanged, this, _1),
bind(&CountryDownloaderChecker::OnCountryDownloadingProgress, this, _1, _2));
m_storage.DownloadCountry(m_index, TMapOptions::EMap);
CHECK(m_index.IsValid(), ());
}
~CountryDownloaderChecker()
void StartDownload()
{
CHECK_EQUAL(m_lastStatus, m_storage.CountryStatusEx(m_index), ());
m_storage.DownloadCountry(m_index, m_files);
}
virtual ~CountryDownloaderChecker()
{
m_storage.Unsubscribe(m_slot);
CHECK_EQUAL(TStatus::EOnDisk, m_lastStatus, ());
CHECK_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, ());
m_storage.Unsubscribe(m_slot);
LocalAndRemoteSizeT localAndRemoteSize = m_storage.CountrySizeInBytes(m_index, m_files);
CHECK_EQUAL(m_bytesDownloaded, localAndRemoteSize.first, ());
CHECK_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, ());
}
protected:
virtual void CheckStatusTransition(TStatus oldStatus, TStatus newStatus) const = 0;
private:
void OnCountryStatusChanged(TIndex const & index)
{
@ -56,25 +68,12 @@ private:
return;
TStatus status = m_storage.CountryStatusEx(m_index);
LOG(LINFO, ("OnCountryStatusChanged", status));
switch (status)
CheckStatusTransition(m_lastStatus, status);
if (status == TStatus::EDownloading)
{
case TStatus::EDownloading:
CHECK(m_lastStatus == TStatus::EDownloading || m_lastStatus == TStatus::ENotDownloaded,
("It's only possible to move from {Downloading|NotDownloaded} to Downloading,"
"old status:",
m_lastStatus, ", new status:", status));
break;
case TStatus::EOnDisk:
CHECK(m_lastStatus == TStatus::EDownloading || m_lastStatus == TStatus::ENotDownloaded,
("It's only possible to move from {Downloading|NotDownloaded} to OnDisk,",
"old status:", m_lastStatus, ", new status:", status));
CHECK_EQUAL(m_totalBytesToDownload, m_bytesDownloaded, ());
break;
default:
CHECK(false, ("Unknown state change: from", m_lastStatus, "to", status));
LocalAndRemoteSizeT localAndRemoteSize = m_storage.CountrySizeInBytes(m_index, m_files);
m_totalBytesToDownload = localAndRemoteSize.second;
}
m_lastStatus = status;
}
@ -82,30 +81,119 @@ private:
{
if (index != m_index)
return;
LOG(LINFO, ("OnCountryDownloadingProgress:", progress));
CHECK_GREATER(progress.first, m_bytesDownloaded, ());
m_bytesDownloaded = progress.first;
m_totalBytesToDownload = progress.second;
CHECK_LESS_OR_EQUAL(m_bytesDownloaded, m_totalBytesToDownload, ());
LocalAndRemoteSizeT localAndRemoteSize = m_storage.CountrySizeInBytes(m_index, m_files);
CHECK_EQUAL(m_totalBytesToDownload, localAndRemoteSize.second, ());
}
Storage & m_storage;
TIndex const m_index;
TStatus m_lastStatus;
int m_slot;
TMapOptions const m_files;
TStatus m_lastStatus;
int64_t m_bytesDownloaded;
int64_t m_totalBytesToDownload;
int m_slot;
};
// Checks following state transitions:
// NotDownloaded -> Downloading -> OnDisk.
class AbsentCountryDownloaderChecker : public CountryDownloaderChecker
{
public:
AbsentCountryDownloaderChecker(Storage & storage, string const & countryFileName,
TMapOptions files)
: CountryDownloaderChecker(storage, countryFileName, files)
{
}
~AbsentCountryDownloaderChecker() override = default;
protected:
void CheckStatusTransition(TStatus oldStatus, TStatus newStatus) const override
{
switch (newStatus)
{
case TStatus::EDownloading:
CHECK_EQUAL(oldStatus, TStatus::ENotDownloaded,
("It's only possible to move from NotDownloaded to Downloading."));
break;
case TStatus::EOnDisk:
CHECK_EQUAL(oldStatus, TStatus::EDownloading,
("It's only possible to move from Downloading to OnDisk."));
break;
default:
CHECK(false, ("Unknown state change: from", oldStatus, "to", newStatus));
}
}
};
// Checks following state transitions:
// NotDownloaded -> InQueue -> Downloading -> OnDisk.
class QueuedCountryDownloaderChecker : public CountryDownloaderChecker
{
public:
QueuedCountryDownloaderChecker(Storage & storage, string const & countryFileName,
TMapOptions files)
: CountryDownloaderChecker(storage, countryFileName, files)
{
}
~QueuedCountryDownloaderChecker() override = default;
protected:
void CheckStatusTransition(TStatus oldStatus, TStatus newStatus) const override
{
switch (newStatus)
{
case TStatus::EInQueue:
CHECK_EQUAL(oldStatus, TStatus::ENotDownloaded,
("It's only possible to move from NotDownloaded to InQueue."));
break;
case TStatus::EDownloading:
CHECK_EQUAL(oldStatus, TStatus::EInQueue,
("It's only possible to move from InQueue to Downloading."));
break;
case TStatus::EOnDisk:
CHECK_EQUAL(oldStatus, TStatus::EDownloading,
("It's only possible to move from Downloading to OnDisk."));
break;
default:
CHECK(false, ("Unknown state change: from", oldStatus, "to", newStatus));
}
}
};
// Removes country's files that can be created during and after downloading.
void CleanupCountryFiles(string const & countryFileName)
{
Platform & platform = GetPlatform();
string const localMapFile =
my::JoinFoldersToPath(platform.WritableDir(), countryFileName + DATA_FILE_EXTENSION);
my::DeleteFileX(localMapFile);
my::DeleteFileX(localMapFile + READY_FILE_EXTENSION);
string const localRoutingFile = localMapFile + ROUTING_FILE_EXTENSION;
my::DeleteFileX(localRoutingFile);
my::DeleteFileX(localRoutingFile + READY_FILE_EXTENSION);
}
void OnCountryDownloaded(string const & mapFileName, TMapOptions files)
{
LOG(LINFO, ("OnCountryDownloaded:", mapFileName, static_cast<int>(files)));
Platform & platform = GetPlatform();
CHECK(my::RenameFileX(
my::JoinFoldersToPath(platform.WritableDir(), mapFileName + READY_FILE_EXTENSION),
my::JoinFoldersToPath(platform.WritableDir(), mapFileName)),
());
string const localMapFile = my::JoinFoldersToPath(platform.WritableDir(), mapFileName);
string const localRoutingFile = localMapFile + ROUTING_FILE_EXTENSION;
if (files & TMapOptions::EMap)
CHECK(my::RenameFileX(localMapFile + READY_FILE_EXTENSION, localMapFile), ());
if (files & TMapOptions::ECarRouting)
CHECK(my::RenameFileX(localRoutingFile + READY_FILE_EXTENSION, localRoutingFile), ());
}
} // namespace
@ -126,31 +214,53 @@ UNIT_TEST(StorageTest_Smoke)
TEST_NOT_EQUAL(i1, i2, ());
}
UNIT_TEST(StorageTest_Download)
UNIT_TEST(StorageTest_SingleCountryDownloading)
{
Platform & platform = GetPlatform();
string const countryFileName = "Azerbaijan";
string const localFileName =
my::JoinFoldersToPath(platform.WritableDir(), countryFileName + DATA_FILE_EXTENSION);
string const downloadedFileName = my::JoinFoldersToPath(
platform.WritableDir(), countryFileName + DATA_FILE_EXTENSION + READY_FILE_EXTENSION);
TEST(!Platform::IsFileExistsByFullPath(localFileName),
("Please, remove", localFileName, "before running the test."));
TEST(!Platform::IsFileExistsByFullPath(downloadedFileName),
("Please, remove", downloadedFileName, "before running the test."));
MY_SCOPE_GUARD(removeLocalFile, bind(&FileWriter::DeleteFileX, localFileName));
MY_SCOPE_GUARD(removeDownloadedFile, bind(&FileWriter::DeleteFileX, downloadedFileName));
string const azerbaijanFileName = "Azerbaijan";
CleanupCountryFiles(azerbaijanFileName);
Storage storage;
storage.Init(&OnCountryDownloaded);
MessageLoop loop;
storage.SetDownloaderForTesting(make_unique<FakeMapFilesDownloader>(loop));
TaskRunner taskRunner;
storage.SetDownloaderForTesting(make_unique<FakeMapFilesDownloader>(taskRunner));
{
CountryDownloaderChecker checker(storage, countryFileName);
loop.Run();
MY_SCOPE_GUARD(cleanupCountryFiles, bind(&CleanupCountryFiles, azerbaijanFileName));
AbsentCountryDownloaderChecker checker(storage, azerbaijanFileName, TMapOptions::EMap);
checker.StartDownload();
taskRunner.Run();
}
{
MY_SCOPE_GUARD(cleanupCountryFiles, bind(&CleanupCountryFiles, azerbaijanFileName));
AbsentCountryDownloaderChecker checker(storage, azerbaijanFileName,
TMapOptions::EMapWithCarRouting);
checker.StartDownload();
taskRunner.Run();
}
}
UNIT_TEST(StorageTest_TwoCountriesDownloading)
{
string const uruguayFileName = "Uruguay";
string const venezuelaFileName = "Venezuela";
CleanupCountryFiles(uruguayFileName);
MY_SCOPE_GUARD(cleanupUruguayFiles, bind(&CleanupCountryFiles, uruguayFileName));
CleanupCountryFiles(venezuelaFileName);
MY_SCOPE_GUARD(cleanupVenezuelaFiles, bind(&CleanupCountryFiles, venezuelaFileName));
Storage storage;
storage.Init(&OnCountryDownloaded);
TaskRunner taskRunner;
storage.SetDownloaderForTesting(make_unique<FakeMapFilesDownloader>(taskRunner));
AbsentCountryDownloaderChecker uruguayChecker(storage, uruguayFileName, TMapOptions::EMap);
QueuedCountryDownloaderChecker venezuelaChecker(storage, venezuelaFileName,
TMapOptions::EMapWithCarRouting);
uruguayChecker.StartDownload();
venezuelaChecker.StartDownload();
taskRunner.Run();
}

4
storage/storage_tests/storage_tests.pro
View file

@ -19,13 +19,13 @@ QT *= core
HEADERS += \
fake_map_files_downloader.hpp \
message_loop.hpp \
task_runner.hpp \
SOURCES += \
../../testing/testingmain.cpp \
country_info_test.cpp \
fake_map_files_downloader.cpp \
message_loop.cpp \
simple_tree_test.cpp \
storage_tests.cpp \
task_runner.cpp \

11
storage/storage_tests/message_loop.cpp → storage/storage_tests/task_runner.cpp
View file

@ -1,15 +1,12 @@
#include "storage/storage_tests/message_loop.hpp"
#include "storage/storage_tests/task_runner.hpp"
#include "base/assert.hpp"
namespace storage
{
MessageLoop::~MessageLoop()
{
CHECK(m_checker.CalledOnOriginalThread(), ());
}
TaskRunner::~TaskRunner() { CHECK(m_checker.CalledOnOriginalThread(), ()); }
void MessageLoop::Run()
void TaskRunner::Run()
{
CHECK(m_checker.CalledOnOriginalThread(), ());
while (!m_tasks.empty())
@ -20,7 +17,7 @@ void MessageLoop::Run()
}
}
void MessageLoop::PostTask(TTask const & task)
void TaskRunner::PostTask(TTask const & task)
{
CHECK(m_checker.CalledOnOriginalThread(), ());
m_tasks.push(task);

10
storage/storage_tests/message_loop.hpp → storage/storage_tests/task_runner.hpp
View file

@ -14,19 +14,19 @@ namespace storage
// message loop where B is run.
//
// *NOTE*, this class is not thread-safe.
class MessageLoop
class TaskRunner
{
public:
public:
using TTask = function<void()>;
~MessageLoop();
~TaskRunner();
void Run();
void PostTask(TTask const & task);
private:
private:
queue<TTask> m_tasks;
ThreadChecker m_checker;
};
} // namespace
} // namespace storage