From 6f81c676324e90f3d34b5a351c0911b4255bf4ad Mon Sep 17 00:00:00 2001 From: Simon Redman Date: Sun, 27 Oct 2019 21:23:52 +0000 Subject: [PATCH] Refactor contacts-getting code to be either "everything" or "one" Fixes bug from mailing list conversation dated 12 October 2019 --- .../kdeconnect/Helpers/ContactsHelper.java | 165 ++++++++++-------- src/org/kde/kdeconnect/NetworkPacket.java | 9 +- .../ContactsPlugin/ContactsPlugin.java | 56 +++--- 3 files changed, 128 insertions(+), 102 deletions(-) diff --git a/src/org/kde/kdeconnect/Helpers/ContactsHelper.java b/src/org/kde/kdeconnect/Helpers/ContactsHelper.java index 78296ccb..ab61a9e5 100644 --- a/src/org/kde/kdeconnect/Helpers/ContactsHelper.java +++ b/src/org/kde/kdeconnect/Helpers/ContactsHelper.java @@ -33,8 +33,8 @@ import android.util.Base64OutputStream; import android.util.Log; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.RequiresApi; -import androidx.collection.LongSparseArray; import java.io.BufferedReader; import java.io.ByteArrayOutputStream; @@ -52,6 +52,7 @@ import java.util.Set; public class ContactsHelper { + static final String LOG_TAG = "ContactsHelper"; /** * Lookup the name and photoID of a contact given a phone number @@ -103,7 +104,7 @@ public class ContactsHelper { } return encodedPhoto.toString(); } catch (Exception ex) { - Log.e("ContactsHelper", ex.toString()); + Log.e(LOG_TAG, ex.toString()); return ""; } } @@ -140,7 +141,7 @@ public class ContactsHelper { } else { // Something went wrong with this contact // If you are experiencing this, please open a bug report indicating how you got here - Log.e("ContactsHelper", "Got a contact which does not have a LOOKUP_KEY"); + Log.e(LOG_TAG, "Got a contact which does not have a LOOKUP_KEY"); continue; } @@ -211,97 +212,110 @@ public class ContactsHelper { } /** - * Return a mapping of contact IDs to a map of the requested data from the Contacts database - *

- * If for some reason there is no row associated with the contact ID in the database, - * there will not be a corresponding field in the returned map + * Get the last-modified timestamp for every contact in the database * - * @param context android.content.Context running the request - * @param IDs collection of contact uIDs to look up - * @param contactsProjection List of column names to extract, defined in ContactsContract.Contacts + * @param context android.content.Context running the request + * @return Mapping of contact uID to last-modified timestamp + */ + @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2) // Need API 18 for contact timestamps + public static Map getAllContactTimestamps(Context context) { + String[] projection = { uID.COLUMN, ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP }; + + Map> databaseValues = accessContactsDatabase(context, projection, null, null, null); + + Map timestamps = new HashMap<>(); + for (uID contactID : databaseValues.keySet()) { + Map data = databaseValues.get(contactID); + timestamps.put( + contactID, + Long.parseLong(data.get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP)) + ); + } + + return timestamps; + } + + /** + * Get the last-modified timestamp for the specified contact + * + * @param context android.content.Context running the request + * @param contactID Contact uID to read + * @throws ContactNotFoundException If the given ID for some reason does not match a contact + * @return Last-modified timestamp of the contact + */ + @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2) // Need API 18 for contact timestamps + public static Long getContactTimestamp(Context context, uID contactID) throws ContactNotFoundException { + String[] projection = { uID.COLUMN, ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP }; + String selection = uID.COLUMN + " = ?"; + String[] selectionArgs = { contactID.toString() }; + + Map> databaseValue = accessContactsDatabase(context, projection, selection, selectionArgs, null); + + if (databaseValue.size() == 0) { + throw new ContactNotFoundException("Querying for contact with id " + contactID + " returned no results."); + } + + if (databaseValue.size() != 1) { + Log.w(LOG_TAG, "Received an improper number of return values from the database in getContactTimestamp: " + databaseValue.size()); + } + + Long timestamp = Long.parseLong(databaseValue.get(contactID).get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP)); + + return timestamp; + } + + /** + * Return a mapping of contact IDs to a map of the requested data from the Contacts database. + * + * @param context android.content.Context running the request + * @param projection List of column names to extract, defined in ContactsContract.Contacts. Must contain uID.COLUMN + * @param selection Parameterizable filter to use with the ContentResolver query. May be null. + * @param selectionArgs Parameters for selection. May be null. + * @param sortOrder Sort order to request from the ContentResolver query. May be null. * @return mapping of contact uIDs to desired values, which are a mapping of column names to the data contained there */ - @TargetApi(Build.VERSION_CODES.HONEYCOMB) // Needed for Cursor.getType(..) - public static Map> getColumnsFromContactsForIDs(Context context, Collection IDs, String[] contactsProjection) { - HashMap> toReturn = new HashMap<>(); - - if (IDs.isEmpty()) { - return toReturn; - } - + private static Map> accessContactsDatabase( + @NonNull Context context, + @NonNull String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String sortOrder + ) { Uri contactsUri = ContactsContract.Contacts.CONTENT_URI; - // Regardless of whether it was requested, we need to look up the uID column - Set lookupProjection = new HashSet<>(Arrays.asList(contactsProjection)); - lookupProjection.add(uID.COLUMN); - - // We need a selection which looks like " IN(?,?,...?)" with one ? per ID - StringBuilder contactsSelection = new StringBuilder(uID.COLUMN); - contactsSelection.append(" IN("); - - for (int i = 0; i < IDs.size(); i++) { - contactsSelection.append("?,"); - } - // Remove trailing comma - contactsSelection.deleteCharAt(contactsSelection.length() - 1); - contactsSelection.append(")"); - - // We need selection arguments as simply a String representation of each ID - List contactsArgs = new ArrayList<>(); - for (uID ID : IDs) { - contactsArgs.add(ID.toString()); - } + HashMap> toReturn = new HashMap<>(); try (Cursor contactsCursor = context.getContentResolver().query( contactsUri, - lookupProjection.toArray(new String[0]), - contactsSelection.toString(), - contactsArgs.toArray(new String[0]), - null + projection, + selection, + selectionArgs, + sortOrder )) { if (contactsCursor != null && contactsCursor.moveToFirst()) { do { - Map requestedData = new HashMap<>(); + Map requestedData = new HashMap<>(); - int lookupKeyIdx = contactsCursor.getColumnIndexOrThrow(uID.COLUMN); - String lookupKey = contactsCursor.getString(lookupKeyIdx); + int uIDIndex = contactsCursor.getColumnIndexOrThrow(uID.COLUMN); + uID uID = new uID(contactsCursor.getString(uIDIndex)); // For each column, collect the data from that column - for (String column : contactsProjection) { + for (String column : projection) { int index = contactsCursor.getColumnIndex(column); // Since we might be getting various kinds of data, Object is the best we can do - Object data; - int type; + String data; if (index == -1) { // This contact didn't have the requested column? Something is very wrong. // If you are experiencing this, please open a bug report indicating how you got here - Log.e("ContactsHelper", "Got a contact which does not have a requested column"); + Log.e(LOG_TAG, "Got a contact which does not have a requested column"); continue; } - - type = contactsCursor.getType(index); - switch (type) { - case Cursor.FIELD_TYPE_INTEGER: - data = contactsCursor.getInt(index); - break; - case Cursor.FIELD_TYPE_FLOAT: - data = contactsCursor.getFloat(index); - break; - case Cursor.FIELD_TYPE_STRING: - data = contactsCursor.getString(index); - break; - case Cursor.FIELD_TYPE_BLOB: - data = contactsCursor.getBlob(index); - break; - default: - Log.e("ContactsHelper", "Got an undefined type of column " + column); - continue; - } + data = contactsCursor.getString(index); requestedData.put(column, data); } - toReturn.put(new uID(lookupKey), requestedData); + toReturn.put(uID, requestedData); } while (contactsCursor.moveToNext()); } } @@ -391,4 +405,17 @@ public class ContactsHelper { return contactLookupKey.equals(other); } } + + /** + * Exception to indicate that a specified contact was not found + */ + public static class ContactNotFoundException extends Exception { + public ContactNotFoundException(uID contactID) { + super("Unable to find contact with ID " + contactID); + } + + public ContactNotFoundException(String message) { + super(message); + } + } } diff --git a/src/org/kde/kdeconnect/NetworkPacket.java b/src/org/kde/kdeconnect/NetworkPacket.java index 531a9b0c..3e628698 100644 --- a/src/org/kde/kdeconnect/NetworkPacket.java +++ b/src/org/kde/kdeconnect/NetworkPacket.java @@ -108,6 +108,13 @@ public class NetworkPacket { return mBody.optInt(key, defaultValue); } + public void set(String key, int value) { + try { + mBody.put(key, value); + } catch (Exception ignored) { + } + } + public long getLong(String key) { return mBody.optLong(key, -1); } @@ -116,7 +123,7 @@ public class NetworkPacket { return mBody.optLong(key, defaultValue); } - public void set(String key, int value) { + public void set(String key, long value) { try { mBody.put(key, value); } catch (Exception ignored) { diff --git a/src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java b/src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java index 265658a1..5e053d8b 100644 --- a/src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java +++ b/src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java @@ -26,12 +26,12 @@ package org.kde.kdeconnect.Plugins.ContactsPlugin; import android.Manifest; import android.annotation.TargetApi; import android.os.Build; -import android.provider.ContactsContract; import android.util.Log; import org.kde.kdeconnect.Helpers.ContactsHelper; import org.kde.kdeconnect.Helpers.ContactsHelper.VCardBuilder; import org.kde.kdeconnect.Helpers.ContactsHelper.uID; +import org.kde.kdeconnect.Helpers.ContactsHelper.ContactNotFoundException; import org.kde.kdeconnect.NetworkPacket; import org.kde.kdeconnect.Plugins.Plugin; import org.kde.kdeconnect.Plugins.PluginFactory; @@ -141,9 +141,10 @@ public class ContactsPlugin extends Plugin { * * @param vcard vcard to apply metadata to * @param uID uID to which the vcard corresponds + * @throws ContactNotFoundException If the given ID for some reason does not match a contact * @return The same VCard as was passed in, but now with KDE Connect-specific fields */ - private VCardBuilder addVCardMetadata(VCardBuilder vcard, uID uID) { + private VCardBuilder addVCardMetadata(VCardBuilder vcard, uID uID) throws ContactNotFoundException { // Append the device ID line // Unclear if the deviceID forms a valid name per the vcard spec. Worry about that later.. vcard.appendLine("X-KDECONNECT-ID-DEV-" + device.getDeviceId(), @@ -151,16 +152,9 @@ public class ContactsPlugin extends Plugin { // Build the timestamp line // Maybe one day this should be changed into the vcard-standard REV key - List uIDs = new ArrayList<>(); - uIDs.add(uID); - - final String[] contactsProjection = new String[]{ - ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP - }; - - Map> timestamp = ContactsHelper.getColumnsFromContactsForIDs(context, uIDs, contactsProjection); + Long timestamp = ContactsHelper.getContactTimestamp(context, uID); vcard.appendLine("X-KDECONNECT-TIMESTAMP", - timestamp.get(uID).get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP).toString()); + timestamp.toString()); return vcard; } @@ -178,26 +172,19 @@ public class ContactsPlugin extends Plugin { private boolean handleRequestAllUIDsTimestamps(@SuppressWarnings("unused") NetworkPacket np) { NetworkPacket reply = new NetworkPacket(PACKET_TYPE_CONTACTS_RESPONSE_UIDS_TIMESTAMPS); - List uIDs = ContactsHelper.getAllContactContactIDs(context); + Map uIDsToTimestamps = ContactsHelper.getAllContactTimestamps(context); - List uIDsAsStrings = new ArrayList<>(uIDs.size()); + int contactCount = uIDsToTimestamps.size(); - for (uID uID : uIDs) { - uIDsAsStrings.add(uID.toString()); + List uIDs = new ArrayList<>(contactCount); + + for (uID contactID : uIDsToTimestamps.keySet()) { + Long timestamp = uIDsToTimestamps.get(contactID); + reply.set(contactID.toString(), timestamp); + uIDs.add(contactID.toString()); } - final String[] contactsProjection = new String[]{ - ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP - }; - - reply.set("uids", uIDsAsStrings); - - // Add last-modified timestamps - Map> uIDsToTimestamps = ContactsHelper.getColumnsFromContactsForIDs(context, uIDs, contactsProjection); - for (uID ID : uIDsToTimestamps.keySet()) { - Map data = uIDsToTimestamps.get(ID); - reply.set(ID.toString(), (Integer) data.get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP)); - } + reply.set("uids", uIDs); device.sendPacket(reply); @@ -230,12 +217,17 @@ public class ContactsPlugin extends Plugin { for (uID uID : uIDsToVCards.keySet()) { VCardBuilder vcard = uIDsToVCards.get(uID); - vcard = this.addVCardMetadata(vcard, uID); + try { + vcard = this.addVCardMetadata(vcard, uID); - // Store this as a valid uID - uIDsAsStrings.add(uID.toString()); - // Add the uid -> vcard pairing to the packet - reply.set(uID.toString(), vcard.toString()); + // Store this as a valid uID + uIDsAsStrings.add(uID.toString()); + // Add the uid -> vcard pairing to the packet + reply.set(uID.toString(), vcard.toString()); + + } catch (ContactsHelper.ContactNotFoundException e) { + e.printStackTrace(); + } } // Add the valid uIDs to the packet