Skip to content

Commit

Permalink
[new downloader] Review fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
@bykoianko @syershov
bykoianko authored and syershov committed Mar 23, 2016
1 parent e80c37d commit 89c87a1
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 74 deletions.
9 changes: 7 additions & 2 deletions 3party/jansson/myjansson.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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))
Expand Down
18 changes: 18 additions & 0 deletions 3party/jansson/myjansson.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Json
};

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);
Expand All @@ -51,4 +52,21 @@ void FromJSONObject(json_t * root, string const & field, vector<T> & result)

void FromJSONObjectOptionalField(json_t * root, string const & field, string & result);
void FromJSONObjectOptionalField(json_t * root, string const & field, json_int_t & result);

template <typename T>
void FromJSONObjectOptionalField(json_t * root, string const & field, vector<T> & 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
2 changes: 1 addition & 1 deletion map/framework.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
108 changes: 50 additions & 58 deletions storage/country.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,7 +43,8 @@ class StoreCountriesSingleMwms : public StoreSingleMwmInterface
{
}

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);
Expand All @@ -62,6 +64,13 @@ class StoreCountriesSingleMwms : public StoreSingleMwmInterface

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));
}

Expand All @@ -76,7 +85,8 @@ class StoreFile2InfoSingleMwms : public StoreSingleMwmInterface
public:
StoreFile2InfoSingleMwms(map<string, CountryInfo> & 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);
Expand All @@ -99,62 +109,48 @@ class StoreFile2InfoSingleMwms : public StoreSingleMwmInterface
};
} // 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<string> 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<string> affiliations;
my::FromJSONObjectOptionalField(node, "affiliations", affiliations);
for (auto const & affilationValue : affiliations)
store.InsertAffiliation(id, affilationValue);

json_int_t nodeSize;
my::FromJSONObjectOptionalField(node, "s", nodeSize);
ASSERT_LESS_OR_EQUAL(0, nodeSize, ());
// 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)
uint32_t mwmCounter = 0;
size_t mwmSize = 0;
vector<json_t *> children;
my::FromJSONObjectOptionalField(node, "g", children);
if (children.empty())
{
mwmCounter = 1; // It's a leaf. Any leaf contains one mwm.
mwmSize = nodeSize;
}
else
{
size_t const groupListSize = json_array_size(children);
for (size_t i = 0; i < groupListSize; ++i)
for (json_t * child : children)
{
json_t * j = json_array_get(children, i);
TMwmSubtreeAttrs const childAttr = LoadGroupSingleMwmsImpl(depth + 1, j, id, store);
TMwmSubtreeAttrs const childAttr = LoadGroupSingleMwmsImpl(depth + 1, child, id, store);
mwmCounter += childAttr.first;
mwmSize += childAttr.second;
}
}
else
{
mwmCounter = 1; // It's a leaf. Any leaf contains one mwm.
mwmSize = nodeSize;
}

if (addedNode != nullptr)
addedNode->SetSubtreeAttrs(mwmCounter, mwmSize);
Expand Down Expand Up @@ -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;
};

Expand All @@ -197,7 +194,8 @@ class StoreCountriesTwoComponentMwms : public StoreTwoComponentMwmInterface
{
}

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);
Expand All @@ -219,7 +217,8 @@ class StoreFile2InfoTwoComponentMwms : public StoreTwoComponentMwmInterface
public:
StoreFile2InfoTwoComponentMwms(map<string, CountryInfo> & 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)
Expand All @@ -231,7 +230,7 @@ class StoreFile2InfoTwoComponentMwms : public StoreTwoComponentMwmInterface
};
} // 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)
Expand All @@ -251,16 +250,10 @@ void LoadGroupTwoComponentMwmsImpl(int depth, json_t * node, TCountryId const &
store.Insert(file, static_cast<uint32_t>(mwmSize), static_cast<uint32_t>(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<json_t *> children;
my::FromJSONObjectOptionalField(node, "g", children);
for (json_t * child : children)
LoadGroupTwoComponentMwmsImpl(depth + 1, child, file, store);
}

bool LoadCountriesTwoComponentMwmsImpl(string const & jsonBuffer,
Expand All @@ -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<int64_t>(version)))
{
StoreCountriesSingleMwms store(countries, affiliations);
if (!LoadCountriesSingleMwmsImpl(jsonBuffer, store))
Expand All @@ -318,13 +311,12 @@ void LoadCountryFile2CountryInfo(string const & jsonBuffer, map<string, CountryI
{
ASSERT(id2info.empty(), ());

int64_t version = -1;
json_int_t version = -1;
try
{
my::Json root(jsonBuffer.c_str());
// version = json_integer_value(json_object_get(root.get(), "v"));
my::FromJSONObjectOptionalField(root.get(), "v", version);
isSingleMwm = version::IsSingleMwm(version);
isSingleMwm = version::IsSingleMwm(static_cast<int64_t>(version));
if (isSingleMwm)
{
StoreFile2InfoSingleMwms store(id2info);
Expand Down
14 changes: 11 additions & 3 deletions storage/country_tree.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ class CountryTree
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())
Expand Down Expand Up @@ -161,8 +170,7 @@ class CountryTree
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)
Expand Down
5 changes: 3 additions & 2 deletions storage/storage.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ class Storage

// |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.
Expand Down Expand Up @@ -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 <class ToDo>
void Storage::ForEachAncestorExceptForTheRoot(TCountryId const & countryId, ToDo && toDo) const
{
Expand Down
Loading

0 comments on commit 89c87a1

Please sign in to comment.