From: Janne Koschinski Date: Tue, 19 Dec 2017 19:18:20 +0000 (+0100) Subject: Implement changes requested in review X-Git-Tag: travis-deploy-test~216 X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=28cee4568aeb1ce3014d11234e40f19e7aeae5bd 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 54e0c59e..25ff8145 100644 --- 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 32ee0b5f..6b824cfd 100644 --- 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; + + 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 976fccb2..db4a6758 100644 --- 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 peers = _peers; - foreach(Peer *peer, peers) { + QList 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 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 500693f4..63d3438a 100644 --- 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 peers, std::function 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 _peers; QHash _peerMap; // containg a list of argtypes for fast access @@ -206,7 +205,7 @@ private: QSet _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 aa1d9571..a2367c37 100644 --- 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 6ae077fc..c97c4bf4 100644 --- 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 0637d226..518ceacb 100644 --- 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 a3f297cf..fa6d1760 100644 --- 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 + +#include "client.h" #include "coresessionwidget.h" -#include -#include CoreSessionWidget::CoreSessionWidget(QWidget *parent) @@ -35,6 +36,8 @@ CoreSessionWidget::CoreSessionWidget(QWidget *parent) void CoreSessionWidget::setData(QMap 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 a62e6b04..e19b23b6 100644 --- 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 -#include #include +#include + +#include "ui_coresessionwidget.h" class CoreSessionWidget: public QWidget { @@ -44,5 +43,3 @@ private: Ui::CoreSessionWidget ui; int _peerId; }; - -#endif //CORESESSIONWIDGET_H