feat(serveur): parcourir le catalogue distant - #16
Conversation
Deuxième étape de la source distante : l'onglet Serveur affiche le catalogue
une fois connecté — albums, artistes, et ce que chacun contient. La lecture
n'est pas branchée, un morceau distant ne fait donc encore rien.
Le transport HTTP est extrait de HttpServerApi vers ServerHttp. Deux clients
partagent désormais la construction d'URL et surtout le classement des erreurs :
les laisser diverger reviendrait à répondre différemment à la même panne.
Le catalogue a ses propres types. Ses identifiants sont des UUID, pas des
entiers MediaStore, et rien ne permet aujourd'hui d'affirmer qu'une piste
distante est le même fichier qu'une piste locale — la RFC-003 du serveur renvoie
cette réconciliation à un jalon ultérieur. Fusionner les modèles maintenant
préjugerait de ce travail.
Trois particularités du serveur, relevées sur une instance réelle :
- les listes renvoient un tableau nu, sans total ni curseur : la fin se déduit
d'une page plus courte que demandée ;
- les détails sont aplatis — `/albums/{id}` rend les champs de l'album au
premier niveau, avec `songs` à côté, et non un objet imbriqué ;
- `album_count` est présent sur la liste des artistes et absent de leur détail.
L'écran n'affiche alors pas de sous-titre plutôt qu'un « 0 album » faux.
Un jeton peut être révoqué depuis un autre appareil : il reste valide selon
l'horloge locale et le serveur le refuse. Le dépôt le périme alors et rejoue
l'appel une fois ; un second refus n'est plus réessayé.
Pas de pochettes : l'API v2 expose un `artwork_hash` mais aucun point d'accès à
l'image. Seul le pont Subsonic les sert, derrière des identifiants distincts.
D'où des listes plutôt qu'une grille, qui n'afficherait que des vignettes vides.
Le compte quitte l'onglet, que le catalogue occupe, et devient un écran ouvert
depuis la barre du haut — d'où le découpage de ServerScreen en deux composables.
Validé contre un waveflow-server 2.0.0-beta.0 local, alimenté d'une bibliothèque
de six fichiers : listes, pagination, détails, ordre des pistes, et la reprise
après péremption du jeton.
Claude-Session: https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLe transport HTTP partagé prend en charge le catalogue distant. L’application ajoute les modèles, API, dépôt, pagination, états Compose, routes et écrans pour les albums et artistes. Les tests couvrent l’authentification, le décodage, la pagination, les erreurs et la déconnexion. ChangesCatalogue distant
Estimated code review effort: 4 (Complexe) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Utilisateur
participant MainActivity
participant CatalogViewModel
participant CatalogRepository
participant HttpCatalogApi
participant ServerHttp
Utilisateur->>MainActivity: Ouvre le catalogue serveur
MainActivity->>CatalogViewModel: Collecte les états
CatalogViewModel->>CatalogRepository: Charge une page d’albums
CatalogRepository->>HttpCatalogApi: Appel avec session valide
HttpCatalogApi->>ServerHttp: GET authentifié
ServerHttp-->>HttpCatalogApi: Réponse JSON
HttpCatalogApi-->>CatalogRepository: Albums distants
CatalogRepository-->>CatalogViewModel: État paginé
CatalogViewModel-->>MainActivity: Liste et état d’affichage
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/waveflow/data/remote/HttpCatalogApi.kt`:
- Around line 42-46: Update the album-detail request in the `HttpCatalogApi`
method to pass `albumId` as a single path segment instead of interpolating it
into the path string. Extend `ServerHttp.get` with an optional `pathSegment`
parameter and append it via `addPathSegment`, while preserving existing path
handling and callers that do not provide a segment.
In `@app/src/main/java/app/waveflow/MainActivity.kt`:
- Around line 118-129: Déplacez les collectes remoteAlbums et remoteArtists hors
du scope racine de WaveFlowRoot et lisez-les uniquement dans la destination
composable(Routes.SERVER) qui les utilise. Déplacez de même remoteAlbumDetail
dans composable(Routes.SERVER_ALBUM_DETAIL) et remoteArtistDetail dans
composable(Routes.SERVER_ARTIST_DETAIL), en supprimant leurs collectes globales
afin que les changements du catalogue ne recomposent pas le Scaffold, le NavHost
ni les autres destinations.
In `@app/src/main/java/app/waveflow/ui/navigation/WaveFlowNavigation.kt`:
- Around line 32-45: Dans l’objet de navigation, éliminez la duplication des
segments serveur en faisant dériver SERVER_ALBUM_DETAIL et SERVER_ARTIST_DETAIL
des préfixes utilisés par serverAlbumDetail et serverArtistDetail, ou
inversement. Conservez les mêmes routes générées et utilisez les constantes de
préfixe existantes afin qu’une modification du segment soit centralisée.
In `@app/src/main/java/app/waveflow/ui/server/catalog/CatalogViewModel.kt`:
- Around line 121-147: Séparez le job partagé dans CatalogViewModel en
albumDetailJob et artistDetailJob, utilisez le job correspondant dans openAlbum
et openArtist, puis annulez les deux dans clear(). Dans
app/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt:207-222,
ajoutez un test lançant openArtist("id-1") puis openAlbum("id-1") avant la fin
du premier chargement, et vérifiez que artistDetail.value.isLoading est faux et
que le détail de l’artiste est renseigné.
- Around line 64-106: Extraire la logique commune de chargement paginé de
loadMoreAlbums et loadMoreArtists dans un helper générique paramétré par l’état,
le job et la fonction repository, en conservant les contrôles d’activité,
endReached, cancellation, mise à jour de PagedList et toMessage. Ajouter sur ce
helper une suppression documentée de TooGenericExceptionCaught, puis faire
déléguer les deux méthodes publiques au helper avec leurs flux et appels
catalogRepository respectifs.
In `@app/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.kt`:
- Around line 138-145: Rends l’action de clic optionnelle dans MediaRow en
remplaçant le callback obligatoire par un callback nullable avec une valeur par
défaut nulle, puis applique Modifier.clickable uniquement lorsqu’un callback
existe. Mets à jour les appels inertes de RemoteDetailScreens.kt pour supprimer
onClick = {}, tout en conservant le comportement cliquable des lignes qui ont
réellement une action.
- Around line 100-123: Update DetailContainer so the loaded-value branch applies
the caller-provided modifier when rendering content(value), preserving that
modifier for the LazyColumn layouts in both detail screens while leaving the
loading and error branches unchanged.
In `@app/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.kt`:
- Around line 65-81: Remonte les états de liste des onglets dans
ServerCatalogScreen en les déclarant avec rememberSaveable(saver =
LazyListState.Saver), puis ajoute-les comme paramètres à AlbumsTab et ArtistsTab
et utilise-les pour leurs LazyColumn respectives. Supprime les
rememberLazyListState locaux afin que chaque onglet conserve sa position lors
des changements de tab.
In `@app/src/main/java/app/waveflow/ui/server/ServerScreen.kt`:
- Around line 78-81: Update the text displayed by ConnectedAccount in
ServerAccountScreen to remove the outdated claim that the server catalogue is
unavailable, and instead state that remote playback is not yet available. Keep
the existing account layout and navigation behavior unchanged.
In `@app/src/main/java/app/waveflow/WaveFlowApp.kt`:
- Around line 77-87: Déplace le bloc KDoc décrivant serverSessionRepository afin
qu’il soit immédiatement placé au-dessus de sa déclaration. Conserve le KDoc
distinct de serverHttp avec cette propriété, en veillant à ce que chaque
documentation soit rattachée au symbole correspondant.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4c0d540a-2514-4e0e-8077-0669646f69a5
📒 Files selected for processing (23)
README.mdapp/src/main/java/app/waveflow/MainActivity.ktapp/src/main/java/app/waveflow/WaveFlowApp.ktapp/src/main/java/app/waveflow/data/remote/CatalogApi.ktapp/src/main/java/app/waveflow/data/remote/CatalogRepository.ktapp/src/main/java/app/waveflow/data/remote/Dto.ktapp/src/main/java/app/waveflow/data/remote/HttpCatalogApi.ktapp/src/main/java/app/waveflow/data/remote/HttpServerApi.ktapp/src/main/java/app/waveflow/data/remote/ServerHttp.ktapp/src/main/java/app/waveflow/data/remote/ServerSessionRepository.ktapp/src/main/java/app/waveflow/model/RemoteCatalog.ktapp/src/main/java/app/waveflow/ui/navigation/WaveFlowNavigation.ktapp/src/main/java/app/waveflow/ui/server/ServerScreen.ktapp/src/main/java/app/waveflow/ui/server/catalog/CatalogUiState.ktapp/src/main/java/app/waveflow/ui/server/catalog/CatalogViewModel.ktapp/src/main/java/app/waveflow/ui/server/catalog/PagedListContainer.ktapp/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.ktapp/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.ktapp/src/test/java/app/waveflow/data/remote/CatalogRepositoryTest.ktapp/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.ktapp/src/test/java/app/waveflow/testing/ServerFakes.ktapp/src/test/java/app/waveflow/ui/server/ServerScreenTest.ktapp/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt
| fun loadMoreAlbums() { | ||
| val current = _albums.value | ||
| if (albumsJob?.isActive == true || current.endReached) return | ||
|
|
||
| albumsJob = viewModelScope.launch { | ||
| _albums.value = current.copy(isLoading = true, errorMessage = null) | ||
| try { | ||
| val page = catalogRepository.albums(offset = current.items.size) | ||
| _albums.value = PagedList( | ||
| items = current.items + page, | ||
| isLoading = false, | ||
| // Le serveur ne dit pas combien il en reste : une page plus | ||
| // courte que demandée est le seul signal de fin. | ||
| endReached = page.size < CATALOG_PAGE_SIZE, | ||
| ) | ||
| } catch (cancellation: CancellationException) { | ||
| throw cancellation | ||
| } catch (error: Exception) { | ||
| _albums.value = current.copy(isLoading = false, errorMessage = error.toMessage()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun loadMoreArtists() { | ||
| val current = _artists.value | ||
| if (artistsJob?.isActive == true || current.endReached) return | ||
|
|
||
| artistsJob = viewModelScope.launch { | ||
| _artists.value = current.copy(isLoading = true, errorMessage = null) | ||
| try { | ||
| val page = catalogRepository.artists(offset = current.items.size) | ||
| _artists.value = PagedList( | ||
| items = current.items + page, | ||
| isLoading = false, | ||
| endReached = page.size < CATALOG_PAGE_SIZE, | ||
| ) | ||
| } catch (cancellation: CancellationException) { | ||
| throw cancellation | ||
| } catch (error: Exception) { | ||
| _artists.value = current.copy(isLoading = false, errorMessage = error.toMessage()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
loadMoreAlbums et loadMoreArtists sont identiques à un type près.
Les deux corps ne diffèrent que par le flux ciblé et l'appel au repository. Un helper générique évite que les deux copies divergent (par exemple si la règle endReached change).
♻️ Refactorisation proposée
+ private fun <T> loadMore(
+ state: MutableStateFlow<PagedList<T>>,
+ job: Job?,
+ fetch: suspend (Int) -> List<T>,
+ ): Job? {
+ val current = state.value
+ if (job?.isActive == true || current.endReached) return job
+ return viewModelScope.launch {
+ state.value = current.copy(isLoading = true, errorMessage = null)
+ try {
+ val page = fetch(current.items.size)
+ state.value = PagedList(
+ items = current.items + page,
+ isLoading = false,
+ // Le serveur ne dit pas combien il en reste : une page plus
+ // courte que demandée est le seul signal de fin.
+ endReached = page.size < CATALOG_PAGE_SIZE,
+ )
+ } catch (cancellation: CancellationException) {
+ throw cancellation
+ } catch (error: Exception) {
+ state.value = current.copy(isLoading = false, errorMessage = error.toMessage())
+ }
+ }
+ }
+
+ fun loadMoreAlbums() {
+ albumsJob = loadMore(_albums, albumsJob) { catalogRepository.albums(offset = it) }
+ }
+
+ fun loadMoreArtists() {
+ artistsJob = loadMore(_artists, artistsJob) { catalogRepository.artists(offset = it) }
+ }Le catch (error: Exception) reste volontaire, mais detekt le signale quatre fois (lignes 81, 102, 129, 143). Un @Suppress("TooGenericExceptionCaught") documenté sur le helper concentre la dérogation en un seul point.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 81-81: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 102-102: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/app/waveflow/ui/server/catalog/CatalogViewModel.kt` around
lines 64 - 106, Extraire la logique commune de chargement paginé de
loadMoreAlbums et loadMoreArtists dans un helper générique paramétré par l’état,
le job et la fonction repository, en conservant les contrôles d’activité,
endReached, cancellation, mise à jour de PagedList et toMessage. Ajouter sur ce
helper une suppression documentée de TooGenericExceptionCaught, puis faire
déléguer les deux méthodes publiques au helper avec leurs flux et appels
catalogRepository respectifs.
Source: Linters/SAST tools
Deux défauts de comportement, le reste tenant à la mise en page et à la lisibilité. Ouvrir un album depuis la page d'un artiste annulait le chargement de cet artiste : les deux détails partageaient un job. L'écran de l'artiste, toujours dans la pile, restait alors bloqué sur son indicateur au retour. Un job par sorte de détail. Les états du catalogue étaient collectés à la racine de WaveFlowRoot : chaque page reçue recomposait le Scaffold, le NavHost et tous les écrans. Ils sont désormais lus dans les seules destinations qui les affichent. Le reste : - l'identifiant d'un détail passe en segment de chemin plutôt qu'interpolé, donc encodé — il vient d'une réponse serveur ou d'un argument de navigation, et un `/` qui s'y glisserait désignerait un autre point d'API ; - les positions de défilement des onglets sont remontées dans l'écran : l'onglet masqué quitte la composition et repartait du haut à chaque retour ; - `MediaRow` accepte un clic nul. Un `Modifier.clickable` inerte annonce la ligne comme actionnable à TalkBack et promet une navigation qui n'arrive pas ; - `DetailContainer` applique le modificateur de l'appelant à la branche chargée, qui le perdait ; - la pagination des albums et des artistes, identique à l'appel près, passe par un helper unique ; - les segments de route serveur ne sont plus écrits deux fois ; - le KDoc de la session, séparé de sa déclaration par l'ajout du transport, la rejoint ; - l'écran de compte n'annonce plus le catalogue comme à venir, il est là. Claude-Session: https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/test/java/app/waveflow/testing/ServerFakes.kt (1)
212-220: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAppliquer
gateàartists.La documentation de
gateindique que les listes attendent ce signal.artistsne fait pasgate?.await(). Les tests ne peuvent donc pas maintenir une requête artistes en vol ni vérifier la protection contre les chargements en double pour cette liste. Ajoutez l’attente aprèsartistCalls++et avantfailIfDue.Correction proposée
): List<RemoteArtist> { artistCalls++ + gate?.await() failIfDue(artistCalls) return artists.page(offset, limit) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/waveflow/testing/ServerFakes.kt` around lines 212 - 220, Update the artists method to await the optional gate immediately after incrementing artistCalls and before invoking failIfDue, so artist list requests can be held in flight consistently with the gate contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/waveflow/ui/navigation/WaveFlowNavigation.kt`:
- Around line 46-48: Update serverAlbumDetail and serverArtistDetail to apply
Uri.encode(...) to their respective albumId and artistId values before
interpolating them into the route, while leaving the destination argument
decoding behavior unchanged.
---
Outside diff comments:
In `@app/src/test/java/app/waveflow/testing/ServerFakes.kt`:
- Around line 212-220: Update the artists method to await the optional gate
immediately after incrementing artistCalls and before invoking failIfDue, so
artist list requests can be held in flight consistently with the gate contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 04f03b78-b396-4ed2-8684-6c179ec724f2
📒 Files selected for processing (13)
app/src/main/java/app/waveflow/MainActivity.ktapp/src/main/java/app/waveflow/WaveFlowApp.ktapp/src/main/java/app/waveflow/data/remote/HttpCatalogApi.ktapp/src/main/java/app/waveflow/data/remote/ServerHttp.ktapp/src/main/java/app/waveflow/ui/components/MediaRow.ktapp/src/main/java/app/waveflow/ui/navigation/WaveFlowNavigation.ktapp/src/main/java/app/waveflow/ui/server/ServerScreen.ktapp/src/main/java/app/waveflow/ui/server/catalog/CatalogViewModel.ktapp/src/main/java/app/waveflow/ui/server/catalog/RemoteDetailScreens.ktapp/src/main/java/app/waveflow/ui/server/catalog/ServerCatalogScreen.ktapp/src/test/java/app/waveflow/data/remote/HttpCatalogApiTest.ktapp/src/test/java/app/waveflow/testing/ServerFakes.ktapp/src/test/java/app/waveflow/ui/server/catalog/CatalogViewModelTest.kt
Un identifiant distant est une chaîne, contrairement aux entiers des routes locales. Un `/` qui s'y glisserait scinderait la route, qui ne correspondrait alors à aucune destination. La navigation décode d'elle-même à la lecture de l'argument, l'aller-retour est donc symétrique. Aligne aussi `PagingCatalogApi.artists` sur `albums` : le portail retenait les albums et pas les artistes, ce qui rendait son contrat faux pour un futur test de concurrence sur les artistes. Claude-Session: https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
Deuxième étape de la source distante. L'onglet Serveur affiche le catalogue une fois connecté : albums, artistes, et ce que chacun contient. La lecture n'est pas branchée — un morceau distant ne fait encore rien.
Ce que le vrai serveur a imposé
J'ai alimenté un
waveflow-server2.0.0-beta.0 local avec six fichiers générés, et trois particularités du protocole ont dicté le code :Les listes renvoient un tableau nu, sans total ni curseur. La fin ne peut donc se déduire que d'une page plus courte que demandée — d'où
endReacheddansPagedListplutôt qu'un compteur.Les détails sont aplatis.
/albums/{id}rend les champs de l'album au premier niveau, avecsongsà côté, et non un objetalbumimbriqué. Les DTO de détail répètent donc les champs — un objet imbriqué n'aurait rien désérialisé.album_countest présent sur la liste des artistes et absent de leur détail. L'écran n'affiche alors pas de sous-titre, plutôt qu'un « 0 album » faux.Pas de pochettes, et ce n'est pas un oubli
L'API v2 expose un
artwork_hashmais aucun point d'accès à l'image. Seul le pont Subsonic les sert, derrière des identifiants distincts que l'utilisateur devrait configurer à part. J'annonçais le contraire au tour précédent — vérification faite, c'est faux.Conséquence sur l'UI : des listes, pas une grille de pochettes comme pour les albums locaux. Une grille n'afficherait que des vignettes vides.
Architecture
Le transport HTTP est extrait de
HttpServerApiversServerHttp. Deux clients partagent maintenant la construction d'URL et surtout le classement des erreurs ; les laisser diverger reviendrait à répondre différemment à la même panne.Le catalogue a ses propres types (
RemoteAlbum,RemoteArtist,RemoteSong). Ses identifiants sont des UUID, pas des entiers MediaStore, et rien ne permet aujourd'hui d'affirmer qu'une piste distante est le même fichier qu'une piste locale — la RFC-003 renvoie cette réconciliation à un jalon ultérieur. Fusionner les modèles maintenant préjugerait de ce travail.Un jeton peut être révoqué depuis un autre appareil : il reste valide selon l'horloge locale et le serveur le refuse.
CatalogRepositoryle périme alors et rejoue l'appel une fois ; un second refus n'est plus réessayé.Le compte quitte l'onglet, que le catalogue occupe, et devient un écran ouvert depuis la barre du haut — d'où le découpage de
ServerScreenenServerSignInScreenetServerAccountScreen.Validation
./gradlew clean testDebugUnitTest assembleDebug→ BUILD SUCCESSFUL, 151 tests, 0 échec, aucun avertissement. 23 tests nouveaux.Contre le vrai serveur : listes, pagination (
offset=1&limit=1), détails, ordre des pistes, et la reprise transparente après péremption du jeton. Les corps des tests sont ceux relevés à cette occasion.Un test s'est révélé creux en cours de route : avec un dispatcher non confiné, la première page se termine avant la seconde demande, donc la garde anti-doublon n'était jamais éprouvée. Corrigé par un portail qui maintient l'appel en vol — sans la garde, ce test tombe désormais.
Limites connues
/api/v2/searchexiste, mais l'ajouter demandait de décider comment il cohabite avec la recherche locale.https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
Summary by CodeRabbit
Nouvelles fonctionnalités
Documentation