sigproxy: Modernize RPC calls (remote signals)
authorManuel Nickschas <sputnick@quassel-irc.org>
Wed, 17 Oct 2018 23:00:12 +0000 (01:00 +0200)
committerManuel Nickschas <sputnick@quassel-irc.org>
Sun, 18 Nov 2018 10:06:43 +0000 (11:06 +0100)
The previous implementation relied on undocumented Qt internals to
transform signals into RpcCall messages. It also relied on the
old-style, slow signal/slot connections and required RPC call
receivers to be actual slots, a feature deprecated in Qt.

Reimplement this feature to make use of PMF connections. Utilize
C++14 features such as variadic lambdas and templates to shift
most of the work to compile time, which is not only more efficient,
but also gives us compile-time checks for most things.

Arguments are now marshalled to and from QVariant using the methods
intended for the purpose, rather than reinterpreting void pointers
into raw memory (which mimicked the way Qt works internally, but
is of course fully unsupported, even though it miraculously worked
basically unchanged since the early days of Qt 4...). The marshalling
code is generated at compile time, which is more efficient than the
previous approach of looking up types and constructing arguments
at runtime.

SignalProxy::attachSignal() now expects the signal to be given as a
member function pointer. SignalProxy::attachSlot() supports member
function pointers as well as functors, similar to QObject::connect().

Remove SignalRelay and related methods that are no longer needed.

While we're at it, rename RpcCall's slotName attribute to signalName,
which better reflects reality.

The reimplementation does not affect the protocol; the serialization
format remains unchanged.

src/client/client.cpp
src/common/protocol.h
src/common/protocols/datastream/datastreampeer.cpp
src/common/protocols/legacy/legacypeer.cpp
src/common/signalproxy.cpp
src/common/signalproxy.h
src/core/coresession.cpp
src/qtui/debugconsole.cpp
src/test/util/mockedpeer.cpp
src/test/util/mockedpeer.h
tests/common/signalproxytest.cpp

index 906684c..8ddacb9 100644 (file)
@@ -89,34 +89,28 @@ Client::Client(std::unique_ptr<AbstractUi> ui, QObject* parent)
 
     SignalProxy* p = signalProxy();
 
-    p->attachSlot(SIGNAL(displayMsg(const Message&)), this, SLOT(recvMessage(const Message&)));
-    p->attachSlot(SIGNAL(displayStatusMsg(QString, QString)), this, SLOT(recvStatusMsg(QString, QString)));
-
-    p->attachSlot(SIGNAL(bufferInfoUpdated(BufferInfo)), _networkModel, SLOT(bufferUpdated(BufferInfo)));
-    p->attachSignal(inputHandler(), SIGNAL(sendInput(BufferInfo, QString)));
-    p->attachSignal(this, SIGNAL(requestNetworkStates()));
-
-    p->attachSignal(this,
-                    SIGNAL(requestCreateIdentity(const Identity&, const QVariantMap&)),
-                    SIGNAL(createIdentity(const Identity&, const QVariantMap&)));
-    p->attachSignal(this, SIGNAL(requestRemoveIdentity(IdentityId)), SIGNAL(removeIdentity(IdentityId)));
-    p->attachSlot(SIGNAL(identityCreated(const Identity&)), this, SLOT(coreIdentityCreated(const Identity&)));
-    p->attachSlot(SIGNAL(identityRemoved(IdentityId)), this, SLOT(coreIdentityRemoved(IdentityId)));
-
-    p->attachSignal(this,
-                    SIGNAL(requestCreateNetwork(const NetworkInfo&, const QStringList&)),
-                    SIGNAL(createNetwork(const NetworkInfo&, const QStringList&)));
-    p->attachSignal(this, SIGNAL(requestRemoveNetwork(NetworkId)), SIGNAL(removeNetwork(NetworkId)));
-    p->attachSlot(SIGNAL(networkCreated(NetworkId)), this, SLOT(coreNetworkCreated(NetworkId)));
-    p->attachSlot(SIGNAL(networkRemoved(NetworkId)), this, SLOT(coreNetworkRemoved(NetworkId)));
-
-    p->attachSignal(this,
-                    SIGNAL(requestPasswordChange(PeerPtr, QString, QString, QString)),
-                    SIGNAL(changePassword(PeerPtr, QString, QString, QString)));
-    p->attachSlot(SIGNAL(passwordChanged(PeerPtr, bool)), this, SLOT(corePasswordChanged(PeerPtr, bool)));
-
-    p->attachSignal(this, SIGNAL(requestKickClient(int)), SIGNAL(kickClient(int)));
-    p->attachSlot(SIGNAL(disconnectFromCore()), this, SLOT(disconnectFromCore()));
+    p->attachSlot(SIGNAL(displayMsg(Message)), this, &Client::recvMessage);
+    p->attachSlot(SIGNAL(displayStatusMsg(QString,QString)), this, &Client::recvStatusMsg);
+
+    p->attachSlot(SIGNAL(bufferInfoUpdated(BufferInfo)), _networkModel, &NetworkModel::bufferUpdated);
+    p->attachSignal(inputHandler(), &ClientUserInputHandler::sendInput);
+    p->attachSignal(this, &Client::requestNetworkStates);
+
+    p->attachSignal(this, &Client::requestCreateIdentity, SIGNAL(createIdentity(Identity,QVariantMap)));
+    p->attachSignal(this, &Client::requestRemoveIdentity, SIGNAL(removeIdentity(IdentityId)));
+    p->attachSlot(SIGNAL(identityCreated(Identity)), this, &Client::coreIdentityCreated);
+    p->attachSlot(SIGNAL(identityRemoved(IdentityId)), this, &Client::coreIdentityRemoved);
+
+    p->attachSignal(this, &Client::requestCreateNetwork, SIGNAL(createNetwork(NetworkInfo,QStringList)));
+    p->attachSignal(this, &Client::requestRemoveNetwork, SIGNAL(removeNetwork(NetworkId)));
+    p->attachSlot(SIGNAL(networkCreated(NetworkId)), this, &Client::coreNetworkCreated);
+    p->attachSlot(SIGNAL(networkRemoved(NetworkId)), this, &Client::coreNetworkRemoved);
+
+    p->attachSignal(this, &Client::requestPasswordChange, SIGNAL(changePassword(PeerPtr,QString,QString,QString)));
+    p->attachSlot(SIGNAL(passwordChanged(PeerPtr,bool)), this, &Client::corePasswordChanged);
+
+    p->attachSignal(this, &Client::requestKickClient, SIGNAL(kickClient(int)));
+    p->attachSlot(SIGNAL(disconnectFromCore()), this, &Client::disconnectFromCore);
 
     p->synchronize(backlogManager());
     p->synchronize(coreInfo());
