Differential D9690 Diff 35815 src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java

Changeset View

Standalone View

View Options

src/org/kde/kdeconnect/Plugins/ContactsPlugin/ContactsPlugin.java

  • This file was added.
1​/*
2​ * ContactsPlugin.java - This file is part of KDE Connect's Android App
3​ * Implement a way to request and send contact information
4​ *
5​ * Copyright 2018 Simon Redman <simon@ergotech.com>
6​ *
7​ * This program is free software; you can redistribute it and/or
8​ * modify it under the terms of the GNU General Public License as
9​ * published by the Free Software Foundation; either version 2 of
10​ * the License or (at your option) version 3 or any later version
11​ * accepted by the membership of KDE e.V. (or its successor approved
12​ * by the membership of KDE e.V.), which shall act as a proxy
13​ * defined in Section 14 of version 3 of the license.
14​ *
15​ * This program is distributed in the hope that it will be useful,
16​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
17​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18​ * GNU General Public License for more details.
19​ *
20​ * You should have received a copy of the GNU General Public License
21​ * along with this program. If not, see <http://www.gnu.org/licenses/>.
22​*/
23​
24​package org.kde.kdeconnect.Plugins.ContactsPlugin;
25​
26​import android.Manifest;
27​import android.annotation.TargetApi;
28​import android.os.Build;
29​import android.provider.ContactsContract;
30​import android.util.Log;
31​
32​import org.kde.kdeconnect.Helpers.ContactsHelper;
33​import org.kde.kdeconnect.Helpers.ContactsHelper.VCardBuilder;
34​import org.kde.kdeconnect.Helpers.ContactsHelper.uID;
35​import org.kde.kdeconnect.NetworkPacket;
36​import org.kde.kdeconnect.Plugins.Plugin;
37​import org.kde.kdeconnect_tp.R;
38​
39​import java.util.ArrayList;
40​import java.util.HashSet;
41​import java.util.List;
42​import java.util.Map;
43​import java.util.Set;
44​
45​@TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)
46​public class ContactsPlugin extends Plugin {
47​
48​ /**
49​ * Used to request the device send the unique ID of every contact
50​ */
51​ public static final String PACKET_TYPE_CONTACTS_REQUEST_ALL_UIDS_TIMESTAMPS = "kdeconnect.contacts.request_all_uids_timestamps";
52​
53​ /**
54​ * Used to request the names for the contacts corresponding to a list of UIDs
55​ * <p>
56​ * It shall contain the key "uids", which will have a list of uIDs (long int, as string)
57​ */
58​ public static final String PACKET_TYPE_CONTACTS_REQUEST_VCARDS_BY_UIDS = "kdeconnect.contacts.request_vcards_by_uid";
59​
60​ /**
61​ * Response indicating the packet contains a list of contact uIDs
62​ * <p>
63​ * It shall contain the key "uids", which will mark a list of uIDs (long int, as string)
64​ * The returned IDs can be used in future requests for more information about the contact
65​ */
66​ public static final String PACKET_TYPE_CONTACTS_RESPONSE_UIDS_TIMESTAMPS = "kdeconnect.contacts.response_uids_timestamps";
67​
68​ /**
69​ * Response indicating the packet contains a list of contact names
70​ * <p>
71​ * It shall contain the key "uids", which will mark a list of uIDs (long int, as string)
72​ * then, for each UID, there shall be a field with the key of that UID and the value of the name of the contact
73​ * <p>
74​ * For example:
75​ * ( 'uids' : ['1', '3', '15'],
76​ * '1' : 'John Smith',
77​ * '3' : 'Abe Lincoln',
78​ * '15' : 'Mom' )
79​ */
80​ public static final String PACKET_TYPE_CONTACTS_RESPONSE_VCARDS = "kdeconnect.contacts.response_vcards";
81​
82​ @Override
83​ public String getDisplayName() {
84​ return context.getResources().getString(R.string.pref_plugin_contacts);
85​ }
86​
87​ @Override
88​ public String getDescription() {
89​ return context.getResources().getString(R.string.pref_plugin_contacts_desc);
90​ }
91​
92​ @Override
93​ public String[] getSupportedPacketTypes() {
94​ return new String[]{
95​ PACKET_TYPE_CONTACTS_REQUEST_ALL_UIDS_TIMESTAMPS,
96​ PACKET_TYPE_CONTACTS_REQUEST_VCARDS_BY_UIDS
97​ };
98​ }
99​
100​ @Override
101​ public String[] getOutgoingPacketTypes() {
102​ return new String[]{
103​ PACKET_TYPE_CONTACTS_RESPONSE_UIDS_TIMESTAMPS,
104​ PACKET_TYPE_CONTACTS_RESPONSE_VCARDS
105​ };
106​ }
107​
mtijinkUnsubmitted
Done

Two things: wouldn't it make more sense to keep the structure of the Android contacts (i.e. multiple numbers for one contact, so only send the name once)?

And the index is never used (and does not uniquely correspond to a single contact), so you don't need to send it.

mtijink: Two things: wouldn't it make more sense to keep the structure of the Android contacts (i.e.
sredmanAuthorUnsubmitted
Done

I did it this way because I couldn't figure out how to use NetworkPackage to just send a pile of things and get them out on the other side, so I assign them unique (sequential) integers here, and the pull them out using the same scheme on the other side...

However, I just had a much better idea. One problem I haven't been able to solve is how to distinguish between two contacts with the same name, but Android has already solved this problem by having unique keys assigned to every contact. So my new thought is to ship the list of IDs, then have each contact in the package stored with that ID as the key. I will have a new version of this plugin up soon!

sredman: I did it this way because I couldn't figure out how to use NetworkPackage to just send a pile…
108​ @Override
109​ public boolean onCreate() {
110​ permissionExplanation = R.string.contacts_permission_explanation;
111​
mtijinkUnsubmitted
Done

You can use the R.string.[...] constant directly here.

mtijink: You can use the `R.string.[...]` constant directly here.
112​ return true;
113​ }
114​
115​ @Override
116​ public boolean isEnabledByDefault() {
117​ return true;
118​ }
mtijinkUnsubmitted
Done

Comment does not correspond with the code.

mtijink: Comment does not correspond with the code.
119​
120​ @Override
121​ public String[] getRequiredPermissions() {
122​ return new String[]{Manifest.permission.READ_CONTACTS};
123​ // One day maybe we will also support WRITE_CONTACTS, but not yet
124​ }
125​
126​ @Override
127​ public int getMinSdk() {
128​ // Need API 18 for contact timestamps
129​ return Build.VERSION_CODES.JELLY_BEAN_MR2;
130​ }
131​
132​ /**
133​ * Add custom fields to the vcard to keep track of KDE Connect-specific fields
134​ * <p>
135​ * These include the local device's uID as well as last-changed timestamp
136​ * <p>
137​ * This might be extended in the future to include more fields
138​ *
139​ * @param vcard vcard to apply metadata to
140​ * @param uID uID to which the vcard corresponds
141​ * @return The same VCard as was passed in, but now with KDE Connect-specific fields
mtijinkUnsubmitted
Done

Can you separate this into a new function? Then we can reuse this stuff no matter how we query the contacts database (e.g. searching, single contacts, etc.) in the future.

Also, some more comments what's happening here would be useful: not everyone has experience with ContentProvider code.

mtijink: Can you separate this into a new function? Then we can reuse this stuff no matter how we query…
142​ */
143​ protected VCardBuilder addVCardMetadata(VCardBuilder vcard, uID uID) {
144​ // Append the device ID line
145​ // Unclear if the deviceID forms a valid name per the vcard spec. Worry about that later..
146​ vcard.appendLine("X-KDECONNECT-ID-DEV-" + device.getDeviceId(),
147​ uID.toString());
148​
149​ // Build the timestamp line
150​ // Maybe one day this should be changed into the vcard-standard REV key
151​ List<uID> uIDs = new ArrayList<>();
152​ uIDs.add(uID);
153​
154​ final String[] contactsProjection = new String[]{
155​ ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP
156​ };
157​
mtijinkUnsubmitted
Done

These comments do not apply to the lines immediately below? Maybe it should be moved a few lines down.

mtijink: These comments do not apply to the lines immediately below? Maybe it should be moved a few…
sredmanAuthorUnsubmitted
Done

They do directly apply in the sense that getting the timestamp is a many-step operation... Do you think it would make more sense to move the comment further down?

sredman: They do directly apply in the sense that getting the timestamp is a many-step operation... Do…
158​ Map<uID, Map<String, Object>> timestamp = ContactsHelper.getColumnsFromContactsForIDs(context, uIDs, contactsProjection);
159​ vcard.appendLine("X-KDECONNECT-TIMESTAMP",
160​ timestamp.get(uID).get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP).toString());
161​
162​ return vcard;
163​ }
164​
165​ /**
166​ * Return a unique identifier (Contacts.LOOKUP_KEY) for all contacts in the Contacts database
167​ * <p>
168​ * The identifiers returned can be used in future requests to get more information
169​ * about the contact
170​ *
mtijinkUnsubmitted
Done

Maybe e-mail addresses are useful too?

mtijink: Maybe e-mail addresses are useful too?
171​ * @param np The package containing the request
172​ * @return true if successfully handled, false otherwise
173​ */
174​ @SuppressWarnings("SameReturnValue")
175​ protected boolean handleRequestAllUIDsTimestamps(@SuppressWarnings("unused") NetworkPacket np) {
176​ NetworkPacket reply = new NetworkPacket(PACKET_TYPE_CONTACTS_RESPONSE_UIDS_TIMESTAMPS);
177​
178​ List<uID> uIDs = ContactsHelper.getAllContactContactIDs(context);
179​
180​ List<String> uIDsAsStrings = new ArrayList<>(uIDs.size());
181​
182​ for (uID uID : uIDs) {
183​ uIDsAsStrings.add(uID.toString());
184​ }
185​
186​ final String[] contactsProjection = new String[]{
187​ ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP
188​ };
189​
190​ reply.set("uids", uIDsAsStrings);
191​
192​ // Add last-modified timestamps
193​ Map<uID, Map<String, Object>> uIDsToTimestamps = ContactsHelper.getColumnsFromContactsForIDs(context, uIDs, contactsProjection);
194​ for (uID ID : uIDsToTimestamps.keySet()) {
195​ Map<String, Object> data = uIDsToTimestamps.get(ID);
196​ reply.set(ID.toString(), (Integer) data.get(ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP));
197​ }
198​
199​ device.sendPacket(reply);
200​
201​ return true;
202​ }
203​
204​ protected boolean handleRequestVCardsByUIDs(NetworkPacket np) {
205​ if (!np.has("uids")) {
206​ Log.e("ContactsPlugin", "handleRequestNamesByUIDs received a malformed packet with no uids key");
207​ return false;
208​ }
209​
210​ List<String> uIDsAsStrings = np.getStringList("uids");
211​
212​ // Convert to Collection<uIDs> to call getVCardsForContactIDs
213​ Set<uID> uIDs = new HashSet<>(uIDsAsStrings.size());
214​ for (String uID : uIDsAsStrings) {
215​ uIDs.add(new uID(uID));
216​ }
217​
218​ Map<uID, VCardBuilder> uIDsToVCards = ContactsHelper.getVCardsForContactIDs(context, uIDs);
219​
220​ // ContactsHelper.getVCardsForContactIDs(..) is allowed to reply without
221​ // some of the requested uIDs if they were not in the database, so update our list
222​ uIDsAsStrings = new ArrayList<>(uIDsToVCards.size());
223​
224​ NetworkPacket reply = new NetworkPacket(PACKET_TYPE_CONTACTS_RESPONSE_VCARDS);
225​
226​ // Add the vcards to the packet
227​ for (uID uID : uIDsToVCards.keySet()) {
228​ VCardBuilder vcard = uIDsToVCards.get(uID);
229​
230​ vcard = this.addVCardMetadata(vcard, uID);
231​
232​ // Store this as a valid uID
233​ uIDsAsStrings.add(uID.toString());
234​ // Add the uid -> vcard pairing to the packet
235​ reply.set(uID.toString(), vcard.toString());
236​ }
237​
mtijinkUnsubmitted
Done

Maybe better to move this check to the beginning of the function: that improves readability and reduces indenting.

mtijink: Maybe better to move this check to the beginning of the function: that improves readability and…
sredmanAuthorUnsubmitted
Done

On thinking more about it, I want to be able to check against many types of packets, so having helper methods for each different type is a better way to clean this up. I have started implementing this.

sredman: On thinking more about it, I want to be able to check against many types of packets, so having…
238​ // Add the valid uIDs to the packet
239​ reply.set("uids", uIDsAsStrings);
240​
241​ device.sendPacket(reply);
242​
243​ return true;
244​ }
245​
246​ @Override
247​ public boolean onPacketReceived(NetworkPacket np) {
248​ switch (np.getType()) {
249​ case PACKET_TYPE_CONTACTS_REQUEST_ALL_UIDS_TIMESTAMPS:
250​ return this.handleRequestAllUIDsTimestamps(np);
251​ case PACKET_TYPE_CONTACTS_REQUEST_VCARDS_BY_UIDS:
252​ return this.handleRequestVCardsByUIDs(np);
253​ default:
254​ Log.e("ContactsPlugin", "Contacts plugin received an unexpected packet!");
255​ return false;
256​ }
257​ }
258​}