RedAuburn/organicmaps
1
Fork 0
forked from organicmaps/organicmaps
Code Pull requests Activity

[storage] downloader interface is cleaned up

Browse source
This commit is contained in:
Arsentiy Milchakov 2019-12-25 10:16:31 +03:00 • committed by mpimenov
parent 3e158affc0
commit 0de4996357
11 changed files with 56 additions and 150 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
android/jni/com/mapswithme/maps/MapManager.cpp
View file

@ -342,7 +342,14 @@ Java_com_mapswithme_maps_downloader_MapManager_nativeIsDownloading(JNIEnv * env,
JNIEXPORT jstring JNICALL
Java_com_mapswithme_maps_downloader_MapManager_nativeGetCurrentDownloadingCountryId(JNIEnv * env, jclass)
{
return jni::ToJavaString(env, GetStorage().GetCurrentDownloadingCountryId());
auto const & downloadingCountries = GetStorage().GetCurrentDownloadingCountries();
if (downloadingCountries.empty())
return nullptr;
ASSERT_EQUAL(downloadingCountries.size(), 1, ());
return jni::ToJavaString(env, downloadingCountries.begin()->first);
}
static void StartBatchingCallbacks()

3
platform/downloader_defines.hpp
View file

@ -28,6 +28,7 @@ inline std::string DebugPrint(DownloadStatus status)
UNREACHABLE();
}
/// <bytes downloaded, total number of bytes>, total can be -1 if size is unknown
int64_t constexpr kUnknownTotalSize = -1;
/// <bytes downloaded, total number of bytes>, total can be kUnknownTotalSize if size is unknown
using Progress = std::pair<int64_t, int64_t>;
} // namespace downloader

6
search/search_integration_tests/downloader_search_test.cpp
View file

@ -87,12 +87,6 @@ class TestMapFilesDownloader : public storage::MapFilesDownloader
{
public:
// MapFilesDownloader overrides:
downloader::Progress GetDownloadingProgress() override { return {}; }
bool IsIdle() override { return false; }
void Pause() override {}
void Resume() override {}
void Remove(storage::CountryId const & id) override {}
void Clear() override {}

31
storage/http_map_files_downloader.cpp
View file

@ -86,33 +86,6 @@ void HttpMapFilesDownloader::Download()
}
}
downloader::Progress HttpMapFilesDownloader::GetDownloadingProgress()
{
CHECK_THREAD_CHECKER(m_checker, ());
ASSERT(nullptr != m_request, ());
return m_request->GetProgress();
}
bool HttpMapFilesDownloader::IsIdle()
{
CHECK_THREAD_CHECKER(m_checker, ());
return m_request.get() == nullptr;
}
void HttpMapFilesDownloader::Pause()
{
CHECK_THREAD_CHECKER(m_checker, ());
m_request.reset();
}
void HttpMapFilesDownloader::Resume()
{
CHECK_THREAD_CHECKER(m_checker, ());
if (!m_request && !m_queue.empty())
Download();
}
void HttpMapFilesDownloader::Remove(CountryId const & id)
{
CHECK_THREAD_CHECKER(m_checker, ());
@ -134,6 +107,10 @@ void HttpMapFilesDownloader::Remove(CountryId const & id)
for (auto const subscriber : m_subscribers)
subscriber->OnFinishDownloading();
}
else if (!m_request)
{
Download();
}
}
void HttpMapFilesDownloader::Clear()

4
storage/http_map_files_downloader.hpp
View file

@ -23,10 +23,6 @@ public:
virtual ~HttpMapFilesDownloader();
// MapFilesDownloader overrides:
downloader::Progress GetDownloadingProgress() override;
bool IsIdle() override;
void Pause() override;
void Resume() override;
void Remove(CountryId const & id) override;
void Clear() override;
Queue const & GetQueue() const override;

9
storage/map_files_downloader.hpp
View file

@ -45,15 +45,6 @@ public:
/// callback. Both callbacks will be invoked on the main thread.
void DownloadMapFile(QueuedCountry & queuedCountry);
/// Returns current downloading progress.
virtual downloader::Progress GetDownloadingProgress() = 0;
/// Returns true when downloader does not perform any job.
virtual bool IsIdle() = 0;
virtual void Pause() = 0;
virtual void Resume() = 0;
// Removes item from m_quarantine queue when list of servers is not received.
// Parent method must be called into override method.
virtual void Remove(CountryId const & id);

70
storage/storage.cpp
View file

