Sync manager optimization

* allow cancellation of synchronization within appropriate time
* sync error notification: use loader, show all accounts, show whether JB Workaround is installed, reorder

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

@@ -62,47 +62,10 @@ public class HttpClient extends OkHttpClient {
     protected String username, password;
 
 
-    public HttpClient() {
+    protected HttpClient(Context context) {
-        super();
-        context = null;
-        initialize();
-    }
-
-    public HttpClient(Context context, String username, String password, boolean preemptive) {
         super();
         this.context = context;
 
-        initialize();
-
-        // authentication
-        this.username = username;
-        this.password = password;
-        if (preemptive)
-            networkInterceptors().add(new PreemptiveAuthenticationInterceptor(username, password));
-        else
-            setAuthenticator(new BasicDigestAuthenticator(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 authentication
-     * 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();
-        context = client.context;
-
-        initialize();
-
-        username = client.username;
-        password = client.password;
-        setAuthenticator(new BasicDigestAuthenticator(host, username, password));
-    }
-
-
-    protected void initialize() {
         if (context != null) {
             // use MemorizingTrustManager to manage self-signed certificates
             MemorizingTrustManager mtm = new MemorizingTrustManager(context);
@@ -131,6 +94,39 @@ public class HttpClient extends OkHttpClient {
             enableLogs();
     }
 
+    public HttpClient(Context context, String username, String password, boolean preemptive) {
+        this(context);
+
+        // authentication
+        this.username = username;
+        this.password = password;
+        if (preemptive)
+            networkInterceptors().add(new PreemptiveAuthenticationInterceptor(username, password));
+        else
+            setAuthenticator(new BasicDigestAuthenticator(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 authentication
+     * 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) {
+        this(client.context);
+
+        username = client.username;
+        password = client.password;
+        setAuthenticator(new BasicDigestAuthenticator(host, username, password));
+    }
+
+    // for testing (mock server doesn't need auth)
+    protected HttpClient() {
+        this(null, null, null, false);
+    }
+
+
     protected void enableLogs() {
         interceptors().add(loggingInterceptor);
     }

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

@@ -59,8 +59,8 @@ public class CalendarSyncManager extends SyncManager {
     protected static final int MAX_MULTIGET = 20;
 
 
-    public CalendarSyncManager(Context context, Account account, Bundle extras, SyncResult result, LocalCalendar calendar) {
+    public CalendarSyncManager(Context context, Account account, Bundle extras, String authority, SyncResult result, LocalCalendar calendar) {
-        super(Constants.NOTIFICATION_CALENDAR_SYNC, context, account, extras, result);
+        super(Constants.NOTIFICATION_CALENDAR_SYNC, context, account, extras, authority, result);
         localCollection = calendar;
     }
 
@@ -130,6 +130,8 @@ public class CalendarSyncManager extends SyncManager {
 
         // download new/updated iCalendars from server
         for (DavResource[] bunch : ArrayUtils.partition(toDownload.toArray(new DavResource[toDownload.size()]), MAX_MULTIGET)) {
+            if (Thread.interrupted())
+                return;
             Constants.log.info("Downloading " + StringUtils.join(bunch, ", "));
 
             if (bunch.length == 1) {

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

@@ -55,7 +55,7 @@ public class CalendarsSyncAdapterService extends Service {
             try {
                 for (LocalCalendar calendar : (LocalCalendar[])LocalCalendar.find(account, provider, LocalCalendar.Factory.INSTANCE, CalendarContract.Calendars.SYNC_EVENTS + "!=0", null)) {
                     Constants.log.info("Synchronizing calendar #"  + calendar.getId() + ", URL: " + calendar.getName());
-                    CalendarSyncManager syncManager = new CalendarSyncManager(getContext(), account, extras, syncResult, calendar);
+                    CalendarSyncManager syncManager = new CalendarSyncManager(getContext(), account, extras, authority, syncResult, calendar);
                     syncManager.performSync();
                 }
             } catch (CalendarStorageException e) {

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

@@ -48,7 +48,7 @@ public class ContactsSyncAdapterService extends Service {
         public void onPerformSync(Account account, Bundle extras, String authority, ContentProviderClient provider, SyncResult syncResult) {
             Constants.log.info("Starting address book sync (" + authority + ")");
 
-            ContactsSyncManager syncManager = new ContactsSyncManager(getContext(), account, extras, provider, syncResult);
+            ContactsSyncManager syncManager = new ContactsSyncManager(getContext(), account, extras, authority, provider, syncResult);
             syncManager.performSync();
 
             Constants.log.info("Address book sync complete");

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

@@ -63,8 +63,8 @@ public class ContactsSyncManager extends SyncManager {
     protected boolean hasVCard4;
 
 
-    public ContactsSyncManager(Context context, Account account, Bundle extras, ContentProviderClient provider, SyncResult result) {
+    public ContactsSyncManager(Context context, Account account, Bundle extras, String authority, ContentProviderClient provider, SyncResult result) {
-        super(Constants.NOTIFICATION_CONTACTS_SYNC, context, account, extras, result);
+        super(Constants.NOTIFICATION_CONTACTS_SYNC, context, account, extras, authority, result);
         this.provider = provider;
     }
 
@@ -140,6 +140,9 @@ public class ContactsSyncManager extends SyncManager {
 
         // download new/updated VCards from server
         for (DavResource[] bunch : ArrayUtils.partition(toDownload.toArray(new DavResource[toDownload.size()]), MAX_MULTIGET)) {
+            if (Thread.interrupted())
+                return;
+
             Constants.log.info("Downloading " + StringUtils.join(bunch, ", "));
 
             if (bunch.length == 1) {

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

@@ -31,6 +31,7 @@ import java.util.Set;
 import java.util.UUID;
 
 import at.bitfire.dav4android.DavResource;
+import at.bitfire.dav4android.exception.ConflictException;
 import at.bitfire.dav4android.exception.DavException;
 import at.bitfire.dav4android.exception.HttpException;
 import at.bitfire.dav4android.exception.UnauthorizedException;
@@ -68,6 +69,7 @@ abstract public class SyncManager {
     protected final Context context;
     protected final Account account;
     protected final Bundle extras;
+    protected final String authority;
     protected final SyncResult syncResult;
 
     protected final AccountSettings settings;
@@ -92,10 +94,11 @@ abstract public class SyncManager {
 
 
 
-    public SyncManager(int notificationId, Context context, Account account, Bundle extras, SyncResult syncResult) {
+    public SyncManager(int notificationId, Context context, Account account, Bundle extras, String authority, SyncResult syncResult) {
         this.context = context;
         this.account = account;
         this.extras = extras;
+        this.authority = authority;
         this.syncResult = syncResult;
 
         // get account settings and generate httpClient
@@ -115,6 +118,8 @@ abstract public class SyncManager {
             Constants.log.info("Preparing synchronization");
             prepare();
 
+            if (Thread.interrupted())
+                return;
             syncPhase = SYNC_PHASE_QUERY_CAPABILITIES;
             Constants.log.info("Querying capabilities");
             queryCapabilities();
@@ -123,6 +128,8 @@ abstract public class SyncManager {
             Constants.log.info("Processing locally deleted entries");
             processLocallyDeleted();
 
+            if (Thread.interrupted())
+                return;
             syncPhase = SYNC_PHASE_PREPARE_DIRTY;
             Constants.log.info("Locally preparing dirty entries");
             prepareDirty();
@@ -138,10 +145,14 @@ abstract public class SyncManager {
                 Constants.log.info("Listing local entries");
                 listLocal();
 
+                if (Thread.interrupted())
+                    return;
                 syncPhase = SYNC_PHASE_LIST_REMOTE;
                 Constants.log.info("Listing remote entries");
                 listRemote();
 
+                if (Thread.interrupted())
+                    return;
                 syncPhase = SYNC_PHASE_COMPARE_LOCAL_REMOTE;
                 Constants.log.info("Comparing local/remote entries");
                 compareLocalRemote();
@@ -197,6 +208,7 @@ abstract public class SyncManager {
                 detailsIntent = new Intent(context, DebugInfoActivity.class);
                 detailsIntent.putExtra(DebugInfoActivity.KEY_EXCEPTION, e);
                 detailsIntent.putExtra(DebugInfoActivity.KEY_ACCOUNT, account);
+                detailsIntent.putExtra(DebugInfoActivity.KEY_AUTHORITY, authority);
                 detailsIntent.putExtra(DebugInfoActivity.KEY_PHASE, syncPhase);
             }
 
@@ -225,7 +237,6 @@ abstract public class SyncManager {
                 notification = builder.getNotification();
             }
             notificationManager.notify(account.name, notificationId, notification);
-
         }
     }
 
@@ -234,11 +245,18 @@ abstract public class SyncManager {
 
     abstract protected void queryCapabilities() throws IOException, HttpException, DavException, CalendarStorageException, ContactsStorageException;
 
+    /**
+     * Process locally deleted entries (DELETE them on the server as well).
+     * Checks Thread.interrupted() before each request to allow quick sync cancellation.
+     */
     protected void processLocallyDeleted() throws CalendarStorageException, ContactsStorageException {
         // Remove locally deleted entries from server (if they have a name, i.e. if they were uploaded before),
         // but only if they don't have changed on the server. Then finally remove them from the local address book.
         LocalResource[] localList = localCollection.getDeleted();
         for (LocalResource local : localList) {
+            if (Thread.interrupted())
+                return;
+
             final String fileName = local.getFileName();
             if (!TextUtils.isEmpty(fileName)) {
                 Constants.log.info(fileName + " has been deleted locally -> deleting from server");
@@ -246,7 +264,7 @@ abstract public class SyncManager {
                     new DavResource(httpClient, collectionURL.newBuilder().addPathSegment(fileName).build())
                             .delete(local.getETag());
                 } catch (IOException|HttpException e) {
-                    Constants.log.warn("Couldn't delete " + fileName + " from server");
+                    Constants.log.warn("Couldn't delete " + fileName + " from server; ignoring (may be downloaded again)");
                 }
             } else
                 Constants.log.info("Removing local record #" + local.getId() + " which has been deleted locally and was never uploaded");
@@ -266,9 +284,16 @@ abstract public class SyncManager {
 
     abstract protected RequestBody prepareUpload(LocalResource resource) throws IOException, CalendarStorageException, ContactsStorageException;
 
+    /**
+     * Uploads dirty records to the server, using a PUT request for each record.
+     * Checks Thread.interrupted() before each request to allow quick sync cancellation.
+     */
     protected void uploadDirty() throws IOException, HttpException, CalendarStorageException, ContactsStorageException {
         // upload dirty contacts
         for (LocalResource local : localCollection.getDirty()) {
+            if (Thread.interrupted())
+                return;
+
             final String fileName = local.getFileName();
 
             DavResource remote = new DavResource(httpClient, collectionURL.newBuilder().addPathSegment(fileName).build());
@@ -281,14 +306,13 @@ abstract public class SyncManager {
                 if (local.getETag() == null) {
                     Constants.log.info("Uploading new record " + fileName);
                     remote.put(body, null, true);
-                    // TODO handle 30x
                 } else {
                     Constants.log.info("Uploading locally modified record " + fileName);
                     remote.put(body, local.getETag(), false);
-                    // TODO handle 30x
                 }
 
-            } catch (PreconditionFailedException e) {
+            } catch (ConflictException|PreconditionFailedException e) {
+                // we can't interact with the user to resolve the conflict, so we treat 409 like 412
                 Constants.log.info("Resource has been modified on the server before upload, ignoring", e);
             }
 
@@ -396,6 +420,7 @@ abstract public class SyncManager {
 
     /**
      * Downloads the remote resources in {@link #toDownload} and stores them locally.
+     * Must check Thread.interrupted() periodically to allow quick sync cancellation.
      */
     abstract protected void downloadRemote() throws IOException, HttpException, DavException, ContactsStorageException, CalendarStorageException;
 

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

@@ -61,7 +61,7 @@ public class TasksSyncAdapterService extends Service {
 
                 for (LocalTaskList taskList : (LocalTaskList[])LocalTaskList.find(account, provider, LocalTaskList.Factory.INSTANCE, null, null)) {
                     Constants.log.info("Synchronizing task list #"  + taskList.getId() + ", URL: " + taskList.getSyncId());
-                    TasksSyncManager syncManager = new TasksSyncManager(getContext(), account, extras, provider, syncResult, taskList);
+                    TasksSyncManager syncManager = new TasksSyncManager(getContext(), account, extras, authority, provider, syncResult, taskList);
                     syncManager.performSync();
                 }
             } catch (CalendarStorageException e) {

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

@@ -62,8 +62,8 @@ public class TasksSyncManager extends SyncManager {
     final protected TaskProvider provider;
 
 
-    public TasksSyncManager(Context context, Account account, Bundle extras, TaskProvider provider, SyncResult result, LocalTaskList taskList) {
+    public TasksSyncManager(Context context, Account account, Bundle extras, String authority, TaskProvider provider, SyncResult result, LocalTaskList taskList) {
-        super(Constants.NOTIFICATION_TASK_SYNC, context, account, extras, result);
+        super(Constants.NOTIFICATION_TASK_SYNC, context, account, extras, authority, result);
         this.provider = provider;
         localCollection = taskList;
     }
@@ -128,6 +128,9 @@ public class TasksSyncManager extends SyncManager {
 
         // download new/updated iCalendars from server
         for (DavResource[] bunch : ArrayUtils.partition(toDownload.toArray(new DavResource[toDownload.size()]), MAX_MULTIGET)) {
+            if (Thread.interrupted())
+                return;
+
             Constants.log.info("Downloading " + StringUtils.join(bunch, ", "));
 
             if (bunch.length == 1) {

app/src/main/java/at/bitfire/davdroid/ui/DebugInfoActivity.java

@@ -9,13 +9,19 @@
 package at.bitfire.davdroid.ui;
 
 import android.accounts.Account;
+import android.accounts.AccountManager;
 import android.app.Activity;
+import android.app.LoaderManager;
+import android.content.AsyncTaskLoader;
 import android.content.ContentResolver;
+import android.content.Context;
 import android.content.Intent;
+import android.content.Loader;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.os.Build;
 import android.os.Bundle;
+import android.os.Debug;
 import android.provider.CalendarContract;
 import android.provider.ContactsContract;
 import android.text.TextUtils;
@@ -30,12 +36,14 @@ import at.bitfire.davdroid.BuildConfig;
 import at.bitfire.davdroid.Constants;
 import at.bitfire.davdroid.R;
 
-public class DebugInfoActivity extends Activity {
+public class DebugInfoActivity extends Activity implements LoaderManager.LoaderCallbacks<String> {
     public static final String
             KEY_EXCEPTION = "exception",
             KEY_ACCOUNT = "account",
+            KEY_AUTHORITY = "authority",
             KEY_PHASE = "phase";
 
+    TextView tvReport;
     String report;
 
     @Override
@@ -44,8 +52,10 @@ public class DebugInfoActivity extends Activity {
 
         setContentView(R.layout.debug_info_activity);
 
-        TextView tvReport = (TextView)findViewById(R.id.text_report);
+        tvReport = (TextView)findViewById(R.id.text_report);
-        tvReport.setText(report = generateReport(getIntent().getExtras()));
+        //tvReport.setText(report = generateReport(getIntent().getExtras()));
+
+        getLoaderManager().initLoader(0, getIntent().getExtras(), this);
     }
 
     @Override
@@ -66,20 +76,114 @@ public class DebugInfoActivity extends Activity {
         }
     }
 
+    @Override
+    public Loader<String> onCreateLoader(int id, Bundle args) {
+        return new ReportLoader(this, args);
+    }
 
-    protected String generateReport(Bundle extras) {
+    @Override
+    public void onLoadFinished(Loader<String> loader, String data) {
+        if (data != null)
+            tvReport.setText(report = data);
+    }
+
+    @Override
+    public void onLoaderReset(Loader<String> loader) {
+    }
+
+
+    static class ReportLoader extends AsyncTaskLoader<String> {
+
+        final Bundle extras;
+
+        public ReportLoader(Context context, Bundle extras) {
+            super(context);
+            this.extras = extras;
+        }
+
+        @Override
+        protected void onStartLoading() {
+            forceLoad();
+        }
+
+        @Override
+        public String loadInBackground() {
             Exception exception = null;
+            String authority = null;
             Account account = null;
             Integer phase = null;
 
             if (extras != null) {
-            exception = (Exception) extras.getSerializable(KEY_EXCEPTION);
+                exception = (Exception)extras.getSerializable(KEY_EXCEPTION);
-            account = (Account) extras.getParcelable(KEY_ACCOUNT);
+                account = extras.getParcelable(KEY_ACCOUNT);
+                authority = extras.getString(KEY_AUTHORITY);
                 phase = extras.getInt(KEY_PHASE);
             }
 
             StringBuilder report = new StringBuilder();
 
+            // begin with most specific information
+
+            if (phase != null)
+                report.append("SYNCHRONIZATION INFO\nSynchronization phase: " + phase + "\n");
+            if (account != null)
+                report.append("Account name: " + account.name + "\n");
+            if (authority != null)
+                report.append("Authority: " + authority + "\n\n");
+
+            if (exception instanceof HttpException) {
+                HttpException http = (HttpException)exception;
+                if (http.request != null)
+                    report.append("HTTP REQUEST:\n" + http.request + "\n\n");
+                if (http.response != null)
+                    report.append("HTTP RESPONSE:\n" + http.response + "\n");
+            }
+
+            if (exception != null) {
+                report.append("STACK TRACE:\n");
+                for (String stackTrace : ExceptionUtils.getRootCauseStackTrace(exception))
+                    report.append(stackTrace + "\n");
+                report.append("\n");
+            }
+
+            try {
+                PackageManager pm = getContext().getPackageManager();
+                String installedFrom = pm.getInstallerPackageName(BuildConfig.APPLICATION_ID);
+                if (TextUtils.isEmpty(installedFrom))
+                    installedFrom = "APK (directly)";
+                else {
+                    PackageInfo installer = pm.getPackageInfo(installedFrom, PackageManager.GET_META_DATA);
+                    if (installer != null)
+                        installedFrom = pm.getApplicationLabel(installer.applicationInfo).toString();
+                }
+                boolean workaroundInstalled = false;
+                try {
+                    workaroundInstalled = pm.getPackageInfo("at.bitfire.davdroid.jbworkaround", 0) != null;
+                } catch(PackageManager.NameNotFoundException e) {}
+                report.append(
+                        "SOFTWARE INFORMATION\n" +
+                                "DAVdroid version: " + BuildConfig.VERSION_NAME + " (" + BuildConfig.VERSION_CODE + ") " + BuildConfig.buildTime.toString() + "\n" +
+                                "Installed from: " + installedFrom + "\n" +
+                                "JB Workaround installed: " + (workaroundInstalled ? "yes" : "no") + "\n\n"
+                );
+            } catch(Exception ex) {
+                Constants.log.error("Couldn't get software information", ex);
+            }
+
+            report.append(
+                    "CONFIGURATION\n" +
+                            "System-wide synchronization: " + (ContentResolver.getMasterSyncAutomatically() ? "automatically" : "manually") + "\n"
+            );
+            AccountManager accountManager = AccountManager.get(getContext());
+            for (Account acc : accountManager.getAccountsByType(Constants.ACCOUNT_TYPE)) {
+                report.append(
+                        "  Account: " + acc.name + "\n" +
+                                "    Address book synchronization: " + syncStatus(acc, ContactsContract.AUTHORITY) + "\n" +
+                                "    Calendar     synchronization: " + syncStatus(acc, CalendarContract.AUTHORITY) + "\n" +
+                                "    OpenTasks    synchronization: " + syncStatus(acc, "org.dmfs.tasks") + "\n\n"
+                );
+            }
+
             try {
                 report.append(
                         "SYSTEM INFORMATION\n" +
@@ -90,62 +194,14 @@ public class DebugInfoActivity extends Activity {
                 Constants.log.error("Couldn't get system details", ex);
             }
 
-        try {
-            PackageManager pm = getPackageManager();
-            String installedFrom = pm.getInstallerPackageName(BuildConfig.APPLICATION_ID);
-            if (TextUtils.isEmpty(installedFrom))
-                installedFrom = "APK (directly)";
-            else {
-                PackageInfo installer = pm.getPackageInfo(installedFrom, PackageManager.GET_META_DATA);
-                if (installer != null)
-                    installedFrom = pm.getApplicationLabel(installer.applicationInfo).toString();
-            }
-            report.append(
-                    "SOFTWARE INFORMATION\n" +
-                    "DAVdroid version: " + BuildConfig.VERSION_NAME + " (" + BuildConfig.VERSION_CODE + ") " + BuildConfig.buildTime.toString() + "\n" +
-                    "Installed from: " + installedFrom + "\n\n"
-            );
-        } catch(Exception ex) {
-            Constants.log.error("Couldn't get software information", ex);
-        }
-
-        report.append(
-                "CONFIGURATION\n" +
-                "System-wide  synchronization: " + (ContentResolver.getMasterSyncAutomatically() ? "automatically" : "manually") + " (overrides account settings)\n"
-        );
-        if (account != null)
-            report.append(
-                    "Account name: " + account.name + "\n" +
-                    "Address book synchronization: " + syncStatus(account, ContactsContract.AUTHORITY) + "\n" +
-                    "Calendar     synchronization: " + syncStatus(account, CalendarContract.AUTHORITY) + "\n" +
-                    "OpenTasks    synchronization: " + syncStatus(account, "org.dmfs.tasks") + "\n\n"
-            );
-
-        if (phase != null) {
-            report.append("SYNCHRONIZATION INFO\nSychronization phase: " + phase + "\n\n");
-        }
-
-        if (exception instanceof HttpException) {
-            HttpException http = (HttpException)exception;
-            if (http.request != null)
-                report.append("HTTP REQUEST:\n" + http.request + "\n\n");
-            if (http.response != null)
-            report.append("HTTP RESPONSE:\n" + http.response + "\n\n");
-        }
-
-        if (exception != null) {
-            report.append("STACK TRACE\n");
-            for (String stackTrace : ExceptionUtils.getRootCauseStackTrace(exception))
-                report.append(stackTrace + "\n");
-        }
-
             return report.toString();
         }
 
-    protected static String syncStatus(Account account, String authority) {
+        protected String syncStatus(Account account, String authority) {
             return ContentResolver.getIsSyncable(account, authority) > 0 ?
                     (ContentResolver.getSyncAutomatically(account, ContactsContract.AUTHORITY) ? "automatically" : "manually") :
                     "—";
         }
+    }
 
 }

dav4android

@@ -1 +1 @@
-Subproject commit 2083d075d3b4a4b9ac0a930af1d019547d7dcf07
+Subproject commit a22eb4eb193c8f22180369791df3671e1cab6f1c