Swift
summaryrefslogtreecommitdiffstats
path: root/Swift
diff options
context:
space:
mode:
authorPeter Burgess <pete.burgess@isode.com>2018-07-24 15:43:55 (GMT)
committerKevin Smith <kevin.smith@isode.com>2018-07-26 13:43:37 (GMT)
commit86fb79cb628fd830232603105c115ebec0cb7bcb (patch)
tree2394330ab0b9f807f293ff2d2e2fd009dbee9592 /Swift
parentd3bbd300be4480c0a3b7285c91b3b27e82ca9c2f (diff)
downloadswift-86fb79cb628fd830232603105c115ebec0cb7bcb.zip
swift-86fb79cb628fd830232603105c115ebec0cb7bcb.tar.bz2
Handle changes in the new roster model properly
The new roster was temporarily resetting the model every time there was a change, this patch sets it up properly to use beginXxxRows, endXxxRows and dataChanged instead. Test-Information: Unit test created to check the onBeginAdd and onAdded signals get called at the correct time and that data is stored properly when a JID is added. Also tested live on test server, checked what happens when the user first logs in, and when contacts log on and off. There was some concern over the new setState function, and its impact on populating the new roster on login. I ran multiple tests, swift-4 vs swift-5 with the new roster, I tested having 3000 users with and without presence in the roster, 100 users and 3000 users split into 30 groups of 100. We tested both the time to populate the roster (pre presence) and the time from login until the roster become responsive and usable. The results weren't terribly different between swift-4 and swift-5 with the new roster, and swift-5 was populating both the old and new rosters, therefore we concluded that the new roster is not causing a significant slowing down during login. Change-Id: I2aba5cbcd81296b49d36c54ee7f07453f1b2db08
Diffstat (limited to 'Swift')
-rw-r--r--Swift/Controllers/Chat/Chattables.cpp12
-rw-r--r--Swift/Controllers/Chat/Chattables.h6
-rw-r--r--Swift/Controllers/Chat/UnitTest/ChattablesTest.cpp137
-rw-r--r--Swift/Controllers/SConscript1
-rw-r--r--Swift/QtUI/ChattablesModel.cpp19
-rw-r--r--Swift/QtUI/ChattablesModel.h6
6 files changed, 171 insertions, 10 deletions
diff --git a/Swift/Controllers/Chat/Chattables.cpp b/Swift/Controllers/Chat/Chattables.cpp
index 599ff48..707046f 100644
--- a/Swift/Controllers/Chat/Chattables.cpp
+++ b/Swift/Controllers/Chat/Chattables.cpp
@@ -24,14 +24,20 @@ void Chattables::addJID(const JID& jid, State::Type type) {
State state;
state.type = type;
state.jid = jid;
+ onBeginAdd(static_cast<int>(list_.size()));
list_.push_back(jid);
states_[jid] = state;
- onAdded(jid);
+ onAdded();
}
void Chattables::setState(const JID& jid, State state) {
- states_[jid] = state;
- onChanged(jid);
+ auto stateIter = states_.find(jid);
+ if (stateIter == states_.end()) {
+ return;
+ }
+ stateIter->second = state;
+ auto listPos = static_cast<int>(std::distance(list_.begin(), std::find(list_.begin(), list_.end(), jid)));
+ onChanged(jid, listPos);
}
}
diff --git a/Swift/Controllers/Chat/Chattables.h b/Swift/Controllers/Chat/Chattables.h
index c5c7b85..c115fb3 100644
--- a/Swift/Controllers/Chat/Chattables.h
+++ b/Swift/Controllers/Chat/Chattables.h
@@ -34,9 +34,9 @@ class Chattables {
void addJID(const JID& jid, State::Type type);
void setState(const JID& jid, State state);
- boost::signals2::signal<void (const JID&)> onAdded;
- boost::signals2::signal<void (const JID&)> onRemoved;
- boost::signals2::signal<void (const JID&)> onChanged;
+ boost::signals2::signal<void (int)> onBeginAdd;
+ boost::signals2::signal<void ()> onAdded;
+ boost::signals2::signal<void (const JID&, int)> onChanged;
/// The UI has activated a chattable item (e.g. clicked in the roster)
boost::signals2::signal<void (const JID&)> onActivated;
private:
diff --git a/Swift/Controllers/Chat/UnitTest/ChattablesTest.cpp b/Swift/Controllers/Chat/UnitTest/ChattablesTest.cpp
new file mode 100644
index 0000000..6339ad9
--- /dev/null
+++ b/Swift/Controllers/Chat/UnitTest/ChattablesTest.cpp
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2018 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
+
+#include <gtest/gtest.h>
+#include <hippomocks.h>
+
+#include <Swift/Controllers/Chat/Chattables.h>
+
+using namespace Swift;
+
+class ChattablesTest : public ::testing::Test {
+ protected:
+ void SetUp() {
+ beginListSize_ = 0;
+ endListSize_ = 0;
+ callsToBegin_ = 0;
+ callsToEnd_ = 0;
+ }
+
+ void TearDown() {
+ }
+
+ void updateBeginListSize(int listSize);
+ void updateEndListSize();
+
+ Chattables chattables_;
+ int beginListSize_;
+ int endListSize_;
+ int callsToBegin_;
+ int callsToEnd_;
+};
+
+void ChattablesTest::updateBeginListSize(int listSize) {
+ beginListSize_ = listSize;
+ callsToBegin_++;
+}
+
+void ChattablesTest::updateEndListSize() {
+ endListSize_ = chattables_.get().size();
+ callsToEnd_++;
+}
+
+TEST_F(ChattablesTest, testAddJID) {
+ chattables_.onBeginAdd.connect([this](int listSize){ updateBeginListSize(listSize); });
+ chattables_.onAdded.connect([this](){ updateEndListSize(); });
+ JID jid("user@swift.jid");
+
+ chattables_.addJID(jid, Chattables::State::Type::Person);
+ ASSERT_EQ(0, beginListSize_);
+ ASSERT_EQ(1, endListSize_);
+ ASSERT_EQ(1, callsToBegin_);
+ ASSERT_EQ(1, callsToEnd_);
+
+ ASSERT_EQ(jid, chattables_.get()[0]);
+ Chattables::State state = chattables_.getState(jid);
+ ASSERT_EQ(jid, state.jid);
+ ASSERT_EQ(Chattables::State::Type::Person, state.type);
+}
+
+TEST_F(ChattablesTest, testAddMultipleJIDs) {
+ chattables_.onBeginAdd.connect([this](int listSize){ updateBeginListSize(listSize); });
+ chattables_.onAdded.connect([this](){ updateEndListSize(); });
+ JID jid0("user0@swift.jid");
+ JID jid1("user1@swift.jid");
+ JID jid2("user2@swift.jid");
+
+ chattables_.addJID(jid0, Chattables::State::Type::Person);
+ ASSERT_EQ(0, beginListSize_);
+ ASSERT_EQ(1, endListSize_);
+ ASSERT_EQ(1, callsToBegin_);
+ ASSERT_EQ(1, callsToEnd_);
+ chattables_.addJID(jid1, Chattables::State::Type::Person);
+ ASSERT_EQ(1, beginListSize_);
+ ASSERT_EQ(2, endListSize_);
+ ASSERT_EQ(2, callsToBegin_);
+ ASSERT_EQ(2, callsToEnd_);
+ chattables_.addJID(jid2, Chattables::State::Type::Person);
+ ASSERT_EQ(2, beginListSize_);
+ ASSERT_EQ(3, endListSize_);
+ ASSERT_EQ(3, callsToBegin_);
+ ASSERT_EQ(3, callsToEnd_);
+}
+
+TEST_F(ChattablesTest, testAddRoomJID) {
+ chattables_.onBeginAdd.connect([this](int listSize){ updateBeginListSize(listSize); });
+ chattables_.onAdded.connect([this](){ updateEndListSize(); });
+ JID jid("room@swift.jid");
+
+ chattables_.addJID(jid, Chattables::State::Type::Room);
+ ASSERT_EQ(0, beginListSize_);
+ ASSERT_EQ(1, endListSize_);
+ ASSERT_EQ(1, callsToBegin_);
+ ASSERT_EQ(1, callsToEnd_);
+
+ ASSERT_EQ(jid, chattables_.get()[0]);
+ Chattables::State state = chattables_.getState(jid);
+ ASSERT_EQ(jid, state.jid);
+ ASSERT_EQ(Chattables::State::Type::Room, state.type);
+}
+
+TEST_F(ChattablesTest, testSetState) {
+ JID jid("user@swift.jid");
+ chattables_.addJID(jid, Chattables::State::Type::Person);
+ ASSERT_EQ(1, chattables_.get().size());
+ ASSERT_EQ(jid, chattables_.get()[0]);
+ ASSERT_EQ(Chattables::State::Type::Person, chattables_.getState(jid).type);
+ ASSERT_EQ(StatusShow::None, chattables_.getState(jid).status);
+
+ JID returnedJID;
+ int returnedIndex;
+ int callsToChanged = 0;
+ chattables_.onChanged.connect([this, &returnedJID, &returnedIndex, &callsToChanged](const JID& jid, int index){
+ returnedJID = jid;
+ returnedIndex = index;
+ callsToChanged++;
+ });
+
+ Chattables::State newState;
+ newState.jid = jid;
+ newState.type = Chattables::State::Type::Room;
+ newState.status = StatusShow::Online;
+ chattables_.setState(jid, newState);
+
+ auto storedState = chattables_.getState(jid);
+
+ ASSERT_EQ(1, chattables_.get().size());
+ ASSERT_EQ(jid, chattables_.get()[0]);
+ ASSERT_EQ(jid, returnedJID);
+ ASSERT_EQ(0, returnedIndex);
+ ASSERT_EQ(1, callsToChanged);
+ ASSERT_EQ(jid, storedState.jid);
+ ASSERT_EQ(Chattables::State::Type::Room, storedState.type);
+ ASSERT_EQ(StatusShow::Online, storedState.status);
+}
diff --git a/Swift/Controllers/SConscript b/Swift/Controllers/SConscript
index 25d326a..3840fbf 100644
--- a/Swift/Controllers/SConscript
+++ b/Swift/Controllers/SConscript
@@ -100,6 +100,7 @@ if env["SCONS_STAGE"] == "build" :
File("Chat/UnitTest/ChatMessageParserTest.cpp"),
File("Chat/UnitTest/ChatsManagerTest.cpp"),
File("Chat/UnitTest/ChatControllerTest.cpp"),
+ File("Chat/UnitTest/ChattablesTest.cpp"),
File("Chat/UnitTest/MUCControllerTest.cpp"),
File("Roster/UnitTest/LeastCommonSubsequenceTest.cpp"),
File("Roster/UnitTest/RosterControllerTest.cpp"),
diff --git a/Swift/QtUI/ChattablesModel.cpp b/Swift/QtUI/ChattablesModel.cpp
index d1257b9..67d0579 100644
--- a/Swift/QtUI/ChattablesModel.cpp
+++ b/Swift/QtUI/ChattablesModel.cpp
@@ -15,10 +15,21 @@
namespace Swift {
ChattablesModel::ChattablesModel(Chattables& chattables, QObject* parent) : QAbstractListModel(parent), chattables_(chattables) {
- //FIXME: scoped connections, do it properly not reset.
- chattables_.onAdded.connect([this](const JID& /*jid*/) {beginResetModel(); endResetModel();});
- chattables_.onRemoved.connect([this](const JID& /*jid*/) {beginResetModel(); endResetModel();});
- chattables_.onChanged.connect([this](const JID& /*jid*/) {beginResetModel(); endResetModel();});
+ using scoped_connection = boost::signals2::scoped_connection;
+ connectionList_.emplace_back(std::make_unique<scoped_connection>(
+ chattables_.onBeginAdd.connect([this](int index) {beginInsertRows(QModelIndex(), index, index);})
+ ));
+ connectionList_.emplace_back(std::make_unique<scoped_connection>(
+ chattables_.onAdded.connect([this]() {endInsertRows();})
+ ));
+ connectionList_.emplace_back(std::make_unique<scoped_connection>(
+ chattables_.onChanged.connect(
+ [this](const JID& jid, int index) {
+ auto modelIndex = createIndex(index, 0, const_cast<JID*>(&jid));
+ dataChanged(modelIndex, modelIndex, {});
+ }
+ )
+ ));
}
int ChattablesModel::rowCount(const QModelIndex& /*parent*/) const {
diff --git a/Swift/QtUI/ChattablesModel.h b/Swift/QtUI/ChattablesModel.h
index 57073aa..6617d97 100644
--- a/Swift/QtUI/ChattablesModel.h
+++ b/Swift/QtUI/ChattablesModel.h
@@ -6,6 +6,11 @@
#pragma once
+#include <memory>
+#include <vector>
+
+#include <boost/signals2/connection.hpp>
+
#include <QAbstractListModel>
namespace Swift {
@@ -23,6 +28,7 @@ class ChattablesModel : public QAbstractListModel {
QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const;
private:
Chattables& chattables_;
+ std::vector<std::unique_ptr<boost::signals2::scoped_connection>> connectionList_;
};
}