From b385baadc8e73ff3c499a0111e2a553d35dd29b6 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Mar 2018 12:30:23 -0700 Subject: Direct chat (un)marking: update internal structure synchronously The asynchronous update first implemented was more verbose and caused more problems than provided solutions. The idea was that the internal directChats map would better reflect the server state if updated asynchronously. However, it also causes a local race condition; e.g., to quickly remove rooms from direct chats one after another becomes very non-trivial (one has to wait until the previous operation succeeds). So after some playing with the code, hitting pitfalls along the way, I decided to align the logic with the one for room tags; synchronously issued signals look uglyish but at least work predictably. And race conditions between several clients generally cannot be cleanly resolved anyway. --- examples/qmc-example.cpp | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) (limited to 'examples') diff --git a/examples/qmc-example.cpp b/examples/qmc-example.cpp index 5ea91856..23a1bff1 100644 --- a/examples/qmc-example.cpp +++ b/examples/qmc-example.cpp @@ -126,7 +126,8 @@ void QMCTest::addAndRemoveTag() if (targetRoom->tags().contains(TestTag)) targetRoom->removeTag(TestTag); - QObject::connect(targetRoom, &Room::tagsChanged, targetRoom, [=] { + // Connect first because the signal is emitted synchronously. + connect(targetRoom, &Room::tagsChanged, targetRoom, [=] { cout << "Room " << targetRoom->id().toStdString() << ", tag(s) changed:" << endl << " " << targetRoom->tagNames().join(", ").toStdString() << endl; @@ -138,7 +139,6 @@ void QMCTest::addAndRemoveTag() QObject::disconnect(targetRoom, &Room::tagsChanged, nullptr, nullptr); } }); - // The reverse order because tagsChanged is emitted synchronously. cout << "Adding a tag" << endl; targetRoom->addTag(TestTag); } @@ -211,40 +211,31 @@ void QMCTest::checkRedactionOutcome(QString evtIdToRedact, void QMCTest::markDirectChat() { - if (c->isDirectChat(targetRoom)) + if (c->isDirectChat(targetRoom->id())) { cout << "Warning: the room is already a direct chat," " only unmarking will be tested" << endl; checkDirectChatOutcome(); } - cout << "Marking the room as a direct chat" << endl; - c->addToDirectChats(targetRoom, c->user()); + // Connect first because the signal is emitted synchronously. connect(c.data(), &Connection::directChatsListChanged, this, &QMCTest::checkDirectChatOutcome); + cout << "Marking the room as a direct chat" << endl; + c->addToDirectChats(targetRoom, c->user()); } void QMCTest::checkDirectChatOutcome() { - if (!c->isDirectChat(targetRoom)) + disconnect(c.data(), &Connection::directChatsListChanged, nullptr, nullptr); + if (!c->isDirectChat(targetRoom->id())) { - cout << "Room not (yet?) added to direct chats, waiting" << endl; + QMC_CHECK("Direct chat test", false); return; } cout << "Room marked as a direct chat, unmarking now" << endl; - disconnect(c.data(), &Connection::directChatsListChanged, nullptr, nullptr); - c->removeFromDirectChats(targetRoom, c->user()); - connect(c.data(), &Connection::directChatsListChanged, this, [this] { - if (c->isDirectChat(targetRoom)) - { - cout << "Room not (yet?) removed from direct chats, waiting" << endl; - return; - } - - QMC_CHECK("Direct chat test", !c->isDirectChat(targetRoom)); - disconnect(c.data(), &Connection::directChatsListChanged, - nullptr, nullptr); - }); + c->removeFromDirectChats(targetRoom->id(), c->user()); + QMC_CHECK("Direct chat test", !c->isDirectChat(targetRoom->id())); } void QMCTest::finalize() -- cgit v1.2.3