Improve ReverseResolution class

* Use QHostAddress type for IP
* Avoid double lookup
* Use larger cache size

src/base/net/reverseresolution.cpp

@@ -28,19 +28,19 @@
 
 #include "reverseresolution.h"
 
-#include <QDebug>
+#include <QHostAddress>
 #include <QHostInfo>
 #include <QString>
 
-const int CACHE_SIZE = 500;
+const int CACHE_SIZE = 2048;
 
 using namespace Net;
 
 namespace
 {
-    bool isUsefulHostName(const QString &hostname, const QString &ip)
+    bool isUsefulHostName(const QString &hostname, const QHostAddress &ip)
     {
-        return (!hostname.isEmpty() && (hostname != ip));
+        return (!hostname.isEmpty() && (hostname != ip.toString()));
     }
 }
 
@@ -52,37 +52,40 @@ ReverseResolution::ReverseResolution(QObject *parent)
 
 ReverseResolution::~ReverseResolution()
 {
-    qDebug("Deleting host name resolver...");
 }
 
-void ReverseResolution::resolve(const QString &ip)
+void ReverseResolution::resolve(const QHostAddress &ip)
 {
-    if (m_cache.contains(ip)) {
-        const QString &hostname = *m_cache.object(ip);
-        qDebug() << "Resolved host name using cache: " << ip << " -> " << hostname;
-        if (isUsefulHostName(hostname, ip))
-            emit ipResolved(ip, hostname);
-    }
-    else {
-        // Actually resolve the ip
-        m_lookups.insert(QHostInfo::lookupHost(ip, this, &ReverseResolution::hostResolved), ip);
+    if (ip.isNull())
+        return;
+
+    const QString *hostname = m_cache.object(ip);
+    if (hostname) {
+        if (hostname->isEmpty())
+            ;  // resolution didn't get meaningful results, so do nothing
+        else
+            emit ipResolved(ip, *hostname);
+        return;
     }
+
+    // do reverse lookup: IP -> hostname
+    const int lookupId = QHostInfo::lookupHost(ip.toString(), this, &ReverseResolution::hostResolved);
+    m_lookups.insert(lookupId, ip);
 }
 
 void ReverseResolution::hostResolved(const QHostInfo &host)
 {
-    const QString ip = m_lookups.take(host.lookupId());
-    Q_ASSERT(!ip.isNull());
+    const QHostAddress ip = m_lookups.take(host.lookupId());
 
-    if (host.error() != QHostInfo::NoError) {
-        qDebug() << "DNS Reverse resolution error: " << host.errorString();
+    if (host.error() != QHostInfo::NoError)
         return;
-    }
 
     const QString hostname = host.hostName();
-
-    qDebug() << Q_FUNC_INFO << ip << QString("->") << hostname;
+    if (isUsefulHostName(hostname, ip)) {
         m_cache.insert(ip, new QString(hostname));
-    if (isUsefulHostName(hostname, ip))
         emit ipResolved(ip, hostname);
+    }
+    else {
+        m_cache.insert(ip, new QString());
+    }
 }

src/base/net/reverseresolution.h

@@ -32,6 +32,7 @@
 #include <QCache>
 #include <QObject>
 
+class QHostAddress;
 class QHostInfo;
 class QString;
 
@@ -46,17 +47,17 @@ namespace Net
         explicit ReverseResolution(QObject *parent = nullptr);
         ~ReverseResolution();
 
-        void resolve(const QString &ip);
+        void resolve(const QHostAddress &ip);
 
     signals:
-        void ipResolved(const QString &ip, const QString &hostname);
+        void ipResolved(const QHostAddress &ip, const QString &hostname);
 
     private slots:
         void hostResolved(const QHostInfo &host);
 
     private:
-        QHash<int /* LookupID */, QString /* IP */> m_lookups;
-        QCache<QString /* IP */, QString /* HostName */> m_cache;
+        QHash<int, QHostAddress> m_lookups;  // <LookupID, IP>
+        QCache<QHostAddress, QString> m_cache;  // <IP, HostName>
     };
 }
 

src/gui/properties/peerlistwidget.cpp

@@ -33,6 +33,7 @@
 #include <QApplication>
 #include <QClipboard>
 #include <QHeaderView>
+#include <QHostAddress>
 #include <QMenu>
 #include <QMessageBox>
 #include <QSet>
@@ -405,7 +406,7 @@ void PeerListWidget::updatePeer(const BitTorrent::TorrentHandle *torrent, const
     m_listModel->setData(m_listModel->index(row, PeerListDelegate::DOWNLOADING_PIECE), downloadingFiles.join('\n'), Qt::ToolTipRole);
 
     if (m_resolver)
-        m_resolver->resolve(peerIp);
+        m_resolver->resolve(peerEndpoint.address.ip);
 
     if (m_resolveCountries) {
         const QIcon icon = UIThemeManager::instance()->getFlagIcon(peer.country());
@@ -417,10 +418,10 @@ void PeerListWidget::updatePeer(const BitTorrent::TorrentHandle *torrent, const
     }
 }
 
-void PeerListWidget::handleResolved(const QString &ip, const QString &hostname)
+void PeerListWidget::handleResolved(const QHostAddress &ip, const QString &hostname) const
 {
     for (auto iter = m_peerItems.cbegin(); iter != m_peerItems.cend(); ++iter) {
-        if (iter.key().address.ip.toString() == ip) {
+        if (iter.key().address.ip == ip) {
             QStandardItem *item {iter.value()};
             item->setData(hostname, Qt::DisplayRole);
             break;

src/gui/properties/peerlistwidget.h

@@ -32,6 +32,7 @@
 #include <QHash>
 #include <QTreeView>
 
+class QHostAddress;
 class QStandardItem;
 class QStandardItemModel;
 
@@ -72,7 +73,7 @@ private slots:
     void banSelectedPeers();
     void copySelectedPeers();
     void handleSortColumnChanged(int col);
-    void handleResolved(const QString &ip, const QString &hostname);
+    void handleResolved(const QHostAddress &ip, const QString &hostname) const;
 
 private:
     void updatePeer(const BitTorrent::TorrentHandle *torrent, const BitTorrent::PeerInfo &peer, bool &isNewPeer);