Implement changes requested in review

- Make Peer::_connectedSince, Peer::_buildDate, Peer::_clientVersion
  private, add getters/setters
- Correct parameter naming in doxygen comment for
  SignalProxy::restrictTargetPeers
- Remove SignalProxy::_peers
- Initialize SignalProxy::_sourcePeer to nullptr
- Add Q_UNUSED for peer parameter for CoreSession::changePassword
- Correct indentation in src/qtui/CMakeLists
- Use pragma in src/qtui/coresessionwidget.h
- Fix header ordering in src/qtui/coresessionwidget.h
- Fix header ordering in src/qtui/coresessionwidget.cpp

diff --git a/src/common/peer.cpp b/src/common/peer.cpp
index 54e0c59..25ff814 100644 (file)
--- a/src/common/peer.cpp
+++ b/src/common/peer.cpp
@@ -33,6 +33,37 @@ AuthHandler *Peer::authHandler() const
     return _authHandler;
 }
 
+const QDateTime &Peer::connectedSince() const {
+    return _connectedSince;
+}
+
+void Peer::setConnectedSince(const QDateTime &connectedSince) {
+    _connectedSince = connectedSince;
+}
+
+const QString &Peer::buildDate() const {
+    return _buildDate;
+}
+
+void Peer::setBuildDate(const QString &buildDate) {
+    _buildDate = buildDate;
+}
+
+const QString &Peer::clientVersion() const {
+    return _clientVersion;
+}
+
+void Peer::setClientVersion(const QString &clientVersion) {
+    _clientVersion = clientVersion;
+}
+
+int Peer::id() const {
+    return _id;
+}
+
+void Peer::setId(int id) {
+    _id = id;
+}
 
 // PeerPtr is used in RPC signatures for enabling receivers to send replies
 // to a particular peer rather than broadcast to all connected ones.

diff --git a/src/common/peer.h b/src/common/peer.h
index 32ee0b5..6b824cf 100644 (file)
--- a/src/common/peer.h
+++ b/src/common/peer.h
@@ -34,7 +34,7 @@ class Peer : public QObject
     Q_OBJECT
 
 public:
-    Peer(AuthHandler *authHandler, QObject *parent = 0);
+    explicit Peer(AuthHandler *authHandler, QObject *parent = 0);
 
     virtual Protocol::Type protocol() const = 0;
     virtual QString description() const = 0;
@@ -42,6 +42,18 @@ public:
     virtual SignalProxy *signalProxy() const = 0;
     virtual void setSignalProxy(SignalProxy *proxy) = 0;
 
+    const QDateTime &connectedSince() const;
+    void setConnectedSince(const QDateTime &connectedSince);
+
+    const QString &buildDate() const;
+    void setBuildDate(const QString &buildDate);
+
+    const QString &clientVersion() const;
+    void setClientVersion(const QString &clientVersion);
+
+    int id() const;
+    void setId(int id);
+
     AuthHandler *authHandler() const;
 
     virtual bool isOpen() const = 0;
@@ -53,12 +65,6 @@ public:
     virtual QString address() const = 0;
     virtual quint16 port() const = 0;
 
-    int _id = -1;
-
-    QDateTime _connectedSince;
-    QString _buildDate;
-    QString _clientVersion;
-
 public slots:
     /* Handshake messages */
     virtual void dispatch(const Protocol::RegisterClient &) = 0;
@@ -91,6 +97,13 @@ protected:
 
 private:
     QPointer<AuthHandler> _authHandler;
+
+    QDateTime _connectedSince;
+
+    QString _buildDate;
+    QString _clientVersion;
+
+    int _id = -1;
 };
 
 // We need to special-case Peer* in attached signals/slots, so typedef it for the meta type system

