From b8ce41ef6c0036d854f5bef0fb52e2a69dc5def2 Mon Sep 17 00:00:00 2001 From: Manuel Nickschas Date: Wed, 17 Oct 2018 22:14:24 +0200 Subject: [PATCH] sigproxy: Actually rename SyncableObjects when requested Until now, renaming a SyncableObject would properly update the maps in SignalProxy, but it would not actually update the objectName property on the client side. Fix this. Remove the renameObject() method in SyncableObject in favor of using setObjectName() and listening to the notifier signal emitted by Qt for triggering the update. This ensures that the actual object name and the expected one stay in sync. Also change places where we unnecessarily rename right after construction to use the appropriate constructor instead. Extend the test case to cover the renaming of SyncableObject. --- src/common/dccconfig.cpp | 3 +-- src/common/identity.cpp | 2 +- src/common/ircuser.cpp | 2 +- src/common/signalproxy.cpp | 9 ++++++--- src/common/syncableobject.cpp | 30 ++++++++++++++---------------- src/common/syncableobject.h | 3 ++- src/common/transfer.cpp | 2 +- src/common/transfermanager.cpp | 4 +--- src/core/coreidentity.cpp | 2 +- tests/common/signalproxytest.cpp | 13 ++++++++++++- 10 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/common/dccconfig.cpp b/src/common/dccconfig.cpp index ae9a0e00..811db523 100644 --- a/src/common/dccconfig.cpp +++ b/src/common/dccconfig.cpp @@ -25,7 +25,7 @@ #include "types.h" DccConfig::DccConfig(QObject* parent) - : SyncableObject(parent) + : SyncableObject("DccConfig", parent) { static auto regTypes = []() -> bool { qRegisterMetaTypeStreamOperators("DccConfig::IpDetectionMode"); @@ -34,7 +34,6 @@ DccConfig::DccConfig(QObject* parent) }(); Q_UNUSED(regTypes); - renameObject("DccConfig"); setAllowClientUpdates(true); } diff --git a/src/common/identity.cpp b/src/common/identity.cpp index 4b78cfd1..9fcd4a0b 100644 --- a/src/common/identity.cpp +++ b/src/common/identity.cpp @@ -195,7 +195,7 @@ void Identity::setId(IdentityId _id) _identityId = _id; SYNC(ARG(_id)) emit idSet(_id); - renameObject(QString::number(id().toInt())); + setObjectName(QString::number(id().toInt())); } void Identity::setIdentityName(const QString& identityName) diff --git a/src/common/ircuser.cpp b/src/common/ircuser.cpp index a952ec0f..e0bcc787 100644 --- a/src/common/ircuser.cpp +++ b/src/common/ircuser.cpp @@ -262,7 +262,7 @@ void IrcUser::setEncrypted(bool encrypted) void IrcUser::updateObjectName() { - renameObject(QString::number(network()->networkId().toInt()) + "/" + _nick); + setObjectName(QString::number(network()->networkId().toInt()) + "/" + _nick); } void IrcUser::updateHostmask(const QString& mask) diff --git a/src/common/signalproxy.cpp b/src/common/signalproxy.cpp index 9e47dd38..7434ea52 100644 --- a/src/common/signalproxy.cpp +++ b/src/common/signalproxy.cpp @@ -372,9 +372,12 @@ void SignalProxy::renameObject(const SyncableObject* obj, const QString& newname void SignalProxy::objectRenamed(const QByteArray& classname, const QString& newname, const QString& oldname) { - if (_syncSlave.contains(classname) && _syncSlave[classname].contains(oldname) && oldname != newname) { - SyncableObject* obj = _syncSlave[classname][newname] = _syncSlave[classname].take(oldname); - requestInit(obj); + if (newname != oldname) { + if (_syncSlave.contains(classname) && _syncSlave[classname].contains(oldname)) { + SyncableObject* obj = _syncSlave[classname][newname] = _syncSlave[classname].take(oldname); + obj->setObjectName(newname); + requestInit(obj); + } } } diff --git a/src/common/syncableobject.cpp b/src/common/syncableobject.cpp index 9fd7704f..695ab913 100644 --- a/src/common/syncableobject.cpp +++ b/src/common/syncableobject.cpp @@ -27,20 +27,29 @@ #include "util.h" SyncableObject::SyncableObject(QObject* parent) - : QObject(parent) + : SyncableObject(QString{}, parent) {} SyncableObject::SyncableObject(const QString& objectName, QObject* parent) : QObject(parent) { + _objectName = objectName; setObjectName(objectName); + + connect(this, &QObject::objectNameChanged, this, [this](auto&& newName) { + for (auto&& proxy : _signalProxies) { + proxy->renameObject(this, newName, _objectName); + } + _objectName = newName; + }); } SyncableObject::SyncableObject(const SyncableObject& other, QObject* parent) - : QObject(parent) - , _initialized(other._initialized) - , _allowClientUpdates(other._allowClientUpdates) -{} + : SyncableObject(QString{}, parent) +{ + _initialized = other._initialized; + _allowClientUpdates = other._allowClientUpdates; +} SyncableObject::~SyncableObject() { @@ -156,17 +165,6 @@ bool SyncableObject::setInitValue(const QString& property, const QVariant& value return QMetaObject::invokeMethod(this, handlername.toLatin1(), param); } -void SyncableObject::renameObject(const QString& newName) -{ - const QString oldName = objectName(); - if (oldName != newName) { - setObjectName(newName); - foreach (SignalProxy* proxy, _signalProxies) { - proxy->renameObject(this, newName, oldName); - } - } -} - void SyncableObject::update(const QVariantMap& properties) { fromVariantMap(properties); diff --git a/src/common/syncableobject.h b/src/common/syncableobject.h index 7d5242b1..f1687d94 100644 --- a/src/common/syncableobject.h +++ b/src/common/syncableobject.h @@ -94,7 +94,6 @@ public slots: protected: void sync_call__(SignalProxy::ProxyMode modeType, const char* funcname, ...) const; - void renameObject(const QString& newName); SyncableObject& operator=(const SyncableObject& other); signals: @@ -108,6 +107,8 @@ private: bool setInitValue(const QString& property, const QVariant& value); +private: + QString _objectName; bool _initialized{false}; bool _allowClientUpdates{false}; diff --git a/src/common/transfer.cpp b/src/common/transfer.cpp index d33a9a37..3a9bd524 100644 --- a/src/common/transfer.cpp +++ b/src/common/transfer.cpp @@ -59,7 +59,7 @@ void Transfer::init() }(); Q_UNUSED(regTypes); - renameObject(QString("Transfer/%1").arg(_uuid.toString())); + setObjectName(QString("Transfer/%1").arg(_uuid.toString())); setAllowClientUpdates(true); } diff --git a/src/common/transfermanager.cpp b/src/common/transfermanager.cpp index 4df5589a..d8b11e33 100644 --- a/src/common/transfermanager.cpp +++ b/src/common/transfermanager.cpp @@ -23,15 +23,13 @@ #include "transfer.h" TransferManager::TransferManager(QObject* parent) - : SyncableObject(parent) + : SyncableObject("TransferManager", parent) { static auto regTypes = []() -> bool { qRegisterMetaTypeStreamOperators("TransferManager::TransferIdList"); return true; }(); Q_UNUSED(regTypes); - - renameObject("TransferManager"); } Transfer* TransferManager::transfer(const QUuid& uuid) const diff --git a/src/core/coreidentity.cpp b/src/core/coreidentity.cpp index f0b75b05..91b6519f 100644 --- a/src/core/coreidentity.cpp +++ b/src/core/coreidentity.cpp @@ -110,7 +110,7 @@ CoreCertManager::CoreCertManager(CoreIdentity& identity) void CoreCertManager::setId(IdentityId id) { - renameObject(QString::number(id.toInt())); + setObjectName(QString::number(id.toInt())); } void CoreCertManager::setSslKey(const QByteArray& encoded) diff --git a/tests/common/signalproxytest.cpp b/tests/common/signalproxytest.cpp index 18b71e1e..a6b91fae 100644 --- a/tests/common/signalproxytest.cpp +++ b/tests/common/signalproxytest.cpp @@ -141,7 +141,6 @@ class SyncObj : public SyncableObject Q_PROPERTY(double doubleProperty READ doubleProperty WRITE setDoubleProperty) public: - int intProperty() const { return _intProperty; @@ -271,6 +270,10 @@ TEST_F(SignalProxyTest, syncableObject) {"intProperty", 17}, {"doubleProperty", 2.3} })))); + + // Rename object + EXPECT_CALL(*_serverPeer, Dispatches(RpcCall(Eq("__objectRenamed__"), ElementsAre("SyncObj", "Bar", "Foo")))); + EXPECT_CALL(*_serverPeer, Dispatches(SyncMessage(Eq("SyncObj"), Eq("Bar"), Eq("setStringProperty"), ElementsAre("Hi Universe")))); } SignalSpy spy; @@ -346,6 +349,14 @@ TEST_F(SignalProxyTest, syncableObject) EXPECT_EQ(17, clientObject.intProperty()); EXPECT_EQ("Quassel", clientObject.stringProperty()); EXPECT_EQ(2.3, clientObject.doubleProperty()); + + // -- Rename object + spy.connect(&clientObject, &SyncObj::stringPropertyChanged); + serverObject.setObjectName("Bar"); + serverObject.setStringProperty("Hi Universe"); + ASSERT_TRUE(spy.wait()); + EXPECT_EQ("Bar", clientObject.objectName()); + EXPECT_EQ("Hi Universe", clientObject.stringProperty()); } #include "signalproxytest.moc" -- 2.20.1