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

Review fixes

Browse source
This commit is contained in:
r.kuznetsov 2017-08-21 14:16:03 +03:00 • committed by Vladimir Byko-Ianko
parent 8910e51e8b
commit a8f973ba1b
9 changed files with 65 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -22,7 +22,7 @@ namespace
{
using namespace storage;
// Here we left 5% for diffs applying.
// The last 5% are left for applying diffs.
float const kMaxProgress = 95.0f;
enum ItemCategory : uint32_t

22
android/src/com/mapswithme/maps/downloader/UpdaterDialogFragment.java
View file

@ -24,6 +24,7 @@ import com.mapswithme.util.UiUtils;
import com.mapswithme.util.Utils;
import com.mapswithme.util.statistics.Statistics;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
@ -55,9 +56,14 @@ public class UpdaterDialogFragment extends BaseMwmDialogFragment
private boolean mAutoUpdate;
@Nullable
private String[] mOutdatedMaps;
/**
* Stores maps which are left to finish autoupdating process.
*/
@Nullable
private Set<String> mLeftoverMaps;
@Nullable
private BaseNewsFragment.NewsDialogListener mDoneListener;
private Set<String> mLeftoverMaps;
@NonNull
private final MapManager.StorageCallback mStorageCallback = new MapManager.StorageCallback()
@ -108,6 +114,8 @@ public class UpdaterDialogFragment extends BaseMwmDialogFragment
}
else if (item.isLeafNode && item.newStatus == CountryItem.STATUS_DONE)
{
if (mLeftoverMaps == null)
throw new AssertionError("mLeftoverMaps can't be null if mOutdatedMaps != null");
mLeftoverMaps.remove(item.countryId);
}
}
@ -311,12 +319,8 @@ public class UpdaterDialogFragment extends BaseMwmDialogFragment
mTotalSize = args.getString(ARG_TOTAL_SIZE);
mTotalSizeMb = args.getLong(ARG_TOTAL_SIZE_MB, 0L);
mOutdatedMaps = args.getStringArray(ARG_OUTDATED_MAPS);
if (mOutdatedMaps != null)
{
mLeftoverMaps = new HashSet<String>();
for (String map : mOutdatedMaps)
mLeftoverMaps.add(map);
}
if (mOutdatedMaps != null && mOutdatedMaps.length > 0)
mLeftoverMaps = new HashSet<>(Arrays.asList(mOutdatedMaps));
}
private void initViews()
@ -336,8 +340,6 @@ public class UpdaterDialogFragment extends BaseMwmDialogFragment
private boolean isAllUpdated()
{
if (mOutdatedMaps == null)
return true;
return mLeftoverMaps.isEmpty();
return mOutdatedMaps == null || mLeftoverMaps == null || mLeftoverMaps.isEmpty();
}
}

2
iphone/Maps/Common/MWMConsts.h
View file

@ -4,3 +4,5 @@ static NSTimeInterval const kDefaultAnimationDuration = .2;
static uint64_t const KB = 1024;
static uint64_t const MB = 1024 * 1024;
// The last 5% are left for applying diffs.
static float const kMaxProgress = 0.95f;

21
iphone/Maps/UI/Autoupdate/MWMAutoupdateController.mm
View file

@ -210,22 +210,20 @@ std::unordered_set<TCountryId> updatingCountries;
GetFramework().GetStorage().GetNodeStatuses(countryId, nodeStatuses);
if (nodeStatuses.m_status == NodeStatus::Error)
{
if (!nodeStatuses.m_groupNode)
updatingCountries.erase(countryId);
self.errorCode = nodeStatuses.m_error;
SEL const process = @selector(processError);
[NSObject cancelPreviousPerformRequestsWithTarget:self selector:process object:nil];
[self performSelector:process withObject:nil afterDelay:0.2];
}
else if (nodeStatuses.m_status == NodeStatus::OnDisk)
if (!nodeStatuses.m_groupNode)
{
if (!nodeStatuses.m_groupNode)
updatingCountries.erase(countryId);
}
else
{
if (!nodeStatuses.m_groupNode)
updatingCountries.insert(countryId);
switch (nodeStatuses.m_status)
{
case NodeStatus::Error:
case NodeStatus::OnDisk: updatingCountries.erase(countryId); break;
default: updatingCountries.insert(countryId);
}
}
if (self.progressFinished && updatingCountries.empty())
@ -266,8 +264,7 @@ std::unordered_set<TCountryId> updatingCountries;
s.GetNodeAttrs(RootId(), nodeAttrs);
auto const p = nodeAttrs.m_downloadingProgress;
auto view = static_cast<MWMAutoupdateView *>(self.view);
// Here we left 5% for diffs applying.
view.progress = 0.95f * static_cast<CGFloat>(p.first) / p.second;
view.progress = kMaxProgress * static_cast<CGFloat>(p.first) / p.second;
if (p.first == p.second)
self.progressFinished = YES;
}

6
iphone/Maps/UI/Downloader/Cells/MWMMapDownloaderTableViewCell.mm
View file

@ -111,8 +111,7 @@
case NodeStatus::Downloading:
{
auto const & prg = nodeAttrs.m_downloadingProgress;
// Here we left 5% for diffs applying.
progress.progress = 0.95f * static_cast<CGFloat>(prg.first) / prg.second;
progress.progress = kMaxProgress * static_cast<CGFloat>(prg.first) / prg.second;
break;
}
case NodeStatus::InQueue: progress.state = MWMCircularProgressStateSpinner; break;
@ -147,8 +146,7 @@
{
if (countryId != m_countryId)
return;
// Here we left 5% for diffs applying.
self.progress.progress = 0.95f * static_cast<CGFloat>(progress.first) / progress.second;
self.progress.progress = kMaxProgress * static_cast<CGFloat>(progress.first) / progress.second;
}
#pragma mark - MWMCircularProgressProtocol