diff --git a/src/common/signalproxy.cpp b/src/common/signalproxy.cpp
index 976fccb..db4a675 100644 (file)
--- a/src/common/signalproxy.cpp
+++ b/src/common/signalproxy.cpp
@@ -209,7 +209,7 @@ SignalProxy::~SignalProxy()
 
 void SignalProxy::setProxyMode(ProxyMode mode)
 {
-    if (_peers.count()) {
+    if (!_peerMap.empty()) {
         qWarning() << Q_FUNC_INFO << "Cannot change proxy mode while connected";
         return;
     }
@@ -268,7 +268,7 @@ bool SignalProxy::addPeer(Peer *peer)
     if (!peer)
         return false;
 
-    if (_peers.contains(peer))
+    if (_peerMap.values().contains(peer))
         return true;
 
     if (!peer->isOpen()) {
@@ -277,7 +277,7 @@ bool SignalProxy::addPeer(Peer *peer)
     }
 
     if (proxyMode() == Client) {
-        if (!_peers.isEmpty()) {
+        if (!_peerMap.isEmpty()) {
             qWarning("SignalProxy: only one peer allowed in client mode!");
             return false;
         }
@@ -290,17 +290,16 @@ bool SignalProxy::addPeer(Peer *peer)
     if (!peer->parent())
         peer->setParent(this);
 
-    if (peer->_id < 0) {
-        peer->_id = nextPeerId();
-        peer->_connectedSince = QDateTime::currentDateTimeUtc();
+    if (peer->id() < 0) {
+        peer->setId(nextPeerId());
+        peer->setConnectedSince(QDateTime::currentDateTimeUtc());
     }
 
-    _peers.insert(peer);
-    _peerMap[peer->_id] = peer;
+    _peerMap[peer->id()] = peer;
 
     peer->setSignalProxy(this);
 
-    if (_peers.count() == 1)
+    if (peerCount() == 1)
         emit connected();
 
     updateSecureState();
@@ -310,10 +309,10 @@ bool SignalProxy::addPeer(Peer *peer)
 
 void SignalProxy::removeAllPeers()
 {
-    Q_ASSERT(proxyMode() == Server || _peers.count() <= 1);
+    Q_ASSERT(proxyMode() == Server || peerCount() <= 1);
     // wee need to copy that list since we modify it in the loop
-    QSet<Peer *> peers = _peers;
-    foreach(Peer *peer, peers) {
+    QList<Peer *> peers = _peerMap.values();
+    for (auto peer : peers) {
         removePeer(peer);
     }
 }
@@ -326,12 +325,12 @@ void SignalProxy::removePeer(Peer *peer)
         return;
     }
 
-    if (_peers.isEmpty()) {
+    if (_peerMap.isEmpty()) {
         qWarning() << "SignalProxy::removePeer(): No peers in use!";
         return;
     }
 
-    if (!_peers.contains(peer)) {
+    if (!_peerMap.values().contains(peer)) {
         qWarning() << "SignalProxy: unknown Peer" << peer;
         return;
     }
@@ -339,8 +338,7 @@ void SignalProxy::removePeer(Peer *peer)
     disconnect(peer, 0, this, 0);
     peer->setSignalProxy(0);
 
-    _peerMap.remove(peer->_id);
-    _peers.remove(peer);
+    _peerMap.remove(peer->id());
     emit peerRemoved(peer);
 
     if (peer->parent() == this)
@@ -348,7 +346,7 @@ void SignalProxy::removePeer(Peer *peer)
 
     updateSecureState();
 
-    if (_peers.isEmpty())
+    if (_peerMap.isEmpty())
         emit disconnected();
 }
 
@@ -515,7 +513,7 @@ void SignalProxy::stopSynchronize(SyncableObject *obj)
 template<class T>
 void SignalProxy::dispatch(const T &protoMessage)
 {
-    foreach (Peer *peer, _peers) {
+    for (auto peer : _peerMap.values()) {
         if (peer->isOpen())
             peer->dispatch(protoMessage);
         else
@@ -810,8 +808,8 @@ void SignalProxy::updateSecureState()
 {
     bool wasSecure = _secure;
 
-    _secure = !_peers.isEmpty();
-    foreach (const Peer *peer,  _peers) {
+    _secure = !_peerMap.isEmpty();
+    for (auto peer :  _peerMap.values()) {
         _secure &= peer->isSecure();
     }
 
@@ -821,15 +819,15 @@ void SignalProxy::updateSecureState()
 
 QVariantList SignalProxy::peerData() {
     QVariantList result;
-    for (auto peer : _peers) {
+    for (auto peer : _peerMap.values()) {
         QVariantMap data;
-        data["id"] = peer->_id;
-        data["clientVersion"] = peer->_clientVersion;
+        data["id"] = peer->id();
+        data["clientVersion"] = peer->clientVersion();
         // We explicitly rename this, as, due to the Debian reproducability changes, buildDate isn’t actually the build
         // date anymore, but on newer clients the date of the last git commit
-        data["clientVersionDate"] = peer->_buildDate;
+        data["clientVersionDate"] = peer->buildDate();
         data["remoteAddress"] = peer->address();
-        data["connectedSince"] = peer->_connectedSince;
+        data["connectedSince"] = peer->connectedSince();
         data["secure"] = peer->isSecure();
         result << data;
     }

diff --git a/src/common/signalproxy.h b/src/common/signalproxy.h
index 500693f..63d3438 100644 (file)
--- a/src/common/signalproxy.h
+++ b/src/common/signalproxy.h
@@ -83,7 +83,7 @@ public:
     /**@{*/
     /**
      * This method allows to send a signal only to a limited set of peers
-     * @param peerIds A list of peers that should receive it
+     * @param peers A list of peers that should receive it
      * @param closure Code you want to execute within of that restricted environment
      */
     void restrictTargetPeers(QSet<Peer*> peers, std::function<void()> closure);
@@ -103,7 +103,7 @@ public:
 #endif
     /**}@*/
 
-    inline int peerCount() const { return _peers.size(); }
+    inline int peerCount() const { return _peerMap.size(); }
     QVariantList peerData();
 
     Peer *peerById(int peerId);
@@ -177,7 +177,6 @@ private:
 
     static void disconnectDevice(QIODevice *dev, const QString &reason = QString());
 
-    QSet<Peer *> _peers;
     QHash<int, Peer*> _peerMap;
 
     // containg a list of argtypes for fast access
@@ -206,7 +205,7 @@ private:
     QSet<Peer *> _restrictedTargets;
     bool _restrictMessageTarget = false;
 
-    Peer *_sourcePeer;
+    Peer *_sourcePeer = nullptr;
 
     friend class SignalRelay;
     friend class SyncableObject;

diff --git a/src/core/coreauthhandler.cpp b/src/core/coreauthhandler.cpp
index aa1d957..a2367c3 100644 (file)
--- a/src/core/coreauthhandler.cpp
+++ b/src/core/coreauthhandler.cpp
@@ -183,8 +183,8 @@ void CoreAuthHandler::handle(const RegisterClient &msg)
     // XXX: FIXME: use client features here: we cannot pass authenticators if the client is too old!
     _peer->dispatch(ClientRegistered(Quassel::features(), configured, backends, useSsl, authenticators));
 
-    _peer->_buildDate = msg.buildDate;
-    _peer->_clientVersion = msg.clientVersion;
+    _peer->setBuildDate(msg.buildDate);
+    _peer->setClientVersion(msg.clientVersion);
 
     if (_legacy && useSsl)
         startSsl();

diff --git a/src/core/coresession.cpp b/src/core/coresession.cpp
index 6ae077f..c97c4bf 100644 (file)
--- a/src/core/coresession.cpp
+++ b/src/core/coresession.cpp
@@ -711,8 +711,9 @@ void CoreSession::globalAway(const QString &msg, const bool skipFormatting)
     }
 }
 
-void CoreSession::changePassword(PeerPtr peer, const QString &userName, const QString &oldPassword, const QString &newPassword)
-{
+void CoreSession::changePassword(PeerPtr peer, const QString &userName, const QString &oldPassword, const QString &newPassword) {
+    Q_UNUSED(peer);
+
     bool success = false;
     UserId uid = Core::validateUser(userName, oldPassword);
     if (uid.isValid() && uid == user())

diff --git a/src/qtui/CMakeLists.txt b/src/qtui/CMakeLists.txt
index 0637d22..518ceac 100644 (file)
--- a/src/qtui/CMakeLists.txt
+++ b/src/qtui/CMakeLists.txt
@@ -67,8 +67,8 @@ set(FORMS
     coreconfigwizardsyncpage.ui
     coreconnectauthdlg.ui
     coreconnectionstatuswidget.ui
-        coresessionwidget.ui
     coreinfodlg.ui
+    coresessionwidget.ui
     debugbufferviewoverlay.ui
     debugconsole.ui
     debuglogwidget.ui

diff --git a/src/qtui/coresessionwidget.cpp b/src/qtui/coresessionwidget.cpp
index a3f297c..fa6d176 100644 (file)
--- a/src/qtui/coresessionwidget.cpp
+++ b/src/qtui/coresessionwidget.cpp
@@ -18,9 +18,10 @@
  *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.         *
  ***************************************************************************/
 
+#include <QDateTime>
+
+#include "client.h"
 #include "coresessionwidget.h"
-#include <QtCore/QDateTime>
-#include <client.h>
 
 
 CoreSessionWidget::CoreSessionWidget(QWidget *parent)
@@ -35,6 +36,8 @@ CoreSessionWidget::CoreSessionWidget(QWidget *parent)
 void CoreSessionWidget::setData(QMap<QString, QVariant> map)
 {
     QLabel *iconSecure = ui.iconSecure;
+    // TODO: Implement "secure" icon
+    Q_UNUSED(iconSecure);
 
     ui.labelRemoteAddress->setText(map["remoteAddress"].toString());
     ui.labelLocation->setText(map["location"].toString());

diff --git a/src/qtui/coresessionwidget.h b/src/qtui/coresessionwidget.h
index a62e6b0..e19b23b 100644 (file)
--- a/src/qtui/coresessionwidget.h
+++ b/src/qtui/coresessionwidget.h
@@ -17,13 +17,12 @@
  *   Free Software Foundation, Inc.,                                       *
  *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.         *
  ***************************************************************************/
+#pragma once
 
-#ifndef CORESESSIONWIDGET_H
-#define CORESESSIONWIDGET_H
-
-#include <QWidget>
-#include <ui_coresessionwidget.h>
 #include <QMap>
+#include <QWidget>
+
+#include "ui_coresessionwidget.h"
 
 class CoreSessionWidget: public QWidget
 {
@@ -44,5 +43,3 @@ private:
     Ui::CoreSessionWidget ui;
     int _peerId;
 };
-
-#endif //CORESESSIONWIDGET_H