From e90ea82ef53001e3ae3f450ebc902ddd0a1903dd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 25 Jun 2018 16:34:07 -0700 Subject: [PATCH 1/4] move AddressManager to NodeList thread, fix AccountManager invokes --- domain-server/src/DomainGatekeeper.cpp | 32 ++--- domain-server/src/DomainGatekeeper.h | 12 +- domain-server/src/DomainServer.cpp | 18 +-- domain-server/src/DomainServer.h | 10 +- .../src/DomainServerSettingsManager.cpp | 16 +-- .../src/DomainServerSettingsManager.h | 8 +- interface/src/Application.cpp | 7 +- interface/src/DiscoverabilityManager.cpp | 2 +- interface/src/DiscoverabilityManager.h | 2 +- interface/src/commerce/Ledger.cpp | 34 ++--- interface/src/commerce/Ledger.h | 56 ++++---- interface/src/ui/LoginDialog.cpp | 18 +-- interface/src/ui/LoginDialog.h | 12 +- interface/src/ui/SnapshotUploader.cpp | 24 ++-- interface/src/ui/SnapshotUploader.h | 10 +- libraries/networking/src/AccountManager.cpp | 131 +++++++++--------- libraries/networking/src/AccountManager.h | 11 +- libraries/networking/src/AddressManager.cpp | 22 +-- libraries/networking/src/AddressManager.h | 8 +- libraries/networking/src/NodeList.cpp | 2 +- .../networking/src/UserActivityLogger.cpp | 4 +- libraries/networking/src/UserActivityLogger.h | 2 +- libraries/ui/src/Tooltip.cpp | 4 +- libraries/ui/src/Tooltip.h | 2 +- 24 files changed, 222 insertions(+), 225 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index b4685b907f..2d78149c7a 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -675,19 +675,19 @@ void DomainGatekeeper::requestUserPublicKey(const QString& username, bool isOpti QNetworkAccessManager::GetOperation, callbackParams); } -QString extractUsernameFromPublicKeyRequest(QNetworkReply& requestReply) { +QString extractUsernameFromPublicKeyRequest(QNetworkReply* requestReply) { // extract the username from the request url QString username; const QString PUBLIC_KEY_URL_REGEX_STRING = "api\\/v1\\/users\\/([A-Za-z0-9_\\.]+)\\/public_key"; QRegExp usernameRegex(PUBLIC_KEY_URL_REGEX_STRING); - if (usernameRegex.indexIn(requestReply.url().toString()) != -1) { + if (usernameRegex.indexIn(requestReply->url().toString()) != -1) { username = usernameRegex.cap(1); } return username.toLower(); } -void DomainGatekeeper::publicKeyJSONCallback(QNetworkReply& requestReply) { - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); +void DomainGatekeeper::publicKeyJSONCallback(QNetworkReply* requestReply) { + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); QString username = extractUsernameFromPublicKeyRequest(requestReply); bool isOptimisticKey = _inFlightPublicKeyRequests.take(username); @@ -707,8 +707,8 @@ void DomainGatekeeper::publicKeyJSONCallback(QNetworkReply& requestReply) { } } -void DomainGatekeeper::publicKeyJSONErrorCallback(QNetworkReply& requestReply) { - qDebug() << "publicKey api call failed:" << requestReply.error(); +void DomainGatekeeper::publicKeyJSONErrorCallback(QNetworkReply* requestReply) { + qDebug() << "publicKey api call failed:" << requestReply->error(); QString username = extractUsernameFromPublicKeyRequest(requestReply); _inFlightPublicKeyRequests.remove(username); } @@ -906,18 +906,18 @@ void DomainGatekeeper::getGroupMemberships(const QString& username) { } -QString extractUsernameFromGroupMembershipsReply(QNetworkReply& requestReply) { +QString extractUsernameFromGroupMembershipsReply(QNetworkReply* requestReply) { // extract the username from the request url QString username; const QString GROUP_MEMBERSHIPS_URL_REGEX_STRING = "api\\/v1\\/groups\\/members\\/([A-Za-z0-9_\\.]+)"; QRegExp usernameRegex(GROUP_MEMBERSHIPS_URL_REGEX_STRING); - if (usernameRegex.indexIn(requestReply.url().toString()) != -1) { + if (usernameRegex.indexIn(requestReply->url().toString()) != -1) { username = usernameRegex.cap(1); } return username.toLower(); } -void DomainGatekeeper::getIsGroupMemberJSONCallback(QNetworkReply& requestReply) { +void DomainGatekeeper::getIsGroupMemberJSONCallback(QNetworkReply* requestReply) { // { // "data":{ // "username":"sethalves", @@ -934,7 +934,7 @@ void DomainGatekeeper::getIsGroupMemberJSONCallback(QNetworkReply& requestReply) // "status":"success" // } - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); if (jsonObject["status"].toString() == "success") { QJsonObject data = jsonObject["data"].toObject(); QJsonObject groups = data["groups"].toObject(); @@ -953,8 +953,8 @@ void DomainGatekeeper::getIsGroupMemberJSONCallback(QNetworkReply& requestReply) _inFlightGroupMembershipsRequests.remove(extractUsernameFromGroupMembershipsReply(requestReply)); } -void DomainGatekeeper::getIsGroupMemberErrorCallback(QNetworkReply& requestReply) { - qDebug() << "getIsGroupMember api call failed:" << requestReply.error(); +void DomainGatekeeper::getIsGroupMemberErrorCallback(QNetworkReply* requestReply) { + qDebug() << "getIsGroupMember api call failed:" << requestReply->error(); _inFlightGroupMembershipsRequests.remove(extractUsernameFromGroupMembershipsReply(requestReply)); } @@ -974,7 +974,7 @@ void DomainGatekeeper::getDomainOwnerFriendsList() { } -void DomainGatekeeper::getDomainOwnerFriendsListJSONCallback(QNetworkReply& requestReply) { +void DomainGatekeeper::getDomainOwnerFriendsListJSONCallback(QNetworkReply* requestReply) { // { // status: "success", // data: { @@ -991,7 +991,7 @@ void DomainGatekeeper::getDomainOwnerFriendsListJSONCallback(QNetworkReply& requ // ] // } // } - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); if (jsonObject["status"].toString() == "success") { _domainOwnerFriends.clear(); QJsonArray friends = jsonObject["data"].toObject()["friends"].toArray(); @@ -1003,8 +1003,8 @@ void DomainGatekeeper::getDomainOwnerFriendsListJSONCallback(QNetworkReply& requ } } -void DomainGatekeeper::getDomainOwnerFriendsListErrorCallback(QNetworkReply& requestReply) { - qDebug() << "getDomainOwnerFriendsList api call failed:" << requestReply.error(); +void DomainGatekeeper::getDomainOwnerFriendsListErrorCallback(QNetworkReply* requestReply) { + qDebug() << "getDomainOwnerFriendsList api call failed:" << requestReply->error(); } void DomainGatekeeper::refreshGroupsCache() { diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h index 2cb9b4c8a9..f8d79179d6 100644 --- a/domain-server/src/DomainGatekeeper.h +++ b/domain-server/src/DomainGatekeeper.h @@ -51,14 +51,14 @@ public slots: void processICEPingReplyPacket(QSharedPointer message); void processICEPeerInformationPacket(QSharedPointer message); - void publicKeyJSONCallback(QNetworkReply& requestReply); - void publicKeyJSONErrorCallback(QNetworkReply& requestReply); + void publicKeyJSONCallback(QNetworkReply* requestReply); + void publicKeyJSONErrorCallback(QNetworkReply* requestReply); - void getIsGroupMemberJSONCallback(QNetworkReply& requestReply); - void getIsGroupMemberErrorCallback(QNetworkReply& requestReply); + void getIsGroupMemberJSONCallback(QNetworkReply* requestReply); + void getIsGroupMemberErrorCallback(QNetworkReply* requestReply); - void getDomainOwnerFriendsListJSONCallback(QNetworkReply& requestReply); - void getDomainOwnerFriendsListErrorCallback(QNetworkReply& requestReply); + void getDomainOwnerFriendsListJSONCallback(QNetworkReply* requestReply); + void getDomainOwnerFriendsListErrorCallback(QNetworkReply* requestReply); void refreshGroupsCache(); diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index a34deebc95..5c602b6629 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -519,8 +519,8 @@ void DomainServer::getTemporaryName(bool force) { QNetworkAccessManager::PostOperation, callbackParameters); } -void DomainServer::handleTempDomainSuccess(QNetworkReply& requestReply) { - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); +void DomainServer::handleTempDomainSuccess(QNetworkReply* requestReply) { + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); // grab the information for the new domain static const QString DATA_KEY = "data"; @@ -565,7 +565,7 @@ void DomainServer::handleTempDomainSuccess(QNetworkReply& requestReply) { } } -void DomainServer::handleTempDomainError(QNetworkReply& requestReply) { +void DomainServer::handleTempDomainError(QNetworkReply* requestReply) { qWarning() << "A temporary name was requested but there was an error creating one. Please try again via domain-server relaunch" << "or from the domain-server settings."; } @@ -1453,7 +1453,7 @@ void DomainServer::sendHeartbeatToMetaverse(const QString& networkAddress) { domainUpdateJSON.toUtf8()); } -void DomainServer::handleMetaverseHeartbeatError(QNetworkReply& requestReply) { +void DomainServer::handleMetaverseHeartbeatError(QNetworkReply* requestReply) { if (!_metaverseHeartbeatTimer) { // avoid rehandling errors from the same issue return; @@ -1462,13 +1462,13 @@ void DomainServer::handleMetaverseHeartbeatError(QNetworkReply& requestReply) { // only attempt to grab a new temporary name if we're already a temporary domain server if (_type == MetaverseTemporaryDomain) { // check if we need to force a new temporary domain name - switch (requestReply.error()) { + switch (requestReply->error()) { // if we have a temporary domain with a bad token, we get a 401 case QNetworkReply::NetworkError::AuthenticationRequiredError: { static const QString DATA_KEY = "data"; static const QString TOKEN_KEY = "api_key"; - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); auto tokenFailure = jsonObject[DATA_KEY].toObject()[TOKEN_KEY]; if (!tokenFailure.isNull()) { @@ -1552,7 +1552,7 @@ void DomainServer::sendICEServerAddressToMetaverseAPI() { domainUpdateJSON.toUtf8()); } -void DomainServer::handleSuccessfulICEServerAddressUpdate(QNetworkReply& requestReply) { +void DomainServer::handleSuccessfulICEServerAddressUpdate(QNetworkReply* requestReply) { _sendICEServerAddressToMetaverseAPIInProgress = false; if (_sendICEServerAddressToMetaverseAPIRedo) { qDebug() << "ice-server address updated with metaverse, but has since changed. redoing update..."; @@ -1563,7 +1563,7 @@ void DomainServer::handleSuccessfulICEServerAddressUpdate(QNetworkReply& request } } -void DomainServer::handleFailedICEServerAddressUpdate(QNetworkReply& requestReply) { +void DomainServer::handleFailedICEServerAddressUpdate(QNetworkReply* requestReply) { _sendICEServerAddressToMetaverseAPIInProgress = false; if (_sendICEServerAddressToMetaverseAPIRedo) { // if we have new data, retry right away, even though the previous attempt didn't go well. @@ -1573,7 +1573,7 @@ void DomainServer::handleFailedICEServerAddressUpdate(QNetworkReply& requestRepl const int ICE_SERVER_UPDATE_RETRY_MS = 2 * 1000; qWarning() << "Failed to update ice-server address with High Fidelity Metaverse - error was" - << requestReply.errorString(); + << requestReply->errorString(); qWarning() << "\tRe-attempting in" << ICE_SERVER_UPDATE_RETRY_MS / 1000 << "seconds"; QTimer::singleShot(ICE_SERVER_UPDATE_RETRY_MS, this, SLOT(sendICEServerAddressToMetaverseAPI())); diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 3703877fa1..c69267f379 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -108,10 +108,10 @@ private slots: void sendHeartbeatToIceServer(); void handleConnectedNode(SharedNodePointer newNode); - void handleTempDomainSuccess(QNetworkReply& requestReply); - void handleTempDomainError(QNetworkReply& requestReply); + void handleTempDomainSuccess(QNetworkReply* requestReply); + void handleTempDomainError(QNetworkReply* requestReply); - void handleMetaverseHeartbeatError(QNetworkReply& requestReply); + void handleMetaverseHeartbeatError(QNetworkReply* requestReply); void queuedQuit(QString quitMessage, int exitCode); @@ -121,8 +121,8 @@ private slots: void handleICEHostInfo(const QHostInfo& hostInfo); void sendICEServerAddressToMetaverseAPI(); - void handleSuccessfulICEServerAddressUpdate(QNetworkReply& requestReply); - void handleFailedICEServerAddressUpdate(QNetworkReply& requestReply); + void handleSuccessfulICEServerAddressUpdate(QNetworkReply* requestReply); + void handleFailedICEServerAddressUpdate(QNetworkReply* requestReply); void updateReplicatedNodes(); void updateDownstreamNodes(); diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index c46b2eda49..fda9119148 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -1826,7 +1826,7 @@ void DomainServerSettingsManager::apiGetGroupID(const QString& groupName) { QNetworkAccessManager::GetOperation, callbackParams); } -void DomainServerSettingsManager::apiGetGroupIDJSONCallback(QNetworkReply& requestReply) { +void DomainServerSettingsManager::apiGetGroupIDJSONCallback(QNetworkReply* requestReply) { // { // "data":{ // "groups":[{ @@ -1857,7 +1857,7 @@ void DomainServerSettingsManager::apiGetGroupIDJSONCallback(QNetworkReply& reque // }, // "status":"success" // } - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); if (jsonObject["status"].toString() == "success") { QJsonArray groups = jsonObject["data"].toObject()["groups"].toArray(); for (int i = 0; i < groups.size(); i++) { @@ -1876,8 +1876,8 @@ void DomainServerSettingsManager::apiGetGroupIDJSONCallback(QNetworkReply& reque } } -void DomainServerSettingsManager::apiGetGroupIDErrorCallback(QNetworkReply& requestReply) { - qDebug() << "******************** getGroupID api call failed:" << requestReply.error(); +void DomainServerSettingsManager::apiGetGroupIDErrorCallback(QNetworkReply* requestReply) { + qDebug() << "******************** getGroupID api call failed:" << requestReply->error(); } void DomainServerSettingsManager::apiGetGroupRanks(const QUuid& groupID) { @@ -1893,7 +1893,7 @@ void DomainServerSettingsManager::apiGetGroupRanks(const QUuid& groupID) { QNetworkAccessManager::GetOperation, callbackParams); } -void DomainServerSettingsManager::apiGetGroupRanksJSONCallback(QNetworkReply& requestReply) { +void DomainServerSettingsManager::apiGetGroupRanksJSONCallback(QNetworkReply* requestReply) { // { // "data":{ // "groups":{ @@ -1926,7 +1926,7 @@ void DomainServerSettingsManager::apiGetGroupRanksJSONCallback(QNetworkReply& re // } bool changed = false; - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); if (jsonObject["status"].toString() == "success") { QJsonObject groups = jsonObject["data"].toObject()["groups"].toObject(); @@ -1972,8 +1972,8 @@ void DomainServerSettingsManager::apiGetGroupRanksJSONCallback(QNetworkReply& re } } -void DomainServerSettingsManager::apiGetGroupRanksErrorCallback(QNetworkReply& requestReply) { - qDebug() << "******************** getGroupRanks api call failed:" << requestReply.error(); +void DomainServerSettingsManager::apiGetGroupRanksErrorCallback(QNetworkReply* requestReply) { + qDebug() << "******************** getGroupRanks api call failed:" << requestReply->error(); } void DomainServerSettingsManager::recordGroupMembership(const QString& name, const QUuid groupID, QUuid rankID) { diff --git a/domain-server/src/DomainServerSettingsManager.h b/domain-server/src/DomainServerSettingsManager.h index 252ff79ae2..bcd33c2bb0 100644 --- a/domain-server/src/DomainServerSettingsManager.h +++ b/domain-server/src/DomainServerSettingsManager.h @@ -133,10 +133,10 @@ signals: void settingsUpdated(); public slots: - void apiGetGroupIDJSONCallback(QNetworkReply& requestReply); - void apiGetGroupIDErrorCallback(QNetworkReply& requestReply); - void apiGetGroupRanksJSONCallback(QNetworkReply& requestReply); - void apiGetGroupRanksErrorCallback(QNetworkReply& requestReply); + void apiGetGroupIDJSONCallback(QNetworkReply* requestReply); + void apiGetGroupIDErrorCallback(QNetworkReply* requestReply); + void apiGetGroupRanksJSONCallback(QNetworkReply* requestReply); + void apiGetGroupRanksErrorCallback(QNetworkReply* requestReply); private slots: void processSettingsRequestPacket(QSharedPointer message); diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6f95a1afe8..56041ba507 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1081,6 +1081,11 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo auto nodeList = DependencyManager::get(); nodeList->startThread(); + // move the AddressManager to the NodeList thread so that domain resets due to domain changes always occur + // before we tell MyAvatar to go to a new location in the new domain + auto addressManager = DependencyManager::get(); + addressManager->moveToThread(nodeList->thread()); + const char** constArgv = const_cast(argv); if (cmdOptionExists(argc, constArgv, "--disableWatchdog")) { DISABLE_WATCHDOG = true; @@ -1231,8 +1236,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo accountManager->setIsAgent(true); accountManager->setAuthURL(NetworkingConstants::METAVERSE_SERVER_URL()); - auto addressManager = DependencyManager::get(); - // use our MyAvatar position and quat for address manager path addressManager->setPositionGetter([this]{ return getMyAvatar()->getWorldPosition(); }); addressManager->setOrientationGetter([this]{ return getMyAvatar()->getWorldOrientation(); }); diff --git a/interface/src/DiscoverabilityManager.cpp b/interface/src/DiscoverabilityManager.cpp index d39aaa4f1a..191908ab04 100644 --- a/interface/src/DiscoverabilityManager.cpp +++ b/interface/src/DiscoverabilityManager.cpp @@ -136,7 +136,7 @@ void DiscoverabilityManager::updateLocation() { setCrashAnnotation("address", currentAddress.toString().toStdString()); } -void DiscoverabilityManager::handleHeartbeatResponse(QNetworkReply& requestReply) { +void DiscoverabilityManager::handleHeartbeatResponse(QNetworkReply* requestReply) { auto dataObject = AccountManager::dataObjectFromResponse(requestReply); if (!dataObject.isEmpty()) { diff --git a/interface/src/DiscoverabilityManager.h b/interface/src/DiscoverabilityManager.h index 0c62ad5663..fc0b1fa759 100644 --- a/interface/src/DiscoverabilityManager.h +++ b/interface/src/DiscoverabilityManager.h @@ -51,7 +51,7 @@ public: static QString findableByString(Discoverability::Mode discoverabilityMode); private slots: - void handleHeartbeatResponse(QNetworkReply& requestReply); + void handleHeartbeatResponse(QNetworkReply* requestReply); private: DiscoverabilityManager(); diff --git a/interface/src/commerce/Ledger.cpp b/interface/src/commerce/Ledger.cpp index 69698e82a6..1f36148049 100644 --- a/interface/src/commerce/Ledger.cpp +++ b/interface/src/commerce/Ledger.cpp @@ -28,15 +28,15 @@ // account synthesizes a result {status: 'success', data: {keyStatus: "preexisting"|"conflicting"|"ok"}} -QJsonObject Ledger::apiResponse(const QString& label, QNetworkReply& reply) { - QByteArray response = reply.readAll(); +QJsonObject Ledger::apiResponse(const QString& label, QNetworkReply* reply) { + QByteArray response = reply->readAll(); QJsonObject data = QJsonDocument::fromJson(response).object(); qInfo(commerce) << label << "response" << QJsonDocument(data).toJson(QJsonDocument::Compact); return data; } // Non-200 responses are not json: -QJsonObject Ledger::failResponse(const QString& label, QNetworkReply& reply) { - QString response = reply.readAll(); +QJsonObject Ledger::failResponse(const QString& label, QNetworkReply* reply) { + QString response = reply->readAll(); qWarning(commerce) << "FAILED" << label << response; // tempResult will be NULL if the response isn't valid JSON. @@ -52,8 +52,8 @@ QJsonObject Ledger::failResponse(const QString& label, QNetworkReply& reply) { return tempResult.object(); } } -#define ApiHandler(NAME) void Ledger::NAME##Success(QNetworkReply& reply) { emit NAME##Result(apiResponse(#NAME, reply)); } -#define FailHandler(NAME) void Ledger::NAME##Failure(QNetworkReply& reply) { emit NAME##Result(failResponse(#NAME, reply)); } +#define ApiHandler(NAME) void Ledger::NAME##Success(QNetworkReply* reply) { emit NAME##Result(apiResponse(#NAME, reply)); } +#define FailHandler(NAME) void Ledger::NAME##Failure(QNetworkReply* reply) { emit NAME##Result(failResponse(#NAME, reply)); } #define Handler(NAME) ApiHandler(NAME) FailHandler(NAME) Handler(buy) Handler(receiveAt) @@ -223,12 +223,12 @@ QString transactionString(const QJsonObject& valueObject) { } static const QString MARKETPLACE_ITEMS_BASE_URL = NetworkingConstants::METAVERSE_SERVER_URL().toString() + "/marketplace/items/"; -void Ledger::historySuccess(QNetworkReply& reply) { +void Ledger::historySuccess(QNetworkReply* reply) { // here we send a historyResult with some extra stuff in it // Namely, the styled text we'd like to show. The issue is the // QML cannot do that easily since it doesn't know what the wallet // public key(s) are. Let's keep it that way - QByteArray response = reply.readAll(); + QByteArray response = reply->readAll(); QJsonObject data = QJsonDocument::fromJson(response).object(); qInfo(commerce) << "history" << "response" << QJsonDocument(data).toJson(QJsonDocument::Compact); @@ -262,7 +262,7 @@ void Ledger::historySuccess(QNetworkReply& reply) { emit historyResult(newData); } -void Ledger::historyFailure(QNetworkReply& reply) { +void Ledger::historyFailure(QNetworkReply* reply) { failResponse("history", reply); } @@ -273,10 +273,10 @@ void Ledger::history(const QStringList& keys, const int& pageNumber, const int& keysQuery("history", "historySuccess", "historyFailure", params); } -void Ledger::accountSuccess(QNetworkReply& reply) { +void Ledger::accountSuccess(QNetworkReply* reply) { // lets set the appropriate stuff in the wallet now auto wallet = DependencyManager::get(); - QByteArray response = reply.readAll(); + QByteArray response = reply->readAll(); QJsonObject data = QJsonDocument::fromJson(response).object()["data"].toObject(); auto salt = QByteArray::fromBase64(data["salt"].toString().toUtf8()); @@ -312,7 +312,7 @@ void Ledger::accountSuccess(QNetworkReply& reply) { emit accountResult(responseData); } -void Ledger::accountFailure(QNetworkReply& reply) { +void Ledger::accountFailure(QNetworkReply* reply) { failResponse("account", reply); } void Ledger::account() { @@ -320,8 +320,8 @@ void Ledger::account() { } // The api/failResponse is called just for the side effect of logging. -void Ledger::updateLocationSuccess(QNetworkReply& reply) { apiResponse("updateLocation", reply); } -void Ledger::updateLocationFailure(QNetworkReply& reply) { failResponse("updateLocation", reply); } +void Ledger::updateLocationSuccess(QNetworkReply* reply) { apiResponse("updateLocation", reply); } +void Ledger::updateLocationFailure(QNetworkReply* reply) { failResponse("updateLocation", reply); } void Ledger::updateLocation(const QString& asset_id, const QString& location, const bool& alsoUpdateSiblings, const bool controlledFailure) { auto wallet = DependencyManager::get(); auto walletScriptingInterface = DependencyManager::get(); @@ -349,11 +349,11 @@ void Ledger::updateLocation(const QString& asset_id, const QString& location, co } } -void Ledger::certificateInfoSuccess(QNetworkReply& reply) { +void Ledger::certificateInfoSuccess(QNetworkReply* reply) { auto wallet = DependencyManager::get(); auto accountManager = DependencyManager::get(); - QByteArray response = reply.readAll(); + QByteArray response = reply->readAll(); QJsonObject replyObject = QJsonDocument::fromJson(response).object(); QStringList keys = wallet->listPublicKeys(); @@ -366,7 +366,7 @@ void Ledger::certificateInfoSuccess(QNetworkReply& reply) { qInfo(commerce) << "certificateInfo" << "response" << QJsonDocument(replyObject).toJson(QJsonDocument::Compact); emit certificateInfoResult(replyObject); } -void Ledger::certificateInfoFailure(QNetworkReply& reply) { +void Ledger::certificateInfoFailure(QNetworkReply* reply) { emit certificateInfoResult(failResponse("certificateInfo", reply)); } void Ledger::certificateInfo(const QString& certificateId) { diff --git a/interface/src/commerce/Ledger.h b/interface/src/commerce/Ledger.h index 8a8fd2630a..ba2f167f4b 100644 --- a/interface/src/commerce/Ledger.h +++ b/interface/src/commerce/Ledger.h @@ -65,36 +65,36 @@ signals: void updateCertificateStatus(const QString& certID, uint certStatus); public slots: - void buySuccess(QNetworkReply& reply); - void buyFailure(QNetworkReply& reply); - void receiveAtSuccess(QNetworkReply& reply); - void receiveAtFailure(QNetworkReply& reply); - void balanceSuccess(QNetworkReply& reply); - void balanceFailure(QNetworkReply& reply); - void inventorySuccess(QNetworkReply& reply); - void inventoryFailure(QNetworkReply& reply); - void historySuccess(QNetworkReply& reply); - void historyFailure(QNetworkReply& reply); - void accountSuccess(QNetworkReply& reply); - void accountFailure(QNetworkReply& reply); - void updateLocationSuccess(QNetworkReply& reply); - void updateLocationFailure(QNetworkReply& reply); - void certificateInfoSuccess(QNetworkReply& reply); - void certificateInfoFailure(QNetworkReply& reply); - void transferAssetToNodeSuccess(QNetworkReply& reply); - void transferAssetToNodeFailure(QNetworkReply& reply); - void transferAssetToUsernameSuccess(QNetworkReply& reply); - void transferAssetToUsernameFailure(QNetworkReply& reply); - void alreadyOwnedSuccess(QNetworkReply& reply); - void alreadyOwnedFailure(QNetworkReply& reply); - void availableUpdatesSuccess(QNetworkReply& reply); - void availableUpdatesFailure(QNetworkReply& reply); - void updateItemSuccess(QNetworkReply& reply); - void updateItemFailure(QNetworkReply& reply); + void buySuccess(QNetworkReply* reply); + void buyFailure(QNetworkReply* reply); + void receiveAtSuccess(QNetworkReply* reply); + void receiveAtFailure(QNetworkReply* reply); + void balanceSuccess(QNetworkReply* reply); + void balanceFailure(QNetworkReply* reply); + void inventorySuccess(QNetworkReply* reply); + void inventoryFailure(QNetworkReply* reply); + void historySuccess(QNetworkReply* reply); + void historyFailure(QNetworkReply* reply); + void accountSuccess(QNetworkReply* reply); + void accountFailure(QNetworkReply* reply); + void updateLocationSuccess(QNetworkReply* reply); + void updateLocationFailure(QNetworkReply* reply); + void certificateInfoSuccess(QNetworkReply* reply); + void certificateInfoFailure(QNetworkReply* reply); + void transferAssetToNodeSuccess(QNetworkReply* reply); + void transferAssetToNodeFailure(QNetworkReply* reply); + void transferAssetToUsernameSuccess(QNetworkReply* reply); + void transferAssetToUsernameFailure(QNetworkReply* reply); + void alreadyOwnedSuccess(QNetworkReply* reply); + void alreadyOwnedFailure(QNetworkReply* reply); + void availableUpdatesSuccess(QNetworkReply* reply); + void availableUpdatesFailure(QNetworkReply* reply); + void updateItemSuccess(QNetworkReply* reply); + void updateItemFailure(QNetworkReply* reply); private: - QJsonObject apiResponse(const QString& label, QNetworkReply& reply); - QJsonObject failResponse(const QString& label, QNetworkReply& reply); + QJsonObject apiResponse(const QString& label, QNetworkReply* reply); + QJsonObject failResponse(const QString& label, QNetworkReply* reply); void send(const QString& endpoint, const QString& success, const QString& fail, QNetworkAccessManager::Operation method, AccountManagerAuth::Type authType, QJsonObject request); void keysQuery(const QString& endpoint, const QString& success, const QString& fail, QJsonObject& extraRequestParams); void keysQuery(const QString& endpoint, const QString& success, const QString& fail); diff --git a/interface/src/ui/LoginDialog.cpp b/interface/src/ui/LoginDialog.cpp index 39a5849d85..11fb8efba7 100644 --- a/interface/src/ui/LoginDialog.cpp +++ b/interface/src/ui/LoginDialog.cpp @@ -185,20 +185,20 @@ void LoginDialog::openUrl(const QString& url) const { } } -void LoginDialog::linkCompleted(QNetworkReply& reply) { +void LoginDialog::linkCompleted(QNetworkReply* reply) { emit handleLinkCompleted(); } -void LoginDialog::linkFailed(QNetworkReply& reply) { - emit handleLinkFailed(reply.errorString()); +void LoginDialog::linkFailed(QNetworkReply* reply) { + emit handleLinkFailed(reply->errorString()); } -void LoginDialog::createCompleted(QNetworkReply& reply) { +void LoginDialog::createCompleted(QNetworkReply* reply) { emit handleCreateCompleted(); } -void LoginDialog::createFailed(QNetworkReply& reply) { - emit handleCreateFailed(reply.errorString()); +void LoginDialog::createFailed(QNetworkReply* reply) { + emit handleCreateFailed(reply->errorString()); } void LoginDialog::signup(const QString& email, const QString& username, const QString& password) { @@ -228,7 +228,7 @@ void LoginDialog::signup(const QString& email, const QString& username, const QS QJsonDocument(payload).toJson()); } -void LoginDialog::signupCompleted(QNetworkReply& reply) { +void LoginDialog::signupCompleted(QNetworkReply* reply) { emit handleSignupCompleted(); } @@ -242,10 +242,10 @@ QString errorStringFromAPIObject(const QJsonValue& apiObject) { } } -void LoginDialog::signupFailed(QNetworkReply& reply) { +void LoginDialog::signupFailed(QNetworkReply* reply) { // parse the returned JSON to see what the problem was - auto jsonResponse = QJsonDocument::fromJson(reply.readAll()); + auto jsonResponse = QJsonDocument::fromJson(reply->readAll()); static const QString RESPONSE_DATA_KEY = "data"; diff --git a/interface/src/ui/LoginDialog.h b/interface/src/ui/LoginDialog.h index 5ebf866fbd..ad8cab9699 100644 --- a/interface/src/ui/LoginDialog.h +++ b/interface/src/ui/LoginDialog.h @@ -42,14 +42,14 @@ signals: void handleSignupFailed(QString errorString); public slots: - void linkCompleted(QNetworkReply& reply); - void linkFailed(QNetworkReply& reply); + void linkCompleted(QNetworkReply* reply); + void linkFailed(QNetworkReply* reply); - void createCompleted(QNetworkReply& reply); - void createFailed(QNetworkReply& reply); + void createCompleted(QNetworkReply* reply); + void createFailed(QNetworkReply* reply); - void signupCompleted(QNetworkReply& reply); - void signupFailed(QNetworkReply& reply); + void signupCompleted(QNetworkReply* reply); + void signupFailed(QNetworkReply* reply); protected slots: Q_INVOKABLE bool isSteamRunning() const; diff --git a/interface/src/ui/SnapshotUploader.cpp b/interface/src/ui/SnapshotUploader.cpp index 4613871d25..96af59f923 100644 --- a/interface/src/ui/SnapshotUploader.cpp +++ b/interface/src/ui/SnapshotUploader.cpp @@ -23,11 +23,11 @@ SnapshotUploader::SnapshotUploader(QUrl inWorldLocation, QString pathname) : _pathname(pathname) { } -void SnapshotUploader::uploadSuccess(QNetworkReply& reply) { +void SnapshotUploader::uploadSuccess(QNetworkReply* reply) { const QString STORY_UPLOAD_URL = "/api/v1/user_stories"; // parse the reply for the thumbnail_url - QByteArray contents = reply.readAll(); + QByteArray contents = reply->readAll(); QJsonParseError jsonError; auto doc = QJsonDocument::fromJson(contents, &jsonError); if (jsonError.error == QJsonParseError::NoError) { @@ -74,11 +74,11 @@ void SnapshotUploader::uploadSuccess(QNetworkReply& reply) { } } -void SnapshotUploader::uploadFailure(QNetworkReply& reply) { - QString replyString = reply.readAll(); - qDebug() << "Error " << reply.errorString() << " uploading snapshot " << _pathname << " from " << _inWorldLocation; +void SnapshotUploader::uploadFailure(QNetworkReply* reply) { + QString replyString = reply->readAll(); + qDebug() << "Error " << reply->errorString() << " uploading snapshot " << _pathname << " from " << _inWorldLocation; if (replyString.size() == 0) { - replyString = reply.errorString(); + replyString = reply->errorString(); } replyString = replyString.left(1000); // Only print first 1000 characters of error qDebug() << "Snapshot upload reply error (truncated):" << replyString; @@ -86,17 +86,17 @@ void SnapshotUploader::uploadFailure(QNetworkReply& reply) { this->deleteLater(); } -void SnapshotUploader::createStorySuccess(QNetworkReply& reply) { - QString replyString = reply.readAll(); +void SnapshotUploader::createStorySuccess(QNetworkReply* reply) { + QString replyString = reply->readAll(); emit DependencyManager::get()->snapshotShared(false, replyString); this->deleteLater(); } -void SnapshotUploader::createStoryFailure(QNetworkReply& reply) { - QString replyString = reply.readAll(); - qDebug() << "Error " << reply.errorString() << " uploading snapshot story " << _pathname << " from " << _inWorldLocation; +void SnapshotUploader::createStoryFailure(QNetworkReply* reply) { + QString replyString = reply->readAll(); + qDebug() << "Error " << reply->errorString() << " uploading snapshot story " << _pathname << " from " << _inWorldLocation; if (replyString.size() == 0) { - replyString = reply.errorString(); + replyString = reply->errorString(); } replyString = replyString.left(1000); // Only print first 1000 characters of error qDebug() << "Snapshot story upload reply error (truncated):" << replyString; diff --git a/interface/src/ui/SnapshotUploader.h b/interface/src/ui/SnapshotUploader.h index ae6d5d55ca..d8e72730c7 100644 --- a/interface/src/ui/SnapshotUploader.h +++ b/interface/src/ui/SnapshotUploader.h @@ -21,12 +21,12 @@ class SnapshotUploader : public QObject { public: SnapshotUploader(QUrl inWorldLocation, QString pathname); public slots: - void uploadSuccess(QNetworkReply& reply); - void uploadFailure(QNetworkReply& reply); - void createStorySuccess(QNetworkReply& reply); - void createStoryFailure(QNetworkReply& reply); + void uploadSuccess(QNetworkReply* reply); + void uploadFailure(QNetworkReply* reply); + void createStorySuccess(QNetworkReply* reply); + void createStoryFailure(QNetworkReply* reply); private: QUrl _inWorldLocation; QString _pathname; }; -#endif // hifi_SnapshotUploader_h \ No newline at end of file +#endif // hifi_SnapshotUploader_h diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index 05f0ec12b5..b3ad2d956e 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -53,14 +54,14 @@ JSONCallbackParameters::JSONCallbackParameters(QObject* jsonCallbackReceiver, co jsonCallbackMethod(jsonCallbackMethod), errorCallbackReceiver(errorCallbackReceiver), errorCallbackMethod(errorCallbackMethod), - updateReciever(updateReceiver), + updateReceiver(updateReceiver), updateSlot(updateSlot) { } -QJsonObject AccountManager::dataObjectFromResponse(QNetworkReply &requestReply) { - QJsonObject jsonObject = QJsonDocument::fromJson(requestReply.readAll()).object(); +QJsonObject AccountManager::dataObjectFromResponse(QNetworkReply* requestReply) { + QJsonObject jsonObject = QJsonDocument::fromJson(requestReply->readAll()).object(); static const QString STATUS_KEY = "status"; static const QString DATA_KEY = "data"; @@ -74,8 +75,7 @@ QJsonObject AccountManager::dataObjectFromResponse(QNetworkReply &requestReply) AccountManager::AccountManager(UserAgentGetter userAgentGetter) : _userAgentGetter(userAgentGetter), - _authURL(), - _pendingCallbackMap() + _authURL() { qRegisterMetaType("OAuthAccessToken"); qRegisterMetaTypeStreamOperators("OAuthAccessToken"); @@ -325,73 +325,76 @@ void AccountManager::sendRequest(const QString& path, if (!callbackParams.isEmpty()) { - // if we have information for a callback, insert the callbackParams into our local map - _pendingCallbackMap.insert(networkReply, callbackParams); - - if (callbackParams.updateReciever && !callbackParams.updateSlot.isEmpty()) { - callbackParams.updateReciever->connect(networkReply, SIGNAL(uploadProgress(qint64, qint64)), + if (callbackParams.updateReceiver && !callbackParams.updateSlot.isEmpty()) { + callbackParams.updateReceiver->connect(networkReply, SIGNAL(uploadProgress(qint64, qint64)), callbackParams.updateSlot.toStdString().c_str()); } } - // if we ended up firing of a request, hook up to it now - connect(networkReply, SIGNAL(finished()), SLOT(processReply())); - } -} + // There's a cleaner way to fire the JSON/error callbacks below and ensure that deleteLater is called for the + // request reply - unfortunately it requires Qt 5.10 which the Android build does not support as of 06/26/18 -void AccountManager::processReply() { - QNetworkReply* requestReply = reinterpret_cast(sender()); + if (callbackParams.jsonCallbackReceiver) { + connect(networkReply, &QNetworkReply::finished, callbackParams.jsonCallbackReceiver, [callbackParams, networkReply] { + if (networkReply->error() == QNetworkReply::NoError) { + if (callbackParams.jsonCallbackReceiver && !callbackParams.jsonCallbackMethod.isEmpty()) { + bool invoked = QMetaObject::invokeMethod(callbackParams.jsonCallbackReceiver, + qPrintable(callbackParams.jsonCallbackMethod), + Q_ARG(QNetworkReply*, networkReply)); - if (requestReply->error() == QNetworkReply::NoError) { - if (requestReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) { - _sessionID = requestReply->rawHeader(METAVERSE_SESSION_ID_HEADER); - } - passSuccessToCallback(requestReply); - } else { - passErrorToCallback(requestReply); - } - requestReply->deleteLater(); -} + if (!invoked) { + QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* " + + "on errorCallbackReceiver."; + Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); + } + } else { + if (VERBOSE_HTTP_REQUEST_DEBUGGING) { + qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback."; + qCDebug(networking) << QJsonDocument::fromJson(networkReply->readAll()); + } + } -void AccountManager::passSuccessToCallback(QNetworkReply* requestReply) { - JSONCallbackParameters callbackParams = _pendingCallbackMap.value(requestReply); - - if (callbackParams.jsonCallbackReceiver) { - // invoke the right method on the callback receiver - QMetaObject::invokeMethod(callbackParams.jsonCallbackReceiver, qPrintable(callbackParams.jsonCallbackMethod), - Q_ARG(QNetworkReply&, *requestReply)); - - // remove the related reply-callback group from the map - _pendingCallbackMap.remove(requestReply); - - } else { - if (VERBOSE_HTTP_REQUEST_DEBUGGING) { - qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback."; - qCDebug(networking) << QJsonDocument::fromJson(requestReply->readAll()); + networkReply->deleteLater(); + } + }); } - requestReply->deleteLater(); - } -} + if (callbackParams.errorCallbackReceiver) { + connect(networkReply, &QNetworkReply::finished, callbackParams.errorCallbackReceiver, [callbackParams, networkReply]{ + if (networkReply->error() != QNetworkReply::NoError) { + if (callbackParams.errorCallbackReceiver && !callbackParams.errorCallbackMethod.isEmpty()) { + bool invoked = QMetaObject::invokeMethod(callbackParams.errorCallbackReceiver, + qPrintable(callbackParams.errorCallbackMethod), + Q_ARG(QNetworkReply*, networkReply)); -void AccountManager::passErrorToCallback(QNetworkReply* requestReply) { - JSONCallbackParameters callbackParams = _pendingCallbackMap.value(requestReply); + if (!invoked) { + QString error = "Could not invoke " + callbackParams.errorCallbackMethod + " with QNetworkReply* " + + "on errorCallbackReceiver."; + Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); + } - if (callbackParams.errorCallbackReceiver) { - // invoke the right method on the callback receiver - QMetaObject::invokeMethod(callbackParams.errorCallbackReceiver, qPrintable(callbackParams.errorCallbackMethod), - Q_ARG(QNetworkReply&, *requestReply)); + } else { + if (VERBOSE_HTTP_REQUEST_DEBUGGING) { + qCDebug(networking) << "Received error response from metaverse API that has no matching callback."; + qCDebug(networking) << "Error" << networkReply->error() << "-" << networkReply->errorString(); + qCDebug(networking) << networkReply->readAll(); + } + } - // remove the related reply-callback group from the map - _pendingCallbackMap.remove(requestReply); - } else { - if (VERBOSE_HTTP_REQUEST_DEBUGGING) { - qCDebug(networking) << "Received error response from metaverse API that has no matching callback."; - qCDebug(networking) << "Error" << requestReply->error() << "-" << requestReply->errorString(); - qCDebug(networking) << requestReply->readAll(); + networkReply->deleteLater(); + } + }); } - requestReply->deleteLater(); + // double check if the finished network reply had a session ID in the header and make + // sure that our session ID matches that value if so + connect(networkReply, &QNetworkReply::finished, this, [this, networkReply]{ + if (networkReply->error() == QNetworkReply::NoError) { + if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) { + _sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER); + } + } + }); } } @@ -826,7 +829,7 @@ void AccountManager::processGeneratedKeypair(QByteArray publicKey, QByteArray pr callbackParameters, QByteArray(), requestMultiPart); } -void AccountManager::publicKeyUploadSucceeded(QNetworkReply& reply) { +void AccountManager::publicKeyUploadSucceeded(QNetworkReply* reply) { qCDebug(networking) << "Uploaded public key to Metaverse API. RSA keypair generation is completed."; // public key upload complete - store the matching private key and persist the account to settings @@ -838,23 +841,17 @@ void AccountManager::publicKeyUploadSucceeded(QNetworkReply& reply) { _isWaitingForKeypairResponse = false; emit newKeypair(); - - // delete the reply object now that we are done with it - reply.deleteLater(); } -void AccountManager::publicKeyUploadFailed(QNetworkReply& reply) { +void AccountManager::publicKeyUploadFailed(QNetworkReply* reply) { // the public key upload has failed - qWarning() << "Public key upload failed from AccountManager" << reply.errorString(); + qWarning() << "Public key upload failed from AccountManager" << reply->errorString(); // we aren't waiting for a response any longer _isWaitingForKeypairResponse = false; // clear our pending private key _pendingPrivateKey.clear(); - - // delete the reply object now that we are done with it - reply.deleteLater(); } void AccountManager::handleKeypairGenerationError() { diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index 9068966512..59c5e12c14 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -38,7 +38,7 @@ public: QString jsonCallbackMethod; QObject* errorCallbackReceiver; QString errorCallbackMethod; - QObject* updateReciever; + QObject* updateReceiver; QString updateSlot; }; @@ -90,7 +90,7 @@ public: DataServerAccountInfo& getAccountInfo() { return _accountInfo; } void setAccountInfo(const DataServerAccountInfo &newAccountInfo); - static QJsonObject dataObjectFromResponse(QNetworkReply& requestReply); + static QJsonObject dataObjectFromResponse(QNetworkReply* requestReply); QUuid getSessionID() const { return _sessionID; } void setSessionID(const QUuid& sessionID); @@ -126,11 +126,10 @@ signals: void newKeypair(); private slots: - void processReply(); void handleKeypairGenerationError(); void processGeneratedKeypair(QByteArray publicKey, QByteArray privateKey); - void publicKeyUploadSucceeded(QNetworkReply& reply); - void publicKeyUploadFailed(QNetworkReply& reply); + void publicKeyUploadSucceeded(QNetworkReply* reply); + void publicKeyUploadFailed(QNetworkReply* reply); void generateNewKeypair(bool isUserKeypair = true, const QUuid& domainID = QUuid()); private: @@ -146,8 +145,6 @@ private: UserAgentGetter _userAgentGetter; QUrl _authURL; - - QMap _pendingCallbackMap; DataServerAccountInfo _accountInfo; bool _isWaitingForTokenRefresh { false }; diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 317be194b8..ea0ab44107 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -377,8 +377,8 @@ void AddressManager::handleLookupString(const QString& lookupString, bool fromSu const QString DATA_OBJECT_DOMAIN_KEY = "domain"; -void AddressManager::handleAPIResponse(QNetworkReply& requestReply) { - QJsonObject responseObject = QJsonDocument::fromJson(requestReply.readAll()).object(); +void AddressManager::handleAPIResponse(QNetworkReply* requestReply) { + QJsonObject responseObject = QJsonDocument::fromJson(requestReply->readAll()).object(); QJsonObject dataObject = responseObject["data"].toObject(); // Lookup succeeded, don't keep re-trying it (especially on server restarts) @@ -396,7 +396,7 @@ void AddressManager::handleAPIResponse(QNetworkReply& requestReply) { const char OVERRIDE_PATH_KEY[] = "override_path"; const char LOOKUP_TRIGGER_KEY[] = "lookup_trigger"; -void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const QNetworkReply& reply) { +void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const QNetworkReply* reply) { const QString DATA_OBJECT_PLACE_KEY = "place"; const QString DATA_OBJECT_USER_LOCATION_KEY = "location"; @@ -461,7 +461,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const emit possibleDomainChangeRequiredViaICEForID(iceServerAddress, domainID); } - LookupTrigger trigger = (LookupTrigger) reply.property(LOOKUP_TRIGGER_KEY).toInt(); + LookupTrigger trigger = (LookupTrigger) reply->property(LOOKUP_TRIGGER_KEY).toInt(); // set our current root place id to the ID that came back @@ -495,7 +495,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const } // check if we had a path to override the path returned - QString overridePath = reply.property(OVERRIDE_PATH_KEY).toString(); + QString overridePath = reply->property(OVERRIDE_PATH_KEY).toString(); if (!overridePath.isEmpty() && overridePath != "/") { // make sure we don't re-handle an overriden path if this was a refresh of info from API @@ -543,10 +543,10 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const } } -void AddressManager::handleAPIError(QNetworkReply& errorReply) { - qCDebug(networking) << "AddressManager API error -" << errorReply.error() << "-" << errorReply.errorString(); +void AddressManager::handleAPIError(QNetworkReply* errorReply) { + qCDebug(networking) << "AddressManager API error -" << errorReply->error() << "-" << errorReply->errorString(); - if (errorReply.error() == QNetworkReply::ContentNotFoundError) { + if (errorReply->error() == QNetworkReply::ContentNotFoundError) { // if this is a lookup that has no result, don't keep re-trying it _previousLookup.clear(); @@ -874,14 +874,14 @@ QString AddressManager::getDomainID() const { return DependencyManager::get()->getDomainHandler().getUUID().toString(); } -void AddressManager::handleShareableNameAPIResponse(QNetworkReply& requestReply) { +void AddressManager::handleShareableNameAPIResponse(QNetworkReply* requestReply) { // make sure that this response is for the domain we're currently connected to auto domainID = DependencyManager::get()->getDomainHandler().getUUID(); - if (requestReply.url().toString().contains(uuidStringWithoutCurlyBraces(domainID))) { + if (requestReply->url().toString().contains(uuidStringWithoutCurlyBraces(domainID))) { // check for a name or default name in the API response - QJsonObject responseObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject responseObject = QJsonDocument::fromJson(requestReply->readAll()).object(); QJsonObject domainObject = responseObject["domain"].toObject(); const QString DOMAIN_NAME_KEY = "name"; diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index 38eb7ee670..37b85a9acd 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -417,13 +417,13 @@ signals: void goForwardPossible(bool isPossible); private slots: - void handleAPIResponse(QNetworkReply& requestReply); - void handleAPIError(QNetworkReply& errorReply); + void handleAPIResponse(QNetworkReply* requestReply); + void handleAPIError(QNetworkReply* errorReply); - void handleShareableNameAPIResponse(QNetworkReply& requestReply); + void handleShareableNameAPIResponse(QNetworkReply* requestReply); private: - void goToAddressFromObject(const QVariantMap& addressMap, const QNetworkReply& reply); + void goToAddressFromObject(const QVariantMap& addressMap, const QNetworkReply* reply); // Set host and port, and return `true` if it was changed. bool setHost(const QString& host, LookupTrigger trigger, quint16 port = 0); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 4920ea97c7..c5c49f68fe 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -549,7 +549,7 @@ void NodeList::handleICEConnectionToDomainServer() { _domainHandler.getICEClientID(), _domainHandler.getPendingDomainID()); } -} +} void NodeList::pingPunchForDomainServer() { // make sure if we're here that we actually still need to ping the domain-server diff --git a/libraries/networking/src/UserActivityLogger.cpp b/libraries/networking/src/UserActivityLogger.cpp index 7a92d4bad9..31357591ae 100644 --- a/libraries/networking/src/UserActivityLogger.cpp +++ b/libraries/networking/src/UserActivityLogger.cpp @@ -75,8 +75,8 @@ void UserActivityLogger::logAction(QString action, QJsonObject details, JSONCall params, NULL, multipart); } -void UserActivityLogger::requestError(QNetworkReply& errorReply) { - qCDebug(networking) << errorReply.error() << "-" << errorReply.errorString(); +void UserActivityLogger::requestError(QNetworkReply* errorReply) { + qCDebug(networking) << errorReply->error() << "-" << errorReply->errorString(); } void UserActivityLogger::launch(QString applicationVersion, bool previousSessionCrashed, int previousSessionRuntime) { diff --git a/libraries/networking/src/UserActivityLogger.h b/libraries/networking/src/UserActivityLogger.h index b44c60eba7..e4b91b1e81 100644 --- a/libraries/networking/src/UserActivityLogger.h +++ b/libraries/networking/src/UserActivityLogger.h @@ -50,7 +50,7 @@ public slots: void wentTo(AddressManager::LookupTrigger trigger, QString destinationType, QString destinationName); private slots: - void requestError(QNetworkReply& errorReply); + void requestError(QNetworkReply* errorReply); private: UserActivityLogger(); diff --git a/libraries/ui/src/Tooltip.cpp b/libraries/ui/src/Tooltip.cpp index c0c015e72f..22bc15d72f 100644 --- a/libraries/ui/src/Tooltip.cpp +++ b/libraries/ui/src/Tooltip.cpp @@ -94,9 +94,9 @@ void Tooltip::requestHyperlinkImage() { } } -void Tooltip::handleAPIResponse(QNetworkReply& requestReply) { +void Tooltip::handleAPIResponse(QNetworkReply* requestReply) { // did a preview image come back? - QJsonObject responseObject = QJsonDocument::fromJson(requestReply.readAll()).object(); + QJsonObject responseObject = QJsonDocument::fromJson(requestReply->readAll()).object(); QJsonObject dataObject = responseObject["data"].toObject(); const QString PLACE_KEY = "place"; diff --git a/libraries/ui/src/Tooltip.h b/libraries/ui/src/Tooltip.h index 5e884a7aea..b1bf7b7f3e 100644 --- a/libraries/ui/src/Tooltip.h +++ b/libraries/ui/src/Tooltip.h @@ -49,7 +49,7 @@ signals: void imageURLChanged(); private slots: - void handleAPIResponse(QNetworkReply& requestReply); + void handleAPIResponse(QNetworkReply* requestReply); private: void requestHyperlinkImage(); From 24f92502b5a9c9862274c9957b0e6ba10acfbbaf Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 26 Jun 2018 15:15:25 -0700 Subject: [PATCH 2/4] add warning output of error from AccountManager --- libraries/networking/src/AccountManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index b3ad2d956e..4f68c2c44c 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -345,6 +345,7 @@ void AccountManager::sendRequest(const QString& path, if (!invoked) { QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* " + "on errorCallbackReceiver."; + qCWarning(networking) << error; Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); } } else { @@ -370,6 +371,7 @@ void AccountManager::sendRequest(const QString& path, if (!invoked) { QString error = "Could not invoke " + callbackParams.errorCallbackMethod + " with QNetworkReply* " + "on errorCallbackReceiver."; + qCWarning(networking) << error; Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); } From 70cab27c1ae9f80b6424a95da85878088121a0e7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 26 Jun 2018 15:30:23 -0700 Subject: [PATCH 3/4] add a safeguard deleteLater for QNetworkReply --- libraries/networking/src/AccountManager.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index 4f68c2c44c..8cb48317b6 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -390,11 +390,19 @@ void AccountManager::sendRequest(const QString& path, // double check if the finished network reply had a session ID in the header and make // sure that our session ID matches that value if so - connect(networkReply, &QNetworkReply::finished, this, [this, networkReply]{ + connect(networkReply, &QNetworkReply::finished, this, [this, callbackParams, networkReply]{ if (networkReply->error() == QNetworkReply::NoError) { if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) { _sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER); } + + if (!callbackParams.jsonCallbackReceiver) { + networkReply->deleteLater(); + } + } else { + if (!callbackParams.errorCallbackReceiver) { + networkReply->deleteLater(); + } } }); } From b5254f1ea50f88058d95e6dc1157c09530d2e8f3 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 26 Jun 2018 16:25:05 -0700 Subject: [PATCH 4/4] cleanup race on deleteLater after callback processed --- domain-server/src/DomainGatekeeper.cpp | 9 +- domain-server/src/DomainServer.cpp | 9 +- .../src/DomainServerSettingsManager.cpp | 6 +- interface/src/DiscoverabilityManager.cpp | 4 +- interface/src/commerce/Ledger.cpp | 2 +- interface/src/ui/LoginDialog.cpp | 9 +- interface/src/ui/Snapshot.cpp | 2 +- interface/src/ui/SnapshotUploader.cpp | 2 +- libraries/networking/src/AccountManager.cpp | 108 +++++++----------- libraries/networking/src/AccountManager.h | 12 +- libraries/networking/src/AddressManager.cpp | 5 +- .../networking/src/UserActivityLogger.cpp | 2 +- libraries/ui/src/Tooltip.cpp | 2 +- 13 files changed, 66 insertions(+), 106 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 2d78149c7a..2a6b78744e 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -660,9 +660,8 @@ void DomainGatekeeper::requestUserPublicKey(const QString& username, bool isOpti // even if we have a public key for them right now, request a new one in case it has just changed JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "publicKeyJSONCallback"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "publicKeyJSONErrorCallback"; @@ -893,9 +892,8 @@ void DomainGatekeeper::getGroupMemberships(const QString& username) { JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "getIsGroupMemberJSONCallback"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "getIsGroupMemberErrorCallback"; const QString GET_IS_GROUP_MEMBER_PATH = "api/v1/groups/members/%2"; @@ -960,9 +958,8 @@ void DomainGatekeeper::getIsGroupMemberErrorCallback(QNetworkReply* requestReply void DomainGatekeeper::getDomainOwnerFriendsList() { JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "getDomainOwnerFriendsListJSONCallback"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "getDomainOwnerFriendsListErrorCallback"; const QString GET_FRIENDS_LIST_PATH = "api/v1/user/friends"; diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 5c602b6629..eccd1c1e20 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -514,7 +514,7 @@ void DomainServer::getTemporaryName(bool force) { // request a temporary name from the metaverse auto accountManager = DependencyManager::get(); - JSONCallbackParameters callbackParameters { this, "handleTempDomainSuccess", this, "handleTempDomainError" }; + JSONCallbackParameters callbackParameters { this, "handleTempDomainSuccess", "handleTempDomainError" }; accountManager->sendRequest("/api/v1/domains/temporary", AccountManagerAuth::None, QNetworkAccessManager::PostOperation, callbackParameters); } @@ -1345,7 +1345,7 @@ void DomainServer::sendPendingTransactionsToServer() { JSONCallbackParameters transactionCallbackParams; - transactionCallbackParams.jsonCallbackReceiver = this; + transactionCallbackParams.callbackReceiver = this; transactionCallbackParams.jsonCallbackMethod = "transactionJSONCallback"; while (i != _pendingAssignmentCredits.end()) { @@ -1449,7 +1449,7 @@ void DomainServer::sendHeartbeatToMetaverse(const QString& networkAddress) { DependencyManager::get()->sendRequest(DOMAIN_UPDATE.arg(uuidStringWithoutCurlyBraces(getID())), AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation, - JSONCallbackParameters(nullptr, QString(), this, "handleMetaverseHeartbeatError"), + JSONCallbackParameters(this, QString(), "handleMetaverseHeartbeatError"), domainUpdateJSON.toUtf8()); } @@ -1531,9 +1531,8 @@ void DomainServer::sendICEServerAddressToMetaverseAPI() { // make sure we hear about failure so we can retry JSONCallbackParameters callbackParameters; - callbackParameters.errorCallbackReceiver = this; + callbackParameters.callbackReceiver = this; callbackParameters.errorCallbackMethod = "handleFailedICEServerAddressUpdate"; - callbackParameters.jsonCallbackReceiver = this; callbackParameters.jsonCallbackMethod = "handleSuccessfulICEServerAddressUpdate"; static bool printedIceServerMessage = false; diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index fda9119148..2bcaa8899e 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -1815,9 +1815,8 @@ void DomainServerSettingsManager::apiRefreshGroupInformation() { void DomainServerSettingsManager::apiGetGroupID(const QString& groupName) { JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "apiGetGroupIDJSONCallback"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "apiGetGroupIDErrorCallback"; const QString GET_GROUP_ID_PATH = "api/v1/groups/names/%1"; @@ -1882,9 +1881,8 @@ void DomainServerSettingsManager::apiGetGroupIDErrorCallback(QNetworkReply* requ void DomainServerSettingsManager::apiGetGroupRanks(const QUuid& groupID) { JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "apiGetGroupRanksJSONCallback"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "apiGetGroupRanksErrorCallback"; const QString GET_GROUP_RANKS_PATH = "api/v1/groups/%1/ranks"; diff --git a/interface/src/DiscoverabilityManager.cpp b/interface/src/DiscoverabilityManager.cpp index 191908ab04..e3a99475ef 100644 --- a/interface/src/DiscoverabilityManager.cpp +++ b/interface/src/DiscoverabilityManager.cpp @@ -97,7 +97,7 @@ void DiscoverabilityManager::updateLocation() { locationObject.insert(AVAILABILITY_KEY_IN_LOCATION, findableByString(static_cast(_mode.get()))); JSONCallbackParameters callbackParameters; - callbackParameters.jsonCallbackReceiver = this; + callbackParameters.callbackReceiver = this; callbackParameters.jsonCallbackMethod = "handleHeartbeatResponse"; // figure out if we'll send a fresh location or just a simple heartbeat @@ -121,7 +121,7 @@ void DiscoverabilityManager::updateLocation() { // we still send a heartbeat to the metaverse server for stats collection JSONCallbackParameters callbackParameters; - callbackParameters.jsonCallbackReceiver = this; + callbackParameters.callbackReceiver = this; callbackParameters.jsonCallbackMethod = "handleHeartbeatResponse"; accountManager->sendRequest(API_USER_HEARTBEAT_PATH, AccountManagerAuth::Optional, diff --git a/interface/src/commerce/Ledger.cpp b/interface/src/commerce/Ledger.cpp index 1f36148049..702251f867 100644 --- a/interface/src/commerce/Ledger.cpp +++ b/interface/src/commerce/Ledger.cpp @@ -68,7 +68,7 @@ Handler(updateItem) void Ledger::send(const QString& endpoint, const QString& success, const QString& fail, QNetworkAccessManager::Operation method, AccountManagerAuth::Type authType, QJsonObject request) { auto accountManager = DependencyManager::get(); const QString URL = "/api/v1/commerce/"; - JSONCallbackParameters callbackParams(this, success, this, fail); + JSONCallbackParameters callbackParams(this, success, fail); qCInfo(commerce) << "Sending" << endpoint << QJsonDocument(request).toJson(QJsonDocument::Compact); accountManager->sendRequest(URL + endpoint, authType, diff --git a/interface/src/ui/LoginDialog.cpp b/interface/src/ui/LoginDialog.cpp index 11fb8efba7..4d8592a9d3 100644 --- a/interface/src/ui/LoginDialog.cpp +++ b/interface/src/ui/LoginDialog.cpp @@ -113,9 +113,8 @@ void LoginDialog::linkSteam() { } JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "linkCompleted"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "linkFailed"; const QString LINK_STEAM_PATH = "api/v1/user/steam/link"; @@ -141,9 +140,8 @@ void LoginDialog::createAccountFromStream(QString username) { } JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "createCompleted"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "createFailed"; const QString CREATE_ACCOUNT_FROM_STEAM_PATH = "api/v1/user/steam/create"; @@ -204,9 +202,8 @@ void LoginDialog::createFailed(QNetworkReply* reply) { void LoginDialog::signup(const QString& email, const QString& username, const QString& password) { JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "signupCompleted"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "signupFailed"; QJsonObject payload; diff --git a/interface/src/ui/Snapshot.cpp b/interface/src/ui/Snapshot.cpp index efcc85b23e..60c039ce1f 100644 --- a/interface/src/ui/Snapshot.cpp +++ b/interface/src/ui/Snapshot.cpp @@ -493,7 +493,7 @@ void Snapshot::uploadSnapshot(const QString& filename, const QUrl& href) { multiPart->append(imagePart); auto accountManager = DependencyManager::get(); - JSONCallbackParameters callbackParams(uploader, "uploadSuccess", uploader, "uploadFailure"); + JSONCallbackParameters callbackParams(uploader, "uploadSuccess", "uploadFailure"); accountManager->sendRequest(SNAPSHOT_UPLOAD_URL, AccountManagerAuth::Required, QNetworkAccessManager::PostOperation, callbackParams, nullptr, multiPart); diff --git a/interface/src/ui/SnapshotUploader.cpp b/interface/src/ui/SnapshotUploader.cpp index 96af59f923..694d0fa8f8 100644 --- a/interface/src/ui/SnapshotUploader.cpp +++ b/interface/src/ui/SnapshotUploader.cpp @@ -60,7 +60,7 @@ void SnapshotUploader::uploadSuccess(QNetworkReply* reply) { rootObject.insert("user_story", userStoryObject); auto accountManager = DependencyManager::get(); - JSONCallbackParameters callbackParams(this, "createStorySuccess", this, "createStoryFailure"); + JSONCallbackParameters callbackParams(this, "createStorySuccess", "createStoryFailure"); accountManager->sendRequest(STORY_UPLOAD_URL, AccountManagerAuth::Required, diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index 8cb48317b6..5b3196a2bf 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -47,15 +47,12 @@ Q_DECLARE_METATYPE(JSONCallbackParameters) const QString ACCOUNTS_GROUP = "accounts"; -JSONCallbackParameters::JSONCallbackParameters(QObject* jsonCallbackReceiver, const QString& jsonCallbackMethod, - QObject* errorCallbackReceiver, const QString& errorCallbackMethod, - QObject* updateReceiver, const QString& updateSlot) : - jsonCallbackReceiver(jsonCallbackReceiver), +JSONCallbackParameters::JSONCallbackParameters(QObject* callbackReceiver, + const QString& jsonCallbackMethod, + const QString& errorCallbackMethod) : + callbackReceiver(callbackReceiver), jsonCallbackMethod(jsonCallbackMethod), - errorCallbackReceiver(errorCallbackReceiver), - errorCallbackMethod(errorCallbackMethod), - updateReceiver(updateReceiver), - updateSlot(updateSlot) + errorCallbackMethod(errorCallbackMethod) { } @@ -323,54 +320,50 @@ void AccountManager::sendRequest(const QString& path, } } - - if (!callbackParams.isEmpty()) { - if (callbackParams.updateReceiver && !callbackParams.updateSlot.isEmpty()) { - callbackParams.updateReceiver->connect(networkReply, SIGNAL(uploadProgress(qint64, qint64)), - callbackParams.updateSlot.toStdString().c_str()); + connect(networkReply, &QNetworkReply::finished, this, [this, networkReply] { + // double check if the finished network reply had a session ID in the header and make + // sure that our session ID matches that value if so + if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) { + _sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER); } - } + }); - // There's a cleaner way to fire the JSON/error callbacks below and ensure that deleteLater is called for the - // request reply - unfortunately it requires Qt 5.10 which the Android build does not support as of 06/26/18 - if (callbackParams.jsonCallbackReceiver) { - connect(networkReply, &QNetworkReply::finished, callbackParams.jsonCallbackReceiver, [callbackParams, networkReply] { - if (networkReply->error() == QNetworkReply::NoError) { - if (callbackParams.jsonCallbackReceiver && !callbackParams.jsonCallbackMethod.isEmpty()) { - bool invoked = QMetaObject::invokeMethod(callbackParams.jsonCallbackReceiver, - qPrintable(callbackParams.jsonCallbackMethod), - Q_ARG(QNetworkReply*, networkReply)); + if (callbackParams.isEmpty()) { + connect(networkReply, &QNetworkReply::finished, networkReply, &QNetworkReply::deleteLater); + } else { + // There's a cleaner way to fire the JSON/error callbacks below and ensure that deleteLater is called for the + // request reply - unfortunately it requires Qt 5.10 which the Android build does not support as of 06/26/18 - if (!invoked) { - QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* " - + "on errorCallbackReceiver."; - qCWarning(networking) << error; - Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); - } - } else { - if (VERBOSE_HTTP_REQUEST_DEBUGGING) { - qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback."; - qCDebug(networking) << QJsonDocument::fromJson(networkReply->readAll()); - } + connect(networkReply, &QNetworkReply::finished, callbackParams.callbackReceiver, + [callbackParams, networkReply] { + if (networkReply->error() == QNetworkReply::NoError) { + if (!callbackParams.jsonCallbackMethod.isEmpty()) { + bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, + qPrintable(callbackParams.jsonCallbackMethod), + Q_ARG(QNetworkReply*, networkReply)); + + if (!invoked) { + QString error = "Could not invoke " + callbackParams.jsonCallbackMethod + " with QNetworkReply* " + + "on callbackReceiver."; + qCWarning(networking) << error; + Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); + } + } else { + if (VERBOSE_HTTP_REQUEST_DEBUGGING) { + qCDebug(networking) << "Received JSON response from metaverse API that has no matching callback."; + qCDebug(networking) << QJsonDocument::fromJson(networkReply->readAll()); } - - networkReply->deleteLater(); } - }); - } - - if (callbackParams.errorCallbackReceiver) { - connect(networkReply, &QNetworkReply::finished, callbackParams.errorCallbackReceiver, [callbackParams, networkReply]{ - if (networkReply->error() != QNetworkReply::NoError) { - if (callbackParams.errorCallbackReceiver && !callbackParams.errorCallbackMethod.isEmpty()) { - bool invoked = QMetaObject::invokeMethod(callbackParams.errorCallbackReceiver, + } else { + if (!callbackParams.errorCallbackMethod.isEmpty()) { + bool invoked = QMetaObject::invokeMethod(callbackParams.callbackReceiver, qPrintable(callbackParams.errorCallbackMethod), Q_ARG(QNetworkReply*, networkReply)); if (!invoked) { QString error = "Could not invoke " + callbackParams.errorCallbackMethod + " with QNetworkReply* " - + "on errorCallbackReceiver."; + + "on callbackReceiver."; qCWarning(networking) << error; Q_ASSERT_X(invoked, "AccountManager::passErrorToCallback", qPrintable(error)); } @@ -382,29 +375,11 @@ void AccountManager::sendRequest(const QString& path, qCDebug(networking) << networkReply->readAll(); } } - - networkReply->deleteLater(); } + + networkReply->deleteLater(); }); } - - // double check if the finished network reply had a session ID in the header and make - // sure that our session ID matches that value if so - connect(networkReply, &QNetworkReply::finished, this, [this, callbackParams, networkReply]{ - if (networkReply->error() == QNetworkReply::NoError) { - if (networkReply->hasRawHeader(METAVERSE_SESSION_ID_HEADER)) { - _sessionID = networkReply->rawHeader(METAVERSE_SESSION_ID_HEADER); - } - - if (!callbackParams.jsonCallbackReceiver) { - networkReply->deleteLater(); - } - } else { - if (!callbackParams.errorCallbackReceiver) { - networkReply->deleteLater(); - } - } - }); } } @@ -830,9 +805,8 @@ void AccountManager::processGeneratedKeypair(QByteArray publicKey, QByteArray pr // setup callback parameters so we know once the keypair upload has succeeded or failed JSONCallbackParameters callbackParameters; - callbackParameters.jsonCallbackReceiver = this; + callbackParameters.callbackReceiver = this; callbackParameters.jsonCallbackMethod = "publicKeyUploadSucceeded"; - callbackParameters.errorCallbackReceiver = this; callbackParameters.errorCallbackMethod = "publicKeyUploadFailed"; sendRequest(uploadPath, AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation, diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index 59c5e12c14..a79b69fe2b 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -28,18 +28,14 @@ class JSONCallbackParameters { public: - JSONCallbackParameters(QObject* jsonCallbackReceiver = nullptr, const QString& jsonCallbackMethod = QString(), - QObject* errorCallbackReceiver = nullptr, const QString& errorCallbackMethod = QString(), - QObject* updateReceiver = nullptr, const QString& updateSlot = QString()); + JSONCallbackParameters(QObject* callbackReceiver = nullptr, const QString& jsonCallbackMethod = QString(), + const QString& errorCallbackMethod = QString()); - bool isEmpty() const { return !jsonCallbackReceiver && !errorCallbackReceiver; } + bool isEmpty() const { return !callbackReceiver; } - QObject* jsonCallbackReceiver; + QObject* callbackReceiver; QString jsonCallbackMethod; - QObject* errorCallbackReceiver; QString errorCallbackMethod; - QObject* updateReceiver; - QString updateSlot; }; namespace AccountManagerAuth { diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index ea0ab44107..3fe75c5495 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -215,9 +215,8 @@ const JSONCallbackParameters& AddressManager::apiCallbackParameters() { static JSONCallbackParameters callbackParams; if (!hasSetupParameters) { - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "handleAPIResponse"; - callbackParams.errorCallbackReceiver = this; callbackParams.errorCallbackMethod = "handleAPIError"; } @@ -917,7 +916,7 @@ void AddressManager::lookupShareableNameForDomainID(const QUuid& domainID) { // no error callback handling // in the case of an error we simply assume there is no default place name - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "handleShareableNameAPIResponse"; DependencyManager::get()->sendRequest(GET_DOMAIN_ID.arg(uuidStringWithoutCurlyBraces(domainID)), diff --git a/libraries/networking/src/UserActivityLogger.cpp b/libraries/networking/src/UserActivityLogger.cpp index 31357591ae..a5ee417939 100644 --- a/libraries/networking/src/UserActivityLogger.cpp +++ b/libraries/networking/src/UserActivityLogger.cpp @@ -65,7 +65,7 @@ void UserActivityLogger::logAction(QString action, QJsonObject details, JSONCall // if no callbacks specified, call our owns if (params.isEmpty()) { - params.errorCallbackReceiver = this; + params.callbackReceiver = this; params.errorCallbackMethod = "requestError"; } diff --git a/libraries/ui/src/Tooltip.cpp b/libraries/ui/src/Tooltip.cpp index 22bc15d72f..bd2c4e6d8f 100644 --- a/libraries/ui/src/Tooltip.cpp +++ b/libraries/ui/src/Tooltip.cpp @@ -83,7 +83,7 @@ void Tooltip::requestHyperlinkImage() { auto accountManager = DependencyManager::get(); JSONCallbackParameters callbackParams; - callbackParams.jsonCallbackReceiver = this; + callbackParams.callbackReceiver = this; callbackParams.jsonCallbackMethod = "handleAPIResponse"; accountManager->sendRequest(GET_PLACE.arg(_title),