From 054c51a7341a79d7504fea077aff40b9269a7bb9 Mon Sep 17 00:00:00 2001 From: simojenki Date: Fri, 2 Feb 2024 12:16:37 +0000 Subject: [PATCH 1/3] Playlist icons working as rendered by ND --- src/music_service.ts | 6 ++++-- src/smapi.ts | 8 +++++++- src/subsonic.ts | 17 +++++++++++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/music_service.ts b/src/music_service.ts index d7a5065..2e51ddf 100644 --- a/src/music_service.ts +++ b/src/music_service.ts @@ -113,7 +113,8 @@ export const albumToAlbumSummary = (it: Album): AlbumSummary => ({ export const playlistToPlaylistSummary = (it: Playlist): PlaylistSummary => ({ id: it.id, - name: it.name + name: it.name, + coverArt: it.coverArt }) export type StreamingHeader = "content-type" | "content-length" | "content-range" | "accept-ranges"; @@ -131,7 +132,8 @@ export type CoverArt = { export type PlaylistSummary = { id: string, - name: string + name: string, + coverArt: BUrn | undefined } export type Playlist = PlaylistSummary & { diff --git a/src/smapi.ts b/src/smapi.ts index 6f878c3..dfbda1d 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -253,7 +253,7 @@ const playlist = (bonobUrl: URLBuilder, playlist: Playlist) => ({ itemType: "playlist", id: `playlist:${playlist.id}`, title: playlist.name, - albumArtURI: playlistAlbumArtURL(bonobUrl, playlist).href(), + albumArtURI: defaultAlbumArtURI(bonobUrl, playlist).href(), canPlay: true, attributes: { readOnly: false, @@ -262,6 +262,7 @@ const playlist = (bonobUrl: URLBuilder, playlist: Playlist) => ({ }, }); +// todo: delete me export const playlistAlbumArtURL = ( bonobUrl: URLBuilder, playlist: Playlist @@ -285,6 +286,7 @@ export const playlistAlbumArtURL = ( } }; +// todo: rename me export const defaultAlbumArtURI = ( bonobUrl: URLBuilder, { coverArt }: { coverArt: BUrn | undefined } @@ -305,6 +307,7 @@ export const iconArtURI = (bonobUrl: URLBuilder, icon: ICON) => pathname: `/icon/${icon}/size/legacy`, }); +// todo: how am I different to defaultAlbumArtURI export const defaultArtistArtURI = ( bonobUrl: URLBuilder, artist: ArtistSummary @@ -872,9 +875,12 @@ function bindSmapiSoapServiceToExpress( .then((it) => Promise.all( it.map((playlist) => { + // todo: whats this odd copy all about, can we just delete it? return { id: playlist.id, name: playlist.name, + coverArt: playlist.coverArt, + // todo: are these every important? entries: [] }; } diff --git a/src/subsonic.ts b/src/subsonic.ts index e55e726..611d9d4 100644 --- a/src/subsonic.ts +++ b/src/subsonic.ts @@ -20,6 +20,7 @@ import { AlbumQueryType, Artist, AuthFailure, + PlaylistSummary } from "./music_service"; import sharp from "sharp"; import _ from "underscore"; @@ -178,12 +179,15 @@ type GetAlbumResponse = { type playlist = { id: string; name: string; + coverArt: string | undefined; }; type GetPlaylistResponse = { + // todo: isnt the type here a composite? playlistSummary && { entry: song[]; } playlist: { id: string; name: string; + coverArt: string | undefined; entry: song[]; }; }; @@ -306,6 +310,13 @@ const asAlbum = (album: album): Album => ({ coverArt: coverArtURN(album.coverArt), }); +// coverArtURN +const asPlayListSummary = (playlist: playlist): PlaylistSummary => ({ + id: playlist.id, + name: playlist.name, + coverArt: coverArtURN(playlist.coverArt) +}) + export const asGenre = (genreName: string) => ({ id: b64Encode(genreName), name: genreName, @@ -862,7 +873,7 @@ export class Subsonic implements MusicService { .getJSON(credentials, "/rest/getPlaylists") .then((it) => it.playlists.playlist || []) .then((playlists) => - playlists.map((it) => ({ id: it.id, name: it.name })) + playlists.map(asPlayListSummary) ), playlist: async (id: string) => subsonic @@ -875,6 +886,7 @@ export class Subsonic implements MusicService { return { id: playlist.id, name: playlist.name, + coverArt: coverArtURN(playlist.coverArt), entries: (playlist.entry || []).map((entry) => ({ ...asTrack( { @@ -898,7 +910,8 @@ export class Subsonic implements MusicService { name, }) .then((it) => it.playlist) - .then((it) => ({ id: it.id, name: it.name })), + // todo: why is this line so similar to other playlist lines?? + .then((it) => ({ id: it.id, name: it.name, coverArt: coverArtURN(it.coverArt) })), deletePlaylist: async (id: string) => subsonic .getJSON(credentials, "/rest/deletePlaylist", { From 9d3016e6169f0d9faecb7422959170ddc5fd6fce Mon Sep 17 00:00:00 2001 From: Simon J Date: Sat, 3 Feb 2024 02:46:47 +0000 Subject: [PATCH 2/3] remove duplication in cover art image url creation --- src/music_service.ts | 2 +- src/smapi.ts | 55 +----- tests/smapi.test.ts | 401 ++++--------------------------------------- 3 files changed, 39 insertions(+), 419 deletions(-) diff --git a/src/music_service.ts b/src/music_service.ts index 2e51ddf..02c5021 100644 --- a/src/music_service.ts +++ b/src/music_service.ts @@ -133,7 +133,7 @@ export type CoverArt = { export type PlaylistSummary = { id: string, name: string, - coverArt: BUrn | undefined + coverArt?: BUrn | undefined } export type Playlist = PlaylistSummary & { diff --git a/src/smapi.ts b/src/smapi.ts index dfbda1d..70ccaac 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -26,7 +26,7 @@ import { Clock } from "./clock"; import { URLBuilder } from "./url_builder"; import { asLANGs, I8N } from "./i8n"; import { ICON, iconForGenre } from "./icon"; -import _, { uniq } from "underscore"; +import _ from "underscore"; import { BUrn, formatForURL } from "./burn"; import { isExpiredTokenError, @@ -253,7 +253,7 @@ const playlist = (bonobUrl: URLBuilder, playlist: Playlist) => ({ itemType: "playlist", id: `playlist:${playlist.id}`, title: playlist.name, - albumArtURI: defaultAlbumArtURI(bonobUrl, playlist).href(), + albumArtURI: coverArtURI(bonobUrl, playlist).href(), canPlay: true, attributes: { readOnly: false, @@ -262,34 +262,9 @@ const playlist = (bonobUrl: URLBuilder, playlist: Playlist) => ({ }, }); -// todo: delete me -export const playlistAlbumArtURL = ( +export const coverArtURI = ( bonobUrl: URLBuilder, - playlist: Playlist -) => { - // todo: this should be put into config, or even just removed for the ND music source - if(process.env["BNB_DISABLE_PLAYLIST_ART"]) return iconArtURI(bonobUrl, "music"); - - const burns: BUrn[] = uniq( - playlist.entries.filter((it) => it.coverArt != undefined), - (it) => it.album.id - ).map((it) => it.coverArt!); - if (burns.length == 0) { - return iconArtURI(bonobUrl, "error"); - } else { - return bonobUrl.append({ - pathname: `/art/${burns - .slice(0, 9) - .map((it) => encodeURIComponent(formatForURL(it))) - .join("&")}/size/180`, - }); - } -}; - -// todo: rename me -export const defaultAlbumArtURI = ( - bonobUrl: URLBuilder, - { coverArt }: { coverArt: BUrn | undefined } + { coverArt }: { coverArt?: BUrn | undefined } ) => pipe( coverArt, @@ -307,22 +282,6 @@ export const iconArtURI = (bonobUrl: URLBuilder, icon: ICON) => pathname: `/icon/${icon}/size/legacy`, }); -// todo: how am I different to defaultAlbumArtURI -export const defaultArtistArtURI = ( - bonobUrl: URLBuilder, - artist: ArtistSummary -) => - pipe( - artist.image, - O.fromNullable, - O.map((it) => - bonobUrl.append({ - pathname: `/art/${encodeURIComponent(formatForURL(it))}/size/180`, - }) - ), - O.getOrElseW(() => iconArtURI(bonobUrl, "vinyl")) - ); - export const sonosifyMimeType = (mimeType: string) => mimeType == "audio/x-flac" ? "audio/flac" : mimeType; @@ -332,7 +291,7 @@ export const album = (bonobUrl: URLBuilder, album: AlbumSummary) => ({ artist: album.artistName, artistId: `artist:${album.artistId}`, title: album.name, - albumArtURI: defaultAlbumArtURI(bonobUrl, album).href(), + albumArtURI: coverArtURI(bonobUrl, album).href(), canPlay: true, // defaults // canScroll: false, @@ -351,7 +310,7 @@ export const track = (bonobUrl: URLBuilder, track: Track) => ({ albumId: `album:${track.album.id}`, albumArtist: track.artist.name, albumArtistId: track.artist.id ? `artist:${track.artist.id}` : undefined, - albumArtURI: defaultAlbumArtURI(bonobUrl, track).href(), + albumArtURI: coverArtURI(bonobUrl, track).href(), artist: track.artist.name, artistId: track.artist.id ? `artist:${track.artist.id}` : undefined, duration: track.duration, @@ -369,7 +328,7 @@ export const artist = (bonobUrl: URLBuilder, artist: ArtistSummary) => ({ id: `artist:${artist.id}`, artistId: artist.id, title: artist.name, - albumArtURI: defaultArtistArtURI(bonobUrl, artist).href(), + albumArtURI: coverArtURI(bonobUrl, { coverArt: artist.image }).href(), }); function splitId(id: string) { diff --git a/tests/smapi.test.ts b/tests/smapi.test.ts index f992d35..2496a35 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -18,11 +18,9 @@ import { track, artist, album, - defaultAlbumArtURI, - defaultArtistArtURI, + coverArtURI, searchResult, iconArtURI, - playlistAlbumArtURL, sonosifyMimeType, ratingAsInt, ratingFromInt, @@ -41,7 +39,6 @@ import { TRIP_HOP, PUNK, aPlaylist, - anAlbumSummary, } from "./builders"; import { InMemoryMusicService } from "./in_memory_music_service"; import supersoap from "./supersoap"; @@ -56,7 +53,6 @@ import dayjs from "dayjs"; import url, { URLBuilder } from "../src/url_builder"; import { iconForGenre } from "../src/icon"; import { formatForURL } from "../src/burn"; -import { range } from "underscore"; import { FixedClock } from "../src/clock"; import { ExpiredTokenError, InvalidTokenError, SmapiAuthTokens, SmapiToken, ToSmapiFault } from "../src/smapi_auth"; @@ -471,7 +467,7 @@ describe("album", () => { itemType: "album", id: `album:${someAlbum.id}`, title: someAlbum.name, - albumArtURI: defaultAlbumArtURI(bonobUrl, someAlbum).href(), + albumArtURI: coverArtURI(bonobUrl, someAlbum).href(), canPlay: true, artist: someAlbum.artistName, artistId: `artist:${someAlbum.artistId}`, @@ -495,299 +491,8 @@ describe("sonosifyMimeType", () => { }); }); -describe("playlistAlbumArtURL", () => { - const coverArt1 = { system: "subsonic", resource: "1" }; - const coverArt2 = { system: "subsonic", resource: "2" }; - const coverArt3 = { system: "subsonic", resource: "3" }; - const coverArt4 = { system: "subsonic", resource: "4" }; - const coverArt5 = { system: "subsonic", resource: "5" }; - - describe("when the playlist has no coverArt ids", () => { - it("should return question mark icon", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ coverArt: undefined }), - aTrack({ coverArt: undefined }), - ], - }); - - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/icon/error/size/legacy?search=yes` - ); - }); - }); - - describe("when the playlist has external ids", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const externalArt1 = { - system: "external", - resource: "http://example.com/image1.jpg", - }; - const externalArt2 = { - system: "external", - resource: "http://example.com/image2.jpg", - }; - - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: externalArt1, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: externalArt2, - album: anAlbumSummary({ id: "album2" }), - }), - ], - }); - it("should format the url with encrypted urn", () => { - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${encodeURIComponent( - formatForURL(externalArt1) - )}&${encodeURIComponent( - formatForURL(externalArt2) - )}/size/180?search=yes` - ); - }); - - describe("when BNB_NO_PLAYLIST_ART is set", () => { - const OLD_ENV = process.env; - - beforeEach(() => { - process.env = { ...OLD_ENV }; - - process.env["BNB_DISABLE_PLAYLIST_ART"] = "true"; - }); - - afterEach(() => { - process.env = OLD_ENV; - }); - - it("should return an icon", () => { - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/icon/music/size/legacy?search=yes` - ); - }); - }); - }); - - describe("when the playlist has 4 tracks from 2 different albums, including some tracks that are missing coverArt urns", () => { - it("should use the cover art once per album", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: undefined, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt1, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt2, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: undefined, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: coverArt3, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt4, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: undefined, - album: anAlbumSummary({ id: "album2" }), - }), - ], - }); - - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${encodeURIComponent( - formatForURL(coverArt1) - )}&${encodeURIComponent(formatForURL(coverArt2))}/size/180?search=yes` - ); - }); - }); - - describe("when the playlist has 4 tracks from 2 different albums", () => { - it("should use the cover art once per album", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: coverArt1, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt2, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: coverArt3, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt4, - album: anAlbumSummary({ id: "album2" }), - }), - ], - }); - - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${encodeURIComponent( - formatForURL(coverArt1) - )}&${encodeURIComponent(formatForURL(coverArt2))}/size/180?search=yes` - ); - }); - }); - - describe("when the playlist has 4 tracks from 3 different albums", () => { - it("should use the cover art once per album", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: coverArt1, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt2, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: coverArt3, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt4, - album: anAlbumSummary({ id: "album3" }), - }), - ], - }); - - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${encodeURIComponent( - formatForURL(coverArt1) - )}&${encodeURIComponent(formatForURL(coverArt2))}&${encodeURIComponent( - formatForURL(coverArt4) - )}/size/180?search=yes` - ); - }); - }); - - describe("when the playlist has 4 tracks from 4 different albums", () => { - it("should return them on the url to the image", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: coverArt1, - album: anAlbumSummary({ id: "album1" }), - }), - aTrack({ - coverArt: coverArt2, - album: anAlbumSummary({ id: "album2" }), - }), - aTrack({ - coverArt: coverArt3, - album: anAlbumSummary({ id: "album3" }), - }), - aTrack({ - coverArt: coverArt4, - album: anAlbumSummary({ id: "album4" }), - }), - aTrack({ - coverArt: coverArt5, - album: anAlbumSummary({ id: "album1" }), - }), - ], - }); - - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${encodeURIComponent( - formatForURL(coverArt1) - )}&${encodeURIComponent(formatForURL(coverArt2))}&${encodeURIComponent( - formatForURL(coverArt3) - )}&${encodeURIComponent(formatForURL(coverArt4))}/size/180?search=yes` - ); - }); - }); - - describe("when the playlist has at least 9 distinct albumIds", () => { - it("should return the first 9 of the ids on the url", () => { - const bonobUrl = url("http://localhost:1234/context-path?search=yes"); - const playlist = aPlaylist({ - entries: [ - aTrack({ - coverArt: { system: "subsonic", resource: "1" }, - album: anAlbumSummary({ id: "1" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "2" }, - album: anAlbumSummary({ id: "2" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "3" }, - album: anAlbumSummary({ id: "3" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "4" }, - album: anAlbumSummary({ id: "4" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "5" }, - album: anAlbumSummary({ id: "5" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "6" }, - album: anAlbumSummary({ id: "6" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "7" }, - album: anAlbumSummary({ id: "7" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "8" }, - album: anAlbumSummary({ id: "8" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "9" }, - album: anAlbumSummary({ id: "9" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "10" }, - album: anAlbumSummary({ id: "10" }), - }), - aTrack({ - coverArt: { system: "subsonic", resource: "11" }, - album: anAlbumSummary({ id: "11" }), - }), - ], - }); - - const burns = range(1, 10) - .map((i) => - encodeURIComponent( - formatForURL({ system: "subsonic", resource: `${i}` }) - ) - ) - .join("&"); - expect(playlistAlbumArtURL(bonobUrl, playlist).href()).toEqual( - `http://localhost:1234/context-path/art/${burns}/size/180?search=yes` - ); - }); - }); -}); - -describe("defaultAlbumArtURI", () => { +describe("coverArtURI", () => { const bonobUrl = new URLBuilder( "http://bonob.example.com:8080/context?search=yes" ); @@ -797,7 +502,7 @@ describe("defaultAlbumArtURI", () => { it("should use it", () => { const coverArt = { system: "subsonic", resource: "12345" }; expect( - defaultAlbumArtURI(bonobUrl, anAlbum({ coverArt })).href() + coverArtURI(bonobUrl, anAlbum({ coverArt })).href() ).toEqual( `http://bonob.example.com:8080/context/art/${encodeURIComponent( formatForURL(coverArt) @@ -813,7 +518,7 @@ describe("defaultAlbumArtURI", () => { resource: "http://example.com/someimage.jpg", }; expect( - defaultAlbumArtURI(bonobUrl, anAlbum({ coverArt })).href() + coverArtURI(bonobUrl, anAlbum({ coverArt })).href() ).toEqual( `http://bonob.example.com:8080/context/art/${encodeURIComponent( formatForURL(coverArt) @@ -826,7 +531,7 @@ describe("defaultAlbumArtURI", () => { describe("when there is no album coverArt", () => { it("should return a vinly icon image", () => { expect( - defaultAlbumArtURI(bonobUrl, anAlbum({ coverArt: undefined })).href() + coverArtURI(bonobUrl, anAlbum({ coverArt: undefined })).href() ).toEqual( "http://bonob.example.com:8080/context/icon/vinyl/size/legacy?search=yes" ); @@ -834,50 +539,6 @@ describe("defaultAlbumArtURI", () => { }); }); -describe("defaultArtistArtURI", () => { - describe("when the artist has no image", () => { - it("should return an icon", () => { - const bonobUrl = url("http://localhost:1234/something?s=123"); - const artist = anArtist({ image: undefined }); - - expect(defaultArtistArtURI(bonobUrl, artist).href()).toEqual( - `http://localhost:1234/something/icon/vinyl/size/legacy?s=123` - ); - }); - }); - - describe("when the resource is subsonic", () => { - it("should use the resource", () => { - const bonobUrl = url("http://localhost:1234/something?s=123"); - const image = { system: "subsonic", resource: "art:1234" }; - const artist = anArtist({ image }); - - expect(defaultArtistArtURI(bonobUrl, artist).href()).toEqual( - `http://localhost:1234/something/art/${encodeURIComponent( - formatForURL(image) - )}/size/180?s=123` - ); - }); - }); - - describe("when the resource is external", () => { - it("should encrypt the resource", () => { - const bonobUrl = url("http://localhost:1234/something?s=123"); - const image = { - system: "external", - resource: "http://example.com/something.jpg", - }; - const artist = anArtist({ image }); - - expect(defaultArtistArtURI(bonobUrl, artist).href()).toEqual( - `http://localhost:1234/something/art/${encodeURIComponent( - formatForURL(image) - )}/size/180?s=123` - ); - }); - }); -}); - describe("wsdl api", () => { const musicService = { generateToken: jest.fn(), @@ -1701,7 +1362,7 @@ describe("wsdl api", () => { itemType: "playlist", id: `playlist:${playlist.id}`, title: playlist.name, - albumArtURI: playlistAlbumArtURL( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, playlist ).href(), @@ -1733,7 +1394,7 @@ describe("wsdl api", () => { itemType: "playlist", id: `playlist:${playlist.id}`, title: playlist.name, - albumArtURI: playlistAlbumArtURL( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, playlist ).href(), @@ -1777,7 +1438,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -1814,7 +1475,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -1866,9 +1527,9 @@ describe("wsdl api", () => { id: `artist:${it.id}`, artistId: it.id, title: it.name, - albumArtURI: defaultArtistArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, - it + { coverArt: it.image } ).href(), })), index: 0, @@ -1911,9 +1572,9 @@ describe("wsdl api", () => { id: `artist:${it.id}`, artistId: it.id, title: it.name, - albumArtURI: defaultArtistArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, - it + { coverArt: it.image } ).href(), })), index: 1, @@ -1972,9 +1633,9 @@ describe("wsdl api", () => { id: `artist:${it.id}`, artistId: it.id, title: it.name, - albumArtURI: defaultArtistArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, - it + { coverArt: it.image } ).href(), })), index: 0, @@ -2001,9 +1662,9 @@ describe("wsdl api", () => { id: `artist:${it.id}`, artistId: it.id, title: it.name, - albumArtURI: defaultArtistArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, - it + { coverArt: it.image } ).href(), }) ), @@ -2118,7 +1779,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2166,7 +1827,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2214,7 +1875,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2262,7 +1923,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2310,7 +1971,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2358,7 +2019,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2404,7 +2065,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2450,7 +2111,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2494,7 +2155,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2541,7 +2202,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${it.id}`, title: it.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, it ).href(), @@ -2929,7 +2590,7 @@ describe("wsdl api", () => { genre: track.genre?.name, genreId: track.genre?.id, duration: track.duration, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, track ).href(), @@ -2977,7 +2638,7 @@ describe("wsdl api", () => { genre: track.genre?.name, genreId: track.genre?.id, duration: track.duration, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, track ).href(), @@ -3020,7 +2681,7 @@ describe("wsdl api", () => { itemType: "album", id: `album:${album.id}`, title: album.name, - albumArtURI: defaultAlbumArtURI( + albumArtURI: coverArtURI( bonobUrlWithAccessToken, album ).href(), From b88fb9f3d9a5d17fd9306a140eb1211ea73a4556 Mon Sep 17 00:00:00 2001 From: Simon J Date: Sat, 3 Feb 2024 02:56:02 +0000 Subject: [PATCH 3/3] Remove unused ability to create collages of images --- src/server.ts | 72 +++-------- tests/server.test.ts | 275 ------------------------------------------- 2 files changed, 14 insertions(+), 333 deletions(-) diff --git a/src/server.ts b/src/server.ts index 916d7f4..8f3ad5e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -31,9 +31,8 @@ import { pipe } from "fp-ts/lib/function"; import { URLBuilder } from "./url_builder"; import makeI8N, { asLANGs, KEY, keys as i8nKeys, LANG } from "./i8n"; import { Icon, ICONS, festivals, features } from "./icon"; -import _, { shuffle } from "underscore"; +import _ from "underscore"; import morgan from "morgan"; -import { takeWithRepeats } from "./utils"; import { parse } from "./burn"; import { axiosImageFetcher, ImageFetcher } from "./subsonic"; import { @@ -558,23 +557,11 @@ function server( }); }); - const GRAVITY_9 = [ - "north", - "northeast", - "east", - "southeast", - "south", - "southwest", - "west", - "northwest", - "centre", - ]; - - app.get("/art/:burns/size/:size", (req, res) => { + app.get("/art/:burn/size/:size", (req, res) => { const serviceToken = apiTokens.authTokenFor( req.query[BONOB_ACCESS_TOKEN_HEADER] as string ); - const urns = req.params["burns"]!.split("&").map(parse); + const urn = parse(req.params["burn"]!); const size = Number.parseInt(req.params["size"]!); if (!serviceToken) { @@ -585,55 +572,24 @@ function server( return musicService .login(serviceToken) - .then((musicLibrary) => - Promise.all( - urns.map((it) => { - if (it.system == "external") { - return serverOpts.externalImageResolver(it.resource); - } else { - return musicLibrary.coverArt(it, size); - } - }) - ) - ) - .then((coverArts) => coverArts.filter((it) => it)) - .then(shuffle) - .then((coverArts) => { - if (coverArts.length == 1) { - const coverArt = coverArts[0]!; + .then((musicLibrary) => { + if (urn.system == "external") { + return serverOpts.externalImageResolver(urn.resource); + } else { + return musicLibrary.coverArt(urn, size); + } + }) + .then((coverArt) => { + if(coverArt) { res.status(200); res.setHeader("content-type", coverArt.contentType); return res.send(coverArt.data); - } else if (coverArts.length > 1) { - const gravity = [...GRAVITY_9]; - return sharp({ - create: { - width: size * 3, - height: size * 3, - channels: 3, - background: { r: 255, g: 255, b: 255 }, - }, - }) - .composite( - takeWithRepeats(coverArts, 9).map((art) => ({ - input: art?.data, - gravity: gravity.pop(), - })) - ) - .png() - .toBuffer() - .then((image) => sharp(image).resize(size).png().toBuffer()) - .then((image) => { - res.status(200); - res.setHeader("content-type", "image/png"); - return res.send(image); - }); } else { return res.status(404).send(); } - }) + }) .catch((e: Error) => { - logger.error(`Failed fetching image ${urns.join("&")}/size/${size}`, { + logger.error(`Failed fetching image ${urn}/size/${size}`, { cause: e, }); return res.status(500).send(); diff --git a/tests/server.test.ts b/tests/server.test.ts index eae291c..5efb90b 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -2,9 +2,7 @@ import { v4 as uuid } from "uuid"; import dayjs from "dayjs"; import request from "supertest"; import Image from "image-js"; -import fs from "fs"; import { either as E, taskEither as TE } from "fp-ts"; -import path from "path"; import { AuthFailure, MusicService } from "../src/music_service"; import makeServer, { @@ -1323,279 +1321,6 @@ describe("server", () => { }); }); - describe("fetching multiple images as a collage", () => { - const png = fs.readFileSync( - path.join( - __dirname, - "..", - "docs", - "images", - "chartreuseFuchsia.png" - ) - ); - - describe("fetching a collage of 4 when all are available", () => { - it("should return the image and a 200", async () => { - const urns = [ - "art:1", - "art:2", - "art:3", - "art:4", - ].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - urns.forEach((_) => { - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - }) - ); - }); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/200?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(200); - expect(res.header["content-type"]).toEqual("image/png"); - - expect(musicService.login).toHaveBeenCalledWith(serviceToken); - urns.forEach((it) => { - expect(musicLibrary.coverArt).toHaveBeenCalledWith(it, 200); - }); - - const image = await Image.load(res.body); - expect(image.width).toEqual(200); - expect(image.height).toEqual(200); - }); - }); - - describe("fetching a collage of 4, however only 1 is available", () => { - it("should return the single image", async () => { - const urns = ["art:1", "art:2", "art:3", "art:4"].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - contentType: "image/some-mime-type", - }) - ); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/200?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(200); - expect(res.header["content-type"]).toEqual( - "image/some-mime-type" - ); - }); - }); - - describe("fetching a collage of 4 and all are missing", () => { - it("should return a 404", async () => { - const urns = ["art:1", "art:2", "art:3", "art:4"].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - urns.forEach((_) => { - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - }); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/200?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(404); - }); - }); - - describe("fetching a collage of 9 when all are available", () => { - it("should return the image and a 200", async () => { - const urns = [ - "artist:1", - "artist:2", - "coverArt:3", - "artist:4", - "artist:5", - "artist:6", - "artist:7", - "artist:8", - "artist:9", - ].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - urns.forEach((_) => { - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - }) - ); - }); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(200); - expect(res.header["content-type"]).toEqual("image/png"); - - expect(musicService.login).toHaveBeenCalledWith(serviceToken); - urns.forEach((it) => { - expect(musicLibrary.coverArt).toHaveBeenCalledWith(it, 180); - }); - - const image = await Image.load(res.body); - expect(image.width).toEqual(180); - expect(image.height).toEqual(180); - }); - }); - - describe("fetching a collage of 9 when only 2 are available", () => { - it("should still return an image and a 200", async () => { - const urns = [ - "artist:1", - "artist:2", - "artist:3", - "artist:4", - "artist:5", - "artist:6", - "artist:7", - "artist:8", - "artist:9", - ].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - }) - ); - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - }) - ); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - musicLibrary.coverArt.mockResolvedValueOnce(undefined); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(200); - expect(res.header["content-type"]).toEqual("image/png"); - - expect(musicService.login).toHaveBeenCalledWith(serviceToken); - urns.forEach((urn) => { - expect(musicLibrary.coverArt).toHaveBeenCalledWith(urn, 180); - }); - - const image = await Image.load(res.body); - expect(image.width).toEqual(180); - expect(image.height).toEqual(180); - }); - }); - - describe("fetching a collage of 11", () => { - it("should still return an image and a 200, though will only display 9", async () => { - const urns = [ - "artist:1", - "artist:2", - "artist:3", - "artist:4", - "artist:5", - "artist:6", - "artist:7", - "artist:8", - "artist:9", - "artist:10", - "artist:11", - ].map(resource => ({ system:"subsonic", resource })); - - musicService.login.mockResolvedValue(musicLibrary); - - urns.forEach((_) => { - musicLibrary.coverArt.mockResolvedValueOnce( - coverArtResponse({ - data: png, - }) - ); - }); - - const res = await request(server) - .get( - `/art/${urns.map(it => encodeURIComponent(formatForURL(it))).join( - "&" - )}/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(200); - expect(res.header["content-type"]).toEqual("image/png"); - - expect(musicService.login).toHaveBeenCalledWith(serviceToken); - urns.forEach((it) => { - expect(musicLibrary.coverArt).toHaveBeenCalledWith(it, 180); - }); - - const image = await Image.load(res.body); - expect(image.width).toEqual(180); - expect(image.height).toEqual(180); - }); - }); - - describe("when the image is not available", () => { - it("should return a 404", async () => { - const coverArtURN = { system:"subsonic", resource:"art:404"}; - - musicService.login.mockResolvedValue(musicLibrary); - musicLibrary.coverArt.mockResolvedValue(undefined); - - const res = await request(server) - .get( - `/art/${encodeURIComponent(formatForURL(coverArtURN))}/size/180?${BONOB_ACCESS_TOKEN_HEADER}=${apiToken}` - ) - .set(BONOB_ACCESS_TOKEN_HEADER, apiToken); - - expect(res.status).toEqual(404); - }); - }); - }); - describe("when there is an error", () => { it("should return a 500", async () => { musicService.login.mockResolvedValue(musicLibrary);