Contacts sync logic

* download external resources (contact images)
* improve ETag handling
* contacts: set UNGROUPED_VISIBLE to 1

app/src/main/java/at/bitfire/davdroid/Constants.java

@@ -21,5 +21,5 @@ public class Constants {
 
 	public static final ProdId ICAL_PRODID = new ProdId("-//bitfire web engineering//DAVdroid " + BuildConfig.VERSION_CODE + " (ical4j 2.0-beta1)//EN");
 
-    public static final Logger log = LoggerFactory.getLogger("DAVdroid");
+    public static final Logger log = LoggerFactory.getLogger("davdroid");
 }

app/src/main/java/at/bitfire/davdroid/HttpClient.java

@@ -48,24 +48,41 @@ public class HttpClient extends OkHttpClient {
         userAgent = "DAVdroid/" + BuildConfig.VERSION_NAME + " (" + date + "; dav4android) Android/" + Build.VERSION.RELEASE;
     }
 
+    protected String username, password;
+
 
     public HttpClient() {
         super();
         initialize();
-        enableLogs();
     }
 
     public HttpClient(String username, String password, boolean preemptive) {
         super();
         initialize();
 
-        enableLogs();
-
         // authentication
+        this.username = username;
+        this.password = password;
         if (preemptive)
             networkInterceptors().add(new PreemptiveAuthenticationInterceptor(username, password));
         else
-            setAuthenticator(new DavAuthenticator(username, password));
+            setAuthenticator(new DavAuthenticator(null, username, password));
+    }
+
+    /**
+     * Creates a new HttpClient (based on another one) which can be used to download external resources:
+     * 1. it does not use preemptive authentiation
+     * 2. it only authenticates against a given host
+     * @param client  user name and password from this client will be used
+     * @param host    authentication will be restricted to this host
+     */
+    public HttpClient(HttpClient client, String host) {
+        super();
+        initialize();
+
+        username = client.username;
+        password = client.password;
+        setAuthenticator(new DavAuthenticator(host, username, password));
     }
 
 
