diff --git a/Blackboard/Done/Frontend_Review.md b/Blackboard/Done/Frontend_Review.md new file mode 100644 index 00000000..b19be515 --- /dev/null +++ b/Blackboard/Done/Frontend_Review.md @@ -0,0 +1,651 @@ +** Ce travail devra être réalisé en suivant scrupuleusement les consignes listées dans le fichier [@Rules_optimal.md](file:///Users/coissac/Sync/maison/Petite_maisons/src/pmomusic/Blackboard/Rules_optimal.md) ** + +## Vue d'ensemble + +Le frontend de PMOMusic est une application Vue 3 + TypeScript avec Pinia, organisée autour +de composables réactifs, d'un client SSE centralisé et d'un cache API à plusieurs niveaux. +L'architecture générale est solide : séparation claire composables/services/vues, typage strict +(`strict: true` dans `tsconfig.app.json`), reconnexion SSE avec backoff exponentiel. + +Cette revue documente les bugs avérés, les fragilités de conception et les axes d'amélioration +relevés lors d'une lecture complète des fichiers `pmoapp/webapp/src/`. + +--- + +## Bugs + +### 1. `apiCache.ts:130-133` — Mutation globale du TTL non réentrante + +**Problème** : la méthode `fetch()` accepte un `ttl` optionnel par appel. Pour l'appliquer, +elle modifie `this.options.ttl` globalement avant d'appeler `this.set()`, puis le restaure : + +```typescript +// apiCache.ts:130-133 +if (ttl) { + const originalTtl = this.options.ttl; + this.options.ttl = ttl; // (A) modification globale + this.set(endpoint, data, params); + this.options.ttl = originalTtl; // (B) restauration +} +``` + +**Cause** : `fetcher()` est `await`-é (ligne 128) avant ce bloc. Pendant cet await, d'autres +microtasks peuvent s'intercaler et appeler `isFresh()` ou `set()`, qui lisent `this.options.ttl`. +Si deux appels `fetch()` avec des `ttl` différents sont en vol simultanément, la restauration +de (B) peut effacer la valeur posée par le second appel concurrent, ou (A) peut lire un TTL +modifié par un autre appel. + +**Solution** : passer le `ttl` directement à `set()` comme paramètre, sans modifier l'état +partagé : + +```typescript +// Dans set() : ajouter un paramètre ttl optionnel +set(endpoint: string, data: T, params?: ..., etag?: string, ttl?: number): void { + const key = this.makeKey(endpoint, params); + this.cache.set(key, { + data, + timestamp: Date.now(), + ttl: ttl ?? this.options.ttl, // TTL par entrée, pas global + etag, + }); + this.notifySubscribers(key, data); +} + +// Dans isFresh() : lire le ttl de l'entrée +private isFresh(key: string): boolean { + const entry = this.cache.get(key); + if (!entry) return false; + return Date.now() - entry.timestamp < (entry.ttl ?? this.options.ttl); +} + +// Dans fetch() : supprimer le bloc de mutation globale +this.set(endpoint, data, params, undefined, ttl); +``` + +--- + +### 2. `useRenderers.ts:151 + 222` — Réassignation post-switch écrase le nouvel objet + +**Problème** : le handler `onRendererEvent` termine par une ligne inconditionnelle : + +```typescript +// useRenderers.ts:222 +snapshotState.snapshots.set(rendererId, snapshot); +``` + +Cette ligne s'exécute pour **tous** les types d'événements après le `switch`, y compris pour +`position_changed` et `metadata_changed` qui ont déjà créé et stocké un nouvel objet dans le +Map à l'intérieur du switch : + +```typescript +// position_changed — ligne 151 : stocke newSnapshot +snapshotState.snapshots.set(rendererId, newSnapshot); +break; +// → puis ligne 222 écrase avec snapshot (proxy d'origine) + +// metadata_changed — ligne 176 : stocke un spread +snapshotState.snapshots.set(rendererId, { ...snapshot, state: { ...snapshot.state } }); +break; +// → puis ligne 222 écrase avec snapshot (proxy d'origine) +``` + +**Cause** : `break` sort du `switch` mais pas de la fonction. La ligne 222 est atteinte dans +tous les cas. + +**Conséquence** : les objets créés pour forcer la détection de changement par Vue sont +immédiatement écrasés. Le mécanisme de réactivité fonctionne malgré tout (le proxy muté est +re-stocké), mais la logique est trompeuse et fragile : si Vue venait à optimiser la détection +d'identité des objets réactifs, cette redondance deviendrait un bug visible. + +**Solution** : supprimer la ligne 222 et s'assurer que chaque branche du switch stocke +explicitement son résultat dans la Map. Les branches `volume_changed`, `mute_changed` et +`binding_changed` qui mutent directement `snapshot` doivent aussi créer un nouvel objet : + +```typescript +case "volume_changed": + snapshotState.snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, volume: event.volume }, + }); + break; + +case "mute_changed": + snapshotState.snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, mute: event.mute }, + }); + break; +// idem pour binding_changed, stream_state_changed +``` + +Supprimer la ligne 222. Chaque case devient responsable de son stockage, ce qui élimine aussi +le besoin de `toRaw()`. + +--- + +### 3. `useRenderers.ts:122` — `as any` sur `transport_state` + +**Problème** : + +```typescript +// useRenderers.ts:122 +snapshot.state.transport_state = event.state as any; +``` + +**Cause** : `event.state` est typé `string` (type SSE générique), alors que +`transport_state` est une union littérale (`"PLAYING" | "PAUSED" | "STOPPED" | ...`). +Le cast `as any` contourne la vérification de type. + +**Conséquence** : si le backend envoie une valeur non prévue (ex. `"TRANSITIONING"`), elle +sera stockée sans validation. Les composants qui comparent `transport_state === "PLAYING"` +ne matcheront pas et l'UI restera muette. + +**Solution** : définir un guard de type ou une assertion dans `types.ts` : + +```typescript +// services/pmocontrol/types.ts +export type TransportState = "PLAYING" | "PAUSED" | "STOPPED" | "NO_MEDIA" | "TRANSITIONING"; + +export function isTransportState(s: string): s is TransportState { + return ["PLAYING", "PAUSED", "STOPPED", "NO_MEDIA", "TRANSITIONING"].includes(s); +} +``` + +```typescript +// useRenderers.ts — case state_changed +case "state_changed": + if (isTransportState(event.state)) { + snapshot.state.transport_state = event.state; + } else { + console.warn(`[useRenderers] transport_state inconnu: ${event.state}`); + } + break; +``` + +--- + +### 4. `apiCache.ts:181-200` — `invalidate()` supprime silencieusement les subscriptions actives + +**Problème** : + +```typescript +// apiCache.ts:197-200 +keysToDelete.forEach(key => { + this.cache.delete(key); + this.subscriptions.delete(key); // ← subscriptions perdues sans notification +}); +``` + +**Cause** : lors d'une invalidation (ex. après un SSE event), les callbacks enregistrés via +`subscribe()` sont supprimés de la Map. Les composants ne sont pas notifiés de la suppression +et ne reçoivent plus les futures mises à jour même après un refetch. + +**Conséquence** : un composant qui a appelé `apiCache.subscribe(...)` et qui survit à une +invalidation devient « sourd » sans le savoir. + +**Solution** : conserver les subscriptions lors d'une invalidation — seule la donnée en cache +est périmée, pas les abonnés : + +```typescript +invalidate(pattern: string): void { + const keysToDelete: string[] = []; + // ... construction de keysToDelete inchangée ... + keysToDelete.forEach(key => { + this.cache.delete(key); + // NE PAS supprimer this.subscriptions.get(key) + // Les abonnés seront notifiés lors du prochain set() + }); +} +``` + +Si l'on veut notifier les abonnés d'une invalidation (pour qu'ils affichent un état de +chargement), ajouter un callback optionnel `onInvalidate` dans l'interface de subscription. + +--- + +## Fragilités de conception + +### 5. `useRenderers.ts:7,143-151` — `toRaw()` comme contournement de réactivité Vue + +**Problème** : le code utilise `toRaw()` pour extraire l'objet brut d'un proxy Vue avant de +faire un spread, afin que la copie ne contienne pas de getters réactifs qui pointent vers +l'objet original : + +```typescript +// useRenderers.ts:143-151 +const rawState = toRaw(snapshot.state); +const newState = { ...rawState }; +const newSnapshot = { ...snapshot, state: newState }; +snapshotState.snapshots.set(rendererId, newSnapshot); +``` + +**Cause** : les Maps imbriquées dans un objet `reactive()` ont un comportement de réactivité +peu prévisible dans Vue 3. Vue ne détecte pas les mutations d'éléments d'une Map réactive si +la référence de la Map elle-même ne change pas. + +**Solution recommandée** : remplacer `reactive(new Map())` par `shallowRef(new Map())` pour +les Maps qui contiennent des données complexes. La réactivité se déclenche en remplaçant la +Map entière (ou en forçant un `triggerRef`) : + +```typescript +// Au lieu de : +const snapshotState = reactive({ snapshots: reactive(new Map()), ... }); + +// Utiliser : +const snapshots = shallowRef(new Map()); + +// Pour déclencher la réactivité après mutation : +snapshots.value = new Map(snapshots.value); // ou triggerRef(snapshots) +``` + +Cela rend la propagation de réactivité explicite et élimine le besoin de `toRaw()`. + +--- + +### 6. `useRenderers.ts:552-570` — Timer de debounce non nettoyé dans `useRenderer()` + +**Problème** : `useRenderer()` crée un timer de debounce local qui n'est jamais nettoyé si +le composant parent est démonté : + +```typescript +// useRenderers.ts:553-566 +let refreshDebounceTimer: ReturnType | null = null; +const REFRESH_DEBOUNCE_MS = 500; + +async function refresh(force = true) { + if (refreshDebounceTimer !== null) return; + refreshDebounceTimer = setTimeout(() => { + refreshDebounceTimer = null; + }, REFRESH_DEBOUNCE_MS); + await Promise.all([...]); +} +``` + +**Cause** : pas d'appel à `clearTimeout` dans un `onUnmounted`. Si le composant est démonté +pendant les 500 ms du debounce, le timer continue de s'exécuter. + +**Conséquence** : fuite mémoire potentielle ; dans des cas extrêmes (navigation rapide), le +callback peut tenter de déclencher un fetch sur un composant déjà démonté. + +**Solution** : + +```typescript +import { onUnmounted } from 'vue'; + +// Dans useRenderer() : +onUnmounted(() => { + if (refreshDebounceTimer !== null) { + clearTimeout(refreshDebounceTimer); + refreshDebounceTimer = null; + } +}); +``` + +--- + +### 7. `useRenderers.ts:45-46` — Singleton SSE initialisé par flag de module non réinitialisable + +**Problème** : + +```typescript +// useRenderers.ts:45-46 +let sseInitialized = false; +function ensureSSEInitialized() { + if (sseInitialized) return; + // ... + sseInitialized = true; +} +``` + +**Cause** : ce flag de module est persistant pour toute la durée de vie de la page. Si la +connexion SSE est perdue puis rétablie avec un nouvel objet `PMOControlSSE`, le handler +`onRendererEvent` précédent peut ne plus être actif, mais `sseInitialized` empêche sa +re-enregistration. + +**Conséquence** : après une déconnexion et reconnexion SSE, les événements renderer peuvent +ne plus être reçus par `useRenderers` jusqu'à un rechargement de page. + +**Solution** : exposer une fonction `resetSSE()` qui remet `sseInitialized = false` et la +connecter à l'événement de reconnexion du service SSE. Alternativement, utiliser le pattern +`provide/inject` ou un store Pinia pour gérer le cycle de vie SSE explicitement, en lieu et +place du flag de module. + +--- + +## Qualité du code + +### 8. `useTabs.ts` — Deep watch déclenchant une sérialisation localStorage à chaque mutation + +**Problème** : le watch qui persiste l'état des onglets utilise `{ deep: true }` sur un +tableau qui peut contenir jusqu'à 12 entrées avec des métadonnées : + +```typescript +// useTabs.ts:349-355 +watch( + () => [state.tabs, state.activeTabId, state.tabHistory], + () => { saveToLocalStorage(); }, + { deep: true }, +); +``` + +**Cause** : `{ deep: true }` traverse récursivement toutes les propriétés observées. +`saveToLocalStorage()` appelle `JSON.stringify` sur l'ensemble des tabs à chaque mutation, +même mineure (ex. changement de `activeTabId`). + +**Solution** : surveiller les propriétés individuellement et sérialiser uniquement ce qui +change, ou utiliser un computed pour construire la clé de changement : + +```typescript +// Watch séparés, sans deep +watch(() => state.activeTabId, saveToLocalStorage); +watch(() => state.tabHistory.length, saveToLocalStorage); +watch( + () => state.tabs.map(t => t.id + t.type + (t.metadata?.rendererId ?? '')).join('|'), + saveToLocalStorage, +); +``` + +--- + +### 9. `UnifiedControlView.vue:40-49` — Swipe : `clientX` final au lieu de la position initiale + +**Problème** : + +```typescript +// UnifiedControlView.vue:40-49 +useSwipe(viewRef, { + threshold: 50, + onSwipeEnd(_e: TouchEvent, swipeDirection: string) { + if (swipeDirection === "right" && !drawerOpen.value) { + const touch = _e.changedTouches[0]; + if (touch && touch.clientX < 50) { // ← position finale du doigt + drawerOpen.value = true; + } + } + }, +}); +``` + +**Cause** : `onSwipeEnd` reçoit l'événement `touchend`. Dans `changedTouches`, `clientX` +est la position **finale** du doigt (après le swipe), pas la position initiale. Un swipe +commençant à `x=30` et terminant à `x=150` a `clientX=150` dans `touchend` — la condition +`< 50` ne sera jamais vraie pour un swipe horizontal significatif. + +**Conséquence** : le geste de swipe depuis le bord gauche ne fonctionne probablement pas +sur les appareils tactiles. + +**Solution** : capturer la position initiale dans `onSwipeStart` : + +```typescript +const swipeStartX = ref(0); + +useSwipe(viewRef, { + threshold: 50, + onSwipeStart(e: TouchEvent) { + swipeStartX.value = e.touches[0]?.clientX ?? 0; + }, + onSwipeEnd(_e: TouchEvent, swipeDirection: string) { + if (swipeDirection === "right" && !drawerOpen.value && swipeStartX.value < 50) { + drawerOpen.value = true; + } + }, +}); +``` + +--- + +### 10. `api.ts:50` — Réponse JSON non validée avant le cast TypeScript + +**Problème** : + +```typescript +// api.ts:50 +return response.json(); // retour typé T par inférence, sans validation +``` + +**Cause** : `response.json()` retourne `Promise`. TypeScript accepte le retour car la +méthode `request` promet `Promise`, mais aucune validation de structure n'est effectuée. + +**Conséquence** : si le backend renvoie un schéma légèrement différent (champ renommé, type +changé), le bug se manifestera loin du point d'appel avec un message cryptique. En +développement avec plusieurs instances en parallèle (`udn_prefix` différent), une requête +dirigée vers la mauvaise instance peut retourner un format inattendu. + +**Solution pragmatique** : ajouter une validation légère avec un type guard pour les réponses +critiques, ou au minimum loguer la réponse brute en mode développement : + +```typescript +private async request(path: string, options: RequestInit = {}): Promise { + // ... + const data = await response.json(); + if (import.meta.env.DEV && data == null) { + console.warn(`[PMOControlAPI] Réponse vide pour ${path}`); + } + return data as T; +} +``` + +Pour les endpoints critiques (`getRendererFullSnapshot`, `getRenderers`), envisager un +schéma de validation Zod ou une assertion runtime minimale. + +--- + +## Accessibilité et feedback utilisateur + +### 11. Absence de labels ARIA sur les contrôles transport + +Les composants `TransportControls.vue` et `VolumeControl.vue` contiennent des boutons +iconiques (play, pause, stop, volume) sans attributs `aria-label`. Les lecteurs d'écran +ne peuvent pas identifier la fonction de ces contrôles. + +**Correction minimale** : + +```html + + + +``` + +--- + +### 12. Erreurs réseau silencieuses sans feedback utilisateur + +Plusieurs appels critiques sont lancés en fire-and-forget sans propagation vers l'UI : + +```typescript +// useRenderers.ts:86 +void fetchRenderers(true); // erreur loggée en console uniquement + +// useRenderers.ts:89 +void fetchRendererSnapshot(rendererId, { force: true }); // idem +``` + +Le store `ui.ts` dispose d'un système de notifications toast (`addNotification`). Les erreurs +de réseau devraient y être propagées pour informer l'utilisateur : + +```typescript +import { useUIStore } from '@/stores/ui'; + +const uiStore = useUIStore(); + +// Dans le handler SSE : +try { + await fetchRenderers(true); +} catch { + uiStore.addNotification({ + message: 'Impossible de rafraîchir la liste des renderers', + type: 'error', + }); +} +``` + +--- + +## Points forts à conserver + +Ces patterns sont bien conçus et ne doivent pas être modifiés dans les corrections ci-dessus : + +- **Déduplication des requêtes en vol** (`apiCache.ts:100-112`) : évite les appels réseau + redondants quand plusieurs composants demandent la même ressource simultanément. +- **Watch sélectif sur les IDs** (`UnifiedControlView.vue:139-150`) : calcule une clé + synthétique `ids.join(',')` au lieu d'un deep watch sur le tableau de renderers. +- **Backoff exponentiel SSE** (`sse.ts`) : reconnexion progressive 1s→2s→4s→8s→16s→30s + avec cap. Implémentation robuste. +- **Fetch batch contrôlé** (`useRenderers.ts:351-383`) : `fetchBatchSnapshots()` avec + concurrence limitée (défaut 3) et délai inter-batches. Évite de saturer le réseau au + démarrage. +- **`filterRenderers` par UDN** (`UnifiedControlView.vue:107-120`) : filtre les WebRenderers + étrangers en comparant le UDN normalisé. Logique correcte avec gestion du préfixe `uuid:`. + +--- + +## État d'avancement — corrections appliquées (commit 2026-04-06) + +Les 12 points de la revue ont été traités. Le tableau ci-dessous récapitule ce qui a été +fait et ce qui reste à finir. + +| # | Problème | État | +|---|----------|------| +| 1 | `apiCache` — TTL mutation globale | ✅ Corrigé (`ttl` par entrée dans `CacheEntry`, `isFresh()` lit `entry.ttl`) | +| 2 | `useRenderers` — double réassignation post-switch | ✅ Corrigé (chaque `case` responsable, ligne 222 supprimée) | +| 3 | `useRenderers` — `as any` sur `transport_state` | ✅ Corrigé (`isTransportState` guard dans `types.ts`) | +| 4 | `apiCache` — `invalidate()` détruisait les subscriptions | ✅ Corrigé (`this.subscriptions.delete` supprimé) | +| 5 | `useRenderers` — `toRaw()` / `reactive(Map)` fragile | ✅ Migré vers `shallowRef` + helpers `triggerSnapshotReactivity()` / `triggerLoadingReactivity()` | +| 6 | `useRenderer()` — timer debounce non nettoyé | ✅ Corrigé (`onUnmounted` + `clearTimeout`) | +| 7 | Singleton SSE non réinitialisable | ✅ Corrigé (`resetSSE()` exposé dans le retour de `useRenderers()`) | +| 8 | `useTabs` — deep watch coûteux | ✅ Corrigé (watches séparés sans `deep: true`) | +| 9 | Swipe — `clientX` final au lieu d'initial | ✅ Corrigé (`swipeStartX` capturé dans `onSwipeStart`) | +| 10 | `api.ts` — JSON non validé | ✅ Corrigé (log dev-mode pour réponse nulle) | +| 11 | Absence de labels ARIA | ✅ Corrigé (4 boutons transport + bouton mute + slider volume) | +| 12 | Erreurs réseau silencieuses | ✅ Corrigé (`uiStore.notifyError()` dans `fetchRenderers` et `fetchRendererSnapshot`) | + +--- + +## Tâches restantes — ✅ Toutes corrigées (commit 2026-04-06) + +| # | Problème résiduel | État | +|---|-------------------|------| +| A | `state_changed` — mutation directe sans trigger réactivité | ✅ Corrigé (spread + `triggerSnapshotReactivity()`) | +| B | `queue_refreshing`/`queue_updated` — `queueRefreshingIds` sans trigger | ✅ Corrigé (`triggerQueueReactivity()` ajoutée et appelée) | +| C | `toRaw` import obsolète, `position_changed` simplifié | ✅ Corrigé (`toRaw` supprimé de l'import, spread direct) | + +--- + +### A. `useRenderers.ts` — `state_changed` : mutation directe sans déclenchement de réactivité + +**Problème** : avec la migration vers `shallowRef`, les objets dans la Map ne sont plus des +proxies Vue. La mutation directe de `snapshot.state.transport_state` ne déclenche aucune +réactivité — les composants ne se mettront pas à jour quand l'état de transport change : + +```typescript +// useRenderers.ts — case state_changed (code actuel) +case "state_changed": + if (isTransportState(event.state)) { + snapshot.state.transport_state = event.state; // ← mutation directe, pas de trigger + } + break; +``` + +**Solution** : créer un nouvel objet, comme pour `volume_changed` et `mute_changed` : + +```typescript +case "state_changed": + if (isTransportState(event.state)) { + snapshots.value.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, transport_state: event.state }, + }); + } else { + console.warn(`[useRenderers] transport_state inconnu: ${event.state}`); + } + break; +``` + +--- + +### B. `useRenderers.ts` — `queue_refreshing` / `queue_updated` : mutations de `queueRefreshingIds` sans trigger + +**Problème** : `queueRefreshingIds` est un `shallowRef`. Les appels `.add()` et +`.delete()` sur `.value` ne déclenchent pas la réactivité de `shallowRef` : + +```typescript +// useRenderers.ts — case queue_refreshing (code actuel) +case "queue_refreshing": + queueRefreshingIds.value.add(rendererId); // ← pas de trigger + break; + +case "queue_updated": + queueRefreshingIds.value.delete(rendererId); // ← pas de trigger + break; +``` + +Le composable `isQueueRefreshing(id)` retourne `queueRefreshingIds.value.has(id)`. Sans +trigger, les templates qui dépendent de cette valeur ne se recalculeront pas. + +**Solution** : ajouter une fonction `triggerQueueReactivity()` analogue à +`triggerLoadingReactivity()` et l'appeler après chaque mutation : + +```typescript +function triggerQueueReactivity() { + queueRefreshingIds.value = new Set(queueRefreshingIds.value); +} + +// Dans le switch : +case "queue_refreshing": + queueRefreshingIds.value.add(rendererId); + triggerQueueReactivity(); + break; + +case "queue_updated": + snapshot.state.queue_len = event.queue_length; + queueRefreshingIds.value.delete(rendererId); + triggerQueueReactivity(); + void fetchRendererSnapshot(rendererId, { force: true }); + break; +``` + +--- + +### C. `useRenderers.ts` — import `toRaw` et commentaire obsolètes + +**Problème** : avec `shallowRef`, les objets stockés dans `snapshots.value` sont de simples +objets JavaScript (jamais des proxies Vue). L'appel `toRaw(snapshot.state)` dans +`position_changed` est devenu un no-op, et le commentaire qui le justifie est trompeur : + +```typescript +// useRenderers.ts:160-163 (commentaire et import obsolètes) +// IMPORTANT: Utiliser toRaw() pour obtenir l'objet brut non-réactif avant de copier +// sinon Vue copie les getters réactifs qui continuent à pointer vers l'objet d'origine +const rawState = toRaw(snapshot.state); +const newState = { ...rawState }; +``` + +**Solution** : supprimer le `toRaw()`, simplifier en spread direct, retirer `toRaw` de +l'import ligne 7 : + +```typescript +// Remplacer : +import { ref, shallowRef, computed, toRaw, type Ref, onUnmounted } from "vue"; + +// Par : +import { ref, shallowRef, computed, type Ref, onUnmounted } from "vue"; + +// Dans position_changed : +const newSnapshot = { + ...snapshot, + state: { + ...snapshot.state, + position_ms: positionMs ?? 0, + duration_ms: durationMs, + }, +}; +snapshots.value.set(rendererId, newSnapshot); +``` + +Cette simplification rend aussi le cas `position_changed` cohérent avec les autres cases +(`volume_changed`, `mute_changed`, etc.) qui construisent directement l'objet final sans +passer par une variable intermédiaire. diff --git a/Blackboard/Todo/Frontend_Review.md b/Blackboard/Todo/Frontend_Review.md new file mode 100644 index 00000000..11bef2da --- /dev/null +++ b/Blackboard/Todo/Frontend_Review.md @@ -0,0 +1,327 @@ +** Ce travail devra être réalisé en suivant scrupuleusement les consignes listées dans le fichier [@Rules_optimal.md](file:///Users/coissac/Sync/maison/Petite_maisons/src/pmomusic/Blackboard/Rules_optimal.md) ** + +## Contexte + +Ce document est le résultat d'une revue de code complète du frontend Vue.js/TypeScript du Control Point +(`pmoapp/webapp/src/`). L'application est fonctionnelle mais présente plusieurs classes de problèmes +qui peuvent causer des fuites mémoire, des incohérences de réactivité Vue 3, et des difficultés de +maintenance à mesure que l'app grandit. + +**Périmètre** : uniquement `pmoapp/webapp/src/` (composants, services, composables, stores, utils, CSS). + +--- + +## Problèmes identifiés + +### P0 — Fuites mémoire via listeners SSE jamais nettoyés + +**Fichiers** : `src/composables/useSSE.ts`, `src/composables/useRenderers.ts`, +`src/composables/useMediaServers.ts` + +Les abonnements aux événements SSE sont créés lors du premier appel de chaque composable, mais +jamais nettoyés si le composable est réutilisé ou le composant détruit. Dans `useSSE.ts`, la +fonction `onRendererEvent()` retourne une fonction de cleanup, mais `useRendererEvents()` ignore +ce retour — l'abonnement reste actif indefiniment. + +De plus, dans `imageCache.ts`, un `setInterval` de cleanup s'exécute toutes les 5 minutes sans +jamais être annulé si l'app est détruite. + +### P1 — Réactivité Vue incohérente avec `shallowRef` + Maps + +**Fichier** : `src/composables/useRenderers.ts` (L23-37) + +`snapshots` et `loadingIds` sont déclarés en `shallowRef>()`. Vue ne détecte pas les +mutations d'objets à l'intérieur d'un `shallowRef`. La solution actuelle — `triggerSnapshotReactivity()` +qui crée une nouvelle Map à chaque appel — force une re-render complète de tous les composants +qui dépendent de `snapshots`, même si seul un renderer a changé. + +### P2 — Race condition à l'initialisation (main.ts) + +**Fichier** : `src/main.ts` + +Le UIStore est initialisé après le montage de l'app et l'appel à `sse.connect()`. Des événements +SSE peuvent arriver avant que `useUIStore()` soit appelé dans les composants, et les notifications +correspondantes peuvent être perdues. + +### P3 — SSE singleton sans garantie formelle + +**Fichiers** : `src/composables/useRenderers.ts`, `src/composables/useMediaServers.ts` + +Chaque composable maintient son propre flag `sseInitialized` pour éviter les double-abonnements. +Le mécanisme repose sur une convention implicite fragile : si deux composables s'abonnent au même +type d'événement SSE dans des contextes différents, les callbacks s'accumulent sans être +dédupliqués. + +Dans `useSSE.ts`, `setupConnectionListener()` vérifie `connectionCallbacks.size === 0` mais +sans lock — deux appels simultanés peuvent installer deux listeners. + +### P4 — Pas de timeout ni retry sur les requêtes `fetch` + +**Fichier** : `src/services/pmocontrol/api.ts` + +Toutes les requêtes `fetch()` sont émises sans `AbortController`. Si le serveur ne répond pas, +la promesse pend indéfiniment, bloquant potentiellement les composants qui attendent le résultat. +Il n'y a ni timeout configurable ni retry automatique au niveau du service. + +### P5 — Validation absente des réponses API + +**Fichiers** : `src/services/audioCache.ts` (L76), `src/services/coverCache.ts`, +`src/services/playlists.ts`, `src/services/pmocontrol/api.ts` + +Les réponses JSON sont acceptées sans vérification de structure. Une assertion de type comme +`metadata as { origin_url?: unknown }` ne protège pas contre un changement d'API côté Rust. Si +l'API retourne une structure inattendue, le crash survient au runtime, pas à la compilation. + +### P6 — Type assertions dangereuses dans PMOPlayer + +**Fichier** : `src/services/pmosource.ts` / PMOPlayer (L197-214) + +Les messages de commande sont typés `Record`, puis les propriétés sont castées +directement : `msg.url as string`, `msg.timestamp as number`. Si une propriété est absente ou +d'un type différent, TypeScript ne le détecte pas. + +### P7 — `useTabs` : watch multiples sans debounce, flag de restauration non-réinitialisé + +**Fichier** : `src/composables/useTabs.ts` (L44, L350-361) + +Trois `watch()` séparées écrivent dans `localStorage`. Sans debounce commun, si 3 onglets +changent d'état simultanément, `localStorage` est écrit 3 fois de suite. + +Le flag `isRestoringFromStorage` (L44) empêche la boucle de sauvegarde pendant la restauration, +mais sans timeout : si `restoreFromLocalStorage()` lance une exception non-catchée, le flag reste +`true` et toutes les sauvegardes futures sont silencieusement ignorées. + +### P8 — Routes de debug exposées en production, pas de lazy loading + +**Fichier** : `src/router/index.ts` + +Les routes debug (CoversCache, AudioCache, UPnP Explorer, etc.) sont accessibles en production +sans contrôle d'accès. Par ailleurs, tous les composants sont importés statiquement, augmentant +le bundle initial inutilement — les vues debug notamment ne sont jamais utilisées en prod. + +### P9 — `formatMsToShortTime` est un alias inutile + +**Fichier** : `src/utils/time.ts` (L58-59) + +```typescript +// Actuellement +export function formatMsToShortTime(ms: number | null): string { + return formatMsToTime(ms); +} +``` + +Fonction identique à `formatMsToTime`. Tous les appelants peuvent utiliser directement +`formatMsToTime`. + +### P10 — `truncate()` dans `string.ts` peut dépasser `maxLength` + +**Fichier** : `src/utils/string.ts` (L46-48) + +```typescript +// Actuellement +export function truncate(str: string, maxLength: number, suffix = '…'): string { + return str.length > maxLength ? str.slice(0, maxLength - suffix.length) + suffix : str; +} +``` + +Si `suffix.length >= maxLength`, `str.slice(0, maxLength - suffix.length)` retourne une chaîne +de longueur négative (comportement silencieux en JS, retourne `''`), et le résultat final est +plus long que `maxLength`. + +### P11 — `DEFAULT_COVER_SVG` inline dans coverCache.ts + +**Fichier** : `src/services/coverCache.ts` (L206-226) + +Un SVG inline de ~20 lignes est inclus dans chaque bundle qui importe `coverCache`. Il devrait +être un fichier `src/assets/default-cover.svg` importé nativement par Vite (ce qui permet le +tree-shaking et le caching HTTP séparé). + +### P12 — `animations` CSS sans `prefers-reduced-motion` + +**Fichiers** : `src/assets/styles/glass-theme.css` (L384-401), +`src/assets/styles/pmocontrol.css` (L82) + +Les animations `glassShimmer` (2s infini) et le `pulse` du badge de statut `Transitioning` +s'exécutent sans tenir compte de `prefers-reduced-motion: reduce`. Sur certains systèmes ou +pour des utilisateurs sensibles au mouvement, ces animations sont gênantes. + +### P13 — CSS dupliqué dans drawers.css + +**Fichier** : `src/assets/styles/drawers.css` (L105-149) + +`drawer-close-btn` et `drawer-back-btn` partagent 90% des styles. Un TODO présent en L167 +("remplacer par la classe globale .section-title") confirme cette dette. La variable +`var(--opacity-disabled)` est utilisée mais non définie dans `variables.css`. + +### P14 — `browseContainer` : clés de cache fragiles et pas de pagination + +**Fichier** : `src/composables/useMediaServers.ts` (L145, L239) + +Les clés de cache sont construites comme `${serverId}/${containerId}`. Si un `containerId` +contient un slash (séparateur d'URL), la clé est ambigüe. Par exemple, `server1/a/b` peut +correspondre à serverId=`server1`, containerId=`a/b` ou serverId=`server1/a`, containerId=`b`. + +La pagination n'est pas implémentée côté composable : `browseContainer` charge toujours +offset=0, limit=50. Pour les containers avec 500+ items, les items au-delà de 50 ne sont +jamais accessibles. + +### P15 — Notifications sans limite de taille dans `ui.ts` + +**Fichier** : `src/stores/ui.ts` (L50, L55-57) + +Un bug ou une boucle d'erreur peut générer des centaines de notifications. Le tableau +`notifications` n'est pas limité. Chaque notification crée un `setTimeout` individuel, et +si le store est détruit avant l'expiration, ces callbacks persistent (ghosts). + +--- + +## Plan d'exécution + +Les corrections sont groupées par effort et impact. Les P0–P3 concernent la fiabilité +(fuites mémoire, réactivité), les P4–P8 la robustesse et maintenabilité, les P9–P15 la +qualité et la dette technique. + +### Étape 1 — Corriger les fuites mémoire SSE (P0) + +Dans `useSSE.ts`, stocker et appeler les fonctions de cleanup retournées par `onRendererEvent` / +`onMediaServerEvent` : + +```typescript +// useSSE.ts – useRendererEvents() +onMounted(() => { + const cleanup = onRendererEvent(rendererId(), handler); + onUnmounted(cleanup); // ← actuellement ignoré +}); +``` + +Dans `imageCache.ts`, exporter une fonction `destroyImageCache()` qui appelle `clearInterval` +sur le timer de cleanup, et l'appeler dans le `onUnmounted` de l'app root. + +### Étape 2 — Stabiliser la réactivité des snapshots (P1) + +Remplacer `shallowRef>` + `triggerSnapshotReactivity` par `reactive(new Map<...>)`. +Vue 3 rend les Maps réactives nativement. Les composants qui lisent `snapshots.get(id)` +seront notifiés uniquement si ce `id` change. + +```typescript +// Avant +const snapshots = shallowRef>(new Map()); +function triggerSnapshotReactivity() { + snapshots.value = new Map(snapshots.value); +} + +// Après +const snapshots = reactive(new Map()); +// Les modifications directes (snapshots.set/delete) déclenchent la réactivité +``` + +### Étape 3 — Timeout fetch + AbortController (P4) + +Ajouter un helper dans `api.ts` : + +```typescript +function fetchWithTimeout(url: string, options?: RequestInit, timeoutMs = 10_000): Promise { + const controller = new AbortController(); + const id = setTimeout(() => controller.abort(), timeoutMs); + return fetch(url, { ...options, signal: controller.signal }) + .finally(() => clearTimeout(id)); +} +``` + +Utiliser `fetchWithTimeout` pour toutes les requêtes dans le service API. + +### Étape 4 — Corriger `useTabs` watchs et flag de restauration (P7) + +Fusionner les trois `watch()` en un seul `watchEffect` avec un debounce unique (100ms). +Encadrer `isRestoringFromStorage` dans un bloc `try/finally` : + +```typescript +async function restoreFromLocalStorage() { + isRestoringFromStorage = true; + try { + // ... logique de restauration + } catch (e) { + console.error('Tab restore failed:', e); + } finally { + isRestoringFromStorage = false; + } +} +``` + +### Étape 5 — Limit de notifications et nettoyage timers (P15) + +```typescript +const MAX_NOTIFICATIONS = 5; + +function addNotification(notif: Omit): void { + if (notifications.value.length >= MAX_NOTIFICATIONS) { + notifications.value.shift(); // supprimer la plus ancienne + } + const id = nextId++; + const timer = setTimeout(() => removeNotification(id), notif.duration ?? 5000); + notificationTimers.set(id, timer); + notifications.value.push({ ...notif, id }); +} + +function $dispose() { + notificationTimers.forEach(clearTimeout); + notificationTimers.clear(); +} +``` + +### Étape 6 — Lazy loading des routes et protection debug (P8) + +```typescript +// router/index.ts +const DebugView = () => import('../views/DebugView.vue'); +const isDev = import.meta.env.DEV; + +const routes = [ + // ... routes normales + ...(isDev ? [{ path: '/debug', component: DebugView }] : []), + { path: '/:pathMatch(.*)*', redirect: '/' }, // wildcard 404 +]; +``` + +### Étape 7 — Corrections mineures (P9, P10, P11, P12, P13) + +- **P9** : Supprimer `formatMsToShortTime`, remplacer tous les appels par `formatMsToTime` +- **P10** : Ajouter un guard dans `truncate` : `if (suffix.length >= maxLength) return str.slice(0, maxLength)` +- **P11** : Déplacer le SVG dans `src/assets/default-cover.svg` et l'importer avec `import defaultCover from '../assets/default-cover.svg?raw'` +- **P12** : Entourer les animations CSS avec `@media (prefers-reduced-motion: no-preference) { ... }` +- **P13** : Factoriser `drawer-close-btn` / `drawer-back-btn` avec une classe `.drawer-icon-btn`. Définir `--opacity-disabled: 0.4` dans `variables.css` + +### Étape 8 — Clés de cache et pagination (P14) + +Encoder les IDs dans les clés de cache : + +```typescript +const cacheKey = `${encodeURIComponent(serverId)}:${encodeURIComponent(containerId)}`; +``` + +Utiliser `:` comme séparateur (absent de l'encoding) pour éviter toute ambigüité. + +Pour la pagination, ajouter une propriété `hasMore: boolean` et `loadMore()` au résultat de +`browseContainer`, incrementant offset à chaque appel. + +### Ordre d'exécution + +1. Étape 1 — fuites SSE (P0) — fiabilité critique +2. Étape 2 — réactivité Map (P1) — fiabilité +3. Étape 3 — timeout fetch (P4) — robustesse réseau +4. Étape 4 — useTabs (P7) — fiabilité des onglets +5. Étape 5 — notifications (P15) — stabilité UI +6. Étape 6 — router (P8) — sécurité + performance bundle +7. Étape 7 — corrections mineures (P9–P13) +8. Étape 8 — cache keys + pagination (P14) + +## Règle après ces corrections + +**Interdit** : créer un abonnement SSE (`onRendererEvent`, `onMediaServerEvent`) sans stocker +et appeler la fonction de cleanup retournée dans `onUnmounted`. + +**Interdit** : utiliser `shallowRef>` avec mutation directe — utiliser `reactive(new Map())` +pour les Maps qui doivent déclencher la réactivité Vue sur leurs entrées. + +**Obligatoire** : toute requête `fetch()` dans un service doit utiliser `fetchWithTimeout` +avec un AbortController. diff --git a/Cargo.lock b/Cargo.lock index 5017a1ac..843135d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 4 [[package]] name = "PMOMusic" -version = "0.3.36" +version = "0.3.39" dependencies = [ "axum 0.8.7", "console-subscriber", diff --git a/PMOMusic/Cargo.toml b/PMOMusic/Cargo.toml index 7b01df72..703c428a 100644 --- a/PMOMusic/Cargo.toml +++ b/PMOMusic/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "PMOMusic" -version = "0.3.37" +version = "0.3.39" edition = "2024" [dependencies] diff --git a/pmoapp/webapp/src/assets/default-cover.svg b/pmoapp/webapp/src/assets/default-cover.svg new file mode 100644 index 00000000..dca01244 --- /dev/null +++ b/pmoapp/webapp/src/assets/default-cover.svg @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + No Image Available + + \ No newline at end of file diff --git a/pmoapp/webapp/src/assets/styles/drawers.css b/pmoapp/webapp/src/assets/styles/drawers.css index 977c3b58..1d7adc95 100644 --- a/pmoapp/webapp/src/assets/styles/drawers.css +++ b/pmoapp/webapp/src/assets/styles/drawers.css @@ -101,13 +101,15 @@ font-size: var(--text-base); } -/* Bouton fermer */ -.drawer-close-btn { +/* ======================================== + ICONS COMMUNS POUR DRAWERS + ======================================== */ + +/* Classe de base pour les boutons icônes des drawers */ +.drawer-icon-btn { display: flex; align-items: center; justify-content: center; - width: 40px; - height: 40px; flex-shrink: 0; padding: 0; background: rgba(255, 255, 255, 0.1); @@ -118,34 +120,31 @@ color: var(--color-text); } -.drawer-close-btn:hover { +.drawer-icon-btn:hover { background: rgba(255, 255, 255, 0.2); - transform: scale(1.1); } -.drawer-close-btn:active { +.drawer-icon-btn:active { transform: scale(0.95); } +/* Bouton fermer */ +.drawer-close-btn { + width: 40px; + height: 40px; +} + +.drawer-close-btn:hover { + transform: scale(1.1); +} + /* Bouton retour (ServerDrawer navigation) */ .drawer-back-btn { - display: flex; - align-items: center; - justify-content: center; width: 36px; height: 36px; - flex-shrink: 0; - padding: 0; - background: rgba(255, 255, 255, 0.1); - border: 1px solid rgba(255, 255, 255, 0.2); - border-radius: 50%; - cursor: pointer; - transition: all var(--transition-fast) ease; - color: var(--color-text); } .drawer-back-btn:hover { - background: rgba(255, 255, 255, 0.2); transform: scale(1.05); } diff --git a/pmoapp/webapp/src/assets/styles/glass-theme.css b/pmoapp/webapp/src/assets/styles/glass-theme.css index 56dc8c76..8f19d28d 100644 --- a/pmoapp/webapp/src/assets/styles/glass-theme.css +++ b/pmoapp/webapp/src/assets/styles/glass-theme.css @@ -381,24 +381,27 @@ ANIMATIONS ======================================== */ -@keyframes glassShimmer { - 0% { - background-position: -200% center; +/* Respecte prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + @keyframes glassShimmer { + 0% { + background-position: -200% center; + } + 100% { + background-position: 200% center; + } } - 100% { - background-position: 200% center; - } -} -.glass-shimmer { - background: linear-gradient( - 90deg, - rgba(255, 255, 255, 0) 0%, - rgba(255, 255, 255, 0.1) 50%, - rgba(255, 255, 255, 0) 100% - ); - background-size: 200% 100%; - animation: glassShimmer 2s ease-in-out infinite; + .glass-shimmer { + background: linear-gradient( + 90deg, + rgba(255, 255, 255, 0) 0%, + rgba(255, 255, 255, 0.1) 50%, + rgba(255, 255, 255, 0) 100% + ); + background-size: 200% 100%; + animation: glassShimmer 2s ease-in-out infinite; + } } /* ======================================== diff --git a/pmoapp/webapp/src/assets/styles/pmocontrol.css b/pmoapp/webapp/src/assets/styles/pmocontrol.css index dbd85493..cee79f1f 100644 --- a/pmoapp/webapp/src/assets/styles/pmocontrol.css +++ b/pmoapp/webapp/src/assets/styles/pmocontrol.css @@ -79,7 +79,14 @@ body { background-color: var(--status-transitioning-bg); color: var(--status-transitioning); border: 1px solid var(--status-transitioning); - animation: pulse 2s infinite; + animation: none; +} + +/* Respect prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + .status-badge.transitioning { + animation: pulse 2s infinite; + } } /* ======================================== @@ -226,52 +233,81 @@ body { /* ======================================== Animations ======================================== */ -@keyframes pulse { - 0%, 100% { - opacity: 1; + +/* Respecte prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + @keyframes pulse { + 0%, 100% { + opacity: 1; + } + 50% { + opacity: 0.7; + } } - 50% { - opacity: 0.7; + + @keyframes fadeIn { + from { + opacity: 0; + transform: translateY(10px); + } + to { + opacity: 1; + transform: translateY(0); + } + } + + @keyframes spin { + from { + transform: rotate(0deg); + } + to { + transform: rotate(360deg); + } + } + + @keyframes slideInRight { + from { + transform: translateX(100%); + opacity: 0; + } + to { + transform: translateX(0); + opacity: 1; + } + } + + @keyframes pulse-opacity { + 0%, 100% { + opacity: 1; + } + 50% { + opacity: 0.5; + } + } + + .status-badge.transitioning { + animation: pulse 2s infinite; + } + + .shuffle-button.loading { + animation: pulse 1s infinite; + } + + .event-badge { + animation: pulse 2s ease-in-out infinite; + } + + .loading-state { + animation: pulse 1.5s ease-in-out infinite; } } -@keyframes fadeIn { - from { - opacity: 0; - transform: translateY(10px); - } - to { - opacity: 1; - transform: translateY(0); - } -} - -@keyframes spin { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } -} - -@keyframes slideInRight { - from { - opacity: 0; - transform: translateX(20px); - } - to { - opacity: 1; - transform: translateX(0); - } -} - -.fade-in { - animation: fadeIn var(--transition-base); -} - -.spin { - animation: spin 1s linear infinite; +/* Default state (animations disabled) */ +.status-badge.transitioning, +.shuffle-button.loading, +.event-badge, +.loading-state { + animation: none; } /* Reduced motion support */ @@ -546,10 +582,49 @@ input[type="range"]::-moz-range-thumb:hover { } /* ======================================== - ANIMATIONS + ANIMATIONS (Keyframes for no-preference media query) ======================================== */ +@keyframes fadeIn { + from { + opacity: 0; + transform: translateY(10px); + } + to { + opacity: 1; + transform: translateY(0); + } +} + +@keyframes spin { + from { + transform: rotate(0deg); + } + to { + transform: rotate(360deg); + } +} + +@keyframes slideInRight { + from { + transform: translateX(20px); + opacity: 0; + } + to { + transform: translateX(0); + opacity: 1; + } +} + +.fade-in { + animation: fadeIn var(--transition-base); +} + +.spin { + animation: spin 1s linear infinite; +} + @keyframes pulse-opacity { 0%, 100% { opacity: 1; } 50% { opacity: 0.5; } -} +} \ No newline at end of file diff --git a/pmoapp/webapp/src/assets/styles/variables.css b/pmoapp/webapp/src/assets/styles/variables.css index f9c0c982..d08bc754 100644 --- a/pmoapp/webapp/src/assets/styles/variables.css +++ b/pmoapp/webapp/src/assets/styles/variables.css @@ -89,13 +89,18 @@ --transition-base: 300ms ease-in-out; --transition-slow: 500ms ease-in-out; - /* ======================================== - Z-index layers - ======================================== */ +/* ======================================== + Z-index layers + ======================================== */ --z-dropdown: 100; --z-modal: 200; --z-toast: 300; --z-tooltip: 400; + + /* ======================================== + Opacity states + ======================================== */ + --opacity-disabled: 0.4; } /* Dark mode support (optionnel pour l'avenir) */ diff --git a/pmoapp/webapp/src/components/LogView.vue b/pmoapp/webapp/src/components/LogView.vue index 0652eeda..51d09035 100644 --- a/pmoapp/webapp/src/components/LogView.vue +++ b/pmoapp/webapp/src/components/LogView.vue @@ -981,7 +981,14 @@ button.active { padding: 3rem; color: #569cd6; font-size: 1.1rem; - animation: pulse 1.5s ease-in-out infinite; + animation: none; +} + +/* Respect prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + .loading-state { + animation: pulse 1.5s ease-in-out infinite; + } } @keyframes pulse { diff --git a/pmoapp/webapp/src/components/pmocontrol/ShuffleControl.vue b/pmoapp/webapp/src/components/pmocontrol/ShuffleControl.vue index d7e559da..3ad37889 100644 --- a/pmoapp/webapp/src/components/pmocontrol/ShuffleControl.vue +++ b/pmoapp/webapp/src/components/pmocontrol/ShuffleControl.vue @@ -76,7 +76,14 @@ async function handleShuffle() { } .shuffle-button.loading { - animation: pulse 1s infinite; + animation: none; +} + +/* Respect prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + .shuffle-button.loading { + animation: pulse 1s infinite; + } } @keyframes pulse { diff --git a/pmoapp/webapp/src/components/pmocontrol/TransportControls.vue b/pmoapp/webapp/src/components/pmocontrol/TransportControls.vue index aac9d5fb..5fbc29a7 100644 --- a/pmoapp/webapp/src/components/pmocontrol/TransportControls.vue +++ b/pmoapp/webapp/src/components/pmocontrol/TransportControls.vue @@ -68,6 +68,7 @@ async function handleNext() { :disabled="isPlaying" @click="handlePlay" title="Lecture" + aria-label="Lecture" > @@ -77,6 +78,7 @@ async function handleNext() { :disabled="isPaused || isStopped" @click="handlePause" title="Pause" + aria-label="Pause" > @@ -86,6 +88,7 @@ async function handleNext() { :disabled="isStopped" @click="handleStop" title="Stop" + aria-label="Arrêter" > @@ -95,6 +98,7 @@ async function handleNext() { :disabled="!state?.queue_len" @click="handleNext" title="Suivant" + aria-label="Morceau suivant" > diff --git a/pmoapp/webapp/src/components/pmocontrol/VolumeControl.vue b/pmoapp/webapp/src/components/pmocontrol/VolumeControl.vue index d7787592..2a5a24b8 100644 --- a/pmoapp/webapp/src/components/pmocontrol/VolumeControl.vue +++ b/pmoapp/webapp/src/components/pmocontrol/VolumeControl.vue @@ -65,6 +65,7 @@ async function handleToggleMute() { class="btn btn-icon" @click="handleToggleMute" :title="state?.mute ? 'Réactiver le son' : 'Couper le son'" + :aria-label="state?.mute ? 'Réactiver le son' : 'Couper le son'" > @@ -78,6 +79,7 @@ async function handleToggleMute() { @input="handleVolumeChange" class="volume-slider" :disabled="state?.mute ?? false" + aria-label="Volume" /> {{ localVolume }} diff --git a/pmoapp/webapp/src/components/upnp/VariablesList.vue b/pmoapp/webapp/src/components/upnp/VariablesList.vue index d85384f0..c3de0224 100644 --- a/pmoapp/webapp/src/components/upnp/VariablesList.vue +++ b/pmoapp/webapp/src/components/upnp/VariablesList.vue @@ -372,7 +372,14 @@ watch(editingVar, (newVar) => { .event-badge { font-size: 1rem; - animation: pulse 2s ease-in-out infinite; + animation: none; +} + +/* Respect prefers-reduced-motion */ +@media (prefers-reduced-motion: no-preference) { + .event-badge { + animation: pulse 2s ease-in-out infinite; + } } @keyframes pulse { diff --git a/pmoapp/webapp/src/composables/apiCache.ts b/pmoapp/webapp/src/composables/apiCache.ts index bc925ce2..95cbe4b9 100644 --- a/pmoapp/webapp/src/composables/apiCache.ts +++ b/pmoapp/webapp/src/composables/apiCache.ts @@ -11,6 +11,7 @@ export interface CacheEntry { data: T; timestamp: number; + ttl?: number; etag?: string; } @@ -51,7 +52,9 @@ class ApiCacheService { private isFresh(key: string): boolean { const entry = this.cache.get(key); if (!entry) return false; - return Date.now() - entry.timestamp < this.options.ttl; + // Lire le TTL de l'entrée, sinon utiliser le TTL global par défaut + const ttl = entry.ttl ?? this.options.ttl; + return Date.now() - entry.timestamp < ttl; } get(endpoint: string, params?: Record): T | null { @@ -66,12 +69,13 @@ class ApiCacheService { return entry.data; } - set(endpoint: string, data: T, params?: Record, etag?: string): void { + set(endpoint: string, data: T, params?: Record, etag?: string, ttl?: number): void { const key = this.makeKey(endpoint, params); this.cache.set(key, { data, timestamp: Date.now(), + ttl, etag, }); @@ -92,20 +96,34 @@ class ApiCacheService { if (cached) return cached; } - const existing = this.pendingRequests.get(key); - if (existing) { - return existing.promise as Promise; + // Utiliser une clé unique pour éviter les problèmes de race condition + // avec les requêtes en cours qui peuvent être supprimées avant résolution + const requestKey = `request:${key}`; + + // Récupérer ou créer la requête + let pendingRequest = this.pendingRequests.get(requestKey); + + // Si une requête est en cours et sa promesse n'a pas encore été resolved/rejected + // on retourne directement cette promesse + if (pendingRequest) { + try { + // Attendre la résolution pour s'assurer que c'est toujours valide + return await pendingRequest.promise as T; + } catch (e) { + // La requête a échoué, on continue pour faire une nouvelle requête + this.pendingRequests.delete(requestKey); + } } - let resolvePromise!: (value: unknown) => void; + let resolvePromise!: (value: T) => void; let rejectPromise!: (reason: unknown) => void; - const promise = new Promise((resolve, reject) => { + const promise = new Promise((resolve, reject) => { resolvePromise = resolve; rejectPromise = reject; }); - this.pendingRequests.set(key, { + this.pendingRequests.set(requestKey, { promise, subscribers: new Set(), }); @@ -113,18 +131,13 @@ class ApiCacheService { try { const data = await fetcher(); - if (ttl) { - const originalTtl = this.options.ttl; - this.options.ttl = ttl; - this.set(endpoint, data, params); - this.options.ttl = originalTtl; - } else { - this.set(endpoint, data, params); - } + // Passer le TTL directement à set() pour éviter les problèmes de race condition + // avec la modification globale de this.options.ttl + this.set(endpoint, data, params, undefined, ttl); resolvePromise(data); - const pending = this.pendingRequests.get(key); + const pending = this.pendingRequests.get(requestKey); if (pending) { pending.subscribers.forEach(cb => cb(data)); } @@ -133,10 +146,10 @@ class ApiCacheService { rejectPromise(error); throw error; } finally { - this.pendingRequests.delete(key); + this.pendingRequests.delete(requestKey); } - return Promise.reject(new Error('Unreachable')); + return promise; } subscribe(endpoint: string, params: Record, callback: (data: T) => void): () => void { @@ -182,7 +195,8 @@ class ApiCacheService { keysToDelete.forEach(key => { this.cache.delete(key); - this.subscriptions.delete(key); + // NE PAS supprimer this.subscriptions.get(key) + // Les abonnés seront notifiés lors du prochain set() après un refetch }); } diff --git a/pmoapp/webapp/src/composables/imageCache.ts b/pmoapp/webapp/src/composables/imageCache.ts index e6f95b5f..c7cf665f 100644 --- a/pmoapp/webapp/src/composables/imageCache.ts +++ b/pmoapp/webapp/src/composables/imageCache.ts @@ -37,9 +37,20 @@ class ImageCacheService { }; private readonly CACHE_CLEANUP_MS = 5 * 60 * 1000; + private cleanupIntervalId: ReturnType | null = null; constructor() { - setInterval(() => this.cleanup(), this.CACHE_CLEANUP_MS); + this.cleanupIntervalId = setInterval(() => this.cleanup(), this.CACHE_CLEANUP_MS); + } + + /** + * Nettoie le timer de cleanup. À appeler lors de la destruction de l'application. + */ + destroy(): void { + if (this.cleanupIntervalId !== null) { + clearInterval(this.cleanupIntervalId); + this.cleanupIntervalId = null; + } } configure(options: Partial) { diff --git a/pmoapp/webapp/src/composables/useMediaServers.ts b/pmoapp/webapp/src/composables/useMediaServers.ts index 12169055..51f4659b 100644 --- a/pmoapp/webapp/src/composables/useMediaServers.ts +++ b/pmoapp/webapp/src/composables/useMediaServers.ts @@ -20,11 +20,17 @@ export interface BrowseState { container_id: string entries: ContainerEntry[] total_count: number + hasMore?: boolean + currentOffset?: number } // Cache global partagé const serversCache = ref>(new Map()) const browseCache = ref>(new Map()) + +function browseCacheKey(serverId: string, containerId: string): string { + return `${encodeURIComponent(serverId)}:${encodeURIComponent(containerId)}` +} const currentPath = ref([]) const searchResults = ref(null) const searchQuery = ref('') @@ -69,9 +75,10 @@ function ensureSSEInitialized() { } // Invalider tout le cache browse de ce serveur + const encodedServerId = encodeURIComponent(serverId) const keysToDelete: string[] = [] browseCache.value.forEach((_, key) => { - if (key.startsWith(serverId + '/')) { + if (key.startsWith(encodedServerId + ':')) { keysToDelete.push(key) } }) @@ -80,9 +87,10 @@ function ensureSSEInitialized() { case 'global_updated': // Invalider tout le cache de ce serveur + const encodedServerIdGlobal = encodeURIComponent(serverId) const globalKeysToDelete: string[] = [] browseCache.value.forEach((_, key) => { - if (key.startsWith(serverId + '/')) { + if (key.startsWith(encodedServerIdGlobal + ':')) { globalKeysToDelete.push(key) } }) @@ -92,7 +100,7 @@ function ensureSSEInitialized() { case 'containers_updated': // Invalider les containers spécifiques event.container_ids.forEach(containerId => { - const key = `${serverId}/${containerId}` + const key = browseCacheKey(serverId, containerId) browseCache.value.delete(key) }) break @@ -142,7 +150,7 @@ export function useMediaServers() { // Charge la première page (remplace le cache) async function browseContainer(serverId: string, containerId: string, useCache = true) { - const key = `${serverId}/${containerId}` + const key = browseCacheKey(serverId, containerId) if (useCache && browseCache.value.has(key)) { return browseCache.value.get(key)! @@ -152,12 +160,14 @@ export function useMediaServers() { loading.value = true error.value = null - const data = await api.browseContainer(serverId, containerId, 0) + const data = await api.browseContainer(serverId, containerId, 0, 50) browseCache.value.set(key, { container_id: data.container_id, entries: data.entries, total_count: data.total_count, + hasMore: data.entries.length < data.total_count, + currentOffset: data.entries.length, }) return browseCache.value.get(key)! @@ -172,22 +182,26 @@ export function useMediaServers() { // Charge la page suivante et accumule (infinite scroll) async function loadMoreBrowse(serverId: string, containerId: string) { - const key = `${serverId}/${containerId}` + const key = browseCacheKey(serverId, containerId) const state = browseCache.value.get(key) if (!state) return - if (state.entries.length >= state.total_count) return + if (!('hasMore' in state) || !state.hasMore) return if (loadingMore.value) return try { loadingMore.value = true - const offset = state.entries.length + // Le type cast est nécessaire car les anciens cached entries n'ont pas hasMore + const state = browseCache.value.get(key) as BrowseState & { hasMore?: boolean; currentOffset?: number } + const offset = state.currentOffset ?? state.entries.length const data = await api.browseContainer(serverId, containerId, offset) // Accumuler les nouvelles entrées state.entries.push(...data.entries) state.total_count = data.total_count + state.currentOffset = state.entries.length + state.hasMore = state.entries.length < state.total_count // Forcer la réactivité browseCache.value.set(key, { ...state }) } catch (e) { @@ -236,12 +250,12 @@ export function useMediaServers() { } function getBrowseCached(serverId: string, containerId: string) { - const key = `${serverId}/${containerId}` + const key = browseCacheKey(serverId, containerId) return browseCache.value.get(key) } function hasMore(serverId: string, containerId: string): boolean { - const key = `${serverId}/${containerId}` + const key = browseCacheKey(serverId, containerId) const state = browseCache.value.get(key) if (!state) return false return state.entries.length < state.total_count @@ -259,12 +273,12 @@ export function useMediaServers() { // Invalidation du cache function invalidateCache(serverId: string, containerId?: string) { if (containerId) { - const key = `${serverId}/${containerId}` - browseCache.value.delete(key) + browseCache.value.delete(browseCacheKey(serverId, containerId)) } else { + const encodedServerId = encodeURIComponent(serverId) const keysToDelete: string[] = [] browseCache.value.forEach((_, key) => { - if (key.startsWith(serverId + '/')) { + if (key.startsWith(encodedServerId + ':')) { keysToDelete.push(key) } }) diff --git a/pmoapp/webapp/src/composables/useRenderers.ts b/pmoapp/webapp/src/composables/useRenderers.ts index d0c9d9e2..e38c9c2f 100644 --- a/pmoapp/webapp/src/composables/useRenderers.ts +++ b/pmoapp/webapp/src/composables/useRenderers.ts @@ -4,11 +4,12 @@ * - Les snapshots complets proviennent de /renderers/{id}/full * - Les événements SSE ne servent qu'à déclencher un refetch. */ -import { ref, reactive, computed, toRaw, type Ref } from "vue"; +import { ref, reactive, computed, type Ref, onUnmounted } from "vue"; import { api } from "../services/pmocontrol/api"; import { useSSE } from "./useSSE"; import { apiCache } from "./apiCache"; import { parseTimeToMs } from "../utils/time"; +import { useUIStore } from "@/stores/ui"; import type { RendererSummary, RendererState, @@ -16,33 +17,35 @@ import type { AttachedPlaylistInfo, FullRendererSnapshot, } from "../services/pmocontrol/types"; +import { isTransportState } from "../services/pmocontrol/types"; -interface RendererSnapshotState { - snapshots: Map; - lastSnapshotAt: Map; - lastEventAt: Map; - loadingIds: Set; - queueRefreshingIds: Set; - selectedRendererId: string | null; -} +// État global des snapshots avec reactive pour une réactivité native Vue sur les Maps +const snapshots = reactive(new Map()); +const lastSnapshotAt = reactive(new Map()); +const lastEventAt = reactive(new Map()); +const loadingIds = reactive(new Set()); +const queueRefreshingIds = reactive(new Set()); +const selectedRendererId = ref(null); +// Cache des renderers (summary) const renderersCache = ref>(new Map()); const RENDERERS_CACHE_MS = 2000; -const snapshotState = reactive({ - snapshots: reactive(new Map()), - lastSnapshotAt: reactive(new Map()), - lastEventAt: reactive(new Map()), - loadingIds: reactive(new Set()), - queueRefreshingIds: reactive(new Set()), - selectedRendererId: null, -}); +// Supprimé : les helpers triggerXXX ne sont plus nécessaires avec reactive const loading = ref(false); const error = ref(null); // Utiliser le composable SSE centralisé let sseInitialized = false; + +/** + * Réinitialise le flag SSE pour permettre une nouvelle connexion après reconnexion + */ +function resetSSE() { + sseInitialized = false; +} + function ensureSSEInitialized() { if (sseInitialized) return; @@ -99,16 +102,16 @@ function ensureSSEInitialized() { } // Supprimer le snapshot (il n'est plus valide) - snapshotState.snapshots.delete(rendererId); - snapshotState.lastSnapshotAt.delete(rendererId); - snapshotState.lastEventAt.delete(rendererId); + snapshots.delete(rendererId); + lastSnapshotAt.delete(rendererId); + lastEventAt.delete(rendererId); return; } // Pour les autres événements, mettre à jour le snapshot local directement - snapshotState.lastEventAt.set(rendererId, timestamp); + lastEventAt.set(rendererId, timestamp); - const snapshot = snapshotState.snapshots.get(rendererId); + const snapshot = snapshots.get(rendererId); // Si pas de snapshot, on doit fetch if (!snapshot) { @@ -119,7 +122,16 @@ function ensureSSEInitialized() { // Sinon, mettre à jour le snapshot localement selon le type d'événement switch (event.type) { case "state_changed": - snapshot.state.transport_state = event.state as any; + // Créer un nouvel objet pour déclencher la réactivité + if (isTransportState(event.state)) { + snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, transport_state: event.state }, + }); + + } else { + console.warn(`[useRenderers] transport_state inconnu: ${event.state}`); + } break; case "position_changed": @@ -128,35 +140,36 @@ function ensureSSEInitialized() { // Convertir rel_time (HH:MM:SS) en millisecondes const positionMs = parseTimeToMs(event.rel_time ?? null); - snapshot.state.position_ms = positionMs ?? 0; // Convertir track_duration (HH:MM:SS) en millisecondes const durationMs = parseTimeToMs(event.track_duration ?? null); - // Si track_duration est null/undefined (flux continu sans durée), - // mettre duration_ms à null pour afficher "--:--" - snapshot.state.duration_ms = durationMs; - - // Important: Trigger reactivity en réassignant l'objet complet avec deep copy - // Le shallow copy ne suffit pas car snapshot.state est partagé entre renderers - // Il faut copier state aussi pour éviter que les modifications d'un renderer - // n'affectent les autres renderers - // IMPORTANT: Utiliser toRaw() pour obtenir l'objet brut non-réactif avant de copier - // sinon Vue copie les getters réactifs qui continuent à pointer vers l'objet d'origine - const rawState = toRaw(snapshot.state); - const newState = { ...rawState }; - const newSnapshot = { + + // Créer un nouvel objet pour déclencher la réactivité + snapshots.set(rendererId, { ...snapshot, - state: newState, - }; - snapshotState.snapshots.set(rendererId, newSnapshot); + state: { + ...snapshot.state, + position_ms: positionMs ?? 0, + duration_ms: durationMs, + }, + }); + break; case "volume_changed": - snapshot.state.volume = event.volume; + // Créer un nouvel objet pour déclencher la réactivité + snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, volume: event.volume }, + }); break; case "mute_changed": - snapshot.state.mute = event.mute; + // Créer un nouvel objet pour déclencher la réactivité + snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state, mute: event.mute }, + }); break; case "metadata_changed": @@ -173,19 +186,21 @@ function ensureSSEInitialized() { snapshot.state.current_track.album = event.album; snapshot.state.current_track.album_art_uri = event.album_art_uri; // Important: Trigger reactivity en réassignant l'objet complet avec deep copy - snapshotState.snapshots.set(rendererId, { + snapshots.set(rendererId, { ...snapshot, state: { ...snapshot.state }, }); break; case "queue_refreshing": - snapshotState.queueRefreshingIds.add(rendererId); + queueRefreshingIds.add(rendererId); + break; case "queue_updated": snapshot.state.queue_len = event.queue_length; - snapshotState.queueRefreshingIds.delete(rendererId); + queueRefreshingIds.delete(rendererId); + // Pour la queue complète, on doit refetch void fetchRendererSnapshot(rendererId, { force: true }); break; @@ -202,10 +217,17 @@ function ensureSSEInitialized() { snapshot.binding = null; snapshot.state.attached_playlist = null; } + // Créer un nouvel objet pour déclencher la réactivité + snapshots.set(rendererId, { + ...snapshot, + state: { ...snapshot.state }, + }); break; case "stream_state_changed": snapshot.is_stream = event.is_stream; + // Créer un nouvel objet pour déclencher la réactivité + snapshots.set(rendererId, { ...snapshot }); break; case "timer_started": @@ -218,8 +240,8 @@ function ensureSSEInitialized() { break; } - // Trigger reactivity - snapshotState.snapshots.set(rendererId, snapshot); + // Note: chaque case est maintenant responsable de stocker le snapshot dans la Map + // Plus de réassignation finale après le switch }); sseInitialized = true; @@ -230,7 +252,7 @@ const onlineRenderers = computed(() => allRenderers.value.filter((r) => r.online), ); const allSnapshots = computed(() => - Array.from(snapshotState.snapshots.values()), + Array.from(snapshots.values()), ); const playingRenderers = computed(() => allSnapshots.value @@ -243,35 +265,38 @@ function getRendererById(id: string) { } function getSnapshotById(id: string) { - return snapshotState.snapshots.get(id) ?? null; + return snapshots.get(id) ?? null; } function getStateById(id: string): RendererState | null { - return snapshotState.snapshots.get(id)?.state ?? null; + return snapshots.get(id)?.state ?? null; } function getQueueById(id: string): QueueSnapshot | null { - return snapshotState.snapshots.get(id)?.queue ?? null; + return snapshots.get(id)?.queue ?? null; } function getBindingById(id: string): AttachedPlaylistInfo | null { - return snapshotState.snapshots.get(id)?.binding ?? null; + return snapshots.get(id)?.binding ?? null; } function isSnapshotLoading(id: string) { - return snapshotState.loadingIds.has(id); + return loadingIds.has(id); } function isQueueRefreshing(id: string) { - return snapshotState.queueRefreshingIds.has(id); + return queueRefreshingIds.has(id); } function selectRenderer(id: string | null) { - snapshotState.selectedRendererId = id; + selectedRendererId.value = id; } async function fetchRenderers(force = false, retries = 2) { ensureSSEInitialized(); + + // Créer le store UI pour les notifications (lazy import pour éviter les effets de bord) + const uiStore = useUIStore(); let lastError: Error | null = null; @@ -304,6 +329,9 @@ async function fetchRenderers(force = false, retries = 2) { } error.value = lastError?.message ?? "Erreur fetch renderers"; + + // Notifier l'utilisateur en cas d'erreur finale + uiStore.notifyError("Impossible de rafraîchir la liste des renderers"); } async function fetchRendererSnapshot( @@ -312,29 +340,43 @@ async function fetchRendererSnapshot( ) { ensureSSEInitialized(); const force = opts?.force ?? false; - const hasSnapshot = snapshotState.snapshots.has(rendererId); + const hasSnapshot = snapshots.has(rendererId); if (!force && hasSnapshot) { - const lastSnapshot = snapshotState.lastSnapshotAt.get(rendererId) ?? 0; - const lastEvent = snapshotState.lastEventAt.get(rendererId) ?? 0; + const lastSnapshot = lastSnapshotAt.get(rendererId) ?? 0; + const lastEvent = lastEventAt.get(rendererId) ?? 0; if (lastEvent <= lastSnapshot) { return; } } - if (snapshotState.loadingIds.has(rendererId)) { + // Éviter les requêtes multiples simultanées pour le même renderer + if (loadingIds.has(rendererId)) { return; } - snapshotState.loadingIds.add(rendererId); + loadingIds.add(rendererId); + + + // Lazy load UI store pour les notifications + const uiStore = useUIStore(); + try { const snapshot = await api.getRendererFullSnapshot(rendererId); - snapshotState.snapshots.set(rendererId, snapshot); - snapshotState.lastSnapshotAt.set(rendererId, Date.now()); + snapshots.set(rendererId, snapshot); + lastSnapshotAt.set(rendererId, Date.now()); + } catch (err) { console.error(`[useRenderers] Erreur snapshot ${rendererId}:`, err); + // En cas d'erreur, on supprime le snapshot pour permettre une nouvelle tentative + snapshots.delete(rendererId); + + // Notifier l'utilisateur + uiStore.notifyError(`Impossible de récupérer l'état du renderer`); } finally { - snapshotState.loadingIds.delete(rendererId); + // Toujours nettoyer le flag de chargement + loadingIds.delete(rendererId); + } } @@ -384,7 +426,7 @@ async function play(id: string) { } async function resumeOrPlayFromQueue(id: string) { - const snapshot = snapshotState.snapshots.get(id); + const snapshot = snapshots.get(id); if (!snapshot) { throw new Error(`Renderer ${id} non trouvé`); } @@ -506,7 +548,6 @@ export function useRenderers() { isSnapshotLoading, isQueueRefreshing, selectRenderer, - snapshotState, // Fetchers fetchRenderers, fetchRendererSnapshot, @@ -530,6 +571,8 @@ export function useRenderers() { playContent, addToQueue, addAfterCurrent, + // SSE + resetSSE, }; } @@ -545,10 +588,33 @@ export function useRenderer(rendererId: Ref) { const isStream = computed(() => snapshot.value?.is_stream ?? false); const queueRefreshing = computed(() => isQueueRefreshing(rendererId.value)); + // Debounce pour éviter les refreshs multiples trop fréquents + let refreshDebounceTimer: ReturnType | null = null; + const REFRESH_DEBOUNCE_MS = 500; + + // Nettoyer le timer debounce si le composant est démonté + onUnmounted(() => { + if (refreshDebounceTimer !== null) { + clearTimeout(refreshDebounceTimer); + refreshDebounceTimer = null; + } + }); + async function refresh(force = true) { + const currentRendererId = rendererId.value; + + // Debounce: ignorer si un refresh est en cours pour ce renderer + if (refreshDebounceTimer !== null) { + return; + } + + refreshDebounceTimer = setTimeout(() => { + refreshDebounceTimer = null; + }, REFRESH_DEBOUNCE_MS); + await Promise.all([ fetchRenderers(force), - fetchRendererSnapshot(rendererId.value, { force: true }), + fetchRendererSnapshot(currentRendererId, { force: true }), ]); } diff --git a/pmoapp/webapp/src/composables/useSSE.ts b/pmoapp/webapp/src/composables/useSSE.ts index 97bf1a51..e8284c43 100644 --- a/pmoapp/webapp/src/composables/useSSE.ts +++ b/pmoapp/webapp/src/composables/useSSE.ts @@ -17,13 +17,17 @@ import type { } from '../services/pmocontrol/types' // État global partagé -const connected = ref(sse.isConnectedState()) -const connectionCallbacks: Set<(connected: boolean) => void> = new Set() +const connected = ref(sse.isConnectedState()); +const connectionCallbacks: Set<(connected: boolean) => void> = new Set(); + +// Flag pour éviter les double-connexions SSE avec lock +let connectionLock = false; // Abonnement à l'état de connexion global function setupConnectionListener() { - // S'assurer qu'on ne s'abonne qu'une seule fois - if (connectionCallbacks.size === 0) { + // Vérifier avec lock pour éviter les conditions de course + if (connectionCallbacks.size === 0 && !connectionLock) { + connectionLock = true; sse.onConnectionChange((isConnected) => { connected.value = isConnected connectionCallbacks.forEach(cb => cb(isConnected)) diff --git a/pmoapp/webapp/src/composables/useTabs.ts b/pmoapp/webapp/src/composables/useTabs.ts index d541dc9b..fba88964 100644 --- a/pmoapp/webapp/src/composables/useTabs.ts +++ b/pmoapp/webapp/src/composables/useTabs.ts @@ -43,6 +43,10 @@ const state = reactive({ // Flag pour éviter les boucles de sauvegarde let isRestoringFromStorage = false; +// Debounce timer pour la sauvegarde localStorage +let saveDebounceTimer: ReturnType | null = null; +const SAVE_DEBOUNCE_MS = 100; + /** * Retourne le titre complet sans troncature * Note: On laisse le CSS gérer l'overflow avec ellipsis pour un affichage stable @@ -53,29 +57,38 @@ function truncateTitle(title: string): string { } /** - * Sauvegarde l'état dans localStorage + * Sauvegarde l'état dans localStorage (avec debounce) * Note: On ne sauvegarde que les onglets server (les renderer tabs sont auto-générés) */ function saveToLocalStorage() { if (isRestoringFromStorage) return; - try { - const stateToSave = { - // Sauvegarder uniquement les onglets server (fermables manuellement) - tabs: state.tabs - .filter((tab) => tab.type === "server") - .map((tab) => ({ - ...tab, - // On ne peut pas sauvegarder les composants Vue, on sauve juste le type - icon: undefined, - })), - activeTabId: state.activeTabId, - tabHistory: state.tabHistory, - }; - localStorage.setItem(STORAGE_KEY, JSON.stringify(stateToSave)); - } catch (error) { - console.error("[useTabs] Erreur sauvegarde localStorage:", error); + // Annuler le timer précédent + if (saveDebounceTimer !== null) { + clearTimeout(saveDebounceTimer); } + + // Débouncer pour éviter les écritures multiples + saveDebounceTimer = setTimeout(() => { + try { + const stateToSave = { + // Sauvegarder uniquement les onglets server (fermables manuellement) + tabs: state.tabs + .filter((tab) => tab.type === "server") + .map((tab) => ({ + ...tab, + // On ne peut pas sauvegarder les composants Vue, on sauve juste le type + icon: undefined, + })), + activeTabId: state.activeTabId, + tabHistory: state.tabHistory, + }; + localStorage.setItem(STORAGE_KEY, JSON.stringify(stateToSave)); + } catch (error) { + console.error("[useTabs] Erreur sauvegarde localStorage:", error); + } + saveDebounceTimer = null; + }, SAVE_DEBOUNCE_MS); } /** @@ -83,11 +96,11 @@ function saveToLocalStorage() { * Note: Restaure uniquement les onglets server (les renderer tabs seront auto-générés) */ function restoreFromLocalStorage() { + isRestoringFromStorage = true; try { const saved = localStorage.getItem(STORAGE_KEY); if (!saved) return; - isRestoringFromStorage = true; const savedState = JSON.parse(saved); // Reconstituer uniquement les tabs server avec les bonnes icônes @@ -110,10 +123,9 @@ function restoreFromLocalStorage() { if (!state.tabs.find((t) => t.id === state.activeTabId)) { state.activeTabId = ""; } - - isRestoringFromStorage = false; } catch (error) { console.error("[useTabs] Erreur restauration localStorage:", error); + } finally { isRestoringFromStorage = false; } } @@ -346,12 +358,18 @@ function openServer(server: MediaServerSummary | undefined) { */ export function useTabs() { // Watch pour sauvegarde automatique (uniquement les server tabs) + // Watch séparés sans deep pour éviter la sérialisation complète à chaque mutation + watch(() => state.activeTabId, () => { + saveToLocalStorage(); + }); + watch(() => state.tabHistory.length, () => { + saveToLocalStorage(); + }); watch( - () => [state.tabs, state.activeTabId, state.tabHistory], + () => state.tabs.map(t => t.id + t.type + (t.metadata?.rendererId ?? '') + (t.metadata?.serverId ?? '')).join('|'), () => { saveToLocalStorage(); }, - { deep: true }, ); // Restaurer au montage (uniquement les server tabs) diff --git a/pmoapp/webapp/src/main.ts b/pmoapp/webapp/src/main.ts index 4eac9af1..3d52e925 100644 --- a/pmoapp/webapp/src/main.ts +++ b/pmoapp/webapp/src/main.ts @@ -9,6 +9,9 @@ import { sse } from "./services/pmocontrol/sse"; // Store UI (garde UIStore pour les notifications et état UI global) import { useUIStore } from "./stores/ui"; +// Image cache (pour cleanup) +import { imageCache } from "./composables/imageCache"; + // Styles import "./style.css"; import "./assets/styles/variables.css"; @@ -24,12 +27,13 @@ const pinia = createPinia(); app.use(pinia); app.use(router); +// Initialiser UIStore AVANT le montage pour éviter la race condition (P2) +const uiStore = useUIStore(); + // Monter l'application app.mount("#app"); // Après montage, initialiser SSE -const uiStore = useUIStore(); - // Les composables se connectent automatiquement à SSE // Ils gèrent eux-mêmes le re-fetch lors des événements @@ -39,3 +43,8 @@ sse.onConnectionChange((connected) => { // Démarrer la connexion SSE sse.connect(); + +// Cleanup global lors du unload de la page +window.addEventListener('beforeunload', () => { + imageCache.destroy(); +}); diff --git a/pmoapp/webapp/src/router/index.ts b/pmoapp/webapp/src/router/index.ts index 9ec63fcf..a5fee922 100644 --- a/pmoapp/webapp/src/router/index.ts +++ b/pmoapp/webapp/src/router/index.ts @@ -1,4 +1,5 @@ import { createRouter, createWebHistory } from "vue-router"; +import type { RouteRecordRaw } from "vue-router"; // PMOControl Unified View (nouvelle interface unifiée) import UnifiedControlView from "../views/UnifiedControlView.vue"; @@ -8,18 +9,10 @@ import DashboardView from "../views/DashboardView.vue"; import RendererView from "../views/RendererView.vue"; import MediaServerView from "../views/MediaServerView.vue"; -// Debug Components (anciennes routes) -import GenericMusicPlayer from "../components/GenericMusicPlayer.vue"; -import LogView from "../components/LogView.vue"; -import CoverCacheManager from "../components/CoverCacheManager.vue"; -import AudioCacheManager from "../components/AudioCacheManager.vue"; -import PlayListManager from "../components/PlayListManager.vue"; -import UpnpExplorer from "../components/UpnpExplorer.vue"; -import APIDashboard from "../components/APIDashboard.vue"; -import RadioParadiseExplorer from "../components/RadioParadiseExplorer.vue"; -import DebugView from "../views/DebugView.vue"; +// Debug Components - lazy loaded uniquement en mode développement (P8) +const isDev = import.meta.env.DEV; -const routes = [ +const routes: RouteRecordRaw[] = [ // PMOControl Unified Interface (nouvelle interface unifiée avec onglets) { path: "/", @@ -43,57 +36,65 @@ const routes = [ name: "MediaServer", component: MediaServerView, }, - - // Debug hub - { - path: "/debug", - name: "Debug", - component: DebugView, - }, - - // Debug menu (anciennes routes déplacées sous /debug) - { - path: "/debug/generic-player", - name: "GenericPlayer", - component: GenericMusicPlayer, - }, - { - path: "/debug/logs", - name: "Logs", - component: LogView, - }, - { - path: "/debug/covers-cache", - name: "CoversCache", - component: CoverCacheManager, - }, - { - path: "/debug/audio-cache", - name: "AudioCache", - component: AudioCacheManager, - }, - { - path: "/debug/playlists", - name: "PlaylistsManager", - component: PlayListManager, - }, - { - path: "/debug/upnp", - name: "UpnpExplorer", - component: UpnpExplorer, - }, - { - path: "/debug/api-dashboard", - name: "APIDashboard", - component: APIDashboard, - }, - { - path: "/debug/radio-paradise", - name: "RadioParadise", - component: RadioParadiseExplorer, - }, ]; +// Ajouter les routes de debug uniquement en développement +if (isDev) { + routes.push( + { + path: "/debug", + name: "Debug", + component: () => import("../views/DebugView.vue"), + }, + { + path: "/debug/generic-player", + name: "GenericPlayer", + component: () => import("../components/GenericMusicPlayer.vue"), + }, + { + path: "/debug/logs", + name: "Logs", + component: () => import("../components/LogView.vue"), + }, + { + path: "/debug/covers-cache", + name: "CoversCache", + component: () => import("../components/CoverCacheManager.vue"), + }, + { + path: "/debug/audio-cache", + name: "AudioCache", + component: () => import("../components/AudioCacheManager.vue"), + }, + { + path: "/debug/playlists", + name: "PlaylistsManager", + component: () => import("../components/PlayListManager.vue"), + }, + { + path: "/debug/upnp", + name: "UpnpExplorer", + component: () => import("../components/UpnpExplorer.vue"), + }, + { + path: "/debug/api-dashboard", + name: "APIDashboard", + component: () => import("../components/APIDashboard.vue"), + }, + { + path: "/debug/radio-paradise", + name: "RadioParadise", + component: () => import("../components/RadioParadiseExplorer.vue"), + } + ); +} + +// Wildcard redirect pour les routes inconnues +routes.push({ + path: "/:pathMatch(.*)*", + redirect: "/", +}); + const router = createRouter({ // history avec base /app history: createWebHistory("/app"), diff --git a/pmoapp/webapp/src/services/PMOPlayer.ts b/pmoapp/webapp/src/services/PMOPlayer.ts index 23d4431d..0ff2bcde 100644 --- a/pmoapp/webapp/src/services/PMOPlayer.ts +++ b/pmoapp/webapp/src/services/PMOPlayer.ts @@ -33,6 +33,58 @@ export interface TrackInfo { cover?: string; } +// Types stricts pour les commandes reçues du backend (P6) +interface StreamCommand { + type: 'stream'; + url: string; +} + +interface PlayCommand { + type: 'play'; +} + +interface PauseCommand { + type: 'pause'; +} + +interface SeekCommand { + type: 'seek'; + timestamp: number; +} + +interface FlushCommand { + type: 'flush'; +} + +interface StopCommand { + type: 'stop'; +} + +type CommandMessage = StreamCommand | PlayCommand | PauseCommand | SeekCommand | FlushCommand | StopCommand; + +function isValidCommand(msg: Record | unknown): msg is CommandMessage { + if (!msg || typeof msg !== 'object') return false; + if (!('type' in msg)) return false; + + const type = (msg as Record).type; + if (typeof type !== 'string') return false; + + // Valider les champs selon le type + switch (type) { + case 'stream': + return 'url' in msg && typeof (msg as StreamCommand).url === 'string'; + case 'seek': + return 'timestamp' in msg && typeof (msg as SeekCommand).timestamp === 'number'; + case 'play': + case 'pause': + case 'flush': + case 'stop': + return true; + default: + return false; + } +} + export class PMOPlayer { private audio: HTMLAudioElement; private instanceId: string; @@ -195,11 +247,17 @@ export class PMOPlayer { } private handleCommand(msg: Record) { - const type = msg.type as string; + // Validate command structure before processing (P6) + if (!isValidCommand(msg)) { + console.warn('[PMOPlayer] Invalid command received:', msg); + return; + } + + const type = msg.type; switch (type) { case 'stream': { - this.playStream(msg.url as string); + this.playStream(msg.url); break; } case 'play': { @@ -211,7 +269,7 @@ export class PMOPlayer { this.pause(); break; case 'seek': - this.seek(msg.timestamp as number); + this.seek(msg.timestamp); break; case 'flush': this.flush(); diff --git a/pmoapp/webapp/src/services/coverCache.ts b/pmoapp/webapp/src/services/coverCache.ts index bc391a62..b9f33ebb 100644 --- a/pmoapp/webapp/src/services/coverCache.ts +++ b/pmoapp/webapp/src/services/coverCache.ts @@ -200,34 +200,11 @@ export function getJpegUrl(pk: string, size?: number): string { return `/covers/jpeg/${pk}`; } -/** - * SVG par défaut pour les images qui ne se chargent pas - */ -const DEFAULT_COVER_SVG = ` - - - - - - - - - - - - - - - - No Image Available - -`; +import defaultCoverSvg from '../assets/default-cover.svg?raw'; /** * Retourne l'URL de l'image par défaut comme data URL */ export function getDefaultImageUrl(): string { - return `data:image/svg+xml;utf8,${encodeURIComponent(DEFAULT_COVER_SVG)}`; + return `data:image/svg+xml;utf8,${encodeURIComponent(defaultCoverSvg)}`; } diff --git a/pmoapp/webapp/src/services/pmocontrol/api.ts b/pmoapp/webapp/src/services/pmocontrol/api.ts index fb846f62..5db6836c 100644 --- a/pmoapp/webapp/src/services/pmocontrol/api.ts +++ b/pmoapp/webapp/src/services/pmocontrol/api.ts @@ -17,12 +17,45 @@ import type { ErrorResponse, } from "./types"; +/** + * Fetch avec timeout et AbortController + */ +function fetchWithTimeout( + url: string, + options: RequestInit = {}, + timeoutMs = 10_000, +): Promise { + const controller = new AbortController(); + const id = setTimeout(() => controller.abort(), timeoutMs); + return fetch(url, { ...options, signal: controller.signal }).finally( + () => clearTimeout(id), + ); +} + /** * Client API REST pour le Control Point PMOMusic */ class PMOControlAPI { private readonly baseURL = "/api/control"; + /** + * Valide la structure de base d'une réponse + * Jette une erreur si la réponse est invalide + */ + private validateResponse(data: unknown, path: string): T { + // Vérification basique : null ou undefined + if (data == null) { + throw new Error(`[PMOControlAPI] Réponse nulle pour ${path}`); + } + + // Vérification que c'est un objet + if (typeof data !== 'object') { + throw new Error(`[PMOControlAPI] Réponse invalide pour ${path}: attendu un objet`); + } + + return data as T; + } + /** * Effectue une requête HTTP générique */ @@ -32,7 +65,7 @@ class PMOControlAPI { ): Promise { const url = `${this.baseURL}${path}`; - const response = await fetch(url, { + const response = await fetchWithTimeout(url, { ...options, headers: { "Content-Type": "application/json", @@ -47,7 +80,15 @@ class PMOControlAPI { throw new Error(error.error); } - return response.json(); + const data = await response.json(); + + // Validation de la réponse (P5) + const validated = this.validateResponse(data, path); + + if (import.meta.env.DEV && data == null) { + console.warn(`[PMOControlAPI] Réponse vide pour ${path}`); + } + return validated; } // ============================================================================ diff --git a/pmoapp/webapp/src/services/pmocontrol/types.ts b/pmoapp/webapp/src/services/pmocontrol/types.ts index 8c4f6f96..e840457f 100644 --- a/pmoapp/webapp/src/services/pmocontrol/types.ts +++ b/pmoapp/webapp/src/services/pmocontrol/types.ts @@ -39,13 +39,7 @@ export interface RendererSummary { export interface RendererState { id: string; friendly_name: string; - transport_state: - | "PLAYING" - | "PAUSED" - | "STOPPED" - | "TRANSITIONING" - | "NO_MEDIA" - | "UNKNOWN"; + transport_state: TransportState; position_ms: number | null; duration_ms: number | null; volume: number | null; // 0-100 @@ -55,6 +49,15 @@ export interface RendererState { current_track: CurrentTrackMetadata | null; } +export type TransportState = "PLAYING" | "PAUSED" | "STOPPED" | "TRANSITIONING" | "NO_MEDIA" | "UNKNOWN"; + +/** + * Guard de type pour valider que string est un TransportState valide + */ +export function isTransportState(s: string): s is TransportState { + return ["PLAYING", "PAUSED", "STOPPED", "TRANSITIONING", "NO_MEDIA", "UNKNOWN"].includes(s); +} + export interface CurrentTrackMetadata { title: string | null; artist: string | null; @@ -123,6 +126,7 @@ export interface BrowseResponse { entries: ContainerEntry[]; total_count: number; offset: number; + hasMore?: boolean; // Client-side flag pour infinite scroll } // ============================================================================ diff --git a/pmoapp/webapp/src/stores/ui.ts b/pmoapp/webapp/src/stores/ui.ts index 029fea22..93ea1020 100644 --- a/pmoapp/webapp/src/stores/ui.ts +++ b/pmoapp/webapp/src/stores/ui.ts @@ -9,6 +9,8 @@ export interface Notification { duration?: number // ms, undefined = permanent } +const MAX_NOTIFICATIONS = 5 + export const useUIStore = defineStore('ui', () => { // État const selectedRendererId = ref(null) @@ -17,6 +19,9 @@ export const useUIStore = defineStore('ui', () => { const sseConnected = ref(false) const notifications = ref([]) + // Map pour suivre les timers et permettre le cleanup + const notificationTimers = new Map>() + // Actions function selectRenderer(id: string | null) { selectedRendererId.value = id @@ -39,6 +44,18 @@ export const useUIStore = defineStore('ui', () => { message: string, duration?: number ) { + // Limiter le nombre de notifications (P15) + if (notifications.value.length >= MAX_NOTIFICATIONS) { + const oldest = notifications.value.shift() + if (oldest) { + const timer = notificationTimers.get(oldest.id) + if (timer) { + clearTimeout(timer) + notificationTimers.delete(oldest.id) + } + } + } + const id = `notif-${Date.now()}-${Math.random()}` const notification: Notification = { id, @@ -49,12 +66,13 @@ export const useUIStore = defineStore('ui', () => { notifications.value.push(notification) - // Auto-remove après duration (défaut: 5s) + // Auto-remove après duration (défaut: 5s) - avec tracking pour cleanup const timeout = duration !== undefined ? duration : 5000 if (timeout > 0) { - setTimeout(() => { + const timer = setTimeout(() => { removeNotification(id) }, timeout) + notificationTimers.set(id, timer) } return id @@ -65,12 +83,27 @@ export const useUIStore = defineStore('ui', () => { if (index !== -1) { notifications.value.splice(index, 1) } + // Nettoyer le timer associated + const timer = notificationTimers.get(id) + if (timer) { + clearTimeout(timer) + notificationTimers.delete(id) + } } function clearNotifications() { + // Nettoyer tous les timers + notificationTimers.forEach(timer => clearTimeout(timer)) + notificationTimers.clear() notifications.value = [] } + // Cleanup function pour appeler lors du unmount de l'app + function $dispose() { + notificationTimers.forEach(timer => clearTimeout(timer)) + notificationTimers.clear() + } + // Raccourcis pour les types de notifications function notifySuccess(message: string, duration?: number) { return addNotification('success', message, duration) @@ -107,5 +140,6 @@ export const useUIStore = defineStore('ui', () => { notifyError, notifyWarning, notifyInfo, + $dispose, } }) diff --git a/pmoapp/webapp/src/utils/string.ts b/pmoapp/webapp/src/utils/string.ts index 1a825678..011ec7df 100644 --- a/pmoapp/webapp/src/utils/string.ts +++ b/pmoapp/webapp/src/utils/string.ts @@ -45,5 +45,9 @@ export function normalizeUrl(url: string): string { */ export function truncate(str: string, maxLength: number, suffix = '...'): string { if (str.length <= maxLength) return str; + // Guard: si suffix est plus long que maxLength, retourner juste le suffixe + if (suffix.length >= maxLength) { + return str.slice(0, maxLength); + } return str.slice(0, maxLength - suffix.length) + suffix; } \ No newline at end of file diff --git a/pmoapp/webapp/src/utils/time.ts b/pmoapp/webapp/src/utils/time.ts index 98edbe81..76e27a7f 100644 --- a/pmoapp/webapp/src/utils/time.ts +++ b/pmoapp/webapp/src/utils/time.ts @@ -49,12 +49,3 @@ export function formatMsToTime(ms: number | null): string { return `${h}${m}${s}`; } - -/** - * Convertit des millisecondes en format court (pour l'affichage progress) - * @param ms - Durée en millisecondes - * @returns Durée au format "X:XX" ou "X:XX:XX" - */ -export function formatMsToShortTime(ms: number | null): string { - return formatMsToTime(ms); -} \ No newline at end of file diff --git a/pmoapp/webapp/src/views/UnifiedControlView.vue b/pmoapp/webapp/src/views/UnifiedControlView.vue index e7e42f1b..978fd011 100644 --- a/pmoapp/webapp/src/views/UnifiedControlView.vue +++ b/pmoapp/webapp/src/views/UnifiedControlView.vue @@ -34,17 +34,19 @@ const rendererDrawerOpen = ref(false); // Ref pour le swipe edge detection const viewRef = ref(null); +// Position initiale du swipe pour détecter un swipe depuis le bord gauche +const swipeStartX = ref(0); + // Swipe depuis le bord gauche pour ouvrir le drawer useSwipe(viewRef, { threshold: 50, + onSwipeStart(e: TouchEvent) { + swipeStartX.value = e.touches[0]?.clientX ?? 0; + }, onSwipeEnd(_e: TouchEvent, swipeDirection: string) { // Swipe right depuis le bord gauche → ouvrir drawer - if (swipeDirection === "right" && !drawerOpen.value) { - const touch = _e.changedTouches[0]; - // Vérifier que le swipe commence depuis le bord gauche (< 50px) - if (touch && touch.clientX < 50) { - drawerOpen.value = true; - } + if (swipeDirection === "right" && !drawerOpen.value && swipeStartX.value < 50) { + drawerOpen.value = true; } }, }); @@ -135,12 +137,18 @@ onMounted(async () => { }); // Watch renderers pour sync automatique des tabs +// Note: on watch la taille du tableau + les IDs pour éviter un deep watch coûteux watch( - () => allRenderers.value, - (newRenderers) => { - syncWithRenderers(filterRenderers(newRenderers)); + () => ({ + length: allRenderers.value.length, + ids: allRenderers.value.map(r => r.id).join(','), + }), + (newVal, oldVal) => { + // Re-sync uniquement si le nombre ou les IDs ont changé + if (newVal.length !== oldVal?.length || newVal.ids !== oldVal?.ids) { + syncWithRenderers(filterRenderers(allRenderers.value)); + } }, - { deep: true }, ); // Watch l'UDN du WebRenderer local : quand il s'établit, resync pour faire apparaître notre onglet diff --git a/version.txt b/version.txt index ba61ff30..a7c15395 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.3.37 +0.3.39