From c153e53c5b67b45740643e2213678351fd9e7607 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 19 Sep 2024 12:55:50 -0400 Subject: [PATCH] feat: Improve upstream scrobble refresh logic and caching controls * Add refreshMinInterval to prevent hammering upstream services * Refactor refresh logic to be simpler and account for backlogged tracks * (test): Improve play generation utils for testing * (test): Refactor testing for upstream scrobble refreshing to use more actual scrobbler class behavior --- .../infrastructure/config/client/index.ts | 28 +++-- src/backend/common/schema/aio-client.json | 16 ++- src/backend/common/schema/aio.json | 32 ++++- src/backend/common/schema/client.json | 16 ++- .../scrobblers/AbstractScrobbleClient.ts | 91 +++++++++++---- src/backend/tests/plays/withDuration.json | 4 +- .../tests/scrobbler/scrobblers.test.ts | 35 +++--- src/backend/tests/utils/PlayTestUtils.ts | 109 +++++++++++++----- 8 files changed, 242 insertions(+), 89 deletions(-) diff --git a/src/backend/common/infrastructure/config/client/index.ts b/src/backend/common/infrastructure/config/client/index.ts index bffcf589..ca28aad5 100644 --- a/src/backend/common/infrastructure/config/client/index.ts +++ b/src/backend/common/infrastructure/config/client/index.ts @@ -30,7 +30,7 @@ export interface MatchLoggingOptions { export interface CommonClientData extends CommonData { } -export interface CommonClientOptions extends RequestRetryOptions { +export interface UpstreamRefreshOptions { /** * Try to get fresh scrobble history from client when tracks to be scrobbled are newer than the last scrobble found in client history * @default true @@ -38,20 +38,35 @@ export interface CommonClientOptions extends RequestRetryOptions { * */ refreshEnabled?: boolean /** - * Force client to refresh scrobbled plays from upstream service if last refresh was at least X seconds ago + * Refresh scrobbled plays from upstream service if last refresh was at least X seconds ago * - * **In most case this setting should NOT be used.** MS intelligently refreshes based on activity so using this setting may increase upstream service load and slow down scrobbles. + * **In most case this setting does NOT need to be changed.** The default value is sufficient for the majority of use-cases. Increasing this setting may increase upstream service load and slow down scrobbles. * - * This setting should only be used in specific scenarios where MS is handling multiple "relaying" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds. + * This setting should only be changed in specific scenarios where MS is handling multiple "relaying" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds. * - * @examples [3] + * @examples [60] + * @default 60 * */ refreshStaleAfter?: number /** - * The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported for the client. + * Minimum time (milliseconds) required to pass before upstream scrobbles can be refreshed. + * + * **In most case this setting does NOT need to be changed.** This will always be equal to or smaller than `refreshStaleAfter`. + * + * @default 5000 + * @examples [5000] + * */ + refreshMinInterval?: number + + /** + * The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported by the client in 1 API call. * */ refreshInitialCount?: number +} + +export interface CommonClientOptions extends RequestRetryOptions, UpstreamRefreshOptions { + /** * Check client for an existing scrobble at the same recorded time as the "new" track to be scrobbled. If an existing scrobble is found this track is not track scrobbled. * @default true @@ -65,7 +80,6 @@ export interface CommonClientOptions extends RequestRetryOptions { match?: MatchLoggingOptions } - /** * Number of times MS should automatically retry scrobbles in dead letter queue * diff --git a/src/backend/common/schema/aio-client.json b/src/backend/common/schema/aio-client.json index 540f004d..10026c3e 100644 --- a/src/backend/common/schema/aio-client.json +++ b/src/backend/common/schema/aio-client.json @@ -62,14 +62,24 @@ "type": "boolean" }, "refreshInitialCount": { - "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported for the client.", + "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported by the client in 1 API call.", "title": "refreshInitialCount", "type": "number" }, + "refreshMinInterval": { + "default": 5000, + "description": "Minimum time (milliseconds) required to pass before upstream scrobbles can be refreshed.\n\n**In most case this setting does NOT need to be changed.** This will always be equal to or smaller than `refreshStaleAfter`.", + "examples": [ + 5000 + ], + "title": "refreshMinInterval", + "type": "number" + }, "refreshStaleAfter": { - "description": "Force client to refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting should NOT be used.** MS intelligently refreshes based on activity so using this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be used in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", + "default": 60, + "description": "Refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting does NOT need to be changed.** The default value is sufficient for the majority of use-cases. Increasing this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be changed in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", "examples": [ - 3 + 60 ], "title": "refreshStaleAfter", "type": "number" diff --git a/src/backend/common/schema/aio.json b/src/backend/common/schema/aio.json index 698db575..954fcfc7 100644 --- a/src/backend/common/schema/aio.json +++ b/src/backend/common/schema/aio.json @@ -351,14 +351,24 @@ "type": "boolean" }, "refreshInitialCount": { - "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported for the client.", + "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported by the client in 1 API call.", "title": "refreshInitialCount", "type": "number" }, + "refreshMinInterval": { + "default": 5000, + "description": "Minimum time (milliseconds) required to pass before upstream scrobbles can be refreshed.\n\n**In most case this setting does NOT need to be changed.** This will always be equal to or smaller than `refreshStaleAfter`.", + "examples": [ + 5000 + ], + "title": "refreshMinInterval", + "type": "number" + }, "refreshStaleAfter": { - "description": "Force client to refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting should NOT be used.** MS intelligently refreshes based on activity so using this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be used in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", + "default": 60, + "description": "Refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting does NOT need to be changed.** The default value is sufficient for the majority of use-cases. Increasing this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be changed in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", "examples": [ - 3 + 60 ], "title": "refreshStaleAfter", "type": "number" @@ -434,14 +444,24 @@ "type": "boolean" }, "refreshInitialCount": { - "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported for the client.", + "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported by the client in 1 API call.", "title": "refreshInitialCount", "type": "number" }, + "refreshMinInterval": { + "default": 5000, + "description": "Minimum time (milliseconds) required to pass before upstream scrobbles can be refreshed.\n\n**In most case this setting does NOT need to be changed.** This will always be equal to or smaller than `refreshStaleAfter`.", + "examples": [ + 5000 + ], + "title": "refreshMinInterval", + "type": "number" + }, "refreshStaleAfter": { - "description": "Force client to refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting should NOT be used.** MS intelligently refreshes based on activity so using this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be used in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", + "default": 60, + "description": "Refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting does NOT need to be changed.** The default value is sufficient for the majority of use-cases. Increasing this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be changed in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", "examples": [ - 3 + 60 ], "title": "refreshStaleAfter", "type": "number" diff --git a/src/backend/common/schema/client.json b/src/backend/common/schema/client.json index a7de9b7e..aa4bb8fd 100644 --- a/src/backend/common/schema/client.json +++ b/src/backend/common/schema/client.json @@ -59,14 +59,24 @@ "type": "boolean" }, "refreshInitialCount": { - "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported for the client.", + "description": "The number of tracks to retrieve on initial refresh (related to scrobbleBacklogCount). If not specified this is the maximum supported by the client in 1 API call.", "title": "refreshInitialCount", "type": "number" }, + "refreshMinInterval": { + "default": 5000, + "description": "Minimum time (milliseconds) required to pass before upstream scrobbles can be refreshed.\n\n**In most case this setting does NOT need to be changed.** This will always be equal to or smaller than `refreshStaleAfter`.", + "examples": [ + 5000 + ], + "title": "refreshMinInterval", + "type": "number" + }, "refreshStaleAfter": { - "description": "Force client to refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting should NOT be used.** MS intelligently refreshes based on activity so using this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be used in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", + "default": 60, + "description": "Refresh scrobbled plays from upstream service if last refresh was at least X seconds ago\n\n**In most case this setting does NOT need to be changed.** The default value is sufficient for the majority of use-cases. Increasing this setting may increase upstream service load and slow down scrobbles.\n\nThis setting should only be changed in specific scenarios where MS is handling multiple \"relaying\" client-services (IE lfm -> lz -> lfm) and there is the potential for a client to be out of sync after more than a few seconds.", "examples": [ - 3 + 60 ], "title": "refreshStaleAfter", "type": "number" diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 795cf415..7a31cef4 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -4,6 +4,7 @@ import EventEmitter from "events"; import { FixedSizeList } from 'fixed-size-list'; import { nanoid } from "nanoid"; import { Simulate } from "react-dom/test-utils"; +import { MarkOptional } from "ts-essentials"; import { DeadLetterScrobble, PlayObject, @@ -26,7 +27,7 @@ import { TIME_WEIGHT, TITLE_WEIGHT, TRANSFORM_HOOK, } from "../common/infrastructure/Atomic.js"; -import { CommonClientConfig } from "../common/infrastructure/config/client/index.js"; +import { CommonClientConfig, UpstreamRefreshOptions } from "../common/infrastructure/config/client/index.js"; import { Notifiers } from "../notifier/Notifiers.js"; import { comparingMultipleArtists, @@ -56,13 +57,14 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i #recentScrobblesList: PlayObject[] = []; scrobbledPlayObjs: FixedSizeList; + lastScrobbledPlayDate?: Dayjs; newestScrobbleTime?: Dayjs oldestScrobbleTime?: Dayjs tracksScrobbled: number = 0; lastScrobbleCheck: Dayjs = dayjs(0) lastScrobbleAttempt: Dayjs = dayjs(0) - refreshEnabled: boolean; + upstreamRefresh: MarkOptional, 'refreshInitialCount'>; checkExistingScrobbles: boolean; verboseOptions; @@ -93,11 +95,23 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i const { options: { refreshEnabled = true, + refreshInitialCount, + refreshMinInterval = 5, + refreshStaleAfter = 60, checkExistingScrobbles = true, verbose = {}, } = {}, } = this.config - this.refreshEnabled = refreshEnabled; + this.upstreamRefresh = { + refreshEnabled, + refreshInitialCount, + refreshMinInterval, + refreshStaleAfter + }; + if(refreshStaleAfter < (refreshMinInterval/1000)) { + this.logger.warn(`refreshMinInterval (${refreshMinInterval}ms) is longer than refreshStaleAfter (${refreshStaleAfter}s)! This would cause refreshStaleAfter to potentially not trigger a refresh. Setting refreshMinInterval to same interval as refreshStaleAfter`); + this.upstreamRefresh.refreshMinInterval = refreshStaleAfter * 1000; + } this.checkExistingScrobbles = checkExistingScrobbles; const { @@ -146,10 +160,11 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i this.logger.verbose(`Fetching up to ${initialLimit} initial scrobbles...`); await this.refreshScrobbles(initialLimit); + this.lastScrobbledPlayDate = this.newestScrobbleTime; } refreshScrobbles = async (limit: number = this.MAX_STORED_SCROBBLES) => { - if (this.refreshEnabled) { + if (this.upstreamRefresh.refreshEnabled) { this.logger.debug('Refreshing recent scrobbles'); const recent = await this.getScrobblesForRefresh(limit); this.logger.debug(`Found ${recent.length} recent scrobbles`); @@ -170,43 +185,74 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i shouldRefreshScrobble = () => { const { - refreshStaleAfter - } = this.config.options || {}; + refreshStaleAfter, + refreshMinInterval, + refreshEnabled + } = this.upstreamRefresh; - if (!this.refreshEnabled) { - this.logger.debug(`Should NOT refresh scrobbles => refreshEnabled is false`); + if (!refreshEnabled) { + this.logger.debug({labels: ['Upstream Refresh']}, `Should NOT refresh => refreshEnabled is false`); + return false; + } + + if(this.queuedScrobbles.length === 0) { + this.logger.debug({labels: ['Upstream Refresh']}, `Should NOT refresh => no scrobbles in queue!`); return false; } const queuedPlayedDate = this.getLatestQueuePlayDate(); - // if next queued play was played more recently than the last time we refreshed upstream scrobbles - if (this.lastScrobbleCheck.unix() < queuedPlayedDate.unix()) { - this.logger.debug('Should refresh scrobbles => queued scrobble playDate is newer than last upstream scrobble refresh'); - return true; + // if newest queued play was played more recently than the last time we refreshed upstream scrobbles + if (this.scrobblesLastCheckedAt().unix() < queuedPlayedDate.unix()) { + if(!this.scrobblesRefreshMinIntervalPassed()) { + this.logger.debug({labels: ['Upstream Refresh']}, `Should refresh but WILL NOT => queued scrobble playDate is newer than last refresh but refreshMinInterval (${refreshMinInterval}ms) has not passed since last check`); + return false; + } else { + this.logger.debug({labels: ['Upstream Refresh']}, 'Should refresh => newest queued scrobble playDate is newer than last refresh'); + return true; + } } - // if the last scrobbled play is at or is newer than the next scrobble then we are inserting (or potentially duping) - // in which case our data is probably stale - if(this.newestScrobbleTime !== undefined && this.newestScrobbleTime.unix() >= queuedPlayedDate.unix()) { - this.logger.debug('Should refresh scrobbles => queued scrobble playDate is equal to or older than the newest upstream scrobble'); - return true; + // if the play date of the last Play scrobbled is *newer* + // than the queued scrobble we are about to scrobble + // then we are inserting a scrobble out of order which can happen if + // * backlogging and upstream returned plays out of order + // * processing dead letter queue + // * two sources have different history + // + // in all cases we probably want to refresh + if(this.lastScrobbledPlayDate !== undefined && this.queuedScrobbles[0].play.meta.newFromSource && this.lastScrobbledPlayDate.unix() > this.queuedScrobbles[0].play.data.playDate.unix()) { + if(!this.scrobblesRefreshMinIntervalPassed()) { + this.logger.debug({labels: ['Upstream Refresh']}, `Should refresh but WILL NOT => queued scrobble playDate is older than last scrobbled play (out-of-order insert) but refreshMinInterval (${refreshMinInterval}ms) has not passed since last check`); + return false; + } else { + this.logger.debug({labels: ['Upstream Refresh']}, 'Should refresh => queued scrobble playDate is older than last scrobbled play (out-of-order insert)'); + return true; + } } + // if it's been X seconds since we last refreshed if(refreshStaleAfter !== undefined) { - const diff = dayjs().diff(this.lastScrobbleCheck, 's'); + const diff = dayjs().diff(this.scrobblesLastCheckedAt(), 's'); if(diff > refreshStaleAfter) { - this.logger.debug(`Should refresh scrobbles => last refresh (${diff}s ago) was longer than refreshStaleAfter (${refreshStaleAfter}s)`); + this.logger.debug({labels: ['Upstream Refresh']}, `Should refresh => last refresh (${diff}s ago) was longer than refreshStaleAfter (${refreshStaleAfter}s)`); return true; } } - this.logger.debug('Scrobble refresh not needed'); return false; } + protected scrobblesLastCheckedAt = () => this.lastScrobbleCheck + protected scrobblesLastCheckedAtDiff = () => dayjs().diff(this.scrobblesLastCheckedAt(), 'ms') + protected scrobblesRefreshMinIntervalPassed = () => { + const { + refreshMinInterval, + } = this.upstreamRefresh; + return this.scrobblesLastCheckedAtDiff() >= refreshMinInterval; + } + public abstract alreadyScrobbled(playObj: PlayObject, log?: boolean): Promise; - scrobblesLastCheckedAt = () => this.lastScrobbleCheck formatPlayObj = (obj: any, options: FormatPlayObjectOptions = {}) => { this.logger.warn('formatPlayObj should be defined by concrete class!'); @@ -237,6 +283,7 @@ export default abstract class AbstractScrobbleClient extends AbstractComponent i addScrobbledTrack = (playObj: PlayObject, scrobbledPlay: PlayObject) => { this.scrobbledPlayObjs.add({play: playObj, scrobble: scrobbledPlay}); + this.lastScrobbledPlayDate = playObj.data.playDate; this.tracksScrobbled++; } @@ -663,7 +710,7 @@ ${closestMatch.breakdowns.join('\n')}`, {leaf: ['Dupe Check']}); this.logger.warn('Cannot process dead letter scrobble because client is not ready.', {leaf: 'Dead Letter'}); return [false, deadScrobble]; } - if (this.getLatestQueuePlayDate() !== undefined && this.lastScrobbleCheck.unix() < this.getLatestQueuePlayDate().unix()) { + if (this.getLatestQueuePlayDate() !== undefined && this.scrobblesLastCheckedAt().unix() < this.getLatestQueuePlayDate().unix()) { await this.refreshScrobbles(); } const [timeFrameValid, timeFrameValidLog] = this.timeFrameIsValid(deadScrobble.play); diff --git a/src/backend/tests/plays/withDuration.json b/src/backend/tests/plays/withDuration.json index 46430477..38949e7d 100644 --- a/src/backend/tests/plays/withDuration.json +++ b/src/backend/tests/plays/withDuration.json @@ -123,8 +123,8 @@ "Disco with Mehdi El" ], "track": "Aquil", - "duration": 3072, - "listenedFor": 3000, + "duration": 230, + "listenedFor": 200, "playDate": "2023-09-20T17:55:05.000Z" }, "meta": { diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index 4880197a..b1cbb2ab 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -11,7 +11,7 @@ import { sleep } from "../../utils.js"; import mixedDuration from '../plays/mixedDuration.json'; import withDuration from '../plays/withDuration.json'; import { MockNetworkError, withRequestInterception } from "../utils/networking.js"; -import { asPlays, generatePlay, normalizePlays } from "../utils/PlayTestUtils.js"; +import { asPlays, generatePlay, generatePlays, normalizePlays } from "../utils/PlayTestUtils.js"; import { TestAuthScrobbler, TestScrobbler } from "./TestScrobbler.js"; @@ -190,15 +190,16 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu it('Is not detected as duplicate when play date is different by more than 60 seconds (low granularity source)', async function () { - testScrobbler.recentScrobbles = normalizePlays(mixedDurPlays, { + const recent = normalizePlays(mixedDurPlays, { initialDate: firstPlayDate, defaultMeta: {source: 'subsonic'} }); + testScrobbler.recentScrobbles = recent; - const timeOffPos = clone(normalizedWithMixedDur[normalizedWithMixedDur.length - 1]); + const timeOffPos = clone(recent[recent.length - 1]); timeOffPos.data.playDate = timeOffPos.data.playDate.add(61, 's'); - const timeOffNeg = clone(normalizedWithMixedDur[normalizedWithMixedDur.length - 1]); + const timeOffNeg = clone(recent[recent.length - 1]); timeOffNeg.data.playDate = timeOffNeg.data.playDate.subtract(61, 's'); assert.isFalse(await testScrobbler.alreadyScrobbled(timeOffPos)); @@ -311,15 +312,16 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu it('Is detected as duplicate when play date is off by less than 60 seconds (low granularity source)', async function () { - testScrobbler.recentScrobbles = normalizePlays(mixedDurPlays, { + const recent = normalizePlays(mixedDurPlays, { initialDate: firstPlayDate, defaultMeta: {source: 'subsonic'} }); + testScrobbler.recentScrobbles = recent; - const timeOffPos = clone(normalizedWithMixedDur[normalizedWithMixedDur.length - 1]); + const timeOffPos = clone(recent[recent.length - 1]); timeOffPos.data.playDate = timeOffPos.data.playDate.add(59, 's'); - const timeOffNeg = clone(normalizedWithMixedDur[normalizedWithMixedDur.length - 1]); + const timeOffNeg = clone(recent[recent.length - 1]); timeOffNeg.data.playDate = timeOffNeg.data.playDate.subtract(59, 's'); assert.isTrue(await testScrobbler.alreadyScrobbled(timeOffPos)); @@ -470,13 +472,13 @@ describe('Upstream Scrobbles', function() { describe('Detects when upstream scrobbles should be refreshed', function() { - const normalizedClose = normalizePlays(withDurPlays, {initialDate: dayjs().subtract(100, 'seconds')}); + const normalizedClose = normalizePlays(generatePlays(10), {endDate: dayjs().subtract(100, 'seconds')}); - beforeEach(function () { + beforeEach(async function () { testScrobbler = generateTestScrobbler(); - testScrobbler.recentScrobbles = normalizedWithMixedDur; - testScrobbler.newestScrobbleTime = normalizedWithMixedDur[0].data.playDate; - testScrobbler.lastScrobbleCheck = dayjs().subtract(60, 'seconds'); + testScrobbler.testRecentScrobbles = normalizedClose; + await testScrobbler.initialize(); + testScrobbler.lastScrobbleCheck = dayjs().subtract(65, 'seconds'); testScrobbler.queuedScrobbles = []; testScrobbler.config.options = {}; }); @@ -491,9 +493,6 @@ describe('Upstream Scrobbles', function() { }); it('Detects queued scrobble date is older than newest scrobble', async function() { - testScrobbler.recentScrobbles = normalizedClose; - testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate; - const newScrobble = generatePlay({ playDate: dayjs().subtract(120, 'seconds') }); @@ -503,8 +502,6 @@ describe('Upstream Scrobbles', function() { }); it('Forces refresh if refreshStaleAfter is set', async function() { - testScrobbler.recentScrobbles = normalizedClose; - testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate; testScrobbler.config.options = { refreshStaleAfter: 10 }; const newScrobble = generatePlay({ @@ -516,9 +513,7 @@ describe('Upstream Scrobbles', function() { }); it('Does not refresh if scrobble is older than last check but newer than newest upstream scrobble', async function() { - testScrobbler.recentScrobbles = normalizedClose; - testScrobbler.newestScrobbleTime = normalizedClose[0].data.playDate; - + testScrobbler.lastScrobbleCheck = dayjs().subtract(40, 'seconds'); const newScrobble = generatePlay({ playDate: dayjs().subtract(80, 'seconds') }); diff --git a/src/backend/tests/utils/PlayTestUtils.ts b/src/backend/tests/utils/PlayTestUtils.ts index cad4545a..8b971b0c 100644 --- a/src/backend/tests/utils/PlayTestUtils.ts +++ b/src/backend/tests/utils/PlayTestUtils.ts @@ -6,6 +6,7 @@ import relativeTime from "dayjs/plugin/relativeTime.js"; import timezone from "dayjs/plugin/timezone.js"; import utc from "dayjs/plugin/utc.js"; import { JsonPlayObject, ObjectPlayData, PlayMeta, PlayObject } from "../../../core/Atomic.js"; +import { sortByNewestPlayDate } from "../../utils.js"; dayjs.extend(utc) dayjs.extend(isBetween); @@ -28,7 +29,9 @@ export const asPlays = (data: object[]): PlayObject[] => { export const normalizePlays = (plays: PlayObject[], options?: { + //sortFunc?: (a: PlayObject, b: PlayObject) => 0 | 1 | -1 initialDate?: Dayjs, + endDate?: Dayjs defaultDuration?: number, defaultData?: ObjectPlayData, defaultMeta?: PlayMeta @@ -36,42 +39,96 @@ export const normalizePlays = (plays: PlayObject[], ): PlayObject[] => { const { initialDate, + endDate, defaultDuration = 180, defaultData = {}, - defaultMeta = {} + defaultMeta = {}, + //sortFunc = sortByNewestPlayDate } = options || {}; - let date = initialDate; - if (date === undefined) { - const firstDatedPlay = plays.find(x => x.data.playDate !== undefined); - if (firstDatedPlay !== undefined) { - date = firstDatedPlay.data.playDate; - } else { - throw new Error('No initial date specified and no play had a defined date'); - } - } - let lastTrackEndsAt: Dayjs | undefined = undefined; + const normalizedPlays: PlayObject[] = []; - for (const play of plays) { + let actualInitialDate: Dayjs | undefined = initialDate; + + if(endDate !== undefined && actualInitialDate !== undefined) { + + // need to redefine durations and listenedFor, if present + const dur = dayjs.duration(endDate.diff(actualInitialDate)); + let remaining = plays.length; + + let index = 0; + let lastDate = endDate; + let remainingTime = dur.asSeconds() + for(const play of plays) { - const cleanPlay = {...play}; + const cleanPlay = {...play}; - if (lastTrackEndsAt === undefined) { - // first track - cleanPlay.data.playDate = date; - } else { - cleanPlay.data.playDate = lastTrackEndsAt.add(1, 'second'); + const remainingAvgTime = remainingTime/remaining; + cleanPlay.data.playDate = lastDate; + cleanPlay.data.duration = faker.number.int({min: 30, max: remainingAvgTime}); + if(cleanPlay.data.listenedFor !== undefined) { + cleanPlay.data.listenedFor = faker.number.int({min: Math.floor(cleanPlay.data.duration * 0.9), max: cleanPlay.data.duration}); + } + cleanPlay.data = { + ...cleanPlay.data, + ...defaultData + } + cleanPlay.meta = { + ...cleanPlay.meta, + ...defaultMeta + } + + if(index + 1 <= plays.length - 1) { + const listenTime = (plays[index+1].data.duration ?? defaultDuration) + faker.number.int({min: 0, max: 2}); + lastDate = cleanPlay.data.playDate.subtract(listenTime, 'seconds'); + } + remaining--; + remainingTime -= cleanPlay.data.duration; + index++; } - cleanPlay.data = { - ...cleanPlay.data, - ...defaultData + + } else { + const progressDirection: 'newer' | 'older' = endDate !== undefined ? 'older' : 'newer'; + if(progressDirection === 'newer' && actualInitialDate === undefined) { + const firstDatedPlay = plays.find(x => x.data.playDate !== undefined); + if (firstDatedPlay !== undefined) { + actualInitialDate = firstDatedPlay.data.playDate; + } else { + throw new Error('No initial date specified and no play had a defined date'); + } } - cleanPlay.meta = { - ...cleanPlay.meta, - ...defaultMeta + + let lastDate: Dayjs = progressDirection === 'newer' ? actualInitialDate : endDate; + let index = 0; + for (const play of plays) { + + const cleanPlay = {...play}; + + cleanPlay.data.playDate = lastDate; + cleanPlay.data = { + ...cleanPlay.data, + ...defaultData + } + cleanPlay.meta = { + ...cleanPlay.meta, + ...defaultMeta + } + + if(progressDirection === 'newer') { + const listenTime = (cleanPlay.data.duration ?? defaultDuration) + faker.number.int({min: 0, max: 2}); + lastDate = cleanPlay.data.playDate.add(listenTime, 'seconds'); + } else if(index + 1 <= plays.length - 1) { + const listenTime = (plays[index+1].data.duration ?? defaultDuration) + faker.number.int({min: 0, max: 2}); + lastDate = cleanPlay.data.playDate.subtract(listenTime, 'seconds'); + } + + normalizedPlays.push(cleanPlay); + index++; + } + + if(progressDirection === 'older') { + normalizedPlays.sort(sortByNewestPlayDate); } - lastTrackEndsAt = cleanPlay.data.playDate.add(cleanPlay.data.duration ?? defaultDuration, 'seconds'); - normalizedPlays.push(cleanPlay); } return normalizedPlays; -- 2.51.2