From 57a8e48d3a182fb413c31d3309b721f160292f5a Mon Sep 17 00:00:00 2001 From: FoxxMD Date: Wed, 16 Oct 2024 13:55:05 +0000 Subject: [PATCH] fix(jellyfin): Fix memory player hanging on end of state Was not passing stopped state due to undefined play but need to do this to let last play actually be scrobbled. Refactor jellyfin session validation and types for MemorySource to correctly allow processing player state with no plays --- src/backend/common/infrastructure/Atomic.ts | 4 + src/backend/sources/JellyfinApiSource.ts | 98 ++++++++++++--------- src/backend/sources/MemorySource.ts | 18 +++- src/backend/tests/jellyfin/jellyfin.test.ts | 51 +++++++---- src/backend/utils.ts | 6 +- 5 files changed, 114 insertions(+), 63 deletions(-) diff --git a/src/backend/common/infrastructure/Atomic.ts b/src/backend/common/infrastructure/Atomic.ts index 4bd75eeb..9af851fc 100644 --- a/src/backend/common/infrastructure/Atomic.ts +++ b/src/backend/common/infrastructure/Atomic.ts @@ -126,6 +126,10 @@ export const asPlayerStateData = (obj: object): obj is PlayerStateData => { return 'platformId' in obj && 'play' in obj; } +export const asPlayerStateDataMaybePlay = (obj: object): obj is PlayerStateDataMaybePlay => { + return 'platformId' in obj; +} + export interface FormatPlayObjectOptions { newFromSource?: boolean parsedFrom?: string diff --git a/src/backend/sources/JellyfinApiSource.ts b/src/backend/sources/JellyfinApiSource.ts index 74c7871f..e0f6bc73 100644 --- a/src/backend/sources/JellyfinApiSource.ts +++ b/src/backend/sources/JellyfinApiSource.ts @@ -48,7 +48,7 @@ import { PlayPlatformId, REPORTED_PLAYER_STATUSES } from "../common/infrastructure/Atomic.js"; import { JellyApiSourceConfig } from "../common/infrastructure/config/source/jellyfin.js"; -import { combinePartsToString, parseBool, } from "../utils.js"; +import { combinePartsToString, genGroupIdStr, getPlatformIdFromData, parseBool, } from "../utils.js"; import { parseArrayFromMaybeString } from "../utils/StringUtils.js"; import MemorySource from "./MemorySource.js"; import { PlayerStateOptions } from "./PlayerState/AbstractPlayerState.js"; @@ -261,55 +261,64 @@ export default class JellyfinApiSource extends MemorySource { } } - isActivityValid = (play: PlayObject, session: SessionInfo): boolean | string => { - if(this.usersAllow.length > 0 && !this.usersAllow.includes(play.meta.user.toLocaleLowerCase())) { - return `'usersAllow does not include user ${play.meta.user}`; + isActivityValid = (state: PlayerStateDataMaybePlay, session: SessionInfo): boolean | string => { + if(this.usersAllow.length > 0 && !this.usersAllow.includes(state.platformId[1].toLocaleLowerCase())) { + return `'usersAllow does not include user ${state.platformId[1]}`; } - if(this.usersBlock.length > 0 && this.usersBlock.includes(play.meta.user.toLocaleLowerCase())) { - return `'usersBlock includes user ${play.meta.user}`; + if(this.usersBlock.length > 0 && this.usersBlock.includes(state.platformId[1].toLocaleLowerCase())) { + return `'usersBlock includes user ${state.platformId[1]}`; } - if(this.devicesAllow.length > 0 && !this.devicesAllow.some(x => play.meta.deviceId.toLocaleLowerCase().includes(x))) { - return `'devicesAllow does not include a phrase found in ${play.meta.deviceId}`; + if(this.devicesAllow.length > 0 && !this.devicesAllow.some(x => state.platformId[0].toLocaleLowerCase().includes(x))) { + return `'devicesAllow does not include a phrase found in ${state.platformId[0]}`; } - if(this.devicesBlock.length > 0 && this.devicesBlock.some(x => play.meta.deviceId.toLocaleLowerCase().includes(x))) { - return `'devicesBlock includes a phrase found in ${play.meta.deviceId}`; + if(this.devicesBlock.length > 0 && this.devicesBlock.some(x => state.platformId[0].toLocaleLowerCase().includes(x))) { + return `'devicesBlock includes a phrase found in ${state.platformId[0]}`; } - const allowedLibraries = this.getAllowedLibraries(); - if(allowedLibraries.length > 0 && !allowedLibraries.map(x => x.paths).flat(1).some(x => session.NowPlayingItem.Path.includes(x))) { - return `media not included in librariesAllow`; - } - - if(allowedLibraries.length === 0) { - const blockedLibraries = this.getBlockedLibraries(); - if(blockedLibraries.length > 0) { - const blockedLibrary = blockedLibraries.find(x => x.paths.some(y => session.NowPlayingItem.Path.includes(y))); - if(blockedLibrary !== undefined) { - return `media included in librariesBlock '${blockedLibrary.name}'`; + + if(session.NowPlayingItem !== undefined) { + const allowedLibraries = this.getAllowedLibraries(); + if(allowedLibraries.length > 0 && !allowedLibraries.map(x => x.paths).flat(1).some(x => session.NowPlayingItem.Path.includes(x))) { + return `media not included in librariesAllow`; + } + + if(allowedLibraries.length === 0) { + const blockedLibraries = this.getBlockedLibraries(); + if(blockedLibraries.length > 0) { + const blockedLibrary = blockedLibraries.find(x => x.paths.some(y => session.NowPlayingItem.Path.includes(y))); + if(blockedLibrary !== undefined) { + return `media included in librariesBlock '${blockedLibrary.name}'`; + } + } + + if(!this.getValidLibraries().map(x => x.paths).flat(1).some(x => session.NowPlayingItem.Path.includes(x))) { + return `media not included in a valid library`; } } + } - if(!this.getValidLibraries().map(x => x.paths).flat(1).some(x => session.NowPlayingItem.Path.includes(x))) { - return `media not included in a valid library`; + if(state.play !== undefined) { + if(state.play.meta.mediaType !== MediaType.Audio + && (state.play.meta.mediaType !== MediaType.Unknown + || state.play.meta.mediaType === MediaType.Unknown && !this.config.data.allowUnknown + ) + ) { + return `media detected as ${state.play.meta.mediaType} (MediaType) is not allowed`; } } - if(play.meta.mediaType !== MediaType.Audio - && (play.meta.mediaType !== MediaType.Unknown - || play.meta.mediaType === MediaType.Unknown && !this.config.data.allowUnknown - ) - ) { - return `media detected as ${play.meta.mediaType} (MediaType) is not allowed`; - } - if('ExtraType' in session.NowPlayingItem && session.NowPlayingItem.ExtraType === 'ThemeSong'/* - || play.data.track === 'theme' && - (play.data.artists === undefined || play.data.artists.length === 0) */) { - return `media detected as a ThemeSong (ExtraType) is not allowed`; - } - if(session.NowPlayingItem.Type !== 'Audio') { - return `media detected as a ${session.NowPlayingItem.Type} (Type) is not allowed`; + if(session.NowPlayingItem !== undefined) { + if('ExtraType' in session.NowPlayingItem && session.NowPlayingItem.ExtraType === 'ThemeSong'/* + || play.data.track === 'theme' && + (play.data.artists === undefined || play.data.artists.length === 0) */) { + return `media detected as a ThemeSong (ExtraType) is not allowed`; + } + if(session.NowPlayingItem.Type !== 'Audio') { + return `media detected as a ${session.NowPlayingItem.Type} (Type) is not allowed`; + } } + return true; } @@ -359,15 +368,22 @@ export default class JellyfinApiSource extends MemorySource { const nonMSSessions = sessions.data .filter(x => x.DeviceId !== this.deviceId) .map(x => [this.sessionToPlayerState(x), x]) - .filter((x: [PlayerStateDataMaybePlay, SessionInfo]) => x[0].play !== undefined) as [PlayerStateData, SessionInfo][]; - const validSessions: PlayerStateData[] = []; + .filter((x: [PlayerStateDataMaybePlay, SessionInfo]) => { + return x[0].play !== undefined + || this.hasPlayer(x[0]); + }) as [PlayerStateDataMaybePlay, SessionInfo][]; + const validSessions: PlayerStateDataMaybePlay[] = []; for(const sessionData of nonMSSessions) { - const validPlay = this.isActivityValid(sessionData[0].play, sessionData[1]); + const validPlay = this.isActivityValid(sessionData[0], sessionData[1]); if(validPlay === true) { validSessions.push(sessionData[0]); } else if(this.logFilterFailure !== false) { - this.logger[this.logFilterFailure](`Player State for -> ${buildTrackString(sessionData[0].play, {include: ['artist', 'track', 'platform']})} <-- is being dropped because ${validPlay}`); + let stateIdentifyingInfo: string = genGroupIdStr(getPlatformIdFromData(sessionData[0])); + if(sessionData[0].play !== undefined) { + stateIdentifyingInfo = buildTrackString(sessionData[0].play, {include: ['artist', 'track', 'platform']}); + } + this.logger[this.logFilterFailure](`Player State for -> ${stateIdentifyingInfo} <-- is being dropped because ${validPlay}`); } } return this.processRecentPlays(validSessions); diff --git a/src/backend/sources/MemorySource.ts b/src/backend/sources/MemorySource.ts index 30dc5ee7..0b52b063 100644 --- a/src/backend/sources/MemorySource.ts +++ b/src/backend/sources/MemorySource.ts @@ -7,9 +7,11 @@ import { PlayObject, SOURCE_SOT, SOURCE_SOT_TYPES, SourcePlayerObj } from "../.. import { buildTrackString } from "../../core/StringUtils.js"; import { asPlayerStateData, + asPlayerStateDataMaybePlay, CALCULATED_PLAYER_STATUSES, InternalConfig, PlayerStateData, + PlayerStateDataMaybePlay, PlayPlatformId, ProgressAwarePlayObject, SourceType, @@ -98,6 +100,16 @@ export default class MemorySource extends AbstractSource { this.playerState.set(idStr, ''); } + hasPlayer = (data: string | PlayerStateDataMaybePlay): boolean => { + let id: string; + if(typeof data === 'string') { + id = data; + } else { + id = genGroupIdStr(getPlatformIdFromData(data)); + } + return this.players.has(id); + } + deletePlayer = (id: string, reason?: string) => { if(!this.players.has(id)) { return; @@ -110,7 +122,7 @@ export default class MemorySource extends AbstractSource { this.emitEvent('playerDelete', {platformId: id}); } - processRecentPlays = (datas: (PlayObject | PlayerStateData)[]) => { + processRecentPlays = (datas: (PlayObject | PlayerStateDataMaybePlay)[]) => { const { options: { @@ -139,7 +151,7 @@ export default class MemorySource extends AbstractSource { for (const [key, player] of this.players.entries()) { - let incomingData: PlayObject | PlayerStateData; + let incomingData: PlayObject | PlayerStateDataMaybePlay; // get all incoming datas relevant for each player (this should only be one) const relevantDatas = datas.filter(x => { const id = getPlatformIdFromData(x); @@ -155,7 +167,7 @@ export default class MemorySource extends AbstractSource { } incomingData = relevantDatas[0]; - const [currPlay, prevPlay] = asPlayerStateData(incomingData) ? player.setState(incomingData.status, incomingData.play) : player.setState(undefined, incomingData); + const [currPlay, prevPlay] = asPlayerStateDataMaybePlay(incomingData) ? player.setState(incomingData.status, incomingData.play) : player.setState(undefined, incomingData); const candidate = prevPlay !== undefined ? prevPlay : currPlay; const playChanged = prevPlay !== undefined; diff --git a/src/backend/tests/jellyfin/jellyfin.test.ts b/src/backend/tests/jellyfin/jellyfin.test.ts index f14918a3..b9cddd27 100644 --- a/src/backend/tests/jellyfin/jellyfin.test.ts +++ b/src/backend/tests/jellyfin/jellyfin.test.ts @@ -15,6 +15,7 @@ import { // @ts-expect-error weird typings? SessionInfo, } from "@jellyfin/sdk/lib/generated-client/index.js"; +import { PlayerStateDataMaybePlay } from "../../common/infrastructure/Atomic.js"; const dataAsFixture = (data: any): TestFixture => { return data as TestFixture; @@ -36,8 +37,24 @@ const createJfApi = (data: JellyApiData): JellyfinApiSource => { const defaultJfApiCreds = {url: 'http://example.com', user: 'MyUser', apiKey: '1234'}; -const validPlay = generatePlay({}, {mediaType: 'Audio', user: 'MyUser', deviceId: '1234'}); -const playWithMeta = (meta: PlayMeta): PlayObject => ({...validPlay, meta: {...validPlay.meta, ...meta}}); +const validPlayerState: PlayerStateDataMaybePlay = { + platformId: ['1234', 'MyUser'], + play: generatePlay({}, {mediaType: 'Audio', user: 'MyUser', deviceId: '1234'}) +} +const playWithMeta = (meta: PlayMeta): PlayerStateDataMaybePlay => { + const {user, deviceId} = meta; + const platformId = validPlayerState.platformId; + return { + ...validPlayerState, + platformId: [deviceId ?? platformId[0], user ?? platformId[1]], + play: { + ...validPlayerState.play, + meta: { + ...validPlayerState.play?.meta, + ...meta + } + } +}}// ({...validPlayerState, meta: {...validPlayerState.meta, ...meta}}); const nowPlayingSession = (data: object): SessionInfo => ({...validSession, NowPlayingItem: {...validSession.NowPlayingItem, ...data}}); @@ -124,7 +141,7 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); expect(jf.isActivityValid(playWithMeta({user: 'SomeOtherUser'}), validSession)).to.not.be.true; - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; expect(jf.isActivityValid(playWithMeta({user: 'myuser'}), validSession)).to.be.true; await jf.destroy(); }); @@ -134,7 +151,7 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); expect(jf.isActivityValid(playWithMeta({user: 'BadUser'}), validSession)).to.not.be.true; - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; expect(jf.isActivityValid(playWithMeta({user: 'myuser'}), validSession)).to.be.true; await jf.destroy(); }); @@ -143,7 +160,7 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds, devicesAllow: ['WebPlayer']}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, validSession)).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.not.be.true; expect(jf.isActivityValid(playWithMeta({deviceId: 'WebPlayer'}), validSession)).to.be.true; await jf.destroy(); }); @@ -152,7 +169,7 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds, devicesBlock: ['WebPlayer']}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; expect(jf.isActivityValid(playWithMeta({deviceId: 'WebPlayer'}), validSession)).to.not.be.true; await jf.destroy(); }); @@ -162,7 +179,7 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); jf.libraries.push({name: 'CoolVideos', paths: ['/data/someOtherFolder'], collectionType: 'musicvideos'}); - expect(jf.isActivityValid(validPlay, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; await jf.destroy(); }); @@ -170,8 +187,8 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds, librariesAllow: ['music']}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; - expect(jf.isActivityValid(validPlay, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.not.be.true; await jf.destroy(); }); @@ -180,8 +197,8 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); jf.libraries.push({name: 'CoolVideos', paths: ['/data/someOtherFolder'], collectionType: 'musicvideos'}); - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; - expect(jf.isActivityValid(validPlay, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; await jf.destroy(); }); @@ -190,8 +207,8 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); jf.libraries.push({name: 'CoolMusic', paths: ['/data/someOtherFolder'], collectionType: 'music'}); - expect(jf.isActivityValid(validPlay, validSession)).to.not.be.true; - expect(jf.isActivityValid(validPlay, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.be.true; await jf.destroy(); }); @@ -203,7 +220,7 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, validSession)).to.be.true; + expect(jf.isActivityValid(validPlayerState, validSession)).to.be.true; await jf.destroy(); }); @@ -212,7 +229,7 @@ describe("Jellyfin API Source", function() { await jf.buildInitData(); jf.libraries.push({name: 'CoolVideos', paths: ['/data/someOtherFolder'], collectionType: 'musicvideos'}); - expect(jf.isActivityValid(validPlay, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Path: '/data/someOtherFolder/myMusic.mp3'}))).to.not.be.true; await jf.destroy(); }); @@ -220,7 +237,7 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, nowPlayingSession({Type: 'Book'}))).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({Type: 'Book'}))).to.not.be.true; await jf.destroy(); }); @@ -246,7 +263,7 @@ describe("Jellyfin API Source", function() { const jf = createJfApi({...defaultJfApiCreds}); await jf.buildInitData(); - expect(jf.isActivityValid(validPlay, nowPlayingSession({ExtraType: 'ThemeSong'}))).to.not.be.true; + expect(jf.isActivityValid(validPlayerState, nowPlayingSession({ExtraType: 'ThemeSong'}))).to.not.be.true; await jf.destroy(); }); diff --git a/src/backend/utils.ts b/src/backend/utils.ts index 641cdc4d..3734a60d 100644 --- a/src/backend/utils.ts +++ b/src/backend/utils.ts @@ -17,10 +17,12 @@ import { TimeoutError, WebapiError } from "spotify-web-api-node/src/response-err import { PlayObject } from "../core/Atomic.js"; import { asPlayerStateData, + asPlayerStateDataMaybePlay, NO_DEVICE, NO_USER, numberFormatOptions, PlayerStateData, + PlayerStateDataMaybePlay, PlayPlatformId, ProgressAwarePlayObject, RegExResult, @@ -454,8 +456,8 @@ export const genGroupIdStr = (id: PlayPlatformId) => { } export const genGroupId = (play: PlayObject): PlayPlatformId => [play.meta.deviceId ?? NO_DEVICE, play.meta.user ?? NO_USER]; -export const getPlatformIdFromData = (data: PlayObject | PlayerStateData) => { - if(asPlayerStateData(data)) { +export const getPlatformIdFromData = (data: PlayObject | PlayerStateDataMaybePlay) => { + if(asPlayerStateDataMaybePlay(data)) { return data.platformId; } return genGroupId(data); -- 2.54.0