Revert "Marking nowPlaying in smapi setPlayedSeconds handler so does not mark when sonos pre-caches a track (#57)" (#66)

This reverts commit c312778e13.

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<string, string>;
             sendStream: boolean;
+            nowPlaying: boolean;
           }) => {
             logger.info(
               `${trace} bnb-> ${
                 req.path
               }, status=${status}, headers=${JSON.stringify(headers)}`
             );
-            res.status(status);
+            (nowPlaying
-            Object.entries(headers)
+              ? musicLibrary.nowPlaying(id)
-              .filter(([_, v]) => v !== undefined)
+              : Promise.resolve(true)
-              .forEach(([header, value]) => {
+            ).then((_) => {
-                res.setHeader(header, value!);
+              res.status(status);
-              });
+              Object.entries(headers)
-            if (sendStream) stream.stream.pipe(filter).pipe(res);
+                .filter(([_, v]) => v !== undefined)
-            else res.send();
+                .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,
             });
           }
         });

src/smapi.ts

@@ -914,26 +914,16 @@ function bindSmapiSoapServiceToExpress(
               .then(({ musicLibrary, type, typeId }) => {
                 switch (type) {
                   case "track":
-                    return musicLibrary
+                    return musicLibrary.track(typeId).then(({ duration }) => {
-                      .track(typeId)
+                      if (
-                      .then(({ duration }) => {
+                        (duration < 30 && +seconds >= 10) ||
-                        if (
+                        (duration >= 30 && +seconds >= 30)
-                          (duration < 30 && +seconds >= 10) ||
+                      ) {
-                          (duration >= 30 && +seconds >= 30)
+                        return musicLibrary.scrobble(typeId);
-                        ) {
+                      } else {
-                          return musicLibrary.scrobble(typeId);
+                        return Promise.resolve(true);
-                        } else {
+                      }
-                          return Promise.resolve(true);
+                    });
-                        }
-                      })
-                      .then(() => {
-                        if (+seconds > 0) {
-                          return musicLibrary.nowPlaying(typeId);
-                        } else {
-                          return Promise.resolve(true);
-                        }
-                      });
-                    break;
                   default:
                     logger.info("Unsupported scrobble", { id, seconds });
                     return Promise.resolve(true);

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",

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", () => {
+              describe("when the played length is 30 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 30 });
-                  trackId,
-                  secondsPlayed: 30,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is > 30 seconds", () => {
+              describe("when the played length is > 30 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 90 });
-                  trackId,
-                  secondsPlayed: 90,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is < 30 seconds", () => {
+              describe("when the played length is < 30 seconds", () => {
-                itShouldNotScroble({
+                itShouldNotScroble({ trackId, secondsPlayed: 29 });
-                  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,
-                });
               });
             });
 
@@ -3181,44 +3139,16 @@ describe("api", () => {
                 );
               });
 
-              describe("when the seconds played is 30 seconds", () => {
+              describe("when the played length is 30 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 30 });
-                  trackId,
-                  secondsPlayed: 30,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is > 30 seconds", () => {
+              describe("when the played length is > 30 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 90 });
-                  trackId,
-                  secondsPlayed: 90,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is < 30 seconds", () => {
+              describe("when the played length is < 30 seconds", () => {
-                itShouldNotScroble({
+                itShouldNotScroble({ trackId, secondsPlayed: 29 });
-                  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,
-                });
               });
             });
 
@@ -3229,52 +3159,20 @@ describe("api", () => {
                 );
               });
 
-              describe("when the seconds played is 29 seconds", () => {
+              describe("when the played length is 29 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 30 });
-                  trackId,
-                  secondsPlayed: 30,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is > 29 seconds", () => {
+              describe("when the played length is > 29 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 30 });
-                  trackId,
-                  secondsPlayed: 30,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is 10 seconds", () => {
+              describe("when the played length is 10 seconds", () => {
-                itShouldScroble({
+                itShouldScroble({ trackId, secondsPlayed: 10 });
-                  trackId,
-                  secondsPlayed: 10,
-                  shouldMarkNowPlaying: true,
-                });
               });
 
-              describe("when the seconds played is < 10 seconds", () => {
+              describe("when the played length is < 10 seconds", () => {
-                itShouldNotScroble({
+                itShouldNotScroble({ trackId, secondsPlayed: 9 });
-                  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,
-                });
               });
             });
           });
@@ -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();
             });
           });