16
storage/diff_scheme/diff_manager.cpp
View file

@ -74,19 +74,19 @@ void Manager::ApplyDiff(ApplyDiffParams && p, std::function<void(bool const resu
diffFile->DeleteFromDisk(MapOptions::Diff);
}
if (!result)
{
std::lock_guard<std::mutex> lock(m_mutex);
m_status = Status::NotAvailable;
// TODO: Log the diff applying error (Downloader_DiffScheme_error (Aloha)).
}
else
if (result)
{
std::lock_guard<std::mutex> lock(m_mutex);
m_diffs.erase(countryId);
if (m_diffs.empty())
m_status = Status::NotAvailable;
}
else
{
std::lock_guard<std::mutex> lock(m_mutex);
m_status = Status::NotAvailable;
// TODO: Log the diff applying error (Downloader_DiffScheme_error (Aloha)).
}
task(result);
});
@ -119,6 +119,8 @@ bool Manager::HasDiffFor(storage::TCountryId const & countryId) const
bool Manager::HasDiffForUnsafe(storage::TCountryId const & countryId) const
{
if (m_status != diffs::Status::Available)
return false;
return m_diffs.find(countryId) != m_diffs.end();
}

13
storage/storage.cpp
View file

@ -1687,8 +1687,7 @@ bool Storage::GetUpdateInfo(TCountryId const & countryId, UpdateInfo & updateInf
GetNodeStatus(descendantNode).status != NodeStatus::OnDiskOutOfDate)
return;
updateInfo.m_numberOfMwmFilesToUpdate += 1; // It's not a group mwm.
if (m_diffManager.GetStatus() == diffs::Status::Available &&
m_diffManager.HasDiffFor(descendantNode.Value().Name()))
if (m_diffManager.HasDiffFor(descendantNode.Value().Name()))
{
updateInfo.m_totalUpdateSizeInBytes +=
m_diffManager.InfoFor(descendantNode.Value().Name()).m_size;
@ -1797,7 +1796,6 @@ void Storage::GetTopmostNodesFor(TCountryId const & countryId, TCountriesVec & n
}
}
TCountryId const Storage::GetParentIdFor(TCountryId const & countryId) const
{
vector<TCountryTreeNode const *> nodes;
@ -1817,19 +1815,14 @@ TCountryId const Storage::GetParentIdFor(TCountryId const & countryId) const
return nodes[0]->Value().GetParent();
}
TMwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt,
int64_t version) const
TMwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt, int64_t version) const
{
if (version::IsSingleMwm(version))
{
if (opt == MapOptions::Nothing)
return 0;
if (m_diffManager.GetStatus() == diffs::Status::Available &&
m_diffManager.HasDiffFor(file.GetName()))
{
if (m_diffManager.HasDiffFor(file.GetName()))
return m_diffManager.InfoFor(file.GetName()).m_size;
}
return file.GetRemoteSize(MapOptions::Map);
}

5
storage/storage.hpp
View file

@ -508,6 +508,8 @@ public:
bool IsInnerNode(TCountryId const & countryId) const;
TLocalAndRemoteSize CountrySizeInBytes(TCountryId const & countryId, MapOptions opt) const;
TMwmSize GetRemoteSize(platform::CountryFile const & file, MapOptions opt,
int64_t version) const;
platform::CountryFile const & GetCountryFile(TCountryId const & countryId) const;
TLocalFilePtr GetLatestLocalFile(platform::CountryFile const & countryFile) const;
TLocalFilePtr GetLatestLocalFile(TCountryId const & countryId) const;
@ -660,9 +662,6 @@ private:
void LoadDiffScheme();
void OnDiffStatusReceived() override;
TMwmSize GetRemoteSize(platform::CountryFile const & file, MapOptions opt,
int64_t version) const;
};
void GetQueuedCountries(Storage::TQueue const & queue, TCountriesSet & resultCountries);

49
storage/storage_tests/queued_country_tests.cpp
View file

@ -13,19 +13,19 @@ UNIT_TEST(QueuedCountry_AddOptions)
TEST_EQUAL(countryId, country.GetCountryId(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
country.AddOptions(MapOptions::Map);
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
TEST(country.SwitchToNextFile(), ());
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
TEST(!country.SwitchToNextFile(), ());
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
}
UNIT_TEST(QueuedCountry_RemoveOptions)
@ -36,57 +36,58 @@ UNIT_TEST(QueuedCountry_RemoveOptions)
{
QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
country.RemoveOptions(MapOptions::Map);
TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
country.RemoveOptions(MapOptions::CarRouting);
TEST_EQUAL(MapOptions::Nothing, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
}
{
QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
country.SwitchToNextFile();
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
country.RemoveOptions(MapOptions::CarRouting);
TEST_EQUAL(MapOptions::Map, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
}
{
QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
country.SwitchToNextFile();
TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
country.RemoveOptions(MapOptions::Map);
TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
country.SwitchToNextFile();
TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetDownloadedFiles(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
TEST_EQUAL(MapOptions::CarRouting, country.GetDownloadedFilesOptions(), ());
}
}
} // namespace storage