Revert "Revert "refresh API info during re-connect - case 570""

This reverts commit 1624146fc2.

libraries/networking/src/AddressManager.cpp

@@ -144,12 +144,21 @@ 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())) {
+        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 {
             // 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)) {
+                                     + (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();
 
                 // If the host changed then we have already saved to history
                 if (hostChanged) {
@@ -165,10 +174,16 @@ 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);
@@ -180,9 +195,13 @@ 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;
     }
 
@@ -276,7 +295,7 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const
 
                     qCDebug(networking) << "Possible domain change required to connect to" << domainHostname
                         << "on" << domainPort;
-                    emit possibleDomainChangeRequired(domainHostname, domainPort);
+                    emit possibleDomainChangeRequired(domainHostname, domainPort, domainID);
                 } else {
                     QString iceServerAddress = domainObject[DOMAIN_ICE_SERVER_ADDRESS_KEY].toString();
 
@@ -315,7 +334,10 @@ void AddressManager::goToAddressFromObject(const QVariantMap& dataObject, const
                 QString overridePath = reply.property(OVERRIDE_PATH_KEY).toString();
 
                 if (!overridePath.isEmpty()) {
-                    handlePath(overridePath, trigger);
+                    // 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);
+                    }
                 } else {
                     // take the path that came back
                     const QString PLACE_PATH_KEY = "path";
@@ -598,7 +620,7 @@ bool AddressManager::setDomainInfo(const QString& hostname, quint16 port, Lookup
 
     DependencyManager::get<NodeList>()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::HandleAddress);
 
-    emit possibleDomainChangeRequired(hostname, port);
+    emit possibleDomainChangeRequired(hostname, port, QUuid());
 
     return hostChanged;
 }
@@ -618,6 +640,13 @@ 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());
 }
