diff --git a/plugins/sms/conversationsdbusinterface.cpp b/plugins/sms/conversationsdbusinterface.cpp --- a/plugins/sms/conversationsdbusinterface.cpp +++ b/plugins/sms/conversationsdbusinterface.cpp @@ -43,6 +43,12 @@ 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 @@ 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.h b/plugins/sms/requestconversationworker.h --- a/plugins/sms/requestconversationworker.h +++ b/plugins/sms/requestconversationworker.h @@ -41,6 +41,13 @@ 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 @@ 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/plugins/sms/requestconversationworker.cpp b/plugins/sms/requestconversationworker.cpp --- a/plugins/sms/requestconversationworker.cpp +++ b/plugins/sms/requestconversationworker.cpp @@ -28,10 +28,12 @@ //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,29 +54,37 @@ 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); + //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 = start; - for(auto it = messagesList.crbegin() + start; it != messagesList.crend(); ++it) { - Q_EMIT conversationMessageRead(it->toVariant()); - i++; - if (i >= end) { + int i = 0; + for(auto it = conversation.crbegin() + start; it != conversation.crend(); ++it) { + if (i >= howMany) { break; } + Q_EMIT conversationMessageRead(it->toVariant()); + i++; } - Q_EMIT finished(); + return i; } void RequestConversationWorker::work() diff --git a/smsapp/conversationlistmodel.h b/smsapp/conversationlistmodel.h --- a/smsapp/conversationlistmodel.h +++ b/smsapp/conversationlistmodel.h @@ -68,7 +68,7 @@ : public QStandardItemModel { Q_OBJECT - Q_PROPERTY(QString deviceId READ deviceId WRITE setDeviceId) + Q_PROPERTY(QString deviceId READ deviceId WRITE setDeviceId NOTIFY deviceIdChanged) public: ConversationListModel(QObject* parent = nullptr); @@ -92,6 +92,9 @@ void createRowFromMessage(const QVariantMap& message); void printDBusError(const QDBusError& error); +Q_SIGNALS: + void deviceIdChanged(); + private: /** * Get all conversations currently known by the conversationsInterface, if any diff --git a/smsapp/conversationlistmodel.cpp b/smsapp/conversationlistmodel.cpp --- a/smsapp/conversationlistmodel.cpp +++ b/smsapp/conversationlistmodel.cpp @@ -53,6 +53,10 @@ return; } + if (deviceId == "") { + return; + } + qCDebug(KDECONNECT_SMS_CONVERSATIONS_LIST_MODEL) << "setDeviceId" << deviceId << "of" << this; if (m_conversationsInterface) { @@ -63,6 +67,7 @@ } m_deviceId = deviceId; + Q_EMIT deviceIdChanged(); // This method still gets called *with a valid deviceID* when the device is not connected while the component is setting up // Detect that case and don't do anything. diff --git a/smsapp/conversationmodel.cpp b/smsapp/conversationmodel.cpp --- a/smsapp/conversationmodel.cpp +++ b/smsapp/conversationmodel.cpp @@ -51,7 +51,8 @@ m_threadId = threadId; clear(); - if (threadId != INVALID_THREAD_ID) { + knownMessageIDs.clear(); + if (m_threadId != INVALID_THREAD_ID && m_deviceId != "") { requestMoreMessages(); } } diff --git a/smsapp/qml/ConversationDisplay.qml b/smsapp/qml/ConversationDisplay.qml --- a/smsapp/qml/ConversationDisplay.qml +++ b/smsapp/qml/ConversationDisplay.qml @@ -34,7 +34,9 @@ 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 @@ sortOrder: Qt.AscendingOrder sortRole: ConversationModel.DateRole sourceModel: ConversationModel { - deviceId: device.id() + deviceId: page.deviceId threadId: page.conversationId } } @@ -120,7 +122,7 @@ } 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 --- a/smsapp/qml/ConversationList.qml +++ b/smsapp/qml/ConversationList.qml @@ -30,6 +30,7 @@ Kirigami.ScrollablePage { + id: page footer: ComboBox { id: devicesCombo enabled: count > 0 @@ -50,11 +51,14 @@ 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 { + ConversationDisplay { id: chatView - ConversationDisplay {} + deviceId: page.lastDeviceId + deviceConnected: page.deviceConnected } ListView { @@ -66,6 +70,7 @@ sortRole: ConversationListModel.DateRole filterCaseSensitivity: Qt.CaseInsensitive sourceModel: ConversationListModel { + id: conversationListModel deviceId: device ? device.id() : "" } } @@ -113,9 +118,19 @@ personUri: model.personUri, phoneNumber: address, conversationId: model.conversationId, - device: device}) + }) } - onClicked: { startChat(); } + onClicked: { + startChat(); + view.currentIndex = index + } + // Keep the currently-open chat highlighted even if this element is not focused + highlighted: chatView.conversationId == model.conversationId + } + + Component.onCompleted: { + currentIndex = -1 + focus = true } }