diff --git a/jest/functional/ItemToTrack.test.ts b/jest/functional/ItemToTrack.test.ts new file mode 100644 index 000000000..b55d94b5c --- /dev/null +++ b/jest/functional/ItemToTrack.test.ts @@ -0,0 +1,50 @@ +import { BaseItemDto, BaseItemKind } from '@jellyfin/sdk/lib/generated-client/models' +import { mapDtoToTrack } from '../../src/utils/mapping/item-to-track' +import { getApi } from '../../src/stores/auth/utils' + +jest.mock('../../src/stores/auth/utils', () => ({ + getApi: jest.fn(), +})) + +jest.mock('../../src/api/queries/image/utils', () => ({ + getItemImageUrl: jest.fn().mockReturnValue('https://example.com/artwork.jpg'), +})) + +describe('mapDtoToTrack', () => { + const item = { + Id: 'track-1', + Name: 'Track 1', + Type: BaseItemKind.Audio, + ArtistItems: [], + RunTimeTicks: 1_800_000_000, + } as BaseItemDto + + it('places the Jellyfin authorization header in Nitro Player extraPayload', () => { + ;(getApi as jest.Mock).mockReturnValue({ + accessToken: 'access-token', + authorizationHeader: + 'MediaBrowser Client="Jellify", Device="iPhone", DeviceId="device", Version="1.2.7", Token="access-token"', + }) + + const track = mapDtoToTrack(item, new Map()) + + expect(track).not.toHaveProperty('headers') + expect(track.extraPayload).toMatchObject({ + headers: { + Authorization: + 'MediaBrowser Client="Jellify", Device="iPhone", DeviceId="device", Version="1.2.7", Token="access-token"', + }, + }) + }) + + it('omits authentication headers when there is no access token', () => { + ;(getApi as jest.Mock).mockReturnValue({ + accessToken: '', + authorizationHeader: 'MediaBrowser Client="Jellify"', + }) + + const track = mapDtoToTrack(item, new Map()) + + expect(track.extraPayload).not.toHaveProperty('headers') + }) +}) diff --git a/jest/functional/Player/queue.test.ts b/jest/functional/Player/queue.test.ts index e531e5942..b0b2c2046 100644 --- a/jest/functional/Player/queue.test.ts +++ b/jest/functional/Player/queue.test.ts @@ -154,7 +154,7 @@ describe('Queue - loadNewQueue', () => { expect(TrackPlayer.skipToIndex).not.toHaveBeenCalled() }) - it('does not call updateTrackMediaInfo directly when starting track URL is empty (resolved by native onTracksNeedUpdate)', async () => { + it('resolves an empty starting track URL before playback', async () => { const dto = createDto('a') const trackWithoutUrl = createTrackItem('a', '') ;(DownloadManager.getAllDownloadedTracks as jest.Mock).mockResolvedValue([]) @@ -170,7 +170,7 @@ describe('Queue - loadNewQueue', () => { }) expect(resolveTrackUrls).not.toHaveBeenCalled() - expect(updateTrackMediaInfo).not.toHaveBeenCalled() + expect(updateTrackMediaInfo).toHaveBeenCalledWith([trackWithoutUrl]) }) it('does not call updateTrackMediaInfo for a downloaded starting track that already has a local URL', async () => { @@ -192,7 +192,7 @@ describe('Queue - loadNewQueue', () => { expect(updateTrackMediaInfo).not.toHaveBeenCalled() }) - it('does not call updateTrackMediaInfo directly when all track URLs are empty (resolved by native onTracksNeedUpdate)', async () => { + it('only resolves the selected track directly when all track URLs are empty', async () => { const dtos = [createDto('a'), createDto('b')] const trackA = createTrackItem('a', '') const trackB = createTrackItem('b', '') @@ -210,7 +210,8 @@ describe('Queue - loadNewQueue', () => { startPlayback: false, }) - expect(updateTrackMediaInfo).not.toHaveBeenCalled() + expect(updateTrackMediaInfo).toHaveBeenCalledTimes(1) + expect(updateTrackMediaInfo).toHaveBeenCalledWith([trackA]) expect(setNewQueue).toHaveBeenCalledWith( expect.arrayContaining([ expect.objectContaining({ id: 'a', url: '' }), @@ -288,6 +289,30 @@ describe('Queue - loadNewQueue', () => { expect(TrackPlayer.play).toHaveBeenCalled() }) + it('updates an empty starting track before calling play', async () => { + const callOrder: string[] = [] + const dto = createDto('a') + const track = createTrackItem('a', '') + ;(filterTracksOnNetworkStatus as jest.Mock).mockReturnValue([dto]) + ;(mapDtoToTrack as jest.Mock).mockReturnValue(track) + ;(updateTrackMediaInfo as jest.Mock).mockImplementation(async () => { + callOrder.push('updateTrackMediaInfo') + }) + ;(TrackPlayer.play as jest.Mock).mockImplementation(async () => { + callOrder.push('play') + }) + + await loadNewQueue({ + track: dto, + index: 0, + tracklist: [dto], + queue: 'Library', + startPlayback: true, + }) + + expect(callOrder).toEqual(['updateTrackMediaInfo', 'play']) + }) + it('calls TrackPlayer.play() after setNewQueue so the queue is ready before playback starts', async () => { const callOrder: string[] = [] const dto = createDto('a') diff --git a/jest/functional/Player/track-media-info.test.ts b/jest/functional/Player/track-media-info.test.ts index 95748554b..46aef1c08 100644 --- a/jest/functional/Player/track-media-info.test.ts +++ b/jest/functional/Player/track-media-info.test.ts @@ -155,11 +155,7 @@ describe('onTracksNeedUpdate', () => { await onTracksNeedUpdate(tracks, 2) - expect(resolveTrackUrls).toHaveBeenCalledWith( - tracks.slice(0, 2), - 'stream', - expect.any(Object), - ) + expect(resolveTrackUrls).toHaveBeenCalledWith(tracks.slice(0, 2), 'stream', undefined) }) it('passes all tracks when the lookahead equals or exceeds the track count', async () => { @@ -168,6 +164,26 @@ describe('onTracksNeedUpdate', () => { await onTracksNeedUpdate(tracks, 10) - expect(resolveTrackUrls).toHaveBeenCalledWith(tracks, 'stream', expect.any(Object)) + expect(resolveTrackUrls).toHaveBeenCalledWith(tracks, 'stream', undefined) + }) + + it('allows overlapping track updates to finish independently', async () => { + const firstTrack = createTrack('a', '') + const secondTrack = createTrack('b', '') + const updatedFirst = createTrack('a', 'https://cdn.example.com/a.mp3') + const updatedSecond = createTrack('b', 'https://cdn.example.com/b.mp3') + const firstDeferred = deferred() + ;(resolveTrackUrls as jest.Mock) + .mockReturnValueOnce(firstDeferred.promise) + .mockResolvedValueOnce([updatedSecond]) + + const firstUpdate = onTracksNeedUpdate([firstTrack], 1) + await onTracksNeedUpdate([secondTrack], 1) + firstDeferred.resolve([updatedFirst]) + await firstUpdate + + expect(TrackPlayer.updateTracks).toHaveBeenCalledTimes(2) + expect(TrackPlayer.updateTracks).toHaveBeenCalledWith([updatedFirst]) + expect(TrackPlayer.updateTracks).toHaveBeenCalledWith([updatedSecond]) }) }) diff --git a/src/player/queuing/index.ts b/src/player/queuing/index.ts index 0cd02c638..604782a3c 100644 --- a/src/player/queuing/index.ts +++ b/src/player/queuing/index.ts @@ -7,11 +7,17 @@ import { applyHapticFeedback } from '../../utils/haptics' import playNextInQueue from './play-next' import playLaterInQueue from './play-later' import loadQueue from './load' +import { updateTrackMediaInfo } from '../../services/player/utils/track-media-info' export const loadNewQueue = async (variables: QueueMutation) => { applyHapticFeedback('info') - await loadQueue({ ...variables }) + const { finalStartIndex, tracks } = await loadQueue({ ...variables }) + const startingTrack = tracks[finalStartIndex] + + if (startingTrack && !startingTrack.url) { + await updateTrackMediaInfo([startingTrack]) + } if (variables.startPlayback) { await TrackPlayer.play() diff --git a/src/services/player/utils/event-handlers.ts b/src/services/player/utils/event-handlers.ts index 0c2107783..7a37a3f77 100644 --- a/src/services/player/utils/event-handlers.ts +++ b/src/services/player/utils/event-handlers.ts @@ -8,17 +8,9 @@ import { captureError } from '../../../utils/logging' import LoggingContext from '../../../utils/logging/enums' import { updateTrackMediaInfo } from './track-media-info' import reportPlaybackCompleted from '../../../api/mutations/playback/functions/playback-completed' -import { AppState, Platform } from 'react-native' +import { AppState } from 'react-native' import reportPlaybackStarted from '../../../api/mutations/playback/functions/playback-started' -/** - * {@link AbortController} for signalling when to bail from an "onTracksNeedUpdate". - * event. - * - * This is only used on iOS - */ -let trackUpdateAbortController: AbortController | null = null - /** * Tracks the most recent playback state so that resume-from-pause can be * distinguished from a genuine first-play, and so that onSeek can include @@ -53,15 +45,11 @@ export async function onTracksNeedUpdate(tracks: TrackItem[], lookahead: number) `[Player Event] onTracksNeedUpdate triggered for ${tracks.length} track(s). Updating media info...`, ) - trackUpdateAbortController?.abort() - const tracksToUpdate = lookahead > 0 ? tracks.slice(0, lookahead) : tracks console.debug(`[Player Event] Updating media info for track lookahead ${tracksToUpdate.length}`) - trackUpdateAbortController = Platform.OS === 'ios' ? new AbortController() : null - - await updateTrackMediaInfo(tracksToUpdate, trackUpdateAbortController?.signal) + await updateTrackMediaInfo(tracksToUpdate) } /** diff --git a/src/types/JellifyTrack.ts b/src/types/JellifyTrack.ts index d79c24c1a..95dfaf76b 100644 --- a/src/types/JellifyTrack.ts +++ b/src/types/JellifyTrack.ts @@ -35,6 +35,7 @@ export type TrackExtraPayload = Record & { */ mediaSourceInfo: string blurhash?: string + headers?: Record } export type SlimifiedBaseItemDto = Pick< diff --git a/src/utils/mapping/item-to-track.ts b/src/utils/mapping/item-to-track.ts index 71afd50b2..06b354d41 100644 --- a/src/utils/mapping/item-to-track.ts +++ b/src/utils/mapping/item-to-track.ts @@ -1,6 +1,5 @@ import { BaseItemDto, BaseItemKind, ImageType } from '@jellyfin/sdk/lib/generated-client/models' import { TrackExtraPayload } from '../../types/JellifyTrack' -import { Api } from '@jellyfin/sdk/lib/api' import { convertRunTimeTicksToSeconds } from './ticks-to-seconds' import { getApi } from '../../stores/auth/utils' import { DownloadedTrack, TrackItem } from 'react-native-nitro-player' @@ -43,10 +42,7 @@ export function mapDtoToTrack( ? getTrackMediaSourceInfo(downloadedTrack.originalTrack) : ({} as const) - // Only include headers when we have an API token (streaming cases). For downloaded tracks it's not needed. - const headers = (api as Api | undefined)?.accessToken - ? { AUTHORIZATION: (api as Api).accessToken } - : undefined + const headers = api.accessToken ? { Authorization: api.authorizationHeader } : undefined if (downloadedTrack?.localPath) { console.debug('Downloaded track path', `file://${downloadedTrack.localPath}`) @@ -57,7 +53,6 @@ export function mapDtoToTrack( } return { - ...(headers ? { headers } : {}), id: item.Id ?? '', url: downloadedTrack?.localPath ? `file://${downloadedTrack.localPath}` : '', artwork: downloadedTrack?.localArtworkPath @@ -82,6 +77,7 @@ export function mapDtoToTrack( mediaSourceInfo: JSON.stringify(mediaSourceInfo), // This will be populated later in the playback flow when we have the MediaSourceInfo available sessionId: '', blurhash: getBlurhashFromDto(item), + ...(headers ? { headers } : {}), } as TrackExtraPayload, } as TrackItem }