diff --git a/plugins/sms/conversationsdbusinterface.h b/plugins/sms/conversationsdbusinterface.h --- a/plugins/sms/conversationsdbusinterface.h +++ b/plugins/sms/conversationsdbusinterface.h @@ -61,7 +61,15 @@ */ QVariantList activeConversations(); - void requestConversation(const QString &conversationID, int start, int end); + /** + * Request the specified range of the specified conversation + * + * If the conversation does not have enough messages to fill the request, + * this method may return fewer messages + * + * Return value is a mapping of the message index to the message + */ + QVariantMap requestConversation(const QString &conversationID, int start, int end); /** * Send a new message to this conversation @@ -77,7 +85,6 @@ Q_SCRIPTABLE void conversationCreated(const QVariantMap& msg); Q_SCRIPTABLE void conversationRemoved(const QString& threadID); 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 diff --git a/plugins/sms/conversationsdbusinterface.cpp b/plugins/sms/conversationsdbusinterface.cpp --- a/plugins/sms/conversationsdbusinterface.cpp +++ b/plugins/sms/conversationsdbusinterface.cpp @@ -64,30 +64,37 @@ return toReturn; } -void ConversationsDbusInterface::requestConversation(const QString& conversationID, int start, int end) +QVariantMap ConversationsDbusInterface::requestConversation(const QString& conversationID, int start, int end) { - const auto messagesList = m_conversations[conversationID].values(); + QVariantMap toReturn; + auto messagesList = m_conversations[conversationID].values(); 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; } - // TODO: Check local cache before requesting new messages - // TODO: Make Android interface capable of requesting small window of messages - m_smsInterface.requestConversation(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 + m_smsInterface.requestConversation(conversationID); + messagesList = m_conversations[conversationID].values(); + } // 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); + toReturn.insert(QString(i), it->toVariant()); + //Q_EMIT conversationMessageReceived(it->toVariant(), i); i++; if (i >= end) { break; } } + + return toReturn; } void ConversationsDbusInterface::addMessage(const ConversationMessage &message) 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,13 @@ /** * Send a request to the remote for a particular conversation + * + * Blocks until the remote returns */ - Q_SCRIPTABLE void requestConversation(const QString& conversationID) const; + Q_SCRIPTABLE void requestConversation(const QString& conversationID); + +Q_SIGNALS: + Q_SCRIPTABLE void messagesPacketHandled(); private: @@ -124,9 +129,17 @@ */ bool handleBatchMessages(const NetworkPacket& np); + /** + * Keep track of one flag per thread + * + * If a request is made for the contents of a thread, set the flag to true + * + * When a message is delivered for a thread, the flag should be set to false + */ + QMap m_threadWaitingRequests; + 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 @@ -51,7 +51,9 @@ bool SmsPlugin::receivePacket(const NetworkPacket& np) { if (np.type() == PACKET_TYPE_SMS_MESSAGES) { - return handleBatchMessages(np); + bool success = handleBatchMessages(np); + Q_EMIT messagesPacketHandled(); + return success; } return true; @@ -75,12 +77,30 @@ sendPacket(np); } -void SmsPlugin::requestConversation (const QString& conversationID) const +void SmsPlugin::requestConversation (const QString& conversationID) { + qint32 threadID = conversationID.toInt(); NetworkPacket np(PACKET_TYPE_SMS_REQUEST_CONVERSATION); - np.set("threadID", conversationID.toInt()); + np.set("threadID", threadID); + + m_threadWaitingRequests[conversationID.toInt()] = true; sendPacket(np); + + // Yield the thread while we wait for the reply + QEventLoop loop; + connect(this, &SmsPlugin::messagesPacketHandled, &loop, &QEventLoop::quit); + loop.exec(); + + if (m_threadWaitingRequests[conversationID.toInt()]) { + // If the remote didn't return messages for this thread (end of conversation?) don't just block forever + // This could also happen in case some messages arrived but simply the ones we want haven't (yet). + // So yes, a potential race condition bug, but very unlikely given that messages arrive in response + // to human requests + qCDebug(KDECONNECT_PLUGIN_SMS) << Q_FUNC_INFO << "Event queue cleared and messages handled, but we never saw messages for the requested conversation"; + } + + return; // New messages have been processed. Return to the caller. } void SmsPlugin::forwardToTelepathy(const ConversationMessage& message) @@ -104,6 +124,7 @@ ConversationMessage message(body.toMap()); forwardToTelepathy(message); m_conversationInterface->addMessage(message); + m_threadWaitingRequests[message.threadID()] = false; } return true; diff --git a/smsapp/conversationmodel.h b/smsapp/conversationmodel.h --- a/smsapp/conversationmodel.h +++ b/smsapp/conversationmodel.h @@ -54,6 +54,7 @@ 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); diff --git a/smsapp/conversationmodel.cpp b/smsapp/conversationmodel.cpp --- a/smsapp/conversationmodel.cpp +++ b/smsapp/conversationmodel.cpp @@ -51,7 +51,7 @@ m_threadId = threadId; clear(); if (!threadId.isEmpty()) { - m_conversationsInterface->requestConversation(threadId, 0, 10); + requestMoreMessages(); } } @@ -70,7 +70,7 @@ 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(conversationMessageReceived(QVariantMap, int)), this, SLOT(createRowFromMessage(QVariantMap, int))); connect(m_conversationsInterface, SIGNAL(conversationUpdated(QVariantMap)), this, SLOT(handleConversationUpdate(QVariantMap))); } @@ -80,6 +80,25 @@ m_conversationsInterface->replyToConversation(m_threadId, message); } +void ConversationModel::requestMoreMessages(const quint32& howMany) +{ + if (m_threadId.isEmpty()) { + return; + } + const auto& numMessages = rowCount(); + const auto& messagesReply = m_conversationsInterface->requestConversation(m_threadId, numMessages, numMessages + howMany); + + setWhenAvailable(messagesReply, [this](const QVariantMap& messages) { + for (auto it = messages.cbegin(); it != messages.cend(); ++it) { + int pos = it.key().toInt(); + QDBusArgument data = it->value(); + QVariantMap message; + data >> message; + createRowFromMessage(message, pos); + } + }, this); +} + void ConversationModel::createRowFromMessage(const QVariantMap& msg, int pos) { const ConversationMessage message(msg);