@ -384,10 +384,10 @@ LocalAndRemoteSize Storage::CountrySizeInBytes(CountryId const & countryId) cons
if (!IsCountryInQueue(countryId) && !IsDiffApplyingInProgressToCountry(countryId))
sizes.first = localFile ? localFile->GetSize(MapFileType::Map) : 0;
if (!m_downloader->IsIdle() && IsCountryFirstInQueue(countryId))
{
sizes.first = m_downloader->GetDownloadingProgress().first + GetRemoteSize(countryFile);
}
auto const it = m_downloadingCountries.find(countryId);
if (it != m_downloadingCountries.cend())
sizes.first = it->second.first + GetRemoteSize(countryFile);
return sizes;
}
@ -444,7 +444,7 @@ Status Storage::CountryStatus(CountryId const & countryId) const
// Check if we are already downloading this country or have it in the queue.
if (IsCountryInQueue(countryId))
{
if (IsCountryFirstInQueue(countryId))
if (m_downloadingCountries.find(countryId) != m_downloadingCountries.cend())
return Status::EDownloading;
return Status::EInQueue;
@ -559,9 +559,9 @@ void Storage::DeleteCountry(CountryId const & countryId, MapFileType type)
DeleteCountryFilesFromDownloader(countryId);
m_diffsDataSource->RemoveDiffForCountry(countryId);
NotifyStatusChangedForHierarchy(countryId);
m_downloadingCountries.erase(countryId);
m_downloader->Resume();
NotifyStatusChangedForHierarchy(countryId);
}
void Storage::DeleteCustomCountryVersion(LocalCountryFile const & localFile)
@ -616,12 +616,11 @@ bool Storage::IsDownloadInProgress() const
return !m_downloader->GetQueue().empty();
}
CountryId Storage::GetCurrentDownloadingCountryId() const
Storage::DownloadingCountries const & Storage::GetCurrentDownloadingCountries() const
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
return IsDownloadInProgress() ? m_downloader->GetQueue().front().GetCountryId()
: storage::CountryId();
return m_downloadingCountries;
}
void Storage::LoadCountriesFile(string const & pathToCountriesFile)
@ -670,6 +669,8 @@ void Storage::OnMapFileDownloadFinished(QueuedCountry const & queuedCountry, Dow
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
m_downloadingCountries.erase(queuedCountry.GetCountryId());
OnMapDownloadFinished(queuedCountry.GetCountryId(), status, queuedCountry.GetFileType());
}
@ -695,8 +696,7 @@ void Storage::ReportProgressForHierarchy(CountryId const & countryId, Progress c
descendants.push_back(container.Value().Name());
});
Progress localAndRemoteBytes =
CalculateProgress(countryId, descendants, leafProgress, setQueue);
Progress localAndRemoteBytes = CalculateProgress(descendants);
ReportProgress(parentId, localAndRemoteBytes);
};
@ -711,6 +711,8 @@ void Storage::OnMapFileDownloadProgress(QueuedCountry const & queuedCountry,
if (m_observers.empty())
return;
m_downloadingCountries[queuedCountry.GetCountryId()] = progress;
ReportProgressForHierarchy(queuedCountry.GetCountryId(), progress);
}
@ -841,14 +843,6 @@ bool Storage::IsCountryInQueue(CountryId const & countryId) const
return find(queue.cbegin(), queue.cend(), countryId) != queue.cend();
}
bool Storage::IsCountryFirstInQueue(CountryId const & countryId) const
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
auto const & queue = m_downloader->GetQueue();
return !queue.empty() && queue.front().GetCountryId() == countryId;
}
bool Storage::IsDiffApplyingInProgressToCountry(CountryId const & countryId) const
{
CHECK_THREAD_CHECKER(m_threadChecker, ());
@ -1364,6 +1358,7 @@ void Storage::OnCountryInQueue(CountryId const & id)
void Storage::OnStartDownloadingCountry(CountryId const & id)
{
m_downloadingCountries[id] = {0, kUnknownTotalSize};
NotifyStatusChangedForHierarchy(id);
}
@ -1468,21 +1463,8 @@ void Storage::GetNodeAttrs(CountryId const & countryId, NodeAttrs & nodeAttrs) c
CountriesVec subtree;
node->ForEachInSubtree(
[&subtree](CountryTree::Node const & d) { subtree.push_back(d.Value().Name()); });
CountryId const & downloadingMwm =
IsDownloadInProgress() ? GetCurrentDownloadingCountryId() : kInvalidCountryId;
Progress downloadingMwmProgress(0, 0);
if (!m_downloader->IsIdle())
{
downloadingMwmProgress = m_downloader->GetDownloadingProgress();
// If we don't know estimated file size then we ignore its progress.
if (downloadingMwmProgress.second == -1)
downloadingMwmProgress = {};
}
CountriesSet setQueue;
GetQueuedCountries(m_downloader->GetQueue(), setQueue);
nodeAttrs.m_downloadingProgress =
CalculateProgress(downloadingMwm, subtree, downloadingMwmProgress, setQueue);
nodeAttrs.m_downloadingProgress = CalculateProgress(subtree);
}
// Local mwm information and information about downloading mwms.
@ -1568,19 +1550,23 @@ void Storage::DoClickOnDownloadMap(CountryId const & countryId)
m_downloadMapOnTheMap(countryId);
}
Progress Storage::CalculateProgress(CountryId const & downloadingMwm, CountriesVec const & mwms,
Progress const & downloadingMwmProgress,
CountriesSet const & mwmsInQueue) const
Progress Storage::CalculateProgress(CountriesVec const & descendants) const
{
// Function calculates progress correctly ONLY if |downloadingMwm| is leaf.
Progress localAndRemoteBytes = {};
for (auto const & d : mwms)
CountriesSet mwmsInQueue;
GetQueuedCountries(m_downloader->GetQueue(), mwmsInQueue);
for (auto const & d : descendants)
{
if (downloadingMwm == d && downloadingMwm != kInvalidCountryId)
auto const downloadingIt = m_downloadingCountries.find(d);
if (downloadingIt != m_downloadingCountries.cend())
{
localAndRemoteBytes.first += downloadingMwmProgress.first;
if (downloadingIt->second.second != downloader::kUnknownTotalSize)
localAndRemoteBytes.first += downloadingIt->second.first;
localAndRemoteBytes.second += GetRemoteSize(GetCountryFile(d));
}
else if (mwmsInQueue.count(d) != 0)
@ -1626,11 +1612,11 @@ void Storage::CancelDownloadNode(CountryId const & countryId)
needNotify = true;
}
m_downloadingCountries.erase(countryId);
if (needNotify)
NotifyStatusChangedForHierarchy(countryId);
});
m_downloader->Resume();
}
void Storage::RetryDownloadNode(CountryId const & countryId)

