From a9a65aa01a77b0285df5426a37ef2ec76586c72d Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 15 Feb 2016 13:36:15 +0300 Subject: [PATCH] [new downloader] Returning a vector of parents of a node and their translation to local language. --- storage/country.hpp | 11 +++++++++- storage/simple_tree.hpp | 16 +++++++++++---- storage/storage.cpp | 45 ++++++++++++++++++++++++++--------------- storage/storage.hpp | 22 ++++++++++++-------- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/storage/country.hpp b/storage/country.hpp index 652648bc80..1c0e6ecf0d 100644 --- a/storage/country.hpp +++ b/storage/country.hpp @@ -25,7 +25,16 @@ namespace storage { using TMapping = map; -/// Serves as a proxy between GUI and downloaded files +/// This class keep all the information about a country in countre tree (TCountriesContainer). +/// It is guaranteed that every node represent a unique region has a unique |m_name| in country tree. +/// If several nodes have the same |m_name| they represents the same region. +/// It could happend in case of disputed territories. +/// That means that +/// * if several leaf nodes have the same |m_name| one mwm file corresponds +/// to all of them +/// * if several expandable (not leaf) nodes have the same |m_name| +/// the same hierarchy, the same set of mwm files and the same attributes correspond to all of them. +/// So in most cases it's enough to find the first inclusion of |Country| in country tree. class Country { friend class update::SizeUpdater; diff --git a/storage/simple_tree.hpp b/storage/simple_tree.hpp index d2ca16d729..23b810ef0e 100644 --- a/storage/simple_tree.hpp +++ b/storage/simple_tree.hpp @@ -77,14 +77,22 @@ public: /// @TODO(bykoianko) The complexity of the method is O(n). But the structure (tree) is built on the start of the program /// and then actively used on run time. This method (and class) should be redesigned to make the function work faster. /// A hash table is being planned to use. - SimpleTree const * const Find(T const & value) const + void Find(T const & value, vector const *> & found) const + { + if (IsEqual(m_value, value)) + found.push_back(this); + for (auto const & child : m_children) + child.Find(value, found); + } + + SimpleTree const * const FindFirst(T const & value) const { if (IsEqual(m_value, value)) return this; for (auto const & child : m_children) { - SimpleTree const * const found = child.Find(value); + SimpleTree const * const found = child.FindFirst(value); if (found != nullptr) return found; } @@ -96,14 +104,14 @@ public: /// When new countries.txt with unique ids will be added FindLeaf will be removed /// and Find will be used intead. /// @TODO(bykoianko) Remove this method on countries.txt update. - SimpleTree const * const FindLeaf(T const & value) const + SimpleTree const * const FindFirstLeaf(T const & value) const { if (IsEqual(m_value, value) && m_children.empty()) return this; // It's a leaf. for (auto const & child : m_children) { - SimpleTree const * const found = child.FindLeaf(value); + SimpleTree const * const found = child.FindFirstLeaf(value); if (found != nullptr) return found; } diff --git a/storage/storage.cpp b/storage/storage.cpp index 1c29aaa2a6..e00c65ef80 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -244,7 +244,7 @@ size_t Storage::GetDownloadedFilesCount() const TCountriesContainer const & NodeFromCountryId(TCountriesContainer const & root, TCountryId const & countryId) { - SimpleTree const * node = root.FindLeaf(Country(countryId)); + SimpleTree const * node = root.FindFirstLeaf(Country(countryId)); CHECK(node, ("Node with id =", countryId, "not found in country tree as a leaf.")); return *node; } @@ -256,7 +256,7 @@ Country const & Storage::CountryLeafByCountryId(TCountryId const & countryId) co Country const & Storage::CountryByCountryId(TCountryId const & countryId) const { - SimpleTree const * node = m_countries.Find(Country(countryId)); + SimpleTree const * node = m_countries.FindFirst(Country(countryId)); CHECK(node, ("Node with id =", countryId, "not found in country tree.")); return node->Value(); } @@ -288,7 +288,7 @@ string const & Storage::CountryName(TCountryId const & countryId) const bool Storage::IsCoutryIdInCountryTree(TCountryId const & countryId) const { - return m_countries.Find(Country(countryId)) != nullptr; + return m_countries.FindFirst(Country(countryId)) != nullptr; } TLocalAndRemoteSize Storage::CountrySizeInBytes(TCountryId const & countryId, MapOptions opt) const @@ -760,12 +760,12 @@ TCountryId Storage::FindCountryIdByFile(string const & name) const return TCountryId(name); } -TCountriesVec Storage::FindAllIndexesByFile(string const & name) const +TCountriesVec Storage::FindAllIndexesByFile(TCountryId const & name) const { // @TODO(bykoianko) This method should be rewritten. At list now name and the param of Find // have different types: string and TCountryId. TCountriesVec result; - if (m_countries.Find(Country(name))) + if (m_countries.FindFirst(Country(name))) result.push_back(name); return result; } @@ -995,7 +995,7 @@ TCountryId const Storage::GetRootId() const void Storage::GetChildren(TCountryId const & parent, TCountriesVec & childrenId) const { - TCountriesContainer const * const parentNode = m_countries.Find(Country(parent)); + TCountriesContainer const * const parentNode = m_countries.FindFirst(Country(parent)); if (parentNode == nullptr) { ASSERT(false, ("TCountryId =", parent, "not found in m_countries.")); @@ -1020,7 +1020,7 @@ void Storage::GetLocalRealMaps(TCountriesVec & localMaps) const void Storage::GetDownloadedChildren(TCountryId const & parent, TCountriesVec & localChildren) const { - TCountriesContainer const * const parentNode = m_countries.Find(Country(parent)); + TCountriesContainer const * const parentNode = m_countries.FindFirst(Country(parent)); if (parentNode == nullptr) { ASSERT(false, ("TCountryId =", parent, "not found in m_countries.")); @@ -1088,7 +1088,7 @@ bool Storage::DownloadNode(TCountryId const & countryId) { // @TODO(bykoianko) Before downloading it's necessary to check if file(s) has been downloaded. // If so, the method should be left with false. - TCountriesContainer const * const node = m_countries.Find(Country(countryId)); + TCountriesContainer const * const node = m_countries.FindFirst(Country(countryId)); CHECK(node, ()); node->ForEachInSubtree([this](TCountriesContainer const & descendantNode) { @@ -1105,7 +1105,7 @@ bool Storage::DeleteNode(TCountryId const & countryId) { // @TODO(bykoianko) Before deleting it's necessary to check if file(s) has been deleted. // If so, the method should be left with false. - TCountriesContainer const * const node = m_countries.Find(Country(countryId)); + TCountriesContainer const * const node = m_countries.FindFirst(Country(countryId)); CHECK(node, ()); node->ForEachInSubtree([this](TCountriesContainer const & descendantNode) { @@ -1143,8 +1143,13 @@ Status Storage::NodeStatus(TCountriesContainer const & node) const void Storage::GetNodeAttrs(TCountryId const & countryId, NodeAttrs & nodeAttrs) const { - TCountriesContainer const * const node = m_countries.Find(Country(countryId)); - CHECK(node, ()); + vector const *> nodes; + m_countries.Find(Country(countryId), nodes); + CHECK(!nodes.empty(), ()); + // If nodes.size() > 1 countryId corresponds to a disputed territories. + // In that case it's guaranteed that most of attributes are equal for + // each element of nodes. See Country class description for further details. + TCountriesContainer const * const node = nodes[0]; Country const & nodeValue = node->Value(); nodeAttrs.m_mwmCounter = nodeValue.GetSubtreeMwmCounter(); @@ -1153,11 +1158,19 @@ void Storage::GetNodeAttrs(TCountryId const & countryId, NodeAttrs & nodeAttrs) nodeAttrs.m_status = statusAndErr.status; nodeAttrs.m_error = statusAndErr.error; nodeAttrs.m_nodeLocalName = m_countryNameGetter(countryId); - nodeAttrs.m_parentCountryId = nodeValue.GetParent(); - if (nodeAttrs.m_parentCountryId.empty()) - nodeAttrs.m_parentLocalName.clear(); - else - nodeAttrs.m_parentLocalName = m_countryNameGetter(nodeAttrs.m_parentCountryId); + + nodeAttrs.m_parentInfo.clear(); + for (auto const & n : nodes) + { + Country const & nValue = n->Value(); + CountryIdAndName countryIdAndName; + countryIdAndName.m_id = nValue.GetParent(); + if (countryIdAndName.m_id.empty()) // The root case. + countryIdAndName.m_localName = string(); + else + countryIdAndName.m_localName = m_countryNameGetter(countryIdAndName.m_id); + nodeAttrs.m_parentInfo.emplace_back(move(countryIdAndName)); + } } void Storage::DoClickOnDownloadMap(TCountryId const & countryId) diff --git a/storage/storage.hpp b/storage/storage.hpp index ff548f910c..5788501ea6 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -18,6 +18,12 @@ namespace storage { +struct CountryIdAndName +{ + TCountryId m_id; + string m_localName; +}; + /// \brief Contains all properties for a node in the country tree. /// It's applicable for expandable and not expandable node id. struct NodeAttrs @@ -54,12 +60,12 @@ struct NodeAttrs /// a device locale. string m_nodeLocalName; - /// The name of the parent in a local language. That means the language dependent on - /// a device locale. For countries and for the root m_parentLocalName == "". - string m_parentLocalName; - - /// Node id of the parent of the node. For the root m_parentLocalName == "". - TCountryId m_parentCountryId; + /// Node id and local name of the parents of the node. + /// For the root m_parentInfo.m_id == "" and m_parentInfo.m_localName == "". + /// Locale language is a language set by Storage::SetLocale(). + /// Note. Most number of nodes have only one parent. But in case of a disputed territories + /// an mwm could have two or even more parents. See Country description for details. + vector m_parentInfo; /// A number for 0 to 99. It reflects downloading progress in case of /// downloading and updating mwm. If downloading or updating is not in progress @@ -367,7 +373,7 @@ public: /// @todo Temporary function to gel all associated indexes for the country file name. /// Will be removed in future after refactoring. - TCountriesVec FindAllIndexesByFile(string const & name) const; + TCountriesVec FindAllIndexesByFile(TCountryId const & name) const; void GetGroupAndCountry(TCountryId const & countryId, string & group, string & country) const; @@ -491,7 +497,7 @@ private: template void Storage::ForEachInSubtree(TCountryId const & root, ToDo && toDo) const { - TCountriesContainer const * const rootNode = m_countries.Find(Country(root)); + TCountriesContainer const * const rootNode = m_countries.FindFirst(Country(root)); if (rootNode == nullptr) { ASSERT(false, ("TCountryId =", root, "not found in m_countries."));