From b8c80e222286a16ef56a5ecfcf7c95aa04a9918e Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 31 May 2016 20:39:16 -0700 Subject: [PATCH 1/2] Revert "refresh API info during re-connect - case 570" This reverts commit e8d7f9614aed57bad25f940c82282c25b91ed92e. --- libraries/networking/src/AddressManager.cpp | 44 +++------------------ libraries/networking/src/AddressManager.h | 9 +---- libraries/networking/src/DomainHandler.cpp | 18 ++++++--- libraries/networking/src/DomainHandler.h | 10 ++--- libraries/networking/src/NodeList.cpp | 14 ++----- 5 files changed, 28 insertions(+), 67 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 80989acd2c..1a452999e4 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -144,21 +144,12 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { // 4. domain network address (IP or dns resolvable hostname) // use our regex'ed helpers to figure out what we're supposed to do with this - if (handleUsername(lookupUrl.authority())) { - // handled a username for lookup - - // in case we're failing to connect to where we thought this user was - // store their username as previous lookup so we can refresh their location via API - _previousLookup = lookupUrl; - } else { + if (!handleUsername(lookupUrl.authority())) { // we're assuming this is either a network address or global place name // check if it is a network address first bool hostChanged; if (handleNetworkAddress(lookupUrl.host() - + (lookupUrl.port() == -1 ? "" : ":" + QString::number(lookupUrl.port())), trigger, hostChanged)) { - - // a network address lookup clears the previous lookup since we don't expect to re-attempt it - _previousLookup.clear(); + + (lookupUrl.port() == -1 ? "" : ":" + QString::number(lookupUrl.port())), trigger, hostChanged)) { // If the host changed then we have already saved to history if (hostChanged) { @@ -174,16 +165,10 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { // we may have a path that defines a relative viewpoint - if so we should jump to that now handlePath(path, trigger); } else if (handleDomainID(lookupUrl.host())){ - // store this domain ID as the previous lookup in case we're failing to connect and want to refresh API info - _previousLookup = lookupUrl; - // no place name - this is probably a domain ID // try to look up the domain ID on the metaverse API attemptDomainIDLookup(lookupUrl.host(), lookupUrl.path(), trigger); } else { - // store this place name as the previous lookup in case we fail to connect and want to refresh API info - _previousLookup = lookupUrl; - // wasn't an address - lookup the place name // we may have a path that defines a relative viewpoint - pass that through the lookup so we can go to it after attemptPlaceNameLookup(lookupUrl.host(), lookupUrl.path(), trigger); @@ -195,13 +180,9 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { } else if (lookupUrl.toString().startsWith('/')) { qCDebug(networking) << "Going to relative path" << lookupUrl.path(); - // a path lookup clears the previous lookup since we don't expect to re-attempt it - _previousLookup.clear(); - // if this is a relative path then handle it as a relative viewpoint handlePath(lookupUrl.path(), trigger, true); emit lookupResultsFinished(); - return true; } @@ -295,7 +276,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const qCDebug(networking) << "Possible domain change required to connect to" << domainHostname << "on" << domainPort; - emit possibleDomainChangeRequired(domainHostname, domainPort, domainID); + emit possibleDomainChangeRequired(domainHostname, domainPort); } else { QString iceServerAddress = domainObject[DOMAIN_ICE_SERVER_ADDRESS_KEY].toString(); @@ -334,10 +315,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const QString overridePath = reply.property(OVERRIDE_PATH_KEY).toString(); if (!overridePath.isEmpty()) { - // make sure we don't re-handle an overriden path if this was a refresh of info from API - if (trigger != LookupTrigger::AttemptedRefresh) { - handlePath(overridePath, trigger); - } + handlePath(overridePath, trigger); } else { // take the path that came back const QString PLACE_PATH_KEY = "path"; @@ -620,7 +598,7 @@ bool AddressManager::setDomainInfo(const QString& hostname, quint16 port, Lookup DependencyManager::get()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::HandleAddress); - emit possibleDomainChangeRequired(hostname, port, QUuid()); + emit possibleDomainChangeRequired(hostname, port); return hostChanged; } @@ -640,13 +618,6 @@ void AddressManager::goToUser(const QString& username) { QByteArray(), nullptr, requestParams); } -void AddressManager::refreshPreviousLookup() { - // if we have a non-empty previous lookup, fire it again now (but don't re-store it in the history) - if (!_previousLookup.isEmpty()) { - handleUrl(_previousLookup, LookupTrigger::AttemptedRefresh); - } -} - void AddressManager::copyAddress() { QApplication::clipboard()->setText(currentAddress().toString()); } @@ -658,10 +629,7 @@ void AddressManager::copyPath() { void AddressManager::addCurrentAddressToHistory(LookupTrigger trigger) { // if we're cold starting and this is called for the first address (from settings) we don't do anything - if (trigger != LookupTrigger::StartupFromSettings - && trigger != LookupTrigger::DomainPathResponse - && trigger != LookupTrigger::AttemptedRefresh) { - + if (trigger != LookupTrigger::StartupFromSettings && trigger != LookupTrigger::DomainPathResponse) { if (trigger == LookupTrigger::Back) { // we're about to push to the forward stack // if it's currently empty emit our signal to say that going forward is now possible diff --git a/libraries/networking/src/AddressManager.h b/libraries/networking/src/AddressManager.h index a3aaee3ba2..643924ff5c 100644 --- a/libraries/networking/src/AddressManager.h +++ b/libraries/networking/src/AddressManager.h @@ -48,8 +48,7 @@ public: Forward, StartupFromSettings, DomainPathResponse, - Internal, - AttemptedRefresh + Internal }; bool isConnected(); @@ -90,8 +89,6 @@ public slots: void goToUser(const QString& username); - void refreshPreviousLookup(); - void storeCurrentAddress(); void copyAddress(); @@ -102,7 +99,7 @@ signals: void lookupResultIsOffline(); void lookupResultIsNotFound(); - void possibleDomainChangeRequired(const QString& newHostname, quint16 newPort, const QUuid& domainID); + void possibleDomainChangeRequired(const QString& newHostname, quint16 newPort); void possibleDomainChangeRequiredViaICEForID(const QString& iceServerHostname, const QUuid& domainID); void locationChangeRequired(const glm::vec3& newPosition, @@ -155,8 +152,6 @@ private: quint64 _lastBackPush = 0; QString _newHostLookupPath; - - QUrl _previousLookup; }; #endif // hifi_AddressManager_h diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index 1efcfc7f27..4f85296f03 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -28,8 +28,16 @@ DomainHandler::DomainHandler(QObject* parent) : QObject(parent), + _uuid(), _sockAddr(HifiSockAddr(QHostAddress::Null, DEFAULT_DOMAIN_SERVER_PORT)), + _assignmentUUID(), + _connectionToken(), + _iceDomainID(), + _iceClientID(), + _iceServerSockAddr(), _icePeer(this), + _isConnected(false), + _settingsObject(), _settingsTimer(this) { _sockAddr.setObjectName("DomainServer"); @@ -97,7 +105,7 @@ void DomainHandler::hardReset() { softReset(); qCDebug(networking) << "Hard reset in NodeList DomainHandler."; - _pendingDomainID = QUuid(); + _iceDomainID = QUuid(); _iceServerSockAddr = HifiSockAddr(); _hostname = QString(); _sockAddr.clear(); @@ -131,7 +139,7 @@ void DomainHandler::setUUID(const QUuid& uuid) { } } -void DomainHandler::setSocketAndID(const QString& hostname, quint16 port, const QUuid& domainID) { +void DomainHandler::setHostnameAndPort(const QString& hostname, quint16 port) { if (hostname != _hostname || _sockAddr.getPort() != port) { // re-set the domain info so that auth information is reloaded @@ -163,8 +171,6 @@ void DomainHandler::setSocketAndID(const QString& hostname, quint16 port, const // grab the port by reading the string after the colon _sockAddr.setPort(port); } - - _pendingDomainID = domainID; } void DomainHandler::setIceServerHostnameAndID(const QString& iceServerHostname, const QUuid& id) { @@ -175,7 +181,7 @@ void DomainHandler::setIceServerHostnameAndID(const QString& iceServerHostname, // refresh our ICE client UUID to something new _iceClientID = QUuid::createUuid(); - _pendingDomainID = id; + _iceDomainID = id; HifiSockAddr* replaceableSockAddr = &_iceServerSockAddr; replaceableSockAddr->~HifiSockAddr(); @@ -337,7 +343,7 @@ void DomainHandler::processICEResponsePacket(QSharedPointer mes DependencyManager::get()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::ReceiveDSPeerInformation); - if (_icePeer.getUUID() != _pendingDomainID) { + if (_icePeer.getUUID() != _iceDomainID) { qCDebug(networking) << "Received a network peer with ID that does not match current domain. Will not attempt connection."; _icePeer.reset(); } else { diff --git a/libraries/networking/src/DomainHandler.h b/libraries/networking/src/DomainHandler.h index 226186f1d0..bcee7668d1 100644 --- a/libraries/networking/src/DomainHandler.h +++ b/libraries/networking/src/DomainHandler.h @@ -58,8 +58,8 @@ public: const QUuid& getAssignmentUUID() const { return _assignmentUUID; } void setAssignmentUUID(const QUuid& assignmentUUID) { _assignmentUUID = assignmentUUID; } - - const QUuid& getPendingDomainID() const { return _pendingDomainID; } + + const QUuid& getICEDomainID() const { return _iceDomainID; } const QUuid& getICEClientID() const { return _iceClientID; } @@ -94,7 +94,7 @@ public: }; public slots: - void setSocketAndID(const QString& hostname, quint16 port = DEFAULT_DOMAIN_SERVER_PORT, const QUuid& id = QUuid()); + void setHostnameAndPort(const QString& hostname, quint16 port = DEFAULT_DOMAIN_SERVER_PORT); void setIceServerHostnameAndID(const QString& iceServerHostname, const QUuid& id); void processSettingsPacketList(QSharedPointer packetList); @@ -136,11 +136,11 @@ private: HifiSockAddr _sockAddr; QUuid _assignmentUUID; QUuid _connectionToken; - QUuid _pendingDomainID; // ID of domain being connected to, via ICE or direct connection + QUuid _iceDomainID; QUuid _iceClientID; HifiSockAddr _iceServerSockAddr; NetworkPeer _icePeer; - bool _isConnected { false }; + bool _isConnected; QJsonObject _settingsObject; QString _pendingPath; QTimer _settingsTimer; diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 082200fccc..16a4083b08 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -50,7 +50,7 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned // handle domain change signals from AddressManager connect(addressManager.data(), &AddressManager::possibleDomainChangeRequired, - &_domainHandler, &DomainHandler::setSocketAndID); + &_domainHandler, &DomainHandler::setHostnameAndPort); connect(addressManager.data(), &AddressManager::possibleDomainChangeRequiredViaICEForID, &_domainHandler, &DomainHandler::setIceServerHostnameAndID); @@ -355,14 +355,6 @@ void NodeList::sendDomainServerCheckIn() { // increment the count of un-replied check-ins _numNoReplyDomainCheckIns++; } - - if (!_publicSockAddr.isNull() && !_domainHandler.isConnected() && !_domainHandler.getPendingDomainID().isNull()) { - // if we aren't connected to the domain-server, and we have an ID - // (that we presume belongs to a domain in the HF Metaverse) - // we request connection information for the domain every so often to make sure what we have is up to date - - DependencyManager::get()->refreshPreviousLookup(); - } } void NodeList::handleDSPathQuery(const QString& newPath) { @@ -470,7 +462,7 @@ void NodeList::handleICEConnectionToDomainServer() { LimitedNodeList::sendPeerQueryToIceServer(_domainHandler.getICEServerSockAddr(), _domainHandler.getICEClientID(), - _domainHandler.getPendingDomainID()); + _domainHandler.getICEDomainID()); } } @@ -483,7 +475,7 @@ void NodeList::pingPunchForDomainServer() { if (_domainHandler.getICEPeer().getConnectionAttempts() == 0) { qCDebug(networking) << "Sending ping packets to establish connectivity with domain-server with ID" - << uuidStringWithoutCurlyBraces(_domainHandler.getPendingDomainID()); + << uuidStringWithoutCurlyBraces(_domainHandler.getICEDomainID()); } else { if (_domainHandler.getICEPeer().getConnectionAttempts() % NUM_DOMAIN_SERVER_PINGS_BEFORE_RESET == 0) { // if we have then nullify the domain handler's network peer and send a fresh ICE heartbeat From 3119b654dc9b0053c5c1bdee7668246ded0fcb87 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 31 May 2016 20:50:12 -0700 Subject: [PATCH 2/2] additional revert related change --- libraries/networking/src/AddressManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index 1a452999e4..1b7ed11cce 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -362,7 +362,7 @@ void AddressManager::handleAPIError(QNetworkReply& errorReply) { if (errorReply.error() == QNetworkReply::ContentNotFoundError) { // if this is a lookup that has no result, don't keep re-trying it - _previousLookup.clear(); + //_previousLookup.clear(); emit lookupResultIsNotFound(); }