fix(api): missed listens from spotify

Listory could miss some listens when the Spotify Api misbehaves.

Sometimes the listens added to the last-recently-played endpoint
are showing up are out-of-order.

Because of our optimization to only retrieve listens newer than
the lastRefreshTime we would skip retrieving those out-of-order listens.

By always retrieving the maximum 50 listens we can be reasonably
sure that we got all the listens.

We also had to improve the handling of duplicate listens, as we now
have a lot of them, curtesy of removing the lastRefreshTime
optimization.

src/listens/dto/create-listen.dto.ts

@@ -1,8 +1,16 @@
 import { Track } from "../../music-library/track.entity";
 import { User } from "../../users/user.entity";
+import { Listen } from "../listen.entity";
 
-export class CreateListenDto {
+// tslint:disable max-classes-per-file
+
+export class CreateListenRequestDto {
   track: Track;
   user: User;
   playedAt: Date;
 }
+
+export class CreateListenResponseDto {
+  listen: Listen;
+  isDuplicate: boolean;
+}

src/listens/listen.repository.ts

@@ -1,8 +1,12 @@
 // tslint:disable: max-classes-per-file
 import { EntityRepository, Repository, SelectQueryBuilder } from "typeorm";
-import { User } from "../users/user.entity";
-import { Listen } from "./listen.entity";
 import { Interval } from "../reports/interval";
+import { User } from "../users/user.entity";
+import {
+  CreateListenRequestDto,
+  CreateListenResponseDto,
+} from "./dto/create-listen.dto";
+import { Listen } from "./listen.entity";
 
 export class ListenScopes extends SelectQueryBuilder<Listen> {
   /**
@@ -32,4 +36,35 @@ export class ListenRepository extends Repository<Listen> {
   get scoped(): ListenScopes {
     return new ListenScopes(this.createQueryBuilder("listen"));
   }
+
+  async insertNoConflict({
+    user,
+    track,
+    playedAt,
+  }: CreateListenRequestDto): Promise<CreateListenResponseDto> {
+    const result = await this.createQueryBuilder()
+      .insert()
+      .values({
+        user,
+        track,
+        playedAt,
+      })
+      .onConflict('("playedAt", "trackId", "userId") DO NOTHING')
+      .execute();
+
+    const [insertedRowIdentifier] = result.identifiers;
+
+    if (!insertedRowIdentifier) {
+      // We did not insert a new listen, it already existed
+      return {
+        listen: await this.findOne({ where: { user, track, playedAt } }),
+        isDuplicate: true,
+      };
+    }
+
+    return {
+      listen: await this.findOne(insertedRowIdentifier.id),
+      isDuplicate: false,
+    };
+  }
 }

src/listens/listens.service.ts

@@ -1,11 +1,13 @@
 import { Injectable } from "@nestjs/common";
-import { getTime, parseISO, getUnixTime } from "date-fns";
 import {
   IPaginationOptions,
   paginate,
   Pagination,
 } from "nestjs-typeorm-paginate";
-import { CreateListenDto } from "./dto/create-listen.dto";
+import {
+  CreateListenRequestDto,
+  CreateListenResponseDto,
+} from "./dto/create-listen.dto";
 import { GetListensDto } from "./dto/get-listens.dto";
 import { Listen } from "./listen.entity";
 import { ListenRepository, ListenScopes } from "./listen.repository";
@@ -18,26 +20,14 @@ export class ListensService {
     user,
     track,
     playedAt,
-  }: CreateListenDto): Promise<Listen> {
+  }: CreateListenRequestDto): Promise<CreateListenResponseDto> {
-    const listen = this.listenRepository.create();
+    const response = await this.listenRepository.insertNoConflict({
+      user,
+      track,
+      playedAt,
+    });
 
-    listen.user = user;
+    return response;
-    listen.track = track;
-    listen.playedAt = playedAt;
-
-    try {
-      await this.listenRepository.save(listen);
-    } catch (err) {
-      if (err.code === "23505") {
-        return this.listenRepository.findOne({
-          where: { user, track, playedAt },
-        });
-      }
-
-      throw err;
-    }
-
-    return listen;
   }
 
   async getListens(

src/sources/spotify/spotify-api/spotify-api.service.ts

@@ -18,10 +18,6 @@ export class SpotifyApiService {
       limit: 50,
     };
 
-    if (lastRefreshTime) {
-      parameters.after = lastRefreshTime.getTime();
-    }
-
     const history = await this.httpService
       .get<PagingObject<PlayHistoryObject>>(`v1/me/player/recently-played`, {
         headers: { Authorization: `Bearer ${accessToken}` },

src/sources/spotify/spotify.service.ts

@@ -1,5 +1,5 @@
 import { Injectable } from "@nestjs/common";
-import { Interval, Timeout } from "@nestjs/schedule";
+import { Interval } from "@nestjs/schedule";
 import { ListensService } from "../../listens/listens.service";
 import { Logger } from "../../logger/logger.service";
 import { Album } from "../../music-library/album.entity";
@@ -80,19 +80,21 @@ export class SpotifyService {
       playHistory.map(async (history) => {
         const track = await this.importTrack(history.track.id);
 
-        this.listensService.createListen({
+        const { isDuplicate } = await this.listensService.createListen({
           user,
           track,
           playedAt: new Date(history.played_at),
         });
 
-        this.logger.debug(
+        if (!isDuplicate) {
-          `New listen found! ${user.id} listened to "${
+          this.logger.debug(
-            track.name
+            `New listen found! ${user.id} listened to "${
-          }" by ${track.artists
+              track.name
-            ?.map((artist) => `"${artist.name}"`)
+            }" by ${track.artists
-            .join(", ")}`
+              ?.map((artist) => `"${artist.name}"`)
-        );
+              .join(", ")}`
+          );
+        }
       })
     );
 
@@ -103,12 +105,14 @@ export class SpotifyService {
         .pop()
     );
 
-    this.logger.debug(
+    /**
-      `Updating last refresh time for user ${
+     * lastRefreshTime was previously used to only get new listens from Spotify
-        user.id
+     * but the Spotify WEB Api was sometimes not adding the listens in the right
-      }, new time: ${newestPlayTime.toISOString()}`
+     * order, causing us to miss some listens.
-    );
+     *
-
+     * The variable will still be set, in case we want to add the functionality
+     * again.
+     */
     this.usersService.updateSpotifyConnection(user, {
       ...user.spotify,
       lastRefreshTime: newestPlayTime,