diff --git a/src/org/kde/kdeconnect/Helpers/SMSHelper.java b/src/org/kde/kdeconnect/Helpers/SMSHelper.java index d7c3ac08..a5f8be2d 100644 --- a/src/org/kde/kdeconnect/Helpers/SMSHelper.java +++ b/src/org/kde/kdeconnect/Helpers/SMSHelper.java @@ -22,6 +22,7 @@ import android.os.Looper; import android.provider.Telephony; import android.telephony.PhoneNumberUtils; import android.util.Log; +import android.util.Pair; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -51,6 +52,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -59,6 +61,7 @@ import java.util.TreeMap; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import kotlin.text.Charsets; @@ -418,60 +421,120 @@ public class SMSHelper { } /** - * Get the last message from each conversation. Can use those thread_ids to look up more - * messages in those conversations + * Get the last message from each conversation. Can use the thread_ids in those messages to look + * up more messages in those conversations + * + * Returns values ordered from most-recently-touched conversation to oldest, if possible. + * Otherwise ordering is undefined. * * @param context android.content.Context running the request - * @return Mapping of thread_id to the first message in each thread + * @return Non-blocking iterable of the first message in each conversation */ - public static Map getConversations( + public static Iterable getConversations( @NonNull Context context ) { Uri uri = SMSHelper.getConversationUri(); + // Used to avoid spewing logs in case there is an overall problem with fetching thread IDs + boolean warnedForNullThreadIDs = false; + + // Used to avoid spewing logs in case the date column doesn't return anything. + boolean warnedForUnorderedOutputs = false; + // Step 1: Populate the list of all known threadIDs - HashSet threadIds = new HashSet<>(); + // This is basically instantaneous even with lots of conversations because we only make one + // query. If someone wanted to squeeze better UI performance out of this method, they could + // iterate over the threadIdCursor instead of getting all the threads before beginning to + // return conversations, but I doubt anyone will ever find it necessary. + List threadIds; try (Cursor threadIdCursor = context.getContentResolver().query( uri, null, null, null, null)) { + List> threadTimestampPair = new ArrayList<>(); while (threadIdCursor != null && threadIdCursor.moveToNext()) { // The "_id" column returned from the `content://sms-mms/conversations?simple=true` URI // is actually what the rest of the world calls a thread_id. // In my limited experimentation, the other columns are not populated, so don't bother // looking at them here. int idColumn = threadIdCursor.getColumnIndex("_id"); + int dateColumn = threadIdCursor.getColumnIndex("date"); + + ThreadID threadID = null; + long messageDate = -1; if (!threadIdCursor.isNull(idColumn)) { - threadIds.add(threadIdCursor.getLong(idColumn)); + threadID = new ThreadID(threadIdCursor.getLong(idColumn)); } + if (!threadIdCursor.isNull(dateColumn)) { + // I think the presence of the "date" column depends on the specifics of the + // device. If it's there, we'll use it to return threads in a sorted order. + // If it's not there, we'll return them unsorted (maybe you get lucky and the + // conversations URI returns sorted anyway). + messageDate = threadIdCursor.getLong(dateColumn); + } + + if (messageDate <= 0) { + if (!warnedForUnorderedOutputs) { + Log.w("SMSHelper", "Got no value for date of thread. Return order of results is undefined."); + warnedForUnorderedOutputs = true; + } + } + + if (threadID == null) { + if (!warnedForNullThreadIDs) { + Log.w("SMSHelper", "Got null for some thread IDs. If these were valid threads, they will not be returned."); + warnedForNullThreadIDs = true; + } + continue; + } + + threadTimestampPair.add(new Pair(threadID, messageDate)); } + + threadIds = threadTimestampPair.stream() + .sorted((left, right) -> right.second.compareTo(left.second)) // Sort most-recent to least-recent (largest to smallest) + .map(threadTimestampPairElement -> threadTimestampPairElement.first).collect(Collectors.toList()); } // Step 2: Get the actual message object from each thread ID - Map firstMessageByThread = new HashMap<>(); + // Do this in an iterator, so that the caller can choose to interrupt us as frequently as + // desired + return new Iterable() { + @NonNull + @Override + public Iterator iterator() { + return new Iterator() { + int threadIdsIndex = 0; - for (Long rawThreadId : threadIds) { - ThreadID threadId = new ThreadID(rawThreadId); + @Override + public boolean hasNext() { + return threadIdsIndex < threadIds.size(); + } - List firstMessage = getMessagesInThread(context, threadId, 1L); + @Override + public Message next() { + ThreadID nextThreadId = threadIds.get(threadIdsIndex); + threadIdsIndex++; - if (firstMessage.size() > 1) { - Log.w("SMSHelper", "getConversations got two messages for the same ThreadID: " + threadId); + List firstMessage = getMessagesInThread(context, nextThreadId, 1L); + + if (firstMessage.size() > 1) { + Log.w("SMSHelper", "getConversations got two messages for the same ThreadID: " + nextThreadId); + } + + if (firstMessage.size() == 0) + { + Log.e("SMSHelper", "ThreadID: " + nextThreadId + " did not return any messages"); + // This is a strange issue, but I don't know how to say what is wrong, so just continue along + return this.next(); + } + return firstMessage.get(0); + } + }; } - - if (firstMessage.size() == 0) - { - Log.e("SMSHelper", "ThreadID: " + threadId + " did not return any messages"); - // This is a strange issue, but I don't know how to say what is wrong, so just continue along - continue; - } - - firstMessageByThread.put(threadId, firstMessage.get(0)); - } - - return firstMessageByThread; + }; } private static int addEventFlag( diff --git a/src/org/kde/kdeconnect/Plugins/SMSPlugin/SMSPlugin.java b/src/org/kde/kdeconnect/Plugins/SMSPlugin/SMSPlugin.java index 9958f574..4422e037 100644 --- a/src/org/kde/kdeconnect/Plugins/SMSPlugin/SMSPlugin.java +++ b/src/org/kde/kdeconnect/Plugins/SMSPlugin/SMSPlugin.java @@ -40,7 +40,7 @@ import org.kde.kdeconnect_tp.BuildConfig; import org.kde.kdeconnect_tp.R; import java.util.ArrayList; -import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.locks.Lock; @@ -50,7 +50,6 @@ import androidx.core.content.ContextCompat; import com.klinker.android.send_message.ApnUtils; import com.klinker.android.send_message.Transaction; -import com.klinker.android.send_message.Utils; import com.klinker.android.logger.Log; import static org.kde.kdeconnect.Plugins.TelephonyPlugin.TelephonyPlugin.PACKET_TYPE_TELEPHONY; @@ -462,7 +461,7 @@ public class SMSPlugin extends Plugin { * @param messages Messages to include in the packet * @return NetworkPacket of type PACKET_TYPE_SMS_MESSAGE */ - private static NetworkPacket constructBulkMessagePacket(Collection messages) { + private static NetworkPacket constructBulkMessagePacket(Iterable messages) { NetworkPacket reply = new NetworkPacket(PACKET_TYPE_SMS_MESSAGE); JSONArray body = new JSONArray(); @@ -489,22 +488,20 @@ public class SMSPlugin extends Plugin { * Send one packet of type PACKET_TYPE_SMS_MESSAGE with the first message in all conversations */ private boolean handleRequestAllConversations(NetworkPacket packet) { - Map conversations = SMSHelper.getConversations(this.context); + Iterable conversations = SMSHelper.getConversations(this.context); // Prepare the mostRecentTimestamp counter based on these messages, since they are the most // recent in every conversation mostRecentTimestampLock.lock(); - for (SMSHelper.Message message : conversations.values()) { + for (SMSHelper.Message message : conversations) { if (message.date > mostRecentTimestamp) { mostRecentTimestamp = message.date; } + NetworkPacket partialReply = constructBulkMessagePacket(Collections.singleton(message)); + device.sendPacket(partialReply); } mostRecentTimestampLock.unlock(); - NetworkPacket reply = constructBulkMessagePacket(conversations.values()); - - device.sendPacket(reply); - return true; }