index 8c2a6a6..d95a529 100644 (file)
@@ -206,12 +206,12 @@ struct SyncMessage : public SignalProxyMessage
 struct RpcCall : public SignalProxyMessage
 {
     RpcCall() = default;
-    RpcCall(QByteArray slotName, QVariantList params)
-        : slotName(std::move(slotName))
+    RpcCall(QByteArray signalName, QVariantList params)
+        : signalName(std::move(signalName))
         , params(std::move(params))
     {}
 
-    QByteArray slotName;
+    QByteArray signalName;
     QVariantList params;
 };
 
index 3eb643b..1027c40 100644 (file)
@@ -328,8 +328,8 @@ void DataStreamPeer::handlePackedFunc(const QVariantList& packedFunc)
             qWarning() << Q_FUNC_INFO << "Received empty RPC call!";
             return;
         }
-        QByteArray slotName = params.takeFirst().toByteArray();
-        handle(Protocol::RpcCall(slotName, params));
+        QByteArray signalName = params.takeFirst().toByteArray();
+        handle(Protocol::RpcCall(signalName, params));
         break;
     }
     case InitRequest: {
@@ -383,7 +383,7 @@ void DataStreamPeer::dispatch(const Protocol::SyncMessage& msg)
 
 void DataStreamPeer::dispatch(const Protocol::RpcCall& msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.slotName << msg.params);
+    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.signalName << msg.params);
 }
 
 void DataStreamPeer::dispatch(const Protocol::InitRequest& msg)
index 027c333..606f91d 100644 (file)
@@ -394,8 +394,8 @@ void LegacyPeer::handlePackedFunc(const QVariant& packedFunc)
             qWarning() << Q_FUNC_INFO << "Received empty RPC call!";
             return;
         }
-        QByteArray slotName = params.takeFirst().toByteArray();
-        handle(Protocol::RpcCall(slotName, params));
+        QByteArray signalName = params.takeFirst().toByteArray();
+        handle(Protocol::RpcCall(signalName, params));
         break;
     }
     case InitRequest: {
@@ -457,7 +457,7 @@ void LegacyPeer::dispatch(const Protocol::SyncMessage& msg)
 
 void LegacyPeer::dispatch(const Protocol::RpcCall& msg)
 {
-    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.slotName << msg.params);
+    dispatchPackedFunc(QVariantList() << (qint16)RpcCall << msg.signalName << msg.params);
 }
 
 void LegacyPeer::dispatch(const Protocol::InitRequest& msg)
index 7434ea5..caafb65 100644 (file)
@@ -18,6 +18,7 @@
  *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.         *
  ***************************************************************************/
 
+#include <algorithm>
 #include <utility>
 
 #include <QCoreApplication>
@@ -49,130 +50,6 @@ public:
     Peer* peer;
 };
 
