[SMS App] Use interface for querying a range of messages

## Summary
This patch adds an interface to return only a specified window of messages, making loading the conversations history smooth, fast, and enjoyable.

The current implementation of the conversation interface loads all messages every time the conversation is requested. This is might be painfully slow to load in case the conversation is large or if there are a lot of MMS/RCS messages in the conversation (since those are wildly slower to load than SMS)

Requires https://invent.kde.org/kde/kdeconnect-android/merge_requests/122 to enable Android functionality

## Test Plan

 - With new Android app and old Desktop app:
   - The Android app will notice the missing fields and query for all messages as before.
 - With old Android app and new Desktop app:
   - The desktop will send fields for the new interface which will not be read and all messages will be returned.
 - With new Android app and new Desktop app:
   - The new interface is used and returns only a certain number of messages at a time.
This commit is contained in:
Simon Redman 2020-11-02 16:40:58 +00:00
parent 9930319158
commit 9ab80593e8
5 changed files with 48 additions and 12 deletions

View file

@ -169,7 +169,20 @@ void ConversationsDbusInterface::updateConversation(const qint64& conversationID
} }
qCDebug(KDECONNECT_CONVERSATIONS) << "Requesting conversation with ID" << conversationID << "from remote"; qCDebug(KDECONNECT_CONVERSATIONS) << "Requesting conversation with ID" << conversationID << "from remote";
conversationsWaitingForMessages.insert(conversationID); conversationsWaitingForMessages.insert(conversationID);
m_smsInterface.requestConversation(conversationID);
// Request a window of messages
qint64 rangeStartTimestamp;
qint64 numberToRequest;
if (m_conversations.contains(conversationID) && m_conversations[conversationID].count() > 0) {
rangeStartTimestamp = m_conversations[conversationID].first().date(); // Request starting from the oldest-available message
numberToRequest = m_conversations[conversationID].count(); // Request an increasing number of messages by requesting more equal to the amount we have
} else {
rangeStartTimestamp = -1; // Value < 0 indicates to return the newest messages
numberToRequest = MIN_NUMBER_TO_REQUEST; // Start off with a small batch
}
if (numberToRequest < MIN_NUMBER_TO_REQUEST) { numberToRequest = MIN_NUMBER_TO_REQUEST; }
m_smsInterface.requestConversation(conversationID, rangeStartTimestamp, numberToRequest);
while (conversationsWaitingForMessages.contains(conversationID)) { while (conversationsWaitingForMessages.contains(conversationID)) {
waitingForMessages.wait(&waitingForMessagesLock); waitingForMessages.wait(&waitingForMessagesLock);
} }

View file

