From 75f8326fe4b7d5d19a8d11a89017272a7e3a35fa Mon Sep 17 00:00:00 2001 From: Synced Synapse Date: Tue, 17 Apr 2018 19:55:26 +0100 Subject: [PATCH] Simplify sharing intent handling Redesign ApiFuture to be more generic and independent of other classes --- .../java/org/xbmc/kore/host/HostManager.java | 45 ----------- .../java/org/xbmc/kore/jsonrpc/ApiFuture.java | 75 ++++++++++--------- .../org/xbmc/kore/jsonrpc/HostConnection.java | 16 +++- .../ui/sections/remote/OpenSharedUrl.java | 31 ++++---- .../ui/sections/remote/RemoteActivity.java | 30 ++++++-- 5 files changed, 91 insertions(+), 106 deletions(-) diff --git a/app/src/main/java/org/xbmc/kore/host/HostManager.java b/app/src/main/java/org/xbmc/kore/host/HostManager.java index 0facaef..d4735d8 100644 --- a/app/src/main/java/org/xbmc/kore/host/HostManager.java +++ b/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 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. - *

- * 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 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 Future withCurrentHost(final Session session) { - final HostConnection conn = getConnection(); - if (conn != null) { - return SESSIONS.submit(new Callable() { - @Override - public T call() throws Exception { - return session.using(conn); - } - }); - } - return null; - } - /** * Returns the current host list * @return Host list diff --git a/app/src/main/java/org/xbmc/kore/jsonrpc/ApiFuture.java b/app/src/main/java/org/xbmc/kore/jsonrpc/ApiFuture.java index 04fb1b3..3909df5 100644 --- a/app/src/main/java/org/xbmc/kore/jsonrpc/ApiFuture.java +++ b/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 *

- * 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 The type of the result of the remote method call. + * @param The type of the result returned by {@link ApiFuture#get()} */ class ApiFuture implements Future { private enum Status { WAITING, OK, ERROR, CANCELLED } @@ -22,38 +24,14 @@ class ApiFuture implements Future { private T ok; private Throwable error; - static Future from(HostConnection host, ApiMethod method) { - final ApiFuture future = new ApiFuture<>(); - host.execute(method, new ApiCallback() { - @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() {} + 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 implements Future { } } - @Override - public boolean cancel(boolean b) { - if (status != Status.WAITING) { - return false; - } + private boolean setResultAndNotify(Status status, T ok, Throwable error) { synchronized (lock) { - status = Status.CANCELLED; - lock.notifyAll(); + if (this.status != Status.WAITING) { + 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 implements Future { 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); + } } diff --git a/app/src/main/java/org/xbmc/kore/jsonrpc/HostConnection.java b/app/src/main/java/org/xbmc/kore/jsonrpc/HostConnection.java index d9aea63..86afa08 100644 --- a/app/src/main/java/org/xbmc/kore/jsonrpc/HostConnection.java +++ b/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 Future execute(ApiMethod method) { - return ApiFuture.from(this, method); + final ApiFuture future = new ApiFuture<>(); + execute(method, new ApiCallback() { + @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; } /** diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/OpenSharedUrl.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/OpenSharedUrl.java index 2101b29..c015f06 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/OpenSharedUrl.java +++ b/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. *

- * 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 - * {@link HostManager.Session}. + * interested in the result). */ -public class OpenSharedUrl implements HostManager.Session { +public class OpenSharedUrl implements Callable { /** * Indicates the stage where the error happened. @@ -39,6 +38,7 @@ public class OpenSharedUrl implements HostManager.Session { } 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 { * @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 { * 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 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 { 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 - .Open(Player.Open.TYPE_PLAYLIST, PlaylistType.VIDEO_PLAYLISTID)).get(); + hostConnection.execute(new Player.Open(Player.Open.TYPE_PLAYLIST, PlaylistType.VIDEO_PLAYLISTID)) + .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; diff --git a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java b/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java index 3e75736..40acaa4 100644 --- a/app/src/main/java/org/xbmc/kore/ui/sections/remote/RemoteActivity.java +++ b/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() { + awaitingShare = getShareExecutor().submit(new Callable() { @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), - Toast.LENGTH_SHORT).show(); + getString(R.string.item_added_to_playlist), + 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()), - Toast.LENGTH_SHORT).show(); + getString(e.stage, e.getMessage()), + Toast.LENGTH_SHORT).show(); } }); } finally {