diff options
5 files changed, 154 insertions, 8 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index e9816e1..41b3946 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -71.37 +69.05 diff --git a/app/src/main/java/org/terst/listen/feed/FeedResolver.kt b/app/src/main/java/org/terst/listen/feed/FeedResolver.kt index 42253d3..57baa51 100644 --- a/app/src/main/java/org/terst/listen/feed/FeedResolver.kt +++ b/app/src/main/java/org/terst/listen/feed/FeedResolver.kt @@ -69,15 +69,23 @@ class FeedResolver( * text/html or application/octet-stream, and rejecting those would fail on real feeds. */ private suspend fun resolveWeb(url: String): Resolution = withContext(Dispatchers.IO) { - val body = try { - fetch(url) ?: return@withContext Resolution.Failed("That address returned nothing.") + val fetched = try { + fetchPage(url) ?: return@withContext Resolution.Failed("That address returned nothing.") } catch (e: IOException) { return@withContext Resolution.Failed(e.message ?: "Could not reach that address.") } - if (looksLikeFeed(body)) return@withContext Resolution.Feed(url) + // The URL after redirects, not the one we asked for. A share link from another app is a + // redirect to the real feed, and storing the share link makes a third party's URL + // load-bearing: retire it and the subscription silently stops updating even though the + // publisher never moved. + val (body, effectiveUrl) = fetched - SubscriptionResolver.findFeedLink(body, url)?.let { return@withContext Resolution.Feed(it) } + if (looksLikeFeed(body)) return@withContext Resolution.Feed(effectiveUrl) + + // Relative links resolve against where the page actually was. + SubscriptionResolver.findFeedLink(body, effectiveUrl) + ?.let { return@withContext Resolution.Feed(it) } Resolution.Failed( "That's a web page, not a podcast feed, and it doesn't link to one. Look for an " + @@ -93,13 +101,18 @@ class FeedResolver( } /** Returns the body, or throws IOException with a message already fit to show a person. */ - private fun fetch(url: String): String? { + private fun fetch(url: String): String? = fetchPage(url)?.first + + /** Body plus the URL it actually came from, which differs whenever a redirect was followed. */ + private fun fetchPage(url: String): Pair<String, String>? { val request = Request.Builder().url(url).header("User-Agent", USER_AGENT).build() client.newCall(request).execute().use { response -> if (!response.isSuccessful) { throw IOException("That address returned HTTP ${response.code}.") } - return response.body?.string() + val body = response.body?.string() ?: return null + // OkHttp follows redirects itself; request.url on the response is where it ended up. + return body to response.request.url.toString() } } diff --git a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt new file mode 100644 index 0000000..cfad449 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt @@ -0,0 +1,89 @@ +package org.terst.listen.playback + +import android.os.Bundle +import androidx.annotation.OptIn +import androidx.media3.common.Player +import androidx.media3.common.util.UnstableApi +import androidx.media3.session.CommandButton +import androidx.media3.session.MediaSession +import androidx.media3.session.SessionCommand +import androidx.media3.session.SessionResult +import com.google.common.util.concurrent.Futures +import com.google.common.util.concurrent.ListenableFuture + +/** + * Seek buttons for the notification, lock screen and Bluetooth controls. + * + * Media3's default layout offers previous/next *track*, which is right for an album and wrong + * for a podcast: with a single item loaded there is no next track, so the notification showed a + * previous button and nothing else. What a listener actually wants there is skip-back and + * skip-forward by seconds, and those are not default buttons -- they have to be published as + * custom commands. + * + * The seek amounts themselves live on the player ([PlaybackService] sets them), so these buttons + * and the in-app bar cannot drift apart. + */ +@OptIn(UnstableApi::class) +object NotificationControls { + + const val ACTION_SEEK_BACK = "org.terst.listen.SEEK_BACK" + const val ACTION_SEEK_FORWARD = "org.terst.listen.SEEK_FORWARD" + + private val seekBackCommand = SessionCommand(ACTION_SEEK_BACK, Bundle.EMPTY) + private val seekForwardCommand = SessionCommand(ACTION_SEEK_FORWARD, Bundle.EMPTY) + + /** + * Media3 ships numbered icons for 5, 10, 15 and 30 seconds only. + * + * Back is 15, so it gets the numbered icon. Forward is 25 and there is no 25 icon, so it + * gets the plain arrow rather than one captioned with a number the button doesn't do. + */ + fun customLayout(): List<CommandButton> = listOf( + CommandButton.Builder() + .setDisplayName("Back 15 seconds") + .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_back_15) + .setSessionCommand(seekBackCommand) + .build(), + CommandButton.Builder() + .setDisplayName("Forward 25 seconds") + .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_forward) + .setSessionCommand(seekForwardCommand) + .build(), + ) + + /** Grants the custom commands and runs them. Without the grant the buttons do nothing. */ + fun callback(): MediaSession.Callback = object : MediaSession.Callback { + + override fun onConnect( + session: MediaSession, + controller: MediaSession.ControllerInfo, + ): MediaSession.ConnectionResult { + val commands = MediaSession.ConnectionResult.DEFAULT_SESSION_COMMANDS.buildUpon() + .add(seekBackCommand) + .add(seekForwardCommand) + .build() + return MediaSession.ConnectionResult.AcceptedResultBuilder(session) + .setAvailableSessionCommands(commands) + .build() + } + + override fun onCustomCommand( + session: MediaSession, + controller: MediaSession.ControllerInfo, + customCommand: SessionCommand, + args: Bundle, + ): ListenableFuture<SessionResult> { + val player: Player = session.player + when (customCommand.customAction) { + ACTION_SEEK_BACK -> player.seekBack() + ACTION_SEEK_FORWARD -> player.seekForward() + // Anything else is a command we never published, so treat it as unknown + // rather than inventing a result code lint has to guess at. + else -> return Futures.immediateFuture( + SessionResult(androidx.media3.session.SessionError.ERROR_NOT_SUPPORTED) + ) + } + return Futures.immediateFuture(SessionResult(SessionResult.RESULT_SUCCESS)) + } + } +} diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt index b65e1b3..c395425 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -26,6 +26,7 @@ import org.terst.listen.data.EpisodeEntity * unit. It is mandatory for background playback and is not an alert; Listen posts no * new-episode notifications. */ +@androidx.annotation.OptIn(androidx.media3.common.util.UnstableApi::class) class PlaybackService : MediaSessionService() { private var player: ExoPlayer? = null @@ -53,7 +54,12 @@ class PlaybackService : MediaSessionService() { .build() player = exo - session = MediaSession.Builder(this, exo).build() + session = MediaSession.Builder(this, exo) + // Media3's default notification offers previous/next track, which for a single + // loaded episode means one dead button. Podcast controls are seek-by-seconds. + .setCustomLayout(NotificationControls.customLayout()) + .setCallback(NotificationControls.callback()) + .build() val episodes = AppDatabase.get(this).episodes() writer = PositionWriter(ExoPlayhead(exo), episodes) diff --git a/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt b/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt index 26d3857..7a819f9 100644 --- a/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt +++ b/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt @@ -112,6 +112,44 @@ class FeedResolverTest { assertEquals(FeedResolver.Resolution.Feed(url("/rss")), resolve(url("/podcasts/510318/up-first"))) } + /** + * A share link from another app is a redirect to the real feed. Storing the share link + * makes a third party's URL load-bearing forever: if they retire it, the subscription + * silently stops updating even though the publisher never moved. + */ + @Test + fun `a redirect resolves to the final feed url`() = runTest { + server.enqueue( + MockResponse().setResponseCode(301).setHeader("Location", url("/real-feed.xml")) + ) + server.enqueue(MockResponse().setBody(feedXml)) + + assertEquals(FeedResolver.Resolution.Feed(url("/real-feed.xml")), resolve(url("/share/abc"))) + } + + @Test + fun `a chain of redirects resolves to the last one`() = runTest { + server.enqueue(MockResponse().setResponseCode(302).setHeader("Location", url("/hop2"))) + server.enqueue(MockResponse().setResponseCode(302).setHeader("Location", url("/final.xml"))) + server.enqueue(MockResponse().setBody(feedXml)) + + assertEquals(FeedResolver.Resolution.Feed(url("/final.xml")), resolve(url("/hop1"))) + } + + /** A discovered feed link must resolve against where the page really was, not where we asked. */ + @Test + fun `a relative feed link resolves against the redirected page`() = runTest { + server.enqueue(MockResponse().setResponseCode(301).setHeader("Location", url("/shows/one"))) + server.enqueue( + MockResponse().setHeader("Content-Type", "text/html").setBody( + """<!DOCTYPE html><html><head> + <link rel="alternate" type="application/rss+xml" href="rss.xml"></head></html>""" + ) + ) + + assertEquals(FeedResolver.Resolution.Feed(url("/shows/rss.xml")), resolve(url("/go/abc"))) + } + @Test fun `a 404 reports the status rather than a parse error`() = runTest { server.enqueue(MockResponse().setResponseCode(404)) |