@ -25,6 +25,11 @@ class Device;
Q_DECLARE_LOGGING_CATEGORY(KDECONNECT_CONVERSATIONS) Q_DECLARE_LOGGING_CATEGORY(KDECONNECT_CONVERSATIONS)
// There is some amount of overhead and delay to making a request, so make sure to request at least a few
#define MIN_NUMBER_TO_REQUEST 25
// Some low-water mark after which we want to fill the cache
#define CACHE_LOW_WATER_MARK_PERCENT 10
class ConversationsDbusInterface class ConversationsDbusInterface
: public QDBusAbstractAdaptor : public QDBusAbstractAdaptor
{ {
@ -44,8 +49,8 @@ public:
QList<ConversationMessage> getConversation(const qint64& conversationID) const; QList<ConversationMessage> getConversation(const qint64& conversationID) const;
/** /**
* Get all of the messages in the requested conversation from the remote device * Get some new messages for the requested conversation from the remote device
* TODO: Make interface capable of requesting smaller window of messages * Requests a quantity of new messages equal to the current number of messages in the conversation
*/ */
void updateConversation(const qint64& conversationID); void updateConversation(const qint64& conversationID);

View file

@ -44,13 +44,24 @@ void RequestConversationWorker::handleRequestConversation()
size_t numHandled = replyForConversation(messagesList, start, howMany); size_t numHandled = replyForConversation(messagesList, start, howMany);
if (numHandled < howMany) { if (numHandled < howMany) {
// In this case, the cache wasn't able to satisfy the request fully. Get more.
size_t numRemaining = howMany - numHandled; 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); parent->updateConversation(conversationID);
messagesList = parent->getConversation(conversationID); messagesList = parent->getConversation(conversationID);
//ConversationsDbusInterface::getConversation blocks until it sees new messages in the requested conversation //ConversationsDbusInterface::updateConversation blocks until it sees new messages in the requested conversation
replyForConversation(messagesList, start + numHandled, numRemaining); replyForConversation(messagesList, start + numHandled, numRemaining);
} else {
// The cache was able to fully satisfy the request but we need to check that it isn't running dry
size_t numCachedMessages = messagesList.count();
size_t requestEnd = start + numHandled;
size_t numRemainingMessages = numCachedMessages - requestEnd;
double percentRemaining = ((double) numRemainingMessages / numCachedMessages) * 100;
if (percentRemaining < CACHE_LOW_WATER_MARK_PERCENT || numRemainingMessages < MIN_NUMBER_TO_REQUEST) {
parent->updateConversation(conversationID);
}
} }
Q_EMIT finished(); Q_EMIT finished();

View file

@ -102,10 +102,12 @@ void SmsPlugin::requestAllConversations()
sendPacket(np); sendPacket(np);
} }
void SmsPlugin::requestConversation (const qint64& conversationID) const void SmsPlugin::requestConversation (const qint64 conversationID, const qint64 rangeStartTimestamp, const qint64 numberToRequest) const
{ {
NetworkPacket np(PACKET_TYPE_SMS_REQUEST_CONVERSATION); NetworkPacket np(PACKET_TYPE_SMS_REQUEST_CONVERSATION);
np.set(QStringLiteral("threadID"), conversationID); np.set(QStringLiteral("threadID"), conversationID);
np.set(QStringLiteral("rangeStartTimestamp"), rangeStartTimestamp);
np.set(QStringLiteral("numberToRequest"), numberToRequest);
sendPacket(np); sendPacket(np);
} }

View file

@ -105,9 +105,12 @@
/** /**
* Packet sent to request all the messages in a particular conversation * Packet sent to request all the messages in a particular conversation
* *
* The body should contain the key "threadID" mapping to the threadID being requested * The following fields are available:
* For example: * "threadID": <long> // (Required) ThreadID to request
* { "threadID": 203 } * "rangeStartTimestamp": <long> // (Optional) Millisecond epoch timestamp indicating the start of the range from which to return messages
* "numberToRequest": <long> // (Optional) Number of messages to return, starting from rangeStartTimestamp.
* // May return fewer than expected if there are not enough or more than expected if many
* // messages have the same timestamp.
*/ */
#define PACKET_TYPE_SMS_REQUEST_CONVERSATION QStringLiteral("kdeconnect.sms.request_conversation") #define PACKET_TYPE_SMS_REQUEST_CONVERSATION QStringLiteral("kdeconnect.sms.request_conversation")
@ -161,9 +164,11 @@ public Q_SLOTS:
/** /**
* Send a request to the remote for a particular conversation * Send a request to the remote for a particular conversation
* *
* TODO: Make interface capable of requesting limited window of messages * @param conversationID The conversation to query
* @param rangeStartTimestamp Return messages with timestamp >= this value. Value <= 0 indicates no limit.
* @param numberToRequest Request this many messages. May return more, may return less. Value <= 0 indicates no limit.
*/ */
Q_SCRIPTABLE void requestConversation(const qint64& conversationID) const; Q_SCRIPTABLE void requestConversation(const qint64 conversationID, const qint64 rangeStartTimestamp = -1, const qint64 numberToRequest = -1) const;
Q_SCRIPTABLE void launchApp(); Q_SCRIPTABLE void launchApp();