From 7d0b33a6a37bc1cf0018f91f1fc470337ed0c69b Mon Sep 17 00:00:00 2001 From: Alex Zolotarev Date: Thu, 6 Oct 2011 01:23:17 +0300 Subject: [PATCH] Added simple url change on error support --- defines.hpp | 6 --- iphone/Maps/Platform/IPhoneDownload.h | 6 ++- iphone/Maps/Platform/IPhoneDownload.mm | 57 ++++++++++++++--------- platform/download_manager.hpp | 3 +- platform/platform_tests/download_test.cpp | 24 ++++++++-- platform/qt_download.cpp | 33 +++++++++---- platform/qt_download.hpp | 5 +- storage/storage.cpp | 4 +- 8 files changed, 91 insertions(+), 47 deletions(-) diff --git a/defines.hpp b/defines.hpp index cb98329964..a1011cc161 100644 --- a/defines.hpp +++ b/defines.hpp @@ -13,12 +13,6 @@ #define DATA_UPDATE_FILE "maps.update" #define BINARY_UPDATE_FILE "binary.update" -#ifdef OMIM_PRODUCTION - #define UPDATE_BASE_URL "http://data.mapswithme.com/" -#else - #define UPDATE_BASE_URL "http://svobodu404popugajam.mapswithme.com:34568/maps/" -#endif - #define WORLD_FILE_NAME "World" #define WORLD_COASTS_FILE_NAME "WorldCoasts" diff --git a/iphone/Maps/Platform/IPhoneDownload.h b/iphone/Maps/Platform/IPhoneDownload.h index 48cfbd77ac..45e7f297bb 100644 --- a/iphone/Maps/Platform/IPhoneDownload.h +++ b/iphone/Maps/Platform/IPhoneDownload.h @@ -1,19 +1,21 @@ #import #include "../../../platform/download_manager.hpp" +#include "../../../platform/url_generator.hpp" #include "../../../std/string.hpp" @interface IPhoneDownload : NSObject { HttpStartParams m_params; + + string m_currentUrl; + UrlGenerator m_urlGenerator; FILE * m_file; /// stores information from the server, can be zero int64_t m_projectedFileSize; NSURLConnection * m_connection; - NSInteger m_retryCounter; - string m_receivedBuffer; } diff --git a/iphone/Maps/Platform/IPhoneDownload.mm b/iphone/Maps/Platform/IPhoneDownload.mm index 88c7669b49..85934ff041 100644 --- a/iphone/Maps/Platform/IPhoneDownload.mm +++ b/iphone/Maps/Platform/IPhoneDownload.mm @@ -19,15 +19,14 @@ #endif #define TIMEOUT_IN_SECONDS 15.0 -#define MAX_AUTOMATIC_RETRIES 3 -string GetDeviceUid() +static string GetDeviceUid() { NSString * uid = [[UIDevice currentDevice] uniqueIdentifier]; return [uid UTF8String]; } -string GetMacAddress() +static string GetMacAddress() { string result; // get wifi mac addr @@ -53,7 +52,7 @@ string GetMacAddress() return result; } -string GetUniqueHashedId() +static string GetUniqueHashedId() { // generate sha2 hash for mac address string const hash = sha2::digest256(GetMacAddress() + GetDeviceUid(), false); @@ -66,6 +65,11 @@ string GetUniqueHashedId() return base64::encode(xoredHash); } +static bool NeedToGenerateUrl(string const & url) +{ + return url.find("http://") != 0 && url.find("https://") != 0; +} + @implementation IPhoneDownload - (string const &) Url @@ -101,7 +105,7 @@ string GetUniqueHashedId() { // Create the request. NSMutableURLRequest * request = - [NSMutableURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithUTF8String:m_params.m_url.c_str()]] + [NSMutableURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithUTF8String:m_currentUrl.c_str()]] cachePolicy:NSURLRequestReloadIgnoringLocalCacheData timeoutInterval:TIMEOUT_IN_SECONDS]; if (m_file) { @@ -126,7 +130,7 @@ string GetUniqueHashedId() [request setHTTPBody:postData]; } // set user-agent with unique client id only for mapswithme requests - if (m_params.m_url.find("mapswithme.com") != string::npos) + if (m_currentUrl.find("mapswithme.com") != string::npos) { static string const uid = GetUniqueHashedId(); [request addValue:[NSString stringWithUTF8String: uid.c_str()] forHTTPHeaderField:@"User-Agent"]; @@ -138,13 +142,11 @@ string GetUniqueHashedId() { m_params = params; - m_retryCounter = 0; - if (!params.m_fileToSave.empty()) { // try to create file first string const tmpFile = m_params.m_fileToSave + DOWNLOADING_FILE_EXTENSION; - m_file = fopen(tmpFile.c_str(), params.m_useResume ? "ab" : "wb"); + m_file = fopen(tmpFile.c_str(), "ab"); if (m_file == 0) { NSLog(@"Error opening %s file for download: %s", tmpFile.c_str(), strerror(errno)); @@ -160,13 +162,16 @@ string GetUniqueHashedId() return NO; } } + + if (NeedToGenerateUrl(m_params.m_url)) + m_currentUrl = m_urlGenerator.PopNextUrl() + m_params.m_url; // create the connection with the request and start loading the data m_connection = [[NSURLConnection alloc] initWithRequest:[self CreateRequest] delegate:self]; if (m_connection == 0) { - NSLog(@"Can't create connection for url %s", params.m_url.c_str()); + NSLog(@"Can't create connection for url %s", m_currentUrl.c_str()); // notify observer about error and exit if (m_params.m_finish) { @@ -255,26 +260,34 @@ string GetUniqueHashedId() } } +- (BOOL) shouldRetry: (NSError *)error +{ + NSInteger const code = [error code]; + return (code < 0 || code == 401 || code == 403 || code == 404) && NeedToGenerateUrl(m_params.m_url); +} + - (void) connection: (NSURLConnection *)connection didFailWithError: (NSError *)error { // inform the user - NSLog(@"Connection failed for url %s\n%@", m_params.m_url.c_str(), [error localizedDescription]); + NSLog(@"Connection failed for url %s\n%@", m_currentUrl.c_str(), [error localizedDescription]); // retry connection if it's network-specific error - if ([error code] < 0 && ++m_retryCounter <= MAX_AUTOMATIC_RETRIES) + if ([self shouldRetry:error]) { - [m_connection release]; - // create the connection with the request and start loading the data - m_connection = [[NSURLConnection alloc] initWithRequest:[self CreateRequest] delegate:self]; - - if (m_connection) + string const newUrl = m_urlGenerator.PopNextUrl(); + if (!newUrl.empty()) { - NSLog(@"Retrying %d time", m_retryCounter); - return; // successfully restarted connection - } + m_currentUrl = newUrl + m_params.m_url; + [m_connection release]; + // create the connection with the request and start loading the data + m_connection = [[NSURLConnection alloc] initWithRequest:[self CreateRequest] delegate:self]; - NSLog(@"Can't retry connection"); - // notify observer about error and exit after this if-block + if (m_connection) + return; // successfully restarted connection + + NSLog(@"Can't retry connection"); + // notify observer about error and exit after this if-block + } } if (m_file) diff --git a/platform/download_manager.hpp b/platform/download_manager.hpp index e7b12a292a..13294aabc7 100644 --- a/platform/download_manager.hpp +++ b/platform/download_manager.hpp @@ -42,7 +42,6 @@ struct HttpStartParams string m_fileToSave; HttpFinishedCallbackT m_finish; HttpProgressCallbackT m_progress; - bool m_useResume; string m_contentType; string m_postData; //!< if not empty, send POST instead of GET }; @@ -54,6 +53,8 @@ class DownloadManager public: virtual ~DownloadManager() {} + /// By default, http resume feature is used for requests which contains out file + /// If url doesn't contain http:// or https:// Url_Generator is used for base server url virtual void HttpRequest(HttpStartParams const & params) = 0; /// @note Doesn't notifies clients on canceling! virtual void CancelDownload(string const & url) = 0; diff --git a/platform/platform_tests/download_test.cpp b/platform/platform_tests/download_test.cpp index 0a01c68b53..226ef56428 100644 --- a/platform/platform_tests/download_test.cpp +++ b/platform/platform_tests/download_test.cpp @@ -225,7 +225,6 @@ UNIT_TEST(DownloadResume) params.m_fileToSave = TEST_FILE_NAME1; params.m_finish = bind(&DlObserver::OnDownloadFinished, &observer1, _1); params.m_progress = bind(&DlObserver::OnDownloadProgress, &observer1, _1); - params.m_useResume = false; gMgr.HttpRequest(params); WAIT_FOR_ASYNC_DOWNLOAD; TEST_EQUAL( observer1.m_result[0].m_error, EHttpDownloadOk, () ); @@ -235,7 +234,6 @@ UNIT_TEST(DownloadResume) params.m_fileToSave = TEST_FILE_NAME2; params.m_finish = bind(&DlObserver::OnDownloadFinished, &observer2, _1); params.m_progress = bind(&DlObserver::OnDownloadProgress, &observer2, _1); - params.m_useResume = false; gMgr.HttpRequest(params); WAIT_FOR_ASYNC_DOWNLOAD; TEST_EQUAL( observer2.m_result[0].m_error, EHttpDownloadOk, () ); @@ -256,7 +254,6 @@ UNIT_TEST(DownloadResume) params.m_fileToSave = TEST_FILE_NAME1; params.m_finish = bind(&DlObserver::OnDownloadFinished, &observer3, _1); params.m_progress = bind(&DlObserver::OnDownloadProgress, &observer3, _1); - params.m_useResume = true; gMgr.HttpRequest(params); WAIT_FOR_ASYNC_DOWNLOAD; TEST_EQUAL( observer3.m_result[0].m_error, EHttpDownloadOk, () ); @@ -294,6 +291,27 @@ UNIT_TEST(DownloadAbsentFile) FileWriter::DeleteFileX(TEST_ABSENT_FILE_NAME); } +UNIT_TEST(DownloadUsingUrlGenerator) +{ + size_t const NUM = 1; + DlObserver observer; + + string const LOCAL_FILE = "unit_test.txt"; + + HttpStartParams params; + params.m_url = LOCAL_FILE; + params.m_fileToSave = LOCAL_FILE; + params.m_finish = bind(&DlObserver::OnDownloadFinished, &observer, _1); + params.m_progress = bind(&DlObserver::OnDownloadProgress, &observer, _1); + gMgr.HttpRequest(params); + WAIT_FOR_ASYNC_DOWNLOAD; + + TEST_NOT_EQUAL( observer.m_result[0].m_error, EHttpDownloadFileNotFound, () ); + TEST( GetPlatform().IsFileExists(LOCAL_FILE), () ); + + FileWriter::DeleteFileX(LOCAL_FILE); +} + // only on Windows files are actually locked by system #ifdef OMIM_OS_WINDOWS UNIT_TEST(DownloadLockedFile) diff --git a/platform/qt_download.cpp b/platform/qt_download.cpp index 25c76c6f3e..94f3df8ef0 100644 --- a/platform/qt_download.cpp +++ b/platform/qt_download.cpp @@ -91,16 +91,21 @@ static QString UserAgent() return userAgent; } +static bool NeedToUseUrlGenerator(string const & url) +{ + return url.find("http://") != 0 && url.find("https://") != 0; +} + QtDownload::QtDownload(QtDownloadManager & manager, HttpStartParams const & params) : QObject(&manager), m_currentUrl(params.m_url.c_str()), m_reply(0), - m_retryCounter(0), m_params(params) + m_params(params) { // if we need to save server response to the file... if (!m_params.m_fileToSave.empty()) { // use temporary file for download m_file.setFileName((m_params.m_fileToSave + DOWNLOADING_FILE_EXTENSION).c_str()); - if (!m_file.open(m_params.m_useResume ? QIODevice::Append : QIODevice::WriteOnly)) + if (!m_file.open(QIODevice::Append)) { QString const err = m_file.errorString(); LOG(LERROR, ("Can't open file while downloading", qPrintable(m_file.fileName()), qPrintable(err))); @@ -120,6 +125,10 @@ QtDownload::QtDownload(QtDownloadManager & manager, HttpStartParams const & para // url acts as a key to find this download by QtDownloadManager::findChild(url) setObjectName(m_params.m_url.c_str()); + // if url doesn't contain valid http:// or https:// we use UrlGenerator + if (NeedToUseUrlGenerator(m_params.m_url)) + m_currentUrl = (m_urlGenerator.PopNextUrl() + m_params.m_url).c_str(); + StartRequest(); } @@ -141,14 +150,14 @@ QtDownload::~QtDownload() m_file.remove(); } } - LOG(LDEBUG, (m_params.m_url)); + LOG(LDEBUG, (m_currentUrl.toString().toLocal8Bit())); } void QtDownload::StartRequest() { QNetworkRequest httpRequest(m_currentUrl); // set user-agent with unique client id only for mapswithme requests - if (m_params.m_url.find("mapswithme.com") != string::npos) + if (m_currentUrl.host().contains("mapswithme.com")) httpRequest.setRawHeader("User-Agent", UserAgent().toAscii()); if (m_file.isOpen()) { @@ -189,11 +198,17 @@ void QtDownload::OnHttpFinished() QNetworkReply::NetworkError const netError = m_reply->error(); if (netError) { - if (netError <= QNetworkReply::UnknownNetworkError && ++m_retryCounter <= MAX_AUTOMATIC_RETRIES) - { // try one more time - m_reply->deleteLater(); - StartRequest(); - return; + if (netError <= QNetworkReply::UnknownNetworkError && NeedToUseUrlGenerator(m_params.m_url)) + { + string const nextUrl = m_urlGenerator.PopNextUrl(); + if (!nextUrl.empty()) + { + // try one more time + m_reply->deleteLater(); + m_currentUrl = (nextUrl + m_params.m_url).c_str(); + StartRequest(); + return; + } } // do not delete file if we can resume it's downloading later if (m_file.isOpen()) diff --git a/platform/qt_download.hpp b/platform/qt_download.hpp index b50ff7e214..ae1adf16ba 100644 --- a/platform/qt_download.hpp +++ b/platform/qt_download.hpp @@ -1,6 +1,7 @@ #pragma once #include "download_manager.hpp" +#include "url_generator.hpp" #include @@ -8,17 +9,17 @@ class QtDownloadManager; class QtDownload : public QObject { -private: Q_OBJECT // can be changed during server redirections QUrl m_currentUrl; QNetworkReply * m_reply; QFile m_file; - int m_retryCounter; HttpStartParams m_params; + UrlGenerator m_urlGenerator; + void StartRequest(); private slots: diff --git a/storage/storage.cpp b/storage/storage.cpp index 588ac836df..a8d6f99309 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -62,7 +62,8 @@ namespace storage string Storage::UpdateBaseUrl() const { - return UPDATE_BASE_URL OMIM_OS_NAME "/" + strings::to_string(m_currentVersion) + "/"; + // we do not add server name here - it should be added automatically in Downloader Engine + return OMIM_OS_NAME "/" + strings::to_string(m_currentVersion) + "/"; } CountriesContainerT const & NodeFromIndex(CountriesContainerT const & root, TIndex const & index) @@ -193,7 +194,6 @@ namespace storage params.m_fileToSave = GetPlatform().WritablePathForFile(it->m_nameWithExt); params.m_finish = bind(&Storage::OnMapDownloadFinished, this, _1); params.m_progress = bind(&Storage::OnMapDownloadProgress, this, _1); - params.m_useResume = true; // enabled resume support by default GetDownloadManager().HttpRequest(params); // notify GUI - new status for country, "Downloading" if (m_observerChange)