diff --git a/backends/cloud/box/boxstorage.cpp b/backends/cloud/box/boxstorage.cpp index 9e036b1187b..8162af299c0 100644 --- a/backends/cloud/box/boxstorage.cpp +++ b/backends/cloud/box/boxstorage.cpp @@ -56,12 +56,16 @@ BoxStorage::BoxStorage(Common::String accessToken, Common::String refreshToken): _token(accessToken), _refreshToken(refreshToken) {} BoxStorage::BoxStorage(Common::String code) { - getAccessToken(new Common::Callback(this, &BoxStorage::codeFlowComplete), code); + getAccessToken( + new Common::Callback(this, &BoxStorage::codeFlowComplete), + new Common::Callback(this, &BoxStorage::codeFlowFailed), + code + ); } BoxStorage::~BoxStorage() {} -void BoxStorage::getAccessToken(BoolCallback callback, Common::String code) { +void BoxStorage::getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback, Common::String code) { if (!KEY || !SECRET) loadKeyAndSecret(); bool codeFlow = (code != ""); @@ -72,7 +76,8 @@ void BoxStorage::getAccessToken(BoolCallback callback, Common::String code) { } Networking::JsonCallback innerCallback = new Common::CallbackBridge(this, &BoxStorage::tokenRefreshed, callback); - Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, getErrorPrintingCallback(), "https://api.box.com/oauth2/token"); + if (errorCallback == nullptr) errorCallback = getErrorPrintingCallback(); + Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, errorCallback, "https://api.box.com/oauth2/token"); if (codeFlow) { request->addPostField("grant_type=authorization_code"); request->addPostField("code=" + code); @@ -117,6 +122,7 @@ void BoxStorage::tokenRefreshed(BoolCallback callback, Networking::JsonResponse void BoxStorage::codeFlowComplete(BoolResponse response) { if (!response.value) { warning("BoxStorage: failed to get access token through code flow"); + CloudMan.removeStorage(this); return; } @@ -124,6 +130,12 @@ void BoxStorage::codeFlowComplete(BoolResponse response) { ConfMan.flushToDisk(); } +void BoxStorage::codeFlowFailed(Networking::ErrorResponse error) { + debug("Box's code flow failed (%s, %ld):", (error.failed ? "failed" : "interrupted"), error.httpResponseCode); + debug("%s", error.response.c_str()); + CloudMan.removeStorage(this); +} + void BoxStorage::saveConfig(Common::String keyPrefix) { ConfMan.set(keyPrefix + "access_token", _token, ConfMan.kCloudDomain); ConfMan.set(keyPrefix + "refresh_token", _refreshToken, ConfMan.kCloudDomain); diff --git a/backends/cloud/box/boxstorage.h b/backends/cloud/box/boxstorage.h index 51f2a9591cd..826d6bef4ed 100644 --- a/backends/cloud/box/boxstorage.h +++ b/backends/cloud/box/boxstorage.h @@ -42,6 +42,7 @@ class BoxStorage: public Id::IdStorage { void tokenRefreshed(BoolCallback callback, Networking::JsonResponse response); void codeFlowComplete(BoolResponse response); + void codeFlowFailed(Networking::ErrorResponse error); /** Constructs StorageInfo based on JSON response from cloud. */ void infoInnerCallback(StorageInfoCallback outerCallback, Networking::JsonResponse json); @@ -108,7 +109,7 @@ public: * Use "" in order to refresh token and pass a callback, so you could * continue your work when new token is available. */ - void getAccessToken(BoolCallback callback, Common::String code = ""); + void getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback = nullptr, Common::String code = ""); Common::String accessToken() { return _token; } }; diff --git a/backends/cloud/cloudmanager.cpp b/backends/cloud/cloudmanager.cpp index b6a721be90c..8b7230fb5c6 100644 --- a/backends/cloud/cloudmanager.cpp +++ b/backends/cloud/cloudmanager.cpp @@ -45,6 +45,7 @@ CloudManager::CloudManager() : _currentStorageIndex(0), _activeStorage(nullptr) CloudManager::~CloudManager() { //TODO: do we have to save storages on manager destruction? delete _activeStorage; + freeStorages(); } Common::String CloudManager::getStorageConfigName(uint32 index) const { @@ -124,6 +125,7 @@ void CloudManager::save() { } void CloudManager::replaceStorage(Storage *storage, uint32 index) { + freeStorages(); if (!storage) error("CloudManager::replaceStorage: NULL storage passed"); if (index >= kStorageTotal) error("CloudManager::replaceStorage: invalid index passed"); delete _activeStorage; @@ -138,6 +140,18 @@ void CloudManager::replaceStorage(Storage *storage, uint32 index) { } } +void CloudManager::removeStorage(Storage *storage) { + // can't just delete it as it's mostly likely the one who calls the method + // it would be freed on freeStorages() call (on next Storage connect or replace) + _storagesToRemove.push_back(storage); +} + +void CloudManager::freeStorages() { + for (uint32 i = 0; i < _storagesToRemove.size(); ++i) + delete _storagesToRemove[i]; + _storagesToRemove.clear(); +} + Storage *CloudManager::getCurrentStorage() const { return _activeStorage; } @@ -207,6 +221,8 @@ void CloudManager::setStorageLastSync(uint32 index, Common::String date) { } void CloudManager::connectStorage(uint32 index, Common::String code) { + freeStorages(); + Storage *storage = nullptr; switch (index) { case kStorageDropboxId: storage = new Dropbox::DropboxStorage(code); break; @@ -214,7 +230,9 @@ void CloudManager::connectStorage(uint32 index, Common::String code) { case kStorageGoogleDriveId: storage = new GoogleDrive::GoogleDriveStorage(code); break; case kStorageBoxId: storage = new Box::BoxStorage(code); break; } - //these would automatically request replaceStorage() when they receive the token + // in these constructors Storages request token using the passed code + // when the token is received, they call replaceStorage() + // or removeStorage(), if some error occurred } void CloudManager::printBool(Storage::BoolResponse response) const { diff --git a/backends/cloud/cloudmanager.h b/backends/cloud/cloudmanager.h index 15409ee3a1e..4442b9bfbeb 100644 --- a/backends/cloud/cloudmanager.h +++ b/backends/cloud/cloudmanager.h @@ -59,6 +59,7 @@ class CloudManager : public Common::Singleton { Common::Array _storages; uint _currentStorageIndex; Storage *_activeStorage; + Common::Array _storagesToRemove; void printBool(Cloud::Storage::BoolResponse response) const; @@ -66,6 +67,9 @@ class CloudManager : public Common::Singleton { Common::String getStorageConfigName(uint32 index) const; + /** Frees memory used by storages which failed to connect. */ + void freeStorages(); + public: CloudManager(); virtual ~CloudManager(); @@ -91,6 +95,9 @@ public: */ void replaceStorage(Storage *storage, uint32 index); + /** Adds storage in the list of storages to remove later. */ + void removeStorage(Storage *storage); + /** * Returns active Storage, which could be used to interact * with cloud storage. diff --git a/backends/cloud/dropbox/dropboxstorage.cpp b/backends/cloud/dropbox/dropboxstorage.cpp index e34912ed66c..4171af04437 100644 --- a/backends/cloud/dropbox/dropboxstorage.cpp +++ b/backends/cloud/dropbox/dropboxstorage.cpp @@ -62,8 +62,9 @@ DropboxStorage::~DropboxStorage() {} void DropboxStorage::getAccessToken(Common::String code) { if (!KEY || !SECRET) loadKeyAndSecret(); - Networking::JsonCallback callback = new Common::Callback(this, &DropboxStorage::codeFlowComplete); - Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(callback, nullptr, "https://api.dropboxapi.com/oauth2/token"); + Networking::JsonCallback callback = new Common::Callback(this, &DropboxStorage::codeFlowComplete); + Networking::ErrorCallback errorCallback = new Common::Callback(this, &DropboxStorage::codeFlowFailed); + Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(callback, errorCallback, "https://api.dropboxapi.com/oauth2/token"); request->addPostField("code=" + code); request->addPostField("grant_type=authorization_code"); request->addPostField("client_id=" + Common::String(KEY)); @@ -83,6 +84,7 @@ void DropboxStorage::codeFlowComplete(Networking::JsonResponse response) { if (!result.contains("access_token") || !result.contains("uid")) { warning("%s", json->stringify(true).c_str()); warning("Bad response, no token/uid passed"); + CloudMan.removeStorage(this); } else { _token = result.getVal("access_token")->asString(); _uid = result.getVal("uid")->asString(); @@ -94,9 +96,16 @@ void DropboxStorage::codeFlowComplete(Networking::JsonResponse response) { delete json; } else { debug("DropboxStorage::codeFlowComplete: got NULL instead of JSON!"); + CloudMan.removeStorage(this); } } +void DropboxStorage::codeFlowFailed(Networking::ErrorResponse error) { + debug("Dropbox's code flow failed (%s, %ld):", (error.failed ? "failed" : "interrupted"), error.httpResponseCode); + debug("%s", error.response.c_str()); + CloudMan.removeStorage(this); +} + void DropboxStorage::saveConfig(Common::String keyPrefix) { ConfMan.set(keyPrefix + "access_token", _token, ConfMan.kCloudDomain); ConfMan.set(keyPrefix + "user_id", _uid, ConfMan.kCloudDomain); diff --git a/backends/cloud/dropbox/dropboxstorage.h b/backends/cloud/dropbox/dropboxstorage.h index 2c9cf163ab2..9dd28ba8108 100644 --- a/backends/cloud/dropbox/dropboxstorage.h +++ b/backends/cloud/dropbox/dropboxstorage.h @@ -42,6 +42,7 @@ class DropboxStorage: public Cloud::Storage { void getAccessToken(Common::String code); void codeFlowComplete(Networking::JsonResponse response); + void codeFlowFailed(Networking::ErrorResponse error); public: /** This constructor uses OAuth code flow to get tokens. */ diff --git a/backends/cloud/googledrive/googledrivestorage.cpp b/backends/cloud/googledrive/googledrivestorage.cpp index 2816301cacb..c4264099fd8 100644 --- a/backends/cloud/googledrive/googledrivestorage.cpp +++ b/backends/cloud/googledrive/googledrivestorage.cpp @@ -56,12 +56,16 @@ GoogleDriveStorage::GoogleDriveStorage(Common::String accessToken, Common::Strin _token(accessToken), _refreshToken(refreshToken) {} GoogleDriveStorage::GoogleDriveStorage(Common::String code) { - getAccessToken(new Common::Callback(this, &GoogleDriveStorage::codeFlowComplete), code); + getAccessToken( + new Common::Callback(this, &GoogleDriveStorage::codeFlowComplete), + new Common::Callback(this, &GoogleDriveStorage::codeFlowFailed), + code + ); } GoogleDriveStorage::~GoogleDriveStorage() {} -void GoogleDriveStorage::getAccessToken(BoolCallback callback, Common::String code) { +void GoogleDriveStorage::getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback, Common::String code) { if (!KEY || !SECRET) loadKeyAndSecret(); bool codeFlow = (code != ""); @@ -72,7 +76,8 @@ void GoogleDriveStorage::getAccessToken(BoolCallback callback, Common::String co } Networking::JsonCallback innerCallback = new Common::CallbackBridge(this, &GoogleDriveStorage::tokenRefreshed, callback); - Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, getErrorPrintingCallback(), "https://accounts.google.com/o/oauth2/token"); //TODO + if (errorCallback == nullptr) errorCallback = getErrorPrintingCallback(); + Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, errorCallback, "https://accounts.google.com/o/oauth2/token"); //TODO if (codeFlow) { request->addPostField("code=" + code); request->addPostField("grant_type=authorization_code"); @@ -118,6 +123,7 @@ void GoogleDriveStorage::tokenRefreshed(BoolCallback callback, Networking::JsonR void GoogleDriveStorage::codeFlowComplete(BoolResponse response) { if (!response.value) { warning("GoogleDriveStorage: failed to get access token through code flow"); + CloudMan.removeStorage(this); return; } @@ -126,6 +132,12 @@ void GoogleDriveStorage::codeFlowComplete(BoolResponse response) { ConfMan.flushToDisk(); } +void GoogleDriveStorage::codeFlowFailed(Networking::ErrorResponse error) { + debug("Google Drive's code flow failed (%s, %ld):", (error.failed ? "failed" : "interrupted"), error.httpResponseCode); + debug("%s", error.response.c_str()); + CloudMan.removeStorage(this); +} + void GoogleDriveStorage::saveConfig(Common::String keyPrefix) { ConfMan.set(keyPrefix + "access_token", _token, ConfMan.kCloudDomain); ConfMan.set(keyPrefix + "refresh_token", _refreshToken, ConfMan.kCloudDomain); diff --git a/backends/cloud/googledrive/googledrivestorage.h b/backends/cloud/googledrive/googledrivestorage.h index 4a7dbab99f1..eee4de90b05 100644 --- a/backends/cloud/googledrive/googledrivestorage.h +++ b/backends/cloud/googledrive/googledrivestorage.h @@ -42,6 +42,7 @@ class GoogleDriveStorage: public Id::IdStorage { void tokenRefreshed(BoolCallback callback, Networking::JsonResponse response); void codeFlowComplete(BoolResponse response); + void codeFlowFailed(Networking::ErrorResponse error); /** Constructs StorageInfo based on JSON response from cloud. */ void infoInnerCallback(StorageInfoCallback outerCallback, Networking::JsonResponse json); @@ -110,7 +111,7 @@ public: * Use "" in order to refresh token and pass a callback, so you could * continue your work when new token is available. */ - void getAccessToken(BoolCallback callback, Common::String code = ""); + void getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback = nullptr, Common::String code = ""); Common::String accessToken() { return _token; } }; diff --git a/backends/cloud/onedrive/onedrivestorage.cpp b/backends/cloud/onedrive/onedrivestorage.cpp index 3c8ea5fe810..e9165943afe 100644 --- a/backends/cloud/onedrive/onedrivestorage.cpp +++ b/backends/cloud/onedrive/onedrivestorage.cpp @@ -57,12 +57,16 @@ OneDriveStorage::OneDriveStorage(Common::String accessToken, Common::String user _token(accessToken), _uid(userId), _refreshToken(refreshToken) {} OneDriveStorage::OneDriveStorage(Common::String code) { - getAccessToken(new Common::Callback(this, &OneDriveStorage::codeFlowComplete), code); + getAccessToken( + new Common::Callback(this, &OneDriveStorage::codeFlowComplete), + new Common::Callback(this, &OneDriveStorage::codeFlowFailed), + code + ); } OneDriveStorage::~OneDriveStorage() {} -void OneDriveStorage::getAccessToken(BoolCallback callback, Common::String code) { +void OneDriveStorage::getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback, Common::String code) { if (!KEY || !SECRET) loadKeyAndSecret(); bool codeFlow = (code != ""); @@ -73,7 +77,8 @@ void OneDriveStorage::getAccessToken(BoolCallback callback, Common::String code) } Networking::JsonCallback innerCallback = new Common::CallbackBridge(this, &OneDriveStorage::tokenRefreshed, callback); - Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, getErrorPrintingCallback(), "https://login.live.com/oauth20_token.srf"); //TODO + if (errorCallback == nullptr) errorCallback = getErrorPrintingCallback(); + Networking::CurlJsonRequest *request = new Networking::CurlJsonRequest(innerCallback, errorCallback, "https://login.live.com/oauth20_token.srf"); //TODO if (codeFlow) { request->addPostField("code=" + code); request->addPostField("grant_type=authorization_code"); @@ -117,6 +122,7 @@ void OneDriveStorage::tokenRefreshed(BoolCallback callback, Networking::JsonResp void OneDriveStorage::codeFlowComplete(BoolResponse response) { if (!response.value) { warning("OneDriveStorage: failed to get access token through code flow"); + CloudMan.removeStorage(this); return; } @@ -125,6 +131,12 @@ void OneDriveStorage::codeFlowComplete(BoolResponse response) { ConfMan.flushToDisk(); } +void OneDriveStorage::codeFlowFailed(Networking::ErrorResponse error) { + debug("OneDrive's code flow failed (%s, %ld):", (error.failed ? "failed" : "interrupted"), error.httpResponseCode); + debug("%s", error.response.c_str()); + CloudMan.removeStorage(this); +} + void OneDriveStorage::saveConfig(Common::String keyPrefix) { ConfMan.set(keyPrefix + "access_token", _token, ConfMan.kCloudDomain); ConfMan.set(keyPrefix + "user_id", _uid, ConfMan.kCloudDomain); diff --git a/backends/cloud/onedrive/onedrivestorage.h b/backends/cloud/onedrive/onedrivestorage.h index 650c240aefb..8ceaaf107ea 100644 --- a/backends/cloud/onedrive/onedrivestorage.h +++ b/backends/cloud/onedrive/onedrivestorage.h @@ -41,6 +41,7 @@ class OneDriveStorage: public Cloud::Storage { void tokenRefreshed(BoolCallback callback, Networking::JsonResponse response); void codeFlowComplete(BoolResponse response); + void codeFlowFailed(Networking::ErrorResponse error); /** Constructs StorageInfo based on JSON response from cloud. */ void infoInnerCallback(StorageInfoCallback outerCallback, Networking::JsonResponse json); @@ -101,7 +102,7 @@ public: * Use "" in order to refresh token and pass a callback, so you could * continue your work when new token is available. */ - void getAccessToken(BoolCallback callback, Common::String code = ""); + void getAccessToken(BoolCallback callback, Networking::ErrorCallback errorCallback = nullptr, Common::String code = ""); Common::String accessToken() { return _token; } };