diff --git a/plugins/sms/conversationsdbusinterface.cpp b/plugins/sms/conversationsdbusinterface.cpp index a2493be40..de9fba3cd 100644 --- a/plugins/sms/conversationsdbusinterface.cpp +++ b/plugins/sms/conversationsdbusinterface.cpp @@ -43,6 +43,12 @@ ConversationsDbusInterface::ConversationsDbusInterface(KdeConnectPlugin* plugin) ConversationsDbusInterface::~ConversationsDbusInterface() { + // Wake all threads which were waiting for a reply from this interface + // This might result in some noise on dbus, but it's better than leaking a bunch of resources! + waitingForMessagesLock.lock(); + conversationsWaitingForMessages.clear(); + waitingForMessages.wakeAll(); + waitingForMessagesLock.unlock(); } QVariantList ConversationsDbusInterface::activeConversations() @@ -130,6 +136,12 @@ QList ConversationsDbusInterface::getConversation(const qin void ConversationsDbusInterface::updateConversation(const qint64& conversationID) { waitingForMessagesLock.lock(); + if (conversationsWaitingForMessages.contains(conversationID)) { + // This conversation is already being waited on, don't allow more than one thread to wait at a time + qCDebug(KDECONNECT_CONVERSATIONS) << "Not allowing two threads to wait for conversationID" << conversationID; + waitingForMessagesLock.unlock(); + return; + } qCDebug(KDECONNECT_CONVERSATIONS) << "Requesting conversation with ID" << conversationID << "from remote"; conversationsWaitingForMessages.insert(conversationID); m_smsInterface.requestConversation(conversationID); diff --git a/plugins/sms/requestconversationworker.cpp b/plugins/sms/requestconversationworker.cpp index 346085f39..2b9714c60 100644 --- a/plugins/sms/requestconversationworker.cpp +++ b/plugins/sms/requestconversationworker.cpp @@ -28,10 +28,12 @@ RequestConversationWorker::RequestConversationWorker(const qint64& conversationI //QObject(interface) conversationID(conversationID) , start(start) - , end(end) , parent(interface) , m_thread(new QThread) { + Q_ASSERT(end >= start && "Not allowed to have a negative-length range"); + howMany = end - start; + this->moveToThread(m_thread); connect(m_thread, &QThread::started, this, &RequestConversationWorker::handleRequestConversation); @@ -52,31 +54,39 @@ void RequestConversationWorker::handleRequestConversation() qCWarning(KDECONNECT_CONVERSATIONS) << "Got a conversationID for a conversation with no messages!" << conversationID; } - // TODO: Reply with all messages we currently have available, even if we don't have enough to completely fill the request // In case the remote takes awhile to respond, we should go ahead and do anything we can from the cache + size_t numHandled = replyForConversation(messagesList, start, howMany); - if (messagesList.length() <= end) { + if (numHandled < howMany) { + size_t numRemaining = howMany - numHandled; // If we don't have enough messages in cache, go get some more // TODO: Make Android interface capable of requesting small window of messages parent->updateConversation(conversationID); messagesList = parent->getConversation(conversationID); - } - - // Messages are sorted in ascending order of keys, meaning the front of the list has the oldest - // messages (smallest timestamp number) - // Therefore, return the end of the list first (most recent messages) - int i = start; - for(auto it = messagesList.crbegin() + start; it != messagesList.crend(); ++it) { - Q_EMIT conversationMessageRead(it->toVariant()); - i++; - if (i >= end) { - break; - } + //ConversationsDbusInterface::getConversation blocks until it sees new messages in the requested conversation + replyForConversation(messagesList, start + numHandled, numRemaining); } Q_EMIT finished(); } +size_t RequestConversationWorker::replyForConversation(const QList& conversation, int start, size_t howMany) { + Q_ASSERT(start >= 0); + // Messages are sorted in ascending order of keys, meaning the front of the list has the oldest + // messages (smallest timestamp number) + // Therefore, return the end of the list first (most recent messages) + int i = 0; + for(auto it = conversation.crbegin() + start; it != conversation.crend(); ++it) { + if (i >= howMany) { + break; + } + Q_EMIT conversationMessageRead(it->toVariant()); + i++; + } + + return i; +} + void RequestConversationWorker::work() { m_thread->start(); diff --git a/plugins/sms/requestconversationworker.h b/plugins/sms/requestconversationworker.h index 4519f58a5..75ec5c01e 100644 --- a/plugins/sms/requestconversationworker.h +++ b/plugins/sms/requestconversationworker.h @@ -41,6 +41,13 @@ public: public Q_SLOTS: + /** + * Main body of this worker + * + * Reply to a request for messages and, if needed, wait for the remote to reply with more + * + * Emits conversationMessageRead for every message handled + */ void handleRequestConversation(); void work(); @@ -50,11 +57,24 @@ Q_SIGNALS: private: qint64 conversationID; - int start; - int end; + int start; // Start of range to request messages + size_t howMany; // Number of messages being requested ConversationsDbusInterface* parent; QThread* m_thread; + + /** + * Reply with all messages we currently have available in the requested range + * + * If the range specified by start and howMany is not in the range of messages in the conversation, + * reply with only as many messages as we have available in that range + * + * @param conversation Conversation to send messages from + * @param start Start of requested range, 0-indexed, inclusive + * @param howMany Maximum number of messages to return + * $return Number of messages processed + */ + size_t replyForConversation(const QList& conversation, int start, size_t howMany); }; #endif // REQUESTCONVERSATIONWORKER_H diff --git a/smsapp/conversationlistmodel.cpp b/smsapp/conversationlistmodel.cpp index 8190332c0..cc8c0e93a 100644 --- a/smsapp/conversationlistmodel.cpp +++ b/smsapp/conversationlistmodel.cpp @@ -53,6 +53,10 @@ void ConversationListModel::setDeviceId(const QString& deviceId) return; } + if (deviceId == "") { + return; + } + qCDebug(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "setDeviceId" << deviceId << "of" << this; if (m_conversationsInterface) { diff --git a/smsapp/qml/ConversationDisplay.qml b/smsapp/qml/ConversationDisplay.qml index 68f95ab60..cc6b056b3 100644 --- a/smsapp/qml/ConversationDisplay.qml +++ b/smsapp/qml/ConversationDisplay.qml @@ -34,7 +34,9 @@ Kirigami.ScrollablePage readonly property QtObject person: PersonData { id: person } - property QtObject device + + property bool deviceConnected + property string deviceId property int conversationId property string phoneNumber @@ -66,7 +68,7 @@ Kirigami.ScrollablePage sortOrder: Qt.AscendingOrder sortRole: ConversationModel.DateRole sourceModel: ConversationModel { - deviceId: device.id() + deviceId: page.deviceId threadId: page.conversationId } } @@ -120,7 +122,7 @@ Kirigami.ScrollablePage } footer: RowLayout { - enabled: page.device + enabled: page.deviceConnected ScrollView { Layout.maximumHeight: page.height / 3 Layout.fillWidth: true diff --git a/smsapp/qml/ConversationList.qml b/smsapp/qml/ConversationList.qml index 7e1a86ff7..c405d2278 100644 --- a/smsapp/qml/ConversationList.qml +++ b/smsapp/qml/ConversationList.qml @@ -30,6 +30,7 @@ import org.kde.kdeconnect.sms 1.0 Kirigami.ScrollablePage { + id: page footer: ComboBox { id: devicesCombo enabled: count > 0 @@ -50,11 +51,16 @@ Kirigami.ScrollablePage visible: !devicesCombo.enabled } + readonly property bool deviceConnected: devicesCombo.enabled readonly property QtObject device: devicesCombo.currentIndex >= 0 ? devicesModel.data(devicesModel.index(devicesCombo.currentIndex, 0), DevicesModel.DeviceRole) : null + readonly property alias lastDeviceId: conversationListModel.deviceId Component { id: chatView - ConversationDisplay {} + ConversationDisplay { + deviceId: page.lastDeviceId + deviceConnected: page.deviceConnected + } } ListView { @@ -66,6 +72,7 @@ Kirigami.ScrollablePage sortRole: ConversationListModel.DateRole filterCaseSensitivity: Qt.CaseInsensitive sourceModel: ConversationListModel { + id: conversationListModel deviceId: device ? device.id() : "" } } @@ -113,7 +120,7 @@ Kirigami.ScrollablePage personUri: model.personUri, phoneNumber: address, conversationId: model.conversationId, - device: device}) + }) } onClicked: { startChat(); } }