diff --git a/package-lock.json b/package-lock.json index 950bafa2..b795f0e5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "multi-scrobbler", - "version": "0.6.1", + "version": "0.6.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "multi-scrobbler", - "version": "0.6.1", + "version": "0.6.2", "license": "MIT", "dependencies": { "@awaitjs/express": "^0.6.3", -- 2.51.2 From c77e4355fcfca23d827d9399ef456669db4d6a56 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 30 Nov 2023 11:14:52 -0500 Subject: [PATCH 02/33] fix(player): Fix listen range default value --- src/backend/sources/PlayerState/AbstractPlayerState.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/sources/PlayerState/AbstractPlayerState.ts b/src/backend/sources/PlayerState/AbstractPlayerState.ts index c835b501..a81c786b 100644 --- a/src/backend/sources/PlayerState/AbstractPlayerState.ts +++ b/src/backend/sources/PlayerState/AbstractPlayerState.ts @@ -31,7 +31,7 @@ export abstract class AbstractPlayerState { playFirstSeenAt?: Dayjs playLastUpdatedAt?: Dayjs currentListenRange?: ListenRange - listenRanges: ListenRange[] + listenRanges: ListenRange[] = []; createdAt: Dayjs = dayjs(); stateLastUpdatedAt: Dayjs = dayjs(); -- 2.51.2 From 6b17babd7ad6820aff2b8cf8b81a44673b141267 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 30 Nov 2023 11:15:49 -0500 Subject: [PATCH 03/33] fix(mopidy): Fix album artist parsing #110 Should be using mapped artist string name rather than model but cannot test due to mopidy being difficult. Hopefully this works? --- src/backend/sources/MopidySource.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/sources/MopidySource.ts b/src/backend/sources/MopidySource.ts index ae578499..9148bd73 100644 --- a/src/backend/sources/MopidySource.ts +++ b/src/backend/sources/MopidySource.ts @@ -133,7 +133,7 @@ export class MopidySource extends MemorySource { performers = [] } = obj; - let artists = artistsVal === null ? [] : artistsVal; + let artists: models.Artist[] = artistsVal === null ? [] : artistsVal; let album: models.Album = albumVal === null ? {} as models.Album : albumVal; if (this.albumBlacklist.length > 0 && album.name !== undefined && this.albumBlacklist.some(x => album.name.toLocaleLowerCase().includes(x))) { album = {} as models.Album; @@ -145,7 +145,7 @@ export class MopidySource extends MemorySource { artists: albumArtists = [] } = album as models.Album; - let actualAlbumArtists = []; + let actualAlbumArtists: models.Artist[] = []; if ((artists.length === 0 || artists.every(x => x.name.toLocaleLowerCase().includes('various'))) && albumArtists.length > 0) { artists = albumArtists; } else { @@ -162,7 +162,7 @@ export class MopidySource extends MemorySource { data: { track: name, album: albumName, - albumArtists: actualAlbumArtists, + albumArtists: actualAlbumArtists.length > 0 ? actualAlbumArtists.map(x => x.name) : [], artists: artists.length > 0 ? artists.map(x => x.name) : [], duration: Math.round(length / 1000), playDate: dayjs() -- 2.51.2 From ed6f7cfe5ffbaee5c12b2e4c96e63f59ba87728a Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 4 Dec 2023 10:47:27 -0500 Subject: [PATCH 04/33] docs(webscrobbler): Add where to find connector list #112 --- docsite/docs/configuration/configuration.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docsite/docs/configuration/configuration.md b/docsite/docs/configuration/configuration.md index 72bc7eb0..9d8ff34b 100644 --- a/docsite/docs/configuration/configuration.md +++ b/docsite/docs/configuration/configuration.md @@ -678,6 +678,10 @@ In `webscrobbler.json` Note: `http://localhost:9078/api/webscrobbler` is matched with the first source that _that does not have a slug defined._ +##### Connectors Black/Whitelist + +MS can be configured to only scrobble, or NOT scrobble, from some WS connectors. Use the name of the website from the [supported websites](https://web-scrobbler.com/) or from the **Connectors** tab in the extension. Note that this **only** affects MS's behavior and does not affect the general connector behavior you have configured within the WebScrobbler extension. + ### ENV-Based | Environmental Variable | Required? | Default | Description | -- 2.51.2 From 6e2e6f4492c3183fb6d9aa2751b4b1f337dc3e45 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 4 Dec 2023 11:02:19 -0500 Subject: [PATCH 05/33] fix(webscrobbler): Correct ENV names to match those documented #111 --- src/backend/sources/ScrobbleSources.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/sources/ScrobbleSources.ts b/src/backend/sources/ScrobbleSources.ts index 87a9b1a5..7cfbc65b 100644 --- a/src/backend/sources/ScrobbleSources.ts +++ b/src/backend/sources/ScrobbleSources.ts @@ -304,10 +304,10 @@ export default class ScrobbleSources { } break; case 'webscrobbler': - const wsShouldUse = parseBool(process.env.WEBSCROBBLER_ENABLE); + const wsShouldUse = parseBool(process.env.WS_ENABLE); const ws = { - blacklist: process.env.WEBSCROBBLER_BLACKLIST, - whitelist: process.env.WEBSCROBBLER_WHITELIST + blacklist: process.env.WS_BLACKLIST, + whitelist: process.env.WS_WHITELIST } if (!Object.values(ws).every(x => x === undefined) || wsShouldUse) { configs.push({ -- 2.51.2 From 5fd0cc3fe466fedbc0abfa55b7c091d788cfa6a2 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 4 Dec 2023 11:06:02 -0500 Subject: [PATCH 06/33] docs(webscrobbler): Move slug into data #111 --- config/webscrobbler.json.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/webscrobbler.json.example b/config/webscrobbler.json.example index 1d27837b..2eee5482 100644 --- a/config/webscrobbler.json.example +++ b/config/webscrobbler.json.example @@ -1,8 +1,8 @@ [ { "name": "MyWebScrobbler", - "slug": null, "data": { + "slug": null, "whitelist": [], "blacklist": [] } -- 2.51.2 From b7d6e9e4974fa76d94fc9644a820ae8c4b4b904f Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 4 Dec 2023 11:06:14 -0500 Subject: [PATCH 07/33] docs: Update schema for logging enums --- src/backend/common/schema/aio.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/backend/common/schema/aio.json b/src/backend/common/schema/aio.json index 04dba18f..4ceaad69 100644 --- a/src/backend/common/schema/aio.json +++ b/src/backend/common/schema/aio.json @@ -1335,8 +1335,15 @@ "LogOptions": { "properties": { "console": { - "$ref": "#/definitions/LogLevel", "description": "Specify the minimum log level streamed to the console (or docker container)", + "enum": [ + "debug", + "error", + false, + "info", + "verbose", + "warn" + ], "title": "console" }, "file": { -- 2.51.2 From 5811912745aaa3930e39ad65c9ce7bcc30855dc9 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Mon, 4 Dec 2023 11:13:20 -0500 Subject: [PATCH 08/33] feat(webscrobbler): Log the best-guess URL for requests using configured slug --- src/backend/sources/WebScrobblerSource.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/backend/sources/WebScrobblerSource.ts b/src/backend/sources/WebScrobblerSource.ts index 1cc68c82..d0233083 100644 --- a/src/backend/sources/WebScrobblerSource.ts +++ b/src/backend/sources/WebScrobblerSource.ts @@ -39,6 +39,12 @@ export class WebScrobblerSource extends MemorySource { }; } + initialize = async () => { + this.logger.info(`Accepting requests at ${this.localUrl}/api/webscrobbler${this.config.data.slug === undefined ? '' : `/${this.config.data.slug}`}`); + this.initialized = true; + return this.initialized; + } + matchSlug(slug: string | undefined) { if (this.config.data.slug === undefined || this.config.data.slug === null) { return slug === undefined; -- 2.51.2 From 5b2449adb0b1196c1448c85d699a1754d328250e Mon Sep 17 00:00:00 2001 From: Bruno Henrique Paiva Date: Tue, 12 Dec 2023 11:37:47 -0300 Subject: [PATCH 09/33] fix(spotify): undefined player state data when in private session --- src/backend/sources/SpotifySource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/sources/SpotifySource.ts b/src/backend/sources/SpotifySource.ts index 69507735..4acb2ed3 100644 --- a/src/backend/sources/SpotifySource.ts +++ b/src/backend/sources/SpotifySource.ts @@ -295,7 +295,7 @@ export default class SpotifySource extends MemorySource { const state = await this.getCurrentPlaybackState(); if(state.playerState !== undefined) { if(state.device.is_private_session) { - this.logger.debug(`Will not track play on Device ${state.device.name} because it is a private session: ${buildTrackString(state.playerState.play)}`); + this.logger.debug(`Will not track play on Device ${state.device.name} because it is in a private session.`); } else { plays.push(state.playerState); } -- 2.51.2 From a993df4f0b77b18d21441624f66fca17619c569d Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 12 Dec 2023 10:01:58 -0500 Subject: [PATCH 10/33] docs: Add contributing guidelines and PR template --- .github/pull_request_template.md | 20 ++++++++++++++++++++ CONTRIBUTING.md | 8 ++++++++ 2 files changed, 28 insertions(+) create mode 100644 .github/pull_request_template.md create mode 100644 CONTRIBUTING.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..f3aa6763 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,20 @@ +## Checklist before requesting a review + +- [ ] **I am opening this PR for the [`develop` branch](https://github.com/FoxxMD/multi-scrobbler/tree/develop) and NOT `master`.** +- [ ] I have read the [contributing guidelines.](../CONTRIBUTING.md) + +## Type of change + +Please delete options that are not relevant. + +- [ ] Bug fix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) +- [ ] This change requires a documentation update + +## Describe your changes + + + +## Issue number and link, if applicable + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..6c06b054 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,8 @@ +# Creating a Pull Request + +Please follow these guidelines when contributing code to this repository: + +* The PR **must be for the [`develop` branch.](https://github.com/FoxxMD/multi-scrobbler/tree/develop)** The `master` branch is for releases only. +* Use [conventional commit](https://www.conventionalcommits.org/en/v1.0.0/#summary) format when creating commits. +* Preferably, please use a [feature branch](https://stackoverflow.com/a/39586780/1469797) instead of committing directly to `develop`. +* Ensure that if your code is covered by [an existing test](./src/backend/tests) that you have updated the test accordingly -- 2.51.2 From aa59bc46078ac6e1250889e65a098ce6bf70660f Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 20 Dec 2023 13:31:40 -0500 Subject: [PATCH 11/33] Fix service documentation link #119 --- src/backend/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/index.ts b/src/backend/index.ts index 1f7d2715..353407ed 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -80,7 +80,7 @@ const configDir = process.env.CONFIG_DIR || path.resolve(projectDir, `./config`) initServer(logger, output); if(process.env.IS_LOCAL === 'true') { - logger.info('multi-scrobbler can be run as a background service! See: https://github.com/FoxxMD/multi-scrobbler/blob/develop/docs/service.md'); + logger.info('multi-scrobbler can be run as a background service! See: https://foxxmd.github.io/multi-scrobbler/docs/installation/service'); } if(appConfigFail !== undefined) { -- 2.51.2 From c37d94e71b0189d5d50e0e9a6ab86b13c0abd454 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 27 Dec 2023 11:02:15 -0500 Subject: [PATCH 12/33] refactor(scrobblers): Consolidate scrobbler client startup surface area * Move logging for init/auth into client instead of builder class * Move startup from builder class to main backend and use initScrobbleMonitoring() instead of calling init/auth individually Paves the way for manual restarting #114 --- src/backend/index.ts | 10 ++++---- .../scrobblers/AbstractScrobbleClient.ts | 9 ++++--- src/backend/scrobblers/LastfmScrobbler.ts | 9 ++++++- .../scrobblers/ListenbrainzScrobbler.ts | 14 ++++------- src/backend/scrobblers/MalojaScrobbler.ts | 12 +++++++--- src/backend/scrobblers/ScrobbleClients.ts | 24 +------------------ 6 files changed, 33 insertions(+), 45 deletions(-) diff --git a/src/backend/index.ts b/src/backend/index.ts index 1f7d2715..3b545c05 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -98,6 +98,11 @@ const configDir = process.env.CONFIG_DIR || path.resolve(projectDir, `./config`) await scrobbleClients.buildClientsFromConfig(notifiers); if (scrobbleClients.clients.length === 0) { logger.warn('No scrobble clients were configured!') + } else { + logger.info('Starting scrobble clients...'); + } + for(const client of scrobbleClients.clients) { + await client.initScrobbleMonitoring(); } const scrobbleSources = root.get('sources');//new ScrobbleSources(localUrl, configDir); @@ -137,11 +142,6 @@ const configDir = process.env.CONFIG_DIR || path.resolve(projectDir, `./config`) } } } - for(const client of scrobbleClients.clients) { - if((await client.isReady())) { - client.initScrobbleMonitoring(); - } - } if (anyNotReady) { logger.info(`Some sources are not ready, open the dashboard to continue`); } diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 9a5af695..834bc409 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -147,6 +147,7 @@ export default abstract class AbstractScrobbleClient implements Authenticatable // default init function, should be overridden if init stage is required initialize = async () => { this.initialized = true; + this.logger.info('Initialized'); return true; } @@ -171,8 +172,7 @@ export default abstract class AbstractScrobbleClient implements Authenticatable // only signal as auth failure if error was NOT either a node network error or a non-showstopping upstream error this.authFailure = !(hasNodeNetworkException(e) || hasUpstreamError(e, false)); this.authed = false; - this.logger.error(`Authentication test failed!${this.authFailure === false ? ' Due to a network issue. Will retry authentication on next heartbeat.' : ''}`); - this.logger.error(e); + this.logger.error(new ErrorWithCause(`Authentication test failed!${this.authFailure === false ? ' Due to a network issue. Will retry authentication on next heartbeat.' : ''}`, {cause: e})); } } @@ -464,7 +464,10 @@ ${closestMatch.breakdowns.join('\n')}`); return; } - await this.startScrobbling(); + this.startScrobbling().catch((e) => { + throw e; + }); + return; } startScrobbling = async () => { diff --git a/src/backend/scrobblers/LastfmScrobbler.ts b/src/backend/scrobblers/LastfmScrobbler.ts index 010c680c..dc31ad0c 100644 --- a/src/backend/scrobblers/LastfmScrobbler.ts +++ b/src/backend/scrobblers/LastfmScrobbler.ts @@ -39,7 +39,14 @@ export default class LastfmScrobbler extends AbstractScrobbleClient { initialize = async () => { // @ts-expect-error TS(2322): Type 'number' is not assignable to type 'boolean'. this.initialized = INITIALIZING; - this.initialized = await this.api.initialize(); + const result = await this.api.initialize(); + this.initialized = result; + if(result) { + this.logger.info('Initialized'); + } else { + this.logger.warn('Could not initialize'); + } + return this.initialized; } diff --git a/src/backend/scrobblers/ListenbrainzScrobbler.ts b/src/backend/scrobblers/ListenbrainzScrobbler.ts index 7b5fd52d..7f55dc07 100644 --- a/src/backend/scrobblers/ListenbrainzScrobbler.ts +++ b/src/backend/scrobblers/ListenbrainzScrobbler.ts @@ -1,14 +1,6 @@ import dayjs from 'dayjs'; import AbstractScrobbleClient from "./AbstractScrobbleClient"; -import { - playObjDataMatch, - removeUndefinedKeys, - setIntersection, - sleep, - sortByOldestPlayDate, -} from "../utils"; -import LastfmApiClient from "../common/vendor/LastfmApiClient"; import { FormatPlayObjectOptions, INITIALIZING } from "../common/infrastructure/Atomic"; import { Notifiers } from "../notifier/Notifiers"; import {Logger} from '@foxxmd/winston'; @@ -19,6 +11,7 @@ import {buildTrackString, capitalize} from "../../core/StringUtils"; import EventEmitter from "events"; import {UpstreamError} from "../common/errors/UpstreamError"; import {isNodeNetworkException} from "../common/errors/NodeErrors"; +import {ErrorWithCause} from "pony-cause"; export default class ListenbrainzScrobbler extends AbstractScrobbleClient { @@ -39,14 +32,15 @@ export default class ListenbrainzScrobbler extends AbstractScrobbleClient { // @ts-expect-error TS(2322): Type 'number' is not assignable to type 'boolean'. this.initialized = INITIALIZING; if(this.config.data.token === undefined) { - this.logger.error('Must provide a User Token'); + this.logger.error('Could not initialize, must provide a User Token'); this.initialized = false; } else { try { await this.api.testConnection(); this.initialized = true; + this.logger.info('Initialized'); } catch (e) { - this.logger.error(e); + this.logger.warn(new ErrorWithCause('Could not initialize', {cause: e})); this.initialized = false; } } diff --git a/src/backend/scrobblers/MalojaScrobbler.ts b/src/backend/scrobblers/MalojaScrobbler.ts index 85589ddb..b1b6f299 100644 --- a/src/backend/scrobblers/MalojaScrobbler.ts +++ b/src/backend/scrobblers/MalojaScrobbler.ts @@ -26,6 +26,7 @@ import EventEmitter from "events"; import normalizeUrl from "normalize-url"; import {UpstreamError} from "../common/errors/UpstreamError"; import {ar} from "@faker-js/faker"; +import {ErrorWithCause} from "pony-cause"; const feat = ["ft.", "ft", "feat.", "feat", "featuring", "Ft.", "Ft", "Feat.", "Feat", "Featuring"]; @@ -199,8 +200,7 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { } return true; } catch (e) { - this.logger.error('Communication test failed'); - this.logger.error(e); + this.logger.error(new ErrorWithCause('Communication test failed', {cause: e})); return false; } } @@ -246,7 +246,13 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { // just checking that we can get a connection // @ts-expect-error TS(2322): Type 'number' is not assignable to type 'boolean'. this.initialized = INITIALIZING; - this.initialized = await this.testConnection(); + const result = await this.testConnection(); + this.initialized = result; + if(result) { + this.logger.info('Initialized'); + } else { + this.logger.warn('Could not initialize'); + } return this.initialized; } diff --git a/src/backend/scrobblers/ScrobbleClients.ts b/src/backend/scrobblers/ScrobbleClients.ts index 34981601..d3495034 100644 --- a/src/backend/scrobblers/ScrobbleClients.ts +++ b/src/backend/scrobblers/ScrobbleClients.ts @@ -325,29 +325,7 @@ ${sources.join('\n')}`); // really shouldn't get here! throw new Error(`Client of type ${type} was not recognized??`); } - if(newClient.initialized === false) { - this.logger.debug(`Attempting ${type} (${name}) initialization...`); - if ((await newClient.initialize()) === false) { - this.logger.error(`${type} (${name}) client failed to initialize. Client needs to be successfully initialized before scrobbling.`); - } else { - this.logger.info(`${type} (${name}) client initialized`); - } - } - if(newClient.authGated()) { - this.logger.debug(`Checking ${type} (${name}) client auth...`); - let success; - try { - await newClient.testAuth(); - success = newClient.authed; - } catch (e) { - success = false; - } - if(!success) { - this.logger.warn(`${type} (${name}) client auth failed.`); - } else { - this.logger.info(`${type} (${name}) client auth OK`); - } - } + newClient.logger.info('Client Added'); this.clients.push(newClient); } -- 2.51.2 From fcc05a8b1957a9aafc8ad23fe99573725ccd482c Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 27 Dec 2023 11:50:57 -0500 Subject: [PATCH 13/33] feat(scrobblers): Implement user-initiated (re)start mechanism #114 * Add api endpoint for restarting client * Implement (re)start link on client card --- .../scrobblers/AbstractScrobbleClient.ts | 2 +- src/backend/server/api.ts | 15 +++++++++++++ .../statusCard/ClientStatusCard.tsx | 22 +++++++++++++++---- .../statusCard/StatusCardSkeleton.tsx | 11 +++++++--- .../components/statusCard/clientDucks.ts | 21 ++++++++++++++++++ src/client/deadLetter/DeadPage.tsx | 1 - src/client/store.ts | 4 +++- 7 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 src/client/components/statusCard/clientDucks.ts diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 834bc409..cf371abf 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -465,7 +465,7 @@ ${closestMatch.breakdowns.join('\n')}`); } this.startScrobbling().catch((e) => { - throw e; + // do nothing, should have already been caught and logged }); return; } diff --git a/src/backend/server/api.ts b/src/backend/server/api.ts index 5aa9deba..4d8be370 100644 --- a/src/backend/server/api.ts +++ b/src/backend/server/api.ts @@ -377,6 +377,21 @@ export const setupApi = (app: ExpressWithAsync, logger: Logger, initialLogOutput } }); + app.use('/api/client/init', clientRequiredMiddle); + app.postAsync('/api/client/init', async function (req, res) { + // @ts-expect-error TS(2339): Property 'scrobbleSource' does not exist on type '... Remove this comment to see the full error message + const client = req.scrobbleClient as AbstractScrobbleClient; + + logger.info('Checking (and trying) to stop scrobbler if already running...'); + if(false === (await client.tryStopScrobbling())) { + return res.status(500).send(); + } + + logger.info('Trying to start scrobbler...'); + await client.initScrobbleMonitoring(); + res.status(200).send('OK'); + }); + app.getAsync('/health', async function(req, res) { return res.redirect(307, `/api/${req.url.slice(1)}`); }); diff --git a/src/client/components/statusCard/ClientStatusCard.tsx b/src/client/components/statusCard/ClientStatusCard.tsx index 6e0bacd3..8d48f937 100644 --- a/src/client/components/statusCard/ClientStatusCard.tsx +++ b/src/client/components/statusCard/ClientStatusCard.tsx @@ -1,10 +1,11 @@ -import React, {Fragment} from 'react'; +import React, {Fragment, useCallback} from 'react'; import StatusCardSkeleton, {StatusCardSkeletonData} from "./StatusCardSkeleton"; import SkeletonParagraph from "../skeleton/SkeletonParagraph"; import {clientAdapter} from "../../status/ducks"; import {RootState} from "../../store"; import {connect, ConnectedProps} from "react-redux"; import {Link} from "react-router-dom"; +import {useStartClientMutation} from "./clientDucks"; export interface ClientStatusCardData extends StatusCardSkeletonData, PropsFromRedux { loading?: boolean @@ -36,11 +37,18 @@ const ClientStatusCard = (props: ClientStatusCardData) => { deadLetterScrobbles = 0 } = {} } = props; + + const [startClientPut, startResult] = useStartClientMutation(); + + const tryStart = useCallback((name: string) => startClientPut({name}), [startClientPut]); + let header: string | undefined = display; let body = ; + const startClientElement =
tryStart(name)} className="capitalize underline cursor-pointer">{status === 'Running' ? 'Restart' : 'Start'}
if(data !== undefined) { const { hasAuth, + hasAuthInteraction, name, type, authed, @@ -49,18 +57,24 @@ const ClientStatusCard = (props: ClientStatusCardData) => { if(type === 'lastfm' || type === 'listenbrainz') header = `${display} (Client)`; - const scrobbled = initialized && (!hasAuth || (hasAuth && authed)) ? Tracks Scrobbled : Tracks Scrobbled + const scrobbled = initialized && (!hasAuth || (hasAuth && authed)) ? Tracks Scrobbled : Tracks Scrobbled; // TODO links body = (
{scrobbled}: {scrobbledCount}
Queued Scrobbles: {queued}
Failed Scrobbles: {deadLetterScrobbles}
- {hasAuth ? (Re)authenticate or initialize : null} + {hasAuthInteraction ? (Re)authenticate : null}
); } return ( - + {body} ); diff --git a/src/client/components/statusCard/StatusCardSkeleton.tsx b/src/client/components/statusCard/StatusCardSkeleton.tsx index 0e95b259..6567d485 100644 --- a/src/client/components/statusCard/StatusCardSkeleton.tsx +++ b/src/client/components/statusCard/StatusCardSkeleton.tsx @@ -1,4 +1,4 @@ -import React, {PropsWithChildren} from 'react'; +import React, {PropsWithChildren, ReactElement} from 'react'; import SkeletonTitle from "../skeleton/SkeletonTitle"; import SkeletonParagraph from "../skeleton/SkeletonParagraph"; import StatusIndicator, {StatusType} from "../StatusIndicator"; @@ -8,6 +8,7 @@ export interface StatusCardSkeletonData { header?: string title?: string subtitle?: string + subtitleRight?: string | ReactElement status?: string statusType?: StatusType } @@ -20,6 +21,7 @@ const StatusCardSkeleton = (props: PropsWithChildren) => statusType, title, subtitle, + subtitleRight, children } = props || {}; @@ -28,13 +30,16 @@ const StatusCardSkeleton = (props: PropsWithChildren) =>
-
+
{header ?
{header}
: null} {title ?
{title}
: null} {subtitle ?
{subtitle}
: null}
-
{status}
+
+
{status}
+ {subtitleRight ?
{subtitleRight}
: null} +
diff --git a/src/client/components/statusCard/clientDucks.ts b/src/client/components/statusCard/clientDucks.ts new file mode 100644 index 00000000..ea90e2a5 --- /dev/null +++ b/src/client/components/statusCard/clientDucks.ts @@ -0,0 +1,21 @@ +import {createApi, fetchBaseQuery} from "@reduxjs/toolkit/dist/query/react"; + +export const scrobblerApi = createApi({ + reducerPath: 'scrobblerApi', + baseQuery: fetchBaseQuery({baseUrl: '/api/'}), + endpoints: (builder) => ({ + startClient: builder.mutation({ + query: (params) => ({ + url: '/client/init', + method: 'POST', + params: { + name: params.name + } + }) + }) + }) +}); + +export const {useStartClientMutation} = scrobblerApi; diff --git a/src/client/deadLetter/DeadPage.tsx b/src/client/deadLetter/DeadPage.tsx index 9e06753e..344d783a 100644 --- a/src/client/deadLetter/DeadPage.tsx +++ b/src/client/deadLetter/DeadPage.tsx @@ -14,7 +14,6 @@ import { import dayjs from "dayjs"; import {RootState} from "../store"; import {connect, ConnectedProps} from "react-redux"; -import {id} from "common-tags"; const displayOpts = { include: recentIncludes, diff --git a/src/client/store.ts b/src/client/store.ts index 2c5fd02d..f67e2166 100644 --- a/src/client/store.ts +++ b/src/client/store.ts @@ -8,6 +8,7 @@ import {logsApi} from "./logs/logsApi"; import {recentApi} from "./recent/recentDucks"; import {scrobbledApi} from "./scrobbled/scrobbledDucks"; import {deadApi, deadSlice} from "./deadLetter/deadLetterDucks"; +import {scrobblerApi} from './components/statusCard/clientDucks'; export const store = configureStore({ reducer: { @@ -17,6 +18,7 @@ export const store = configureStore({ [recentApi.reducerPath]: recentApi.reducer, [deadApi.reducerPath]: deadApi.reducer, [scrobbledApi.reducerPath]: scrobbledApi.reducer, + [scrobblerApi.reducerPath]: scrobblerApi.reducer, //parts: statusReducer clients: clientSlice.reducer, sources: sourceSlice.reducer, @@ -26,7 +28,7 @@ export const store = configureStore({ // Adding the api middleware enables caching, invalidation, polling, // and other useful features of `rtk-query`. middleware: (getDefaultMiddleware) => - getDefaultMiddleware().concat([statusApi.middleware, logsApi.middleware, recentApi.middleware, scrobbledApi.middleware, deadApi.middleware]), + getDefaultMiddleware().concat([statusApi.middleware, logsApi.middleware, recentApi.middleware, scrobbledApi.middleware, deadApi.middleware, scrobblerApi.middleware]), }) // optional, but required for refetchOnFocus/refetchOnReconnect behaviors -- 2.51.2 From 77c3a2f0c7732f9fc869dacfd0508d481ec89b58 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 28 Dec 2023 09:56:25 -0500 Subject: [PATCH 14/33] fix(listenbrainz): Treat 400 responses as non-showstopping #114 --- .../common/vendor/ListenbrainzApiClient.ts | 34 +++++++++++++ .../scrobblers/ListenbrainzScrobbler.ts | 16 +++--- .../tests/listenbrainz/listenbrainz.test.ts | 49 +++++++++++++++++++ 3 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/backend/common/vendor/ListenbrainzApiClient.ts b/src/backend/common/vendor/ListenbrainzApiClient.ts index ca322ebd..a708bcd6 100644 --- a/src/backend/common/vendor/ListenbrainzApiClient.ts +++ b/src/backend/common/vendor/ListenbrainzApiClient.ts @@ -15,6 +15,7 @@ import { parseTrackCredits, uniqueNormalizedStrArr } from "../../utils/StringUtils"; +import {UpstreamError} from "../errors/UpstreamError"; export interface ArtistMBIDMapping { @@ -137,7 +138,40 @@ export class ListenbrainzApiClient extends AbstractApiClient { } catch (e) { const { message, + err, + status, + response: { + body = undefined, + text = undefined, + } = {} } = e; + // TODO check err for network exception + if(status !== undefined) { + const msgParts = [`(HTTP Status ${status})`]; + // if the response is 400 then its likely there was an issue with the data we sent rather than an error with the service + let showStopper = status !== 400; + if(body !== undefined) { + if(typeof body === 'object') { + if('code' in body) { + msgParts.push(`Code ${body.code}`); + } + if('error' in body) { + msgParts.push(`Error => ${body.error}`); + } + if('message' in body) { + msgParts.push(`Message => ${body.error}`); + } + // if('track_metadata' in body) { + // msgParts.push(`Track Metadata => ${JSON.stringify(body.track_metadata)}`); + // } + } else if(typeof body === 'string') { + msgParts.push(`Response => ${body}`); + } + } else if (text !== undefined) { + msgParts.push(`Response => ${text}`); + } + throw new UpstreamError(`Listenbrainz API Request Failed => ${msgParts.join(' | ')}`, {cause: e, showStopper}); + } throw e; } } diff --git a/src/backend/scrobblers/ListenbrainzScrobbler.ts b/src/backend/scrobblers/ListenbrainzScrobbler.ts index 7f55dc07..bf4fc397 100644 --- a/src/backend/scrobblers/ListenbrainzScrobbler.ts +++ b/src/backend/scrobblers/ListenbrainzScrobbler.ts @@ -108,17 +108,13 @@ export default class ListenbrainzScrobbler extends AbstractScrobbleClient { //await sleep(1000); return playObj; } catch (e) { - let message = e.message; - if(e.response !== undefined) { - if(e.response.body !== undefined) { - message = e.response.body.messsage; - } else if(e.response.text !== undefined) { - message = e.response.text; - } + await this.notifier.notify({title: `Client - ${capitalize(this.type)} - ${this.name} - Scrobble Error`, message: `Failed to scrobble => ${buildTrackString(playObj)} | Error: ${e.message}`, priority: 'error'}); + this.logger.error(`Failed to scrobble => ${e.message}`, {payload: rawPayload}); + if(e instanceof UpstreamError) { + throw e; + } else { + throw new UpstreamError(`Error occurred while making Listenbrainz API request: ${e.message}`, {cause: e, showStopper: true}); } - await this.notifier.notify({title: `Client - ${capitalize(this.type)} - ${this.name} - Scrobble Error`, message: `Failed to scrobble => ${buildTrackString(playObj)} | Error: ${message}`, priority: 'error'}); - this.logger.error(`Failed to scrobble => ${message}`, {payload: rawPayload}); - throw new UpstreamError(`Error received from Listenbrainz API: ${message}`, {cause: e, showStopper: true}); } finally { this.logger.debug(`Raw Payload:`, {rawPayload}); } diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index 99479e06..b1f9d372 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -14,6 +14,11 @@ import incorrectMultiArtistsTrackName from './incorrectlyMapped/multiArtistsInTr import veryWrong from './incorrectlyMapped/veryWrong.json'; import {ListenbrainzApiClient, ListenResponse} from "../../common/vendor/ListenbrainzApiClient"; +import {PlayObject} from "../../../core/Atomic"; +import dayjs from "dayjs"; +import {withRequestInterception} from "../utils/networking"; +import {http, HttpResponse} from "msw"; +import {UpstreamError} from "../../common/errors/UpstreamError"; interface ExpectedResults { artists: string[] @@ -103,3 +108,47 @@ describe('Listenbrainz Listen Parsing', function () { }); }); }); + +describe('Listenbrainz Response Behavior', function() { + + const client = new ListenbrainzApiClient('test', + { + token: 'test', + username: 'test' + }); + + it('Should recognize bad requests as non-showstopping',withRequestInterception( + [ + http.post('https://api.listenbrainz.org/1/submit-listens', () => { + // @ts-expect-error + return HttpResponse.json({code: 400, error: 'artist_mbids MBID format invalid'}, {status: 400}); + }) + ], + async function() { + const play: PlayObject = { + data: { + artists: ['Celldweller'], + album: 'The Complete Cellout, Volume 01', + track: 'Frozen', + duration: 299, + playDate: dayjs(), + meta: { + brainz: { + artist: 'fad8967c-a327-4af5-a64a-d4de66ece652;100846a7-06f6-4129-97ce-4409b9a9a311', + album: '2eb6a8fb-14f6-436e-9bdf-2f9d0d8cbae0', + track: '677862e0-3603-4120-8c44-ee9a70893647', + releaseGroup: 'bd3bb964-6da7-4d59-b0aa-f8bf639cd419' + } + } + }, + meta: {} + } + try { + await client.submitListen(play); + } catch (e) { + assert.isTrue(e instanceof UpstreamError); + assert.isTrue(e.showStopper === false); + } + } + )); +}); -- 2.51.2 From e9561b5783256f2def68c55b682e327aa2dc43a8 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 28 Dec 2023 10:07:11 -0500 Subject: [PATCH 15/33] fix(jellyfin): Split musicbrainz mbid artist identifier semicolon #114 Jellyfin can apparently return multiple artist mbids but does so in one string deliminated by a semicolon --- src/backend/common/vendor/LastfmApiClient.ts | 2 +- src/backend/common/vendor/ListenbrainzApiClient.ts | 2 +- src/backend/sources/JellyfinSource.ts | 2 +- src/core/Atomic.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/common/vendor/LastfmApiClient.ts b/src/backend/common/vendor/LastfmApiClient.ts index dc9c160b..fa9e1ab6 100644 --- a/src/backend/common/vendor/LastfmApiClient.ts +++ b/src/backend/common/vendor/LastfmApiClient.ts @@ -72,7 +72,7 @@ export default class LastfmApiClient extends AbstractApiClient { meta: { brainz: { album: albumMbid === '' ? undefined : albumMbid, - artist: artistMbid === '' ? undefined : artistMbid, + artist: artistMbid === '' ? undefined : artistMbid.split(','), track: mbid === '' ? undefined : mbid } } diff --git a/src/backend/common/vendor/ListenbrainzApiClient.ts b/src/backend/common/vendor/ListenbrainzApiClient.ts index a708bcd6..2405e0df 100644 --- a/src/backend/common/vendor/ListenbrainzApiClient.ts +++ b/src/backend/common/vendor/ListenbrainzApiClient.ts @@ -283,7 +283,7 @@ export class ListenbrainzApiClient extends AbstractApiClient { additional_info: { duration: play.data.duration !== undefined ? Math.round(duration) : undefined, track_mbid: brainz.track, - artist_mbids: brainz.artist !== undefined ? [brainz.artist] : undefined, + artist_mbids: brainz.artist, release_mbid: brainz.album, release_group_mbid: brainz.releaseGroup } diff --git a/src/backend/sources/JellyfinSource.ts b/src/backend/sources/JellyfinSource.ts index 698e40c1..532d8b3c 100644 --- a/src/backend/sources/JellyfinSource.ts +++ b/src/backend/sources/JellyfinSource.ts @@ -158,7 +158,7 @@ export default class JellyfinSource extends MemorySource { playDate, meta: { brainz: { - artist: Provider_musicbrainzartist, + artist: Provider_musicbrainzartist !== undefined ? Provider_musicbrainzartist.split(';') : undefined, album: Provider_musicbrainzalbum, albumArtist: Provider_musicbrainzalbumartist, track: Provider_musicbrainztrack, diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index b9d5ba66..71c8e01c 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -66,7 +66,7 @@ export interface TrackData { meta?: { brainz?: { - artist?: string + artist?: string[] albumArtist?: string album?: string track?: string -- 2.51.2 From c0e5bb872af712b95c5a7b946583506da1f756bf Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 11:09:51 -0500 Subject: [PATCH 16/33] fix(lastfm): Fix handling of non-string mbid string values #114 --- src/backend/common/vendor/LastfmApiClient.ts | 10 ++++--- src/backend/sources/JellyfinSource.ts | 5 ++-- src/core/StringUtils.ts | 29 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/backend/common/vendor/LastfmApiClient.ts b/src/backend/common/vendor/LastfmApiClient.ts index fa9e1ab6..c443b9c4 100644 --- a/src/backend/common/vendor/LastfmApiClient.ts +++ b/src/backend/common/vendor/LastfmApiClient.ts @@ -6,6 +6,8 @@ import {DEFAULT_RETRY_MULTIPLIER, FormatPlayObjectOptions} from "../infrastructu import { LastfmData } from "../infrastructure/config/client/lastfm"; import { PlayObject } from "../../../core/Atomic"; import {isNodeNetworkException} from "../errors/NodeErrors"; +import {nonEmptyStringOrDefault, splitByFirstFound} from "../../../core/StringUtils"; +import {source} from "common-tags"; const badErrors = [ 'api key suspended', @@ -61,7 +63,7 @@ export default class LastfmApiClient extends AbstractApiClient { mbid, } = obj; // arbitrary decision yikes - let artistStrings = artists !== undefined ? artists.split(',') : [artistName]; + let artistStrings = splitByFirstFound(artists, [','], [artistName]); return { data: { artists: [...new Set(artistStrings)] as string[], @@ -71,9 +73,9 @@ export default class LastfmApiClient extends AbstractApiClient { playDate: time !== undefined ? dayjs.unix(time) : undefined, meta: { brainz: { - album: albumMbid === '' ? undefined : albumMbid, - artist: artistMbid === '' ? undefined : artistMbid.split(','), - track: mbid === '' ? undefined : mbid + album: nonEmptyStringOrDefault(albumMbid), + artist: splitByFirstFound(artistMbid, [',',';'], undefined), + track: nonEmptyStringOrDefault(mbid) } } }, diff --git a/src/backend/sources/JellyfinSource.ts b/src/backend/sources/JellyfinSource.ts index 532d8b3c..3f662995 100644 --- a/src/backend/sources/JellyfinSource.ts +++ b/src/backend/sources/JellyfinSource.ts @@ -17,7 +17,8 @@ import { PlayerStateOptions } from "./PlayerState/AbstractPlayerState"; import {Logger} from "@foxxmd/winston"; import { JellyfinPlayerState } from "./PlayerState/JellyfinPlayerState"; import { PlayObject } from "../../core/Atomic"; -import { buildTrackString, truncateStringToLength } from "../../core/StringUtils"; +import {buildTrackString, splitByFirstFound, truncateStringToLength} from "../../core/StringUtils"; +import {source} from "common-tags"; const shortDeviceId = truncateStringToLength(10, ''); @@ -158,7 +159,7 @@ export default class JellyfinSource extends MemorySource { playDate, meta: { brainz: { - artist: Provider_musicbrainzartist !== undefined ? Provider_musicbrainzartist.split(';') : undefined, + artist: splitByFirstFound(Provider_musicbrainzartist, [';'], undefined), album: Provider_musicbrainzalbum, albumArtist: Provider_musicbrainzalbumartist, track: Provider_musicbrainztrack, diff --git a/src/core/StringUtils.ts b/src/core/StringUtils.ts index b4192658..f8d03b3b 100644 --- a/src/core/StringUtils.ts +++ b/src/core/StringUtils.ts @@ -5,6 +5,7 @@ import relativeTime from "dayjs/plugin/relativeTime.js"; import duration from "dayjs/plugin/duration.js"; import timezone from "dayjs/plugin/timezone.js"; import {AmbPlayObject, TrackStringOptions} from "./Atomic"; +import {str} from "ajv"; dayjs.extend(utc) dayjs.extend(isBetween); @@ -98,3 +99,31 @@ export const slice = (str: string, index: number, count: number, add?: string): export const capitalize = (str: any) => { return str.charAt(0).toUpperCase() + str.slice(1) } + +/** + * Split a string-ish variable by a list of deliminators and return the first actually split array or default to returning the string as the first element. + * + * Returns empty array, or user defined value, if variable is undefined / null / not a string / or an empty string. + * */ +export const splitByFirstFound = (str: any, delims = [','], onNotAStringVal: T): string[] | T => { + if(str === undefined || str === null || typeof str !== 'string' || str.trim() === '') { + return onNotAStringVal; + } + for(const d of delims) { + const split = d.split(d); + if(split.length > 1) { + return split; + } + } + return [str]; +} + +/** + * Returns value if it is a non-empty string or returns default value + * */ +export const nonEmptyStringOrDefault = (str: any, defaultVal: T = undefined): string | T => { + if (str === undefined || str === null || typeof str !== 'string' || str.trim() === '') { + return defaultVal; + } + return str; +} -- 2.51.2 From 064a435c79497d36be99adc05976fa10702265d1 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 12:34:19 -0500 Subject: [PATCH 17/33] Simplify debug mode usage for testing --- register.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/register.js b/register.js index fd70d45e..8870ec05 100644 --- a/register.js +++ b/register.js @@ -21,5 +21,7 @@ tsNode.register({ project: './src/backend/tsconfig.json' }); -process.env.CONSOLE_LEVEL = parseBool(process.env.DEBUG_MODE) ? undefined : 'false'; +if(!parseBool(process.env.DEBUG_MODE)) { + process.env.CONSOLE_LEVEL = 'false'; +} process.env.FILE_LEVEL = 'false'; -- 2.51.2 From f126bf0ec0b8cf3bf9b896866359823525fb5d6d Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 12:38:24 -0500 Subject: [PATCH 18/33] refactor: Improve string comparison robustness * Fix undefined token inclusion caused by token lists with differing lengths * Always compare longer to shorter string so sameness parameter order is invariant * Add comments to make logic easier to understand * Add tests to test new functionality --- .../tests/scrobbler/scrobblers.test.ts | 1 - src/backend/tests/strings.test.ts | 21 +++++ src/backend/utils/StringUtils.ts | 76 ++++++++++++++----- 3 files changed, 80 insertions(+), 18 deletions(-) diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index dcef5326..172358e4 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -285,7 +285,6 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu const sonDiffPlay = clone(son); sonDiffPlay.data.playDate = sonDiffPlay.data.playDate.subtract(son.data.duration + 1, 's'); - sonDiffPlay.data.artists = [sonDiffPlay.data.artists[1]] assert.isTrue(await testScrobbler.alreadyScrobbled(sonDiffPlay)); }); diff --git a/src/backend/tests/strings.test.ts b/src/backend/tests/strings.test.ts index b4ceb9cc..626844d6 100644 --- a/src/backend/tests/strings.test.ts +++ b/src/backend/tests/strings.test.ts @@ -73,4 +73,25 @@ describe('String Comparisons', function () { assert.isAtMost( result.highScore, 99, `Comparing: '${test[0]}' | '${test[1]}'`); } }); + + it('should handle strings with different lengths', async function () { + const tests = [ + ['The Amazing Bongo Hop', 'The Bongo Hop'], + ] + + for(const test of tests) { + const result = compareNormalizedStrings(test[0], test[1]); + assert.isAtMost( result.highScore, 53, `Comparing: '${test[0]}' | '${test[1]}'`); + } + }); + + it('should be string parameter order invariant', async function () { + const longerString = 'Nidia Gongora TEST'; + const shorterString = 'Nidia Gongora' + + const result1 = compareNormalizedStrings(longerString, shorterString); + const result2 = compareNormalizedStrings(shorterString, longerString); + + assert.equal( result1.highScore, result2.highScore, `Comparing: '${longerString}' | '${shorterString}'`); + }); }); diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index acd1df87..295ee2f6 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -167,23 +167,55 @@ export const compareScrobbleArtists = (existing: PlayObject, candidate: PlayObje return compareNormalizedStrings(existingArtists.reduce((acc, curr) => `${acc} ${curr}`, ''), candidateArtists.reduce((acc, curr) => `${acc} ${curr}`, '')).highScore; } +/** + * Compare the sameness of two strings after making them token-order independent + * + * Transform two strings before comparing in order to have as little difference between them as possible: + * + * * First, normalize (lower case, remove extraneous whitespace, remove punctuation, make all characters standard ANSI) strings and split into tokens + * * Second, reorder tokens in the shorter list so that they mirror order of tokens in longer list as closely as possible + * * Finally, concat back to strings and compare with sameness strategies + * + * */ export const compareNormalizedStrings = (existing: string, candidate: string): StringSamenessResult => { + // there may be scenarios where a track differs in *ordering* of ancillary information between sources + // EX My Track (feat. Art1, Art2) -- My Track (feat. Art2 Art1) + + // first remove lower case, extraneous whitespace, punctuation, and replace non-ansi with ansi characters const normalExisting = normalizeStr(existing, {keepSingleWhitespace: true}); const normalCandidate = normalizeStr(candidate, {keepSingleWhitespace: true}); - // there may be scenarios where a track differs in *ordering* of ancillary information between sources - // EX My Track (feat. Art1, Art2) -- My Track (feat. Art2 Art1) - // so instead of naively comparing the entire track string against the candidate we - // * first try to match up all white-space separated tokens - // * recombine with closest tokens in order - // * then check sameness + // split by "token" const eTokens = normalExisting.split(' '); const cTokens = normalCandidate.split(' '); - const orderedCandidateTokens = eTokens.reduce((acc: { ordered: string[], remaining: string[] }, curr) => { + + let longerTokens: string[], + shorterTokens: string[]; + + if (eTokens.length > cTokens.length) { + longerTokens = eTokens; + shorterTokens = cTokens; + } else { + longerTokens = cTokens; + shorterTokens = eTokens; + } + + // we will use longest string (token list) as the reducer and order the shorter list to match it + // so we don't have to deal with undefined positions in the shorter list + + const orderedCandidateTokens = longerTokens.reduce((acc: { ordered: string[], remaining: string[] }, curr) => { + // if we've run out of tokens in the shorter list just return + if (acc.remaining.length === 0) { + return acc; + } + + // on each iteration of tokens in the long list + // we iterate through remaining tokens from the shorter list and find the token with the most sameness + let highScore = 0; - let highIndex = undefined; + let highIndex = 0; let index = 0; for (const token of acc.remaining) { const result = stringSameness(curr, token); @@ -194,18 +226,28 @@ export const compareNormalizedStrings = (existing: string, candidate: string): S index++; } + // then remove the most same token from the remaining short list tokens const splicedRemaining = [...acc.remaining]; splicedRemaining.splice(highIndex, 1); - return {ordered: acc.ordered.concat(acc.remaining[highIndex]), remaining: splicedRemaining}; - }, {ordered: [], remaining: cTokens}); - - const allOrderedCandidateTokens = orderedCandidateTokens.ordered.concat(orderedCandidateTokens.remaining); - const orderedCandidateString = allOrderedCandidateTokens.join(' '); - - // since we have already "matched" up words by order we don't want to use cosine strat + return { + // finally add the most same token to the ordered short list + ordered: acc.ordered.concat(acc.remaining[highIndex]), + // and return the remaining short list tokens + remaining: splicedRemaining + }; + }, { + // "ordered" is the result of ordering tokens in the shorter list to match longer token order + ordered: [], + // remaining is the initial shorter list + remaining: shorterTokens + }); + + // since we have already "matched" up tokens by order we don't want to use cosine strat // bc it only does comparisons between whole words in a sentence (instead of all letters in a string) // which makes it inaccurate for small-n sentences and typos - - return stringSameness(normalExisting, orderedCandidateString, {transforms: [], strategies: [levenStrategy, diceStrategy]}); + return stringSameness(longerTokens.join(' '), orderedCandidateTokens.ordered.join(' '), { + transforms: [], + strategies: [levenStrategy, diceStrategy] + }) } -- 2.51.2 From 0f721b94c2d976856dacafe3b385b3f70b7eef95 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 13:02:44 -0500 Subject: [PATCH 19/33] refactor: Move Play time comparison code into own file --- .../scrobblers/AbstractScrobbleClient.ts | 2 +- src/backend/scrobblers/MalojaScrobbler.ts | 3 +- src/backend/sources/AbstractSource.ts | 2 +- src/backend/sources/JellyfinSource.ts | 4 +- src/backend/utils.ts | 140 +----------------- src/backend/utils/TimeUtils.ts | 136 +++++++++++++++++ src/core/Atomic.ts | 11 ++ 7 files changed, 153 insertions(+), 145 deletions(-) create mode 100644 src/backend/utils/TimeUtils.ts diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index cf371abf..67ba527b 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -1,7 +1,6 @@ import dayjs, {Dayjs} from "dayjs"; import { comparingMultipleArtists, - isPlayTemporallyClose, mergeArr, playObjDataMatch, pollingBackoff, setIntersection, sleep, sortByOldestPlayDate, @@ -28,6 +27,7 @@ import {hasUpstreamError, UpstreamError} from "../common/errors/UpstreamError"; import {nanoid} from "nanoid"; import {ErrorWithCause, messageWithCauses} from "pony-cause"; import {hasNodeNetworkException} from "../common/errors/NodeErrors"; +import {isPlayTemporallyClose} from "../utils/TimeUtils"; export default abstract class AbstractScrobbleClient implements Authenticatable { diff --git a/src/backend/scrobblers/MalojaScrobbler.ts b/src/backend/scrobblers/MalojaScrobbler.ts index b1b6f299..bfcf1b14 100644 --- a/src/backend/scrobblers/MalojaScrobbler.ts +++ b/src/backend/scrobblers/MalojaScrobbler.ts @@ -8,7 +8,7 @@ import { sleep, sortByOldestPlayDate, parseRetryAfterSecsFromObj, - isPlayTemporallyClose, + } from "../utils"; import {DEFAULT_RETRY_MULTIPLIER, FormatPlayObjectOptions, INITIALIZING} from "../common/infrastructure/Atomic"; import { MalojaClientConfig } from "../common/infrastructure/config/client/maloja"; @@ -27,6 +27,7 @@ import normalizeUrl from "normalize-url"; import {UpstreamError} from "../common/errors/UpstreamError"; import {ar} from "@faker-js/faker"; import {ErrorWithCause} from "pony-cause"; +import {isPlayTemporallyClose} from "../utils/TimeUtils"; const feat = ["ft.", "ft", "feat.", "feat", "featuring", "Ft.", "Ft", "Feat.", "Feat", "Featuring"]; diff --git a/src/backend/sources/AbstractSource.ts b/src/backend/sources/AbstractSource.ts index 8e748eb7..2501ea3a 100644 --- a/src/backend/sources/AbstractSource.ts +++ b/src/backend/sources/AbstractSource.ts @@ -1,6 +1,5 @@ import dayjs, {Dayjs} from "dayjs"; import { - isPlayTemporallyClose, genGroupId, genGroupIdStrFromPlay, mergeArr, @@ -34,6 +33,7 @@ import { PlayObject } from "../../core/Atomic"; import {buildTrackString, capitalize} from "../../core/StringUtils"; import {isNodeNetworkException} from "../common/errors/NodeErrors"; import {ErrorWithCause} from "pony-cause"; +import {isPlayTemporallyClose} from "../utils/TimeUtils"; export interface RecentlyPlayedOptions { limit?: number diff --git a/src/backend/sources/JellyfinSource.ts b/src/backend/sources/JellyfinSource.ts index 3f662995..14227347 100644 --- a/src/backend/sources/JellyfinSource.ts +++ b/src/backend/sources/JellyfinSource.ts @@ -1,13 +1,10 @@ import MemorySource from "./MemorySource"; import dayjs, {Dayjs} from "dayjs"; import { - isPlayTemporallyClose, combinePartsToString, parseBool, parseDurationFromTimestamp, playObjDataMatch, - comparePlayTemporally, - temporalPlayComparisonSummary, doubleReturnNewline, } from "../utils"; import { JellySourceConfig } from "../common/infrastructure/config/source/jellyfin"; @@ -19,6 +16,7 @@ import { JellyfinPlayerState } from "./PlayerState/JellyfinPlayerState"; import { PlayObject } from "../../core/Atomic"; import {buildTrackString, splitByFirstFound, truncateStringToLength} from "../../core/StringUtils"; import {source} from "common-tags"; +import {comparePlayTemporally, isPlayTemporallyClose, temporalPlayComparisonSummary} from "../utils/TimeUtils"; const shortDeviceId = truncateStringToLength(10, ''); diff --git a/src/backend/utils.ts b/src/backend/utils.ts index fcd19d53..0f1ed578 100644 --- a/src/backend/utils.ts +++ b/src/backend/utils.ts @@ -9,7 +9,6 @@ import { asPlayerStateData, DEFAULT_SCROBBLE_DURATION_THRESHOLD, DEFAULT_SCROBBLE_PERCENT_THRESHOLD, - lowGranularitySources, NO_DEVICE, NO_USER, numberFormatOptions, @@ -27,7 +26,7 @@ import backoffStrategies from '@kenyip/backoff-strategies'; import {ScrobbleThresholds} from "./common/infrastructure/config/source"; import {replaceResultTransformer, stripIndentTransformer, TemplateTag, trimResultTransformer} from 'common-tags'; import {Duration} from "dayjs/plugin/duration.js"; -import {ListenRangeData, PlayObject} from "../core/Atomic"; +import {PlayObject} from "../core/Atomic"; import address from "address"; dayjs.extend(utc); @@ -423,143 +422,6 @@ export const remoteHostStr = (req: Request): string => { return `${host}${proxy !== undefined ? ` (${proxy})` : ''}${agent !== undefined ? ` (UA: ${agent})` : ''}`; } -export const isPlayTemporallyClose = (existingPlay: PlayObject, candidatePlay: PlayObject, options: { diffThreshold?: number, fuzzyDuration?: boolean, useListRanges?: boolean} = {}): boolean => { - return comparePlayTemporally(existingPlay, candidatePlay, options).close; -} - -export interface TemporalPlayComparison { - close: boolean - date?: { - threshold: number - diff: number - fuzzyDurationDiff?: number - fuzzyListenedDiff?: number - } - range?: false | ListenRangeData -} - -export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, existingPlay?: PlayObject, candidatePlay?: PlayObject) => { - const parts: string[] = []; - if (existingPlay !== undefined && candidatePlay !== undefined) { - if (existingPlay.data.playDate.isSame(candidatePlay.data.playDate, 'day')) { - parts.push(`Existing: ${existingPlay.data.playDate.format('HH:mm:ssZ')} - Candidate: ${candidatePlay.data.playDate.format('HH:mm:ssZ')}`); - } else { - parts.push(`Existing: ${existingPlay.data.playDate.toISOString()} - Candidate: ${candidatePlay.data.playDate.toISOString()}`); - } - } - parts.push(`Close: ${data.close ? 'YES' : 'NO'}`); - if (data.date !== undefined) { - parts.push(`Play Diff: ${formatNumber(data.date.diff, {toFixed: 0})}s (Needed <${data.date.threshold}s)`) - } - if (data.date.fuzzyDurationDiff !== undefined) { - parts.push(`Fuzzy Duration Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); - } - if (data.date.fuzzyListenedDiff !== undefined) { - parts.push(`Fuzzy Listened Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); - } - if (data.range !== undefined) { - if (data.range === false) { - parts.push('Candidate not played during Existing tracked listening'); - } else { - parts.push(`Candidate played during tracked listening range from existing: ${data.range[0].timestamp.format('HH:mm:ssZ')} => ${data.range[1].timestamp.format('HH:mm:ssZ')}`); - } - } else { - parts.push('One or both Plays did not have have tracked listening to compare'); - } - return parts.join(' | '); -} - -export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: PlayObject, options: { diffThreshold?: number, fuzzyDuration?: boolean, useListRanges?: boolean} = {}): TemporalPlayComparison => { - - const result: TemporalPlayComparison = { - close: false - }; - - const { - meta:{ - source, - }, - data: { - playDate: existingPlayDate, - duration: existingDuration, - listenRanges: existingRanges, - listenedFor: existingListenedFor, - } - } = existingPlay; - - const { - data: { - playDate: newPlayDate, - duration: newDuration, - listenRanges: newRanges, - listenedFor: newListenedFor, - } - } = candidatePlay; - - const { - diffThreshold = lowGranularitySources.some(x => x.toLocaleLowerCase() === source) ? 60 : 10, - fuzzyDuration = false, - useListRanges = true, - } = options; - - // cant compare! - if(existingPlayDate === undefined || newPlayDate === undefined) { - return result; - } - - const referenceDuration = newDuration ?? existingDuration; - const referenceListenedFor = newListenedFor ?? existingListenedFor; - - let playDiffThreshold = diffThreshold; - - // check if existing play time is same as new play date - let scrobblePlayDiff = Math.abs(existingPlayDate.unix() - newPlayDate.unix()); - result.date = { - threshold: diffThreshold, - diff: scrobblePlayDiff - }; - - if (scrobblePlayDiff <= playDiffThreshold) { - result.close = true; - } - - if(useListRanges && existingRanges !== undefined) { - // since we know when the existing track was listened to - // we can check if the new track play date took place while the existing one was being listened to - // which would indicate (assuming same source) the new track is a duplicate - for(const range of existingRanges) { - if(newPlayDate.isBetween(range.start.timestamp, range.end.timestamp)) { - result.range = range; - result.close = true; - break; - } - } - if(result.range === undefined) { - result.range = false; - } - } - - // if the source has a duration its possible one play was scrobbled at the beginning of the track and the other at the end - // so check if the duration matches the diff between the two play dates - if (result.close === false && referenceDuration !== undefined && fuzzyDuration) { - result.date.fuzzyDurationDiff = Math.abs(scrobblePlayDiff - referenceDuration); - if(result.date.fuzzyDurationDiff < 10) { // TODO use finer comparison for this? - result.close = true; - } - } - // if the source has listened duration (maloja) it may differ from actual track duration - // and its possible (spotify) the candidate play date is set at the end of this duration - // so check if there is a close match between candidate play date and source + listened for - if(result.close === false && referenceListenedFor !== undefined && fuzzyDuration) { - result.date.fuzzyListenedDiff = Math.abs(scrobblePlayDiff - referenceListenedFor); - if(result.date.fuzzyListenedDiff < 10) { // TODO use finer comparison for this? - result.close = true; - } - } - - return result; -} - export const combinePartsToString = (parts: any[], glue: string = '-'): string | undefined => { const cleanParts: string[] = []; for (const part of parts) { diff --git a/src/backend/utils/TimeUtils.ts b/src/backend/utils/TimeUtils.ts new file mode 100644 index 00000000..83f0177c --- /dev/null +++ b/src/backend/utils/TimeUtils.ts @@ -0,0 +1,136 @@ +import {PlayObject, TemporalPlayComparison} from "../../core/Atomic"; +import {lowGranularitySources} from "../common/infrastructure/Atomic"; +import {formatNumber} from "../utils"; + +export const isPlayTemporallyClose = (existingPlay: PlayObject, candidatePlay: PlayObject, options: { + diffThreshold?: number, + fuzzyDuration?: boolean, + useListRanges?: boolean +} = {}): boolean => { + return comparePlayTemporally(existingPlay, candidatePlay, options).close; +} + +export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, existingPlay?: PlayObject, candidatePlay?: PlayObject) => { + const parts: string[] = []; + if (existingPlay !== undefined && candidatePlay !== undefined) { + if (existingPlay.data.playDate.isSame(candidatePlay.data.playDate, 'day')) { + parts.push(`Existing: ${existingPlay.data.playDate.format('HH:mm:ssZ')} - Candidate: ${candidatePlay.data.playDate.format('HH:mm:ssZ')}`); + } else { + parts.push(`Existing: ${existingPlay.data.playDate.toISOString()} - Candidate: ${candidatePlay.data.playDate.toISOString()}`); + } + } + parts.push(`Close: ${data.close ? 'YES' : 'NO'}`); + if (data.date !== undefined) { + parts.push(`Play Diff: ${formatNumber(data.date.diff, {toFixed: 0})}s (Needed <${data.date.threshold}s)`) + } + if (data.date.fuzzyDurationDiff !== undefined) { + parts.push(`Fuzzy Duration Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); + } + if (data.date.fuzzyListenedDiff !== undefined) { + parts.push(`Fuzzy Listened Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); + } + if (data.range !== undefined) { + if (data.range === false) { + parts.push('Candidate not played during Existing tracked listening'); + } else { + parts.push(`Candidate played during tracked listening range from existing: ${data.range[0].timestamp.format('HH:mm:ssZ')} => ${data.range[1].timestamp.format('HH:mm:ssZ')}`); + } + } else { + parts.push('One or both Plays did not have have tracked listening to compare'); + } + return parts.join(' | '); +} +export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: PlayObject, options: { + diffThreshold?: number, + fuzzyDuration?: boolean, + useListRanges?: boolean +} = {}): TemporalPlayComparison => { + + const result: TemporalPlayComparison = { + close: false + }; + + const { + meta: { + source, + }, + data: { + playDate: existingPlayDate, + duration: existingDuration, + listenRanges: existingRanges, + listenedFor: existingListenedFor, + } + } = existingPlay; + + const { + data: { + playDate: newPlayDate, + duration: newDuration, + listenRanges: newRanges, + listenedFor: newListenedFor, + } + } = candidatePlay; + + const { + diffThreshold = lowGranularitySources.some(x => x.toLocaleLowerCase() === source) ? 60 : 10, + fuzzyDuration = false, + useListRanges = true, + } = options; + + // cant compare! + if (existingPlayDate === undefined || newPlayDate === undefined) { + return result; + } + + const referenceDuration = newDuration ?? existingDuration; + const referenceListenedFor = newListenedFor ?? existingListenedFor; + + let playDiffThreshold = diffThreshold; + + // check if existing play time is same as new play date + let scrobblePlayDiff = Math.abs(existingPlayDate.unix() - newPlayDate.unix()); + result.date = { + threshold: diffThreshold, + diff: scrobblePlayDiff + }; + + if (scrobblePlayDiff <= playDiffThreshold) { + result.close = true; + } + + if (useListRanges && existingRanges !== undefined) { + // since we know when the existing track was listened to + // we can check if the new track play date took place while the existing one was being listened to + // which would indicate (assuming same source) the new track is a duplicate + for (const range of existingRanges) { + if (newPlayDate.isBetween(range.start.timestamp, range.end.timestamp)) { + result.range = range; + result.close = true; + break; + } + } + if (result.range === undefined) { + result.range = false; + } + } + + // if the source has a duration its possible one play was scrobbled at the beginning of the track and the other at the end + // so check if the duration matches the diff between the two play dates + if (result.close === false && referenceDuration !== undefined && fuzzyDuration) { + result.date.fuzzyDurationDiff = Math.abs(scrobblePlayDiff - referenceDuration); + if (result.date.fuzzyDurationDiff < 10) { // TODO use finer comparison for this? + result.close = true; + } + } + // if the source has listened duration (maloja) it may differ from actual track duration + // and its possible (spotify) the candidate play date is set at the end of this duration + // so check if there is a close match between candidate play date and source + listened for + if (result.close === false && referenceListenedFor !== undefined && fuzzyDuration) { + result.date.fuzzyListenedDiff = Math.abs(scrobblePlayDiff - referenceListenedFor); + if (result.date.fuzzyListenedDiff < 10) { // TODO use finer comparison for this? + result.close = true; + } + } + + return result; +} diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index 71c8e01c..272502ad 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -207,3 +207,14 @@ export interface DeadLetterScrobble extends QueuedS export type Second = number; export type Millisecond = number; + +export interface TemporalPlayComparison { + close: boolean + date?: { + threshold: number + diff: number + fuzzyDurationDiff?: number + fuzzyListenedDiff?: number + } + range?: false | ListenRangeData +} -- 2.51.2 From 78abfc2168bde0661e984ee666f68e4e16c1db52 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 13:55:14 -0500 Subject: [PATCH 20/33] refactor: Temporal comparison result data improvements #121 * Refactor using 'close' boolean to 'match' granularity * Makes using granularity for future logic easier * Easier logging for granularity in summary * Remove intermediate temporal functions in classes for DRY and so we can use comparison results * Add Time Detail to match breakdown for more visibility during logging --- .../scrobblers/AbstractScrobbleClient.ts | 39 ++++--- src/backend/scrobblers/MalojaScrobbler.ts | 5 - src/backend/sources/AbstractSource.ts | 6 +- src/backend/sources/JellyfinSource.ts | 10 +- src/backend/sources/MemorySource.ts | 3 +- src/backend/utils.ts | 38 ------- src/backend/utils/TimeUtils.ts | 105 ++++++++++++++---- src/core/Atomic.ts | 9 +- 8 files changed, 127 insertions(+), 88 deletions(-) diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 67ba527b..48069552 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -19,7 +19,13 @@ import {Logger} from '@foxxmd/winston'; import { CommonClientConfig } from "../common/infrastructure/config/client/index"; import { Notifiers } from "../notifier/Notifiers"; import {FixedSizeList} from 'fixed-size-list'; -import {DeadLetterScrobble, PlayObject, QueuedScrobble, SourceScrobble, TrackStringOptions} from "../../core/Atomic"; +import { + DeadLetterScrobble, + PlayObject, + QueuedScrobble, + TA_CLOSE, TA_FUZZY, + TrackStringOptions +} from "../../core/Atomic"; import {buildTrackString, capitalize, truncateStringToLength} from "../../core/StringUtils"; import EventEmitter from "events"; import {compareScrobbleArtists, compareScrobbleTracks, normalizeStr} from "../utils/StringUtils"; @@ -27,7 +33,12 @@ import {hasUpstreamError, UpstreamError} from "../common/errors/UpstreamError"; import {nanoid} from "nanoid"; import {ErrorWithCause, messageWithCauses} from "pony-cause"; import {hasNodeNetworkException} from "../common/errors/NodeErrors"; -import {isPlayTemporallyClose} from "../utils/TimeUtils"; +import { + comparePlayTemporally, + temporalAccuracyIsAtLeast, + temporalAccuracyToString, + temporalPlayComparisonSummary +} from "../utils/TimeUtils"; export default abstract class AbstractScrobbleClient implements Authenticatable { @@ -246,21 +257,13 @@ export default abstract class AbstractScrobbleClient implements Authenticatable } const matchPlayDate = dtInvariantMatches.find((x: ScrobbledPlayObject) => { - const [closeTime, fuzzyTime = false] = this.compareExistingScrobbleTime(x.play, playObj); - return closeTime; + const temporalComparison = comparePlayTemporally(x.play, playObj); + return temporalAccuracyIsAtLeast(TA_CLOSE, temporalComparison.match) }); return [matchPlayDate, dtInvariantMatches]; } - protected compareExistingScrobbleTime = (existing: PlayObject, candidate: PlayObject): [boolean, boolean?] => { - let closeTime = isPlayTemporallyClose(existing, candidate); - let fuzzyTime = false; - if(!closeTime) { - fuzzyTime = isPlayTemporallyClose(existing, candidate, {fuzzyDuration: true}); - } - return [closeTime, fuzzyTime]; - } protected compareExistingScrobbleTitle = (existing: PlayObject, candidate: PlayObject): number => { return Math.min(compareScrobbleTracks(existing, candidate)/100, 1); } @@ -335,8 +338,13 @@ export default abstract class AbstractScrobbleClient implements Authenticatable //const referenceMatch = referenceApiScrobbleResponse !== undefined && playObjDataMatch(x, referenceApiScrobbleResponse); - const [closeTime, fuzzyTime = false] = this.compareExistingScrobbleTime(x, playObj); - const timeMatch = (closeTime ? 1 : (fuzzyTime ? 0.6 : 0)); + const temporalComparison = comparePlayTemporally(x, playObj); + let timeMatch = 0; + if(temporalAccuracyIsAtLeast(TA_CLOSE, temporalComparison.match)) { + timeMatch = 1; + } else if(temporalComparison.match === TA_FUZZY) { + timeMatch = 0.6; + } const titleMatch = this.compareExistingScrobbleTitle(x, playObj); @@ -377,7 +385,8 @@ export default abstract class AbstractScrobbleClient implements Authenticatable //`Reference: ${(referenceMatch ? 1 : 0)} * ${REFERENCE_WEIGHT} = ${referenceScore.toFixed(2)}`, artistBreakdown, `Title: ${titleMatch.toFixed(2)} * ${TITLE_WEIGHT} = ${titleScore.toFixed(2)}`, - `Time: ${timeMatch} * ${TIME_WEIGHT} = ${timeScore.toFixed(2)}`, + `Time: (${capitalize(temporalAccuracyToString(temporalComparison.match))}) ${timeMatch} * ${TIME_WEIGHT} = ${timeScore.toFixed(2)}`, + `Time Detail => ${temporalPlayComparisonSummary(temporalComparison, x, playObj)}`, `Score ${score.toFixed(2)} => ${score >= DUP_SCORE_THRESHOLD ? 'Matched!' : 'No Match'}` ]; diff --git a/src/backend/scrobblers/MalojaScrobbler.ts b/src/backend/scrobblers/MalojaScrobbler.ts index bfcf1b14..37eae4df 100644 --- a/src/backend/scrobblers/MalojaScrobbler.ts +++ b/src/backend/scrobblers/MalojaScrobbler.ts @@ -3,10 +3,7 @@ import request from 'superagent'; import dayjs from 'dayjs'; import compareVersions from 'compare-versions'; import { - playObjDataMatch, - setIntersection, sleep, - sortByOldestPlayDate, parseRetryAfterSecsFromObj, } from "../utils"; @@ -25,9 +22,7 @@ import {buildTrackString, capitalize} from "../../core/StringUtils"; import EventEmitter from "events"; import normalizeUrl from "normalize-url"; import {UpstreamError} from "../common/errors/UpstreamError"; -import {ar} from "@faker-js/faker"; import {ErrorWithCause} from "pony-cause"; -import {isPlayTemporallyClose} from "../utils/TimeUtils"; const feat = ["ft.", "ft", "feat.", "feat", "featuring", "Ft.", "Ft", "Feat.", "Feat", "Featuring"]; diff --git a/src/backend/sources/AbstractSource.ts b/src/backend/sources/AbstractSource.ts index 2501ea3a..b5ce5fc1 100644 --- a/src/backend/sources/AbstractSource.ts +++ b/src/backend/sources/AbstractSource.ts @@ -29,11 +29,11 @@ import { SourceConfig } from "../common/infrastructure/config/source/sources"; import {EventEmitter} from "events"; import {FixedSizeList} from "fixed-size-list"; import TupleMap from "../common/TupleMap"; -import { PlayObject } from "../../core/Atomic"; +import {PlayObject, TA_CLOSE} from "../../core/Atomic"; import {buildTrackString, capitalize} from "../../core/StringUtils"; import {isNodeNetworkException} from "../common/errors/NodeErrors"; import {ErrorWithCause} from "pony-cause"; -import {isPlayTemporallyClose} from "../utils/TimeUtils"; +import {comparePlayTemporally, temporalAccuracyIsAtLeast} from "../utils/TimeUtils"; export interface RecentlyPlayedOptions { limit?: number @@ -176,7 +176,7 @@ export default abstract class AbstractSource implements Authenticatable { }); } for(const list of lists) { - const existing = list.find(x => playObjDataMatch(x, play) && isPlayTemporallyClose(x, play)); + const existing = list.find(x => playObjDataMatch(x, play) && temporalAccuracyIsAtLeast(TA_CLOSE, comparePlayTemporally(x, play).match)); if(existing) { return existing; } diff --git a/src/backend/sources/JellyfinSource.ts b/src/backend/sources/JellyfinSource.ts index 14227347..d28ac8ee 100644 --- a/src/backend/sources/JellyfinSource.ts +++ b/src/backend/sources/JellyfinSource.ts @@ -13,10 +13,14 @@ import EventEmitter from "events"; import { PlayerStateOptions } from "./PlayerState/AbstractPlayerState"; import {Logger} from "@foxxmd/winston"; import { JellyfinPlayerState } from "./PlayerState/JellyfinPlayerState"; -import { PlayObject } from "../../core/Atomic"; +import {PlayObject, TA_CLOSE} from "../../core/Atomic"; import {buildTrackString, splitByFirstFound, truncateStringToLength} from "../../core/StringUtils"; import {source} from "common-tags"; -import {comparePlayTemporally, isPlayTemporallyClose, temporalPlayComparisonSummary} from "../utils/TimeUtils"; +import { + comparePlayTemporally, + temporalAccuracyIsAtLeast, + temporalPlayComparisonSummary +} from "../utils/TimeUtils"; const shortDeviceId = truncateStringToLength(10, ''); @@ -308,7 +312,7 @@ export default class JellyfinSource extends MemorySource { Temporal Comparison => ${temporalPlayComparisonSummary(temporalResult, currPlay, playObj)}`); } - if(temporalResult.close) { + if(temporalAccuracyIsAtLeast(TA_CLOSE,temporalResult.match)) { existingTracked = currPlay; } break; diff --git a/src/backend/sources/MemorySource.ts b/src/backend/sources/MemorySource.ts index a05cd81d..2b37532e 100644 --- a/src/backend/sources/MemorySource.ts +++ b/src/backend/sources/MemorySource.ts @@ -4,8 +4,6 @@ import { sortByOldestPlayDate, toProgressAwarePlayObject, getProgress, - playPassesScrobbleThreshold, - timePassesScrobbleThreshold, thresholdResultSummary, genGroupId, genGroupIdStr, @@ -32,6 +30,7 @@ import {SimpleIntervalJob, Task, ToadScheduler} from "toad-scheduler"; import {SourceConfig} from "../common/infrastructure/config/source/sources"; import {EventEmitter} from "events"; import objectHash from 'object-hash'; +import {timePassesScrobbleThreshold} from "../utils/TimeUtils"; export default class MemorySource extends AbstractSource { diff --git a/src/backend/utils.ts b/src/backend/utils.ts index 0f1ed578..fb634183 100644 --- a/src/backend/utils.ts +++ b/src/backend/utils.ts @@ -7,8 +7,6 @@ import {TimeoutError, WebapiError} from "spotify-web-api-node/src/response-error import Ajv, {Schema} from 'ajv'; import { asPlayerStateData, - DEFAULT_SCROBBLE_DURATION_THRESHOLD, - DEFAULT_SCROBBLE_PERCENT_THRESHOLD, NO_DEVICE, NO_USER, numberFormatOptions, @@ -23,7 +21,6 @@ import {Request} from "express"; import pathUtil from "path"; import {ErrorWithCause, getErrorCause} from "pony-cause"; import backoffStrategies from '@kenyip/backoff-strategies'; -import {ScrobbleThresholds} from "./common/infrastructure/config/source"; import {replaceResultTransformer, stripIndentTransformer, TemplateTag, trimResultTransformer} from 'common-tags'; import {Duration} from "dayjs/plugin/duration.js"; import {PlayObject} from "../core/Atomic"; @@ -476,41 +473,6 @@ export const getProgress = (initial: ProgressAwarePlayObject, curr: PlayObject): return undefined; } -export const playPassesScrobbleThreshold = (play: PlayObject, thresholds: ScrobbleThresholds): ScrobbleThresholdResult => { - const progressed = Math.round(Math.abs(dayjs().diff(play.data.playDate, 's'))); - return timePassesScrobbleThreshold(thresholds, progressed, play.data.duration); -} - -export const timePassesScrobbleThreshold = (thresholds: ScrobbleThresholds, secondsTracked: number, playDuration?: number): ScrobbleThresholdResult => { - let durationPasses = undefined, - durationThreshold: number | null = thresholds.duration ?? DEFAULT_SCROBBLE_DURATION_THRESHOLD, - percentPasses = undefined, - percentThreshold: number | null = thresholds.percent ?? DEFAULT_SCROBBLE_PERCENT_THRESHOLD, - percent: number | undefined; - - if (percentThreshold !== null && playDuration !== undefined && playDuration !== 0) { - percent = Math.round(((secondsTracked / playDuration) * 100)); - percentPasses = percent >= percentThreshold; - } - if (durationThreshold !== null || percentPasses === undefined) { - durationPasses = secondsTracked >= durationThreshold; - } - - return { - passes: (durationPasses ?? false) || (percentPasses ?? false), - duration: { - passes: durationPasses, - threshold: durationThreshold, - value: secondsTracked - }, - percent: { - passes: percentPasses, - value: percent, - threshold: percentThreshold - } - } -} - export const thresholdResultSummary = (result: ScrobbleThresholdResult) => { const parts: string[] = []; if(result.duration.passes !== undefined) { diff --git a/src/backend/utils/TimeUtils.ts b/src/backend/utils/TimeUtils.ts index 83f0177c..b74a0575 100644 --- a/src/backend/utils/TimeUtils.ts +++ b/src/backend/utils/TimeUtils.ts @@ -1,14 +1,21 @@ -import {PlayObject, TemporalPlayComparison} from "../../core/Atomic"; -import {lowGranularitySources} from "../common/infrastructure/Atomic"; +import { + PlayObject, + TA_CLOSE, + TA_EXACT, + TA_FUZZY, + TA_NONE, + TemporalAccuracy, + TemporalPlayComparison +} from "../../core/Atomic"; +import { + DEFAULT_SCROBBLE_DURATION_THRESHOLD, + DEFAULT_SCROBBLE_PERCENT_THRESHOLD, + lowGranularitySources, + ScrobbleThresholdResult +} from "../common/infrastructure/Atomic"; import {formatNumber} from "../utils"; - -export const isPlayTemporallyClose = (existingPlay: PlayObject, candidatePlay: PlayObject, options: { - diffThreshold?: number, - fuzzyDuration?: boolean, - useListRanges?: boolean -} = {}): boolean => { - return comparePlayTemporally(existingPlay, candidatePlay, options).close; -} +import {ScrobbleThresholds} from "../common/infrastructure/config/source"; +import {capitalize} from "../../core/StringUtils"; export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, existingPlay?: PlayObject, candidatePlay?: PlayObject) => { const parts: string[] = []; @@ -19,7 +26,7 @@ export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, exis parts.push(`Existing: ${existingPlay.data.playDate.toISOString()} - Candidate: ${candidatePlay.data.playDate.toISOString()}`); } } - parts.push(`Close: ${data.close ? 'YES' : 'NO'}`); + parts.push(`Temporal Sameness: ${capitalize(temporalAccuracyToString(data.match))}`); if (data.date !== undefined) { parts.push(`Play Diff: ${formatNumber(data.date.diff, {toFixed: 0})}s (Needed <${data.date.threshold}s)`) } @@ -33,10 +40,10 @@ export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, exis if (data.range === false) { parts.push('Candidate not played during Existing tracked listening'); } else { - parts.push(`Candidate played during tracked listening range from existing: ${data.range[0].timestamp.format('HH:mm:ssZ')} => ${data.range[1].timestamp.format('HH:mm:ssZ')}`); + parts.push(`Candidate played during tracked listening range from Existing ${data.range[0].timestamp.format('HH:mm:ssZ')} => ${data.range[1].timestamp.format('HH:mm:ssZ')}`); } } else { - parts.push('One or both Plays did not have have tracked listening to compare'); + parts.push('Range Comparison N/A'); } return parts.join(' | '); } @@ -47,7 +54,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P } = {}): TemporalPlayComparison => { const result: TemporalPlayComparison = { - close: false + match: TA_NONE }; const { @@ -94,8 +101,10 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P diff: scrobblePlayDiff }; - if (scrobblePlayDiff <= playDiffThreshold) { - result.close = true; + if(scrobblePlayDiff <= 1) { + result.match = TA_EXACT; + } else if (scrobblePlayDiff <= playDiffThreshold) { + result.match = TA_CLOSE; } if (useListRanges && existingRanges !== undefined) { @@ -105,7 +114,9 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P for (const range of existingRanges) { if (newPlayDate.isBetween(range.start.timestamp, range.end.timestamp)) { result.range = range; - result.close = true; + if(!temporalAccuracyIsAtLeast(TA_CLOSE, result.match)) { + result.match = TA_CLOSE; + } break; } } @@ -116,21 +127,73 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P // if the source has a duration its possible one play was scrobbled at the beginning of the track and the other at the end // so check if the duration matches the diff between the two play dates - if (result.close === false && referenceDuration !== undefined && fuzzyDuration) { + if (result.match === TA_NONE && referenceDuration !== undefined) { result.date.fuzzyDurationDiff = Math.abs(scrobblePlayDiff - referenceDuration); if (result.date.fuzzyDurationDiff < 10) { // TODO use finer comparison for this? - result.close = true; + result.match = TA_FUZZY; } } // if the source has listened duration (maloja) it may differ from actual track duration // and its possible (spotify) the candidate play date is set at the end of this duration // so check if there is a close match between candidate play date and source + listened for - if (result.close === false && referenceListenedFor !== undefined && fuzzyDuration) { + if (result.match === TA_NONE && referenceListenedFor !== undefined && fuzzyDuration) { result.date.fuzzyListenedDiff = Math.abs(scrobblePlayDiff - referenceListenedFor); if (result.date.fuzzyListenedDiff < 10) { // TODO use finer comparison for this? - result.close = true; + result.match = TA_FUZZY } } return result; } +export const timePassesScrobbleThreshold = (thresholds: ScrobbleThresholds, secondsTracked: number, playDuration?: number): ScrobbleThresholdResult => { + let durationPasses = undefined, + durationThreshold: number | null = thresholds.duration ?? DEFAULT_SCROBBLE_DURATION_THRESHOLD, + percentPasses = undefined, + percentThreshold: number | null = thresholds.percent ?? DEFAULT_SCROBBLE_PERCENT_THRESHOLD, + percent: number | undefined; + + if (percentThreshold !== null && playDuration !== undefined && playDuration !== 0) { + percent = Math.round(((secondsTracked / playDuration) * 100)); + percentPasses = percent >= percentThreshold; + } + if (durationThreshold !== null || percentPasses === undefined) { + durationPasses = secondsTracked >= durationThreshold; + } + + return { + passes: (durationPasses ?? false) || (percentPasses ?? false), + duration: { + passes: durationPasses, + threshold: durationThreshold, + value: secondsTracked + }, + percent: { + passes: percentPasses, + value: percent, + threshold: percentThreshold + } + } +} + +export const temporalAccuracyIsAtLeast = (expected: TemporalAccuracy, found: TemporalAccuracy): boolean => { + if(typeof expected === 'number') { + if(typeof found === 'number') { + return found <= expected; + } + return false; + } + return found === false; +} + +export const temporalAccuracyToString = (acc: TemporalAccuracy): string => { + switch(acc) { + case 1: + return 'exact'; + case 2: + return 'close'; + case 3: + return 'fuzzy'; + case false: + return 'no correlation'; + } +} diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index 272502ad..45dad801 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -208,8 +208,15 @@ export interface DeadLetterScrobble extends QueuedS export type Second = number; export type Millisecond = number; +export type TemporalAccuracy = 1 | 2 | 3 | false; + +export const TA_EXACT: TemporalAccuracy = 1; +export const TA_CLOSE: TemporalAccuracy = 2; +export const TA_FUZZY: TemporalAccuracy = 3; +export const TA_NONE: TemporalAccuracy = false; + export interface TemporalPlayComparison { - close: boolean + match: TemporalAccuracy date?: { threshold: number diff: number -- 2.51.2 From 12c2c0a132f7ee874f06598d05cd0ca1814595b0 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 14:46:33 -0500 Subject: [PATCH 21/33] feat(scrobbler): Change fuzzy diff threshold to be inclusive --- src/backend/tests/scrobbler/scrobblers.test.ts | 16 +++++++++++++++- src/backend/utils/TimeUtils.ts | 8 ++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index 172358e4..166b7acb 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -14,12 +14,15 @@ import {sleep} from "../../utils"; import {MockNetworkError, withRequestInterception} from "../utils/networking"; const firstPlayDate = dayjs().subtract(1, 'hour'); +const olderFirstPlayDate = dayjs().subtract(4, 'hour'); const withDurPlays = asPlays(withDuration); const mixedDurPlays = asPlays(mixedDuration); const normalizedWithDur = normalizePlays(withDurPlays, {initialDate: firstPlayDate}); const normalizedWithMixedDur = normalizePlays(mixedDurPlays, {initialDate: firstPlayDate}); +const normalizedWithMixedDurOlder = normalizePlays(mixedDurPlays, {initialDate: olderFirstPlayDate}); + const testScrobbler = new TestScrobbler(); testScrobbler.verboseOptions = { match: { @@ -237,7 +240,7 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu assert.isTrue(await testScrobbler.alreadyScrobbled(diffPlay)); }); - it('Is detected as duplicate when play date is off by less than 10 seconds (high granularity source)', async function () { + it('Is detected as duplicate when play date is off by 10 seconds or less (high granularity source)', async function () { testScrobbler.recentScrobbles = normalizedWithMixedDur; @@ -249,6 +252,17 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu assert.isTrue(await testScrobbler.alreadyScrobbled(timeOffPos)); assert.isTrue(await testScrobbler.alreadyScrobbled(timeOffNeg)); + + // 10 seconds fuzzy diff inclusive + const son = normalizedWithMixedDurOlder.find(x => x.data.track === 'Sonora') + son.data.playDate = dayjs().subtract(1, 'hour').set('minute', 26).set('second', 20); + son.data.duration = 267; + son.data.listenedFor = undefined; + testScrobbler.recentScrobbles = normalizedWithMixedDurOlder.concat(son); + + const offSon = clone(son); + offSon.data.playDate = dayjs().subtract(1, 'hour').set('minute', 30).set('second', 37); + assert.isTrue(await testScrobbler.alreadyScrobbled(offSon)); }); it('Is detected as duplicate when play date is off by less than 60 seconds (low granularity source)', async function () { diff --git a/src/backend/utils/TimeUtils.ts b/src/backend/utils/TimeUtils.ts index b74a0575..43d21a66 100644 --- a/src/backend/utils/TimeUtils.ts +++ b/src/backend/utils/TimeUtils.ts @@ -31,10 +31,10 @@ export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, exis parts.push(`Play Diff: ${formatNumber(data.date.diff, {toFixed: 0})}s (Needed <${data.date.threshold}s)`) } if (data.date.fuzzyDurationDiff !== undefined) { - parts.push(`Fuzzy Duration Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); + parts.push(`Fuzzy Duration Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <= 10s)`); } if (data.date.fuzzyListenedDiff !== undefined) { - parts.push(`Fuzzy Listened Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <10s)`); + parts.push(`Fuzzy Listened Diff: ${formatNumber(data.date.fuzzyDurationDiff, {toFixed: 0})}s (Needed <= 10s)`); } if (data.range !== undefined) { if (data.range === false) { @@ -129,7 +129,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P // so check if the duration matches the diff between the two play dates if (result.match === TA_NONE && referenceDuration !== undefined) { result.date.fuzzyDurationDiff = Math.abs(scrobblePlayDiff - referenceDuration); - if (result.date.fuzzyDurationDiff < 10) { // TODO use finer comparison for this? + if (result.date.fuzzyDurationDiff <= 10) { // TODO use finer comparison for this? result.match = TA_FUZZY; } } @@ -138,7 +138,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P // so check if there is a close match between candidate play date and source + listened for if (result.match === TA_NONE && referenceListenedFor !== undefined && fuzzyDuration) { result.date.fuzzyListenedDiff = Math.abs(scrobblePlayDiff - referenceListenedFor); - if (result.date.fuzzyListenedDiff < 10) { // TODO use finer comparison for this? + if (result.date.fuzzyListenedDiff <= 10) { // TODO use finer comparison for this? result.match = TA_FUZZY } } -- 2.51.2 From 79c48af3e925c160807b34a617818517cfd748a0 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 15:24:24 -0500 Subject: [PATCH 22/33] chore(tests): Fix TS compile warning on test for bad data --- src/backend/tests/listenbrainz/listenbrainz.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index b1f9d372..15897816 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -134,6 +134,7 @@ describe('Listenbrainz Response Behavior', function() { playDate: dayjs(), meta: { brainz: { + // @ts-expect-error artist: 'fad8967c-a327-4af5-a64a-d4de66ece652;100846a7-06f6-4129-97ce-4409b9a9a311', album: '2eb6a8fb-14f6-436e-9bdf-2f9d0d8cbae0', track: '677862e0-3603-4120-8c44-ee9a70893647', -- 2.51.2 From ea408ac7a4c50b8bb7d0c16fcdb85c9c943b83cd Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 15:25:38 -0500 Subject: [PATCH 23/33] feat(scrobbler): Use play data to extract non-joined track name when comparing for duplicate check --- .../scrobblers/AbstractScrobbleClient.ts | 3 +- src/backend/tests/plays/mixedDuration.json | 19 ++++++++++++ .../tests/scrobbler/scrobblers.test.ts | 31 +++++++++++++++++++ src/backend/utils/StringUtils.ts | 18 +++++++++-- 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 48069552..519acb70 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -265,7 +265,8 @@ export default abstract class AbstractScrobbleClient implements Authenticatable } protected compareExistingScrobbleTitle = (existing: PlayObject, candidate: PlayObject): number => { - return Math.min(compareScrobbleTracks(existing, candidate)/100, 1); + const result = compareScrobbleTracks(existing, candidate); + return Math.min(result.highScore/100, 1); } protected compareExistingScrobbleArtist = (existing: PlayObject, candidate: PlayObject): [number, number] => { diff --git a/src/backend/tests/plays/mixedDuration.json b/src/backend/tests/plays/mixedDuration.json index 881cb5c4..0e03affd 100644 --- a/src/backend/tests/plays/mixedDuration.json +++ b/src/backend/tests/plays/mixedDuration.json @@ -133,5 +133,24 @@ "meta": { "source": "Maloja" } + }, + { + "data": { + "artists": [ + "9th Wonder", + "Cordae", + "Dinner Party", + "Kamasi Washington", + "Phoelix", + "Robert Glasper", + "Terrace Martin" + ], + "track": "Freeze Tag", + "album": "Dinner Party: Dessert", + "playDate": "2024-01-01T16:29:26.000Z" + }, + "meta": { + "source": "Maloja" + } } ] diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index 166b7acb..6d7e2dbc 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -12,6 +12,7 @@ import {asPlays, generatePlay, normalizePlays} from "../utils/PlayTestUtils"; import dayjs from "dayjs"; import {sleep} from "../../utils"; import {MockNetworkError, withRequestInterception} from "../utils/networking"; +import {PlayObject} from "../../../core/Atomic"; const firstPlayDate = dayjs().subtract(1, 'hour'); const olderFirstPlayDate = dayjs().subtract(4, 'hour'); @@ -302,6 +303,36 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu assert.isTrue(await testScrobbler.alreadyScrobbled(sonDiffPlay)); }); + it('Is detected as duplicate when artists are included in joiner', async function () { + const ref = normalizedWithMixedDurOlder.find(x => x.data.track === 'Freeze Tag'); + ref.data.playDate = dayjs().subtract(1, 'hour').set('minute', 29).set('second', 26) + + const spotifyPlay: PlayObject = { + data: { + artists: [ + "Terrace Martin", + "Robert Glasper", + "9th Wonder", + "Kamasi Washington", + "Dinner Party", + "Cordae", + "Phoelix" + ], + album: "Dinner Party: Dessert", + track: "Freeze Tag (feat. Cordae & Phoelix)", + "duration": 191.375, + "playDate": dayjs().subtract(1, 'hour').set('minute', 29).set('second', 27) + }, + meta: { + source: 'Spotify' + } + } + + testScrobbler.recentScrobbles = normalizedWithMixedDurOlder.concat(ref); + + assert.isTrue(await testScrobbler.alreadyScrobbled(spotifyPlay)); + }); + describe('When at least one play has duration', function () { it('Is detected as duplicate when play date is close to the end of an existing scrobble', async function () { diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index 295ee2f6..8451635d 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -135,7 +135,7 @@ export const findDelimiters = (str: string) => { return found; } -export const compareScrobbleTracks = (existing: PlayObject, candidate: PlayObject): number => { +export const compareScrobbleTracks = (existing: PlayObject, candidate: PlayObject): StringSamenessResult => { const { data: { track: existingTrack, @@ -148,7 +148,21 @@ export const compareScrobbleTracks = (existing: PlayObject, candidate: PlayObjec } } = candidate; - return compareNormalizedStrings(existingTrack, candidateTrack).highScore; + // try to remove any joiners based on existing artists + const existingCredits = parseTrackCredits(existingTrack); + const existingPrimary = existingCredits !== undefined ? existingCredits.primary : existingTrack; + + const candidateCredits = parseTrackCredits(candidateTrack); + const candidatePrimary = candidateCredits !== undefined ? candidateCredits.primary : candidateTrack; + + // take whichever score is higher + const creditsCleanedTrackSameness = compareNormalizedStrings(existingPrimary, candidatePrimary); + const naiveTrackSameness = compareNormalizedStrings(existingTrack, candidateTrack); + + if(creditsCleanedTrackSameness.highScore > naiveTrackSameness.highScore) { + return creditsCleanedTrackSameness; + } + return naiveTrackSameness; } export const compareScrobbleArtists = (existing: PlayObject, candidate: PlayObject): number => { -- 2.51.2 From 48ddd4272f12ae5298390021841dd2a6cee37c05 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 2 Jan 2024 15:38:13 -0500 Subject: [PATCH 24/33] feat(scrobbler): Improve log readability for duplicate checking --- .../scrobblers/AbstractScrobbleClient.ts | 8 ++--- src/backend/tests/plays/mixedDuration.json | 33 ------------------- .../tests/scrobbler/scrobblers.test.ts | 2 +- src/backend/tests/utils/PlayTestUtils.ts | 2 +- 4 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/backend/scrobblers/AbstractScrobbleClient.ts b/src/backend/scrobblers/AbstractScrobbleClient.ts index 519acb70..725250b8 100644 --- a/src/backend/scrobblers/AbstractScrobbleClient.ts +++ b/src/backend/scrobblers/AbstractScrobbleClient.ts @@ -294,7 +294,7 @@ export default abstract class AbstractScrobbleClient implements Authenticatable // return early if we don't care about checking existing if (false === this.checkExistingScrobbles) { if (this.verboseOptions.match.onNoMatch) { - this.logger.debug(`(Existing Check) Source: ${buildTrackString(playObj, scoreTrackOpts)} => No Match because existing scrobble check is FALSE`); + this.logger.debug(`${capitalize(playObj.meta.source ?? 'Source')}: ${buildTrackString(playObj, scoreTrackOpts)} => No Match because existing scrobble check is FALSE`, {leaf: ['Dupe Check']}); } return undefined; } @@ -325,7 +325,7 @@ export default abstract class AbstractScrobbleClient implements Authenticatable // (either user doesnt want to check history or there is no history to check!) if (this.recentScrobbles.length === 0) { if (this.verboseOptions.match.onNoMatch) { - this.logger.debug(`(Existing Check) ${buildTrackString(playObj, scoreTrackOpts)} => No Match because no recent scrobbles returned from API`); + this.logger.debug(`${buildTrackString(playObj, scoreTrackOpts)} => No Match because no recent scrobbles returned from API`, {leaf: ['Dupe Check']}); } return undefined; } @@ -410,10 +410,10 @@ export default abstract class AbstractScrobbleClient implements Authenticatable if ((existingScrobble !== undefined && this.verboseOptions.match.onMatch) || (existingScrobble === undefined && this.verboseOptions.match.onNoMatch)) { const closestScrobble = `Closest Scrobble: ${buildTrackString(closestMatch.scrobble, scoreTrackOpts)} => ${closestMatch.confidence}`; - this.logger.debug(`(Existing Check) Source: ${buildTrackString(playObj, scoreTrackOpts)} => ${closestScrobble}`); + this.logger.debug(`${capitalize(playObj.meta.source ?? 'Source')}: ${buildTrackString(playObj, scoreTrackOpts)} => ${closestScrobble}`, {leaf: ['Dupe Check']}); if (this.verboseOptions.match.confidenceBreakdown === true) { this.logger.debug(`Breakdown: -${closestMatch.breakdowns.join('\n')}`); +${closestMatch.breakdowns.join('\n')}`, {leaf: ['Dupe Check']}); } } return existingScrobble; diff --git a/src/backend/tests/plays/mixedDuration.json b/src/backend/tests/plays/mixedDuration.json index 0e03affd..a8ee964e 100644 --- a/src/backend/tests/plays/mixedDuration.json +++ b/src/backend/tests/plays/mixedDuration.json @@ -7,9 +7,6 @@ "track": "Fricassee", "duration": 305, "playDate": "2023-09-20T14:32:39.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -19,9 +16,6 @@ ], "track": "Jimbó", "playDate": "2023-09-20T14:46:00.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -33,9 +27,6 @@ "track": "Aisha", "duration": 559, "playDate": "2023-09-20T15:10:09.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -47,9 +38,6 @@ "track": "Ballade No. 4 in F Minor, Op. 52", "duration": 575, "playDate": "2023-09-20T15:17:12.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -62,9 +50,6 @@ "duration": 327, "listenedFor": 327, "playDate": "2023-09-27T20:02:17.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -74,9 +59,6 @@ ], "track": "Aprieta (Oye Cómo Va)", "playDate": "2023-09-20T15:38:12.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -86,9 +68,6 @@ ], "track": "Another Brick in the Wall, Pt. 1", "playDate": "2023-09-20T15:39:12.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -101,9 +80,6 @@ "duration": 575, "listenedFor": 570, "playDate": "2023-09-20T15:41:12.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -116,9 +92,6 @@ "track": "Aquil", "listenedFor": 300, "playDate": "2023-09-20T17:55:05.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -129,9 +102,6 @@ "track": "Da Art of Storytellin' (Pt. 1)", "duration": 422, "playDate": "2023-09-20T15:39:12.000Z" - }, - "meta": { - "source": "Maloja" } }, { @@ -148,9 +118,6 @@ "track": "Freeze Tag", "album": "Dinner Party: Dessert", "playDate": "2024-01-01T16:29:26.000Z" - }, - "meta": { - "source": "Maloja" } } ] diff --git a/src/backend/tests/scrobbler/scrobblers.test.ts b/src/backend/tests/scrobbler/scrobblers.test.ts index 6d7e2dbc..e70484aa 100644 --- a/src/backend/tests/scrobbler/scrobblers.test.ts +++ b/src/backend/tests/scrobbler/scrobblers.test.ts @@ -339,7 +339,7 @@ describe('Detects duplicate and unique scrobbles from client recent history', fu testScrobbler.recentScrobbles = normalizedWithDur; - const timeEnd = clone(normalizedWithDur[normalizedWithMixedDur.length - 1]); + const timeEnd = clone(normalizedWithDur[normalizedWithMixedDur.length - 2]); timeEnd.data.playDate = timeEnd.data.playDate.add(timeEnd.data.duration, 's'); assert.isTrue(await testScrobbler.alreadyScrobbled(timeEnd)); diff --git a/src/backend/tests/utils/PlayTestUtils.ts b/src/backend/tests/utils/PlayTestUtils.ts index e4bf1cc7..49fd73b4 100644 --- a/src/backend/tests/utils/PlayTestUtils.ts +++ b/src/backend/tests/utils/PlayTestUtils.ts @@ -88,7 +88,7 @@ export const generatePlay = (data: ObjectPlayData = {}, meta: PlayMeta = {}): Pl ...data }, meta: { - source: faker.word.noun(), + source: ['Spotify', 'Listenbrainz', 'Lastfm', 'Jellyfin', 'Plex'][faker.number.int({min: 0, max: 4})], ...meta, } } -- 2.51.2 From 1a90c23e764a419fc36514724b083819a6b0c7d0 Mon Sep 17 00:00:00 2001 From: Bujiraso Date: Tue, 2 Jan 2024 16:16:33 -0400 Subject: [PATCH 25/33] fix: correct json-schema.app links * Add missing 'backend' folder * Change 'develop' branch to 'master' --- docsite/docs/configuration/configuration.md | 40 ++++++++++----------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/docsite/docs/configuration/configuration.md b/docsite/docs/configuration/configuration.md index 9d8ff34b..120b4400 100644 --- a/docsite/docs/configuration/configuration.md +++ b/docsite/docs/configuration/configuration.md @@ -81,7 +81,7 @@ There is also a [**kitchensink example**](kitchensink.md) that provides examples ### All-in-One File Configuration -[**Explore the schema for this configuration, along with an example generator and validator, here**](https://json-schema.app/view/%23?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fcommon%2Fschema%2Faio.json) +[**Explore the schema for this configuration, along with an example generator and validator, here**](https://json-schema.app/view/%23?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Faio.json) Example directory structure: @@ -203,7 +203,7 @@ Client ID/Secret. Make sure to also whitelist your redirect URI in the applicati ### File-Based -See [`spotify.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/spotify.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FSpotifySourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`spotify.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/spotify.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FSpotifySourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Plex](https://plex.tv) @@ -217,7 +217,7 @@ Check the [instructions](plex.md) on how to setup a [webhooks](https://support.p ### File-Based -See [`plex.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/plex.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FPlexSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`plex.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/plex.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FPlexSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Tautulli](https://tautulli.com) @@ -231,7 +231,7 @@ Check the [instructions](plex.md) on how to setup a notification agent. ### File-Based -See [`tautulli.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/tautulli.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FTautulliSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`tautulli.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/tautulli.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FTautulliSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Subsonic](http://www.subsonic.org/) @@ -254,7 +254,7 @@ Can use this source for any application that implements the [Subsonic API](http: ### File-Based -See [`subsonic.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/subsonic.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FSubSonicSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`subsonic.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/subsonic.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FSubSonicSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Jellyfin](https://jellyfin.org/) @@ -284,7 +284,7 @@ If you see errors in the MS logs regarding `missing headers` when using Jellyfin ### File-Based -See [`jellyfin.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/jellyfin.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FJellySourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`jellyfin.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/jellyfin.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FJellySourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Last.fm (Source)](https://www.last.fm) @@ -296,7 +296,7 @@ No support for ENV based for Last.fm as a client (only source) ### File-Based -See [`lastfm.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/lastfm.json.example), change `configureAs` to `source`. Or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FLastfmSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`lastfm.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/lastfm.json.example), change `configureAs` to `source`. Or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FLastfmSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Listenbrainz (Source)](https://listenbrainz.org) @@ -308,7 +308,7 @@ On your [profile page](https://listenbrainz.org/profile/) find your **User Token ### File-Based -See [`listenbrainz.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/listenbrainz.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FListenBrainzSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`listenbrainz.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/listenbrainz.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FListenBrainzSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) **Change `configureAs` to `source`** @@ -340,7 +340,7 @@ After starting multi-scrobbler with credentials in-place open the dashboard (`ht ### File-Based -See [`deezer.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/deezer.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FDeezerSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`deezer.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/deezer.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FDeezerSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Youtube Music](https://music.youtube.com) @@ -366,7 +366,7 @@ NOTES: ### File-Based -See [`ytmusic.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/ytmusic.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FYTMusicSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`ytmusic.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/ytmusic.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FYTMusicSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [MPRIS](https://specifications.freedesktop.org/mpris-spec/latest/) @@ -390,7 +390,7 @@ multi-scrobbler can listen to this interface and scrobble tracks played by **any ### File-Based -See [`mpris.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/mpris.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FMPRISSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`mpris.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/mpris.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FMPRISSourceConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Mopidy](https://mopidy.com/) @@ -400,7 +400,7 @@ multi-scrobbler can scrobble tracks played from any Mopidy backend source, regar ### File-Based -See [`mopidy.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/mopidy.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FMopidySourceConfig/%23%2Fdefinitions%2FMopidyData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`mopidy.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/mopidy.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FMopidySourceConfig/%23%2Fdefinitions%2FMopidyData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) Configuration Options: @@ -557,7 +557,7 @@ MS transforms this to: `http://mydomain.com:80/jriverReverse/MCWS/v1/` ### File-Based -See [`jriver.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/jriver.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FJRiverSourceConfig/%23%2Fdefinitions%2FJRiverData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`jriver.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/jriver.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FJRiverSourceConfig/%23%2Fdefinitions%2FJRiverData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [Kodi](https://kodi.tv/) @@ -625,7 +625,7 @@ MS transforms this to: `http://mydomain.com:80/kodiReverse/jsonrpc` ### File-Based -See [`kodi.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/kodi.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FKodiSourceConfig/%23%2Fdefinitions%2FKodiData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`kodi.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/kodi.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FKodiSourceConfig/%23%2Fdefinitions%2FKodiData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) ## [WebScrobbler](https://web-scrobbler.com/) @@ -692,7 +692,7 @@ MS can be configured to only scrobble, or NOT scrobble, from some WS connectors. ### File-Based -See [`webscrobbler.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/webscrobbler.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FWebScrobblerSourceConfig/%23%2Fdefinitions%2FWebScrobblerData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fsource.json) +See [`webscrobbler.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/webscrobbler.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FWebScrobblerSourceConfig/%23%2Fdefinitions%2FWebScrobblerData?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fsource.json) # Client Configurations @@ -707,7 +707,7 @@ See [`webscrobbler.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob ### File-Based -See [`maloja.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/maloja.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FMalojaClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fclient.json) +See [`maloja.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/maloja.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FMalojaClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fclient.json) ## [Last.fm](https://www.last.fm) @@ -730,7 +730,7 @@ or replace `localhost:9078` with your own base URL ### File-Based -See [`lastfm.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/lastfm.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FLastfmClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fclient.json) +See [`lastfm.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/lastfm.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23/%23%2Fdefinitions%2FLastfmClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fclient.json) ## [Listenbrainz](https://listenbrainz.org) @@ -749,7 +749,7 @@ On your [profile page](https://listenbrainz.org/profile/) find your **User Token ### File-Based -See [`listenbrainz.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/listenbrainz.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FListenBrainzClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Fclient.json) +See [`listenbrainz.json.example`](https://github.com/FoxxMD/multi-scrobbler/blob/master/config/listenbrainz.json.example) or [explore the schema with an example and live editor/validator](https://json-schema.app/view/%23%2Fdefinitions%2FListenBrainzClientConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Fclient.json) # Monitoring @@ -798,7 +798,7 @@ Webhooks are configured in the main [config.json](#all-in-one-file-configuration ### [Gotify](https://gotify.net/) -Refer to the [config schema for GotifyConfig](https://json-schema.app/view/%23/%23%2Fdefinitions%2FGotifyConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Faio.json) +Refer to the [config schema for GotifyConfig](https://json-schema.app/view/%23/%23%2Fdefinitions%2FGotifyConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Faio.json) multi-scrobbler optionally supports setting message notification priority via `info` `warn` and `error` mappings. @@ -820,7 +820,7 @@ EX ### [Ntfy](https://ntfy.sh/) -Refer to the [config schema for NtfyConfig](https://json-schema.app/view/%23/%23%2Fdefinitions%2FNtfyConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fdevelop%2Fsrc%2Fcommon%2Fschema%2Faio.json) +Refer to the [config schema for NtfyConfig](https://json-schema.app/view/%23/%23%2Fdefinitions%2FNtfyConfig?url=https%3A%2F%2Fraw.githubusercontent.com%2FFoxxMD%2Fmulti-scrobbler%2Fmaster%2Fsrc%2Fbackend%2Fcommon%2Fschema%2Faio.json) multi-scrobbler optionally supports setting message notification priority via `info` `warn` and `error` mappings. -- 2.51.2 From e0f034fa99749e893e2a6a634f983652d0249295 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 3 Jan 2024 12:42:27 -0500 Subject: [PATCH 26/33] feat: Implement play completion context #121 * Track datetime player "completes" play at * Add scrobble datetime SOC metadata * Log SOC used when printing play datetime * Use hinted SOC scrobble datetime when comparing existing scrobbles * Use hinted SOC scrobble datetime when building scrobbler client scrobble payload * Hint Spotify backlog plays use END play date for scrobble SOC --- .../common/vendor/ListenbrainzApiClient.ts | 3 +- src/backend/scrobblers/LastfmScrobbler.ts | 3 +- src/backend/scrobblers/MalojaScrobbler.ts | 12 +++-- .../PlayerState/AbstractPlayerState.ts | 14 +++--- src/backend/sources/SpotifySource.ts | 27 ++++++++--- src/backend/utils/TimeUtils.ts | 45 ++++++++++++++++--- src/core/Atomic.ts | 12 ++++- src/core/StringUtils.ts | 21 ++++++--- 8 files changed, 106 insertions(+), 31 deletions(-) diff --git a/src/backend/common/vendor/ListenbrainzApiClient.ts b/src/backend/common/vendor/ListenbrainzApiClient.ts index 2405e0df..03ed0ffc 100644 --- a/src/backend/common/vendor/ListenbrainzApiClient.ts +++ b/src/backend/common/vendor/ListenbrainzApiClient.ts @@ -16,6 +16,7 @@ import { uniqueNormalizedStrArr } from "../../utils/StringUtils"; import {UpstreamError} from "../errors/UpstreamError"; +import {getScrobbleTsSOCDate} from "../../utils/TimeUtils"; export interface ArtistMBIDMapping { @@ -275,7 +276,7 @@ export class ListenbrainzApiClient extends AbstractApiClient { } } = play; return { - listened_at: (playDate ?? dayjs()).unix(), + listened_at: getScrobbleTsSOCDate(play).unix(), track_metadata: { artist_name: artists[0], track_name: track, diff --git a/src/backend/scrobblers/LastfmScrobbler.ts b/src/backend/scrobblers/LastfmScrobbler.ts index dc31ad0c..9523acd2 100644 --- a/src/backend/scrobblers/LastfmScrobbler.ts +++ b/src/backend/scrobblers/LastfmScrobbler.ts @@ -19,6 +19,7 @@ import {buildTrackString, capitalize} from "../../core/StringUtils"; import EventEmitter from "events"; import {UpstreamError} from "../common/errors/UpstreamError"; import {isNodeNetworkException} from "../common/errors/NodeErrors"; +import {getScrobbleTsSOCDate} from "../utils/TimeUtils"; export default class LastfmScrobbler extends AbstractScrobbleClient { @@ -163,7 +164,7 @@ export default class LastfmScrobbler extends AbstractScrobbleClient { duration, track, album, - timestamp: playDate.unix(), + timestamp: getScrobbleTsSOCDate(playObj).unix(), mbid, }; diff --git a/src/backend/scrobblers/MalojaScrobbler.ts b/src/backend/scrobblers/MalojaScrobbler.ts index 37eae4df..0046aa88 100644 --- a/src/backend/scrobblers/MalojaScrobbler.ts +++ b/src/backend/scrobblers/MalojaScrobbler.ts @@ -23,6 +23,7 @@ import EventEmitter from "events"; import normalizeUrl from "normalize-url"; import {UpstreamError} from "../common/errors/UpstreamError"; import {ErrorWithCause} from "pony-cause"; +import {getScrobbleTsSOCDate, getScrobbleTsSOCDateWithContext} from "../utils/TimeUtils"; const feat = ["ft.", "ft", "feat.", "feat", "featuring", "Ft.", "Ft", "Feat.", "Feat", "Featuring"]; @@ -377,16 +378,17 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { album, track, duration, - playDate, listenedFor } = {} } = playObj; + const [pd, scrobbleTsSOC] = getScrobbleTsSOCDateWithContext(playObj); + const scrobbleData: MalojaScrobbleRequestData = { title: track, album, key: apiKey, - time: playDate.unix(), + time: pd.unix(), // https://github.com/FoxxMD/multi-scrobbler/issues/42#issuecomment-1100184135 length: duration, }; @@ -424,6 +426,8 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { } = {} } = playObj; + const pd = getScrobbleTsSOCDate(playObj); + const sType = newFromSource ? 'New' : 'Backlog'; const scrobbleData = this.playToClientPayload(playObj); @@ -448,7 +452,7 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { if(status === 'success') { if(track !== undefined) { scrobbleResponse = { - time: playDate.unix(), + time: pd.unix(), track: { ...track, length: duration @@ -476,7 +480,7 @@ export default class MalojaScrobbler extends AbstractScrobbleClient { const { body: { track: { - time: mTime = playDate.unix(), + time: mTime = pd.unix(), duration: mDuration = duration, album: mAlbum = album, ...rest diff --git a/src/backend/sources/PlayerState/AbstractPlayerState.ts b/src/backend/sources/PlayerState/AbstractPlayerState.ts index a81c786b..05b64c29 100644 --- a/src/backend/sources/PlayerState/AbstractPlayerState.ts +++ b/src/backend/sources/PlayerState/AbstractPlayerState.ts @@ -104,7 +104,7 @@ export abstract class AbstractPlayerState { } else if (status !== undefined) { if (status === 'stopped' && this.reportedStatus !== 'stopped' && this.currentPlay !== undefined) { this.stopPlayer(); - const play = this.getPlayedObject(); + const play = this.getPlayedObject(true); this.clearPlayer(); return [play, play]; } @@ -125,7 +125,7 @@ export abstract class AbstractPlayerState { if (!playObjDataMatch(this.currentPlay, play)/* || (true !== false)*/) { // TODO check new play date and listen range to see if they intersect this.logger.debug(`Incoming play state (${buildTrackString(play, {include: ['trackId', 'artist', 'track']})}) does not match existing state, removing existing: ${buildTrackString(this.currentPlay, {include: ['trackId', 'artist', 'track']})}`) this.currentListenSessionEnd(); - const played = this.getPlayedObject(); + const played = this.getPlayedObject(true); this.setCurrentPlay(play, undefined, reportedTS); if (this.calculatedStatus !== CALCULATED_PLAYER_STATUSES.playing) { this.calculatedStatus = CALCULATED_PLAYER_STATUSES.unknown; @@ -137,7 +137,7 @@ export abstract class AbstractPlayerState { } else if (this.isSessionRepeat(play.meta.trackProgressPosition, reportedTS)) { // if we detect the track has been restarted end listen session and treat as a new play this.currentListenSessionEnd(); - const played = this.getPlayedObject(); + const played = this.getPlayedObject(true); play.data.playDate = dayjs(); this.setCurrentPlay(play, undefined, reportedTS); return [this.getPlayedObject(), played]; @@ -179,18 +179,22 @@ export abstract class AbstractPlayerState { this.currentListenSessionEnd(); } - getPlayedObject(): PlayObject | undefined { + getPlayedObject(completed: boolean = false): PlayObject | undefined { if(this.currentPlay !== undefined) { let ranges = [...this.listenRanges]; if (this.currentListenRange !== undefined) { ranges.push(this.currentListenRange); } + if(completed) { + this.logger.debug('Generating play object with playDateCompleted'); + } return { data: { ...this.currentPlay.data, playDate: this.playFirstSeenAt, listenedFor: this.getListenDuration(), - listenRanges: ranges + listenRanges: ranges, + playDateCompleted: completed ? dayjs() : undefined }, meta: this.currentPlay.meta } diff --git a/src/backend/sources/SpotifySource.ts b/src/backend/sources/SpotifySource.ts index 4acb2ed3..0bc83af7 100644 --- a/src/backend/sources/SpotifySource.ts +++ b/src/backend/sources/SpotifySource.ts @@ -1,4 +1,4 @@ -import dayjs from "dayjs"; +import dayjs, {Dayjs} from "dayjs"; import { readJson, writeFile, @@ -28,7 +28,7 @@ import AlbumObjectSimplified = SpotifyApi.AlbumObjectSimplified; import UserDevice = SpotifyApi.UserDevice; import MemorySource from "./MemorySource"; import {ErrorWithCause} from "pony-cause"; -import { PlayObject } from "../../core/Atomic"; +import {PlayObject, SCROBBLE_TS_SOC_END, SCROBBLE_TS_SOC_START, ScrobbleTsSOC} from "../../core/Atomic"; import { buildTrackString, truncateStringToLength } from "../../core/StringUtils"; import {isNodeNetworkException} from "../common/errors/NodeErrors"; import {hasUpstreamError, UpstreamError} from "../common/errors/UpstreamError"; @@ -77,11 +77,13 @@ export default class SpotifySource extends MemorySource { let album: AlbumObjectSimplified; let name: string; let duration_ms: number; - let played_at: string; + let played_at: Dayjs; + let playDateCompleted: Dayjs | undefined; let id: string; let url: string; let playbackPosition: number | undefined; let deviceId: string | undefined; + let scrobbleTsSOC: ScrobbleTsSOC; if (asPlayHistoryObject(obj)) { @@ -100,7 +102,9 @@ export default class SpotifySource extends MemorySource { } = {} } = track; - played_at = pa; + scrobbleTsSOC = SCROBBLE_TS_SOC_END; + played_at = dayjs(pa); + playDateCompleted = played_at; artists = art; name = n; id = i; @@ -130,7 +134,8 @@ export default class SpotifySource extends MemorySource { } = {} } = item as TrackObjectFull; - played_at = dayjs(timestamp).toISOString(); + scrobbleTsSOC = SCROBBLE_TS_SOC_START; + played_at = dayjs(timestamp); artists = art; name = n; id = i; @@ -161,13 +166,15 @@ export default class SpotifySource extends MemorySource { album: albumName, track: name, duration: duration_ms / 1000, - playDate: dayjs(played_at), + playDate: played_at, + playDateCompleted }, meta: { deviceId: deviceId ?? `${NO_DEVICE}-${NO_USER}`, source: 'Spotify', trackId: id, trackProgressPosition: playbackPosition, + scrobbleTsSOC, newFromSource, url: { web: url @@ -306,7 +313,13 @@ export default class SpotifySource extends MemorySource { plays.push(currPlay); } } - return this.processRecentPlays(plays); + const newPlays = this.processRecentPlays(plays); + // hint that scrobble timestamp source of truth should be when the track ended (player changed tracks) + // rather than when we first saw the track + // + // this is because Spotify play history (getMyRecentlyPlayedTracks) timestamps based on end of play + // and when we backlog we want timestamps to be as accurate as possible + return newPlays.map(x => ({...x, meta: {...x.meta, scrobbleTsSOC: SCROBBLE_TS_SOC_END}})) } getPlayHistory = async (options: RecentlyPlayedOptions = {}) => { diff --git a/src/backend/utils/TimeUtils.ts b/src/backend/utils/TimeUtils.ts index 43d21a66..9a83b84d 100644 --- a/src/backend/utils/TimeUtils.ts +++ b/src/backend/utils/TimeUtils.ts @@ -1,5 +1,5 @@ import { - PlayObject, + PlayObject, SCROBBLE_TS_SOC_END, SCROBBLE_TS_SOC_START, ScrobbleTsSOC, TA_CLOSE, TA_EXACT, TA_FUZZY, @@ -16,6 +16,7 @@ import { import {formatNumber} from "../utils"; import {ScrobbleThresholds} from "../common/infrastructure/config/source"; import {capitalize} from "../../core/StringUtils"; +import dayjs, {Dayjs} from "dayjs"; export const temporalPlayComparisonSummary = (data: TemporalPlayComparison, existingPlay?: PlayObject, candidatePlay?: PlayObject) => { const parts: string[] = []; @@ -60,24 +61,34 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P const { meta: { source, + //scrobbleTsSOC: existingScrobbleTsSOC = SCROBBLE_TS_SOC_START, }, data: { - playDate: existingPlayDate, + // playDate: existingPlayDate, + // playDateCompleted: existingPlayDateCompleted, duration: existingDuration, listenRanges: existingRanges, listenedFor: existingListenedFor, } } = existingPlay; + const [existingTsSOCDate, existingTsSOC] = getScrobbleTsSOCDateWithContext(existingPlay); + const { + // meta: { + // scrobbleTsSOC: candidateScrobbleTsSOC = SCROBBLE_TS_SOC_START, + // }, data: { - playDate: newPlayDate, + // playDate: newPlayDate, + // playDateCompleted: candidatePlayDateCompleted, duration: newDuration, listenRanges: newRanges, listenedFor: newListenedFor, } } = candidatePlay; + const [candidateTsSOCDate, candidateTsSOC] = getScrobbleTsSOCDateWithContext(candidatePlay); + const { diffThreshold = lowGranularitySources.some(x => x.toLocaleLowerCase() === source) ? 60 : 10, fuzzyDuration = false, @@ -85,7 +96,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P } = options; // cant compare! - if (existingPlayDate === undefined || newPlayDate === undefined) { + if (existingTsSOCDate === undefined || candidateTsSOCDate === undefined) { return result; } @@ -95,7 +106,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P let playDiffThreshold = diffThreshold; // check if existing play time is same as new play date - let scrobblePlayDiff = Math.abs(existingPlayDate.unix() - newPlayDate.unix()); + let scrobblePlayDiff = Math.abs(existingTsSOCDate.unix() - candidateTsSOCDate.unix()); result.date = { threshold: diffThreshold, diff: scrobblePlayDiff @@ -112,7 +123,7 @@ export const comparePlayTemporally = (existingPlay: PlayObject, candidatePlay: P // we can check if the new track play date took place while the existing one was being listened to // which would indicate (assuming same source) the new track is a duplicate for (const range of existingRanges) { - if (newPlayDate.isBetween(range.start.timestamp, range.end.timestamp)) { + if (candidateTsSOCDate.isBetween(range.start.timestamp, range.end.timestamp)) { result.range = range; if(!temporalAccuracyIsAtLeast(TA_CLOSE, result.match)) { result.match = TA_CLOSE; @@ -197,3 +208,25 @@ export const temporalAccuracyToString = (acc: TemporalAccuracy): string => { return 'no correlation'; } } + +export const getScrobbleTsSOCDateWithContext = (data: PlayObject): [Dayjs, ScrobbleTsSOC] => { + const { + meta: { + scrobbleTsSOC = SCROBBLE_TS_SOC_START, + }, + data: { + playDate = dayjs(), + playDateCompleted + } + } = data; + + if(scrobbleTsSOC === SCROBBLE_TS_SOC_END && playDateCompleted !== undefined) { + return [playDateCompleted, SCROBBLE_TS_SOC_END]; + } + return [playDate, SCROBBLE_TS_SOC_START]; +} + +export const getScrobbleTsSOCDate = (data: PlayObject): Dayjs => { + const [date, _] = getScrobbleTsSOCDateWithContext(data); + return date; +} diff --git a/src/core/Atomic.ts b/src/core/Atomic.ts index 45dad801..35e8a03f 100644 --- a/src/core/Atomic.ts +++ b/src/core/Atomic.ts @@ -37,7 +37,7 @@ export interface TrackStringOptions { transformers?: { artists?: (a: string[]) => T | string track?: (t: string,data: AmbPlayObject, hasExistingParts?: boolean) => T | string - time?: (t: Dayjs) => T | string + time?: (t: Dayjs, i?: ScrobbleTsSOC) => T | string timeFromNow?: (t: Dayjs) => T | string reducer?: (arr: (T | string)[]) => T //(acc: T, curr: T | string) => T } @@ -83,6 +83,7 @@ export interface PlayData extends TrackData { /** Number of seconds the track was listened to */ listenedFor?: number listenRanges?: ListenRangeData[] + playDateCompleted?: Dayjs | string } export interface PlayMeta { @@ -122,9 +123,16 @@ export interface PlayMeta { nowPlaying?: boolean + scrobbleTsSOC?: ScrobbleTsSOC + [key: string]: any } +export type ScrobbleTsSOC = 1 | 2; + +export const SCROBBLE_TS_SOC_START: ScrobbleTsSOC = 1; +export const SCROBBLE_TS_SOC_END: ScrobbleTsSOC = 2; + export interface AmbPlayObject { data: PlayData, meta: PlayMeta @@ -140,10 +148,12 @@ export interface JsonPlayObject extends AmbPlayObject { export interface ObjectPlayData extends PlayData { playDate?: Dayjs + playDateCompleted?: Dayjs } export interface JsonPlayData extends PlayData { playDate?: Dayjs + playDateCompleted?: Dayjs } export type LogLevel = "error" | "warn" | "info" | "verbose" | "debug"; diff --git a/src/core/StringUtils.ts b/src/core/StringUtils.ts index f8d03b3b..543896f0 100644 --- a/src/core/StringUtils.ts +++ b/src/core/StringUtils.ts @@ -4,7 +4,7 @@ import isBetween from "dayjs/plugin/isBetween.js"; import relativeTime from "dayjs/plugin/relativeTime.js"; import duration from "dayjs/plugin/duration.js"; import timezone from "dayjs/plugin/timezone.js"; -import {AmbPlayObject, TrackStringOptions} from "./Atomic"; +import {AmbPlayObject, SCROBBLE_TS_SOC_END, SCROBBLE_TS_SOC_START, ScrobbleTsSOC, TrackStringOptions} from "./Atomic"; import {str} from "ajv"; dayjs.extend(utc) @@ -28,7 +28,7 @@ export const truncateStringToLength = (length: any, truncStr = '...') => (val: a export const defaultTrackTransformer = (input: any, data: AmbPlayObject, hasExistingParts: boolean = false) => hasExistingParts ? `- ${input}` : input; export const defaultReducer = (acc, curr) => `${acc} ${curr}`; export const defaultArtistFunc = (a: string[]) => a.join(' / '); -export const defaultTimeFunc = (t: Dayjs | undefined) => t === undefined ? '@ N/A' : `@ ${t.local().format()}`; +export const defaultTimeFunc = (t: Dayjs | undefined, i?: ScrobbleTsSOC) => t === undefined ? '@ N/A' : `@ ${t.local().format()} ${i === undefined ? '' : (i === SCROBBLE_TS_SOC_START ? '(S)' : '(C)')}`; export const defaultTimeFromNowFunc = (t: Dayjs | undefined) => t === undefined ? undefined : `(${t.local().fromNow()})`; export const defaultBuildTrackStringTransformers = { artists: defaultArtistFunc, @@ -52,14 +52,23 @@ export const buildTrackString = (playObj: AmbPlayObject, options: Tr artists, album, track, - playDate + playDate, + playDateCompleted } = {}, meta: { - trackId + trackId, + scrobbleTsSOC = SCROBBLE_TS_SOC_START } = {}, } = playObj; - const pd = typeof playDate === 'string' ? dayjs(playDate) : playDate; + let pd: Dayjs; + let usedTsSOC: ScrobbleTsSOC = scrobbleTsSOC; + if(scrobbleTsSOC === SCROBBLE_TS_SOC_END && playDateCompleted !== undefined) { + pd = typeof playDateCompleted === 'string' ? dayjs(playDateCompleted) : playDateCompleted; + } else { + usedTsSOC = SCROBBLE_TS_SOC_START; + pd = typeof playDate === 'string' ? dayjs(playDate) : playDate; + } const strParts: (T | string)[] = []; if (include.includes('trackId') && trackId !== undefined) { @@ -72,7 +81,7 @@ export const buildTrackString = (playObj: AmbPlayObject, options: Tr strParts.push(trackFunc(track, playObj, strParts.length > 0)); } if (include.includes('time')) { - strParts.push(timeFunc(pd)); + strParts.push(timeFunc(pd, usedTsSOC)); } if (include.includes('timeFromNow')) { const tfn = timeFromNow(pd); -- 2.51.2 From fd49d17de4301536c741d1fa505fa9f72f62d224 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 4 Jan 2024 08:49:14 -0500 Subject: [PATCH 27/33] Remove unused import --- src/core/StringUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/StringUtils.ts b/src/core/StringUtils.ts index 543896f0..0a2c817d 100644 --- a/src/core/StringUtils.ts +++ b/src/core/StringUtils.ts @@ -5,7 +5,6 @@ import relativeTime from "dayjs/plugin/relativeTime.js"; import duration from "dayjs/plugin/duration.js"; import timezone from "dayjs/plugin/timezone.js"; import {AmbPlayObject, SCROBBLE_TS_SOC_END, SCROBBLE_TS_SOC_START, ScrobbleTsSOC, TrackStringOptions} from "./Atomic"; -import {str} from "ajv"; dayjs.extend(utc) dayjs.extend(isBetween); -- 2.51.2 From 369e18b1c2058a3bb2455f54081d28918963a943 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 4 Jan 2024 10:57:03 -0500 Subject: [PATCH 28/33] feat(player): Handle state transfer for single-platform sources The assumption is that single platform sources (like spotify) can only play from one platform at a time. If they are smart (like Spotify) the "new" platform picks up where the old platform left off WRT playing music. This commit implements the same behavior for MS player state by transferring existing state from the existing (previous) platform to the newly discovered platform. --- src/backend/sources/MemorySource.ts | 10 +++- .../PlayerState/AbstractPlayerState.ts | 47 +++++++++++++++---- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/backend/sources/MemorySource.ts b/src/backend/sources/MemorySource.ts index 2b37532e..c467d2df 100644 --- a/src/backend/sources/MemorySource.ts +++ b/src/backend/sources/MemorySource.ts @@ -128,11 +128,17 @@ export default class MemorySource extends AbstractSource { const idStr = genGroupIdStr(id); if (!this.players.has(idStr)) { this.setNewPlayer(idStr, this.logger, id); + + if(!this.multiPlatform && this.players.size > 1) { + // new platform should have old platform data transferred + const [id,firstPlayer] = Array.from(this.players.entries())[0]; + const newPlayer = this.players.get(idStr); + firstPlayer.transferToNewPlayer(newPlayer); + this.deletePlayer(id); + } } } - //const deadPlatformIds: string[] = []; - for (const [key, player] of this.players.entries()) { let incomingData: PlayObject | PlayerStateData; diff --git a/src/backend/sources/PlayerState/AbstractPlayerState.ts b/src/backend/sources/PlayerState/AbstractPlayerState.ts index 05b64c29..61d21f67 100644 --- a/src/backend/sources/PlayerState/AbstractPlayerState.ts +++ b/src/backend/sources/PlayerState/AbstractPlayerState.ts @@ -12,6 +12,7 @@ import { ListenProgress } from "./ListenProgress"; import {PlayObject, Second, SourcePlayerObj} from "../../../core/Atomic"; import { buildTrackString } from "../../../core/StringUtils"; import {ListenRange} from "./ListenRange"; +import {id} from "common-tags"; export interface PlayerStateIntervals { staleInterval?: number @@ -122,11 +123,12 @@ export abstract class AbstractPlayerState { } if (this.currentPlay !== undefined) { - if (!playObjDataMatch(this.currentPlay, play)/* || (true !== false)*/) { // TODO check new play date and listen range to see if they intersect + const currentPlayMatches = playObjDataMatch(this.currentPlay, play); + if (!currentPlayMatches) { // TODO check new play date and listen range to see if they intersect this.logger.debug(`Incoming play state (${buildTrackString(play, {include: ['trackId', 'artist', 'track']})}) does not match existing state, removing existing: ${buildTrackString(this.currentPlay, {include: ['trackId', 'artist', 'track']})}`) this.currentListenSessionEnd(); const played = this.getPlayedObject(true); - this.setCurrentPlay(play, undefined, reportedTS); + this.setCurrentPlay(play, {reportedTS}); if (this.calculatedStatus !== CALCULATED_PLAYER_STATUSES.playing) { this.calculatedStatus = CALCULATED_PLAYER_STATUSES.unknown; } @@ -139,7 +141,7 @@ export abstract class AbstractPlayerState { this.currentListenSessionEnd(); const played = this.getPlayedObject(true); play.data.playDate = dayjs(); - this.setCurrentPlay(play, undefined, reportedTS); + this.setCurrentPlay(play, {reportedTS}); return [this.getPlayedObject(), played]; } else { if(this.currentListenRange !== undefined) { @@ -179,7 +181,7 @@ export abstract class AbstractPlayerState { this.currentListenSessionEnd(); } - getPlayedObject(completed: boolean = false): PlayObject | undefined { + public getPlayedObject(completed: boolean = false): PlayObject | undefined { if(this.currentPlay !== undefined) { let ranges = [...this.listenRanges]; if (this.currentListenRange !== undefined) { @@ -202,7 +204,7 @@ export abstract class AbstractPlayerState { return undefined; } - getListenDuration(): Second{ + public getListenDuration(): Second{ let listenDur: number = 0; let ranges = [...this.listenRanges]; if (this.currentListenRange !== undefined) { @@ -326,7 +328,14 @@ export abstract class AbstractPlayerState { return false; } - protected setCurrentPlay(play: PlayObject, status?: ReportedPlayerStatus, reportedTS?: Dayjs) { + protected setCurrentPlay(play: PlayObject, options?: CurrentPlayOptions) { + + const { + status, + reportedTS, + listenSessionManaged = true + } = options || {}; + this.currentPlay = play; this.playFirstSeenAt = dayjs(); this.listenRanges = []; @@ -338,12 +347,12 @@ export abstract class AbstractPlayerState { this.reportedStatus = status; } - if (!['stopped'].includes(this.reportedStatus)) { + if (listenSessionManaged && !['stopped'].includes(this.reportedStatus)) { this.currentListenSessionContinue(play.meta.trackProgressPosition, reportedTS); } } - textSummary() { + public textSummary() { let parts = ['']; let play: string; if (this.currentPlay !== undefined) { @@ -365,7 +374,7 @@ export abstract class AbstractPlayerState { return parts.join('\n'); } - logSummary() { + public logSummary() { this.logger.debug(this.textSummary()); } @@ -385,7 +394,7 @@ export abstract class AbstractPlayerState { return lastRange.end.position; } - getApiState(): SourcePlayerObj { + public getApiState(): SourcePlayerObj { return { platformId: this.platformIdStr, play: this.getPlayedObject(), @@ -402,4 +411,22 @@ export abstract class AbstractPlayerState { } } } + + public transferToNewPlayer(newPlayer: AbstractPlayerState) { + this.logger.debug(`Transferring state to new Player (${newPlayer.platformIdStr})`); + newPlayer.calculatedStatus = this.calculatedStatus; + if(this.currentPlay !== undefined) { + newPlayer.setCurrentPlay(this.currentPlay, {status: this.reportedStatus, listenSessionManaged: false}); + } + newPlayer.currentListenRange = this.currentListenRange; + newPlayer.listenRanges = this.listenRanges; + newPlayer.playFirstSeenAt = this.playFirstSeenAt; + newPlayer.playLastUpdatedAt = this.playLastUpdatedAt; + } +} + +export interface CurrentPlayOptions { + status?: ReportedPlayerStatus, + reportedTS?: Dayjs + listenSessionManaged?: boolean } -- 2.51.2 From 051dd5d128b606ecb8eb6d08f7956fb82476030a Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Thu, 4 Jan 2024 11:10:39 -0500 Subject: [PATCH 29/33] feat(player): Improved player deletion behavior * Consolidate playerDelete event emitter to deletePlayer function * Consolidate logging for player deletion to deletePlayer function * Add removal reason when transferring player state for single-platform sources --- src/backend/sources/MemorySource.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/backend/sources/MemorySource.ts b/src/backend/sources/MemorySource.ts index c467d2df..7328c2d9 100644 --- a/src/backend/sources/MemorySource.ts +++ b/src/backend/sources/MemorySource.ts @@ -57,14 +57,12 @@ export default class MemorySource extends AbstractSource { } cleanupPlayers = () => { - const deadPlatformIds: string[] = []; + const deadPlatformIds: [string, string?][] = []; for (const [key, player] of this.players.entries()) { // no communication from the source was received for this player const isStale = player.checkStale(); if (isStale && player.checkOrphaned() && player.isDead()) { - player.logger.debug(`Removed after being orphaned for ${dayjs.duration(player.stateIntervalOptions.orphanedInterval, 'seconds').asMinutes()} minutes`); - deadPlatformIds.push(player.platformIdStr); - this.emitEvent('playerDelete', {platformId: player.platformIdStr}); + deadPlatformIds.push([player.platformIdStr, `Removed after being orphaned for ${dayjs.duration(player.stateIntervalOptions.orphanedInterval, 'seconds').asMinutes()} minutes`]); } else if (isStale) { const state = player.getApiState(); // @ts-ignore @@ -78,8 +76,8 @@ export default class MemorySource extends AbstractSource { } } } - for (const deadId of deadPlatformIds) { - this.deletePlayer(deadId); + for (const [deadId, reason] of deadPlatformIds) { + this.deletePlayer(deadId, reason); } } @@ -107,9 +105,13 @@ export default class MemorySource extends AbstractSource { this.playerState.set(idStr, ''); } - deletePlayer = (id: string) => { + deletePlayer = (id: string, reason?: string) => { + if(reason !== undefined) { + this.players.get(id)?.logger.debug(reason); + } this.players.delete(id); this.playerState.delete(id); + this.emitEvent('playerDelete', {platformId: id}); } processRecentPlays = (datas: (PlayObject | PlayerStateData)[]) => { @@ -134,7 +136,7 @@ export default class MemorySource extends AbstractSource { const [id,firstPlayer] = Array.from(this.players.entries())[0]; const newPlayer = this.players.get(idStr); firstPlayer.transferToNewPlayer(newPlayer); - this.deletePlayer(id); + this.deletePlayer(id, 'Removed due to player transfer'); } } } -- 2.51.2 From 10fea9b23dddfdb87b9ad0b0fb7368ecb24e021f Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Fri, 5 Jan 2024 11:06:09 -0500 Subject: [PATCH 30/33] feat(source): Improve (re)start behavior and ui * Move (re)start to same location on status card as client and change verbage to be the same * Fix api behavior to use correct poll function so that backlog is triggered --- src/backend/server/api.ts | 4 +++- .../components/statusCard/SourceStatusCard.tsx | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/backend/server/api.ts b/src/backend/server/api.ts index 4d8be370..475939ac 100644 --- a/src/backend/server/api.ts +++ b/src/backend/server/api.ts @@ -372,8 +372,10 @@ export const setupApi = (app: ExpressWithAsync, logger: Logger, initialLogOutput source.logger.info('Source is already polling! Restarting polling...'); const stopRes = await source.tryStopPolling(); if(stopRes === true) { - source.startPolling(); + source.poll(); } + } else { + source.poll(); } }); diff --git a/src/client/components/statusCard/SourceStatusCard.tsx b/src/client/components/statusCard/SourceStatusCard.tsx index 5afb7124..e6b45378 100644 --- a/src/client/components/statusCard/SourceStatusCard.tsx +++ b/src/client/components/statusCard/SourceStatusCard.tsx @@ -41,6 +41,7 @@ const SourceStatusCard = (props: SourceStatusCardData) => { method: 'GET', }); },[data]); + let startSourceElement = null; if(data !== undefined) { const { @@ -63,16 +64,25 @@ const SourceStatusCard = (props: SourceStatusCardData) => { const discovered = (!hasAuth || authed) ? Tracks Discovered : Tracks Discovered; + if((!hasAuth || authed) && canPoll) { + startSourceElement =
{status === 'Polling' ? 'Restart' : 'Start'}
+ } + // TODO links body = (
{platformIds.map(x => )}
{discovered}: {tracksDiscovered}
- {canPoll && hasAuthInteraction ? (Re)authenticate and (re)start polling : null} - {canPoll && (!hasAuth || authed) ?
Restart Polling
: null} + {canPoll && hasAuthInteraction ? (Re)authenticate : null}
); } return ( - + {body} ); -- 2.51.2 From 9709d6348ac3dd61383595b598a862540ac7d93f Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Fri, 5 Jan 2024 11:15:30 -0500 Subject: [PATCH 31/33] refactor(source): Start consolidating isReady behavior --- src/backend/index.ts | 23 ++++------------------- src/backend/sources/AbstractSource.ts | 4 ++++ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/backend/index.ts b/src/backend/index.ts index 605a1163..62933796 100644 --- a/src/backend/index.ts +++ b/src/backend/index.ts @@ -121,25 +121,10 @@ const configDir = process.env.CONFIG_DIR || path.resolve(projectDir, `./config`) let anyNotReady = false; for (const source of scrobbleSources.sources.filter(x => x.canPoll === true)) { await sleep(1500); // stagger polling by 1.5 seconds so that log messages for each source don't get mixed up - switch (source.type) { - case 'spotify': - if ((source as SpotifySource).spotifyApi !== undefined) { - if ((source as SpotifySource).spotifyApi.getAccessToken() === undefined) { - anyNotReady = true; - } else { - (source as SpotifySource).poll(); - } - } - break; - case 'lastfm': - if(source.initialized === true) { - source.poll(); - } - break; - default: - if (source.poll !== undefined) { - source.poll(); - } + if(source.isReady()) { + source.poll(); + } else { + anyNotReady = true; } } if (anyNotReady) { diff --git a/src/backend/sources/AbstractSource.ts b/src/backend/sources/AbstractSource.ts index b5ce5fc1..0d3d41ad 100644 --- a/src/backend/sources/AbstractSource.ts +++ b/src/backend/sources/AbstractSource.ts @@ -123,6 +123,10 @@ export default abstract class AbstractSource implements Authenticatable { } } + public isReady() { + return this.initialized && !this.authGated(); + } + getRecentlyPlayed = async (options: RecentlyPlayedOptions = {}): Promise => { return []; } -- 2.51.2 From afb2e76b2d5104a0b4deff533e1e27399145140d Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Tue, 9 Jan 2024 11:50:42 -0500 Subject: [PATCH 32/33] feat: Improve track credits parsing to recognize post-feat suffixes * Refactor credit parsing into two stages: joiner separation and credits parsing * Break credit parsing into wrapped vs. non-wrapped for simpler regexes * Implement suffix matching after credits * Add tests for wrapped vs. non-wrapped credits and with suffixes --- package.json | 2 +- .../tests/listenbrainz/listenbrainz.test.ts | 5 +- src/backend/tests/utils/interfaces.ts | 5 + src/backend/tests/utils/playTestData.json | 188 ++++++++++++++++++ src/backend/tests/{ => utils}/strings.test.ts | 34 +++- src/backend/utils/StringUtils.ts | 86 +++++++- 6 files changed, 302 insertions(+), 18 deletions(-) create mode 100644 src/backend/tests/utils/interfaces.ts create mode 100644 src/backend/tests/utils/playTestData.json rename src/backend/tests/{ => utils}/strings.test.ts (75%) diff --git a/package.json b/package.json index e86d3b21..5e0a1d6a 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "typedoc": "typedoc", "circular": "madge --circular --extensions ts src/index.ts", "test": "react-scripts test", - "test:backend": "mocha --extension ts --reporter spec --recursive src/backend/tests/scrobbler/**/*.test.ts", + "test:backend": "mocha --extension ts --reporter spec --recursive src/backend/tests/**/*.test.ts", "eject": "react-scripts eject", "dev": "concurrently -p name -c \"yellow,magenta,blue\" -n \"webpack-server,nodemon-server,CRA\" \"npm run dev:server:webpack\" \"npm run dev:server:nodemon\" \"npm run dev:client\"", "dev:client": "BROWSER=none REACT_APP_VERSION=$npm_package_version react-scripts start", diff --git a/src/backend/tests/listenbrainz/listenbrainz.test.ts b/src/backend/tests/listenbrainz/listenbrainz.test.ts index 15897816..b47cd1eb 100644 --- a/src/backend/tests/listenbrainz/listenbrainz.test.ts +++ b/src/backend/tests/listenbrainz/listenbrainz.test.ts @@ -19,11 +19,8 @@ import dayjs from "dayjs"; import {withRequestInterception} from "../utils/networking"; import {http, HttpResponse} from "msw"; import {UpstreamError} from "../../common/errors/UpstreamError"; +import {ExpectedResults} from "../utils/interfaces"; -interface ExpectedResults { - artists: string[] - track: string -} interface LZTestFixture { data: ListenResponse expected: ExpectedResults diff --git a/src/backend/tests/utils/interfaces.ts b/src/backend/tests/utils/interfaces.ts new file mode 100644 index 00000000..90791a1c --- /dev/null +++ b/src/backend/tests/utils/interfaces.ts @@ -0,0 +1,5 @@ +export interface ExpectedResults { + artists: string[] + track: string + album?: String +} diff --git a/src/backend/tests/utils/playTestData.json b/src/backend/tests/utils/playTestData.json new file mode 100644 index 00000000..8ca16818 --- /dev/null +++ b/src/backend/tests/utils/playTestData.json @@ -0,0 +1,188 @@ +[ + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Eugene Naumenko" + ], + "track": "Take-Off feat. Diag", + "album": "1000 Years" + }, + "expected": { + "artists": [ + "Eugene Naumenko", + "Diag" + ], + "track": "Take-Off", + "album": "1000 Years" + } + }, + { + "caseHints": [ + "track", + "remix" + ], + "data": { + "artists": [ + "Djfredse" + ], + "track": "Anny Sky - Together With You (Djfredse Remix)" + }, + "expected": { + "artists": [ + "Djfredse", + "Anny Sky" + ], + "track": "Together With You (Djfredse Remix)" + } + }, + { + "caseHints": [ + "track", + "remix", + "joiner" + ], + "data": { + "artists": [ + "Chriss" + ], + "track": "Criminal mind Ft Akon (Remix Braquer vos têtes)", + "album": "Music Is Life" + }, + "expected": { + "artists": [ + "Chriss", + "Akon" + ], + "track": "Criminal mind (Remix Braquer vos têtes)", + "album": "Music Is Life" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Luis Zunel" + ], + "track": "Reset my Blues (Featuring Peergynt Lobogris)" + }, + "expected": { + "artists": [ + "Luis Zunel", + "Peergynt Lobogris" + ], + "track": "Reset my Blues" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Woochia" + ], + "track": "La Flaque (feat.Ecstatik)" + }, + "expected": { + "artists": [ + "Woochia", + "Ecstatik" + ], + "track": "La Flaque" + } + }, + { + "caseHints": [ + "track", + "joiner", + "remix" + ], + "data": { + "artists": [ + "Alicia Keys", + "Kaash Paige", + "Diamond Platnumz" + ], + "track": "Wasted Energy (feat. Kaash Paige & Diamond Platnumz) - Remix" + }, + "expected": { + "artists": [ + "Alicia Keys", + "Kaash Paige", + "Diamond Platnumz" + ], + "track": "Wasted Energy - Remix" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Tyler, the Creator" + ], + "track": "WUSYANAME (feat. Youngboy Never Broke Again & Ty Dolla $ign)" + }, + "expected": { + "artists": [ + "Tyler, the Creator", + "Youngboy Never Broke Again", + "Ty Dolla $ign" + ], + "track": "WUSYANAME" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Tyler, the Creator" + ], + "track": "Trashwang (feat. Na' kel, Jasper Dolphin, Lucas & L-Boy)" + }, + "expected": { + "artists": [ + "Tyler, the Creator", + "Na' kel", + "Jasper Dolphin", + "L-Boy", + "Lucas" + ], + "track": "Trashwang" + } + }, + { + "caseHints": [ + "track", + "joiner" + ], + "data": { + "artists": [ + "Childish Gambino" + ], + "track": "12.38 (feat. 21 Savage, Ink & Kadhja Bonet)" + }, + "expected": { + "artists": [ + "Childish Gambino", + "21 Savage", + "Ink", + "Kadhja Bonet" + ], + "track": "12.38" + } + } +] diff --git a/src/backend/tests/strings.test.ts b/src/backend/tests/utils/strings.test.ts similarity index 75% rename from src/backend/tests/strings.test.ts rename to src/backend/tests/utils/strings.test.ts index 626844d6..912dbb38 100644 --- a/src/backend/tests/strings.test.ts +++ b/src/backend/tests/utils/strings.test.ts @@ -1,7 +1,19 @@ import {describe, it} from 'mocha'; import {assert} from 'chai'; -import {compareNormalizedStrings} from "../utils/StringUtils"; - +import {compareNormalizedStrings, parseTrackCredits, uniqueNormalizedStrArr} from "../../utils/StringUtils"; +import testData from './playTestData.json'; +import {ExpectedResults} from "./interfaces"; +import {intersect} from "../../utils"; + +interface PlayTestFixture { + caseHints: string[] + data: { + track: string + artists: string[] + album?: string + } + expected: ExpectedResults +} describe('String Comparisons', function () { @@ -95,3 +107,21 @@ describe('String Comparisons', function () { assert.equal( result1.highScore, result2.highScore, `Comparing: '${longerString}' | '${shorterString}'`); }); }); + +describe('Play Strings',function () { + + const testFixtures = testData as unknown as PlayTestFixture[]; + const joinerData = testFixtures.filter(x => intersect(['joiner','track'], x.caseHints).length === 2); + + it('should parse joiners from track title', function() { + for(const test of joinerData) { + const res = parseTrackCredits(test.data.track); + let artists: string[] = [...test.data.artists]; + if(res.secondary !== undefined) { + artists = uniqueNormalizedStrArr([...artists, ...res.secondary]); + } + assert.equal(res.primaryComposite, test.expected.track); + assert.sameDeepMembers(artists, test.expected.artists); + } + }); +}); diff --git a/src/backend/utils/StringUtils.ts b/src/backend/utils/StringUtils.ts index 8451635d..922dad4c 100644 --- a/src/backend/utils/StringUtils.ts +++ b/src/backend/utils/StringUtils.ts @@ -31,14 +31,59 @@ export const normalizeStr = (str: string, options?: {keepSingleWhitespace?: bool export interface PlayCredits { primary: string + primaryComposite: string secondary?: string[] + suffix?: string } +/** + * Matches if the secondary string is wrapped in parenthesis-like symbols. Returns joiner, credits, and + * suffix = if anything appears after end of wrapped string + * + * EX (feat. Kaash Paige & Diamond Platnumz) - Remix + * !!!! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ******* + * + * + * */ +export const SECONDARY_CAPTURED_REGEX = new RegExp(/[(\[]\s*(?ft\.?\W|feat\.?\W|featuring|vs\.?\W)\s*(?.*)[)\]](?.*)/i); + + +/** + * Matches if the secondary string is NOT wrapped in parenthesis-like symbols. Returns joiner, credits, and + * suffix = if anything appears wrapped or starting with " - " proceeding string appearing after joiner + * + * EX feat. Diag + * !!!! ^^^^ + * + * Ft Akon, Paige & Djfredse (Remix Braquer vos têtes) + * !! ^^^^^^^^^^^^^^^^^^^^^^ ************************* + * + * feat. Kaash Paige & Diamond Platnumz - Remix + * !!!! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ******* + * + * */ +export const SECONDARY_FREE_REGEX = new RegExp(/^\s*(?ft\.?\W|feat\.?\W|featuring|vs\.?\W)\s*(?(?:.+?(?= - |\s*[(\[]))|(?:.*))(?.*)/i); + +const SECONDARY_REGEX_STRATS: RegExp[] = [SECONDARY_CAPTURED_REGEX, SECONDARY_FREE_REGEX]; + +/** + * Matches JUST primary and secondary sections separated by a REQUIRED joiner that can optionally be wrapped in parenthesis-like symbols + * + * EX + * Criminal mind Ft Akon (Remix Braquer vos têtes) + * ^^^^^^^^^^^^^********************************** + * + * Wasted Energy (feat. Kaash Paige & Diamond Platnumz) - Remix + * ^^^^^^^^^^^^^^********************************************** + * + * */ +export const PRIMARY_SECONDARY_SECTIONS_REGEX = new RegExp(/^(?.+?)(?(?:[(\[]?(?:\Wft\.?|\Wfeat\.?|featuring|\Wvs\.)).*)/i); + /** * For matching the most common track/artist pattern that has a joiner * * Primary ft. 2nd Artist, 3rd Artist - * Primary (2nd Artist) + * Primary (2nd Artist) - Suffix * Primary [featuring 2nd Artist] * * ____ @@ -49,28 +94,44 @@ export interface PlayCredits { * => MUST begin with joiner ft. feat. featuring with vs. * => May have closing character ) ] * */ -export const SECONDARY_ARTISTS_SECTION_REGEX = new RegExp(/^(?[^(\[]*)?(?[(\[]?(?\Wft\.?|\Wfeat\.?|featuring|\Wvs\.?) (?[^)\]]*)(?:[)\]]|\s*)$)/i); // export const SECONDARY_ARTISTS_REGEX = new RegExp(//ig); export const parseCredits = (str: string, delimiters?: boolean | string[]): PlayCredits => { if (str.trim() === '') { return undefined; } + let primary: string | undefined; let secondary: string[] = []; - const results = parseRegexSingleOrFail(SECONDARY_ARTISTS_SECTION_REGEX, str); - if (results !== undefined) { - primary = results.named.primary !== undefined ? results.named.primary.trim() : undefined; + let suffix: string | undefined; + const results = parseRegexSingleOrFail(PRIMARY_SECONDARY_SECTIONS_REGEX, str); + if(results !== undefined) { + let delims: string[] | undefined; if (Array.isArray(delimiters)) { delims = delimiters; } else if (delimiters === false) { delims = []; } - secondary = parseStringList(results.named.secondaryArtists as string, delims) + + primary = results.named.primary.trim(); + for(const strat of SECONDARY_REGEX_STRATS) { + const secCredits = parseRegexSingleOrFail(strat, results.named.secondary); + if(secCredits !== undefined) { + secondary = parseStringList(secCredits.named.credits as string, delims) + suffix = secCredits.named.creditsSuffix; + break; + } + } + if(secondary === undefined) { + // uh oh, this shouldn't have happened! Return nothing since we don't know how to parse this + return undefined; + } return { primary, - secondary - }; + primaryComposite: `${primary}${suffix ?? ''}`, + secondary, + suffix + } } return undefined; } @@ -92,6 +153,7 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (primaries.length > 1) { return { primary: primaries[0], + primaryComposite: primaries[0], secondary: primaries.slice(1).concat(withJoiner.secondary) } } @@ -102,11 +164,13 @@ export const parseArtistCredits = (str: string, delimiters?: boolean | string[]) if (artists.length > 1) { return { primary: artists[0], + primaryComposite: artists[0], secondary: artists.slice(1) } } return { - primary: artists[0] + primary: artists[0], + primaryComposite: artists[0], } } export const parseTrackCredits = (str: string, delimiters?: boolean | string[]): PlayCredits | undefined => parseCredits(str, delimiters); @@ -150,10 +214,10 @@ export const compareScrobbleTracks = (existing: PlayObject, candidate: PlayObjec // try to remove any joiners based on existing artists const existingCredits = parseTrackCredits(existingTrack); - const existingPrimary = existingCredits !== undefined ? existingCredits.primary : existingTrack; + const existingPrimary = existingCredits !== undefined ? existingCredits.primaryComposite : existingTrack; const candidateCredits = parseTrackCredits(candidateTrack); - const candidatePrimary = candidateCredits !== undefined ? candidateCredits.primary : candidateTrack; + const candidatePrimary = candidateCredits !== undefined ? candidateCredits.primaryComposite : candidateTrack; // take whichever score is higher const creditsCleanedTrackSameness = compareNormalizedStrings(existingPrimary, candidatePrimary); -- 2.51.2 From a6ddc2b9f57a2354761d80ca40f4e9949291e982 Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 10 Jan 2024 08:45:10 -0500 Subject: [PATCH 33/33] chore: Bump version for release --- flatpak/io.github.foxxmd.multiscrobbler.metainfo.xml | 1 + package-lock.json | 2 +- package.json | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/flatpak/io.github.foxxmd.multiscrobbler.metainfo.xml b/flatpak/io.github.foxxmd.multiscrobbler.metainfo.xml index 8794c0d9..2acbd3c5 100644 --- a/flatpak/io.github.foxxmd.multiscrobbler.metainfo.xml +++ b/flatpak/io.github.foxxmd.multiscrobbler.metainfo.xml @@ -43,6 +43,7 @@ + diff --git a/package-lock.json b/package-lock.json index b795f0e5..24fe9a8c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "multi-scrobbler", - "version": "0.6.2", + "version": "0.6.3", "lockfileVersion": 3, "requires": true, "packages": { diff --git a/package.json b/package.json index 5e0a1d6a..6b9e4268 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "multi-scrobbler", - "version": "0.6.2", + "version": "0.6.3", "description": "scrobble plays from multiple sources to multiple clients", "scripts": { "schema": "npm run -s schema-aio & npm run -s schema-source & npm run -s schema-client & npm run -s schema-aiosource & npm run -s schema-aioclient",