19
storage/storage.hpp
View file

@ -157,6 +157,7 @@ public:
using DeleteCallback = std::function<bool(storage::CountryId const &, LocalFilePtr const)>;
using ChangeCountryFunction = std::function<void(CountryId const &)>;
using ProgressFunction = std::function<void(CountryId const &, downloader::Progress const &)>;
using DownloadingCountries = std::unordered_map<CountryId, downloader::Progress>;
private:
/// We support only one simultaneous request at the moment
@ -251,6 +252,8 @@ private:
StartDownloadingCallback m_startDownloadingCallback;
DownloadingCountries m_downloadingCountries;
void LoadCountriesFile(std::string const & pathToCountriesFile);
void ReportProgress(CountryId const & countryId, downloader::Progress const & p);
@ -534,9 +537,9 @@ public:
/// Notifies observers about country status change.
void DeleteCustomCountryVersion(platform::LocalCountryFile const & localFile);
bool IsDownloadInProgress() const;
DownloadingCountries const & GetCurrentDownloadingCountries() const;
CountryId GetCurrentDownloadingCountryId() const;
bool IsDownloadInProgress() const;
/// @param[out] res Populated with oudated countries.
void GetOutdatedCountries(std::vector<Country const *> & countries) const;
@ -574,9 +577,6 @@ private:
// Returns true when country is in the downloader's queue.
bool IsCountryInQueue(CountryId const & countryId) const;
// Returns true when country is first in the downloader's queue.
bool IsCountryFirstInQueue(CountryId const & countryId) const;
// Returns true if we started the diff applying procedure for an mwm with countryId.
bool IsDiffApplyingInProgressToCountry(CountryId const & countryId) const;
@ -632,14 +632,7 @@ private:
/// |descendants| All descendants of the parent node.
/// |downloadingMwm| Downloading leaf node country id if any. If not, downloadingMwm == kInvalidCountryId.
/// |downloadingMwm| Must be only leaf.
/// If downloadingMwm != kInvalidCountryId |downloadingMwmProgress| is a progress of downloading
/// the leaf node in bytes. |downloadingMwmProgress.first| == number of downloaded bytes.
/// |downloadingMwmProgress.second| == number of bytes in downloading files.
/// |mwmsInQueue| hash table made from |m_queue|.
downloader::Progress CalculateProgress(CountryId const & downloadingMwm,
CountriesVec const & descendants,
downloader::Progress const & downloadingMwmProgress,
CountriesSet const & mwmsInQueue) const;
downloader::Progress CalculateProgress(CountriesVec const & descendants) const;
template <class ToDo>
void ForEachAncestorExceptForTheRoot(std::vector<CountryTree::Node const *> const & nodes,

42
storage/storage_tests/fake_map_files_downloader.cpp
View file

@ -27,13 +27,11 @@ void FakeMapFilesDownloader::Download(QueuedCountry & queuedCountry)
m_queue.push_back(queuedCountry);
if (m_queue.size() != 1)
{
for (auto const subscriber : m_subscribers)
subscriber->OnCountryInQueue(queuedCountry.GetCountryId());
for (auto const subscriber : m_subscribers)
subscriber->OnCountryInQueue(queuedCountry.GetCountryId());
if (m_queue.size() != 1)
return;
}
for (auto const subscriber : m_subscribers)
subscriber->OnStartDownloading();
@ -41,40 +39,6 @@ void FakeMapFilesDownloader::Download(QueuedCountry & queuedCountry)
Download();
}
downloader::Progress FakeMapFilesDownloader::GetDownloadingProgress()
{
CHECK_THREAD_CHECKER(m_checker, ());
return m_progress;
}
bool FakeMapFilesDownloader::IsIdle()
{
CHECK_THREAD_CHECKER(m_checker, ());
return m_writer.get() == nullptr;
}
void FakeMapFilesDownloader::Pause()
{
CHECK_THREAD_CHECKER(m_checker, ());
m_writer.reset();
++m_timestamp;
}
void FakeMapFilesDownloader::Resume()
{
CHECK_THREAD_CHECKER(m_checker, ());
if (m_queue.empty() || m_writer)
return;
m_writer.reset(new FileWriter(m_queue.front().GetFileDownloadPath()));
++m_timestamp;
m_taskRunner.PostTask(std::bind(&FakeMapFilesDownloader::DownloadNextChunk, this, m_timestamp));
}
void FakeMapFilesDownloader::Remove(CountryId const & id)
{
CHECK_THREAD_CHECKER(m_checker, ());

4
storage/storage_tests/fake_map_files_downloader.hpp
View file

@ -34,10 +34,6 @@ public:
~FakeMapFilesDownloader();
// MapFilesDownloader overrides:
downloader::Progress GetDownloadingProgress() override;
bool IsIdle() override;
void Pause() override;
void Resume() override;
void Remove(CountryId const & id) override;
void Clear() override;
Queue const & GetQueue() const override;

9
storage/storage_tests/storage_tests.cpp
View file

@ -315,7 +315,7 @@ public:
TaskRunner & runner)
: CountryDownloaderChecker(
storage, countryId, MapFileType::Map,
vector<Status>{Status::ENotDownloaded, Status::EDownloading, Status::ENotDownloaded})
vector<Status>{Status::ENotDownloaded, Status::EInQueue, Status::EDownloading, Status::ENotDownloaded})
, m_runner(runner)
{
}
@ -345,7 +345,7 @@ unique_ptr<CountryDownloaderChecker> AbsentCountryDownloaderChecker(Storage & st
{
return make_unique<CountryDownloaderChecker>(
storage, countryId, type,
vector<Status>{Status::ENotDownloaded, Status::EDownloading, Status::EOnDisk});
vector<Status>{Status::ENotDownloaded, Status::EInQueue, Status::EDownloading, Status::EOnDisk});
}
// Checks following state transitions:
@ -356,7 +356,7 @@ unique_ptr<CountryDownloaderChecker> CancelledCountryDownloaderChecker(Storage &
{
return make_unique<CountryDownloaderChecker>(
storage, countryId, type,
vector<Status>{Status::ENotDownloaded, Status::EDownloading, Status::ENotDownloaded});
vector<Status>{Status::ENotDownloaded, Status::EInQueue, Status::EDownloading, Status::ENotDownloaded});
}
class CountryStatusChecker
@ -770,7 +770,8 @@ UNIT_CLASS_TEST(StorageTest, DownloadedMap)
{
auto algeriaCentralChecker = make_unique<CountryDownloaderChecker>(
storage, algeriaCentralCountryId, MapFileType::Map,
vector<Status>{Status::ENotDownloaded, Status::EDownloading, Status::EOnDisk});
vector<Status>{Status::ENotDownloaded, Status::EInQueue, Status::EDownloading,
Status::EOnDisk});
auto algeriaCoastChecker = make_unique<CountryDownloaderChecker>(
storage, algeriaCoastCountryId, MapFileType::Map,