Skip to content

Commit

Permalink
fix(jellyfin): Fix memory player hanging on end of state
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@FoxxMD
FoxxMD committed Oct 16, 2024
1 parent 2d3ab86 commit 57a8e48
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 63 deletions.
4 changes: 4 additions & 0 deletions src/backend/common/infrastructure/Atomic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
98 changes: 57 additions & 41 deletions src/backend/sources/JellyfinApiSource.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
18 changes: 15 additions & 3 deletions src/backend/sources/MemorySource.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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: {
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down
51 changes: 34 additions & 17 deletions src/backend/tests/jellyfin/jellyfin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}});

Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -162,16 +179,16 @@ 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();
});

it('Should allow activity based on libraries allow', async 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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -212,15 +229,15 @@ 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();
});

it('Should disallow NowPlayingItem that is not valid Type', async 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();
});

Expand All @@ -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();
});

Expand Down
6 changes: 4 additions & 2 deletions src/backend/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 57a8e48

Please sign in to comment.