From c312778e13cd35f743dab9c300d7417dab19e69b Mon Sep 17 00:00:00 2001 From: Simon J Date: Mon, 27 Sep 2021 19:13:47 +1000 Subject: [PATCH] Marking nowPlaying in smapi setPlayedSeconds handler so does not mark when sonos pre-caches a track (#57) --- src/server.ts | 28 +++++----------- src/smapi.ts | 3 ++ tests/server.test.ts | 14 -------- tests/smapi.test.ts | 80 +++++++++++++++++++++++++++++++++----------- 4 files changed, 72 insertions(+), 53 deletions(-) diff --git a/src/server.ts b/src/server.ts index 0f537e0..8e41fc5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -311,7 +311,7 @@ function server( }) .then((stream) => ({ musicLibrary: it, stream })) ) - .then(({ musicLibrary, stream }) => { + .then(({ stream }) => { logger.info( `stream response from music service for ${id}, status=${ stream.status @@ -330,32 +330,25 @@ function server( filter, headers, sendStream, - nowPlaying, }: { status: number; filter: Transform; headers: Record; sendStream: boolean; - nowPlaying: boolean; }) => { logger.info( `<- /stream/track/${id}, status=${status}, headers=${JSON.stringify( headers )}` ); - (nowPlaying - ? musicLibrary.nowPlaying(id) - : Promise.resolve(true) - ).then((_) => { - res.status(status); - Object.entries(headers) - .filter(([_, v]) => v !== undefined) - .forEach(([header, value]) => { - res.setHeader(header, value!); - }); - if (sendStream) stream.stream.pipe(filter).pipe(res); - else res.send(); - }); + res.status(status); + Object.entries(headers) + .filter(([_, v]) => v !== undefined) + .forEach(([header, value]) => { + res.setHeader(header, value!); + }); + if (sendStream) stream.stream.pipe(filter).pipe(res); + else res.send(); }; if (stream.status == 200) { @@ -370,7 +363,6 @@ function server( "accept-ranges": stream.headers["accept-ranges"], }, sendStream: req.method == "GET", - nowPlaying: req.method == "GET", }); } else if (stream.status == 206) { respondWith({ @@ -385,7 +377,6 @@ function server( "accept-ranges": stream.headers["accept-ranges"], }, sendStream: req.method == "GET", - nowPlaying: req.method == "GET", }); } else { respondWith({ @@ -393,7 +384,6 @@ function server( filter: new PassThrough(), headers: {}, sendStream: req.method == "GET", - nowPlaying: false, }); } }); diff --git a/src/smapi.ts b/src/smapi.ts index cd3191a..ee901fc 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -888,6 +888,9 @@ function bindSmapiSoapServiceToExpress( switch (type) { case "track": musicLibrary.track(typeId).then(({ duration }) => { + if(+seconds > 0) { + musicLibrary.nowPlaying(typeId); + } if ( (duration < 30 && +seconds >= 10) || (duration >= 30 && +seconds >= 30) diff --git a/tests/server.test.ts b/tests/server.test.ts index 20e68e6..d1dff50 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -707,7 +707,6 @@ describe("server", () => { const musicLibrary = { stream: jest.fn(), scrobble: jest.fn(), - nowPlaying: jest.fn(), }; let now = dayjs(); const accessTokens = new ExpiringAccessTokens({ now: () => now }); @@ -871,7 +870,6 @@ describe("server", () => { expect(res.status).toEqual(404); - expect(musicLibrary.nowPlaying).not.toHaveBeenCalled(); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -892,7 +890,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -913,7 +910,6 @@ describe("server", () => { expect(Object.keys(res.headers)).not.toContain("content-range"); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -933,7 +929,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -953,7 +948,6 @@ describe("server", () => { expect(Object.keys(res.headers)).not.toContain("content-range"); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -972,7 +966,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -992,7 +985,6 @@ describe("server", () => { expect(res.header["content-range"]).toBeUndefined(); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -1012,7 +1004,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -1034,7 +1025,6 @@ describe("server", () => { ); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -1055,7 +1045,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const requestedRange = "40-"; @@ -1078,7 +1067,6 @@ describe("server", () => { expect(res.header["content-range"]).toBeUndefined(); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId, range: requestedRange, @@ -1101,7 +1089,6 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); - musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -1124,7 +1111,6 @@ describe("server", () => { ); expect(musicService.login).toHaveBeenCalledWith(authToken); - expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId, range: "4000-5000", diff --git a/tests/smapi.test.ts b/tests/smapi.test.ts index 740cb43..272d872 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -472,6 +472,7 @@ describe("api", () => { deletePlaylist: jest.fn(), removeFromPlaylist: jest.fn(), scrobble: jest.fn(), + nowPlaying: jest.fn(), }; const accessTokens = { mint: jest.fn(), @@ -2810,9 +2811,11 @@ describe("api", () => { function itShouldScroble({ trackId, secondsPlayed, + shouldMarkNowPlaying, }: { trackId: string; secondsPlayed: number; + shouldMarkNowPlaying: boolean, }) { it("should scrobble", async () => { musicLibrary.scrobble.mockResolvedValue(true); @@ -2827,15 +2830,22 @@ describe("api", () => { expect(accessTokens.mint).toHaveBeenCalledWith(authToken); expect(musicLibrary.track).toHaveBeenCalledWith(trackId); expect(musicLibrary.scrobble).toHaveBeenCalledWith(trackId); + if(shouldMarkNowPlaying) { + expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); + } else { + expect(musicLibrary.nowPlaying).not.toHaveBeenCalled(); + } }); } function itShouldNotScroble({ trackId, secondsPlayed, + shouldMarkNowPlaying, }: { trackId: string; secondsPlayed: number; + shouldMarkNowPlaying: boolean, }) { it("should scrobble", async () => { const result = await ws.setPlayedSecondsAsync({ @@ -2848,6 +2858,11 @@ describe("api", () => { expect(accessTokens.mint).toHaveBeenCalledWith(authToken); expect(musicLibrary.track).toHaveBeenCalledWith(trackId); expect(musicLibrary.scrobble).not.toHaveBeenCalled(); + if(shouldMarkNowPlaying) { + expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); + } else { + expect(musicLibrary.nowPlaying).not.toHaveBeenCalled(); + } }); } @@ -2858,16 +2873,24 @@ describe("api", () => { ); }); - describe("when the played length is 30 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 30 }); + describe("when the seconds played is 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 30, shouldMarkNowPlaying: true }); }); - describe("when the played length is > 30 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 90 }); + describe("when the seconds played is > 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 90, shouldMarkNowPlaying: true }); }); - describe("when the played length is < 30 seconds", () => { - itShouldNotScroble({ trackId, secondsPlayed: 29 }); + describe("when the seconds played is < 30 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 29, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 1 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 1, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 0 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 0, shouldMarkNowPlaying: false }); }); }); @@ -2878,16 +2901,24 @@ describe("api", () => { ); }); - describe("when the played length is 30 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 30 }); + describe("when the seconds played is 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 30, shouldMarkNowPlaying: true }); }); - describe("when the played length is > 30 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 90 }); + describe("when the seconds played is > 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 90, shouldMarkNowPlaying: true }); }); - describe("when the played length is < 30 seconds", () => { - itShouldNotScroble({ trackId, secondsPlayed: 29 }); + describe("when the seconds played is < 30 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 29, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 1 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 1, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 0 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 0, shouldMarkNowPlaying: false }); }); }); @@ -2898,20 +2929,28 @@ describe("api", () => { ); }); - describe("when the played length is 29 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 30 }); + describe("when the seconds played is 29 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 30, shouldMarkNowPlaying: true }); }); - describe("when the played length is > 29 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 30 }); + describe("when the seconds played is > 29 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 30, shouldMarkNowPlaying: true }); }); - describe("when the played length is 10 seconds", () => { - itShouldScroble({ trackId, secondsPlayed: 10 }); + describe("when the seconds played is 10 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 10, shouldMarkNowPlaying: true }); }); - describe("when the played length is < 10 seconds", () => { - itShouldNotScroble({ trackId, secondsPlayed: 9 }); + describe("when the seconds played is < 10 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 9, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 1 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 1, shouldMarkNowPlaying: true }); + }); + + describe("when the seconds played is 0 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 0, shouldMarkNowPlaying: false }); }); }); }); @@ -2926,6 +2965,7 @@ describe("api", () => { expect(result[0]).toEqual({ setPlayedSecondsResult: null }); expect(musicService.login).toHaveBeenCalledWith(authToken); expect(accessTokens.mint).toHaveBeenCalledWith(authToken); + expect(musicLibrary.nowPlaying).not.toHaveBeenCalled(); expect(musicLibrary.scrobble).not.toHaveBeenCalled(); }); });