@@ -629,7 +658,10 @@ 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) {
+    if (trigger != LookupTrigger::StartupFromSettings
+        && trigger != LookupTrigger::DomainPathResponse
+        && trigger != LookupTrigger::AttemptedRefresh) {
+
         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

libraries/networking/src/AddressManager.h

@@ -48,7 +48,8 @@ public:
         Forward,
         StartupFromSettings,
         DomainPathResponse,
-        Internal
+        Internal,
+        AttemptedRefresh
     };
 
     bool isConnected();
@@ -89,6 +90,8 @@ public slots:
 
     void goToUser(const QString& username);
 
+    void refreshPreviousLookup();
+
     void storeCurrentAddress();
 
     void copyAddress();
@@ -99,7 +102,7 @@ signals:
     void lookupResultIsOffline();
     void lookupResultIsNotFound();
 
-    void possibleDomainChangeRequired(const QString& newHostname, quint16 newPort);
+    void possibleDomainChangeRequired(const QString& newHostname, quint16 newPort, const QUuid& domainID);
     void possibleDomainChangeRequiredViaICEForID(const QString& iceServerHostname, const QUuid& domainID);
 
     void locationChangeRequired(const glm::vec3& newPosition,
@@ -152,6 +155,8 @@ private:
     quint64 _lastBackPush = 0;
 
     QString _newHostLookupPath;
+    
+    QUrl _previousLookup;
 };
 
 #endif // hifi_AddressManager_h

libraries/networking/src/DomainHandler.cpp

@@ -28,16 +28,8 @@
 
 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");
@@ -105,7 +97,7 @@ void DomainHandler::hardReset() {
     softReset();
 
     qCDebug(networking) << "Hard reset in NodeList DomainHandler.";
-    _iceDomainID = QUuid();
+    _pendingDomainID = QUuid();
     _iceServerSockAddr = HifiSockAddr();
     _hostname = QString();
     _sockAddr.clear();
@@ -139,7 +131,7 @@ void DomainHandler::setUUID(const QUuid& uuid) {
     }
 }
 
-void DomainHandler::setHostnameAndPort(const QString& hostname, quint16 port) {
+void DomainHandler::setSocketAndID(const QString& hostname, quint16 port, const QUuid& domainID) {
 
     if (hostname != _hostname || _sockAddr.getPort() != port) {
         // re-set the domain info so that auth information is reloaded
@@ -171,6 +163,8 @@ void DomainHandler::setHostnameAndPort(const QString& hostname, quint16 port) {
         // grab the port by reading the string after the colon
         _sockAddr.setPort(port);
     }
+
+    _pendingDomainID = domainID;
 }
 
 void DomainHandler::setIceServerHostnameAndID(const QString& iceServerHostname, const QUuid& id) {
@@ -181,7 +175,7 @@ void DomainHandler::setIceServerHostnameAndID(const QString& iceServerHostname,
         // refresh our ICE client UUID to something new
         _iceClientID = QUuid::createUuid();
         
-        _iceDomainID = id;
+        _pendingDomainID = id;
 
         HifiSockAddr* replaceableSockAddr = &_iceServerSockAddr;
         replaceableSockAddr->~HifiSockAddr();
@@ -343,7 +337,7 @@ void DomainHandler::processICEResponsePacket(QSharedPointer<ReceivedMessage> mes
 
     DependencyManager::get<NodeList>()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::ReceiveDSPeerInformation);
 
-    if (_icePeer.getUUID() != _iceDomainID) {
+    if (_icePeer.getUUID() != _pendingDomainID) {
         qCDebug(networking) << "Received a network peer with ID that does not match current domain. Will not attempt connection.";
         _icePeer.reset();
     } else {

libraries/networking/src/DomainHandler.h

@@ -59,7 +59,7 @@ public:
     const QUuid& getAssignmentUUID() const { return _assignmentUUID; }
     void setAssignmentUUID(const QUuid& assignmentUUID) { _assignmentUUID = assignmentUUID; }
 
-    const QUuid& getICEDomainID() const { return _iceDomainID; }
+    const QUuid& getPendingDomainID() const { return _pendingDomainID; }
 
     const QUuid& getICEClientID() const { return _iceClientID; }
 
@@ -94,7 +94,7 @@ public:
     };
 
 public slots:
-    void setHostnameAndPort(const QString& hostname, quint16 port = DEFAULT_DOMAIN_SERVER_PORT);
+    void setSocketAndID(const QString& hostname, quint16 port = DEFAULT_DOMAIN_SERVER_PORT, const QUuid& id = QUuid());
     void setIceServerHostnameAndID(const QString& iceServerHostname, const QUuid& id);
 
     void processSettingsPacketList(QSharedPointer<ReceivedMessage> packetList);
@@ -136,11 +136,11 @@ private:
     HifiSockAddr _sockAddr;
     QUuid _assignmentUUID;
     QUuid _connectionToken;
-    QUuid _iceDomainID;
+    QUuid _pendingDomainID; // ID of domain being connected to, via ICE or direct connection
     QUuid _iceClientID;
     HifiSockAddr _iceServerSockAddr;
     NetworkPeer _icePeer;
-    bool _isConnected;
+    bool _isConnected { false };
     QJsonObject _settingsObject;
     QString _pendingPath;
     QTimer _settingsTimer;

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::setHostnameAndPort);
+            &_domainHandler, &DomainHandler::setSocketAndID);
 
     connect(addressManager.data(), &AddressManager::possibleDomainChangeRequiredViaICEForID,
             &_domainHandler, &DomainHandler::setIceServerHostnameAndID);
@@ -355,6 +355,14 @@ 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<AddressManager>()->refreshPreviousLookup();
+    }
 }
 
 void NodeList::handleDSPathQuery(const QString& newPath) {
@@ -462,7 +470,7 @@ void NodeList::handleICEConnectionToDomainServer() {
 
         LimitedNodeList::sendPeerQueryToIceServer(_domainHandler.getICEServerSockAddr(),
                                                   _domainHandler.getICEClientID(),
-                                                  _domainHandler.getICEDomainID());
+                                                  _domainHandler.getPendingDomainID());
     }
 }
 
@@ -475,7 +483,7 @@ void NodeList::pingPunchForDomainServer() {
 
         if (_domainHandler.getICEPeer().getConnectionAttempts() == 0) {
             qCDebug(networking) << "Sending ping packets to establish connectivity with domain-server with ID"
-                << uuidStringWithoutCurlyBraces(_domainHandler.getICEDomainID());
+                << uuidStringWithoutCurlyBraces(_domainHandler.getPendingDomainID());
         } 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