From 89340dd45418deec82b30b4fba08c28aee3baca5 Mon Sep 17 00:00:00 2001 From: Simon J Date: Sat, 20 Nov 2021 18:22:24 +1100 Subject: [PATCH] Fix bug where sonos app cannot navigate from track to artist when subsonic returns null artistId on song (#79) --- src/music_service.ts | 2 +- src/smapi.ts | 4 +- src/subsonic.ts | 12 +++--- tests/in_memory_music_service.test.ts | 4 +- tests/smapi.test.ts | 54 +++++++++++++++++++++++++++ tests/subsonic.test.ts | 48 ++++++++++++++++++------ 6 files changed, 101 insertions(+), 23 deletions(-) diff --git a/src/music_service.ts b/src/music_service.ts index 78bfb5a..e291aa1 100644 --- a/src/music_service.ts +++ b/src/music_service.ts @@ -25,7 +25,7 @@ export type AuthFailure = { }; export type ArtistSummary = { - id: string; + id: string | undefined; name: string; image: BUrn | undefined; }; diff --git a/src/smapi.ts b/src/smapi.ts index b88a72d..042c7ac 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -312,10 +312,10 @@ export const track = (bonobUrl: URLBuilder, track: Track) => ({ album: track.album.name, albumId: `album:${track.album.id}`, albumArtist: track.artist.name, - albumArtistId: `artist:${track.artist.id}`, + albumArtistId: track.artist.id? `artist:${track.artist.id}` : undefined, albumArtURI: defaultAlbumArtURI(bonobUrl, track).href(), artist: track.artist.name, - artistId: `artist:${track.artist.id}`, + artistId: track.artist.id ? `artist:${track.artist.id}` : undefined, duration: track.duration, genre: track.album.genre?.name, genreId: track.album.genre?.id, diff --git a/src/subsonic.ts b/src/subsonic.ts index 8633355..2ab889b 100644 --- a/src/subsonic.ts +++ b/src/subsonic.ts @@ -144,12 +144,14 @@ type GetArtistResponse = SubsonicResponse & { }; }; -type song = { +export type song = { id: string; parent: string | undefined; title: string; album: string | undefined; + albumId: string | undefined; artist: string | undefined; + artistId: string | undefined; track: number | undefined; year: string | undefined; genre: string | undefined; @@ -159,8 +161,6 @@ type song = { bitRate: number | undefined; suffix: string | undefined; contentType: string | undefined; - albumId: string | undefined; - artistId: string | undefined; type: string | undefined; userRating: number | undefined; starred: string | undefined; @@ -270,9 +270,9 @@ export const asTrack = (album: Album, song: song): Track => ({ coverArt: coverArtURN(song.coverArt), album, artist: { - id: `${song.artistId!}`, - name: song.artist!, - image: artistImageURN({ artistId: song.artistId }), + id: song.artistId, + name: song.artist ? song.artist : "?", + image: song.artistId ? artistImageURN({ artistId: song.artistId }) : undefined, }, rating: { love: song.starred != undefined, diff --git a/tests/in_memory_music_service.test.ts b/tests/in_memory_music_service.test.ts index 5fc342e..d358eb2 100644 --- a/tests/in_memory_music_service.test.ts +++ b/tests/in_memory_music_service.test.ts @@ -126,8 +126,8 @@ describe("InMemoryMusicService", () => { describe("when it exists", () => { it("should provide an artist", async () => { - expect(await musicLibrary.artist(artist1.id)).toEqual(artist1); - expect(await musicLibrary.artist(artist2.id)).toEqual(artist2); + expect(await musicLibrary.artist(artist1.id!)).toEqual(artist1); + expect(await musicLibrary.artist(artist2.id!)).toEqual(artist2); }); }); diff --git a/tests/smapi.test.ts b/tests/smapi.test.ts index 47a94c5..0290f5c 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -398,6 +398,60 @@ describe("track", () => { }, }); }); + + describe("when there is no artistId from subsonic", () => { + it("should not send an artist id to sonos", () => { + const bonobUrl = url("http://localhost:4567/foo?access-token=1234"); + const someTrack = aTrack({ + id: uuid(), + // audio/x-flac should be mapped to audio/flac + mimeType: "audio/x-flac", + name: "great song", + duration: randomInt(1000), + number: randomInt(100), + album: anAlbum({ + name: "great album", + id: uuid(), + genre: { id: "genre101", name: "some genre" }, + }), + artist: anArtist({ name: "great artist", id: undefined }), + coverArt: { system: "subsonic", resource: "887766" }, + rating: { + love: true, + stars: 5 + } + }); + + expect(track(bonobUrl, someTrack)).toEqual({ + itemType: "track", + id: `track:${someTrack.id}`, + mimeType: "audio/flac", + title: someTrack.name, + + trackMetadata: { + album: someTrack.album.name, + albumId: `album:${someTrack.album.id}`, + albumArtist: someTrack.artist.name, + albumArtistId: undefined, + albumArtURI: `http://localhost:4567/foo/art/${encodeURIComponent(formatForURL(someTrack.coverArt!))}/size/180?access-token=1234`, + artist: someTrack.artist.name, + artistId: undefined, + duration: someTrack.duration, + genre: someTrack.album.genre?.name, + genreId: someTrack.album.genre?.id, + trackNumber: someTrack.number, + }, + dynamic: { + property: [ + { + name: "rating", + value: `${ratingAsInt(someTrack.rating)}`, + }, + ], + }, + }); + }); + }); }); describe("album", () => { diff --git a/tests/subsonic.test.ts b/tests/subsonic.test.ts index fde58de..2bf0c0c 100644 --- a/tests/subsonic.test.ts +++ b/tests/subsonic.test.ts @@ -18,6 +18,7 @@ import { asTrack, artistImageURN, images, + song, } from "../src/subsonic"; import axios from "axios"; @@ -627,10 +628,33 @@ describe("artistURN", () => { }); describe("asTrack", () => { - const album = anAlbum(); - const track = aTrack(); + describe("when the song has no artistId", () => { + const album = anAlbum(); + const track = aTrack({ artist: { id: undefined, name: "Not in library so no id", image: undefined }}); + + it("should provide no artistId", () => { + const result = asTrack(album, { ...asSongJson(track) }); + expect(result.artist.id).toBeUndefined(); + expect(result.artist.name).toEqual("Not in library so no id"); + expect(result.artist.image).toBeUndefined(); + }); + }); + + describe("when the song has no artist name", () => { + const album = anAlbum(); + + it("should provide a ? to sonos", () => { + const result = asTrack(album, { id: '1' } as any as song); + expect(result.artist.id).toBeUndefined(); + expect(result.artist.name).toEqual("?"); + expect(result.artist.image).toBeUndefined(); + }); + }); describe("invalid rating.stars values", () => { + const album = anAlbum(); + const track = aTrack(); + describe("a value greater than 5", () => { it("should be returned as 0", () => { const result = asTrack(album, { ...asSongJson(track), userRating: 6 }); @@ -861,7 +885,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: `${artist.id}`, @@ -923,7 +947,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -979,7 +1003,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1033,7 +1057,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1090,7 +1114,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1144,7 +1168,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1199,7 +1223,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1255,7 +1279,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1309,7 +1333,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id, @@ -1361,7 +1385,7 @@ describe("Subsonic", () => { .generateToken({ username, password }) .then((it) => it as AuthSuccess) .then((it) => navidrome.login(it.authToken)) - .then((it) => it.artist(artist.id)); + .then((it) => it.artist(artist.id!)); expect(result).toEqual({ id: artist.id,