-// ==================================================
-//  SignalRelay
-// ==================================================
-class SignalProxy::SignalRelay : public QObject
-{
-    /* Q_OBJECT is not necessary or even allowed, because we implement
-       qt_metacall ourselves (and don't use any other features of the meta
-       object system)
-    */
-public:
-    SignalRelay(SignalProxy* parent)
-        : QObject(parent)
-        , _proxy(parent)
-    {}
-    inline SignalProxy* proxy() const { return _proxy; }
-
-    int qt_metacall(QMetaObject::Call _c, int _id, void** _a) override;
-
-    void attachSignal(QObject* sender, int signalId, const QByteArray& funcName);
-    void detachSignal(QObject* sender, int signalId = -1);
-
-private:
-    struct Signal
-    {
-        QObject* sender{nullptr};
-        int signalId{-1};
-        QByteArray signature;
-        Signal(QObject* sender, int sigId, QByteArray signature)
-            : sender(sender)
-            , signalId(sigId)
-            , signature(std::move(signature))
-        {}
-        Signal() = default;
-    };
-
-    SignalProxy* _proxy;
-    QHash<int, Signal> _slots;
-};
-
-void SignalProxy::SignalRelay::attachSignal(QObject* sender, int signalId, const QByteArray& funcName)
-{
-    // we ride without safetybelts here... all checking for valid method etc pp has to be done by the caller
-    // all connected methodIds are offset by the standard methodCount of QObject
-    int slotId;
-    for (int i = 0;; i++) {
-        if (!_slots.contains(i)) {
-            slotId = i;
-            break;
-        }
-    }
-
-    QByteArray fn;
-    if (!funcName.isEmpty()) {
-        fn = QMetaObject::normalizedSignature(funcName);
-    }
-    else {
-        fn = SIGNAL(fakeMethodSignature());
-        fn = fn.replace("fakeMethodSignature()", sender->metaObject()->method(signalId).methodSignature());
-    }
-
-    _slots[slotId] = Signal(sender, signalId, fn);
-
-    QMetaObject::connect(sender, signalId, this, QObject::staticMetaObject.methodCount() + slotId);
-}
-
-void SignalProxy::SignalRelay::detachSignal(QObject* sender, int signalId)
-{
-    QHash<int, Signal>::iterator slotIter = _slots.begin();
-    while (slotIter != _slots.end()) {
-        if (slotIter->sender == sender && (signalId == -1 || slotIter->signalId == signalId)) {
-            slotIter = _slots.erase(slotIter);
-            if (signalId != -1)
-                break;
-        }
-        else {
-            ++slotIter;
-        }
-    }
-}
-
-int SignalProxy::SignalRelay::qt_metacall(QMetaObject::Call _c, int _id, void** _a)
-{
-    _id = QObject::qt_metacall(_c, _id, _a);
-    if (_id < 0)
-        return _id;
-
-    if (_c == QMetaObject::InvokeMetaMethod) {
-        if (_slots.contains(_id)) {
-            QObject* caller = sender();
-
-            SignalProxy::ExtendedMetaObject* eMeta = proxy()->extendedMetaObject(caller->metaObject());
-            Q_ASSERT(eMeta);
-
-            const Signal& signal = _slots[_id];
-
-            QVariantList params;
-
-            const QList<int>& argTypes = eMeta->argTypes(signal.signalId);
-            for (int i = 0; i < argTypes.size(); i++) {
-                if (argTypes[i] == 0) {
-                    qWarning() << "SignalRelay::qt_metacall(): received invalid data for argument number" << i << "of signal"
-                               << QString("%1::%2")
-                                      .arg(caller->metaObject()->className())
-                                      .arg(caller->metaObject()->method(signal.signalId).methodSignature().constData());
-                    qWarning() << "                            - make sure all your data types are known by the Qt MetaSystem";
-                    return _id;
-                }
-                params << QVariant(argTypes[i], _a[i + 1]);
-            }
-
-            if (proxy()->_restrictMessageTarget) {
-                for (auto peer : proxy()->_restrictedTargets) {
-                    if (peer != nullptr)
-                        proxy()->dispatch(peer, RpcCall(signal.signature, params));
-                }
-            }
-            else
-                proxy()->dispatch(RpcCall(signal.signature, params));
-        }
-        _id -= _slots.count();
-    }
-    return _id;
-}
-
 // ==================================================
 //  SignalProxy
 // ==================================================
@@ -211,6 +88,9 @@ SignalProxy::~SignalProxy()
 
     removeAllPeers();
 
+    // Ensure that we don't try to clean up while destroying ourselves
+    disconnect(this, &QObject::destroyed, this, &SignalProxy::detachSlotObjects);
+
     _current = nullptr;
 }
 
