diff --git a/README.md b/README.md index 739a1a6..213601e 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,8 @@ in-memory SQLite for Room, so the DAO is exercised without a device. | `CatalogViewModelTest` | paging, end of list, in-flight guard, clear on sign-out | | `MediaItemMapperTest` | local vs remote track identity, unreachable marker URI | | `RemoteStreamResolverTest` | ticket swap, local passthrough, DataSpec preserved | +| `ArtworkUrlsTest` | URL only when a cover exists, proxy prefix, invalid address | +| `ServerImageAuthInterceptorTest` | signing scope, third-party host, refresh on 401 | Fakes and the `Dispatchers.Main` rule live in `src/test/java/app/waveflow/testing/`. @@ -186,9 +188,19 @@ and a long queue would outlast it before reaching its last tracks. A player and one queue mechanism. Listing endpoints return a bare array — no total, no cursor — so the end of a -list is inferred from a page shorter than requested. Cover art is not shown: -the v2 API exposes an `artwork_hash` but no endpoint serving the image; only -the Subsonic facade does, behind its own separate credential. +list is inferred from a page shorter than requested. + +Cover art comes from `/api/v2/artwork/{artwork_hash}`, behind the same bearer as +the rest of the native API. Coil knows nothing about the session, so an +interceptor signs those requests — and only those: an origin other than the +connected server's (scheme, host and port) is never handed the token. + +The URL is keyed on the hash rather than on the entity id, which the endpoint +would also accept. The hash names the content, so replacing a cover changes the +URL and the stale image is not served from cache for a day; and an album and its +tracks share one hash, hence one cache entry and one download instead of one per +row. No hash means no cover, and no URL — otherwise every coverless row would +cost a 404. Sign-in posts to `/api/v2/auth/login` with the device model as the session name, so the server lists it among the account's devices. The access token diff --git a/app/src/main/java/app/waveflow/WaveFlowApp.kt b/app/src/main/java/app/waveflow/WaveFlowApp.kt index 20edd99..79547c1 100644 --- a/app/src/main/java/app/waveflow/WaveFlowApp.kt +++ b/app/src/main/java/app/waveflow/WaveFlowApp.kt @@ -8,11 +8,15 @@ import app.waveflow.data.MusicRepository import app.waveflow.data.PlaylistRepository import app.waveflow.data.RoomPlaylistRepository import app.waveflow.data.local.WaveFlowDatabase +import coil.ImageLoader +import coil.ImageLoaderFactory +import okhttp3.OkHttpClient import app.waveflow.data.remote.CatalogRepository import app.waveflow.data.remote.DataStoreSessionStore import app.waveflow.data.remote.HttpCatalogApi import app.waveflow.data.remote.HttpServerApi import app.waveflow.data.remote.ServerHttp +import app.waveflow.data.remote.ServerImageAuthInterceptor import app.waveflow.data.remote.ServerSessionRepository import app.waveflow.playback.Media3PlaybackController import app.waveflow.playback.PlaybackController @@ -28,7 +32,7 @@ import kotlinx.coroutines.launch * un simple conteneur suffit tant que le graphe reste petit. On migrera vers * Hilt quand le nombre de dépendances le justifiera. */ -class WaveFlowApp : Application() { +class WaveFlowApp : Application(), ImageLoaderFactory { lateinit var container: AppContainer private set @@ -37,6 +41,21 @@ class WaveFlowApp : Application() { container = AppContainer(this) container.restoreServerSession() } + + /** + * Chargeur d'images unique, partagé par les pochettes locales et distantes. + * + * Les secondes viennent de `/api/v2/artwork/`, derrière le même jeton que le + * reste de l'API : Coil ne connaît rien de la session, c'est l'intercepteur + * qui la lui apporte. + */ + override fun newImageLoader(): ImageLoader = ImageLoader.Builder(this) + .okHttpClient { + OkHttpClient.Builder() + .addInterceptor(ServerImageAuthInterceptor(container.serverSessionRepository)) + .build() + } + .build() } /** Conteneur d'objets partagés à l'échelle de l'application. */ diff --git a/app/src/main/java/app/waveflow/data/remote/ArtworkUrls.kt b/app/src/main/java/app/waveflow/data/remote/ArtworkUrls.kt new file mode 100644 index 0000000..3f48903 --- /dev/null +++ b/app/src/main/java/app/waveflow/data/remote/ArtworkUrls.kt @@ -0,0 +1,44 @@ +package app.waveflow.data.remote + +import android.net.Uri +import androidx.core.net.toUri + +/** + * Construit les adresses de pochettes d'un serveur. + * + * L'adresse est bâtie sur le **hachage** de la pochette, et non sur + * l'identifiant de l'entité qui la porte — `/api/v2/artwork/` accepte les deux. + * Le hachage désigne le contenu, ce qui donne deux propriétés que l'identifiant + * n'a pas : + * + * - remplacer une jaquette change le hachage, donc l'adresse, donc la clé de + * cache. Sur l'identifiant, l'ancienne image resterait affichée aussi + * longtemps que le cache la garde — le serveur annonce `max-age=86400` ; + * - un album et ses pistes portent le même hachage. Une seule entrée de cache + * et un seul téléchargement, là où l'identifiant en aurait produit autant que + * de lignes affichées. + * + * Une entité sans hachage n'a pas de pochette : aucune adresse n'est produite. + * En construire une coûterait un aller-retour par ligne pour un 404 à chaque + * fois, à chaque défilement. + */ +class ArtworkUrls(private val serverUrl: String, private val http: ServerHttp) { + + fun forHash(artworkHash: String?): Uri? { + if (artworkHash.isNullOrBlank()) return null + + // Le hachage est un segment à part entière, et non interpolé : il vient + // d'une réponse serveur, et un `/` qui s'y glisserait désignerait un + // autre point d'API. + // + // Construite en plein rendu d'une liste : une adresse de serveur + // invalide doit coûter une vignette, pas l'écran entier. + return runCatching { + http.absoluteUrl(serverUrl, "/$PATH", pathSegment = artworkHash).toUri() + }.getOrNull() + } + + private companion object { + const val PATH = "api/v2/artwork" + } +} diff --git a/app/src/main/java/app/waveflow/data/remote/Dto.kt b/app/src/main/java/app/waveflow/data/remote/Dto.kt index e6edb48..64f09dc 100644 --- a/app/src/main/java/app/waveflow/data/remote/Dto.kt +++ b/app/src/main/java/app/waveflow/data/remote/Dto.kt @@ -60,13 +60,15 @@ internal data class AlbumResponse( val artist: String? = null, @SerialName("artist_id") val artistId: String? = null, val year: Int? = null, + @SerialName("artwork_hash") val artworkHash: String? = null, ) { - fun toModel() = RemoteAlbum( + fun toModel(artwork: ArtworkUrls) = RemoteAlbum( id = id, title = title, artist = artist, artistId = artistId, year = year, + artworkUri = artwork.forHash(artworkHash), ) } @@ -75,8 +77,14 @@ internal data class ArtistResponse( val id: String, val name: String, @SerialName("album_count") val albumCount: Int? = null, + @SerialName("artwork_hash") val artworkHash: String? = null, ) { - fun toModel() = RemoteArtist(id = id, name = name, albumCount = albumCount) + fun toModel(artwork: ArtworkUrls) = RemoteArtist( + id = id, + name = name, + albumCount = albumCount, + artworkUri = artwork.forHash(artworkHash), + ) } @Serializable @@ -88,8 +96,9 @@ internal data class SongResponse( val artist: String? = null, val track: Int? = null, @SerialName("duration_ms") val durationMs: Long, + @SerialName("artwork_hash") val artworkHash: String? = null, ) { - fun toModel() = RemoteSong( + fun toModel(artwork: ArtworkUrls) = RemoteSong( id = id, title = title, album = album, @@ -97,6 +106,7 @@ internal data class SongResponse( artist = artist, trackNumber = track, durationMs = durationMs, + artworkUri = artwork.forHash(artworkHash), ) } @@ -107,10 +117,11 @@ internal data class AlbumDetailResponse( val artist: String? = null, @SerialName("artist_id") val artistId: String? = null, val year: Int? = null, + @SerialName("artwork_hash") val artworkHash: String? = null, val songs: List = emptyList(), ) { val album: AlbumResponse - get() = AlbumResponse(id, title, artist, artistId, year) + get() = AlbumResponse(id, title, artist, artistId, year, artworkHash) } /** `{"url": "/api/v2/stream/", "expires_at": }` — l'URL est relative. */ @@ -125,8 +136,9 @@ internal data class ArtistDetailResponse( val id: String, val name: String, @SerialName("album_count") val albumCount: Int? = null, + @SerialName("artwork_hash") val artworkHash: String? = null, val albums: List = emptyList(), ) { val artist: ArtistResponse - get() = ArtistResponse(id, name, albumCount) + get() = ArtistResponse(id, name, albumCount, artworkHash) } diff --git a/app/src/main/java/app/waveflow/data/remote/HttpCatalogApi.kt b/app/src/main/java/app/waveflow/data/remote/HttpCatalogApi.kt index 5c1a9e8..5439172 100644 --- a/app/src/main/java/app/waveflow/data/remote/HttpCatalogApi.kt +++ b/app/src/main/java/app/waveflow/data/remote/HttpCatalogApi.kt @@ -21,7 +21,7 @@ class HttpCatalogApi( path = ALBUMS, query = pageQuery(offset, limit), accessToken = accessToken, - ).decode>().map { it.toModel() } + ).decode>().map { it.toModel(artwork(serverUrl)) } override suspend fun artists( serverUrl: String, @@ -33,7 +33,7 @@ class HttpCatalogApi( path = ARTISTS, query = pageQuery(offset, limit), accessToken = accessToken, - ).decode>().map { it.toModel() } + ).decode>().map { it.toModel(artwork(serverUrl)) } override suspend fun album( serverUrl: String, @@ -45,11 +45,12 @@ class HttpCatalogApi( pathSegment = albumId, accessToken = accessToken, ).decode().let { response -> + val artwork = artwork(serverUrl) RemoteAlbumDetail( - album = response.album.toModel(), + album = response.album.toModel(artwork), // Le serveur ne garantit pas l'ordre des morceaux d'un album ; // le numéro de piste, lui, est ce que l'utilisateur attend. - songs = response.songs.map { it.toModel() }.sortedWith(BY_TRACK_THEN_TITLE), + songs = response.songs.map { it.toModel(artwork) }.sortedWith(BY_TRACK_THEN_TITLE), ) } @@ -63,9 +64,10 @@ class HttpCatalogApi( pathSegment = artistId, accessToken = accessToken, ).decode().let { response -> + val artwork = artwork(serverUrl) RemoteArtistDetail( - artist = response.artist.toModel(), - albums = response.albums.map { it.toModel() }, + artist = response.artist.toModel(artwork), + albums = response.albums.map { it.toModel(artwork) }, ) } @@ -84,6 +86,8 @@ class HttpCatalogApi( return http.absoluteUrl(serverUrl, ticket.url) } + private fun artwork(serverUrl: String) = ArtworkUrls(serverUrl, http) + private fun pageQuery(offset: Int, limit: Int) = mapOf( "offset" to offset.toString(), "limit" to limit.toString(), diff --git a/app/src/main/java/app/waveflow/data/remote/ServerHttp.kt b/app/src/main/java/app/waveflow/data/remote/ServerHttp.kt index 2d60095..64568cf 100644 --- a/app/src/main/java/app/waveflow/data/remote/ServerHttp.kt +++ b/app/src/main/java/app/waveflow/data/remote/ServerHttp.kt @@ -119,12 +119,7 @@ class ServerHttp( pathSegment: String?, query: Map, ): HttpUrl { - val trimmed = trim().trimEnd('/') - if (trimmed.isEmpty()) throw ServerException.Rejected("Adresse du serveur vide.") - - val absolute = if (trimmed.contains("://")) trimmed else "https://$trimmed" - val base = absolute.toHttpUrlOrNull() - ?: throw ServerException.Rejected("Adresse du serveur invalide : $this") + val base = parseBase(this) return base.newBuilder() .addPathSegments(path) @@ -145,14 +140,17 @@ class ServerHttp( * Seul un chemin absolu du serveur est accepté. Une URL complète ou une * référence réseau (`//hôte/…`) désignerait un autre hôte que celui où * l'utilisateur s'est authentifié. + * + * @param pathSegment ajouté après [path], et encodé — un identifiant ou un + * hachage venu d'une réponse n'a pas à être interpolé dans le chemin. */ - fun absoluteUrl(serverUrl: String, path: String): String { + fun absoluteUrl(serverUrl: String, path: String, pathSegment: String? = null): String { if (!path.startsWith("/") || path.startsWith("//")) { throw ServerException.Unexpected("Chemin de diffusion inattendu : $path") } return serverUrl - .toApiUrl(path = path.removePrefix("/"), pathSegment = null, query = emptyMap()) + .toApiUrl(path = path.removePrefix("/"), pathSegment = pathSegment, query = emptyMap()) .toString() } @@ -173,6 +171,22 @@ class ServerHttp( } companion object { + /** + * Normalise l'adresse saisie par l'utilisateur. + * + * Le schéma est le seul ajout : `192.168.1.10:4533` seul n'est pas une + * URL pour OkHttp alors que c'est ce qu'on tape. Exposée pour que la + * signature des requêtes d'images compare le même hôte que les appels. + */ + fun parseBase(serverUrl: String): HttpUrl { + val trimmed = serverUrl.trim().trimEnd('/') + if (trimmed.isEmpty()) throw ServerException.Rejected("Adresse du serveur vide.") + + val absolute = if (trimmed.contains("://")) trimmed else "https://$trimmed" + return absolute.toHttpUrlOrNull() + ?: throw ServerException.Rejected("Adresse du serveur invalide : $serverUrl") + } + private val JSON_MEDIA_TYPE = "application/json; charset=utf-8".toMediaType() /** diff --git a/app/src/main/java/app/waveflow/data/remote/ServerImageAuth.kt b/app/src/main/java/app/waveflow/data/remote/ServerImageAuth.kt new file mode 100644 index 0000000..bf92624 --- /dev/null +++ b/app/src/main/java/app/waveflow/data/remote/ServerImageAuth.kt @@ -0,0 +1,77 @@ +package app.waveflow.data.remote + +import app.waveflow.model.ServerSession +import kotlinx.coroutines.runBlocking +import okhttp3.Interceptor +import okhttp3.Response + +/** + * Porte le jeton de session sur les requêtes d'images du serveur. + * + * `/api/v2/artwork/` exige le même Bearer que le reste de l'API native, et + * Coil ne connaît rien de la session : cet intercepteur l'ajoute pour lui. + * + * Seules les requêtes vers l'origine du serveur connecté sont signées. Une + * pochette locale — un `content://` — ne passe pas par OkHttp, mais une + * jaquette venue d'ailleurs pourrait ; lui joindre le jeton reviendrait à le + * confier à un tiers. + * + * L'appel est bloquant : les intercepteurs OkHttp le sont, et s'exécutent sur + * ses propres fils. + */ +class ServerImageAuthInterceptor( + private val sessionRepository: ServerSessionRepository, +) : Interceptor { + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val session = sessionRepository.session.value as? ServerSession.Connected + ?: return chain.proceed(request) + + if (!request.url.isSameOriginAs(session.serverUrl)) return chain.proceed(request) + + val token = runCatching { runBlocking { sessionRepository.validAccessToken() } } + .getOrNull() + ?: return chain.proceed(request) + + val signed = chain.proceed(request.withBearer(token)) + if (signed.code != HTTP_UNAUTHORIZED) return signed + + // Jeton révoqué ailleurs : l'horloge locale le croyait bon. Comme pour + // le catalogue, on le périme et on rejoue une fois — un second refus + // veut dire que la session est fermée, et la pochette manquera. + signed.close() + val renewed = runCatching { + runBlocking { + sessionRepository.expireAccessToken() + sessionRepository.validAccessToken() + } + }.getOrNull() ?: return chain.proceed(request) + + return chain.proceed(request.withBearer(renewed)) + } + + private fun okhttp3.Request.withBearer(token: String) = + newBuilder().header("Authorization", "Bearer $token").build() + + /** + * Compare l'origine — schéma, hôte et port — à celle du serveur connecté. + * + * Le schéma compte autant que le reste : un serveur joint en HTTPS et une + * adresse en `http://` vers le même hôte et le même port enverraient le + * jeton en clair. Les ports par défaut suffisent à les distinguer quand + * ils sont implicites, pas quand le port est explicite — ce qui est le cas + * courant d'un serveur auto-hébergé. + * + * L'adresse saisie par l'utilisateur passe par la même normalisation que + * les appels d'API : sans schéma, elle est jointe en HTTPS. + */ + private fun okhttp3.HttpUrl.isSameOriginAs(serverUrl: String): Boolean { + val server = runCatching { ServerHttp.parseBase(serverUrl) }.getOrNull() ?: return false + return scheme == server.scheme && host == server.host && port == server.port + } + + private companion object { + const val HTTP_UNAUTHORIZED = 401 + } +} diff --git a/app/src/main/java/app/waveflow/model/RemoteCatalog.kt b/app/src/main/java/app/waveflow/model/RemoteCatalog.kt index ad9dcda..26ca2e5 100644 --- a/app/src/main/java/app/waveflow/model/RemoteCatalog.kt +++ b/app/src/main/java/app/waveflow/model/RemoteCatalog.kt @@ -1,5 +1,7 @@ package app.waveflow.model +import android.net.Uri + /** * Le catalogue d'un serveur WaveFlow. * @@ -9,6 +11,9 @@ package app.waveflow.model * distante est la même qu'une piste locale — la RFC-003 du serveur renvoie * explicitement cette réconciliation à un jalon ultérieur. Fusionner les deux * modèles maintenant reviendrait à préjuger de ce travail. + * + * `artworkUri` est nul quand l'entité n'a pas de pochette. Sa construction + * appartient à la couche de données ; l'affichage n'a qu'à la charger. */ data class RemoteAlbum( val id: String, @@ -16,13 +21,14 @@ data class RemoteAlbum( val artist: String?, val artistId: String?, val year: Int?, + val artworkUri: Uri?, ) data class RemoteArtist( val id: String, val name: String, - /** Connu depuis la liste, absent du détail : le serveur ne le renvoie pas. */ val albumCount: Int?, + val artworkUri: Uri?, ) data class RemoteSong( @@ -33,6 +39,7 @@ data class RemoteSong( val artist: String?, val trackNumber: Int?, val durationMs: Long, + val artworkUri: Uri?, ) /** Un album et son contenu, tels que renvoyés d'un seul appel. */ diff --git a/app/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.kt b/app/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.kt index 48aeb74..769dfb1 100644 --- a/app/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.kt +++ b/app/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.kt @@ -43,6 +43,7 @@ fun RemoteAlbumDetailScreen( ) { item { RemoteDetailHeader( + artworkUri = detail.album.artworkUri, title = detail.album.title, subtitle = detail.album.artist.orUnknownArtist(), summary = listOfNotNull( @@ -79,17 +80,16 @@ fun RemoteArtistDetailScreen( ) { item { RemoteDetailHeader( + artworkUri = detail.artist.artworkUri, title = detail.artist.name, subtitle = "Artiste", - // Le compte du serveur est absent sur ce chemin ; celui des - // albums renvoyés est ce qu'on sait vraiment. - summary = albumCountLabel(detail.albums.size), + summary = albumCountLabel(detail.artist.albumCount ?: detail.albums.size), ) } items(detail.albums, key = { it.id }) { album -> MediaRow( - artworkUri = null, + artworkUri = album.artworkUri, title = album.title, subtitle = album.year?.toString().orEmpty(), onClick = { onAlbumClick(album) }, @@ -141,12 +141,13 @@ private fun DetailContainer( */ @Composable private fun RemoteDetailHeader( + artworkUri: android.net.Uri?, title: String, subtitle: String, summary: String, ) { MediaRow( - artworkUri = null, + artworkUri = artworkUri, title = title, subtitle = listOf(subtitle, summary).filter { it.isNotBlank() }.joinToString(" · "), artworkShape = CircleShape, @@ -161,7 +162,7 @@ private fun RemoteSongRow( onClick: () -> Unit, ) { MediaRow( - artworkUri = null, + artworkUri = song.artworkUri, title = song.title, subtitle = listOfNotNull( song.artist?.takeIf { it.isNotBlank() }, diff --git a/app/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.kt b/app/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.kt index 4c89414..3389449 100644 --- a/app/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.kt +++ b/app/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.kt @@ -33,8 +33,8 @@ private enum class CatalogTab(val label: String) { * Catalogue d'un serveur connecté. * * En listes et non en grille de pochettes, contrairement aux albums locaux : - * l'API v2 n'expose aucun point d'accès aux images, une grille n'afficherait - * donc que des vignettes vides. + * le catalogue d'un serveur se parcourt volontiers par recherche et par nom, + * là où la bibliothèque de l'appareil tient dans quelques écrans. */ @Composable fun ServerCatalogScreen( @@ -113,7 +113,7 @@ private fun AlbumsTab( ) { items(state.items, key = { it.id }) { album -> MediaRow( - artworkUri = null, + artworkUri = album.artworkUri, title = album.title, subtitle = album.artist.orUnknownArtist(), onClick = { onAlbumClick(album) }, @@ -147,10 +147,8 @@ private fun ArtistsTab( ) { items(state.items, key = { it.id }) { artist -> MediaRow( - artworkUri = null, + artworkUri = artist.artworkUri, title = artist.name, - // Le serveur omet le compte sur certains chemins : mieux - // vaut une ligne sans sous-titre qu'un « 0 album » faux. subtitle = artist.albumCount?.let(::albumCountLabel).orEmpty(), onClick = { onArtistClick(artist) }, artworkShape = CircleShape, diff --git a/app/src/test/java/app/waveflow/data/remote/ArtworkUrlsTest.kt b/app/src/test/java/app/waveflow/data/remote/ArtworkUrlsTest.kt new file mode 100644 index 0000000..d132f7f --- /dev/null +++ b/app/src/test/java/app/waveflow/data/remote/ArtworkUrlsTest.kt @@ -0,0 +1,72 @@ +package app.waveflow.data.remote + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** Les adresses de pochettes, et surtout quand il ne faut pas en produire. */ +@RunWith(RobolectricTestRunner::class) +class ArtworkUrlsTest { + + private fun urls(serverUrl: String = "https://musique.test") = + ArtworkUrls(serverUrl, ServerHttp()) + + @Test + fun `une pochette donne une adresse sur son hachage`() { + val uri = urls().forHash("abc123") + + assertEquals("https://musique.test/api/v2/artwork/abc123", uri.toString()) + } + + @Test + fun `une jaquette remplacee change d'adresse`() { + // C'est tout l'intérêt du hachage : sur l'identifiant de l'entité, + // l'adresse serait identique et le cache resservirait l'ancienne image + // — le serveur annonce `max-age=86400`. + val avant = urls().forHash("abc123") + val apres = urls().forHash("def456") + + assertNotEquals(avant, apres) + } + + @Test + fun `un album et ses pistes partagent une seule adresse`() { + // Relevé sur le serveur : ils portent le même hachage. Une entrée de + // cache et un téléchargement, au lieu d'un par ligne affichée. + val album = urls().forHash("abc123") + val piste = urls().forHash("abc123") + + assertEquals(album, piste) + } + + @Test + fun `un hachage est encode et ne peut pas designer un autre point d'API`() { + val uri = urls().forHash("../stream/vole") + + assertEquals("https://musique.test/api/v2/artwork/..%2Fstream%2Fvole", uri.toString()) + } + + @Test + fun `sans pochette aucune adresse n'est produite`() { + // Une adresse produirait un 404 par ligne de liste, à chaque défilement. + assertNull(urls().forHash(null)) + assertNull(urls().forHash(" ")) + } + + @Test + fun `le prefixe de proxy est conserve`() { + val uri = urls("https://hote.test/musique").forHash("abc123") + + assertEquals("https://hote.test/musique/api/v2/artwork/abc123", uri.toString()) + } + + @Test + fun `une adresse de serveur invalide ne fait pas tomber une liste`() { + // Ces adresses sont construites en plein rendu d'une liste : mieux vaut + // une ligne sans vignette qu'une exception qui vide l'écran. + assertNull(urls(serverUrl = " ").forHash("abc123")) + } +} diff --git a/app/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.kt b/app/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.kt index 33d226a..8a52148 100644 --- a/app/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.kt +++ b/app/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.kt @@ -9,6 +9,8 @@ import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner /** * Le client du catalogue face à un serveur de test. @@ -16,7 +18,12 @@ import org.junit.Test * Les corps sont ceux relevés sur `waveflow-server` 2.0.0-beta.0 : notamment le * fait que les détails sont **aplatis** — `/albums/{id}` renvoie les champs de * l'album au premier niveau, avec `songs` à côté, et non un objet imbriqué. + * + * Robolectric depuis que le catalogue porte des adresses de pochettes : ce sont + * des `android.net.Uri`, que `ArtworkUrls` construit sans jamais lever — une + * JVM nue les rendrait donc toutes nulles, en silence. */ +@RunWith(RobolectricTestRunner::class) class HttpCatalogApiTest { private lateinit var server: MockWebServer @@ -122,6 +129,37 @@ class HttpCatalogApiTest { ) } + @Test + fun `le hachage de pochette devient une adresse, sur toutes les entites`() = runTest { + // La liste d'albums, la liste d'artistes, le détail d'un album et ses + // pistes : chacun porte son propre `artwork_hash`, et chacun doit le + // voir devenir une adresse. + server.enqueue(MockResponse().setBody(ALBUMS_WITH_ARTWORK)) + server.enqueue(MockResponse().setBody(ARTISTS_WITH_ARTWORK)) + server.enqueue(MockResponse().setBody(ALBUM_DETAIL_WITH_ARTWORK)) + + val albums = api.albums(url(), "wfa_1", 0, 50) + assertEquals("${url()}/api/v2/artwork/hash-album", albums[0].artworkUri.toString()) + // Le second n'en a pas : aucune adresse, pas une adresse vers un 404. + assertNull(albums[1].artworkUri) + + val artists = api.artists(url(), "wfa_1", 0, 50) + assertEquals("${url()}/api/v2/artwork/hash-artiste", artists[0].artworkUri.toString()) + + val detail = api.album(url(), "wfa_1", "1daf991a") + assertEquals("${url()}/api/v2/artwork/hash-album", detail.album.artworkUri.toString()) + assertEquals("${url()}/api/v2/artwork/hash-piste", detail.songs[0].artworkUri.toString()) + } + + @Test + fun `le detail d'un artiste porte les pochettes de ses albums`() = runTest { + server.enqueue(MockResponse().setBody(ARTIST_DETAIL_WITH_ARTWORK)) + + val detail = api.artist(url(), "wfa_1", "f7ba66f7") + assertEquals("${url()}/api/v2/artwork/hash-artiste", detail.artist.artworkUri.toString()) + assertEquals("${url()}/api/v2/artwork/hash-album", detail.albums[0].artworkUri.toString()) + } + @Test fun `le ticket de diffusion devient une URL absolue`() = runTest { server.enqueue(MockResponse().setBody(TICKET_BODY)) @@ -308,6 +346,67 @@ class HttpCatalogApiTest { } """.trimIndent() + /** Un album avec pochette, un sans : les deux cas dans une même page. */ + val ALBUMS_WITH_ARTWORK = """ + [ + { + "id": "1daf991a", "library_id": "l", "title": "Nuit Blanche", + "artist": "Aurore", "artwork_hash": "hash-album", + "created_at": 0, "play_count": 0 + }, + { + "id": "ecbc899a", "library_id": "l", "title": "Second Souffle", + "artist": "Aurore", "artwork_hash": null, + "created_at": 0, "play_count": 0 + } + ] + """.trimIndent() + + /** + * Un artiste avec pochette. + * + * Le serveur n'en produit aucun aujourd'hui — son scanner ne peuple pas + * `artist.artwork_hash` — mais la colonne existe, la requête la + * sélectionne, et la route artwork autorise déjà un hachage porté par un + * artiste. Figer l'absence ici ferait échouer ce test le jour où le + * scanner s'y met, pour un comportement pourtant juste. + */ + val ARTISTS_WITH_ARTWORK = """ + [ + { + "id": "f7ba66f7", "library_id": "l", "name": "Aurore", + "artwork_hash": "hash-artiste", "album_count": 2 + } + ] + """.trimIndent() + + val ALBUM_DETAIL_WITH_ARTWORK = """ + { + "id": "1daf991a", "library_id": "l", "title": "Nuit Blanche", + "artist": "Aurore", "artwork_hash": "hash-album", + "songs": [ + { + "id": "c1", "library_id": "l", "title": "Première Lueur", + "track": 1, "duration_ms": 3030, "suffix": "mp3", "size": 1, + "artwork_hash": "hash-piste", "created_at": 0 + } + ] + } + """.trimIndent() + + val ARTIST_DETAIL_WITH_ARTWORK = """ + { + "id": "f7ba66f7", "library_id": "l", "name": "Aurore", + "artwork_hash": "hash-artiste", + "albums": [ + { + "id": "1daf991a", "library_id": "l", "title": "Nuit Blanche", + "artwork_hash": "hash-album", "created_at": 0, "play_count": 0 + } + ] + } + """.trimIndent() + /** L'URL est relative au serveur, c'est ce que rend `stream-ticket`. */ val TICKET_BODY = """ {"url": "/api/v2/stream/VkdLrczM", "expires_at": 1786395364096} diff --git a/app/src/test/java/app/waveflow/data/remote/ServerImageAuthInterceptorTest.kt b/app/src/test/java/app/waveflow/data/remote/ServerImageAuthInterceptorTest.kt new file mode 100644 index 0000000..caf09f7 --- /dev/null +++ b/app/src/test/java/app/waveflow/data/remote/ServerImageAuthInterceptorTest.kt @@ -0,0 +1,169 @@ +package app.waveflow.data.remote + +import app.waveflow.model.ServerSession +import app.waveflow.testing.FakeServerApi +import app.waveflow.testing.FakeSessionStore +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * La signature des requêtes d'images. + * + * Coil ne connaît rien de la session : tout se joue dans cet intercepteur, et + * une erreur y enverrait un jeton d'accès à un hôte tiers. + */ +@RunWith(RobolectricTestRunner::class) +class ServerImageAuthInterceptorTest { + + private lateinit var server: MockWebServer + + @Before + fun setUp() { + server = MockWebServer() + server.start() + } + + @After + fun tearDown() { + server.shutdown() + } + + private fun url(): String = server.url("/").toString().trimEnd('/') + + private suspend fun sessions( + serverUrl: String = url(), + api: FakeServerApi = FakeServerApi(), + connected: Boolean = true, + ): ServerSessionRepository { + val stored = if (connected) { + ServerSession.Connected( + serverUrl = serverUrl, + username = "admin", + accessToken = "wfa_stocke", + refreshToken = "wfr_stocke", + deviceId = "appareil-1", + accessExpiresAtMs = Long.MAX_VALUE, + ) + } else { + ServerSession.Disconnected + } + + return ServerSessionRepository( + api = api, + store = FakeSessionStore(stored = stored), + deviceName = "Pixel de test", + now = { 0L }, + ).also { it.restore() } + } + + private fun clientWith(sessions: ServerSessionRepository) = OkHttpClient.Builder() + .addInterceptor(ServerImageAuthInterceptor(sessions)) + .build() + + private fun fetch(client: OkHttpClient, target: String) { + client.newCall(Request.Builder().url(target).build()).execute().close() + } + + @Test + fun `une pochette du serveur connecte porte le jeton`() = runTest { + server.enqueue(MockResponse().setBody("image")) + + fetch(clientWith(sessions()), "${url()}/api/v2/artwork/1daf991a") + + assertEquals("Bearer wfa_stocke", server.takeRequest().getHeader("Authorization")) + } + + @Test + fun `sans session rien n'est signe`() = runTest { + server.enqueue(MockResponse().setBody("image")) + + fetch(clientWith(sessions(connected = false)), "${url()}/api/v2/artwork/1daf991a") + + assertNull(server.takeRequest().getHeader("Authorization")) + } + + @Test + fun `un hote tiers ne recoit pas le jeton`() = runTest { + // Une jaquette servie ailleurs — une pochette locale distante, un cache + // d'images — ne doit pas se voir confier le jeton du serveur. + val autre = MockWebServer() + autre.start() + autre.enqueue(MockResponse().setBody("image")) + + try { + val sessions = sessions(serverUrl = url()) + fetch(clientWith(sessions), autre.url("/pochette.jpg").toString()) + + assertNull(autre.takeRequest().getHeader("Authorization")) + } finally { + autre.shutdown() + } + } + + @Test + fun `le meme hote en clair ne recoit pas le jeton d'un serveur en HTTPS`() = runTest { + // Le port ne suffit pas à distinguer les deux quand il est explicite, + // ce qui est le cas courant d'un serveur auto-hébergé : sans comparer + // le schéma, le jeton partirait en clair vers le même hôte. + server.enqueue(MockResponse().setBody("image")) + val enHttps = "https://${server.hostName}:${server.port}" + + fetch(clientWith(sessions(serverUrl = enHttps)), "${url()}/api/v2/artwork/abc123") + + assertNull(server.takeRequest().getHeader("Authorization")) + } + + @Test + fun `un jeton refuse est renouvele et la requete rejouee`() = runTest { + // Révocation depuis un autre appareil : l'horloge locale croit le jeton + // encore valide, seul le serveur sait que non. + server.enqueue(MockResponse().setResponseCode(401)) + server.enqueue(MockResponse().setBody("image")) + + val api = FakeServerApi() + fetch(clientWith(sessions(api = api)), "${url()}/api/v2/artwork/1daf991a") + + assertEquals("Bearer wfa_stocke", server.takeRequest().getHeader("Authorization")) + assertEquals("Bearer wfa_1", server.takeRequest().getHeader("Authorization")) + assertEquals(1, api.refreshCalls) + } + + @Test + fun `un second refus n'est pas rejoue indefiniment`() = runTest { + server.enqueue(MockResponse().setResponseCode(401)) + server.enqueue(MockResponse().setResponseCode(401)) + + val sessions = sessions() + val client = clientWith(sessions) + + val code = client.newCall( + Request.Builder().url("${url()}/api/v2/artwork/1daf991a").build(), + ).execute().use { it.code } + + assertEquals(401, code) + assertEquals("deux tentatives, pas plus", 2, server.requestCount) + } + + @Test + fun `la session reste utilisable apres une pochette signee`() = runTest { + // L'intercepteur bloque sur le mutex de session : s'il le gardait, tout + // appel d'API suivant se figerait. + server.enqueue(MockResponse().setBody("image")) + val sessions = sessions() + + fetch(clientWith(sessions), "${url()}/api/v2/artwork/1daf991a") + + assertEquals("wfa_stocke", runBlocking { sessions.validAccessToken() }) + } +} diff --git a/app/src/test/java/app/waveflow/testing/Fakes.kt b/app/src/test/java/app/waveflow/testing/Fakes.kt index d233adb..d60162d 100644 --- a/app/src/test/java/app/waveflow/testing/Fakes.kt +++ b/app/src/test/java/app/waveflow/testing/Fakes.kt @@ -51,6 +51,7 @@ fun remoteSong( albumId: String? = "album-$id", trackNumber: Int? = 1, durationMs: Long = 60_000L, + artworkUri: Uri? = null, ): RemoteSong = RemoteSong( id = id, title = title, @@ -59,6 +60,7 @@ fun remoteSong( artist = artist, trackNumber = trackNumber, durationMs = durationMs, + artworkUri = artworkUri, ) class FakeMusicRepository( diff --git a/app/src/test/java/app/waveflow/testing/ServerFakes.kt b/app/src/test/java/app/waveflow/testing/ServerFakes.kt index dbde707..05f7ba6 100644 --- a/app/src/test/java/app/waveflow/testing/ServerFakes.kt +++ b/app/src/test/java/app/waveflow/testing/ServerFakes.kt @@ -134,7 +134,7 @@ class FakeCatalogApi( ): RemoteAlbumDetail { record(serverUrl, accessToken, null) return RemoteAlbumDetail( - album = RemoteAlbum(albumId, "Album", null, null, null), + album = RemoteAlbum(albumId, "Album", null, null, null, null), songs = emptyList(), ) } @@ -146,7 +146,7 @@ class FakeCatalogApi( ): RemoteArtistDetail { record(serverUrl, accessToken, null) return RemoteArtistDetail( - artist = RemoteArtist(artistId, "Artiste", null), + artist = RemoteArtist(artistId, "Artiste", null, null), albums = emptyList(), ) } @@ -239,7 +239,7 @@ class PagingCatalogApi( detailFailure?.let { throw it } return RemoteAlbumDetail( album = albums.firstOrNull { it.id == albumId } - ?: RemoteAlbum(albumId, "Album", null, null, null), + ?: RemoteAlbum(albumId, "Album", null, null, null, null), songs = emptyList(), ) } @@ -253,7 +253,7 @@ class PagingCatalogApi( detailFailure?.let { throw it } return RemoteArtistDetail( artist = artists.firstOrNull { it.id == artistId } - ?: RemoteArtist(artistId, "Artiste", null), + ?: RemoteArtist(artistId, "Artiste", null, null), albums = emptyList(), ) } diff --git a/app/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt b/app/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt index 9dde7e5..0846cc2 100644 --- a/app/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt +++ b/app/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt @@ -62,7 +62,7 @@ class CatalogViewModelTest { } private fun albums(count: Int): List = - (1..count).map { RemoteAlbum("id-$it", "Album $it", "Aurore", "artiste-1", null) } + (1..count).map { RemoteAlbum("id-$it", "Album $it", "Aurore", "artiste-1", null, null) } @Test fun `une session ouverte declenche la premiere page`() =