Add an option to choose a different executor for completion handlers (#342)

By default, the main thread is used.

This required quite a bit of refactoring, because `AsyncTask` doesn’t allow running the completion on anything else than the UI thread! I documented the reasons in the `Request` interface (private documentation).

I took the opportunity to move the base asynchronous task to a top-level class. There is still a specialized class internal to `AbstractClient` to provide easy access to the client’s properties and methods.

The offline mode also had to be refactored, because now that the completion executor can be changed to a potentially different thread, or even a pool of threads, we have to synchronize the fallback logic.

Fixes #92.

Author: Clément Le Provost

algoliasearch/src/main/java/com/algolia/search/saas/AbstractClient.java

@@ -23,13 +23,14 @@
 
 package com.algolia.search.saas;
 
-import android.os.AsyncTask;
 import android.os.Build;
 import android.os.Handler;
 import android.os.Looper;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 
+import com.algolia.search.saas.helpers.HandlerExecutor;
+
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.json.JSONTokener;
@@ -49,6 +50,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.zip.GZIPInputStream;
@@ -141,12 +143,12 @@ private static class HostStatus {
      */
     private HashMap<String, String> headers = new HashMap<String, String>();
 
-    /** Handler used to execute operations on the main thread. */
-    protected Handler mainHandler = new Handler(Looper.getMainLooper());
-
     /** Thread pool used to run asynchronous requests. */
     protected ExecutorService searchExecutorService = Executors.newFixedThreadPool(4);
 
+    /** Executor used to run completion handlers. By default, runs on the main thread. */
+    protected @NonNull Executor completionExecutor = new HandlerExecutor(new Handler(Looper.getMainLooper()));
+
     protected Map<String, WeakReference<Object>> indices = new HashMap<>();
 
     // ----------------------------------------------------------------------
@@ -372,6 +374,16 @@ private List<String> getWriteHostsThatAreUp() {
         return hostsThatAreUp(writeHosts);
     }
 
+    /**
+     * Change the executor on which completion handlers are executed.
+     * By default, completion handlers are executed on the main thread.
+     *
+     * @param completionExecutor The new completion executor to use.
+     */
+    public void setCompletionExecutor(@NonNull Executor completionExecutor) {
+        this.completionExecutor = completionExecutor;
+    }
+
     // ----------------------------------------------------------------------
     // Utilities
     // ----------------------------------------------------------------------
@@ -670,127 +682,28 @@ boolean isUpOrCouldBeRetried(String host) {
     // ----------------------------------------------------------------------
 
     /**
-     * Abstract {@link Request} implementation using an `AsyncTask`.
-     * Derived classes just have to implement the {@link #run()} method.
+     * Abstract convenience implementation of {@link FutureRequest} using the client's default executors.
      */
-    abstract protected class AsyncTaskRequest implements Request {
-        /** The completion handler notified of the result. May be null if the caller omitted it. */
-        private CompletionHandler completionHandler;
-
-        /** The executor used to execute the request. */
-        private ExecutorService executorService;
-
-        private boolean finished = false;
-
-        /**
-         * The underlying asynchronous task.
-         */
-        private AsyncTask<Void, Void, APIResult> task = new AsyncTask<Void, Void, APIResult>() {
-            @Override
-            protected APIResult doInBackground(Void... params) {
-                try {
-                    return new APIResult(run());
-                } catch (AlgoliaException e) {
-                    return new APIResult(e);
-                }
-            }
-
-            @Override
-            protected void onPostExecute(APIResult result) {
-                finished = true;
-                if (completionHandler != null) {
-                    completionHandler.requestCompleted(result.content, result.error);
-                }
-            }
-
-            @Override
-            protected void onCancelled(APIResult apiResult) {
-                finished = true;
-            }
-        };
-
+    abstract protected class AsyncTaskRequest extends FutureRequest {
         /**
-         * Construct a new request with the specified completion handler, executing on the client's default executor.
+         * Construct a new request with the specified completion handler, executing on the client's search executor,
+         * and calling the completion handler on the client's completion executor.
          *
-         * @param completionHandler The completion handler to be notified of results. May be null if the caller omitted it.
+         * @param completionHandler  The completion handler to be notified of results. May be null if the caller omitted it.
          */
         protected AsyncTaskRequest(@Nullable CompletionHandler completionHandler) {
             this(completionHandler, searchExecutorService);
         }
 
         /**
-         * Construct a new request with the specified completion handler, executing on the specified executor.
-         *
-         * @param completionHandler The completion handler to be notified of results. May be null if the caller omitted it.
-         * @param executorService Executor service on which to execute the request.
-         */
-        protected AsyncTaskRequest(@Nullable CompletionHandler completionHandler, @NonNull ExecutorService executorService) {
-            this.completionHandler = completionHandler;
-            this.executorService = executorService;
-        }
-
-        /**
-         * Run this request synchronously. To be implemented by derived classes.
-         * <p>
-         * <strong>Do not call this method directly.</strong> Will be run in a background thread when calling
-         * {@link #start()}.
-         * </p>
-         *
-         * @return The request's result.
-         * @throws AlgoliaException If an error was encountered.
-         */
-        @NonNull
-        abstract protected JSONObject run() throws AlgoliaException;
-
-        /**
-         * Run this request asynchronously.
-         *
-         * @return This instance.
-         */
-        public AsyncTaskRequest start() {
-            // WARNING: Starting with Honeycomb (3.0), `AsyncTask` execution is serial, so we must force parallel
-            // execution. See <http://developer.android.com/reference/android/os/AsyncTask.html>.
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
-                task.executeOnExecutor(executorService);
-            } else {
-                task.execute();
-            }
-            return this;
-        }
-
-        /**
-         * Cancel this request.
-         * The listener will not be called after a request has been cancelled.
-         * <p>
-         * WARNING: Cancelling a request may or may not cancel the underlying network call, depending how late the
-         * cancellation happens. In other words, a cancelled request may have already been executed by the server. In any
-         * case, cancelling never carries "undo" semantics.
-         * </p>
-         */
-        @Override
-        public void cancel() {
-            // NOTE: We interrupt the task's thread to better cope with timeouts.
-            task.cancel(true /* mayInterruptIfRunning */);
-        }
-
-        /**
-         * Test if this request is still running.
-         *
-         * @return true if completed or cancelled, false if still running.
-         */
-        @Override
-        public boolean isFinished() {
-            return finished;
-        }
-
-        /**
-         * Test if this request has been cancelled.
+         * Construct a new request with the specified completion handler, executing on the specified executor, and
+         * calling the completion handler on the client's completion executor.
          *
-         * @return true if cancelled, false otherwise.
+         * @param completionHandler  The completion handler to be notified of results. May be null if the caller omitted it.
+         * @param requestExecutor    Executor on which to execute the request.
          */
-        @Override
-        public boolean isCancelled() {
-            return task.isCancelled();
+        protected AsyncTaskRequest(@Nullable CompletionHandler completionHandler, @NonNull Executor requestExecutor) {
+            super(completionHandler, requestExecutor, completionExecutor);
         }
     }
 }

algoliasearch/src/main/java/com/algolia/search/saas/FutureRequest.java

@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2012-2017 Algolia
+ * http://www.algolia.com/
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.algolia.search.saas;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.util.Log;
+
+import org.json.JSONObject;
+
+import java.util.concurrent.Callable;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.FutureTask;
+
+/**
+ * Abstract {@link Request} implementation, using a {@link java.util.concurrent.Future Future} internally.
+ * Derived classes just have to implement the {@link #run()} method.
+ */
+abstract class FutureRequest implements Request {
+    /** The completion handler notified of the result. May be null if the caller omitted it. */
+    private final @Nullable CompletionHandler completionHandler;
+
+    /** The executor used to execute the request. */
+    private final @NonNull Executor requestExecutor;
+
+    /** The executor used to execute the completion handler. */
+    private final @NonNull Executor completionExecutor;
+
+    /** The callable running the request. */
+    private Callable<APIResult> callable = new Callable<APIResult>() {
+        @Override
+        public APIResult call() throws Exception {
+            try {
+                return new APIResult(run());
+            } catch (AlgoliaException e) {
+                return new APIResult(e);
+            }
+        }
+    };
+
+    /**
+     * The future used to manage this request.
+     * Compared to the raw `Callable`, the future gives us cancellation (built-in) and completion (the overridden
+     * `done()` method).
+     */
+    private FutureTask<APIResult> task = new FutureTask<APIResult>(callable) {
+        @Override
+        protected void done() {
+            if (completionHandler == null) {
+                return;
+            }
+            try {
+                final APIResult result = get();
+                completionExecutor.execute(new Runnable() {
+                    @Override
+                    public void run() {
+                        // NOTE: Cancellation might have intervened after the request execution, but before the
+                        // completion handler has been called.
+                        if (isCancelled()) {
+                            return;
+                        }
+                        completionHandler.requestCompleted(result.content, result.error);
+                    }
+                });
+            }
+            // If the task was cancelled, the call to `get()` will throw a `CancellationException`.
+            catch (CancellationException e) {
+                // Ignore.
+            }
+            catch (InterruptedException | ExecutionException e) {
+                // Should never happen => log the error, but do not crash.
+                Log.e(this.getClass().getName(), "When processing in background", e);
+            }
+        }
+    };
+
+    /**
+     * Construct a new request with the specified completion handler, executing on the specified executor, and
+     * calling the completion handler on another executor.
+     *
+     * @param completionHandler The completion handler to be notified of results. May be null if the caller omitted it.
+     * @param requestExecutor Executor on which to execute the request.
+     * @param completionExecutor Executor on which to call the completion handler.
+     */
+    FutureRequest(@Nullable CompletionHandler completionHandler, @NonNull Executor requestExecutor, @NonNull Executor completionExecutor) {
+        this.completionHandler = completionHandler;
+        this.requestExecutor = requestExecutor;
+        this.completionExecutor = completionExecutor;
+    }
+
+    /**
+     * Run this request synchronously. To be implemented by derived classes.
+     * <p>
+     * <strong>Do not call this method directly.</strong> Will be run in a background thread when calling
+     * {@link #start()}.
+     * </p>
+     *
+     * @return The request's result.
+     * @throws AlgoliaException If an error was encountered.
+     */
+    @NonNull
+    abstract protected JSONObject run() throws AlgoliaException;
+
+    /**
+     * Run this request asynchronously.
+     *
+     * @return This instance.
+     */
+    public FutureRequest start() {
+        requestExecutor.execute(task);
+        return this;
+    }
+
+    /**
+     * Cancel this request.
+     * The listener will not be called after a request has been cancelled.
+     * <p>
+     * WARNING: Cancelling a request may or may not cancel the underlying network call, depending how late the
+     * cancellation happens. In other words, a cancelled request may have already been executed by the server. In any
+     * case, cancelling never carries "undo" semantics.
+     * </p>
+     */
+    @Override
+    public void cancel() {
+        // NOTE: We interrupt the task's thread to better cope with timeouts.
+        task.cancel(true /* mayInterruptIfRunning */);
+    }
+
+    /**
+     * Test if this request is still running.
+     *
+     * @return true if completed or cancelled, false if still running.
+     */
+    @Override
+    public boolean isFinished() {
+        return task.isDone();
+    }
+
+    /**
+     * Test if this request has been cancelled.
+     *
+     * @return true if cancelled, false otherwise.
+     */
+    @Override
+    public boolean isCancelled() {
+        return task.isCancelled();
+    }
+}

algoliasearch/src/main/java/com/algolia/search/saas/Request.java

@@ -31,6 +31,27 @@
  */
 public interface Request
 {
+    // ==============================================================================================================
+    // DISCUSSION: Why don't we use `AsyncTask` directly?
+    // --------------------------------------------------
+    // `AsyncTask` does not fit our purpose for many reasons:
+    //
+    // - `AsyncTask` has serial execution, whereas we need parallel execution. We don't want network requests to be
+    //   processed serially, especially when searching "as you type"!
+    //
+    // - Having a custom interface leads to better encapsulation. We can refactor the underlying implementation without
+    //   any breaking change.
+    //
+    // - The public API of `AsyncTask` is very verbose, and does not fit our purpose:
+    //     - `cancel(.)` has an extraneous parameter `mayInterruptIfRunning`;
+    //     - no `isFinished()` method.
+    //
+    // - `AsyncTask` does not allow calling the completion handler on another thread than the UI thread.
+    //
+    // - In theory, `AsyncTask` must be created on the UI thread, making it potentially dangerous to call API methods
+    //   from another thread.
+    // ==============================================================================================================
+
     /**
      * Cancel this request.
      * The listener will not be called after a request has been cancelled.