@@ -237,7 +117,6 @@ void SignalProxy::init()
 {
     _heartBeatInterval = 0;
     _maxHeartBeatCount = 0;
-    _signalRelay = new SignalRelay(this);
     setHeartBeatInterval(30);
     setMaxHeartBeatCount(2);
     _secure = false;
@@ -249,7 +128,7 @@ void SignalProxy::initServer() {}
 
 void SignalProxy::initClient()
 {
-    attachSlot("__objectRenamed__", this, SLOT(objectRenamed(QByteArray, QString, QString)));
+    attachSlot("__objectRenamed__", this, &SignalProxy::objectRenamed);
 }
 
 void SignalProxy::setHeartBeatInterval(int secs)
@@ -405,41 +284,24 @@ SignalProxy::ExtendedMetaObject* SignalProxy::createExtendedMetaObject(const QMe
     return _extendedMetaObjects[meta];
 }
 
-bool SignalProxy::attachSignal(QObject* sender, const char* signal, const QByteArray& sigName)
+void SignalProxy::attachSlotObject(const QByteArray& signalName, std::unique_ptr<SlotObjectBase> slotObject)
 {
-    const QMetaObject* meta = sender->metaObject();
-    QByteArray sig(meta->normalizedSignature(signal).mid(1));
-    int methodId = meta->indexOfMethod(sig.constData());
-    if (methodId == -1 || meta->method(methodId).methodType() != QMetaMethod::Signal) {
-        qWarning() << "SignalProxy::attachSignal(): No such signal" << signal;
-        return false;
-    }
-
-    createExtendedMetaObject(meta);
-    _signalRelay->attachSignal(sender, methodId, sigName);
+    // Remove all attached slots related to the context upon its destruction
+    connect(slotObject->context(), &QObject::destroyed, this, &SignalProxy::detachSlotObjects, Qt::UniqueConnection);
 
-    disconnect(sender, &QObject::destroyed, this, &SignalProxy::detachObject);
-    connect(sender, &QObject::destroyed, this, &SignalProxy::detachObject);
-    return true;
+    _attachedSlots.emplace(QMetaObject::normalizedSignature(signalName.constData()), std::move(slotObject));
 }
 
-bool SignalProxy::attachSlot(const QByteArray& sigName, QObject* recv, const char* slot)
+void SignalProxy::detachSlotObjects(const QObject *context)
 {
-    const QMetaObject* meta = recv->metaObject();
-    int methodId = meta->indexOfMethod(meta->normalizedSignature(slot).mid(1));
-    if (methodId == -1 || meta->method(methodId).methodType() == QMetaMethod::Method) {
-        qWarning() << "SignalProxy::attachSlot(): No such slot" << slot;
-        return false;
+    for (auto&& it = _attachedSlots.begin(); it != _attachedSlots.end(); ) {
+        if (it->second->context() == context) {
+            it = _attachedSlots.erase(it);
+        }
+        else {
+            ++it;
+        }
     }
-
-    createExtendedMetaObject(meta);
-
-    QByteArray funcName = QMetaObject::normalizedSignature(sigName.constData());
-    _attachedSlots.insert(funcName, qMakePair(recv, methodId));
-
-    disconnect(recv, &QObject::destroyed, this, &SignalProxy::detachObject);
-    connect(recv, &QObject::destroyed, this, &SignalProxy::detachObject);
-    return true;
 }
 
 void SignalProxy::synchronize(SyncableObject* obj)
@@ -464,32 +326,6 @@ void SignalProxy::synchronize(SyncableObject* obj)
     obj->synchronize(this);
 }
 
-void SignalProxy::detachObject(QObject* obj)
-{
-    // Don't try to connect SignalProxy from itself on shutdown
-    if (obj != this) {
-        detachSignals(obj);
-        detachSlots(obj);
-    }
-}
-
-void SignalProxy::detachSignals(QObject* sender)
-{
-    _signalRelay->detachSignal(sender);
-}
-
-void SignalProxy::detachSlots(QObject* receiver)
-{
-    SlotHash::iterator slotIter = _attachedSlots.begin();
-    while (slotIter != _attachedSlots.end()) {
-        if (slotIter.value().first == receiver) {
-            slotIter = _attachedSlots.erase(slotIter);
-        }
-        else
-            ++slotIter;
-    }
-}
-
 void SignalProxy::stopSynchronize(SyncableObject* obj)
 {
     // we can't use a className here, since it might be effed up, if we receive the call as a result of a decon
@@ -505,6 +341,19 @@ void SignalProxy::stopSynchronize(SyncableObject* obj)
     obj->stopSynchronize(this);
 }
 
+void SignalProxy::dispatchSignal(QByteArray sigName, QVariantList params)
+{
+    RpcCall rpcCall{std::move(sigName), std::move(params)};
+    if (_restrictMessageTarget) {
+        for (auto&& peer : _restrictedTargets) {
+            dispatch(peer, rpcCall);
+        }
+    }
+    else {
+        dispatch(rpcCall);
+    }
+}
+
 template<class T>
 void SignalProxy::dispatch(const T& protoMessage)
 {
@@ -576,6 +425,18 @@ void SignalProxy::handle(Peer* peer, const SyncMessage& syncMessage)
     invokeSlot(receiver, eMeta->updatedRemotelyId());
 }
 
+void SignalProxy::handle(Peer* peer, const RpcCall& rpcCall)
+{
+    Q_UNUSED(peer)
+
+    auto range = _attachedSlots.equal_range(rpcCall.signalName);
+    std::for_each(range.first, range.second, [&rpcCall](const auto& p) {
+        if (!p.second->invoke(rpcCall.params)) {
+            qWarning() << "Could not invoke slot for remote signal" << rpcCall.signalName;
+        }
+    });
+}
+
 void SignalProxy::handle(Peer* peer, const InitRequest& initRequest)
 {
     if (!_syncSlave.contains(initRequest.className)) {
@@ -613,22 +474,6 @@ void SignalProxy::handle(Peer* peer, const InitData& initData)
     setInitData(obj, initData.initData);
 }
 
-void SignalProxy::handle(Peer* peer, const RpcCall& rpcCall)
-{
-    QObject* receiver;
-    int methodId;
-    SlotHash::const_iterator slot = _attachedSlots.constFind(rpcCall.slotName);
-    while (slot != _attachedSlots.constEnd() && slot.key() == rpcCall.slotName) {
-        receiver = (*slot).first;
-        methodId = (*slot).second;
-        if (!invokeSlot(receiver, methodId, rpcCall.params, peer)) {
-            ExtendedMetaObject* eMeta = extendedMetaObject(receiver);
-            qWarning("SignalProxy::handleSignal(): invokeMethod for \"%s\" failed ", eMeta->methodName(methodId).constData());
-        }
-        ++slot;
-    }
-}
-
 bool SignalProxy::invokeSlot(QObject* receiver, int methodId, const QVariantList& params, QVariant& returnValue, Peer* peer)
 {
     ExtendedMetaObject* eMeta = extendedMetaObject(receiver);
@@ -789,7 +634,7 @@ void SignalProxy::dumpProxyStats()
 
     qDebug() << this;
     qDebug() << "              Proxy Mode:" << mode;
-    qDebug() << "          attached Slots:" << _attachedSlots.count();
+    qDebug() << "          attached Slots:" << _attachedSlots.size();
     qDebug() << " number of synced Slaves:" << slaveCount;
     qDebug() << "number of Classes cached:" << _extendedMetaObjects.count();
 }
@@ -868,9 +713,19 @@ void SignalProxy::setTargetPeer(Peer* targetPeer)
     _targetPeer = targetPeer;
 }
 
-// ==================================================
-//  ExtendedMetaObject
-// ==================================================
+// ---- SlotObjectBase ---------------------------------------------------------------------------------------------------------------------
+
+SignalProxy::SlotObjectBase::SlotObjectBase(const QObject* context)
+    : _context{context}
+{}
+
+const QObject* SignalProxy::SlotObjectBase::context() const
+{
+    return _context;
+}
+
+//  ---- ExtendedMetaObject ----------------------------------------------------------------------------------------------------------------
+
 SignalProxy::ExtendedMetaObject::ExtendedMetaObject(const QMetaObject* meta, bool checkConflicts)
     : _meta(meta)
     , _updatedRemotelyId(_meta->indexOfSignal("updatedRemotely()"))
index 49033cd..885ebe6 100644 (file)
 
 #include <functional>
 #include <initializer_list>
+#include <memory>
+#include <type_traits>
+#include <unordered_map>
+#include <utility>
 
+#include <QDebug>
 #include <QEvent>
+#include <QMetaMethod>
 #include <QSet>
+#include <QThread>
 
+#include "funchelpers.h"
 #include "protocol.h"
+#include "types.h"
 
 struct QMetaObject;
 class QIODevice;
@@ -40,7 +49,9 @@ class COMMON_EXPORT SignalProxy : public QObject
 {
     Q_OBJECT
 
-    class SignalRelay;
+    template<typename Slot, typename Callable = typename FunctionTraits<Slot>::FunctionType>
+    class SlotObject;
+    class SlotObjectBase;
 
 public:
     enum ProxyMode
@@ -68,8 +79,65 @@ public:
 
     bool addPeer(Peer* peer);
 
-    bool attachSignal(QObject* sender, const char* signal, const QByteArray& sigName = QByteArray());
-    bool attachSlot(const QByteArray& sigName, QObject* recv, const char* slot);
+    /**
+     * Attaches a signal for remote emission.
+     *
+     * After calling this method, whenever the sender emits the given signal, an RpcCall message is sent to connected peers.
+     * On the other end, a slot can be attached to handle this message by calling attachSlot().
+
+     * By default, the signal name being sent is as if the SIGNAL() macro had been used, i.e. the normalized signature prefixed with a '2'.
+     * This can be overridden by explicitly providing the signalName argument.
+     *
+     * @sa attachSlot
+     *
+     * @param sender The sender of the signal
+     * @param signal The signal itself (given as a member function pointer)
+     * @param signalName Optional string to be used instead of the actual signal name. Will be normalized.
+     * @returns true if attaching the signal was successful
+     */
+    template<typename Signal>
+    bool attachSignal(const typename FunctionTraits<Signal>::ClassType* sender, Signal signal, const QByteArray& signalName = {});
+
+    /**
+     * Attaches a slot to a remote signal.
+     *
+     * After calling this method, upon receipt of an RpcCall message with a signalName matching the signalName parameter, the given slot
+     * is called with the parameters contained in the message. This is intended to be used in conjunction with attachSignal() on the other
+     * end of the connection.
+     *
+     * Normally, the signalName should be given using the SIGNAL() macro; it will be normalized automatically.
+     *
+     * @sa attachSignal
+     *
+     * @param signalName Name of the signal as stored in the RpcCall message
+     * @param receiver Receiver of the signal
+     * @param slot     Slot to be called (given as a member function pointer)
+     * @returns true if attaching the slot was successful
+     */
+    template<typename Slot, typename = std::enable_if_t<std::is_member_function_pointer<Slot>::value>>
+    bool attachSlot(const QByteArray& signalName, typename FunctionTraits<Slot>::ClassType* receiver, Slot slot);
+
+    /**
+     * @overload
+     *
+     * Attaches a functor to a remote signal.
+     *
+     * After calling this method, upon receipt of an RpcCall message with a signalName matching the signalName parameter, the given functor
+     * is invoked with the parameters contained in the message. This is intended to be used in conjunction with attachSignal() on the other
+     * end of the connection. This overload can be used, for example, with a lambda that accepts arguments matching the RpcCall parameter
+     * list.
+     *
+     * The context parameter controls the lifetime of the connection; if the context is deleted, the functor is deleted as well.
+     *
+     * @sa attachSignal
+     *
+     * @param signalName Name of the signal as stored in the RpcCall message
+     * @param context QObject context controlling the lifetime of the callable
+     * @param slot    The functor to be invoked
+     * @returns true if attaching the functor was successful
+     */
+    template<typename Slot, typename = std::enable_if_t<!std::is_member_function_pointer<Slot>::value>>
+    bool attachSlot(const QByteArray& signalName, const QObject* context, Slot slot);
 
     void synchronize(SyncableObject* obj);
     void stopSynchronize(SyncableObject* obj);
@@ -129,11 +197,6 @@ public:
     Peer* targetPeer();
     void setTargetPeer(Peer* targetPeer);
 
-public slots:
-    void detachObject(QObject* obj);
-    void detachSignals(QObject* sender);
-    void detachSlots(QObject* receiver);
-
 protected:
     void customEvent(QEvent* event) override;
     void sync_call__(const SyncableObject* obj, ProxyMode modeType, const char* funcname, va_list ap);
@@ -169,6 +232,29 @@ private:
 
     int nextPeerId() { return _lastPeerId++; }
 
+    /**
+     * Attaches a SlotObject for the given signal name.
+     *
+     * @param signalName Signal name to be associated with the SlotObject
+     * @param slotObject The SlotObject instance to be invoked for incoming and matching RpcCall messages
+     */
+    void attachSlotObject(const QByteArray& signalName, std::unique_ptr<SlotObjectBase> slotObject);
+
+    /**
+     * Deletes all SlotObjects associated with the given context.
+     *
+     * @param context The context
+     */
+    void detachSlotObjects(const QObject* context);
+
+    /**
+     * Dispatches an RpcMessage for the given signal and parameters.
+     *
+     * @param signalName The signal
+     * @param params     The parameters
+     */
+    void dispatchSignal(QByteArray signalName, QVariantList params);
+
     template<class T>
     void dispatch(const T& protoMessage);
     template<class T>
@@ -194,18 +280,13 @@ private:
 
     static void disconnectDevice(QIODevice* dev, const QString& reason = QString());
 
+private:
     QHash<int, Peer*> _peerMap;
 
     // containg a list of argtypes for fast access
     QHash<const QMetaObject*, ExtendedMetaObject*> _extendedMetaObjects;
 
-    // SignalRelay for all manually attached signals
-    SignalRelay* _signalRelay;
-
-    // RPC function -> (object, slot ID)
-    using MethodId = QPair<QObject*, int>;
-    using SlotHash = QMultiHash<QByteArray, MethodId>;
-    SlotHash _attachedSlots;
+    std::unordered_multimap<QByteArray, std::unique_ptr<SlotObjectBase>, Hash<QByteArray>> _attachedSlots;  ///< Attached slot objects
 
     // slaves for sync
     using ObjectId = QHash<QString, SyncableObject*>;
@@ -225,11 +306,129 @@ private:
     Peer* _sourcePeer = nullptr;
     Peer* _targetPeer = nullptr;
 
-    friend class SignalRelay;
     friend class SyncableObject;
     friend class Peer;
 };
 
+// ---- Template function implementations ---------------------------------------
+
+template<typename Signal>
+bool SignalProxy::attachSignal(const typename FunctionTraits<Signal>::ClassType* sender, Signal signal, const QByteArray& signalName)
+{
+    static_assert(std::is_member_function_pointer<Signal>::value, "Signal must be given as member function pointer");
+
+    // Determine the signalName to be stored in the RpcCall
+    QByteArray name;
+    if (signalName.isEmpty()) {
+        auto method = QMetaMethod::fromSignal(signal);
+        if (!method.isValid()) {
+            qWarning().nospace() << Q_FUNC_INFO << ": Function pointer is not a signal";
+            return false;
+        }
+        name = "2" + method.methodSignature();  // SIGNAL() prefixes the signature with "2"
+    }
+    else {
+        name = QMetaObject::normalizedSignature(signalName.constData());
+    }
+
+    // Upon signal emission, marshall the signal's arguments into a QVariantList and dispatch an RpcCall message
+    connect(sender, signal, this, [this, signalName = std::move(name)](auto&&... args) {
+        this->dispatchSignal(std::move(signalName), {QVariant::fromValue<decltype(args)>(args)...});
+    });
+
+    return true;
+}
+
+template<typename Slot, typename>
+bool SignalProxy::attachSlot(const QByteArray& signalName, typename FunctionTraits<Slot>::ClassType* receiver, Slot slot)
+{
+    // Create a wrapper function that invokes the member function pointer for the receiver instance
+    attachSlotObject(signalName, std::make_unique<SlotObject<Slot>>(receiver, [receiver, slot = std::move(slot)](auto&&... args) {
+        (receiver->*slot)(std::forward<decltype(args)>(args)...);
+    }));
+    return true;
+}
+
+template<typename Slot, typename>
+bool SignalProxy::attachSlot(const QByteArray& signalName, const QObject* context, Slot slot)
+{
+    static_assert(!std::is_same<Slot, const char*>::value, "Old-style slots not supported");
+
+    attachSlotObject(signalName, std::make_unique<SlotObject<Slot>>(context, std::move(slot)));
+    return true;
+}
+
+/**
+ * Base object for storing a slot (or functor) to be invoked with a list of arguments.
+ *
+ * @note Having this untemplated base class for SlotObject allows for handling slots in the implementation rather than in the header.
+ */
+class COMMON_EXPORT SignalProxy::SlotObjectBase
+{
+public:
+    virtual ~SlotObjectBase() = default;
+
+    /**
+     * @returns The context associated with the slot
+     */
+    const QObject* context() const;
+
+    /**
+     * Invokes the slot with the given list of parameters.
+     *
+     * If the parameters cannot all be converted to the slot's argument types, or there is a mismatch in argument count,
+     * the invocation will fail.
+     *
+     * @param params List of arguments marshalled as QVariants
+     * @returns true if the invocation was successful
+     */
+    virtual bool invoke(const QVariantList& params) const = 0;
+
+protected:
+    SlotObjectBase(const QObject* context);
+
+private:
+    const QObject* _context;
+};
+
+/**
+ * Specialization of SlotObjectBase for a particular type of slot.
+ *
+ * Callable may be a function wrapper around a member function pointer of type Slot,
+ * or a functor that can be invoked directly.
+ *
+ * @tparam Slot     Type of the slot, used for determining the callable's signature
+ * @tparam Callable Type of the actual callable to be invoked
+ */
+template<typename Slot, typename Callable>
+class SignalProxy::SlotObject : public SlotObjectBase
+{
+public:
+    /**
+     * Constructs a SlotObject for the given callable, whose lifetime is controlled by context.
+     *
+     * @param context  Context object; if destroyed, the slot object will be destroyed as well by SignalProxy.
+     * @param callable Callable to be invoked
+     */
+    SlotObject(const QObject* context, Callable callable)
+        : SlotObjectBase(context)
+        , _callable(std::move(callable))
+    {}
+
+    // See base class
+    bool invoke(const QVariantList& params) const override
+    {
+        if (QThread::currentThread() != context()->thread()) {
+            qWarning() << "Cannot call slot in different thread!";
+            return false;
+        }
+        return invokeWithArgsList(_callable, params);
+    }
+
+private:
+    Callable _callable;
+};
+
 // ==================================================
 //  ExtendedMetaObject
 // ==================================================
index 3bab9d9..11a9530 100644 (file)
@@ -90,29 +90,25 @@ CoreSession::CoreSession(UserId uid, bool restoreState, bool strictIdentEnabled,
     connect(p, &SignalProxy::connected, this, &CoreSession::clientsConnected);
     connect(p, &SignalProxy::disconnected, this, &CoreSession::clientsDisconnected);
 
-    p->attachSlot(SIGNAL(sendInput(BufferInfo, QString)), this, SLOT(msgFromClient(BufferInfo, QString)));
-    p->attachSignal(this, SIGNAL(displayMsg(Message)));
-    p->attachSignal(this, SIGNAL(displayStatusMsg(QString, QString)));
-
-    p->attachSignal(this, SIGNAL(identityCreated(const Identity&)));
-    p->attachSignal(this, SIGNAL(identityRemoved(IdentityId)));
-    p->attachSlot(SIGNAL(createIdentity(const Identity&, const QVariantMap&)),
-                  this,
-                  SLOT(createIdentity(const Identity&, const QVariantMap&)));
-    p->attachSlot(SIGNAL(removeIdentity(IdentityId)), this, SLOT(removeIdentity(IdentityId)));
-
-    p->attachSignal(this, SIGNAL(networkCreated(NetworkId)));
-    p->attachSignal(this, SIGNAL(networkRemoved(NetworkId)));
-    p->attachSlot(SIGNAL(createNetwork(const NetworkInfo&, const QStringList&)),
-                  this,
-                  SLOT(createNetwork(const NetworkInfo&, const QStringList&)));
-    p->attachSlot(SIGNAL(removeNetwork(NetworkId)), this, SLOT(removeNetwork(NetworkId)));
-
-    p->attachSlot(SIGNAL(changePassword(PeerPtr, QString, QString, QString)), this, SLOT(changePassword(PeerPtr, QString, QString, QString)));
-    p->attachSignal(this, SIGNAL(passwordChanged(PeerPtr, bool)));
-
-    p->attachSlot(SIGNAL(kickClient(int)), this, SLOT(kickClient(int)));
-    p->attachSignal(this, SIGNAL(disconnectFromCore()));
+    p->attachSlot(SIGNAL(sendInput(BufferInfo,QString)), this, &CoreSession::msgFromClient);
+    p->attachSignal(this, &CoreSession::displayMsg);
+    p->attachSignal(this, &CoreSession::displayStatusMsg);
+
+    p->attachSignal(this, &CoreSession::identityCreated);
+    p->attachSignal(this, &CoreSession::identityRemoved);
+    p->attachSlot(SIGNAL(createIdentity(Identity,QVariantMap)), this, selectOverload<const Identity&, const QVariantMap&>(&CoreSession::createIdentity));
+    p->attachSlot(SIGNAL(removeIdentity(IdentityId)), this, &CoreSession::removeIdentity);
+
+    p->attachSignal(this, &CoreSession::networkCreated);
+    p->attachSignal(this, &CoreSession::networkRemoved);
+    p->attachSlot(SIGNAL(createNetwork(NetworkInfo,QStringList)), this,&CoreSession::createNetwork);
+    p->attachSlot(SIGNAL(removeNetwork(NetworkId)), this, &CoreSession::removeNetwork);
+
+    p->attachSlot(SIGNAL(changePassword(PeerPtr,QString,QString,QString)), this, &CoreSession::changePassword);
+    p->attachSignal(this, &CoreSession::passwordChanged);
+
+    p->attachSlot(SIGNAL(kickClient(int)), this, &CoreSession::kickClient);
+    p->attachSignal(this, &CoreSession::disconnectFromCore);
 
     QVariantMap data;
     data["quasselVersion"] = Quassel::buildInfo().fancyVersionString;
@@ -518,8 +514,8 @@ Protocol::SessionState CoreSession::sessionState() const
 
 void CoreSession::initScriptEngine()
 {
-    signalProxy()->attachSlot(SIGNAL(scriptRequest(QString)), this, SLOT(scriptRequest(QString)));
-    signalProxy()->attachSignal(this, SIGNAL(scriptResult(QString)));
+    signalProxy()->attachSlot(SIGNAL(scriptRequest(QString)), this, &CoreSession::scriptRequest);
+    signalProxy()->attachSignal(this, &CoreSession::scriptResult);
 
     // FIXME
     // QScriptValue storage_ = scriptEngine->newQObject(storage);
index 29ae930..dee9b62 100644 (file)
@@ -28,8 +28,8 @@ DebugConsole::DebugConsole(QWidget* parent)
 {
     ui.setupUi(this);
 
-    Client::signalProxy()->attachSignal(this, SIGNAL(scriptRequest(QString)));
-    Client::signalProxy()->attachSlot(SIGNAL(scriptResult(QString)), this, SLOT(scriptResult(QString)));
+    Client::signalProxy()->attachSignal(this, &DebugConsole::scriptRequest);
+    Client::signalProxy()->attachSlot(SIGNAL(scriptResult(QString)), this, &DebugConsole::scriptResult);
 }
 
 void DebugConsole::on_evalButton_clicked()
index e182d46..32f0ec6 100644 (file)
@@ -58,7 +58,7 @@ void PrintTo(const ProtocolMessage& msg, std::ostream* os)
 
         void operator()(const Protocol::RpcCall& rpcCall) const
         {
-            *_os << "RpcCall{slotName = " << PrintToString(rpcCall.slotName)
+            *_os << "RpcCall{signalName = " << PrintToString(rpcCall.signalName)
                  << ", params = " << PrintToString(rpcCall.params)
                  << "}";
         }
@@ -157,7 +157,7 @@ struct SyncMessageExpectation
 
 struct RpcCallExpectation
 {
-    Matcher<QByteArray> slotName;
+    Matcher<QByteArray> signalName;
     Matcher<QVariantList> params;
 };
 
@@ -221,7 +221,7 @@ public:
                 ADD_FAILURE() << "Did not expect an RpcCall!";
                 return true;
             }
-            EXPECT_THAT(rpcCall.slotName, e->slotName);
+            EXPECT_THAT(rpcCall.signalName, e->signalName);
             EXPECT_THAT(rpcCall.params, e->params);
             return true;
         }
@@ -279,9 +279,9 @@ Matcher<const ProtocolMessage&> SyncMessage(Matcher<QByteArray> className, Match
     return MakeMatcher(new ProtocolMessageMatcher{SyncMessageExpectation{std::move(className), std::move(objectName), std::move(slotName), std::move(params)}});
 }
 
-Matcher<const ProtocolMessage&> RpcCall(Matcher<QByteArray> slotName, Matcher<QVariantList> params)
+Matcher<const ProtocolMessage&> RpcCall(Matcher<QByteArray> signalName, Matcher<QVariantList> params)
 {
-    return MakeMatcher(new ProtocolMessageMatcher{RpcCallExpectation{std::move(slotName), std::move(params)}});
+    return MakeMatcher(new ProtocolMessageMatcher{RpcCallExpectation{std::move(signalName), std::move(params)}});
 }
 
 Matcher<const ProtocolMessage&> InitRequest(Matcher<QByteArray> className, Matcher<QString> objectName)
index fde3d1c..6533da3 100644 (file)
@@ -90,7 +90,7 @@ TEST_UTIL_EXPORT ::testing::Matcher<const ProtocolMessage&> SyncMessage(
         ::testing::Matcher<QVariantList> params);
 
 TEST_UTIL_EXPORT ::testing::Matcher<const ProtocolMessage&> RpcCall(
-        ::testing::Matcher<QByteArray> slotName,
+        ::testing::Matcher<QByteArray> signalName,
         ::testing::Matcher<QVariantList> params);
 
 TEST_UTIL_EXPORT ::testing::Matcher<const ProtocolMessage&> InitRequest(
index a6b91fa..d8442f6 100644 (file)
@@ -100,6 +100,7 @@ TEST_F(SignalProxyTest, attachSignal)
         EXPECT_CALL(*_clientPeer, Dispatches(RpcCall(Eq(SIGNAL(sendData(int,QString))), ElementsAre(42, "Hello"))));
         EXPECT_CALL(*_clientPeer, Dispatches(RpcCall(Eq(SIGNAL(sendExtraData(int,QString))), ElementsAre(42, "Hello"))));
         EXPECT_CALL(*_serverPeer, Dispatches(RpcCall(Eq("2sendData(int,QString)"), ElementsAre(23, "World"))));
+        EXPECT_CALL(*_serverPeer, Dispatches(RpcCall(Eq("2sendToFunctor(int,QString)"), ElementsAre(17, "Hi Universe"))));
     }
 
     ProxyObject::Spy clientSpy, serverSpy;
@@ -107,14 +108,21 @@ TEST_F(SignalProxyTest, attachSignal)
     ProxyObject serverObject{&serverSpy, _serverPeer};
 
     // Deliberately not normalize some of the macro invocations
-    _clientProxy.attachSignal(&clientObject, SIGNAL(sendData(int, const QString&)));
-    _serverProxy.attachSlot(SIGNAL(sendData(int,QString)), &serverObject, SLOT(receiveData(int, const QString&)));
+    _clientProxy.attachSignal(&clientObject, &ProxyObject::sendData);
+    _serverProxy.attachSlot(SIGNAL(sendData(int,QString)), &serverObject, &ProxyObject::receiveData);
 
-    _clientProxy.attachSignal(&clientObject, SIGNAL(sendMoreData(int,QString)), SIGNAL(sendExtraData(int,QString)));
-    _serverProxy.attachSlot(SIGNAL(sendExtraData(int, const QString&)), &serverObject, SLOT(receiveExtraData(int,QString)));
+    _clientProxy.attachSignal(&clientObject, &ProxyObject::sendMoreData, SIGNAL(sendExtraData(int, const QString&)));
+    _serverProxy.attachSlot(SIGNAL(sendExtraData(int, const QString&)), &serverObject, &ProxyObject::receiveExtraData);
 
-    _serverProxy.attachSignal(&serverObject, SIGNAL(sendData(int,QString)));
-    _clientProxy.attachSlot(SIGNAL(sendData(int, const QString&)), &clientObject, SLOT(receiveData(int,QString)));
+    _serverProxy.attachSignal(&serverObject, &ProxyObject::sendData);
+    //_clientProxy.attachSlot(SIGNAL(sendData(int, const QString&)), &clientObject, SLOT(receiveData(int,QString)));
+    _clientProxy.attachSlot(SIGNAL(sendData(int, const QString&)), &clientObject, &ProxyObject::receiveData);
+
+    _serverProxy.attachSignal(&serverObject, &ProxyObject::sendToFunctor);
+    _clientProxy.attachSlot(SIGNAL(sendToFunctor(int, const QString&)), this, [this, &clientSpy](int i, const QString& s) {
+        EXPECT_EQ(_clientPeer, SignalProxy::current()->sourcePeer());
+        clientSpy.notify(std::make_pair(i, s));
+    });
 
     emit clientObject.sendData(42, "Hello");
     ASSERT_TRUE(serverSpy.wait());
@@ -127,6 +135,10 @@ TEST_F(SignalProxyTest, attachSignal)
     emit serverObject.sendData(23, "World");
     ASSERT_TRUE(clientSpy.wait());
     EXPECT_EQ(ProxyObject::Data(23, "World"), clientSpy.value());
+
+    emit serverObject.sendToFunctor(17, "Hi Universe");
+    ASSERT_TRUE(clientSpy.wait());
+    EXPECT_EQ(ProxyObject::Data(17, "Hi Universe"), clientSpy.value());
 }
 
 // -----------------------------------------------------------------------------------------------------------------------------------------