From: Manuel Nickschas Date: Wed, 17 Oct 2018 23:00:12 +0000 (+0200) Subject: sigproxy: Modernize RPC calls (remote signals) X-Git-Tag: test-travis-01~102 X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=db00831bca59a012242d1ad5fac52a20c6cd2956;ds=sidebyside sigproxy: Modernize RPC calls (remote signals) 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. --- diff --git a/src/client/client.cpp b/src/client/client.cpp index 906684c7..8ddacb91 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -89,34 +89,28 @@ Client::Client(std::unique_ptr 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()); diff --git a/src/common/protocol.h b/src/common/protocol.h index 8c2a6a6d..d95a529e 100644 --- a/src/common/protocol.h +++ b/src/common/protocol.h @@ -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; }; diff --git a/src/common/protocols/datastream/datastreampeer.cpp b/src/common/protocols/datastream/datastreampeer.cpp index 3eb643b2..1027c405 100644 --- a/src/common/protocols/datastream/datastreampeer.cpp +++ b/src/common/protocols/datastream/datastreampeer.cpp @@ -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) diff --git a/src/common/protocols/legacy/legacypeer.cpp b/src/common/protocols/legacy/legacypeer.cpp index 027c3333..606f91d4 100644 --- a/src/common/protocols/legacy/legacypeer.cpp +++ b/src/common/protocols/legacy/legacypeer.cpp @@ -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) diff --git a/src/common/signalproxy.cpp b/src/common/signalproxy.cpp index 7434ea52..caafb65b 100644 --- a/src/common/signalproxy.cpp +++ b/src/common/signalproxy.cpp @@ -18,6 +18,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * ***************************************************************************/ +#include #include #include @@ -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 _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::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& 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 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 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()")) diff --git a/src/common/signalproxy.h b/src/common/signalproxy.h index 49033cdf..885ebe60 100644 --- a/src/common/signalproxy.h +++ b/src/common/signalproxy.h @@ -24,11 +24,20 @@ #include #include +#include +#include +#include +#include +#include #include +#include #include +#include +#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::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 + bool attachSignal(const typename FunctionTraits::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::value>> + bool attachSlot(const QByteArray& signalName, typename FunctionTraits::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::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 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 void dispatch(const T& protoMessage); template @@ -194,18 +280,13 @@ private: static void disconnectDevice(QIODevice* dev, const QString& reason = QString()); +private: QHash _peerMap; // containg a list of argtypes for fast access QHash _extendedMetaObjects; - // SignalRelay for all manually attached signals - SignalRelay* _signalRelay; - - // RPC function -> (object, slot ID) - using MethodId = QPair; - using SlotHash = QMultiHash; - SlotHash _attachedSlots; + std::unordered_multimap, Hash> _attachedSlots; ///< Attached slot objects // slaves for sync using ObjectId = QHash; @@ -225,11 +306,129 @@ private: Peer* _sourcePeer = nullptr; Peer* _targetPeer = nullptr; - friend class SignalRelay; friend class SyncableObject; friend class Peer; }; +// ---- Template function implementations --------------------------------------- + +template +bool SignalProxy::attachSignal(const typename FunctionTraits::ClassType* sender, Signal signal, const QByteArray& signalName) +{ + static_assert(std::is_member_function_pointer::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(args)...}); + }); + + return true; +} + +template +bool SignalProxy::attachSlot(const QByteArray& signalName, typename FunctionTraits::ClassType* receiver, Slot slot) +{ + // Create a wrapper function that invokes the member function pointer for the receiver instance + attachSlotObject(signalName, std::make_unique>(receiver, [receiver, slot = std::move(slot)](auto&&... args) { + (receiver->*slot)(std::forward(args)...); + })); + return true; +} + +template +bool SignalProxy::attachSlot(const QByteArray& signalName, const QObject* context, Slot slot) +{ + static_assert(!std::is_same::value, "Old-style slots not supported"); + + attachSlotObject(signalName, std::make_unique>(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 +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 // ================================================== diff --git a/src/core/coresession.cpp b/src/core/coresession.cpp index 3bab9d9d..11a95301 100644 --- a/src/core/coresession.cpp +++ b/src/core/coresession.cpp @@ -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(&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); diff --git a/src/qtui/debugconsole.cpp b/src/qtui/debugconsole.cpp index 29ae930f..dee9b623 100644 --- a/src/qtui/debugconsole.cpp +++ b/src/qtui/debugconsole.cpp @@ -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() diff --git a/src/test/util/mockedpeer.cpp b/src/test/util/mockedpeer.cpp index e182d464..32f0ec6f 100644 --- a/src/test/util/mockedpeer.cpp +++ b/src/test/util/mockedpeer.cpp @@ -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 slotName; + Matcher signalName; Matcher 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 SyncMessage(Matcher className, Match return MakeMatcher(new ProtocolMessageMatcher{SyncMessageExpectation{std::move(className), std::move(objectName), std::move(slotName), std::move(params)}}); } -Matcher RpcCall(Matcher slotName, Matcher params) +Matcher RpcCall(Matcher signalName, Matcher params) { - return MakeMatcher(new ProtocolMessageMatcher{RpcCallExpectation{std::move(slotName), std::move(params)}}); + return MakeMatcher(new ProtocolMessageMatcher{RpcCallExpectation{std::move(signalName), std::move(params)}}); } Matcher InitRequest(Matcher className, Matcher objectName) diff --git a/src/test/util/mockedpeer.h b/src/test/util/mockedpeer.h index fde3d1c5..6533da38 100644 --- a/src/test/util/mockedpeer.h +++ b/src/test/util/mockedpeer.h @@ -90,7 +90,7 @@ TEST_UTIL_EXPORT ::testing::Matcher SyncMessage( ::testing::Matcher params); TEST_UTIL_EXPORT ::testing::Matcher RpcCall( - ::testing::Matcher slotName, + ::testing::Matcher signalName, ::testing::Matcher params); TEST_UTIL_EXPORT ::testing::Matcher InitRequest( diff --git a/tests/common/signalproxytest.cpp b/tests/common/signalproxytest.cpp index a6b91fae..d8442f61 100644 --- a/tests/common/signalproxytest.cpp +++ b/tests/common/signalproxytest.cpp @@ -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()); } // -----------------------------------------------------------------------------------------------------------------------------------------