diff --git a/plugins/sms/CMakeLists.txt b/plugins/sms/CMakeLists.txt --- a/plugins/sms/CMakeLists.txt +++ b/plugins/sms/CMakeLists.txt @@ -1,6 +1,7 @@ set(kdeconnect_sms_SRCS smsplugin.cpp conversationsdbusinterface.cpp + requestconversationworker.cpp ) include_directories(${CMAKE_BINARY_DIR}) diff --git a/plugins/sms/conversationsdbusinterface.h b/plugins/sms/conversationsdbusinterface.h --- a/plugins/sms/conversationsdbusinterface.h +++ b/plugins/sms/conversationsdbusinterface.h @@ -49,9 +49,20 @@ explicit ConversationsDbusInterface(KdeConnectPlugin* plugin); ~ConversationsDbusInterface() override; - void addMessage(const ConversationMessage &message); + void addMessages(const QList &messages); void removeMessage(const QString& internalId); + /** + * Return a shallow copy of the requested conversation + */ + QList getConversation(const qint32& conversationID) const; + + /** + * Get all of the messages in the requested conversation from the remote device + * TODO: Make interface capable of requesting smaller window of messages + */ + void updateConversation(const qint32& conversationID); + public Q_SLOTS: /** * Return a list of the first message in every conversation @@ -61,23 +72,43 @@ */ QVariantList activeConversations(); - void requestConversation(const QString &conversationID, int start, int end); + /** + * Request the specified range of the specified conversation + * + * Emits conversationUpdated for every message in the requested range + * + * If the conversation does not have enough messages to fill the request, + * this method may return fewer messages + */ + void requestConversation(const qint32 &conversationID, int start, int end); /** * Send a new message to this conversation */ - void replyToConversation(const QString& conversationID, const QString& message); + void replyToConversation(const qint32& conversationID, const QString& message); /** * Send the request to the Telephony plugin to update the list of conversation threads */ void requestAllConversationThreads(); Q_SIGNALS: + /** + * Emitted whenever a conversation with no cached messages is added, either because the cache + * is being populated or because a new conversation has been created + */ Q_SCRIPTABLE void conversationCreated(const QVariantMap& msg); - Q_SCRIPTABLE void conversationRemoved(const QString& threadID); + + /** + * Emitted whenever a conversation is being deleted + */ + Q_SCRIPTABLE void conversationRemoved(const qint32& conversationID); + + /** + * Emitted whenever a message is added to a conversation and it is the newest message in the + * conversation + */ Q_SCRIPTABLE void conversationUpdated(const QVariantMap& msg) const; - Q_SCRIPTABLE void conversationMessageReceived(const QVariantMap& msg, int pos) const; private /*methods*/: QString newId(); //Generates successive identifitiers to use as public ids @@ -92,16 +123,20 @@ * The messages are stored as a QMap of the timestamp to the actual message object so that * we can use .values() to get a sorted list of messages from least- to most-recent */ - QHash> m_conversations; + QHash> m_conversations; /** * Mapping of threadID to the set of uIDs known in the corresponding conversation */ - QHash> m_known_messages; + QHash> m_known_messages; int m_lastId; SmsDbusInterface m_smsInterface; + + QSet conversationsWaitingForMessages; + QMutex waitingForMessagesLock; + QWaitCondition waitingForMessages; }; #endif // CONVERSATIONSDBUSINTERFACE_H diff --git a/plugins/sms/conversationsdbusinterface.cpp b/plugins/sms/conversationsdbusinterface.cpp --- a/plugins/sms/conversationsdbusinterface.cpp +++ b/plugins/sms/conversationsdbusinterface.cpp @@ -22,6 +22,8 @@ #include "interfaces/dbusinterfaces.h" #include "interfaces/conversationmessage.h" +#include "requestconversationworker.h" + #include #include @@ -64,60 +66,84 @@ return toReturn; } -void ConversationsDbusInterface::requestConversation(const QString& conversationID, int start, int end) +void ConversationsDbusInterface::requestConversation(const qint32& conversationID, int start, int end) { - const auto messagesList = m_conversations[conversationID].values(); + QThread* handlingThread = new QThread(); + RequestConversationWorker* worker = new RequestConversationWorker(conversationID, start, end, this); + worker->moveToThread(handlingThread); + connect(handlingThread, &QThread::started, + worker, &RequestConversationWorker::handleRequestConversation); + connect(handlingThread, &QThread::finished, + handlingThread, &QObject::deleteLater); + connect(worker, &RequestConversationWorker::finished, + handlingThread, &QThread::quit); + connect(worker, &RequestConversationWorker::finished, + worker, &QObject::deleteLater); + connect(worker, &RequestConversationWorker::conversationMessageRead, + this, &ConversationsDbusInterface::conversationUpdated, + Qt::QueuedConnection); + handlingThread->start(); +} - if (messagesList.isEmpty()) { - // Since there are no messages in the conversation, it's likely that it is a junk ID, but go ahead anyway - qCWarning(KDECONNECT_CONVERSATIONS) << "Got a conversationID for a conversation with no messages!" << conversationID; - } +void ConversationsDbusInterface::addMessages(const QList &messages) +{ + QSet updatedConversationIDs; - // TODO: Check local cache before requesting new messages - // TODO: Make Android interface capable of requesting small window of messages - m_smsInterface.requestConversation(conversationID); + for (const auto& message : messages) { + const qint32& threadId = message.threadID(); - // 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 conversationMessageReceived(it->toVariant(), i); - i++; - if (i >= end) { - break; + if (m_known_messages[threadId].contains(message.uID())) { + // This message has already been processed. Don't do anything. + continue; } - } -} -void ConversationsDbusInterface::addMessage(const ConversationMessage &message) -{ - const QString& threadId = QString::number(message.threadID()); + updatedConversationIDs.insert(message.threadID()); - if (m_known_messages[threadId].contains(message.uID())) { - // This message has already been processed. Don't do anything. - return; - } + // Store the Message in the list corresponding to its thread + bool newConversation = !m_conversations.contains(threadId); + const auto& threadPosition = m_conversations[threadId].insert(message.date(), message); + m_known_messages[threadId].insert(message.uID()); - // Store the Message in the list corresponding to its thread - bool newConversation = !m_conversations.contains(threadId); - m_conversations[threadId].insert(message.date(), message); - m_known_messages[threadId].insert(message.uID()); + // If this message was inserted at the end of the list, it is the latest message in the conversation + bool latestMessage = threadPosition == m_conversations[threadId].end() - 1; - // Tell the world about what just happened - if (newConversation) { - Q_EMIT conversationCreated(message.toVariant()); - } else { - Q_EMIT conversationUpdated(message.toVariant()); + // Tell the world about what just happened + if (newConversation) { + Q_EMIT conversationCreated(message.toVariant()); + } else if (latestMessage) { + Q_EMIT conversationUpdated(message.toVariant()); + } } + + waitingForMessagesLock.lock(); + // Remove the waiting flag for all conversations which we just processed + conversationsWaitingForMessages.subtract(updatedConversationIDs); + waitingForMessages.wakeAll(); + waitingForMessagesLock.unlock(); } void ConversationsDbusInterface::removeMessage(const QString& internalId) { // TODO: Delete the specified message from our internal structures } -void ConversationsDbusInterface::replyToConversation(const QString& conversationID, const QString& message) +QList ConversationsDbusInterface::getConversation(const qint32& conversationID) const +{ + return m_conversations.value(conversationID).values(); +} + +void ConversationsDbusInterface::updateConversation(const qint32& conversationID) +{ + waitingForMessagesLock.lock(); + conversationsWaitingForMessages.insert(conversationID); + m_smsInterface.requestConversation(conversationID); + while (conversationsWaitingForMessages.contains(conversationID)) { + waitingForMessages.wait(&waitingForMessagesLock); + } + waitingForMessagesLock.unlock(); +} + +void ConversationsDbusInterface::replyToConversation(const qint32& conversationID, const QString& message) { const auto messagesList = m_conversations[conversationID]; if (messagesList.isEmpty()) { diff --git a/plugins/sms/requestconversationworker.h b/plugins/sms/requestconversationworker.h new file mode 100644 --- /dev/null +++ b/plugins/sms/requestconversationworker.h @@ -0,0 +1,55 @@ +/** + * Copyright 2018 Simon Redman + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License or (at your option) version 3 or any later version + * accepted by the membership of KDE e.V. (or its successor approved + * by the membership of KDE e.V.), which shall act as a proxy + * defined in Section 14 of version 3 of the license. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef REQUESTCONVERSATIONWORKER_H +#define REQUESTCONVERSATIONWORKER_H + +#include "conversationsdbusinterface.h" + +#include + +/** + * In case we need to wait for more messages to be downloaded from Android, + * Do the actual work of a requestConversation call in a separate thread + * + * This class is the worker for that thread + */ +class RequestConversationWorker : public QObject +{ + Q_OBJECT + +public: + RequestConversationWorker(const qint32& conversationID, int start, int end, ConversationsDbusInterface* interface); + +public Q_SLOTS: + void handleRequestConversation(); + +Q_SIGNALS: + void conversationMessageRead(const QVariantMap& msg) const; + void finished(); + +private: + qint32 conversationID; + int start; + int end; + ConversationsDbusInterface* parent; +}; + +#endif // REQUESTCONVERSATIONWORKER_H diff --git a/plugins/sms/requestconversationworker.cpp b/plugins/sms/requestconversationworker.cpp new file mode 100644 --- /dev/null +++ b/plugins/sms/requestconversationworker.cpp @@ -0,0 +1,65 @@ +/** + * Copyright 2018 Simon Redman + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License or (at your option) version 3 or any later version + * accepted by the membership of KDE e.V. (or its successor approved + * by the membership of KDE e.V.), which shall act as a proxy + * defined in Section 14 of version 3 of the license. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "requestconversationworker.h" + +#include "conversationsdbusinterface.h" + +#include + +RequestConversationWorker::RequestConversationWorker(const qint32& conversationID, int start, int end, ConversationsDbusInterface* interface) : + //QObject(interface) + conversationID(conversationID) + , start(start) + , end(end) + , parent(interface) +{ +} + +void RequestConversationWorker::handleRequestConversation() +{ + auto messagesList = parent->getConversation(conversationID); + + if (messagesList.isEmpty()) { + // Since there are no messages in the conversation, it's likely that it is a junk ID, but go ahead anyway + qCWarning(KDECONNECT_CONVERSATIONS) << "Got a conversationID for a conversation with no messages!" << conversationID; + } + + if (messagesList.length() <= end) { + // 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; + } + } + + Q_EMIT finished(); +} diff --git a/plugins/sms/smsplugin.h b/plugins/sms/smsplugin.h --- a/plugins/sms/smsplugin.h +++ b/plugins/sms/smsplugin.h @@ -109,8 +109,10 @@ /** * Send a request to the remote for a particular conversation + * + * TODO: Make interface capable of requesting limited window of messages */ - Q_SCRIPTABLE void requestConversation(const QString& conversationID) const; + Q_SCRIPTABLE void requestConversation(const qint32& conversationID) const; private: @@ -126,7 +128,6 @@ QDBusInterface m_telepathyInterface; ConversationsDbusInterface* m_conversationInterface; - }; #endif diff --git a/plugins/sms/smsplugin.cpp b/plugins/sms/smsplugin.cpp --- a/plugins/sms/smsplugin.cpp +++ b/plugins/sms/smsplugin.cpp @@ -75,10 +75,10 @@ sendPacket(np); } -void SmsPlugin::requestConversation (const QString& conversationID) const +void SmsPlugin::requestConversation (const qint32& conversationID) const { NetworkPacket np(PACKET_TYPE_SMS_REQUEST_CONVERSATION); - np.set("threadID", conversationID.toInt()); + np.set("threadID", conversationID); sendPacket(np); } @@ -99,13 +99,17 @@ bool SmsPlugin::handleBatchMessages(const NetworkPacket& np) { const auto messages = np.get("messages"); + QList messagesList; + messagesList.reserve(messages.count()); for (const QVariant& body : messages) { ConversationMessage message(body.toMap()); forwardToTelepathy(message); - m_conversationInterface->addMessage(message); + messagesList.append(message); } + m_conversationInterface->addMessages(messagesList); + return true; } diff --git a/smsapp/conversationmodel.h b/smsapp/conversationmodel.h --- a/smsapp/conversationmodel.h +++ b/smsapp/conversationmodel.h @@ -24,16 +24,19 @@ #include #include +#include #include "interfaces/dbusinterfaces.h" Q_DECLARE_LOGGING_CATEGORY(KDECONNECT_SMS_CONVERSATION_MODEL) +#define INVALID_THREAD_ID -1 + class ConversationModel : public QStandardItemModel { Q_OBJECT - Q_PROPERTY(QString threadId READ threadId WRITE setThreadId) + Q_PROPERTY(qint32 threadId READ threadId WRITE setThreadId) Q_PROPERTY(QString deviceId READ deviceId WRITE setDeviceId) public: @@ -47,22 +50,24 @@ Q_ENUMS(Roles) - QString threadId() const; - void setThreadId(const QString &threadId); + qint32 threadId() const; + void setThreadId(const qint32& threadId); QString deviceId() const { return m_deviceId; } void setDeviceId(const QString &/*deviceId*/); Q_INVOKABLE void sendReplyToConversation(const QString& message); + Q_INVOKABLE void requestMoreMessages(const quint32& howMany = 10); private Q_SLOTS: void createRowFromMessage(const QVariantMap &msg, int pos); void handleConversationUpdate(const QVariantMap &msg); private: DeviceConversationsDbusInterface* m_conversationsInterface; QString m_deviceId; - QString m_threadId; + qint32 m_threadId = INVALID_THREAD_ID; + QSet knownMessageIDs; }; #endif // CONVERSATIONMODEL_H diff --git a/smsapp/conversationmodel.cpp b/smsapp/conversationmodel.cpp --- a/smsapp/conversationmodel.cpp +++ b/smsapp/conversationmodel.cpp @@ -39,20 +39,20 @@ { } -QString ConversationModel::threadId() const +qint32 ConversationModel::threadId() const { return m_threadId; } -void ConversationModel::setThreadId(const QString &threadId) +void ConversationModel::setThreadId(const qint32& threadId) { if (m_threadId == threadId) return; m_threadId = threadId; clear(); - if (!threadId.isEmpty()) { - m_conversationsInterface->requestConversation(threadId, 0, 10); + if (!(threadId == INVALID_THREAD_ID)) { + requestMoreMessages(); } } @@ -63,15 +63,13 @@ qCDebug(KDECONNECT_SMS_CONVERSATION_MODEL) << "setDeviceId" << "of" << this; if (m_conversationsInterface) { - disconnect(m_conversationsInterface, SIGNAL(conversationMessageReceived(QVariantMap, int)), this, SLOT(createRowFromMessage(QVariantMap, int))); disconnect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdate(QVariantMap))); delete m_conversationsInterface; } m_deviceId = deviceId; m_conversationsInterface = new DeviceConversationsDbusInterface(deviceId, this); - connect(m_conversationsInterface, SIGNAL(conversationMessageReceived(QVariantMap,int)), this, SLOT(createRowFromMessage(QVariantMap,int))); connect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdate(QVariantMap))); } @@ -81,11 +79,20 @@ m_conversationsInterface->replyToConversation(m_threadId, message); } +void ConversationModel::requestMoreMessages(const quint32& howMany) +{ + if (m_threadId == INVALID_THREAD_ID) { + return; + } + const auto& numMessages = rowCount(); + m_conversationsInterface->requestConversation(m_threadId, numMessages, numMessages + howMany); +} + void ConversationModel::createRowFromMessage(const QVariantMap& msg, int pos) { const ConversationMessage message(msg); - if (!(message.threadID() == m_threadId.toInt())) { + if (!(message.threadID() == m_threadId)) { // Because of the asynchronous nature of the current implementation of this model, if the // user clicks quickly between threads or for some other reason a message comes when we're // not expecting it, we should not display it in the wrong place @@ -95,18 +102,26 @@ << "Discarding."; return; } + + if (knownMessageIDs.contains(message.uID())) { + qCDebug(KDECONNECT_SMS_CONVERSATION_MODEL) + << "Ignoring duplicate message with ID" << message.uID(); + return; + } + auto item = new QStandardItem; item->setText(message.body()); item->setData(message.type() == ConversationMessage::MessageTypeSent, FromMeRole); item->setData(message.date(), DateRole); insertRow(pos, item); + knownMessageIDs.insert(message.uID()); } void ConversationModel::handleConversationUpdate(const QVariantMap& msg) { const ConversationMessage message(msg); - if (!(message.threadID() == m_threadId.toInt())) { + if (!(message.threadID() == m_threadId)) { // If a conversation which we are not currently viewing was updated, discard the information qCDebug(KDECONNECT_SMS_CONVERSATION_MODEL) << "Saw update for thread" << message.threadID() diff --git a/smsapp/qml/ConversationDisplay.qml b/smsapp/qml/ConversationDisplay.qml --- a/smsapp/qml/ConversationDisplay.qml +++ b/smsapp/qml/ConversationDisplay.qml @@ -33,12 +33,26 @@ id: person } property QtObject device - property string conversationId + property int conversationId property string phoneNumber title: person.person && person.person.name ? person.person.name : phoneNumber + /** + * Build a chat message which is representative of all chat messages + * + * In other words, one which I can use to get a reasonable height guess + */ + ChatMessage { + id: genericMessage + messageBody: "Generic Message Body" + dateTime: new Date('2000-0-0') + visible: false + enabled: false + } + ListView { + id: viewport model: QSortFilterProxyModel { id: model sortOrder: Qt.AscendingOrder @@ -55,12 +69,46 @@ messageBody: model.display sentByMe: model.fromMe dateTime: new Date(model.date) + + ListView.onAdd: { + if (index == viewport.count - 1) + // This message is being inserted at the newest position + // We want to scroll to show it if the user is "almost" looking at it + + // Define some fudge area. If the message is being drawn offscreen but within + // this distance, we move to show it anyway. + // Selected to be genericMessage.height because that value scales for different + // font sizes / DPI / etc. -- Better ideas are welcome! + // Double the value works nicely + var offscreenFudge = 2 * genericMessage.height + + var viewportYBottom = viewport.contentY + viewport.height + + if (y < viewportYBottom + genericMessage.height) { + viewport.currentIndex = index + } + } } - // Set the view to start at the bottom of the page and track new elements if it was not manually scrolled up - currentIndex: atYEnd ? - count - 1 : - currentIndex + onMovementEnded: { + // Unset the highlightRangeMode if it was set previously + highlightRangeMode = ListView.ApplyRange + highlightMoveDuration: -1 // "Re-enable" the highlight animation + + if (atYBeginning) { + // "Lock" the view to the message currently at the beginning of the view + // This prevents the view from snapping to the top of the messages we are about to request + currentIndex = 0 // Index 0 is the beginning of the view + preferredHighlightBegin = visibleArea.yPosition + preferredHighlightEnd = preferredHighlightBegin + currentItem.height + highlightRangeMode = ListView.StrictlyEnforceRange + + highlightMoveDuration = 1 // This is not ideal: I would like to disable the highlight animation altogether + + // Get more messages + model.sourceModel.requestMoreMessages() + } + } } footer: RowLayout {