Simplify sharing intent handling

Redesign ApiFuture to be more generic and independent of other classes

app/src/main/java/org/xbmc/kore/host/HostManager.java

@@ -39,10 +39,6 @@ import org.xbmc.kore.utils.NetUtils;
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 
 /**
  * Manages XBMC Hosts
@@ -53,21 +49,6 @@ import java.util.concurrent.Future;
 public class HostManager {
 	private static final String TAG = LogUtils.makeLogTag(HostManager.class);
 
-    /**
-     * A block of code that is run in the background thread and receives a
-     * reference to the current host.
-     *
-     * @see #withCurrentHost(Session)
-     */
-    public interface Session<T> {
-        T using(HostConnection host) throws Exception;
-    }
-
-    /**
-     * Provides the thread where all sessions are run.
-     */
-    private static final ExecutorService SESSIONS = Executors.newSingleThreadExecutor();
-
 	// Singleton instance
 	private static volatile HostManager instance = null;
 
@@ -121,32 +102,6 @@ public class HostManager {
 		return instance;
 	}
 
-    /**
-     * Runs a session block.
-     * <p>
-     * This method provides a context for awaiting {@link org.xbmc.kore.jsonrpc.ApiFuture
-     * future} objects returned by callback-less remote method invocations. This
-     * enables a more natural style of doing a sequence of remote calls instead
-     * of nesting or chaining callbacks.
-     *
-     * @param session The function to run
-     * @param <T> The type of the value returned by the session
-     * @return a future wrapping the value returned (or exception thrown) by the
-     * session; null when there's no current host.
-     */
-    public <T> Future<T> withCurrentHost(final Session<T> session) {
-        final HostConnection conn = getConnection();
-        if (conn != null) {
-            return SESSIONS.submit(new Callable<T>() {
-                @Override
-                public T call() throws Exception {
-                    return session.using(conn);
-                }
-            });
-        }
-        return null;
-    }
-
 	/**
 	 * Returns the current host list
 	 * @return Host list

app/src/main/java/org/xbmc/kore/jsonrpc/ApiFuture.java

@@ -9,11 +9,13 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 /**
- * A Java future wrapping the result of a Kodi remote method call.
+ * A Java future implementation, with explicit methods to complete the Future
  * <p>
- * Instantiable only through {@link HostConnection#execute(ApiMethod)}.
+ * Don't forget that a call to {@link ApiFuture#get()} blocks the current
+ * thread until it's unblocked by {@link ApiFuture#cancel(boolean)},
+ * {@link ApiFuture#complete(Object)}  or {@link ApiFuture#completeExceptionally(Throwable)}
  *
- * @param <T> The type of the result of the remote method call.
+ * @param <T> The type of the result returned by {@link ApiFuture#get()}
  */
 class ApiFuture<T> implements Future<T> {
     private enum Status { WAITING, OK, ERROR, CANCELLED }
@@ -22,38 +24,14 @@ class ApiFuture<T> implements Future<T> {
     private T ok;
     private Throwable error;
 
-    static <T> Future<T> from(HostConnection host, ApiMethod<T> method) {
+    ApiFuture() {}
-        final ApiFuture<T> future = new ApiFuture<>();
-        host.execute(method, new ApiCallback<T>() {
-            @Override
-            public void onSuccess(T result) {
-                synchronized (future.lock) {
-                    future.ok = result;
-                    future.status = Status.OK;
-                    future.lock.notifyAll();
-                }
-            }
-
-            @Override
-            public void onError(int errorCode, String description) {
-                synchronized (future.lock) {
-                    future.error = new ApiException(errorCode, description);
-                    future.status = Status.ERROR;
-                    future.lock.notifyAll();
-                }
-            }
-        }, null);
-        return future;
-    }
-
-    private ApiFuture() {}
 
     @Override
     public T get() throws InterruptedException, ExecutionException {
         try {
             return get(0, TimeUnit.MILLISECONDS);
         } catch (TimeoutException e) {
-            throw new IllegalStateException("impossible");
+            throw new IllegalStateException("Request timed out. This should not happen when time out is disabled!");
         }
     }
 
@@ -83,18 +61,26 @@ class ApiFuture<T> implements Future<T> {
         }
     }
 
-    @Override
+    private boolean setResultAndNotify(Status status, T ok, Throwable error) {
-    public boolean cancel(boolean b) {
-        if (status != Status.WAITING) {
-            return false;
-        }
         synchronized (lock) {
-            status = Status.CANCELLED;
+            if (this.status != Status.WAITING) {
-            lock.notifyAll();
+                return false;
+            }
+
+            this.status = status;
+            if (status == Status.OK) this.ok = ok;
+            if (status == Status.ERROR) this.error = error;
+
+            this.lock.notifyAll();
             return true;
         }
     }
 
+    @Override
+    public boolean cancel(boolean b) {
+        return setResultAndNotify(Status.CANCELLED, null, null);
+    }
+
     @Override
     public boolean isCancelled() {
         return status == Status.CANCELLED;
@@ -105,4 +91,21 @@ class ApiFuture<T> implements Future<T> {
         return status != Status.WAITING;
     }
 
+    /**
+     * If not already completed, sets the value returned by get() to the given value.
+     * @param value - the result value
+     * @return true if this invocation caused this CompletableFuture to transition to a completed state, else false
+     */
+    public boolean complete(T value) {
+        return setResultAndNotify(Status.OK, value, null);
+    }
+
+    /**
+     * If not already completed, causes invocations of get() to throw the given exception.
+     * @param ex = the exception
+     * @return true if this invocation caused this CompletableFuture to transition to a completed state, else false
+     */
+    public boolean completeExceptionally(Throwable ex) {
+        return setResultAndNotify(Status.ERROR, null, ex);
+    }
 }

app/src/main/java/org/xbmc/kore/jsonrpc/HostConnection.java

@@ -32,7 +32,6 @@ import com.squareup.okhttp.RequestBody;
 import com.squareup.okhttp.Response;
 
 import org.xbmc.kore.host.HostInfo;
-import org.xbmc.kore.host.HostManager;
 import org.xbmc.kore.jsonrpc.notification.Application;
 import org.xbmc.kore.jsonrpc.notification.Input;
 import org.xbmc.kore.jsonrpc.notification.Player;
@@ -354,10 +353,21 @@ public class HostConnection {
      * @return the future result of the method call. API errors will be wrapped in
      * an {@link java.util.concurrent.ExecutionException ExecutionException} like
      * regular futures.
-     * @see org.xbmc.kore.host.HostManager#withCurrentHost(HostManager.Session)
      */
     public <T> Future<T> execute(ApiMethod<T> method) {
-        return ApiFuture.from(this, method);
+        final ApiFuture<T> future = new ApiFuture<>();
+        execute(method, new ApiCallback<T>() {
+            @Override
+            public void onSuccess(T result) {
+                future.complete(result);
+            }
+
+            @Override
+            public void onError(int errorCode, String description) {
+                future.completeExceptionally(new ApiException(errorCode, description));
+            }
+        }, null);
+        return future;
     }
 
     /**

app/src/main/java/org/xbmc/kore/ui/sections/remote/OpenSharedUrl.java

@@ -5,7 +5,6 @@ package org.xbmc.kore.ui.sections.remote;
  */
 
 import org.xbmc.kore.R;
-import org.xbmc.kore.host.HostManager;
 import org.xbmc.kore.jsonrpc.HostConnection;
 import org.xbmc.kore.jsonrpc.method.Player;
 import org.xbmc.kore.jsonrpc.method.Playlist;
@@ -14,17 +13,17 @@ import org.xbmc.kore.jsonrpc.type.PlaylistType;
 import org.xbmc.kore.utils.LogUtils;
 
 import java.util.List;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 
 /**
  * Sends a series of commands to Kodi in a background thread to open the video.
  * <p>
- * This is meant to be passed to {@link HostManager#withCurrentHost(HostManager.Session)}
+ * This is meant to be passed to {@link java.util.concurrent.Executor}
  * and the resulting future should be awaited in a background thread as well (if you're
- * interested in the result), either in an {@link android.os.AsyncTask} or another
+ * interested in the result).
- * {@link HostManager.Session}.
  */
-public class OpenSharedUrl implements HostManager.Session<Boolean> {
+public class OpenSharedUrl implements Callable<Boolean> {
 
     /**
      * Indicates the stage where the error happened.
@@ -39,6 +38,7 @@ public class OpenSharedUrl implements HostManager.Session<Boolean> {
     }
 
     private static final String TAG = LogUtils.makeLogTag(OpenSharedUrl.class);
+    private final HostConnection hostConnection;
     private final String pluginUrl;
     private final String notificationTitle;
     private final String notificationText;
@@ -50,14 +50,14 @@ public class OpenSharedUrl implements HostManager.Session<Boolean> {
      * @param notificationText The notification to be shown when the host is currently
      *                         playing a video
      */
-    public OpenSharedUrl(String pluginUrl, String notificationTitle, String notificationText) {
+    public OpenSharedUrl(HostConnection hostConnection, String pluginUrl, String notificationTitle, String notificationText) {
+        this.hostConnection = hostConnection;
         this.pluginUrl = pluginUrl;
         this.notificationTitle = notificationTitle;
         this.notificationText = notificationText;
     }
 
     /**
-     * @param host The host to send the commands to
      * @return whether the host is currently playing a video. If so, the shared url
      * is added to the playlist and not played immediately.
      * @throws Error when any of the commands sent fails
@@ -65,11 +65,12 @@ public class OpenSharedUrl implements HostManager.Session<Boolean> {
      * future while waiting on one of the internal futures.
      */
     @Override
-    public Boolean using(HostConnection host) throws Error, InterruptedException {
+    public Boolean call() throws Error, InterruptedException {
         int stage = R.string.error_get_active_player;
         try {
             List<PlayerType.GetActivePlayersReturnType> players =
-                    host.execute(new Player.GetActivePlayers()).get();
+                    hostConnection.execute(new Player.GetActivePlayers())
+                        .get();
             boolean videoIsPlaying = false;
             for (PlayerType.GetActivePlayersReturnType player : players) {
                 if (player.type.equals(PlayerType.GetActivePlayersReturnType.VIDEO)) {
@@ -81,21 +82,23 @@ public class OpenSharedUrl implements HostManager.Session<Boolean> {
             stage = R.string.error_queue_media_file;
             if (!videoIsPlaying) {
                 LogUtils.LOGD(TAG, "Clearing video playlist");
-                host.execute(new Playlist.Clear(PlaylistType.VIDEO_PLAYLISTID)).get();
+                hostConnection.execute(new Playlist.Clear(PlaylistType.VIDEO_PLAYLISTID))
+                    .get();
             }
 
             LogUtils.LOGD(TAG, "Queueing file");
             PlaylistType.Item item = new PlaylistType.Item();
             item.file = pluginUrl;
-            host.execute(new Playlist.Add(PlaylistType.VIDEO_PLAYLISTID, item)).get();
+            hostConnection.execute(new Playlist.Add(PlaylistType.VIDEO_PLAYLISTID, item))
+                .get();
 
             if (!videoIsPlaying) {
                 stage = R.string.error_play_media_file;
-                host.execute(new Player
+                hostConnection.execute(new Player.Open(Player.Open.TYPE_PLAYLIST, PlaylistType.VIDEO_PLAYLISTID))
-                        .Open(Player.Open.TYPE_PLAYLIST, PlaylistType.VIDEO_PLAYLISTID)).get();
+                    .get();
             } else {
                 // no get() to ignore the exception that will be thrown by OkHttp
-                host.execute(new Player.Notification(notificationTitle, notificationText));
+                hostConnection.execute(new Player.Notification(notificationTitle, notificationText));
             }
 
             return videoIsPlaying;

app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java

@@ -40,7 +40,6 @@ import org.xbmc.kore.R;
 import org.xbmc.kore.Settings;
 import org.xbmc.kore.host.HostConnectionObserver;
 import org.xbmc.kore.host.HostManager;
-import org.xbmc.kore.jsonrpc.HostConnection;
 import org.xbmc.kore.jsonrpc.method.Application;
 import org.xbmc.kore.jsonrpc.method.AudioLibrary;
 import org.xbmc.kore.jsonrpc.method.GUI;
@@ -64,7 +63,10 @@ import org.xbmc.kore.utils.Utils;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLEncoder;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -340,6 +342,17 @@ public class RemoteActivity extends BaseActivity
         }
     }
 
+    /**
+     * Provides the thread where the intent will be handled
+     */
+    private static ExecutorService SHARE_EXECUTOR = null;
+    private static ExecutorService getShareExecutor() {
+        if (SHARE_EXECUTOR == null) {
+            SHARE_EXECUTOR = Executors.newSingleThreadExecutor();
+        }
+        return SHARE_EXECUTOR;
+    }
+
     /**
      * Handles the intent that started this activity, namely to start playing something on Kodi
      * @param intent Start intent for the activity
@@ -372,7 +385,7 @@ public class RemoteActivity extends BaseActivity
 
         String title = getString(R.string.app_name);
         String text = getString(R.string.item_added_to_playlist);
-        pendingShare = hostManager.withCurrentHost(new OpenSharedUrl(videoUrl, title, text));
+        pendingShare = getShareExecutor().submit(new OpenSharedUrl(hostManager.getConnection(), videoUrl, title, text));
         awaitShare();
         intent.setAction(null);
     }
@@ -388,9 +401,9 @@ public class RemoteActivity extends BaseActivity
      * again when the activity is resumed and a {@link #pendingShare} exists.
      */
     private void awaitShare() {
-        awaitingShare = hostManager.withCurrentHost(new HostManager.Session<Void>() {
+        awaitingShare = getShareExecutor().submit(new Callable<Void>() {
             @Override
-            public Void using(HostConnection host) throws Exception {
+            public Void call() throws Exception {
                 try {
                     final boolean wasAlreadyPlaying = pendingShare.get();
                     pendingShare = null;
@@ -399,8 +412,9 @@ public class RemoteActivity extends BaseActivity
                         public void run() {
                             if (wasAlreadyPlaying) {
                                 Toast.makeText(RemoteActivity.this,
-                                        getString(R.string.item_added_to_playlist),
+                                    getString(R.string.item_added_to_playlist),
-                                        Toast.LENGTH_SHORT).show();
+                                    Toast.LENGTH_SHORT)
+                                    .show();
                             }
                             refreshPlaylist();
                         }
@@ -414,8 +428,8 @@ public class RemoteActivity extends BaseActivity
                         @Override
                         public void run() {
                             Toast.makeText(RemoteActivity.this,
-                                    getString(e.stage, e.getMessage()),
+                                getString(e.stage, e.getMessage()),
-                                    Toast.LENGTH_SHORT).show();
+                                Toast.LENGTH_SHORT).show();
                         }
                     });
                 } finally {