From 370afcc67549c9504dad9a8c44168ca415b7b49b Mon Sep 17 00:00:00 2001 From: Alex Zolotarev Date: Mon, 28 Oct 2013 01:00:39 +0300 Subject: [PATCH] Added versioning support to guides and fixed loading bug, when older json was loaded from Writable dir instead of new one from Resources --- data/android-guides.json | 3 +- data/ios-guides.json | 1 + storage/guides.cpp | 87 ++++++++++++++++++-------- storage/guides.hpp | 29 ++++++--- storage/storage_tests/guides_tests.cpp | 64 ++++++++++++------- 5 files changed, 125 insertions(+), 59 deletions(-) diff --git a/data/android-guides.json b/data/android-guides.json index d567f7d1b4..9e4f50bc43 100644 --- a/data/android-guides.json +++ b/data/android-guides.json @@ -1,5 +1,6 @@ { -"UK":{ + "version":2, + "UK":{ "url":"https://play.google.com/store/apps/details?id=com.guidewithme.uk", "appId":"com.guidewithme.uk", "adTitles":{ diff --git a/data/ios-guides.json b/data/ios-guides.json index 349a84bfa9..c91bc801f0 100644 --- a/data/ios-guides.json +++ b/data/ios-guides.json @@ -1,4 +1,5 @@ { + "version":2, "UK":{ "url":"itms-apps://itunes.apple.com/app/id687855665", "appId":"guidewithme-uk://", diff --git a/storage/guides.cpp b/storage/guides.cpp index 398a97087f..7f0531b482 100644 --- a/storage/guides.cpp +++ b/storage/guides.cpp @@ -86,18 +86,42 @@ string DebugPrint(GuideInfo const & r) bool GuidesManager::RestoreFromFile() { + int resourcesVersion = -1, downloadedVersion = -1; + MapT fromResources, fromDownloaded; + Platform & pl = GetPlatform(); try { - ReaderPtr r(GetPlatform().GetReader(GetDataFileName())); - string data; - r.ReadAsString(data); - return ValidateAndParseGuidesData(data); + string json; + ReaderPtr(pl.GetReader(GetDataFileName(), "r")).ReadAsString(json); + resourcesVersion = ValidateAndParseGuidesData(json, fromResources); } - catch (RootException const & ex) + catch (RootException const &) { - LOG(LWARNING, (ex.Msg())); - return false; } + try + { + string json; + ReaderPtr(pl.GetReader(GetDataFileName(), "w")).ReadAsString(json); + downloadedVersion = ValidateAndParseGuidesData(json, fromDownloaded); + } + catch (RootException const &) + { + } + + if (downloadedVersion > resourcesVersion) + { + m_version = downloadedVersion; + m_file2Info.swap(fromDownloaded); + return true; + } + else if (resourcesVersion > downloadedVersion || resourcesVersion >= 0) + { + m_version = resourcesVersion; + m_file2Info.swap(fromResources); + return true; + } + LOG(LWARNING, ("Guides descriptions were not loaded")); + return false; } void GuidesManager::UpdateGuidesData() @@ -129,7 +153,7 @@ bool GuidesManager::GetGuideInfo(string const & id, GuideInfo & appInfo) const return false; } -string GuidesManager::GetGuidesDataUrl() const +string GuidesManager::GetGuidesDataUrl() { #ifdef DEBUG return "http://application.server/rest/guides/debug/" + GetDataFileName(); @@ -138,12 +162,12 @@ string GuidesManager::GetGuidesDataUrl() const #endif } -string GuidesManager::GetDataFileName() const +string GuidesManager::GetDataFileName() { return OMIM_OS_NAME "-" GUIDES_DATA_FILE_SUFFIX; } -string GuidesManager::GetDataFileFullPath() const +string GuidesManager::GetDataFileFullPath() { return GetPlatform().WritableDir() + GetDataFileName(); } @@ -153,10 +177,15 @@ void GuidesManager::OnFinish(downloader::HttpRequest & request) if (request.Status() == downloader::HttpRequest::ECompleted) { string const & data = request.Data(); - if (ValidateAndParseGuidesData(data) && !m_file2Info.empty()) + // Sanity check if we forgot to update json version on servers + MapT tmpGuides; + int const downloadedVersion = ValidateAndParseGuidesData(data, tmpGuides); + if (downloadedVersion > m_version && !m_file2Info.empty()) { + // Load into the memory even if we fail to save it later + m_version = downloadedVersion; + m_file2Info.swap(tmpGuides); // Save only if file was parsed successfully and info exists! - string const path = GetDataFileFullPath(); try { @@ -181,10 +210,11 @@ void GuidesManager::OnFinish(downloader::HttpRequest & request) m_httpRequest.reset(); } -bool GuidesManager::ValidateAndParseGuidesData(string const & jsonData) +int GuidesManager::ValidateAndParseGuidesData(string const & jsonData, MapT & guidesInfo) { - typedef my::Json::Exception ParseException; - + guidesInfo.clear(); + // 0 means "version" key is absent in json + int version = 0; try { my::Json root(jsonData.c_str()); @@ -196,15 +226,20 @@ bool GuidesManager::ValidateAndParseGuidesData(string const & jsonData) json_t * entry = json_object_iter_value(iter); if (entry) { - GuideInfo info(entry); - if (info.IsValid()) + if (json_is_integer(entry) && string(json_object_iter_key(iter)) == "version") + version = json_integer_value(entry); + else { - json_t * keys = json_object_get(entry, "keys"); - for (size_t i = 0; i < json_array_size(keys); ++i) + GuideInfo info(entry); + if (info.IsValid()) { - char const * key = json_string_value(json_array_get(keys, i)); - if (key) - temp[key] = info; + json_t * keys = json_object_get(entry, "keys"); + for (size_t i = 0; i < json_array_size(keys); ++i) + { + char const * key = json_string_value(json_array_get(keys, i)); + if (key) + temp[key] = info; + } } } } @@ -212,13 +247,13 @@ bool GuidesManager::ValidateAndParseGuidesData(string const & jsonData) iter = json_object_iter_next(root.get(), iter); } - m_file2Info.swap(temp); - return true; + guidesInfo.swap(temp); + return version; } - catch (ParseException const & ex) + catch (my::Json::Exception const & ex) { LOG(LWARNING, ("Failed to parse guides data:", ex.Msg())); - return false; + return -1; } } diff --git a/storage/guides.hpp b/storage/guides.hpp index 6f83968395..0120130147 100644 --- a/storage/guides.hpp +++ b/storage/guides.hpp @@ -9,7 +9,6 @@ #include "../../3party/jansson/jansson_handle.hpp" - namespace guides { @@ -35,9 +34,15 @@ public: string DebugPrint(GuideInfo const & r); - class GuidesManager : private noncopyable { + /// For unit tests visibility + friend void UnitTest_Guides_SmokeTest(); + friend void UnitTest_Guides_CorrectlyParseData(); + friend void UnitTest_Guides_SaveRestoreFromFile(); + friend void UnitTest_Guides_ComplexNames(); + friend void UnitTest_Guides_CheckDataFiles(); + /// @name Guides managment //@{ public: @@ -53,20 +58,24 @@ public: void SetWasAdvertised(string const & appID); //@} - bool ValidateAndParseGuidesData(string const & jsonData); - - /// Public visibility for unit tests only! - string GetDataFileFullPath() const; - private: + typedef map MapT; + + /// @param[in] guidesInfo filled with correct info about guides on success + /// @return -1 if failed to parse or json version number otherwise. 0 means version is absent in json. + int ValidateAndParseGuidesData(string const & jsonData, MapT & guidesInfo); + void OnFinish(downloader::HttpRequest & request); - string GetGuidesDataUrl() const; - string GetDataFileName() const; + static string GetGuidesDataUrl(); + static string GetDataFileName(); + static string GetDataFileFullPath(); /// Map from mwm file name (key) to guide info. - typedef map MapT; MapT m_file2Info; + /// Loaded Guides json version + int m_version; + scoped_ptr m_httpRequest; //@} }; diff --git a/storage/storage_tests/guides_tests.cpp b/storage/storage_tests/guides_tests.cpp index c8ea290045..41acf421ef 100644 --- a/storage/storage_tests/guides_tests.cpp +++ b/storage/storage_tests/guides_tests.cpp @@ -7,13 +7,17 @@ #include "../../coding/file_writer.hpp" #include "../../coding/file_reader.hpp" +// Needed for friend functions to work correctly +namespace guides +{ UNIT_TEST(Guides_SmokeTest) { - guides::GuidesManager manager; - guides::GuideInfo info; + GuidesManager manager; + GuideInfo info; - string str = "{ \"UK\": {" + string const str = "{ \"version\": 2," + "\"UK\": {" "\"url\": \"https://itunes.apple.com/app/uk-travel-guide-with-me/id687855665\"," "\"appId\": \"com.guideswithme.uk\"," "\"adTitles\": { \"en\": \"UK title\", \"ru\": \"ВБ заголовок\" }," @@ -21,11 +25,14 @@ UNIT_TEST(Guides_SmokeTest) "\"keys\": [ \"Guernsey\", \"Mercy\" ]" "} }"; - TEST(!manager.ValidateAndParseGuidesData("invalidtest"), ()); - TEST(manager.ValidateAndParseGuidesData("{}"), ()); + GuidesManager::MapT data; + TEST_EQUAL(-1, manager.ValidateAndParseGuidesData("invalidtest", data), ()); + TEST_EQUAL(0, manager.ValidateAndParseGuidesData("{}", data), ()); + manager.m_file2Info.swap(data); TEST(!manager.GetGuideInfo("Guernsey", info), ()); - TEST(manager.ValidateAndParseGuidesData(str), ()); + TEST_EQUAL(2, manager.ValidateAndParseGuidesData(str, data), ()); + manager.m_file2Info.swap(data); TEST(manager.GetGuideInfo("Guernsey", info), ()); TEST(info.IsValid(), ()); @@ -36,7 +43,7 @@ UNIT_TEST(Guides_SmokeTest) TEST_EQUAL(info.GetAdTitle("zh"), "UK title", ()); TEST_EQUAL(info.GetAdMessage("zh"), "UK message", ()); - guides::GuideInfo info1; + GuideInfo info1; TEST(manager.GetGuideInfo("Mercy", info1), ()); TEST(info1.IsValid(), ()); TEST_EQUAL(info, info1, ()); @@ -46,10 +53,11 @@ UNIT_TEST(Guides_SmokeTest) UNIT_TEST(Guides_CorrectlyParseData) { - guides::GuidesManager manager; - guides::GuideInfo info; + GuidesManager manager; + GuideInfo info; - string strLondonIsle = "{ \"UK_1\": {" + string strLondonIsle = "{ \"version\": 2," + "\"UK_1\": {" "\"url\": \"https://itunes.apple.com/app/uk-travel-guide-with-me/id687855665\"," "\"appId\": \"com.guideswithme.uk\"," "\"adTitles\": { \"en\": \"UK title\", \"ru\": \"ВБ заголовок\" }," @@ -66,7 +74,9 @@ UNIT_TEST(Guides_CorrectlyParseData) string validKeys[] = { "London", "Isle of Man" }; string invalidKeys[] = { "london", "Lond", "Isle", "Man" }; - TEST(manager.ValidateAndParseGuidesData(strLondonIsle), ()); + GuidesManager::MapT data; + TEST_EQUAL(2, manager.ValidateAndParseGuidesData(strLondonIsle, data), ()); + manager.m_file2Info.swap(data); for (size_t i = 0; i < ARRAY_SIZE(validKeys); ++i) { TEST(manager.GetGuideInfo(validKeys[i], info), (i)); @@ -93,10 +103,11 @@ UNIT_TEST(Guides_CorrectlyParseData) UNIT_TEST(Guides_ComplexNames) { - guides::GuidesManager manager; - guides::GuideInfo info; + GuidesManager manager; + GuideInfo info; - string strLondonIsle = "{\"Côte_d'Ivoire\": {" + string const strLondonIsle = "{ \"version\": 123456," + "\"Côte_d'Ivoire\": {" "\"url\": \"https://itunes.apple.com/app/uk-travel-guide-with-me/id687855665\"," "\"appId\": \"com.guideswithme.uk\"," "\"adTitles\": { \"en\": \"Côte_d'Ivoire title\", \"ru\": \"КДВар заголовок\" }," @@ -113,7 +124,9 @@ UNIT_TEST(Guides_ComplexNames) string validKeys[] = { "Côte_d'Ivoire", "Беларусь" }; string invalidKeys[] = { "Не Беларусь", "Côte_d'IvoireCôte_d'IvoireCôte_d'Ivoire" }; - TEST(manager.ValidateAndParseGuidesData(strLondonIsle), ()); + GuidesManager::MapT data; + TEST_EQUAL(123456, manager.ValidateAndParseGuidesData(strLondonIsle, data), ()); + manager.m_file2Info.swap(data); for (size_t i = 0; i < ARRAY_SIZE(validKeys); ++i) { TEST(manager.GetGuideInfo(validKeys[i], info), (i)); @@ -126,10 +139,11 @@ UNIT_TEST(Guides_ComplexNames) UNIT_TEST(Guides_SaveRestoreFromFile) { - guides::GuidesManager manager; - guides::GuideInfo info; + GuidesManager manager; + GuideInfo info; - string strLondonIsle = "{ \"UK_1\": {" + string const strLondonIsle = "{ \"version\": 2," + "\"UK_1\": {" "\"url\": \"https://itunes.apple.com/app/uk-travel-guide-with-me/id687855665\"," "\"appId\": \"com.guideswithme.uk\"," "\"adTitles\": { \"en\": \"UK title\", \"ru\": \"ВБ заголовок\" }," @@ -143,7 +157,9 @@ UNIT_TEST(Guides_SaveRestoreFromFile) "\"keys\": [ \"Isle of Man\" ]" "} }"; - TEST(manager.ValidateAndParseGuidesData(strLondonIsle), ()); + GuidesManager::MapT data; + TEST_EQUAL(2, manager.ValidateAndParseGuidesData(strLondonIsle, data), ()); + manager.m_file2Info.swap(data); string const path = manager.GetDataFileFullPath(); { @@ -176,8 +192,8 @@ UNIT_TEST(Guides_CheckDataFiles) string const path = GetPlatform().WritableDir(); string arr[] = { "android-guides.json", "ios-guides.json" }; - guides::GuidesManager manager; - guides::GuideInfo info; + GuidesManager manager; + GuideInfo info; for (size_t i = 0; i < ARRAY_SIZE(arr); ++i) { @@ -185,8 +201,12 @@ UNIT_TEST(Guides_CheckDataFiles) string str; reader.ReadAsString(str); - TEST(manager.ValidateAndParseGuidesData(str), ()); + GuidesManager::MapT data; + TEST_LESS(0, manager.ValidateAndParseGuidesData(str, data), ()); + manager.m_file2Info.swap(data); TEST(manager.GetGuideInfo("UK_England", info), ()); TEST(info.IsValid(), ()); } } + +} // namespace guides