diff --git a/3party/jansson/myjansson.cpp b/3party/jansson/myjansson.cpp index 84ed12e0dd..f1de73f435 100644 --- a/3party/jansson/myjansson.cpp +++ b/3party/jansson/myjansson.cpp @@ -2,7 +2,12 @@ namespace my { -void FromJSON(json_t * root, string & result) { result = string(json_string_value(root)); } +void FromJSON(json_t * root, string & result) +{ + if (!json_is_string(root)) + MYTHROW(my::Json::Exception, ("The field must contain a json string.")); + result = string(json_string_value(root)); +} void FromJSONObject(json_t * root, string const & field, string & result) { @@ -54,7 +59,7 @@ void FromJSONObjectOptionalField(json_t * root, string const & field, string & r json_t * val = json_object_get(root, field.c_str()); if (!val) { - result = string(""); + result.clear(); return; } if (!json_is_string(val)) diff --git a/3party/jansson/myjansson.hpp b/3party/jansson/myjansson.hpp index 7b6b587368..b3e93169a1 100644 --- a/3party/jansson/myjansson.hpp +++ b/3party/jansson/myjansson.hpp @@ -30,6 +30,7 @@ public: }; void FromJSON(json_t * root, string & result); +void FromJSON(json_t * root, json_t *& value) { value = root; } void FromJSONObject(json_t * root, string const & field, string & result); void FromJSONObject(json_t * root, string const & field, strings::UniString & result); void FromJSONObject(json_t * root, string const & field, double & result); @@ -51,4 +52,21 @@ void FromJSONObject(json_t * root, string const & field, vector & result) void FromJSONObjectOptionalField(json_t * root, string const & field, string & result); void FromJSONObjectOptionalField(json_t * root, string const & field, json_int_t & result); + +template +void FromJSONObjectOptionalField(json_t * root, string const & field, vector & result) +{ + json_t * arr = json_object_get(root, field.c_str()); + if (!arr) + { + result.clear(); + return; + } + if (!json_is_array(arr)) + MYTHROW(my::Json::Exception, ("The field", field, "must contain a json array.")); + size_t sz = json_array_size(arr); + result.resize(sz); + for (size_t i = 0; i < sz; ++i) + FromJSON(json_array_get(arr, i), result[i]); +} } // namespace my diff --git a/map/framework.cpp b/map/framework.cpp index c88615b1e6..c4215f3718 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -285,10 +285,10 @@ void Framework::Migrate(bool keepDownloaded) Framework::Framework() : m_storage(platform::migrate::NeedMigrate() ? COUNTRIES_OBSOLETE_FILE : COUNTRIES_FILE) + , m_deferredCountryUpdate(milliseconds(300)) , m_bmManager(*this) , m_fixedSearchResults(0) , m_lastReportedCountry(kInvalidCountryId) - , m_deferredCountryUpdate(seconds(1)) { // Restore map style before classificator loading int mapStyle = MapStyleLight; diff --git a/storage/country.cpp b/storage/country.cpp index d987fe0937..850dafea8a 100644 --- a/storage/country.cpp +++ b/storage/country.cpp @@ -23,7 +23,8 @@ namespace class StoreSingleMwmInterface { public: - virtual Country * InsertToCountryTree(TCountryId const & id, uint32_t mapSize, int depth, + virtual ~StoreSingleMwmInterface() = default; + virtual Country * InsertToCountryTree(TCountryId const & id, uint32_t mapSize, size_t depth, TCountryId const & parent) = 0; virtual void InsertOldMwmMapping(TCountryId const & newId, TCountryId const & oldId) = 0; virtual void InsertAffiliation(TCountryId const & countryId, string const & affilation) = 0; @@ -42,7 +43,8 @@ public: { } - Country * InsertToCountryTree(TCountryId const & id, uint32_t mapSize, int depth, + // StoreSingleMwmInterface overrides: + Country * InsertToCountryTree(TCountryId const & id, uint32_t mapSize, size_t depth, TCountryId const & parent) override { Country country(id, parent); @@ -62,6 +64,13 @@ public: void InsertAffiliation(TCountryId const & countryId, string const & affilation) override { + auto const countryIdRange = m_affiliations.equal_range(countryId); + for (auto it = countryIdRange.first; it != countryIdRange.second; ++it) + { + if (it->second == affilation) + return; // No need key with the same value. It could happend in case of a disputable territory. + } + m_affiliations.insert(make_pair(countryId, affilation)); } @@ -76,7 +85,8 @@ class StoreFile2InfoSingleMwms : public StoreSingleMwmInterface public: StoreFile2InfoSingleMwms(map & file2info) : m_file2info(file2info) {} - Country * InsertToCountryTree(TCountryId const & id, uint32_t /* mapSize */, int /* depth */, + // StoreSingleMwmInterface overrides: + Country * InsertToCountryTree(TCountryId const & id, uint32_t /* mapSize */, size_t /* depth */, TCountryId const & /* parent */) override { CountryInfo info(id); @@ -99,38 +109,23 @@ public: }; } // namespace -TMwmSubtreeAttrs LoadGroupSingleMwmsImpl(int depth, json_t * node, TCountryId const & parent, +TMwmSubtreeAttrs LoadGroupSingleMwmsImpl(size_t depth, json_t * node, TCountryId const & parent, StoreSingleMwmInterface & store) { - uint32_t mwmCounter = 0; - size_t mwmSize = 0; - TCountryId id; my::FromJSONObject(node, "id", id); // Mapping two component (big) mwms to one componenst (small) ones. - json_t * oldIds = json_object_get(node, "old"); - if (oldIds) - { - size_t const oldListSize = json_array_size(oldIds); - for (size_t k = 0; k < oldListSize; ++k) - { - string oldIdValue = json_string_value(json_array_get(oldIds, k)); - store.InsertOldMwmMapping(id, oldIdValue); - } - } + vector oldIds; + my::FromJSONObjectOptionalField(node, "old", oldIds); + for (auto const & oldId : oldIds) + store.InsertOldMwmMapping(id, oldId); - // Mapping affiliations to one componenst (small) mwms. - json_t * affiliations = json_object_get(node, "affiliations"); - if (affiliations) - { - size_t const affiliationsSize = json_array_size(affiliations); - for (size_t k = 0; k < affiliationsSize; ++k) - { - string affilationValue = json_string_value(json_array_get(affiliations, k)); - store.InsertAffiliation(id, affilationValue); - } - } + // Mapping affiliations to one component (small) mwms. + vector affiliations; + my::FromJSONObjectOptionalField(node, "affiliations", affiliations); + for (auto const & affilationValue : affiliations) + store.InsertAffiliation(id, affilationValue); json_int_t nodeSize; my::FromJSONObjectOptionalField(node, "s", nodeSize); @@ -138,23 +133,24 @@ TMwmSubtreeAttrs LoadGroupSingleMwmsImpl(int depth, json_t * node, TCountryId co // We expect that mwm and routing files should be less than 2GB. Country * addedNode = store.InsertToCountryTree(id, nodeSize, depth, parent); - json_t * children = json_object_get(node, "g"); - if (children) - { - size_t const groupListSize = json_array_size(children); - for (size_t i = 0; i < groupListSize; ++i) - { - json_t * j = json_array_get(children, i); - TMwmSubtreeAttrs const childAttr = LoadGroupSingleMwmsImpl(depth + 1, j, id, store); - mwmCounter += childAttr.first; - mwmSize += childAttr.second; - } - } - else + uint32_t mwmCounter = 0; + size_t mwmSize = 0; + vector children; + my::FromJSONObjectOptionalField(node, "g", children); + if (children.empty()) { mwmCounter = 1; // It's a leaf. Any leaf contains one mwm. mwmSize = nodeSize; } + else + { + for (json_t * child : children) + { + TMwmSubtreeAttrs const childAttr = LoadGroupSingleMwmsImpl(depth + 1, child, id, store); + mwmCounter += childAttr.first; + mwmSize += childAttr.second; + } + } if (addedNode != nullptr) addedNode->SetSubtreeAttrs(mwmCounter, mwmSize); @@ -182,7 +178,8 @@ namespace class StoreTwoComponentMwmInterface { public: - virtual void Insert(string const & id, uint32_t mapSize, uint32_t /* routingSize */, int depth, + virtual ~StoreTwoComponentMwmInterface() = default; + virtual void Insert(string const & id, uint32_t mapSize, uint32_t /* routingSize */, size_t depth, TCountryId const & parent) = 0; }; @@ -197,7 +194,8 @@ public: { } - void Insert(string const & id, uint32_t mapSize, uint32_t routingSize, int depth, + // StoreTwoComponentMwmInterface overrides: + void Insert(string const & id, uint32_t mapSize, uint32_t routingSize, size_t depth, TCountryId const & parent) override { Country country(id, parent); @@ -219,7 +217,8 @@ class StoreFile2InfoTwoComponentMwms : public StoreTwoComponentMwmInterface public: StoreFile2InfoTwoComponentMwms(map & file2info) : m_file2info(file2info) {} - void Insert(string const & id, uint32_t mapSize, uint32_t /* routingSize */, int /* depth */, + // StoreTwoComponentMwmInterface overrides: + void Insert(string const & id, uint32_t mapSize, uint32_t /* routingSize */, size_t /* depth */, TCountryId const & /* parent */) override { if (mapSize == 0) @@ -231,7 +230,7 @@ public: }; } // namespace -void LoadGroupTwoComponentMwmsImpl(int depth, json_t * node, TCountryId const & parent, +void LoadGroupTwoComponentMwmsImpl(size_t depth, json_t * node, TCountryId const & parent, StoreTwoComponentMwmInterface & store) { // @TODO(bykoianko) After we stop supporting two component mwms (with routing files) @@ -251,16 +250,10 @@ void LoadGroupTwoComponentMwmsImpl(int depth, json_t * node, TCountryId const & store.Insert(file, static_cast(mwmSize), static_cast(routingSize), depth, parent); - json_t * children = json_object_get(node, "g"); - if (children) - { - size_t const groupListSize = json_array_size(children); - for (size_t i = 0; i < groupListSize; ++i) - { - json_t * j = json_array_get(children, i); - LoadGroupTwoComponentMwmsImpl(depth + 1, j, file, store); - } - } + vector children; + my::FromJSONObjectOptionalField(node, "g", children); + for (json_t * child : children) + LoadGroupTwoComponentMwmsImpl(depth + 1, child, file, store); } bool LoadCountriesTwoComponentMwmsImpl(string const & jsonBuffer, @@ -285,13 +278,13 @@ int64_t LoadCountries(string const & jsonBuffer, TCountryTree & countries, countries.Clear(); affiliations.clear(); - int64_t version = -1; + json_int_t version = -1; try { my::Json root(jsonBuffer.c_str()); my::FromJSONObject(root.get(), "v", version); - if (version::IsSingleMwm(version)) + if (version::IsSingleMwm(static_cast(version))) { StoreCountriesSingleMwms store(countries, affiliations); if (!LoadCountriesSingleMwmsImpl(jsonBuffer, store)) @@ -318,13 +311,12 @@ void LoadCountryFile2CountryInfo(string const & jsonBuffer, map(version)); if (isSingleMwm) { StoreFile2InfoSingleMwms store(id2info); diff --git a/storage/country_tree.hpp b/storage/country_tree.hpp index 902082a2e9..ef809d5281 100644 --- a/storage/country_tree.hpp +++ b/storage/country_tree.hpp @@ -47,7 +47,16 @@ public: TValue const & Value() const { return m_value; } TValue & Value() { return m_value; } - Node * AddAtDepth(int level, TValue const & value) + /// \brief Adds a node to a tree with root == |this|. + /// \param level is depth where a node with |value| will be add. |level| == 0 means |this|. + /// When the method is called the node |this| exists. So it's wrong call it with |level| == 0. + /// If |level| == 1 the node with |value| will be added as a child of this. + /// If |level| == 2 the node with |value| will be added as a child of the last added child of the |this|. + /// And so on. + /// \param value is a value of node which will be added. + /// \note This method does not let to add a node to an arbitrary place in the tree. + /// It's posible to add children only from "right side". + Node * AddAtDepth(size_t level, TValue const & value) { Node * node = this; while (--level > 0 && !node->m_children.empty()) @@ -161,8 +170,7 @@ public: return *m_countryTree; } - /// @return reference is valid only up to the next tree structure modification - TValue & AddAtDepth(int level, TValue const & value) + TValue & AddAtDepth(size_t level, TValue const & value) { Node * added = nullptr; if (level == 0) diff --git a/storage/storage.hpp b/storage/storage.hpp index f0d05b78cb..cf15258deb 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -193,6 +193,7 @@ private: // |m_affiliations| is a mapping from countryId to the list of names of // geographical objects (such as countries) that encompass this countryId. + // Note. Affiliations is inherited from ancestors of the countryId in country tree. // |m_affiliations| is filled during Storage initialization or during migration process. // It is filled with data of countries.txt (field "affiliations"). // Once filled |m_affiliations| is not changed. @@ -583,10 +584,10 @@ void Storage::ForEachInSubtreeAndInQueue(TCountryId const & root, ToDo && toDo) /// Calls functor |toDo| with signature /// void(const TCountryId const & parentId, TCountriesVec const & descendantCountryId) /// for each ancestor except for the main root of the tree. -/// |descendantsCountryId| is a vector of country id of descendats of |parentId|. /// Note. In case of disputable territories several nodes with the same name may be /// present in the country tree. In that case ForEachAncestorExceptForTheRoot calls -/// |toDo| for parents of each way to the root in the country tree. +/// |toDo| for parents of each way to the root in the country tree. In case of diamond +/// trees toDo is called for common part of ways to the root only once. template void Storage::ForEachAncestorExceptForTheRoot(TCountryId const & countryId, ToDo && toDo) const { diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 75cd0ede48..7503ca4528 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -101,6 +101,10 @@ string const kSingleMwmCountriesTxt = string(R"({ "s": 1234, "old": [ "Country1" + ], + "affiliations": + [ + "Stepchild Land1", "Stepchild Land2" ] }, { @@ -108,8 +112,16 @@ string const kSingleMwmCountriesTxt = string(R"({ "s": 1111, "old": [ "Country1" + ], + "affiliations": + [ + "Child Land1" ] } + ], + "affiliations": + [ + "Parent Land1" ] }, { @@ -127,8 +139,16 @@ string const kSingleMwmCountriesTxt = string(R"({ "s": 1234, "old": [ "Country2" + ], + "affiliations": + [ + "Stepchild Land1", "Stepchild Land2" ] } + ], + "affiliations": + [ + "Parent Land2" ] } ]})"); @@ -990,18 +1010,34 @@ UNIT_TEST(StorageTest_GetChildren) UNIT_TEST(StorageTest_GetAffiliations) { Storage storage(kSingleMwmCountriesTxt, make_unique()); - string const abkhaziaId = "Abkhazia"; - TMappingAffiliations const expectedAffiliations = {{abkhaziaId.c_str(), "Georgia"}, - {abkhaziaId.c_str(), "Russia"}, - {abkhaziaId.c_str(), "Europe"}}; - auto const rangeResultAffiliations = storage.GetAffiliations().equal_range(abkhaziaId); - TMappingAffiliations const resultAffiliations(rangeResultAffiliations.first, - rangeResultAffiliations.second); - TEST(expectedAffiliations == resultAffiliations, ()); + string const abkhaziaId = "Abkhazia"; + TMappingAffiliations const expectedAffiliations1 = {{abkhaziaId.c_str(), "Georgia"}, + {abkhaziaId.c_str(), "Russia"}, + {abkhaziaId.c_str(), "Europe"}}; + auto const rangeResultAffiliations1 = storage.GetAffiliations().equal_range(abkhaziaId); + TMappingAffiliations const resultAffiliations1(rangeResultAffiliations1.first, + rangeResultAffiliations1.second); + TEST(expectedAffiliations1 == resultAffiliations1, ()); auto const rangeResultNoAffiliations = storage.GetAffiliations().equal_range("Algeria"); TEST(rangeResultNoAffiliations.first == rangeResultNoAffiliations.second, ()); + + // Affiliation inheritance. + string const disputableId = "Disputable Territory"; + auto const rangeResultAffiliations2 = storage.GetAffiliations().equal_range(disputableId); + TMappingAffiliations const resultAffiliations2(rangeResultAffiliations2.first, + rangeResultAffiliations2.second); + TMappingAffiliations const expectedAffiliations2 = {{disputableId.c_str(), "Stepchild Land1"}, + {disputableId.c_str(), "Stepchild Land2"}}; + TEST(expectedAffiliations2 == resultAffiliations2, ()); + + string const indisputableId = "Indisputable Territory Of Country1"; + auto const rangeResultAffiliations3 = storage.GetAffiliations().equal_range(indisputableId); + TMappingAffiliations const resultAffiliations3(rangeResultAffiliations3.first, + rangeResultAffiliations3.second); + TMappingAffiliations const expectedAffiliations3 = {{indisputableId.c_str(), "Child Land1"}}; + TEST(expectedAffiliations3 == resultAffiliations3, ()); } UNIT_TEST(StorageTest_HasCountryId)