From 8b34d160dcc3d25d60b270bfbcff366a575ad1b1 Mon Sep 17 00:00:00 2001 From: ibaker Date: Tue, 2 Nov 2021 15:12:32 +0000 Subject: [PATCH] Fix implementations of MediaLibrarySessionCallback to pass `params` The documentation makes it clear this value should be propagated through to the result. PiperOrigin-RevId: 407085751 --- .../media3/demo/session/PlaybackService.kt | 4 +- .../media3/session/LibraryResult.java | 11 +++++- .../session/MediaBrowserListenerTest.java | 4 +- .../MediaLibrarySessionCallbackTest.java | 5 ++- .../session/MockMediaLibraryService.java | 39 ++++++++++--------- 5 files changed, 36 insertions(+), 27 deletions(-) diff --git a/demos/session/src/main/java/androidx/media3/demo/session/PlaybackService.kt b/demos/session/src/main/java/androidx/media3/demo/session/PlaybackService.kt index 0be0f57ac1..75fc845d3b 100644 --- a/demos/session/src/main/java/androidx/media3/demo/session/PlaybackService.kt +++ b/demos/session/src/main/java/androidx/media3/demo/session/PlaybackService.kt @@ -50,9 +50,7 @@ class PlaybackService : MediaLibraryService() { browser: MediaSession.ControllerInfo, params: LibraryParams? ): ListenableFuture> { - return Futures.immediateFuture( - LibraryResult.ofItem(MediaItemTree.getRootItem(), /* params= */ null) - ) + return Futures.immediateFuture(LibraryResult.ofItem(MediaItemTree.getRootItem(), params)) } override fun onGetItem( diff --git a/libraries/session/src/main/java/androidx/media3/session/LibraryResult.java b/libraries/session/src/main/java/androidx/media3/session/LibraryResult.java index a3a307a4c6..68ee14c7fe 100644 --- a/libraries/session/src/main/java/androidx/media3/session/LibraryResult.java +++ b/libraries/session/src/main/java/androidx/media3/session/LibraryResult.java @@ -153,7 +153,7 @@ public final class LibraryResult implements Bundleable { /** The optional parameters. */ @Nullable public final MediaLibraryService.LibraryParams params; - /** Constructs a new instance with {@link #resultCode}{@code ==}{@link #RESULT_SUCCESS}. */ + /** Creates an instance with {@link #resultCode}{@code ==}{@link #RESULT_SUCCESS}. */ public static LibraryResult ofVoid() { return new LibraryResult<>( RESULT_SUCCESS, @@ -163,6 +163,15 @@ public final class LibraryResult implements Bundleable { VALUE_TYPE_VOID); } + /** + * Creates an instance with {@link #resultCode}{@code ==}{@link #RESULT_SUCCESS} and optional + * {@link LibraryParams params}. + */ + public static LibraryResult ofVoid(@Nullable LibraryParams params) { + return new LibraryResult<>( + RESULT_SUCCESS, SystemClock.elapsedRealtime(), params, /* value= */ null, VALUE_TYPE_VOID); + } + /** * Creates an instance with a media item and {@link #resultCode}{@code ==}{@link #RESULT_SUCCESS}. * diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerTest.java index 41a6734fc3..607e54473f 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaBrowserListenerTest.java @@ -183,7 +183,7 @@ public class MediaBrowserListenerTest extends MediaControllerListenerTest { .postAndSync(() -> browser.getChildren(parentId, page, pageSize, params)) .get(TIMEOUT_MS, MILLISECONDS); assertThat(result.resultCode).isEqualTo(RESULT_SUCCESS); - assertThat(result.params).isNull(); + MediaTestUtils.assertLibraryParamsEquals(params, result.params); MediaTestUtils.assertPaginatedListHasIds( result.value, MediaBrowserConstants.GET_CHILDREN_RESULT, page, pageSize); @@ -205,7 +205,7 @@ public class MediaBrowserListenerTest extends MediaControllerListenerTest { .postAndSync(() -> browser.getChildren(parentId, page, pageSize, params)) .get(LONG_TIMEOUT_MS, MILLISECONDS); assertThat(result.resultCode).isEqualTo(RESULT_SUCCESS); - assertThat(result.params).isNull(); + MediaTestUtils.assertLibraryParamsEquals(params, result.params); assertThat(result.value).hasSize(LONG_LIST_COUNT); for (int i = 0; i < result.value.size(); i++) { diff --git a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaLibrarySessionCallbackTest.java b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaLibrarySessionCallbackTest.java index 7117a9a978..531e60d7a9 100644 --- a/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaLibrarySessionCallbackTest.java +++ b/libraries/test_session_current/src/androidTest/java/androidx/media3/session/MediaLibrarySessionCallbackTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.MILLISECONDS; import android.content.Context; +import androidx.annotation.Nullable; import androidx.media3.session.MediaLibraryService.LibraryParams; import androidx.media3.session.MediaLibraryService.MediaLibrarySession; import androidx.media3.session.MediaSession.ControllerInfo; @@ -82,11 +83,11 @@ public class MediaLibrarySessionCallbackTest { MediaLibrarySession session, ControllerInfo browser, String parentId, - LibraryParams params) { + @Nullable LibraryParams params) { assertThat(parentId).isEqualTo(testParentId); MediaTestUtils.assertLibraryParamsEquals(testParams, params); latch.countDown(); - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); } }; diff --git a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java index e42450d912..b49d2eb8cd 100644 --- a/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java +++ b/libraries/test_session_current/src/main/java/androidx/media3/session/MockMediaLibraryService.java @@ -52,6 +52,7 @@ import android.content.Context; import android.os.Bundle; import android.os.HandlerThread; import androidx.annotation.GuardedBy; +import androidx.annotation.Nullable; import androidx.media3.common.MediaItem; import androidx.media3.common.MediaMetadata; import androidx.media3.common.util.BundleableUtil; @@ -183,7 +184,7 @@ public class MockMediaLibraryService extends MediaLibraryService { @Override public ListenableFuture> onGetLibraryRoot( - MediaLibrarySession session, ControllerInfo browser, LibraryParams params) { + MediaLibrarySession session, ControllerInfo browser, @Nullable LibraryParams params) { assertLibraryParams(params); return Futures.immediateFuture(LibraryResult.ofItem(ROOT_ITEM, ROOT_PARAMS)); } @@ -218,30 +219,32 @@ public class MockMediaLibraryService extends MediaLibraryService { String parentId, int page, int pageSize, - LibraryParams params) { + @Nullable LibraryParams params) { assertLibraryParams(params); if (PARENT_ID.equals(parentId)) { return Futures.immediateFuture( LibraryResult.ofItemList( - getPaginatedResult(GET_CHILDREN_RESULT, page, pageSize), /* params= */ null)); + getPaginatedResult(GET_CHILDREN_RESULT, page, pageSize), params)); } else if (PARENT_ID_LONG_LIST.equals(parentId)) { List list = new ArrayList<>(LONG_LIST_COUNT); for (int i = 0; i < LONG_LIST_COUNT; i++) { list.add(createPlayableMediaItem(TestUtils.getMediaIdInFakeTimeline(i))); } - return Futures.immediateFuture(LibraryResult.ofItemList(list, /* params= */ null)); + return Futures.immediateFuture(LibraryResult.ofItemList(list, params)); } else if (PARENT_ID_ERROR.equals(parentId)) { return Futures.immediateFuture(LibraryResult.ofError(LibraryResult.RESULT_ERROR_BAD_VALUE)); } // Includes the case of PARENT_ID_NO_CHILDREN. - return Futures.immediateFuture( - LibraryResult.ofItemList(ImmutableList.of(), /* params= */ null)); + return Futures.immediateFuture(LibraryResult.ofItemList(ImmutableList.of(), params)); } @Override @SuppressWarnings("FutureReturnValueIgnored") public ListenableFuture> onSearch( - MediaLibrarySession session, ControllerInfo browser, String query, LibraryParams params) { + MediaLibrarySession session, + ControllerInfo browser, + String query, + @Nullable LibraryParams params) { assertLibraryParams(params); if (SEARCH_QUERY.equals(query)) { MockMediaLibraryService.this.session.notifySearchResultChanged( @@ -266,7 +269,7 @@ public class MockMediaLibraryService extends MediaLibraryService { // SEARCH_QUERY_EMPTY_RESULT and SEARCH_QUERY_ERROR will be handled here. MockMediaLibraryService.this.session.notifySearchResultChanged(browser, query, 0, params); } - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); } @Override @@ -276,21 +279,19 @@ public class MockMediaLibraryService extends MediaLibraryService { String query, int page, int pageSize, - LibraryParams params) { + @Nullable LibraryParams params) { assertLibraryParams(params); if (SEARCH_QUERY.equals(query)) { return Futures.immediateFuture( - LibraryResult.ofItemList( - getPaginatedResult(SEARCH_RESULT, page, pageSize), /* params= */ null)); + LibraryResult.ofItemList(getPaginatedResult(SEARCH_RESULT, page, pageSize), params)); } else if (SEARCH_QUERY_LONG_LIST.equals(query)) { List list = new ArrayList<>(LONG_LIST_COUNT); for (int i = 0; i < LONG_LIST_COUNT; i++) { list.add(createPlayableMediaItem(TestUtils.getMediaIdInFakeTimeline(i))); } - return Futures.immediateFuture(LibraryResult.ofItemList(list, /* params= */ null)); + return Futures.immediateFuture(LibraryResult.ofItemList(list, params)); } else if (SEARCH_QUERY_EMPTY_RESULT.equals(query)) { - return Futures.immediateFuture( - LibraryResult.ofItemList(ImmutableList.of(), /* params= */ null)); + return Futures.immediateFuture(LibraryResult.ofItemList(ImmutableList.of(), params)); } else { // SEARCH_QUERY_ERROR will be handled here. return Futures.immediateFuture(LibraryResult.ofError(LibraryResult.RESULT_ERROR_BAD_VALUE)); @@ -309,25 +310,25 @@ public class MockMediaLibraryService extends MediaLibraryService { case SUBSCRIBE_ID_NOTIFY_CHILDREN_CHANGED_TO_ALL: MockMediaLibraryService.this.session.notifyChildrenChanged( parentId, NOTIFY_CHILDREN_CHANGED_ITEM_COUNT, NOTIFY_CHILDREN_CHANGED_PARAMS); - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); case SUBSCRIBE_ID_NOTIFY_CHILDREN_CHANGED_TO_ONE: MockMediaLibraryService.this.session.notifyChildrenChanged( MediaTestUtils.getTestControllerInfo(MockMediaLibraryService.this.session), parentId, NOTIFY_CHILDREN_CHANGED_ITEM_COUNT, NOTIFY_CHILDREN_CHANGED_PARAMS); - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); case SUBSCRIBE_ID_NOTIFY_CHILDREN_CHANGED_TO_ALL_WITH_NON_SUBSCRIBED_ID: MockMediaLibraryService.this.session.notifyChildrenChanged( unsubscribedId, NOTIFY_CHILDREN_CHANGED_ITEM_COUNT, NOTIFY_CHILDREN_CHANGED_PARAMS); - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); case SUBSCRIBE_ID_NOTIFY_CHILDREN_CHANGED_TO_ONE_WITH_NON_SUBSCRIBED_ID: MockMediaLibraryService.this.session.notifyChildrenChanged( MediaTestUtils.getTestControllerInfo(MockMediaLibraryService.this.session), unsubscribedId, NOTIFY_CHILDREN_CHANGED_ITEM_COUNT, NOTIFY_CHILDREN_CHANGED_PARAMS); - return Futures.immediateFuture(LibraryResult.ofVoid()); + return Futures.immediateFuture(LibraryResult.ofVoid(params)); default: // fall out } return Futures.immediateFuture(LibraryResult.ofError(LibraryResult.RESULT_ERROR_BAD_VALUE)); @@ -354,7 +355,7 @@ public class MockMediaLibraryService extends MediaLibraryService { return Futures.immediateFuture(new SessionResult(SessionResult.RESULT_ERROR_BAD_VALUE)); } - private void assertLibraryParams(LibraryParams params) { + private void assertLibraryParams(@Nullable LibraryParams params) { synchronized (MockMediaLibraryService.class) { if (assertLibraryParams) { assertLibraryParamsEquals(expectedParams, params);