From a3a30455d07137e3a56b400a3ea9e30e87c734b9 Mon Sep 17 00:00:00 2001 From: Simon J Date: Sat, 16 Oct 2021 14:51:07 +1100 Subject: [PATCH] Revert "Marking nowPlaying in smapi setPlayedSeconds handler so does not mark when sonos pre-caches a track (#57)" (#66) This reverts commit c312778e13cd35f743dab9c300d7417dab19e69b. --- src/server.ts | 28 ++++++--- src/smapi.ts | 30 +++------ tests/server.test.ts | 14 +++++ tests/smapi.test.ts | 143 ++++++------------------------------------- 4 files changed, 63 insertions(+), 152 deletions(-) diff --git a/src/server.ts b/src/server.ts index 5d8caed..de42a15 100644 --- a/src/server.ts +++ b/src/server.ts @@ -356,7 +356,7 @@ function server( }) .then((stream) => ({ musicLibrary: it, stream })) ) - .then(({ stream }) => { + .then(({ musicLibrary, stream }) => { logger.info( `${trace} bnb<- stream response from music service for ${id}, status=${ stream.status @@ -375,25 +375,32 @@ function server( filter, headers, sendStream, + nowPlaying, }: { status: number; filter: Transform; headers: Record; sendStream: boolean; + nowPlaying: boolean; }) => { logger.info( `${trace} bnb-> ${ req.path }, status=${status}, headers=${JSON.stringify(headers)}` ); - 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(); + (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(); + }); }; if (stream.status == 200) { @@ -408,6 +415,7 @@ function server( "accept-ranges": stream.headers["accept-ranges"], }, sendStream: req.method == "GET", + nowPlaying: req.method == "GET", }); } else if (stream.status == 206) { respondWith({ @@ -422,6 +430,7 @@ function server( "accept-ranges": stream.headers["accept-ranges"], }, sendStream: req.method == "GET", + nowPlaying: req.method == "GET", }); } else { respondWith({ @@ -429,6 +438,7 @@ function server( filter: new PassThrough(), headers: {}, sendStream: req.method == "GET", + nowPlaying: false, }); } }); diff --git a/src/smapi.ts b/src/smapi.ts index 0113dc5..f15eaf3 100644 --- a/src/smapi.ts +++ b/src/smapi.ts @@ -914,26 +914,16 @@ function bindSmapiSoapServiceToExpress( .then(({ musicLibrary, type, typeId }) => { switch (type) { case "track": - return musicLibrary - .track(typeId) - .then(({ duration }) => { - if ( - (duration < 30 && +seconds >= 10) || - (duration >= 30 && +seconds >= 30) - ) { - return musicLibrary.scrobble(typeId); - } else { - return Promise.resolve(true); - } - }) - .then(() => { - if (+seconds > 0) { - return musicLibrary.nowPlaying(typeId); - } else { - return Promise.resolve(true); - } - }); - break; + return musicLibrary.track(typeId).then(({ duration }) => { + if ( + (duration < 30 && +seconds >= 10) || + (duration >= 30 && +seconds >= 30) + ) { + return musicLibrary.scrobble(typeId); + } else { + return Promise.resolve(true); + } + }); default: logger.info("Unsupported scrobble", { id, seconds }); return Promise.resolve(true); diff --git a/tests/server.test.ts b/tests/server.test.ts index 609f958..d1526c2 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -707,6 +707,7 @@ describe("server", () => { const musicLibrary = { stream: jest.fn(), scrobble: jest.fn(), + nowPlaying: jest.fn(), }; let now = dayjs(); const accessTokens = new ExpiringAccessTokens({ now: () => now }); @@ -870,6 +871,7 @@ describe("server", () => { expect(res.status).toEqual(404); + expect(musicLibrary.nowPlaying).not.toHaveBeenCalled(); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -890,6 +892,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -909,6 +912,7 @@ 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 }); }); }); @@ -928,6 +932,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -946,6 +951,7 @@ 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 }); }); }); @@ -964,6 +970,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -982,6 +989,7 @@ describe("server", () => { expect(res.header["content-range"]).toBeUndefined(); expect(musicService.login).toHaveBeenCalledWith(authToken); + expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -1001,6 +1009,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -1021,6 +1030,7 @@ describe("server", () => { ); expect(musicService.login).toHaveBeenCalledWith(authToken); + expect(musicLibrary.nowPlaying).toHaveBeenCalledWith(trackId); expect(musicLibrary.stream).toHaveBeenCalledWith({ trackId }); }); }); @@ -1041,6 +1051,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const requestedRange = "40-"; @@ -1062,6 +1073,7 @@ 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, @@ -1084,6 +1096,7 @@ describe("server", () => { musicService.login.mockResolvedValue(musicLibrary); musicLibrary.stream.mockResolvedValue(stream); + musicLibrary.nowPlaying.mockResolvedValue(true); const res = await request(server) .get( @@ -1105,6 +1118,7 @@ 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 e8ccc85..6fdb43d 100644 --- a/tests/smapi.test.ts +++ b/tests/smapi.test.ts @@ -3071,11 +3071,9 @@ describe("api", () => { function itShouldScroble({ trackId, secondsPlayed, - shouldMarkNowPlaying, }: { trackId: string; secondsPlayed: number; - shouldMarkNowPlaying: boolean; }) { it("should scrobble", async () => { musicLibrary.scrobble.mockResolvedValue(true); @@ -3090,22 +3088,15 @@ 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({ @@ -3118,11 +3109,6 @@ 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(); - } }); } @@ -3133,44 +3119,16 @@ describe("api", () => { ); }); - 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: 30 }); }); - describe("when the seconds played is > 30 seconds", () => { - itShouldScroble({ - trackId, - secondsPlayed: 90, - shouldMarkNowPlaying: true, - }); + describe("when the played length is > 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 90 }); }); - 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, - }); + describe("when the played length is < 30 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 29 }); }); }); @@ -3181,44 +3139,16 @@ describe("api", () => { ); }); - 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: 30 }); }); - describe("when the seconds played is > 30 seconds", () => { - itShouldScroble({ - trackId, - secondsPlayed: 90, - shouldMarkNowPlaying: true, - }); + describe("when the played length is > 30 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 90 }); }); - 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, - }); + describe("when the played length is < 30 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 29 }); }); }); @@ -3229,52 +3159,20 @@ describe("api", () => { ); }); - 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 > 29 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 30 }); }); - describe("when the seconds played is 10 seconds", () => { - itShouldScroble({ - trackId, - secondsPlayed: 10, - shouldMarkNowPlaying: true, - }); + describe("when the played length is 10 seconds", () => { + itShouldScroble({ trackId, secondsPlayed: 10 }); }); - 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, - }); + describe("when the played length is < 10 seconds", () => { + itShouldNotScroble({ trackId, secondsPlayed: 9 }); }); }); }); @@ -3289,7 +3187,6 @@ 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(); }); });