diff --git a/packages/app/app/actions/queue.test.ts b/packages/app/app/actions/queue.test.ts index 24399aebf6..58a58602f6 100644 --- a/packages/app/app/actions/queue.test.ts +++ b/packages/app/app/actions/queue.test.ts @@ -6,16 +6,16 @@ describe('Queue actions tests', () => { describe('finds streams for track', () => { const getSelectedStreamProvider = jest.spyOn(QueueOperations, 'getSelectedStreamProvider'); - const getTrackStreams = jest.spyOn(QueueOperations, 'getTrackStreams'); + const resolveTrackStreams = jest.spyOn(QueueOperations, 'resolveTrackStreams'); afterEach(() => { getSelectedStreamProvider.mockReset(); - getTrackStreams.mockReset(); + resolveTrackStreams.mockReset(); }); test('remote track is removed from the queue when no streams are available', () => { // Mock an empty search result for streams. - getTrackStreams.mockResolvedValueOnce([]); + resolveTrackStreams.mockResolvedValueOnce([]); // Configure a dummy stream provider. It is not actually used in this execution path. getSelectedStreamProvider.mockReturnValueOnce({}); diff --git a/packages/app/app/actions/queue.ts b/packages/app/app/actions/queue.ts index c5c22c069b..3a05083963 100644 --- a/packages/app/app/actions/queue.ts +++ b/packages/app/app/actions/queue.ts @@ -63,7 +63,8 @@ export const getSelectedStreamProvider = (getState) => { return _.find(streamProviders, { sourceName: selected.streamProviders }); }; -export const getTrackStreams = async ( +// Exported to facilitate testing. +export const resolveTrackStreams = async ( track: Track | LocalTrack, selectedStreamProvider: StreamProvider ) => { @@ -168,20 +169,18 @@ export const selectNewStream = (index: number, streamId: string) => async (dispa export const findStreamsForTrack = (index: number) => async (dispatch, getState) => { const {queue, settings}: RootState = getState(); - const track = queue.queueItems[index]; - if (track && !track.local && isEmpty(track.streams)) { - dispatch(updateQueueItem({ - ...track, - loading: true - })); + if (track && !track.local && trackHasNoFirstStream(track)) { + if (!track.loading) { + dispatch(updateQueueItem({ + ...track, + loading: true + })); + } const selectedStreamProvider = getSelectedStreamProvider(getState); try { - let streamData = await getTrackStreams( - track, - selectedStreamProvider - ); + let streamData: TrackStream[] = await getTrackStreams(track, selectedStreamProvider); if (settings.useStreamVerification) { try { @@ -207,10 +206,14 @@ export const findStreamsForTrack = (index: number) => async (dispatch, getState) if (streamData?.length === 0) { dispatch(removeFromQueue(index)); } else { - streamData = [ - await selectedStreamProvider.getStreamForId(streamData.find(Boolean)?.id), - ...streamData.slice(1) - ]; + streamData = await resolveSourceUrlForTheFirstStream(streamData, selectedStreamProvider); + + const firstStream = streamData[0]; + if (!firstStream?.stream) { + const remainingStreams = streamData.slice(1); + removeFirstStream(track, index, remainingStreams, dispatch); + return; + } dispatch( updateQueueItem({ @@ -240,6 +243,46 @@ export const findStreamsForTrack = (index: number) => async (dispatch, getState) } }; +async function getTrackStreams(track: QueueItem, streamProvider: StreamProvider): Promise { + if (isEmpty(track.streams)) { + return resolveTrackStreams( + track, + streamProvider + ); + } + return track.streams; +} + +async function resolveSourceUrlForTheFirstStream(trackStreams: TrackStream[], streamProvider: StreamProvider): Promise { + if (isEmpty(trackStreams[0].stream)) { + return [ + await streamProvider.getStreamForId(trackStreams.find(Boolean)?.id), + ...trackStreams.slice(1) + ]; + } + // The stream URL might already be resolved, for example for a previously played track. + return trackStreams; +} + +export function trackHasNoFirstStream(track: QueueItem): boolean { + return isEmpty(track?.streams) || isEmpty(track.streams[0].stream); +} + +function removeFirstStream(track: QueueItem, trackIndex: number, remainingStreams: TrackStream[], dispatch): void { + if (remainingStreams.length === 0) { + // no more streams are available + dispatch(removeFromQueue(trackIndex)); + } else { + // remove the first (unavailable) stream + dispatch(updateQueueItem({ + ...track, + loading: true, + error: false, + streams: remainingStreams + })); + } +} + export function playTrack(streamProviders, item: QueueItem) { return (dispatch) => { dispatch(clearQueue()); diff --git a/packages/app/app/containers/AlbumViewContainer/AlbumViewContainer.test.tsx b/packages/app/app/containers/AlbumViewContainer/AlbumViewContainer.test.tsx index cc11e21f5d..a8acfbb324 100644 --- a/packages/app/app/containers/AlbumViewContainer/AlbumViewContainer.test.tsx +++ b/packages/app/app/containers/AlbumViewContainer/AlbumViewContainer.test.tsx @@ -202,7 +202,7 @@ describe('Album view container', () => { artist: 'test artist', name: 'test track 1', streams: [{ - data: 'test-stream-data', + stream: 'test-stream-url', author: { name: 'test author' } diff --git a/packages/app/app/containers/PlayerBarContainer/PlayerBarContainer.test.tsx b/packages/app/app/containers/PlayerBarContainer/PlayerBarContainer.test.tsx index 44f8202531..56b491de39 100644 --- a/packages/app/app/containers/PlayerBarContainer/PlayerBarContainer.test.tsx +++ b/packages/app/app/containers/PlayerBarContainer/PlayerBarContainer.test.tsx @@ -129,7 +129,8 @@ describe('PlayerBar container', () => { streams: [{ duration: 300, title: 'test track 1', - skipSegments: [] + skipSegments: [], + stream: 'stream URL' }] }] }, @@ -154,7 +155,8 @@ describe('PlayerBar container', () => { streams: [{ duration: 300, title: 'test track 1', - skipSegments: [] + skipSegments: [], + stream: 'stream URL 1' }] }, { artist: 'test artist 2', @@ -162,7 +164,8 @@ describe('PlayerBar container', () => { streams: [{ duration: 300, title: 'test track 2', - skipSegments: [] + skipSegments: [], + stream: 'stream URL 2' }] }] }, @@ -187,7 +190,8 @@ describe('PlayerBar container', () => { streams: [{ duration: 300, title: 'test track 1', - skipSegments: [] + skipSegments: [], + stream: 'stream URL 1' }] }, { artist: 'test artist 2', @@ -200,7 +204,8 @@ describe('PlayerBar container', () => { duration: 10, endTime: 10, startTime: 0 - }] + }], + stream: 'stream URL 2' }] }] }, @@ -225,7 +230,8 @@ describe('PlayerBar container', () => { streams: [{ duration: 300, title: 'test track 1', - skipSegments: [] + skipSegments: [], + stream: 'stream URL' }] }] }, @@ -272,6 +278,53 @@ describe('PlayerBar container', () => { }); }); + it('should remove the track when no more stream URLs can be resolved', async () => { + const { component, store } = mountComponent({ + queue: { + currentSong: 0, + queueItems: [ + { + uuid: 'uuid1', + artist: 'test artist name', + name: 'track without streams' + } + ] + }, + plugin: { + plugins: { + streamProviders: [ + { + sourceName: 'Mocked Stream Provider', + search: jest.fn().mockResolvedValueOnce([ + { + id: 'stream 1 ID', + source: 'Mocked Stream Provider' + }, + { + id: 'stream 2 ID', + source: 'Mocked Stream Provider' + } + ]), + getStreamForId: jest.fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ + stream: null, + source: 'Mocked Stream Provider' + }) + } + ] + }, + selected: { + streamProviders: 'Mocked Stream Provider' + } + } + }); + await waitFor(() => { + const state = store.getState(); + expect(state.queue.queueItems.length).toBe(0); + }); + }); + const mountComponent = (initialStore?: AnyProps) => { const store = configureMockStore({ ...buildStoreState() diff --git a/packages/app/app/containers/PlayerBarContainer/hooks.ts b/packages/app/app/containers/PlayerBarContainer/hooks.ts index 3e48d3c153..c21168d50d 100644 --- a/packages/app/app/containers/PlayerBarContainer/hooks.ts +++ b/packages/app/app/containers/PlayerBarContainer/hooks.ts @@ -14,12 +14,11 @@ import * as playerActions from '../../actions/player'; import * as queueActions from '../../actions/queue'; import * as settingsActions from '../../actions/settings'; import * as favoritesActions from '../../actions/favorites'; -import { favoritesSelectors } from '../../selectors/favorites'; +import { favoritesSelectors, getFavoriteTrack } from '../../selectors/favorites'; import { playerSelectors } from '../../selectors/player'; import { queue as queueSelector } from '../../selectors/queue'; import { settingsSelector } from '../../selectors/settings'; -import { getFavoriteTrack } from '../../selectors/favorites'; -import { QueueItem } from '../../reducers/queue'; +import { QueueItem, QueueStore } from '../../reducers/queue'; export const useSeekbarProps = () => { const dispatch = useDispatch(); @@ -255,12 +254,10 @@ export const useStreamLookup = () => { const queue = useSelector(queueSelector); useEffect(() => { - const isStreamLoading = Boolean(queue.queueItems.find((item) => item.loading)); - - if (!isStreamLoading) { + if (shouldSearchForStreams(queue)) { const currentSong: QueueItem = queue.queueItems[queue.currentSong]; - if (currentSong && isEmpty(currentSong.streams)) { + if (currentSong && queueActions.trackHasNoFirstStream(currentSong)) { dispatch(queueActions.findStreamsForTrack(queue.currentSong)); return; } @@ -273,3 +270,19 @@ export const useStreamLookup = () => { } }, [queue]); }; + +const shouldSearchForStreams = (queue: QueueStore): boolean => { + const currentlyLoadingTrack = queue.queueItems.find((item) => item.loading); + if (!currentlyLoadingTrack) { + // No track is currently "loading": start searching for steams. + return true; + } + if (isEmpty(currentlyLoadingTrack.streams)) { + // Streams are not yet resolved: this happens when the track is being marked as "loading" + // while still resolving streams. We don't need to start a search until that operation completes or fails. + return false; + } + const firstStream = currentlyLoadingTrack.streams[0]; + // A search should be performed if the first stream URL wasn't yet resolved. + return !firstStream?.stream; +}; diff --git a/packages/app/test/storeBuilders.ts b/packages/app/test/storeBuilders.ts index d0db0f9e47..7264d9db59 100644 --- a/packages/app/test/storeBuilders.ts +++ b/packages/app/test/storeBuilders.ts @@ -279,13 +279,12 @@ export const buildStoreState = () => { sourceName: 'Test Stream Provider', search: jest.fn(() => ([{ id: 'test-stream-id', - data: 'test-stream-data', author: { name: 'test author' } }])), getStreamForId: jest.fn(() => ({ - data: 'test-stream-data', + stream: 'test-stream-url', author: { name: 'test author' } @@ -296,7 +295,6 @@ export const buildStoreState = () => { sourceName: 'Different Stream Provider', search: jest.fn(() => ([{ id: 'test-stream-id-1', - data: 'different-test-stream-data', source: 'Different Stream Provider', author: { name: 'test author' @@ -304,7 +302,7 @@ export const buildStoreState = () => { }])), getStreamForId: jest.fn((id: string) => ({ id: 'test-stream-id-1', - data: id, + stream: 'different-test-stream-url', source: 'Different Stream Provider' }) as TrackStream) }