feat(serveur): se connecter à un serveur WaveFlow - #15
Conversation
Premier pas de la source distante : un onglet Serveur qui ouvre une session et la maintient. Rien du catalogue distant n'est encore affiché, et rien de la bibliothèque locale ne part nulle part — les deux sources restent séparées, d'où une section à part plutôt qu'un filtre sur les écrans existants. La connexion poste sur `/api/v2/auth/login` avec le modèle de l'appareil comme nom de session, que le serveur liste parmi les appareils du compte. Le jeton d'accès vaut un quart d'heure et se renouvelle par `/api/v2/auth/refresh`. Ce renouvellement est le point délicat : le jeton de rafraîchissement tourne à chaque usage, le serveur invalidant l'ancien dès qu'il en émet un nouveau. Deux renouvellements concurrents partiraient donc du même jeton, et le second serait refusé — une session perdue alors qu'elle était valide. Tout ce qui touche aux jetons passe par un seul mutex, et l'écriture sur disque précède la mise à jour de l'état en mémoire. Les échecs sont classés selon ce que l'utilisateur peut en faire : un refus d'identifiants demande une ressaisie, un serveur injoignable seulement de réessayer, et le renouvellement ne ferme la session que dans le premier cas. Les jetons vont dans un DataStore, protégés par le bac à sable applicatif et non par du chiffrement : `security-crypto` n'est jamais sorti d'alpha et n'est plus maintenu. Le compromis est documenté dans le code et le README. Le trafic en clair est autorisé, un serveur auto-hébergé vivant le plus souvent sur un réseau local sans certificat. On aimerait restreindre aux plages privées, mais `<domain>` n'accepte pas la notation CIDR ; une adresse saisie sans schéma est donc jointe en HTTPS. Validé contre un waveflow-server 2.0.0-beta.0 local : connexion, rotation, rejeu refusé, mauvais mot de passe, nom d'appareil vide, déconnexion, et le renouvellement automatique du dépôt. 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 (6)
📝 WalkthroughWalkthroughL’application ajoute la connexion à un serveur WaveFlow, la persistance et le renouvellement des sessions, la déconnexion, ainsi qu’une destination Compose dédiée. Le catalogue distant, le streaming et la synchronisation restent indisponibles. ChangesAuthentification serveur
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ServerScreen
participant ServerViewModel
participant ServerSessionRepository
participant HttpServerApi
participant DataStoreSessionStore
ServerScreen->>ServerViewModel: identifiants
ServerViewModel->>ServerSessionRepository: connect(...)
ServerSessionRepository->>HttpServerApi: login(...)
HttpServerApi-->>ServerSessionRepository: AuthTokens
ServerSessionRepository->>DataStoreSessionStore: write(Connected)
ServerSessionRepository-->>ServerViewModel: session connectée
ServerViewModel-->>ServerScreen: ServerUiState
Possibly related PRs
🚥 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: 18
🤖 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/HttpServerApi.kt`:
- Around line 73-80: Dans la fonction de requête de HttpServerApi, englober
également response.use { ... } dans withContext(Dispatchers.IO), afin que
body?.string() et toException() s’exécutent sur le dispatcher IO. Conserver le
traitement actuel des réponses réussies et des erreurs, ainsi que la fermeture
de la réponse via use.
- Around line 26-34: Update the HttpServerApi constructor and its WaveFlowApp
wiring so production reuses the application’s shared Coil OkHttpClient instead
of creating a default OkHttpClient instance. Configure the client with an
appropriate total callTimeout while preserving the existing JSON behavior and
ServerApi contract.
- Around line 147-160: Update the onResponse implementation in Call.await to
resume with the cancellation-handler overload, closing response via the provided
responseToClose action if cancellation occurs before delivery; preserve the
existing onFailure behavior and cancellation call handling.
In `@app/src/main/java/app/waveflow/data/remote/ServerApi.kt`:
- Around line 52-65: Update the ServerException hierarchy to accept and pass
through an optional Throwable cause to the superclass, and update its subclasses
(Unauthorized, Rejected, Unreachable, and Unexpected) accordingly. Propagate the
original IOException and SerializationException when HttpServerApi creates these
exceptions so ServerSessionRepository logging retains the underlying stack
trace.
In `@app/src/main/java/app/waveflow/data/remote/ServerSessionRepository.kt`:
- Around line 53-56: Déplace l’appel réseau api.login dans connect hors de la
section mutex.withLock, puis acquiers le mutex uniquement autour de
persist(tokens.toSession(serverUrl)). Conserve la sémantique où la dernière
connexion concurrente persistée prévaut, en t’appuyant sur la protection
existante de ServerViewModel.
In `@app/src/main/java/app/waveflow/data/remote/SessionStore.kt`:
- Line 41: Update SessionStore.read() to catch IOException, including DataStore
corruption failures, while collecting dataStore.data; return a disconnected
ServerSession instead of propagating the exception. Preserve the existing
toSession() conversion for successful reads so ServerSessionRepository.restore()
receives a valid fallback session on unreadable or corrupted storage.
In `@app/src/main/java/app/waveflow/MainActivity.kt`:
- Around line 334-341: Modifiez AudioPermissionGate pour qu’il ne remplace le
NavHost que pour les destinations nécessitant la bibliothèque locale, tout en
laissant Routes.SERVER accessible sans permission audio. Conservez ServerScreen
et son entrée dans le NavHost hors de cette restriction, sans modifier ses
callbacks existants.
In `@app/src/main/java/app/waveflow/ui/server/ServerScreen.kt`:
- Around line 167-175: Update the error message Text in the state.errorMessage
rendering block to mark it as a live/dynamic accessibility region, ensuring
TalkBack announces newly displayed connection errors while preserving the
existing styling and layout.
- Around line 144-147: Update the password field’s keyboard configuration in
ServerScreen to provide keyboardActions for ImeAction.Done, invoking the same
submission behavior as the visible confirmation button so pressing the keyboard
action completes the form.
In `@app/src/main/java/app/waveflow/ui/server/ServerViewModel.kt`:
- Around line 68-73: Update ServerViewModel.disconnect() to wrap the
sessionRepository.disconnect() call and subsequent local state update in
try/catch, handling both expected repository failures and unexpected exceptions
such as DataStore IOException so errors do not escape the viewModelScope
coroutine or crash the application.
In `@app/src/main/java/app/waveflow/WaveFlowApp.kt`:
- Around line 74-78: Update the session persistence setup around
ServerSessionRepository and DataStoreSessionStore so refresh tokens are
encrypted before being stored, using a key protected by Android Keystore.
Configure the DataStore backing file to be excluded from Android backups, while
preserving the existing repository and session-store behavior.
- Around line 81-82: Update restoreServerSession to handle failures from
serverSessionRepository.restore before launching the applicationScope coroutine:
map expected DataStoreSessionStore.read errors to ServerSession.Disconnected,
and log unexpected exceptions rather than allowing them to escape without a
CoroutineExceptionHandler.
- Line 76: Exclude datastore/session_serveur.preferences_pb from backups by
adding a domain="file" exclusion in backup_rules.xml, and matching exclusions in
both the cloud-backup and device-transfer sections of data_extraction_rules.xml.
Preserve the existing DataStoreSessionStore configuration and verify backup and
restore behavior.
In `@app/src/main/res/xml/network_security_config.xml`:
- Line 14: Remove the global cleartext permission from the base-config in the
network security configuration. Ensure the default policy requires HTTPS for all
hosts; if local-network HTTP remains necessary, make it an explicit opt-in
restricted to a strictly validated host and prevent credentials from being sent
without a user warning.
In `@app/src/test/java/app/waveflow/data/remote/HttpServerApiTest.kt`:
- Around line 93-125: Ajoutez dans HttpServerApiTest un test couvrant une
réponse HTTP 500 avec un corps d’erreur JSON, en appelant api.login via echecDe.
Vérifiez que l’erreur retournée est ServerException.Unexpected et que son
message reprend le message serveur « Database unavailable », afin de couvrir la
branche 5xx de toException.
In `@app/src/test/java/app/waveflow/testing/ServerFakes.kt`:
- Around line 41-45: Étendez FakeServerApi.refresh dans
app/src/test/java/app/waveflow/testing/ServerFakes.kt#L41-L45 pour enregistrer
refreshToken dans lastRefreshToken, utiliser le nom d’utilisateur retenu lors de
la connexion pour nextTokens, et permettre une suspension contrôlée afin de
tester la concurrence. Dans
app/src/test/java/app/waveflow/data/remote/ServerSessionRepositoryTest.kt#L116-L143,
vérifiez lastRefreshToken après le renouvellement et ajoutez deux appels
concurrents à validAccessToken() confirmant refreshCalls == 1. Dans
app/src/test/java/app/waveflow/ui/server/ServerViewModelTest.kt#L137-L148,
ajoutez un test où le stockage échoue pendant la déconnexion et vérifiez que
l’état contient un message d’erreur sans propager l’exception.
In `@app/src/test/java/app/waveflow/ui/server/ServerScreenTest.kt`:
- Around line 124-131: Remplacez l’utilisation de occurrencesDe dans le test
`aucun jeton n'est affiche a l'ecran` par une recherche par sous-chaîne afin de
détecter les jetons intégrés dans une phrase, et appliquez également cette
adaptation à l’assertion « Adresse du serveur » autour de la ligne 121 sans
modifier son résultat attendu.
In `@app/src/test/java/app/waveflow/ui/server/ServerViewModelTest.kt`:
- Around line 110-123: Corrigez le test `un echec n'empeche pas une nouvelle
tentative` pour utiliser une seule instance de `ServerViewModel` et vérifier
qu'une connexion échouée peut être suivie d'une connexion réussie. Faites
évoluer `FakeServerApi.loginFailure` pour permettre de désactiver l'échec entre
les deux appels, par exemple en le rendant mutable ou via une méthode
`acceptLogins()`, puis réutilisez le même ViewModel après l'échec.
🪄 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: b4ee003e-717c-45be-98a5-726682038957
📒 Files selected for processing (22)
README.mdapp/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/app/waveflow/MainActivity.ktapp/src/main/java/app/waveflow/WaveFlowApp.ktapp/src/main/java/app/waveflow/data/remote/Dto.ktapp/src/main/java/app/waveflow/data/remote/HttpServerApi.ktapp/src/main/java/app/waveflow/data/remote/ServerApi.ktapp/src/main/java/app/waveflow/data/remote/ServerSessionRepository.ktapp/src/main/java/app/waveflow/data/remote/SessionStore.ktapp/src/main/java/app/waveflow/model/ServerSession.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/ServerUiState.ktapp/src/main/java/app/waveflow/ui/server/ServerViewModel.ktapp/src/main/res/xml/network_security_config.xmlapp/src/test/java/app/waveflow/data/remote/HttpServerApiTest.ktapp/src/test/java/app/waveflow/data/remote/ServerSessionRepositoryTest.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/ServerViewModelTest.ktgradle/libs.versions.toml
Corrections retenues, par ordre de gravité. L'onglet Serveur était inatteignable sans la permission audio : la porte enveloppait tout le NavHost. Elle enveloppe désormais chaque écran qui lit la bibliothèque de l'appareil, pas le NavHost lui-même — le sortir de la porte l'aurait recomposé dans un autre sous-arbre et lui aurait fait perdre sa pile de navigation. Les deux effets qu'elle déclenchait sont idempotents, donc sans conséquence à être relancés par écran. Un stockage en échec pendant la déconnexion faisait tomber l'application : l'exception quittait `viewModelScope`. Même filet que dans PlaylistsViewModel. Une session illisible ou corrompue faisait de même au démarrage : `read()` rattrape l'IOException et repart déconnecté plutôt que d'empêcher le lancement. Les jetons partaient dans les sauvegardes cloud et le transfert d'appareil. Exclus des deux : stockés en clair, un jeton de rafraîchissement restauré ailleurs y ouvrirait le compte. La lecture du corps de réponse passe désormais sous le dispatcher IO avec l'appel lui-même, et une annulation survenant entre l'arrivée de la réponse et sa remise la referme au lieu de retenir la connexion. Un `callTimeout` borne l'appel entier : les délais par défaut d'OkHttp ne portent que sur chaque étape prise à part. `ServerException` transporte sa cause, pour que les journaux gardent la trace d'origine. « Terminé » au clavier valide le formulaire, et le message d'erreur devient une région active pour que TalkBack l'annonce. Côté tests, deux étaient creux et sont corrigés : la reprise après échec utilisait deux ViewModels au lieu d'un seul, et la recherche de jetons à l'écran comparait en égalité stricte, laissant passer un jeton glissé dans une phrase. S'ajoutent la branche 5xx, la rotation vérifiée sur deux renouvellements successifs, et surtout deux demandes concurrentes de jeton : sans le mutex, ce dernier tombe — la sérialisation n'était couverte par rien jusqu'ici. Écartés : sortir `api.login` du mutex, qui échangerait un invariant du dépôt contre une garde d'interface ; chiffrer les jetons par le Keystore et interdire le trafic en clair, deux arbitrages déjà documentés et qui reviennent à l'utilisateur ; partager le client OkHttp de Coil, dont la configuration ne vise pas les appels d'API. Claude-Session: https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/waveflow/ui/server/ServerViewModel.kt (1)
51-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCapturez les échecs de persistance dans
connect().
sessionRepository.connect()peut lever uneIOExceptionquandstore.write()échoue après une connexion réussie. Le bloc actuel capture seulementServerException. L’exception quitte alorsviewModelScope.Conservez la propagation de
CancellationException. Capturez aussi l’erreur de stockage, réinitialisezisConnecting, puis exposez un message utilisateur. Ajoutez un test avecFakeSessionStore(writeFailure = IOException(...))surconnect().As per path instructions, « les erreurs sont capturées (
catch) plutôt que de faire crasher la collecte ».🤖 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/ServerViewModel.kt` around lines 51 - 65, Update the connect() coroutine in ServerViewModel so it also catches IOException from sessionRepository.connect(), while continuing to rethrow CancellationException. For storage failures, reset isConnecting to false and expose an appropriate user-facing error message through local, matching the existing ServerException handling. Add a connect() test using FakeSessionStore(writeFailure = IOException(...)) to verify the failure is captured rather than escaping viewModelScope.Source: Path instructions
🤖 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/HttpServerApi.kt`:
- Around line 76-79: Update the request handling in the surrounding API method
to wrap client.newCall(request).await().use in a try/catch for body-read
IOException failures, call currentCoroutineContext().ensureActive() before
converting the error, and rethrow ServerException.Unreachable with the original
exception as cause. Add coverage using
SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY.
In `@app/src/main/java/app/waveflow/MainActivity.kt`:
- Around line 255-256: Dans la lambda composable gated de MainActivity, retirez
le paramètre modifier = Modifier.padding(innerPadding) lors de l’appel à
AudioPermissionGate, afin que le padding soit appliqué uniquement par le
conteneur Box. Vérifiez également l’appel associé signalé vers les lignes
268-272 pour garantir qu’AudioPermissionPrompt ne reçoit pas une seconde fois
les mêmes insets.
In `@README.md`:
- Around line 179-181: Update the README security statement near the token
storage description to limit the guarantee to exclusion from cloud backup and
device transfer, without implying that tokens never reach the server. Preserve
the existing explanation that signing in again asks for the password.
---
Outside diff comments:
In `@app/src/main/java/app/waveflow/ui/server/ServerViewModel.kt`:
- Around line 51-65: Update the connect() coroutine in ServerViewModel so it
also catches IOException from sessionRepository.connect(), while continuing to
rethrow CancellationException. For storage failures, reset isConnecting to false
and expose an appropriate user-facing error message through local, matching the
existing ServerException handling. Add a connect() test using
FakeSessionStore(writeFailure = IOException(...)) to verify the failure is
captured rather than escaping viewModelScope.
🪄 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: 46077527-7e80-4ac0-a2f5-611d29a18292
📒 Files selected for processing (14)
README.mdapp/src/main/java/app/waveflow/MainActivity.ktapp/src/main/java/app/waveflow/data/remote/HttpServerApi.ktapp/src/main/java/app/waveflow/data/remote/ServerApi.ktapp/src/main/java/app/waveflow/data/remote/SessionStore.ktapp/src/main/java/app/waveflow/ui/server/ServerScreen.ktapp/src/main/java/app/waveflow/ui/server/ServerViewModel.ktapp/src/main/res/xml/backup_rules.xmlapp/src/main/res/xml/data_extraction_rules.xmlapp/src/test/java/app/waveflow/data/remote/HttpServerApiTest.ktapp/src/test/java/app/waveflow/data/remote/ServerSessionRepositoryTest.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/ServerViewModelTest.kt
Deux crashs de la même famille que ceux déjà corrigés, tous deux sur un chemin d'échec que rien ne rattrapait. Une coupure survenant *pendant* la lecture du corps lève depuis `string()`, et non depuis le rappel d'échec de l'appel : l'IOException nue traversait toute la pile jusqu'à un `viewModelScope` qui n'attend que des `ServerException`. Elle devient un `Unreachable`, avec sa cause. La coroutine est vérifiée active avant la conversion : OkHttp signale aussi l'annulation par une IOException, et la reconvertir masquerait l'abandon de l'écran. `connect` ne rattrapait que les `ServerException`. Le serveur peut accepter la connexion et l'enregistrement échouer ensuite ; l'exception quittait alors la portée. Même filet que `disconnect`. La porte de permission recevait les marges du Scaffold alors que le Box qui l'englobe les pose déjà : le message de refus les prenait deux fois. Le README affirmait qu'un jeton « ne quitte jamais l'appareil », ce qui est faux — il part à chaque requête authentifiée. Ce qui est vrai, et seulement cela, c'est qu'il n'est pas recopié sur un autre appareil par une sauvegarde. Claude-Session: https://claude.ai/code/session_01F89rkrDB9TxcwHbfgNoyY1
Premier pas de la source distante. Un onglet Serveur ouvre une session sur un WaveFlow Server et la maintient. Rien du catalogue distant n'est encore affiché, et rien de la bibliothèque locale ne part nulle part.
Les deux sources restent séparées : l'onglet est une section à part, pas un filtre sur les écrans existants. C'est ce que permet la RFC-003, qui refuse explicitement de deviner une correspondance entre pistes locales et pistes serveur.
Ce qui est branché
POST /api/v2/auth/loginavec le modèle de l'appareil comme nom de session — le serveur le liste ainsi parmi les appareils du compte. Le jeton d'accès vaut un quart d'heure, se renouvelle par/api/v2/auth/refresh, et la session survit au redémarrage.Le point délicat : la rotation
Le jeton de rafraîchissement tourne à chaque usage — le serveur invalide l'ancien dès qu'il en émet un nouveau. Vérifié en conditions réelles : rejouer le précédent donne 401.
Deux renouvellements concurrents partiraient donc du même jeton, et le second serait refusé : une session perdue alors qu'elle était parfaitement valide. Tout ce qui touche aux jetons passe par un seul mutex, et
persistécrit sur disque avant de publier l'état — l'app ne doit jamais annoncer une session que le prochain démarrage ne retrouverait pas.Les échecs sont classés selon ce que l'utilisateur peut en faire :
UnauthorizedRejectedUnreachableUnexpectedUn renouvellement ne ferme la session que dans le premier cas.
Deux compromis assumés
Les jetons ne sont pas chiffrés. Ils vont dans un DataStore, protégés par le bac à sable applicatif et le chiffrement de l'appareil.
security-crypto, la seule brique androidx qui ferait mieux, n'est jamais sortie d'alpha et n'est plus maintenue. Un appareil rooté expose donc le jeton de rafraîchissement ; la contrepartie est qu'il se révoque depuis le serveur.Le trafic en clair est autorisé. Un serveur auto-hébergé vit le plus souvent sur un réseau local sans certificat. J'ai d'abord voulu n'ouvrir que les plages privées, mais
<domain>d'une configuration de sécurité réseau n'accepte que des noms d'hôtes et des adresses littérales — la notation CIDR n'existe pas, et l'aurait fait planter au démarrage. L'autorisation est donc générale ; une adresse saisie sans schéma est jointe en HTTPS.Validation
./gradlew clean testDebugUnitTest assembleDebug→ BUILD SUCCESSFUL, 122 tests, 0 échec, aucun avertissement. 41 tests nouveaux, répartis sur le client HTTP, le dépôt de session, le ViewModel et l'écran.Et surtout, contre un vrai serveur. J'ai lancé un
waveflow-server2.0.0-beta.0 en local et vérifié le parcours complet : connexion, rotation des jetons, rejeu refusé, mauvais mot de passe, nom d'appareil vide, déconnexion, plus le renouvellement automatique du dépôt en avançant son horloge. Les corps de réponse et d'erreur des tests sont ceux relevés à cette occasion — dont un piège : un corps mal formé fait répondre du texte brut, pas le{code, message}habituel, ce que le parseur d'erreur tolère désormais.Deux comportements ont été validés par retrait du correctif : sans la marge d'expiration, le test de renouvellement anticipé tombe ; sans la persistance après renouvellement, celui de la rotation tombe. Chaque fois seul le test concerné.
Limite connue
L'onglet Serveur vit à l'intérieur de
AudioPermissionGatecomme le reste duNavHost: refuser la permission audio le rend inatteignable, alors qu'il n'en a aucun besoin. Sans catalogue distant il n'y a encore rien à y faire, mais la porte devra sortir de là quand il arrivera.Toujours pas d'essai sur appareil réel.
Summary by CodeRabbit
Nouvelles fonctionnalités
Documentation