From 0602e1f077286ad121c2abc418788165eb3a4aa8 Mon Sep 17 00:00:00 2001 From: simojenki Date: Sat, 15 Feb 2025 06:48:23 +0000 Subject: [PATCH] Remove tracks function, replace with just getting album --- src/music_library.ts | 3 +- src/smapi.ts | 3 +- src/subsonic_music_library.ts | 21 +- tests/in_memory_music_service.test.ts | 22 +- tests/in_memory_music_service.ts | 8 +- tests/smapi.test.ts | 19 +- tests/subsonic.test.ts | 259 ++++++++++++++++---- tests/subsonic_music_library.test.ts | 326 -------------------------- 8 files changed, 242 insertions(+), 419 deletions(-) diff --git a/src/music_library.ts b/src/music_library.ts index 457eac2..554ffae 100644 --- a/src/music_library.ts +++ b/src/music_library.ts @@ -175,8 +175,7 @@ export interface MusicLibrary { artists(q: ArtistQuery): Promise>; artist(id: string): Promise; albums(q: AlbumQuery): Promise>; - album(id: string): Promise; - tracks(albumId: string): Promise; + album(id: string): Promise; track(trackId: string): Promise; genres(): Promise; years(): Promise; diff --git a/src/smapi.ts b/src/smapi.ts index 83e509c..545f542 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -981,7 +981,8 @@ function bindSmapiSoapServiceToExpress( }); case "album": return musicLibrary - .tracks(typeId!) + .album(typeId!) + .then(it => it.tracks) .then(slice2(paging)) .then(([page, total]) => { return getMetadataResult({ diff --git a/src/subsonic_music_library.ts b/src/subsonic_music_library.ts index f0efa61..80aef49 100644 --- a/src/subsonic_music_library.ts +++ b/src/subsonic_music_library.ts @@ -9,6 +9,7 @@ import { AlbumQuery, ArtistQuery, MusicLibrary, + Album, AlbumSummary, Rating, Artist, @@ -19,9 +20,7 @@ import { import { Subsonic, CustomPlayers, - GetAlbumResponse, asTrack, - asAlbumSummary, PingResponse, NO_CUSTOM_PLAYERS, asToken, @@ -171,25 +170,11 @@ export class SubsonicMusicLibrary implements MusicLibrary { albums = async (q: AlbumQuery): Promise> => this.subsonic.getAlbumList2(this.credentials, q); - // todo: this should probably return an Album - album = (id: string): Promise => - this.subsonic.getAlbum(this.credentials, id).then(albumToAlbumSummary); + album = (id: string): Promise => + this.subsonic.getAlbum(this.credentials, id); genres = () => this.subsonic.getGenres(this.credentials); - // todo: do we even need this if Album has tracks? - tracks = (albumId: string) => - this.subsonic - .getJSON(this.credentials, "/rest/getAlbum", { - id: albumId, - }) - .then((it) => it.album) - .then((album) => - (album.song || []).map((song) => - asTrack(asAlbumSummary(album), song, this.customPlayers) - ) - ); - track = (trackId: string) => this.subsonic.getTrack(this.credentials, trackId); diff --git a/tests/in_memory_music_service.test.ts b/tests/in_memory_music_service.test.ts index d167c84..7a53f5a 100644 --- a/tests/in_memory_music_service.test.ts +++ b/tests/in_memory_music_service.test.ts @@ -167,23 +167,6 @@ describe("InMemoryMusicService", () => { service.hasTracks(track1, track2, track3, track4); }); - describe("fetching tracks for an album", () => { - it("should return only tracks on that album", async () => { - expect(await musicLibrary.tracks(artist1Album1.id)).toEqual([ - { ...track1, rating: { love: false, stars: 0 } }, - { ...track2, rating: { love: false, stars: 0 } }, - ]); - }); - }); - - describe("fetching tracks for an album that doesnt exist", () => { - it("should return empty array", async () => { - expect(await musicLibrary.tracks("non existant album id")).toEqual( - [] - ); - }); - }); - describe("fetching a single track", () => { describe("when it exists", () => { it("should return the track", async () => { @@ -424,7 +407,10 @@ describe("InMemoryMusicService", () => { describe("when it exists", () => { it("should provide an album", async () => { expect(await musicLibrary.album(artist1_album5.id)).toEqual( - artist1_album5 + { + ...artist1_album5, + tracks: [] + } ); }); }); diff --git a/tests/in_memory_music_service.ts b/tests/in_memory_music_service.ts index 73ae167..d1fede7 100644 --- a/tests/in_memory_music_service.ts +++ b/tests/in_memory_music_service.ts @@ -102,7 +102,7 @@ export class InMemoryMusicService implements MusicService { pipe( this.artists.flatMap((it) => it.albums).find((it) => it.id === id), O.fromNullable, - O.map((it) => Promise.resolve(it)), + O.map((it) => Promise.resolve({ ...it, tracks: [] })), O.getOrElse(() => Promise.reject(`No album with id '${id}'`)) ), genres: () => @@ -117,12 +117,6 @@ export class InMemoryMusicService implements MusicService { A.sort(fromCompare((x, y) => ordString.compare(x.id, y.id))) ) ), - tracks: (albumId: string) => - Promise.resolve( - this.tracks - .filter((it) => it.album.id === albumId) - .map((it) => ({ ...it, rating: { love: false, stars: 0 } })) - ), rate: (_: string, _2: Rating) => Promise.resolve(false), track: (trackId: string) => pipe( diff --git a/tests/smapi.test.ts b/tests/smapi.test.ts index 7c552c3..95e5140 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -41,6 +41,8 @@ import { PUNK, aPlaylist, aRadioStation, + anArtistSummary, + anAlbumSummary, } from "./builders"; import { InMemoryMusicService } from "./in_memory_music_service"; import supersoap from "./supersoap"; @@ -2356,10 +2358,8 @@ describe("wsdl api", () => { }); describe("asking for an album", () => { - const album = anAlbum(); - const artist = anArtist({ - albums: [album], - }); + const album = anAlbumSummary(); + const artist = anArtistSummary(); const track1 = aTrack({ artist, album, number: 1 }); const track2 = aTrack({ artist, album, number: 2 }); @@ -2370,7 +2370,12 @@ describe("wsdl api", () => { const tracks = [track1, track2, track3, track4, track5]; beforeEach(() => { - musicLibrary.tracks.mockResolvedValue(tracks); + musicLibrary.album.mockResolvedValue(anAlbum({ + ...album, + artistName: artist.name, + artistId: artist.id, + tracks + })); }); describe("asking for all for an album", () => { @@ -2394,7 +2399,7 @@ describe("wsdl api", () => { total: tracks.length, }) ); - expect(musicLibrary.tracks).toHaveBeenCalledWith(album.id); + expect(musicLibrary.album).toHaveBeenCalledWith(album.id); }); }); @@ -2421,7 +2426,7 @@ describe("wsdl api", () => { total: tracks.length, }) ); - expect(musicLibrary.tracks).toHaveBeenCalledWith(album.id); + expect(musicLibrary.album).toHaveBeenCalledWith(album.id); }); }); }); diff --git a/tests/subsonic.test.ts b/tests/subsonic.test.ts index da84a65..939d89b 100644 --- a/tests/subsonic.test.ts +++ b/tests/subsonic.test.ts @@ -29,7 +29,8 @@ import { TranscodingCustomPlayers, CustomPlayers, NO_CUSTOM_PLAYERS, - Subsonic + Subsonic, + asGenre } from "../src/subsonic"; import { b64Encode } from "../src/b64"; @@ -779,54 +780,232 @@ describe("subsonic", () => { }); describe("getting an album", () => { - beforeEach(() => { - customPlayers.encodingFor.mockReturnValue(O.none); + describe("when there are no custom players", () => { + beforeEach(() => { + customPlayers.encodingFor.mockReturnValue(O.none); + }); + + describe("when the album has some tracks", () => { + const artistId = "artist6677" + const artistName = "Fizzy Wizzy" + + const albumSummary = anAlbumSummary({ artistId, artistName }) + const artistSumamry = anArtistSummary({ id: artistId, name: artistName }) + + // todo: fix these ratings + const tracks = [ + aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), + aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), + aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), + aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), + ]; + + const album = anAlbum({ + ...albumSummary, + tracks, + artistId, + artistName, + }); + + beforeEach(() => { + mockGET.mockImplementationOnce(() => + Promise.resolve(ok(getAlbumJson(album))) + ); + }); + + it("should return the album", async () => { + const result = await subsonic.getAlbum(credentials, album.id); + + expect(result).toEqual(album); + + expect(axios.get).toHaveBeenCalledWith( + url.append({ pathname: "/rest/getAlbum" }).href(), + { + params: asURLSearchParams({ + ...authParamsPlusJson, + id: album.id, + }), + headers, + } + ); + }); + }); + + describe("when the album has no tracks", () => { + const artistId = "artist6677" + const artistName = "Fizzy Wizzy" + + const albumSummary = anAlbumSummary({ artistId, artistName }) + + const album = anAlbum({ + ...albumSummary, + tracks: [], + artistId, + artistName, + }); + + beforeEach(() => { + mockGET.mockImplementationOnce(() => + Promise.resolve(ok(getAlbumJson(album))) + ); + }); + + it("should return the album", async () => { + const result = await subsonic.getAlbum(credentials, album.id); + + expect(result).toEqual(album); + + expect(axios.get).toHaveBeenCalledWith( + url.append({ pathname: "/rest/getAlbum" }).href(), + { + params: asURLSearchParams({ + ...authParamsPlusJson, + id: album.id, + }), + headers, + } + ); + }); + }); + }); - describe("when it exists", () => { - const artistId = "artist6677" - const artistName = "Fizzy Wizzy" + describe("when a custom player is configured for the mime type", () => { + const hipHop = asGenre("Hip-Hop"); + const tripHop = asGenre("Trip-Hop"); - const albumSummary = anAlbumSummary({ artistId, artistName }) - const artistSumamry = anArtistSummary({ id: artistId, name: artistName }) + const albumSummary = anAlbumSummary({ id: "album1", name: "Burnin", genre: hipHop }); - // todo: fix these ratings - const tracks = [ - aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), - aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), - aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), - aTrack({ artist: artistSumamry, album: albumSummary, rating: { love: false, stars: 0 } }), - ]; + const artistSummary = anArtistSummary({ + id: "artist1", + name: "Bob Marley" + }); - const album = anAlbum({ - ...albumSummary, - tracks, - artistId, - artistName, - }); + const alac = aTrack({ + artist: artistSummary, + album: albumSummary, + encoding: { + player: "bonob", + mimeType: "audio/alac", + }, + genre: hipHop, + rating: { + love: true, + stars: 3, + }, + }); + const m4a = aTrack({ + artist: artistSummary, + album: albumSummary, + encoding: { + player: "bonob", + mimeType: "audio/m4a", + }, + genre: hipHop, + rating: { + love: false, + stars: 0, + }, + }); + const mp3 = aTrack({ + artist: artistSummary, + album: albumSummary, + encoding: { + player: "bonob", + mimeType: "audio/mp3", + }, + genre: tripHop, + rating: { + love: true, + stars: 5, + }, + }); - beforeEach(() => { - mockGET.mockImplementationOnce(() => - Promise.resolve(ok(getAlbumJson(album))) - ); - }); + const album = anAlbum({ + ...albumSummary, + tracks: [alac, m4a, mp3] + }) + + beforeEach(() => { + customPlayers.encodingFor + .mockReturnValueOnce( + O.of({ player: "bonob+audio/alac", mimeType: "audio/flac" }) + ) + .mockReturnValueOnce( + O.of({ player: "bonob+audio/m4a", mimeType: "audio/opus" }) + ) + .mockReturnValueOnce(O.none); - it("should return the album", async () => { - const result = await subsonic.getAlbum(credentials, album.id); + mockGET.mockImplementationOnce(() => + Promise.resolve(ok(getAlbumJson(album))) + ); + }); - expect(result).toEqual(album); + it("should return the album with custom players applied", async () => { + const result = await subsonic.getAlbum(credentials, album.id); - expect(axios.get).toHaveBeenCalledWith( - url.append({ pathname: "/rest/getAlbum" }).href(), - { - params: asURLSearchParams({ - ...authParamsPlusJson, - id: album.id, - }), - headers, - } - ); - }); + expect(result).toEqual({ + ...album, + tracks: [ + { + ...alac, + encoding: { + player: "bonob+audio/alac", + mimeType: "audio/flac", + }, + // todo: this doesnt seem right? why dont the ratings come back? + rating: { + love: false, + stars: 0 + } + }, + { + ...m4a, + encoding: { + player: "bonob+audio/m4a", + mimeType: "audio/opus", + }, + rating: { + love: false, + stars: 0 + } + }, + { + ...mp3, + encoding: { + player: "bonob", + mimeType: "audio/mp3", + }, + rating: { + love: false, + stars: 0 + } + }, + ] + }); + + expect(axios.get).toHaveBeenCalledWith( + url.append({ pathname: "/rest/getAlbum" }).href(), + { + params: asURLSearchParams({ + ...authParamsPlusJson, + id: album.id, + }), + headers, + } + ); + + expect(customPlayers.encodingFor).toHaveBeenCalledTimes(3); + expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(1, { + mimeType: "audio/alac", + }); + expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(2, { + mimeType: "audio/m4a", + }); + expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(3, { + mimeType: "audio/mp3", + }); + }); }); }); }); diff --git a/tests/subsonic_music_library.test.ts b/tests/subsonic_music_library.test.ts index e84c891..66faeb1 100644 --- a/tests/subsonic_music_library.test.ts +++ b/tests/subsonic_music_library.test.ts @@ -55,7 +55,6 @@ import { ROCK, aRadioStation, anAlbumSummary, - anArtistSummary } from "./builders"; import { b64Encode } from "../src/b64"; import { BUrn } from "../src/burn"; @@ -2592,332 +2591,7 @@ describe("SubsonicMusicLibrary", () => { }); }); - describe("getting tracks", () => { - describe("for an album", () => { - describe("when there are no custom players", () => { - beforeEach(() => { - customPlayers.encodingFor.mockReturnValue(O.none); - }); - - describe("when the album has multiple tracks, some of which are rated", () => { - const hipHop = asGenre("Hip-Hop"); - const tripHop = asGenre("Trip-Hop"); - - const albumSummary = anAlbumSummary({ - id: "album1", - name: "Burnin", - genre: hipHop, - }); - - const artistSummary = anArtistSummary({ - id: "artist1", - name: "Bob Marley" - }); - - const track1 = aTrack({ - artist: artistSummary, - album: albumSummary, - genre: hipHop, - rating: { - love: true, - stars: 3, - }, - }); - const track2 = aTrack({ - artist: artistSummary, - album: albumSummary, - genre: hipHop, - rating: { - love: false, - stars: 0, - }, - }); - const track3 = aTrack({ - artist: artistSummary, - album: albumSummary, - genre: tripHop, - rating: { - love: true, - stars: 5, - }, - }); - const track4 = aTrack({ - artist: artistSummary, - album: albumSummary, - genre: tripHop, - rating: { - love: false, - stars: 1, - }, - }); - - const album = anAlbum({ - ...albumSummary, - tracks: [track1, track2, track3, track4] - }) - - beforeEach(() => { - mockGET.mockImplementationOnce(() => - Promise.resolve(ok(getAlbumJson(album))) - ); - }); - - it("should return the album", async () => { - const result = await subsonic.tracks(album.id); - - // todo: not this - const blatRating = (t: Track) => ({ - ...t, - rating: { - love: false, - stars: 0 - } - }) - - expect(result).toEqual([ - blatRating(track1), - blatRating(track2), - blatRating(track3), - blatRating(track4), - ]); - - expect(axios.get).toHaveBeenCalledWith( - url.append({ pathname: "/rest/getAlbum" }).href(), - { - params: asURLSearchParams({ - ...authParamsPlusJson, - id: album.id, - }), - headers, - } - ); - }); - }); - - describe("when the album has only 1 track", () => { - // todo: why do i care about the genre in here? - const flipFlop = asGenre("Flip-Flop"); - - const albumSummary = anAlbumSummary({ - id: "album1", - name: "Burnin", - genre: flipFlop, - }); - - const artistSummary = anArtistSummary({ - id: "artist1", - name: "Bob Marley" - }); - - const track = aTrack({ - artist: artistSummary, - album: albumSummary, - genre: flipFlop, - }); - - const album = anAlbum({ - ...albumSummary, - tracks: [track] - }); - - beforeEach(() => { - mockGET.mockImplementationOnce(() => - Promise.resolve(ok(getAlbumJson(album))) - ); - }); - - it("should return the album", async () => { - const result = await subsonic.tracks(album.id); - - expect(result).toEqual([{ - ...track, - // todo: not sure about this - rating: { - love: false, - stars: 0 - } - }]); - - expect(axios.get).toHaveBeenCalledWith( - url.append({ pathname: "/rest/getAlbum" }).href(), - { - params: asURLSearchParams({ - ...authParamsPlusJson, - id: album.id, - }), - headers, - } - ); - }); - }); - - describe("when the album has no tracks", () => { - const album = anAlbum({ - tracks: [] - }) - - beforeEach(() => { - mockGET.mockImplementationOnce(() => - Promise.resolve(ok(getAlbumJson(album))) - ); - }); - - it("should empty array", async () => { - const result = await subsonic.tracks(album.id); - - expect(result).toEqual([]); - - expect(axios.get).toHaveBeenCalledWith( - url.append({ pathname: "/rest/getAlbum" }).href(), - { - params: asURLSearchParams({ - ...authParamsPlusJson, - id: album.id, - }), - headers, - } - ); - }); - }); - }); - - describe("when a custom player is configured for the mime type", () => { - const hipHop = asGenre("Hip-Hop"); - const tripHop = asGenre("Trip-Hop"); - - const albumSummary = anAlbumSummary({ id: "album1", name: "Burnin", genre: hipHop }); - - const artistSummary = anArtistSummary({ - id: "artist1", - name: "Bob Marley" - }); - - const alac = aTrack({ - artist: artistSummary, - album: albumSummary, - encoding: { - player: "bonob", - mimeType: "audio/alac", - }, - genre: hipHop, - rating: { - love: true, - stars: 3, - }, - }); - const m4a = aTrack({ - artist: artistSummary, - album: albumSummary, - encoding: { - player: "bonob", - mimeType: "audio/m4a", - }, - genre: hipHop, - rating: { - love: false, - stars: 0, - }, - }); - const mp3 = aTrack({ - artist: artistSummary, - album: albumSummary, - encoding: { - player: "bonob", - mimeType: "audio/mp3", - }, - genre: tripHop, - rating: { - love: true, - stars: 5, - }, - }); - - const album = anAlbum({ - ...albumSummary, - tracks: [alac, m4a, mp3] - }) - - beforeEach(() => { - customPlayers.encodingFor - .mockReturnValueOnce( - O.of({ player: "bonob+audio/alac", mimeType: "audio/flac" }) - ) - .mockReturnValueOnce( - O.of({ player: "bonob+audio/m4a", mimeType: "audio/opus" }) - ) - .mockReturnValueOnce(O.none); - - mockGET.mockImplementationOnce(() => - Promise.resolve(ok(getAlbumJson(album))) - ); - }); - - it("should return the album with custom players applied", async () => { - const result = await subsonic.tracks(album.id); - - expect(result).toEqual([ - { - ...alac, - encoding: { - player: "bonob+audio/alac", - mimeType: "audio/flac", - }, - // todo: this doesnt seem right? why dont the ratings come back? - rating: { - love: false, - stars: 0 - } - }, - { - ...m4a, - encoding: { - player: "bonob+audio/m4a", - mimeType: "audio/opus", - }, - rating: { - love: false, - stars: 0 - } - }, - { - ...mp3, - encoding: { - player: "bonob", - mimeType: "audio/mp3", - }, - rating: { - love: false, - stars: 0 - } - }, - ]); - - expect(axios.get).toHaveBeenCalledWith( - url.append({ pathname: "/rest/getAlbum" }).href(), - { - params: asURLSearchParams({ - ...authParamsPlusJson, - id: album.id, - }), - headers, - } - ); - - expect(customPlayers.encodingFor).toHaveBeenCalledTimes(3); - expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(1, { - mimeType: "audio/alac", - }); - expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(2, { - mimeType: "audio/m4a", - }); - expect(customPlayers.encodingFor).toHaveBeenNthCalledWith(3, { - mimeType: "audio/mp3", - }); - }); - }); - }); - describe("a single track", () => { const pop = asGenre("Pop");