@@ -73,12 +90,16 @@ public class HttpClient extends OkHttpClient {
         // don't follow redirects automatically because this may rewrite DAV methods to GET
         setFollowRedirects(false);
 
-        setConnectTimeout(20, TimeUnit.SECONDS);
+        // set timeouts
+        setConnectTimeout(30, TimeUnit.SECONDS);
         setWriteTimeout(15, TimeUnit.SECONDS);
         setReadTimeout(45, TimeUnit.SECONDS);
 
         // add User-Agent to every request
         networkInterceptors().add(userAgentInterceptor);
+
+        // enable logs
+        enableLogs();
     }
 
     protected void enableLogs() {
@@ -111,11 +132,18 @@ public class HttpClient extends OkHttpClient {
     }
 
     @RequiredArgsConstructor
-    static class DavAuthenticator implements Authenticator {
+    public static class DavAuthenticator implements Authenticator {
-        final String username, password;
+        final String host, username, password;
 
         @Override
         public Request authenticate(Proxy proxy, Response response) throws IOException {
+            Request request = response.request();
+
+            if (host != null && !request.httpUrl().host().equalsIgnoreCase(host)) {
+                Constants.log.warn("Not authenticating against " +  host + " for security reasons!");
+                return null;
+            }
+
             // check whether this is the first authentication try with our credentials
             Response priorResponse = response.priorResponse();
             boolean triedBefore = priorResponse != null ? priorResponse.request().header(HEADER_AUTHORIZATION) != null : false;
@@ -126,7 +154,7 @@ public class HttpClient extends OkHttpClient {
             //List<HttpUtils.AuthScheme> schemes = HttpUtils.parseWwwAuthenticate(response.headers("WWW-Authenticate"));
             // TODO Digest auth
 
-            return response.request().newBuilder()
+            return request.newBuilder()
                     .header(HEADER_AUTHORIZATION, Credentials.basic(username, password))
                     .build();
         }

app/src/main/java/at/bitfire/davdroid/resource/LocalContact.java

@@ -36,17 +36,18 @@ public class LocalContact extends AndroidContact {
     public void clearDirty(String eTag) throws ContactsStorageException {
         try {
             ContentValues values = new ContentValues(1);
-            values.put(COLUMN_ETAG, eTag);
             values.put(ContactsContract.RawContacts.DIRTY, 0);
+            values.put(COLUMN_ETAG, eTag);
             addressBook.provider.update(rawContactSyncURI(), values, null, null);
         } catch (RemoteException e) {
             throw new ContactsStorageException("Couldn't clear dirty flag", e);
         }
     }
 
-    public void updateUID(String uid) throws ContactsStorageException {
+    public void updateFileNameAndUID(String uid) throws ContactsStorageException {
         try {
             ContentValues values = new ContentValues(1);
+            values.put(COLUMN_FILENAME, uid + ".vcf");
             values.put(COLUMN_UID, uid);
             addressBook.provider.update(rawContactSyncURI(), values, null, null);
         } catch (RemoteException e) {

app/src/main/java/at/bitfire/davdroid/syncadapter/ContactsSyncAdapterService.java

@@ -22,7 +22,10 @@ import android.util.Log;
 
 import com.squareup.okhttp.HttpUrl;
 import com.squareup.okhttp.MediaType;
+import com.squareup.okhttp.OkHttpClient;
+import com.squareup.okhttp.Request;
 import com.squareup.okhttp.RequestBody;
+import com.squareup.okhttp.Response;
 import com.squareup.okhttp.ResponseBody;
 
 import org.apache.commons.io.Charsets;
@@ -30,6 +33,7 @@ import org.apache.commons.io.Charsets;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
 import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -37,11 +41,13 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.UUID;
 
 import at.bitfire.dav4android.DavAddressBook;
 import at.bitfire.dav4android.DavResource;
 import at.bitfire.dav4android.exception.DavException;
 import at.bitfire.dav4android.exception.HttpException;
+import at.bitfire.dav4android.exception.PreconditionFailedException;
 import at.bitfire.dav4android.property.AddressData;
 import at.bitfire.dav4android.property.GetCTag;
 import at.bitfire.dav4android.property.GetContentType;
@@ -56,7 +62,9 @@ import at.bitfire.vcard4android.Contact;
 import at.bitfire.vcard4android.ContactsStorageException;
 import ezvcard.VCardVersion;
 import ezvcard.property.Uid;
+import ezvcard.util.IOUtils;
 import lombok.Cleanup;
+import lombok.RequiredArgsConstructor;
 
 public class ContactsSyncAdapterService extends Service {
 	private static ContactsSyncAdapter syncAdapter;
@@ -130,9 +138,9 @@ public class ContactsSyncAdapterService extends Service {
                 // assign file names and UIDs to new contacts so that we can use the file name as an index
                 localList = addressBook.getWithoutFileName();
                 for (LocalContact local : localList) {
-                    String uuid = Uid.random().toString();
+                    String uuid = UUID.randomUUID().toString();
                     Constants.log.info("Found local contact #" + local.getId() + " without file name; assigning name UID/name " + uuid + "[.vcf]");
-                    local.updateUID(uuid);
+                    local.updateFileNameAndUID(uuid);
                 }
 
                 // upload dirty contacts
@@ -147,32 +155,48 @@ public class ContactsSyncAdapterService extends Service {
                             local.getContact().toStream(hasVCard4 ? VCardVersion.V4_0 : VCardVersion.V3_0).toByteArray()
                     );
 
-                    if (local.eTag == null) {
+                    try {
-                        Constants.log.info("Uploading new contact " + fileName);
+                        if (local.eTag == null) {
-                        remote.put(vCard, null, local.eTag);
+                            Constants.log.info("Uploading new contact " + fileName);
-                    } else {
+                            remote.put(vCard, null, true);
-                        Constants.log.info("Uploading locally modified contact " + fileName);
+                            // TODO handle 30x
-                        remote.put(vCard, local.eTag, null);
+                        } else {
+                            Constants.log.info("Uploading locally modified contact " + fileName);
+                            remote.put(vCard, local.eTag, false);
+                            // TODO handle 30x
+                        }
+
+                    } catch(PreconditionFailedException e) {
+                        Constants.log.info("Contact has been modified on the server before upload, ignoring", e);
                     }
 
-                    GetETag newETag = (GetETag) remote.properties.get(GetETag.NAME);
+                    String eTag = null;
-                    local.clearDirty(newETag != null ? newETag.eTag : null);
+                    GetETag newETag = (GetETag)remote.properties.get(GetETag.NAME);
+                    if (newETag != null) {
+                        eTag = newETag.eTag;
+                        Constants.log.debug("Received new ETag=" + eTag + " after uploading");
+                    } else
+                        Constants.log.debug("Didn't receive new ETag after uploading, setting to null");
+
+                    local.clearDirty(eTag);
                 }
 
                 // check CTag (ignore on manual sync)
                 String currentCTag = null;
+                GetCTag getCTag = (GetCTag) dav.properties.get(GetCTag.NAME);
+                if (getCTag != null)
+                    currentCTag = getCTag.cTag;
+
+                String localCTag = null;
                 if (extras.containsKey(ContentResolver.SYNC_EXTRAS_MANUAL))
                     Constants.log.info("Manual sync, ignoring CTag");
-                else {
+                else
-                    GetCTag getCTag = (GetCTag) dav.properties.get(GetCTag.NAME);
+                    localCTag = addressBook.getCTag();
-                    if (getCTag != null)
-                        currentCTag = getCTag.cTag;
-                }
 
-                if (currentCTag != null && !(currentCTag.equals(addressBook.getCTag()))) {
+                if (currentCTag != null && currentCTag.equals(localCTag)) {
                     Constants.log.info("Remote address book didn't change (CTag=" + currentCTag + "), no need to list VCards");
 
-                } else {
+                } else /* remote CTag has changed */ {
                     // fetch list of local contacts and build hash table to index file name
                     localList = addressBook.getAll();
                     Map<String, LocalContact> localContacts = new HashMap<>(localList.length);
@@ -227,9 +251,13 @@ public class ContactsSyncAdapterService extends Service {
 
                     Constants.log.info("Downloading " + toDownload.size() + " contacts (" + MAX_MULTIGET + " at once)");
 
+                    // prepare downloader which may be used to download external resource like contact photos
+                    Contact.Downloader downloader = new ResourceDownloader(httpClient, addressBookURL);
+
                     // download new/updated VCards from server
                     for (DavResource[] bunch : ArrayUtils.partition(toDownload.toArray(new DavResource[toDownload.size()]), MAX_MULTIGET)) {
                         Constants.log.info("Downloading " + TextUtils.join(" + ", bunch));
+
                         if (bunch.length == 1) {
                             // only one contact, use GET
                             DavResource remote = bunch[0];
@@ -239,7 +267,7 @@ public class ContactsSyncAdapterService extends Service {
                             String eTag = ((GetETag)remote.properties.get(GetETag.NAME)).eTag;
 
                             @Cleanup InputStream stream = body.byteStream();
-                            processVCard(addressBook, localContacts, remote.fileName(), eTag, stream, body.contentType().charset(Charsets.UTF_8));
+                            processVCard(addressBook, localContacts, remote.fileName(), eTag, stream, body.contentType().charset(Charsets.UTF_8), downloader);
 
                         } else {
                             // multiple contacts, use multi-get
@@ -270,7 +298,7 @@ public class ContactsSyncAdapterService extends Service {
                                     throw new DavException("Received multi-get response without address data");
 
                                 @Cleanup InputStream stream = new ByteArrayInputStream(addressData.vCard.getBytes());
-                                processVCard(addressBook, localContacts, remote.fileName(), eTag, stream, charset);
+                                processVCard(addressBook, localContacts, remote.fileName(), eTag, stream, charset, downloader);
                             }
                         }
                     }
@@ -290,8 +318,8 @@ public class ContactsSyncAdapterService extends Service {
         }
 
 
-        private void processVCard(LocalAddressBook addressBook, Map<String, LocalContact>localContacts, String fileName, String eTag, InputStream stream, Charset charset) throws IOException, ContactsStorageException {
+        private void processVCard(LocalAddressBook addressBook, Map<String, LocalContact>localContacts, String fileName, String eTag, InputStream stream, Charset charset, Contact.Downloader downloader) throws IOException, ContactsStorageException {
-            Contact contacts[] = Contact.fromStream(stream, charset);
+            Contact contacts[] = Contact.fromStream(stream, charset, downloader);
             if (contacts.length == 1) {
                 Contact newData = contacts[0];
 
@@ -313,4 +341,35 @@ public class ContactsSyncAdapterService extends Service {
 
     }
 
+
+    @RequiredArgsConstructor
+    static class ResourceDownloader implements Contact.Downloader {
+        final HttpClient httpClient;
+        final HttpUrl baseUrl;
+
+        @Override
+        public byte[] download(String url, String accepts) {
+            HttpUrl httpUrl = HttpUrl.parse(url);
+            HttpClient resourceClient = new HttpClient(httpClient, httpUrl.host());
+            try {
+                Response response = resourceClient.newCall(new Request.Builder()
+                        .get()
+                        .url(httpUrl)
+                        .build()).execute();
+
+                ResponseBody body = response.body();
+                if (body != null) {
+                    @Cleanup InputStream stream = body.byteStream();
+                    if (response.isSuccessful() && stream != null) {
+                        return IOUtils.toByteArray(stream);
+                    } else
+                        Constants.log.error("Couldn't download external resource");
+                }
+            } catch(IOException e) {
+                Constants.log.error("Couldn't download external resource", e);
+            }
+            return null;
+        }
+    }
+
 }

app/src/main/java/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.java

@@ -10,7 +10,9 @@ package at.bitfire.davdroid.ui.setup;
 import android.accounts.Account;
 import android.accounts.AccountManager;
 import android.app.Fragment;
+import android.content.ContentProviderClient;
 import android.content.ContentResolver;
+import android.content.ContentValues;
 import android.os.Bundle;
 import android.provider.CalendarContract;
 import android.provider.ContactsContract;
@@ -31,11 +33,14 @@ import java.util.List;
 
 import at.bitfire.davdroid.Constants;
 import at.bitfire.davdroid.R;
+import at.bitfire.davdroid.resource.LocalAddressBook;
 import at.bitfire.davdroid.resource.LocalCalendar;
 import at.bitfire.davdroid.resource.LocalStorageException;
 import at.bitfire.davdroid.resource.LocalTaskList;
 import at.bitfire.davdroid.resource.ServerInfo;
 import at.bitfire.davdroid.syncadapter.AccountSettings;
+import at.bitfire.vcard4android.ContactsStorageException;
+import lombok.Cleanup;
 
 public class AccountDetailsFragment extends Fragment implements TextWatcher {
 	public static final String TAG = "davdroid.AccountDetails";
@@ -92,7 +97,17 @@ public class AccountDetailsFragment extends Fragment implements TextWatcher {
 		Bundle userData = AccountSettings.createBundle(serverInfo);
 
 		if (accountManager.addAccountExplicitly(account, serverInfo.getPassword(), userData)) {
-			addSync(account, ContactsContract.AUTHORITY, serverInfo.getAddressBooks(), null);
+			addSync(account, ContactsContract.AUTHORITY, serverInfo.getAddressBooks(), new AddSyncCallback() {
+                @Override
+                public void createLocalCollection(Account account, ServerInfo.ResourceInfo resource) throws LocalStorageException, ContactsStorageException {
+                    @Cleanup("release") ContentProviderClient provider = getActivity().getContentResolver().acquireContentProviderClient(ContactsContract.AUTHORITY);
+                    LocalAddressBook addressBook = new LocalAddressBook(account, provider);
+                    ContentValues settings = new ContentValues(2);
+                    settings.put(ContactsContract.Settings.SHOULD_SYNC, 1);
+                    settings.put(ContactsContract.Settings.UNGROUPED_VISIBLE, 1);
+                    addressBook.updateSettings(settings);
+                }
+            });
 
 			addSync(account, CalendarContract.AUTHORITY, serverInfo.getCalendars(), new AddSyncCallback() {
 				@Override
@@ -114,7 +129,7 @@ public class AccountDetailsFragment extends Fragment implements TextWatcher {
 	}
 
 	protected interface AddSyncCallback {
-		void createLocalCollection(Account account, ServerInfo.ResourceInfo resource) throws LocalStorageException;
+		void createLocalCollection(Account account, ServerInfo.ResourceInfo resource) throws LocalStorageException, ContactsStorageException;
 	}
 
 	protected void addSync(Account account, String authority, List<ServerInfo.ResourceInfo> resourceList, AddSyncCallback callback) {
@@ -125,7 +140,7 @@ public class AccountDetailsFragment extends Fragment implements TextWatcher {
 				if (callback != null)
 					try {
 						callback.createLocalCollection(account, resource);
-					} catch(LocalStorageException e) {
+					} catch(LocalStorageException|ContactsStorageException e) {
 						Log.e(TAG, "Couldn't add sync collection", e);
 						Toast.makeText(getActivity(), "Couldn't set up synchronization for " + authority, Toast.LENGTH_LONG).show();
 					}

dav4android

@@ -1 +1 @@
-Subproject commit a6975918ed614eef93c222451fd0981c60ec3ad9
+Subproject commit 487f8d544ffd1ade5751fa4768fc4062b86d9ac7

vcard4android

@@ -1 +1 @@
-Subproject commit 8053287e694d5a1b5c609fbbe3af97775e5a99af
+Subproject commit 83de70faf59054a5ca3dec82f932cd071695034f