[enhancement]: better version checks for lyrics, Navidrome (#529)

- Actually make serverfeatures partial - Navidrome: only set multiple structured lyrics if extension exists - Navidrome/Subsonic: minor type checking of OS extension (Navidrome implementation detail) - Jellyfin: add separate knob for lyrics. Note, this should also probably be behind some version check...
jeffvli · Mar 5, 2024 · 73845a9 · 73845a9
1 parent d52d913
commit 73845a9
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 34 deletions.
diff --git a/src/renderer/api/features.types.ts b/src/renderer/api/features.types.ts
@@ -1,6 +1,7 @@
 export enum ServerFeature {
+    MULTIPLE_STRUCTURED_LYRICS = 'multipleStructuredLyrics',
+    SINGLE_STRUCTURED_LYRIC = 'singleStructuredLyric',
     SMART_PLAYLISTS = 'smartPlaylists',
-    SONG_LYRICS = 'songLyrics',
 }
 
-export type ServerFeatures = Record<Partial<ServerFeature>, boolean>;
+export type ServerFeatures = Partial<Record<ServerFeature, boolean>>;
diff --git a/src/renderer/api/jellyfin/jellyfin-controller.ts b/src/renderer/api/jellyfin/jellyfin-controller.ts
@@ -961,8 +961,7 @@ const getServerInfo = async (args: ServerInfoArgs): Promise<ServerInfo> => {
     }
 
     const features: ServerFeatures = {
-        smartPlaylists: false,
-        songLyrics: true,
+        singleStructuredLyric: true,
     };
 
     return {

diff --git a/src/renderer/api/navidrome/navidrome-controller.ts b/src/renderer/api/navidrome/navidrome-controller.ts
@@ -50,6 +50,7 @@ import {
 } from '../types';
 import { hasFeature } from '/@/renderer/api/utils';
 import { ServerFeature, ServerFeatures } from '/@/renderer/api/features.types';
+import { SubsonicExtensions } from '/@/renderer/api/subsonic/subsonic-types';
 
 const authenticate = async (
     url: string,
@@ -528,20 +529,21 @@ const getServerInfo = async (args: ServerInfoArgs): Promise<ServerInfo> => {
             throw new Error('Failed to get server extensions');
         }
 
-        for (const extension of res.body.openSubsonicExtensions) {
-            navidromeFeatures[extension.name] = extension.versions;
+        // The type here isn't necessarily an array (even though it's supposed to be). This is
+        // an implementation detail of Navidrome 0.50. Do a type check to make sure it's actually
+        // an array, and not an empty object.
+        if (Array.isArray(res.body.openSubsonicExtensions)) {
+            for (const extension of res.body.openSubsonicExtensions) {
+                navidromeFeatures[extension.name] = extension.versions;
+            }
         }
     }
 
     const features: ServerFeatures = {
-        smartPlaylists: false,
-        songLyrics: true,
+        multipleStructuredLyrics: !!navidromeFeatures[SubsonicExtensions.SONG_LYRICS],
+        smartPlaylists: !!navidromeFeatures[NavidromeExtensions.SMART_PLAYLISTS],
     };
 
-    if (navidromeFeatures[NavidromeExtensions.SMART_PLAYLISTS]) {
-        features[ServerFeature.SMART_PLAYLISTS] = true;
-    }
-
     return { features, id: apiClientProps.server?.id, version: ping.body.serverVersion! };
 };
 

diff --git a/src/renderer/api/subsonic/subsonic-controller.ts b/src/renderer/api/subsonic/subsonic-controller.ts
@@ -384,10 +384,7 @@ const getServerInfo = async (args: ServerInfoArgs): Promise<ServerInfo> => {
         throw new Error('Failed to ping server');
     }
 
-    const features: ServerFeatures = {
-        smartPlaylists: false,
-        songLyrics: false,
-    };
+    const features: ServerFeatures = {};
 
     if (!ping.body.openSubsonic || !ping.body.serverVersion) {
         return { features, version: ping.body.version };
@@ -400,12 +397,14 @@ const getServerInfo = async (args: ServerInfoArgs): Promise<ServerInfo> => {
     }
 
     const subsonicFeatures: Record<string, number[]> = {};
-    for (const extension of res.body.openSubsonicExtensions) {
-        subsonicFeatures[extension.name] = extension.versions;
+    if (Array.isArray(res.body.openSubsonicExtensions)) {
+        for (const extension of res.body.openSubsonicExtensions) {
+            subsonicFeatures[extension.name] = extension.versions;
+        }
     }
 
     if (subsonicFeatures[SubsonicExtensions.SONG_LYRICS]) {
-        features.songLyrics = true;
+        features.multipleStructuredLyrics = true;
     }
 
     return { features, id: apiClientProps.server?.id, version: ping.body.serverVersion };

diff --git a/src/renderer/api/subsonic/subsonic-types.ts b/src/renderer/api/subsonic/subsonic-types.ts
@@ -218,7 +218,7 @@ const extension = z.object({
 });
 
 const serverInfo = z.object({
-    openSubsonicExtensions: z.array(extension),
+    openSubsonicExtensions: z.array(extension).optional(),
 });
 
 const structuredLyricsParameters = z.object({
@@ -266,7 +266,6 @@ export enum SubsonicExtensions {
     TRANSCODE_OFFSET = 'transcodeOffset',
 }
 
-
 export const ssType = {
     _parameters: {
         albumList: albumListParameters,

diff --git a/src/renderer/api/utils.ts b/src/renderer/api/utils.ts
@@ -45,5 +45,5 @@ export const hasFeature = (server: ServerListItem | null, feature: ServerFeature
         return false;
     }
 
-    return server.features[feature];
+    return server.features[feature] ?? false;
 };
diff --git a/src/renderer/features/lyrics/queries/lyric-query.ts b/src/renderer/features/lyrics/queries/lyric-query.ts
@@ -96,7 +96,18 @@ export const useSongLyricsBySong = (
             if (!server) throw new Error('Server not found');
             if (!song) return null;
 
-            if (server.type === ServerType.JELLYFIN) {
+            if (hasFeature(server, ServerFeature.MULTIPLE_STRUCTURED_LYRICS)) {
+                const subsonicLyrics = await api.controller
+                    .getStructuredLyrics({
+                        apiClientProps: { server, signal },
+                        query: { songId: song.id },
+                    })
+                    .catch(console.error);
+
+                if (subsonicLyrics) {
+                    return subsonicLyrics;
+                }
+            } else if (hasFeature(server, ServerFeature.SINGLE_STRUCTURED_LYRIC)) {
                 const jfLyrics = await api.controller
                     .getLyrics({
                         apiClientProps: { server, signal },
@@ -113,17 +124,6 @@ export const useSongLyricsBySong = (
                         source: server?.name ?? 'music server',
                     };
                 }
-            } else if (hasFeature(server, ServerFeature.SONG_LYRICS)) {
-                const subsonicLyrics = await api.controller
-                    .getStructuredLyrics({
-                        apiClientProps: { server, signal },
-                        query: { songId: song.id },
-                    })
-                    .catch(console.error);
-
-                if (subsonicLyrics) {
-                    return subsonicLyrics;
-                }
             } else if (song.lyrics) {
                 return {
                     artist: song.artists?.[0]?.name,