From a4406c83bbd649a0d286f21cf8b1ac70fd83ccbb Mon Sep 17 00:00:00 2001 From: Mnigos Date: Wed, 10 Apr 2024 17:36:16 +0200 Subject: [PATCH] refactor(modules/history): move complex logic to service --- src/modules/history/history.module.ts | 5 +- .../history/history.repository.spec.ts | 124 +------------ src/modules/history/history.repository.ts | 66 +------ src/modules/history/history.scheduler.spec.ts | 12 +- src/modules/history/history.scheduler.ts | 6 +- src/modules/history/history.service.spec.ts | 169 ++++++++++++++++++ src/modules/history/history.service.ts | 70 ++++++++ src/modules/history/index.ts | 1 + 8 files changed, 254 insertions(+), 199 deletions(-) create mode 100644 src/modules/history/history.service.spec.ts create mode 100644 src/modules/history/history.service.ts diff --git a/src/modules/history/history.module.ts b/src/modules/history/history.module.ts index bf0026a7..dfb87480 100644 --- a/src/modules/history/history.module.ts +++ b/src/modules/history/history.module.ts @@ -6,6 +6,7 @@ import { History } from './history.entity' import { HistoryRepository } from './history.repository' import { HistoryScheduler } from './history.scheduler' import { HistoryTracksModule } from './tracks' +import { HistoryService } from './history.service' import { UsersModule } from '@modules/users' import { SpotifyModule } from '@modules/spotify' @@ -22,7 +23,7 @@ import { AlbumsModule } from '@modules/albums' AlbumsModule, HistoryTracksModule, ], - providers: [HistoryRepository, HistoryScheduler], - exports: [HistoryRepository, HistoryScheduler], + providers: [HistoryRepository, HistoryScheduler, HistoryService], + exports: [HistoryRepository, HistoryScheduler, HistoryService], }) export class HistoryModule {} diff --git a/src/modules/history/history.repository.spec.ts b/src/modules/history/history.repository.spec.ts index 18c54273..075cac28 100644 --- a/src/modules/history/history.repository.spec.ts +++ b/src/modules/history/history.repository.spec.ts @@ -1,18 +1,14 @@ import { Test } from '@nestjs/testing' import { DataSource } from 'typeorm' import { MockProxy, mock } from 'vitest-mock-extended' -import { PlayHistory } from '@spotify/web-api-ts-sdk' import { HistoryRepository } from './history.repository' import { History } from './history.entity' -import { HistoryTracksRepository } from './tracks' -import { HistoryTrack } from './tracks' describe('HistoryRepository', () => { const userId = 'userId' let historyRepository: HistoryRepository - let historyTracksRepository: HistoryTracksRepository let historyEntityMock: MockProxy @@ -25,19 +21,12 @@ describe('HistoryRepository', () => { createEntityManager: vi.fn(), }, }, - { - provide: HistoryTracksRepository, - useValue: { - createHistoryTracksFromPlayHistory: vi.fn(), - findLastHistoryTrackByUser: vi.fn(), - }, - }, + HistoryRepository, ], }).compile() historyRepository = module.get(HistoryRepository) - historyTracksRepository = module.get(HistoryTracksRepository) historyEntityMock = mock() }) @@ -63,115 +52,4 @@ describe('HistoryRepository', () => { }, }) }) - - describe('updateOrCreateHistoryByUser', () => { - let historyTrack: MockProxy - let historyTracks: MockProxy - let playHistoryMock: MockProxy - - beforeEach(() => { - historyTrack = mock() - historyTracks = [historyTrack] - playHistoryMock = mock() - - historyEntityMock.historyTracks = historyTracks - }) - - test('should create new history', async () => { - const findHistoryByUserSpy = vi - .spyOn(historyRepository, 'findHistoryByUser') - .mockResolvedValue(null) - const createHistoryTracksFromPlayHistorySpy = vi - .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory') - .mockResolvedValue(historyTracks) - const createSpy = vi - .spyOn(historyRepository, 'create') - .mockReturnValue(historyEntityMock) - const saveSpy = vi - .spyOn(historyRepository, 'save') - .mockResolvedValue(historyEntityMock) - - expect( - await historyRepository.updateOrCreateHistoryByUser( - historyEntityMock.user, - playHistoryMock - ) - ).toEqual(historyEntityMock) - - expect(findHistoryByUserSpy).toHaveBeenCalledWith( - historyEntityMock.user.id - ) - expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalledWith( - playHistoryMock, - historyEntityMock.user - ) - expect(createSpy).toHaveBeenCalledWith({ - user: historyEntityMock.user, - historyTracks, - }) - expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) - }) - - test('should update existing history if historyTracks is not empty', async () => { - const findHistoryByUserSpy = vi - .spyOn(historyRepository, 'findHistoryByUser') - .mockResolvedValue(historyEntityMock) - const findLastHistoryTrackByUserSpy = vi - .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser') - .mockResolvedValue(historyTrack) - const createHistoryTracksFromPlayHistorySpy = vi - .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory') - .mockResolvedValue(historyTracks) - const saveSpy = vi - .spyOn(historyRepository, 'save') - .mockResolvedValue(historyEntityMock) - - expect( - await historyRepository.updateOrCreateHistoryByUser( - historyEntityMock.user, - playHistoryMock - ) - ).toEqual(historyEntityMock) - - expect(findHistoryByUserSpy).toHaveBeenCalledWith( - historyEntityMock.user.id - ) - expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith( - historyEntityMock.user.id - ) - expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled() - expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) - }) - - test('should update existing history if historyTracks is empty', async () => { - const findHistoryByUserSpy = vi - .spyOn(historyRepository, 'findHistoryByUser') - .mockResolvedValue(historyEntityMock) - const findLastHistoryTrackByUserSpy = vi - .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser') - .mockResolvedValue(null) - const createHistoryTracksFromPlayHistorySpy = vi - .spyOn(historyTracksRepository, 'createHistoryTracksFromPlayHistory') - .mockResolvedValue(historyTracks) - const saveSpy = vi - .spyOn(historyRepository, 'save') - .mockResolvedValue(historyEntityMock) - - expect( - await historyRepository.updateOrCreateHistoryByUser( - historyEntityMock.user, - playHistoryMock - ) - ).toEqual(historyEntityMock) - - expect(findHistoryByUserSpy).toHaveBeenCalledWith( - historyEntityMock.user.id - ) - expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith( - historyEntityMock.user.id - ) - expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled() - expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) - }) - }) }) diff --git a/src/modules/history/history.repository.ts b/src/modules/history/history.repository.ts index bd66ef08..ad0614dd 100644 --- a/src/modules/history/history.repository.ts +++ b/src/modules/history/history.repository.ts @@ -1,18 +1,11 @@ import { Injectable } from '@nestjs/common' import { DataSource, Repository } from 'typeorm' -import { PlayHistory } from '@spotify/web-api-ts-sdk' import { History } from './history.entity' -import { HistoryTracksRepository } from './tracks' - -import { User } from '@modules/users' @Injectable() export class HistoryRepository extends Repository { - constructor( - private readonly dataSource: DataSource, - private readonly historyTracksRepository: HistoryTracksRepository - ) { + constructor(private readonly dataSource: DataSource) { super(History, dataSource.createEntityManager()) } @@ -25,61 +18,4 @@ export class HistoryRepository extends Repository { }, }) } - - async updateOrCreateHistoryByUser(user: User, playHistory: PlayHistory[]) { - const foundHistory = await this.findHistoryByUser(user.id) - - if (!foundHistory) { - const historyTracks = - await this.historyTracksRepository.createHistoryTracksFromPlayHistory( - playHistory, - user - ) - - const newHistory = this.create({ - user, - historyTracks, - }) - - return this.save(newHistory) - } - - const lastHistoryTrack = - await this.historyTracksRepository.findLastHistoryTrackByUser( - foundHistory.user.id - ) - - if (lastHistoryTrack) { - const latestPlayHistory = playHistory.filter( - (_, index) => - index < - playHistory.findIndex( - ({ track, played_at }) => - track.id === lastHistoryTrack.track.externalId && - new Date(played_at).getTime() === - lastHistoryTrack.playedAt.getTime() - ) - ) - - const newHistoryTracks = - await this.historyTracksRepository.createHistoryTracksFromPlayHistory( - latestPlayHistory, - user - ) - - foundHistory.historyTracks.push(...newHistoryTracks) - - return this.save(foundHistory) - } - - const newHistoryTracks = - await this.historyTracksRepository.createHistoryTracksFromPlayHistory( - playHistory, - user - ) - - foundHistory.historyTracks.push(...newHistoryTracks) - - return this.save(foundHistory) - } } diff --git a/src/modules/history/history.scheduler.spec.ts b/src/modules/history/history.scheduler.spec.ts index b3dd00ee..c96b5d36 100644 --- a/src/modules/history/history.scheduler.spec.ts +++ b/src/modules/history/history.scheduler.spec.ts @@ -6,8 +6,8 @@ import { AccessToken, MaxInt } from '@spotify/web-api-ts-sdk' import { ConfigService } from '@nestjs/config' import { HistoryScheduler } from './history.scheduler' -import { HistoryRepository } from './history.repository' import { History } from './history.entity' +import { HistoryService } from './history.service' import { UsersRepository } from '@modules/users' import { SpotifyAuthService } from '@modules/spotify/auth' @@ -21,7 +21,7 @@ describe('HistoryScheduler', () => { let usersRepository: UsersRepository let spotifyAuthService: SpotifyAuthService let spotifyPlayerService: SpotifyPlayerService - let historyRepository: HistoryRepository + let historyService: HistoryService let schedulerRegistry: SchedulerRegistry beforeEach(async () => { @@ -52,9 +52,9 @@ describe('HistoryScheduler', () => { }, }, { - provide: HistoryRepository, + provide: HistoryService, useValue: { - updateOrCreateHistoryByUser: vi.fn(), + updateOrCreate: vi.fn(), }, }, { @@ -77,7 +77,7 @@ describe('HistoryScheduler', () => { usersRepository = module.get(UsersRepository) spotifyAuthService = module.get(SpotifyAuthService) spotifyPlayerService = module.get(SpotifyPlayerService) - historyRepository = module.get(HistoryRepository) + historyService = module.get(HistoryService) schedulerRegistry = module.get(SchedulerRegistry) }) @@ -134,7 +134,7 @@ describe('HistoryScheduler', () => { > ).mockResolvedValue(recentlyPlayedTracksPageMock) const updateOrCreateHistoryByUserSpy = vi - .spyOn(historyRepository, 'updateOrCreateHistoryByUser') + .spyOn(historyService, 'updateOrCreate') .mockResolvedValue(mock()) await historyScheduler.fetchUserHistory(userMock) diff --git a/src/modules/history/history.scheduler.ts b/src/modules/history/history.scheduler.ts index a04ca22e..23c2dd02 100644 --- a/src/modules/history/history.scheduler.ts +++ b/src/modules/history/history.scheduler.ts @@ -9,7 +9,7 @@ import { SchedulerRegistry } from '@nestjs/schedule' import { ConfigService } from '@nestjs/config' import ms from 'ms' -import { HistoryRepository } from './history.repository' +import { HistoryService } from './history.service' import { User, UsersRepository } from '@modules/users' import { SpotifyAuthService } from '@modules/spotify/auth' @@ -27,7 +27,7 @@ export class HistoryScheduler implements OnModuleInit { private readonly usersRepository: UsersRepository, private readonly spotifyAuthService: SpotifyAuthService, private readonly spotifyPlayerService: SpotifyPlayerService, - private readonly historyRepository: HistoryRepository, + private readonly historyService: HistoryService, private readonly schedulerRegistry: SchedulerRegistry, private readonly configService: ConfigService ) {} @@ -73,6 +73,6 @@ export class HistoryScheduler implements OnModuleInit { false ) - await this.historyRepository.updateOrCreateHistoryByUser(user, items) + await this.historyService.updateOrCreate(user, items) } } diff --git a/src/modules/history/history.service.spec.ts b/src/modules/history/history.service.spec.ts new file mode 100644 index 00000000..65594b95 --- /dev/null +++ b/src/modules/history/history.service.spec.ts @@ -0,0 +1,169 @@ +import { Test } from '@nestjs/testing' +import { MockProxy, mock } from 'vitest-mock-extended' +import { PlayHistory } from '@spotify/web-api-ts-sdk' + +import { HistoryRepository } from './history.repository' +import { HistoryService } from './history.service' +import { History } from './history.entity' +import { + HistoryTrack, + HistoryTracksRepository, + HistoryTracksService, +} from './tracks' + +describe('HistoryService', () => { + let historyService: HistoryService + let historyRepository: HistoryRepository + let historyTracksRepository: HistoryTracksRepository + let historyTracksService: HistoryTracksService + + beforeEach(async () => { + const module = await Test.createTestingModule({ + providers: [ + HistoryService, + { + provide: HistoryRepository, + useValue: { + findHistoryByUser: vi.fn(), + create: vi.fn(), + save: vi.fn(), + }, + }, + { + provide: HistoryTracksRepository, + useValue: { + findLastHistoryTrackByUser: vi.fn(), + }, + }, + { + provide: HistoryTracksService, + useValue: { + create: vi.fn(), + }, + }, + ], + }).compile() + + historyService = module.get(HistoryService) + historyRepository = module.get(HistoryRepository) + historyTracksRepository = module.get(HistoryTracksRepository) + historyTracksService = module.get(HistoryTracksService) + }) + + test('should be defined', () => { + expect(historyService).toBeDefined() + }) + + describe('updateOrCreate', () => { + let historyTrack: MockProxy + let historyTracks: MockProxy + let playHistoryMock: MockProxy + let historyEntityMock: MockProxy + + beforeEach(() => { + historyEntityMock = mock() + historyTrack = mock() + historyTracks = [historyTrack] + playHistoryMock = mock() + + historyEntityMock.historyTracks = historyTracks + }) + + test('should create new history', async () => { + const findHistoryByUserSpy = vi + .spyOn(historyRepository, 'findHistoryByUser') + .mockResolvedValue(null) + const createHistoryTracksFromPlayHistorySpy = vi + .spyOn(historyTracksService, 'create') + .mockResolvedValue(historyTracks) + const createSpy = vi + .spyOn(historyRepository, 'create') + .mockReturnValue(historyEntityMock) + const saveSpy = vi + .spyOn(historyRepository, 'save') + .mockResolvedValue(historyEntityMock) + + expect( + await historyService.updateOrCreate( + historyEntityMock.user, + playHistoryMock + ) + ).toEqual(historyEntityMock) + + expect(findHistoryByUserSpy).toHaveBeenCalledWith( + historyEntityMock.user.id + ) + expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalledWith( + playHistoryMock, + historyEntityMock.user + ) + expect(createSpy).toHaveBeenCalledWith({ + user: historyEntityMock.user, + historyTracks, + }) + expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) + }) + + test('should update existing history if historyTracks is not empty', async () => { + const findHistoryByUserSpy = vi + .spyOn(historyRepository, 'findHistoryByUser') + .mockResolvedValue(historyEntityMock) + const findLastHistoryTrackByUserSpy = vi + .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser') + .mockResolvedValue(historyTrack) + const createHistoryTracksFromPlayHistorySpy = vi + .spyOn(historyTracksService, 'create') + .mockResolvedValue(historyTracks) + const saveSpy = vi + .spyOn(historyRepository, 'save') + .mockResolvedValue(historyEntityMock) + + expect( + await historyService.updateOrCreate( + historyEntityMock.user, + playHistoryMock + ) + ).toEqual(historyEntityMock) + + expect(findHistoryByUserSpy).toHaveBeenCalledWith( + historyEntityMock.user.id + ) + expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith( + historyEntityMock.user.id + ) + expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled() + expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) + }) + + test('should update existing history if historyTracks is empty', async () => { + const findHistoryByUserSpy = vi + .spyOn(historyRepository, 'findHistoryByUser') + .mockResolvedValue(historyEntityMock) + const findLastHistoryTrackByUserSpy = vi + .spyOn(historyTracksRepository, 'findLastHistoryTrackByUser') + .mockResolvedValue(null) + const createHistoryTracksFromPlayHistorySpy = vi + .spyOn(historyTracksService, 'create') + .mockResolvedValue(historyTracks) + const saveSpy = vi + .spyOn(historyRepository, 'save') + .mockResolvedValue(historyEntityMock) + + expect( + await historyService.updateOrCreate( + historyEntityMock.user, + playHistoryMock + ) + ).toEqual(historyEntityMock) + + expect(findHistoryByUserSpy).toHaveBeenCalledWith( + historyEntityMock.user.id + ) + expect(findLastHistoryTrackByUserSpy).toHaveBeenCalledWith( + historyEntityMock.user.id + ) + expect(createHistoryTracksFromPlayHistorySpy).toHaveBeenCalled() + expect(saveSpy).toHaveBeenCalledWith(historyEntityMock) + }) + }) +}) diff --git a/src/modules/history/history.service.ts b/src/modules/history/history.service.ts new file mode 100644 index 00000000..2b83bc6f --- /dev/null +++ b/src/modules/history/history.service.ts @@ -0,0 +1,70 @@ +import { Injectable } from '@nestjs/common' +import { PlayHistory } from '@spotify/web-api-ts-sdk' + +import { HistoryRepository } from './history.repository' +import { HistoryTracksRepository, HistoryTracksService } from './tracks' + +import { User } from '@modules/users' + +@Injectable() +export class HistoryService { + constructor( + private readonly historyRepository: HistoryRepository, + private readonly historyTracksRepository: HistoryTracksRepository, + private readonly historyTracksService: HistoryTracksService + ) {} + + async updateOrCreate(user: User, playHistory: PlayHistory[]) { + const foundHistory = await this.historyRepository.findHistoryByUser(user.id) + + if (!foundHistory) { + const historyTracks = await this.historyTracksService.create( + playHistory, + user + ) + + const newHistory = this.historyRepository.create({ + user, + historyTracks, + }) + + return this.historyRepository.save(newHistory) + } + + const lastHistoryTrack = + await this.historyTracksRepository.findLastHistoryTrackByUser( + foundHistory.user.id + ) + + if (lastHistoryTrack) { + const latestPlayHistory = playHistory.filter( + (_, index) => + index < + playHistory.findIndex( + ({ track, played_at }) => + track.id === lastHistoryTrack.track.externalId && + new Date(played_at).getTime() === + lastHistoryTrack.playedAt.getTime() + ) + ) + + const newHistoryTracks = await this.historyTracksService.create( + latestPlayHistory, + user + ) + + foundHistory.historyTracks.push(...newHistoryTracks) + + return this.historyRepository.save(foundHistory) + } + + const newHistoryTracks = await this.historyTracksService.create( + playHistory, + user + ) + + foundHistory.historyTracks.push(...newHistoryTracks) + + return this.historyRepository.save(foundHistory) + } +} diff --git a/src/modules/history/index.ts b/src/modules/history/index.ts index 642b64c1..04b6cf65 100644 --- a/src/modules/history/index.ts +++ b/src/modules/history/index.ts @@ -2,3 +2,4 @@ export * from './history.entity' export * from './history.repository' export * from './history.module' export * from './history.scheduler' +export * from './history.service'