Bugfix/large playlist crash (#111)

* get all song coverArt as they are rendered

doing it all up front was too heavy
temporarily disabled mapping artwork in setQueue, need to fix this

* use cache data for track artwork when available

* fix round art in context menu for songs

* set only the first artwork at play time

then set the rest in the playback service

* handle both cached images and fetching images

* remove commented code

* fix shuffle

fix first thumbnail not being updated on shuffle for now playing background

app/hooks/fetch.ts

@@ -1,8 +1,9 @@
 import { CacheItemTypeKey } from '@app/models/cache'
-import { Album, AlbumCoverArt, Playlist, Song } from '@app/models/library'
+import { Album, Playlist, Song } from '@app/models/library'
 import { mapAlbum, mapArtist, mapArtistInfo, mapPlaylist, mapSong } from '@app/models/map'
 import queryClient from '@app/queryClient'
 import { useStore } from '@app/state/store'
+import { SubsonicApiClient } from '@app/subsonic/api'
 import { GetAlbumList2TypeBase, Search3Params, StarParams } from '@app/subsonic/params'
 import { cacheDir } from '@app/util/fs'
 import { mapCollectionById } from '@app/util/state'
@@ -31,7 +32,7 @@ function cacheStarredData<T extends { id: string; starred?: undefined | any }>(i
 }
 
 function cacheAlbumCoverArtData<T extends { id: string; coverArt?: string }>(item: T) {
-  queryClient.setQueryData<AlbumCoverArt>(qk.albumCoverArt(item.id), { albumId: item.id, coverArt: item.coverArt })
+  queryClient.setQueryData<string | undefined>(qk.albumCoverArt(item.id), item.coverArt)
 }
 
 export const useFetchArtists = () => {
@@ -109,22 +110,23 @@ export const useFetchPlaylist = () => {
   }
 }
 
+export async function fetchAlbum(id: string, client: SubsonicApiClient): Promise<{ album: Album; songs?: Song[] }> {
+  const res = await client.getAlbum({ id })
+
+  cacheStarredData(res.data.album)
+  res.data.songs.forEach(cacheStarredData)
+
+  cacheAlbumCoverArtData(res.data.album)
+
+  return {
+    album: mapAlbum(res.data.album),
+    songs: res.data.songs.map(mapSong),
+  }
+}
+
 export const useFetchAlbum = () => {
   const client = useClient()
-
-  return async (id: string): Promise<{ album: Album; songs?: Song[] }> => {
-    const res = await client().getAlbum({ id })
-
-    cacheStarredData(res.data.album)
-    res.data.songs.forEach(cacheStarredData)
-
-    cacheAlbumCoverArtData(res.data.album)
-
-    return {
-      album: mapAlbum(res.data.album),
-      songs: res.data.songs.map(mapSong),
-    }
-  }
+  return async (id: string) => fetchAlbum(id, client())
 }
 
 export const useFetchAlbumList = () => {
@@ -196,17 +198,23 @@ export type FetchExisingFileOptions = {
   itemId: string
 }
 
-export const useFetchExistingFile: () => (options: FetchExisingFileOptions) => Promise<string | undefined> = () => {
-  const serverId = useStore(store => store.settings.activeServerId)
+export async function fetchExistingFile(
+  options: FetchExisingFileOptions,
+  serverId: string | undefined,
+): Promise<string | undefined> {
+  const { itemType, itemId } = options
+  const fileDir = cacheDir(serverId, itemType, itemId)
 
-  return async ({ itemType, itemId }) => {
-    const fileDir = cacheDir(serverId, itemType, itemId)
-    try {
-      const dir = await RNFS.readDir(fileDir)
-      console.log('existing file:', dir[0].path)
-      return dir[0].path
-    } catch {}
-  }
+  try {
+    const dir = await RNFS.readDir(fileDir)
+    console.log('existing file:', dir[0].path)
+    return dir[0].path
+  } catch {}
+}
+
+export const useFetchExistingFile = () => {
+  const serverId = useStore(store => store.settings.activeServerId)
+  return async (options: FetchExisingFileOptions) => fetchExistingFile(options, serverId)
 }
 
 function assertMimeType(expected?: string, actual?: string) {
@@ -237,69 +245,71 @@ export type FetchFileOptions = FetchExisingFileOptions & {
   progress?: (received: number, total: number) => void
 }
 
-export const useFetchFile: () => (options: FetchFileOptions) => Promise<string> = () => {
-  const serverId = useStore(store => store.settings.activeServerId)
+export async function fetchFile(options: FetchFileOptions, serverId: string | undefined): Promise<string> {
+  let { itemType, itemId, fromUrl, useCacheBuster, expectedContentType, progress } = options
+  useCacheBuster = useCacheBuster === undefined ? true : useCacheBuster
 
-  return async ({ itemType, itemId, fromUrl, useCacheBuster, expectedContentType, progress }) => {
-    useCacheBuster = useCacheBuster === undefined ? true : useCacheBuster
+  const fileDir = cacheDir(serverId, itemType, itemId)
+  const filePathNoExt = path.join(fileDir, useCacheBuster ? useStore.getState().settings.cacheBuster : itemType)
 
-    const fileDir = cacheDir(serverId, itemType, itemId)
-    const filePathNoExt = path.join(fileDir, useCacheBuster ? useStore.getState().settings.cacheBuster : itemType)
+  try {
+    await RNFS.unlink(fileDir)
+  } catch {}
 
-    try {
-      await RNFS.unlink(fileDir)
-    } catch {}
+  const headers = { 'User-Agent': userAgent }
 
-    const headers = { 'User-Agent': userAgent }
+  // we send a HEAD first for two reasons:
+  // 1. to follow any redirects and get the actual URL (DownloadManager does not support redirects)
+  // 2. to obtain the mime-type up front so we can use it for the file extension/validation
+  const headRes = await fetch(fromUrl, { method: 'HEAD', headers })
 
-    // we send a HEAD first for two reasons:
-    // 1. to follow any redirects and get the actual URL (DownloadManager does not support redirects)
-    // 2. to obtain the mime-type up front so we can use it for the file extension/validation
-    const headRes = await fetch(fromUrl, { method: 'HEAD', headers })
-
-    if (headRes.status > 399) {
-      throw new Error(`HTTP status error ${headRes.status}. File: ${itemType} ID: ${itemId}`)
-    }
-
-    const contentType = headRes.headers.get('content-type') || undefined
-    assertMimeType(expectedContentType, contentType)
-
-    const contentDisposition = headRes.headers.get('content-disposition') || undefined
-    const filename = contentDisposition ? cd.parse(contentDisposition).parameters.filename : undefined
-
-    let extension: string | undefined
-    if (filename) {
-      extension = path.extname(filename) || undefined
-      if (extension) {
-        extension = extension.substring(1)
-      }
-    } else if (contentType) {
-      extension = mime.extension(contentType) || undefined
-    }
-
-    const config = ReactNativeBlobUtil.config({
-      addAndroidDownloads: {
-        useDownloadManager: true,
-        notification: false,
-        mime: contentType,
-        description: 'subtracks',
-        path: extension ? `${filePathNoExt}.${extension}` : filePathNoExt,
-      },
-    })
-
-    const fetchParams: Parameters<typeof config['fetch']> = ['GET', headRes.url, headers]
-
-    let res: FetchBlobResponse
-    if (progress) {
-      res = await config.fetch(...fetchParams).progress(progress)
-    } else {
-      res = await config.fetch(...fetchParams)
-    }
-
-    const downloadPath = res.path()
-    queryClient.setQueryData<string>(qk.existingFiles(itemType, itemId), downloadPath)
-
-    console.log('downloaded file:', downloadPath)
-    return downloadPath
+  if (headRes.status > 399) {
+    throw new Error(`HTTP status error ${headRes.status}. File: ${itemType} ID: ${itemId}`)
   }
+
+  const contentType = headRes.headers.get('content-type') || undefined
+  assertMimeType(expectedContentType, contentType)
+
+  const contentDisposition = headRes.headers.get('content-disposition') || undefined
+  const filename = contentDisposition ? cd.parse(contentDisposition).parameters.filename : undefined
+
+  let extension: string | undefined
+  if (filename) {
+    extension = path.extname(filename) || undefined
+    if (extension) {
+      extension = extension.substring(1)
+    }
+  } else if (contentType) {
+    extension = mime.extension(contentType) || undefined
+  }
+
+  const config = ReactNativeBlobUtil.config({
+    addAndroidDownloads: {
+      useDownloadManager: true,
+      notification: false,
+      mime: contentType,
+      description: 'subtracks',
+      path: extension ? `${filePathNoExt}.${extension}` : filePathNoExt,
+    },
+  })
+
+  const fetchParams: Parameters<typeof config['fetch']> = ['GET', headRes.url, headers]
+
+  let res: FetchBlobResponse
+  if (progress) {
+    res = await config.fetch(...fetchParams).progress(progress)
+  } else {
+    res = await config.fetch(...fetchParams)
+  }
+
+  const downloadPath = res.path()
+  queryClient.setQueryData<string>(qk.existingFiles(itemType, itemId), downloadPath)
+
+  console.log('downloaded file:', downloadPath)
+  return downloadPath
+}
+
+export const useFetchFile = () => {
+  const serverId = useStore(store => store.settings.activeServerId)
+  return async (options: FetchFileOptions) => fetchFile(options, serverId)
 }

app/hooks/query.ts

@@ -1,19 +1,11 @@
 import { CacheImageSize, CacheItemTypeKey } from '@app/models/cache'
-import { Album, AlbumCoverArt, Artist, Playlist, Song, StarrableItemType } from '@app/models/library'
+import { Album, Artist, Playlist, Song, StarrableItemType } from '@app/models/library'
 import { CollectionById } from '@app/models/state'
 import queryClient from '@app/queryClient'
 import { useStore } from '@app/state/store'
 import { GetAlbumList2TypeBase, Search3Params, StarParams } from '@app/subsonic/params'
 import _ from 'lodash'
-import {
-  InfiniteData,
-  useInfiniteQuery,
-  UseInfiniteQueryResult,
-  useMutation,
-  useQueries,
-  useQuery,
-  UseQueryResult,
-} from 'react-query'
+import { useInfiniteQuery, useMutation, useQueries, useQuery } from 'react-query'
 import {
   useFetchAlbum,
   useFetchAlbumList,
@@ -88,7 +80,7 @@ export const useQueryArtistTopSongs = (artistName?: string) => {
     },
   )
 
-  return useFixCoverArt(querySuccess ? query : backupQuery)
+  return querySuccess ? query : backupQuery
 }
 
 export const useQueryPlaylists = () => useQuery(qk.playlists, useFetchPlaylists())
@@ -109,7 +101,7 @@ export const useQueryPlaylist = (id: string, placeholderPlaylist?: Playlist) =>
     },
   })
 
-  return useFixCoverArt(query)
+  return query
 }
 
 export const useQueryAlbum = (id: string, placeholderAlbum?: Album) => {
@@ -120,7 +112,7 @@ export const useQueryAlbum = (id: string, placeholderAlbum?: Album) => {
       placeholderAlbum ? { album: placeholderAlbum } : undefined,
   })
 
-  return useFixCoverArt(query)
+  return query
 }
 
 export const useQueryAlbumList = (type: GetAlbumList2TypeBase, size: number) => {
@@ -172,7 +164,7 @@ export const useQuerySearchResults = (params: Search3Params) => {
     },
   )
 
-  return useFixCoverArt(query)
+  return query
 }
 
 export const useQueryHomeLists = (types: GetAlbumList2TypeBase[], size: number) => {
@@ -314,93 +306,18 @@ export const useQueryArtistArtPath = (artistId: string, size: CacheImageSize = '
   return { ...query, data: existing.data || query.data, isExistingFetching: existing.isFetching }
 }
 
-type WithSongs = Song[] | { songs?: Song[] }
-type InfiniteWithSongs = { songs: Song[] }
-type AnyDataWithSongs = WithSongs | InfiniteData<InfiniteWithSongs>
-type AnyQueryWithSongs = UseQueryResult<WithSongs> | UseInfiniteQueryResult<{ songs: Song[] }>
-
-function getSongs<T extends AnyDataWithSongs>(data: T | undefined): Song[] {
-  if (!data) {
-    return []
-  }
-
-  if (Array.isArray(data)) {
-    return data
-  }
-
-  if ('pages' in data) {
-    return data.pages.flatMap(p => p.songs)
-  }
-
-  return data.songs || []
-}
-
-function setSongCoverArt<T extends AnyQueryWithSongs>(query: T, coverArts: UseQueryResult<AlbumCoverArt>[]): T {
-  if (!query.data) {
-    return query
-  }
-
-  const mapSongCoverArt = (song: Song) => ({
-    ...song,
-    coverArt: coverArts.find(c => c.data?.albumId === song.albumId)?.data?.coverArt,
-  })
-
-  if (Array.isArray(query.data)) {
-    return {
-      ...query,
-      data: query.data.map(mapSongCoverArt),
-    }
-  }
-
-  if ('pages' in query.data) {
-    return {
-      ...query,
-      data: {
-        pages: query.data.pages.map(p => ({
-          ...p,
-          songs: p.songs.map(mapSongCoverArt),
-        })),
-      },
-    }
-  }
-
-  if (query.data.songs) {
-    return {
-      ...query,
-      data: {
-        ...query.data,
-        songs: query.data.songs.map(mapSongCoverArt),
-      },
-    }
-  }
-
-  return query
-}
-
-// song cover art comes back from the api as a unique id per song even if it all points to the same
-// album art, which prevents us from caching it once, so we need to use the album's cover art
-const useFixCoverArt = <T extends AnyQueryWithSongs>(query: T) => {
+export const useQueryAlbumCoverArtPath = (albumId?: string, size: CacheImageSize = 'thumbnail') => {
   const fetchAlbum = useFetchAlbum()
 
-  const songs = getSongs(query.data)
-  const albumIds = _.uniq((songs || []).map(s => s.albumId).filter((id): id is string => id !== undefined))
-
-  const coverArts = useQueries(
-    albumIds.map(id => ({
-      queryKey: qk.albumCoverArt(id),
-      queryFn: async (): Promise<AlbumCoverArt> => {
-        const res = await fetchAlbum(id)
-        return { albumId: res.album.id, coverArt: res.album.coverArt }
-      },
+  const query = useQuery(
+    qk.albumCoverArt(albumId || '-1'),
+    async () => (await fetchAlbum(albumId || '-1')).album.coverArt,
+    {
+      enabled: !!albumId,
       staleTime: Infinity,
       cacheTime: Infinity,
-      notifyOnChangeProps: ['data', 'isFetched'] as any,
-    })),
+    },
   )
 
-  if (coverArts.every(c => c.isFetched)) {
-    return setSongCoverArt(query, coverArts)
-  }
-
-  return query
+  return useQueryCoverArtPath(query.data, size)
 }

app/hooks/trackplayer.ts

@@ -1,13 +1,11 @@
 import { Song } from '@app/models/library'
 import { QueueContextType, TrackExt } from '@app/models/trackplayer'
 import queryClient from '@app/queryClient'
+import queueService from '@app/queueservice'
 import { useStore, useStoreDeep } from '@app/state/store'
 import { getQueue, SetQueueOptions, trackPlayerCommands } from '@app/state/trackplayer'
 import userAgent from '@app/util/userAgent'
-import _ from 'lodash'
 import TrackPlayer from 'react-native-track-player'
-import { useQueries } from 'react-query'
-import { useFetchExistingFile, useFetchFile } from './fetch'
 import qk from './queryKeys'
 
 export const usePlay = () => {
@@ -92,87 +90,50 @@ export const useIsPlaying = (contextId: string | undefined, track: number) => {
   return contextId === queueContextId && track === currentTrackIdx
 }
 
+export function mapSongToTrackExt(song: Song): TrackExt {
+  return {
+    id: song.id,
+    title: song.title,
+    artist: song.artist || 'Unknown Artist',
+    album: song.album || 'Unknown Album',
+    url: useStore.getState().buildStreamUri(song.id),
+    artwork: require('@res/fallback.png'),
+    userAgent,
+    duration: song.duration,
+    artistId: song.artistId,
+    albumId: song.albumId,
+    track: song.track,
+    discNumber: song.discNumber,
+  }
+}
+
 export const useSetQueue = (type: QueueContextType, songs?: Song[]) => {
   const _setQueue = useStore(store => store.setQueue)
-  const client = useStore(store => store.client)
-  const buildStreamUri = useStore(store => store.buildStreamUri)
-  const fetchFile = useFetchFile()
-  const fetchExistingFile = useFetchExistingFile()
-
-  const songCoverArt = _.uniq((songs || []).map(s => s.coverArt)).filter((c): c is string => c !== undefined)
-
-  const coverArtPaths = useQueries(
-    songCoverArt.map(coverArt => ({
-      queryKey: qk.coverArt(coverArt, 'thumbnail'),
-      queryFn: async () => {
-        if (!client) {
-          return
-        }
-
-        const itemType = 'coverArtThumb'
-
-        const existingCache = queryClient.getQueryData<string | undefined>(qk.existingFiles(itemType, coverArt))
-        if (existingCache) {
-          return existingCache
-        }
-
-        const existingDisk = await fetchExistingFile({ itemId: coverArt, itemType })
-        if (existingDisk) {
-          return existingDisk
-        }
-
-        const fromUrl = client.getCoverArtUri({ id: coverArt, size: '256' })
-        return await fetchFile({
-          itemType,
-          itemId: coverArt,
-          fromUrl,
-          expectedContentType: 'image',
-        })
-      },
-      enabled: !!client && !!songs,
-      staleTime: Infinity,
-      cacheTime: Infinity,
-      notifyOnChangeProps: ['data', 'isFetched'] as any,
-    })),
-  )
-
-  const songCoverArtToPath = _.zipObject(
-    songCoverArt,
-    coverArtPaths.map(c => c.data),
-  )
-
-  const mapSongToTrackExt = (s: Song): TrackExt => {
-    let artwork = require('@res/fallback.png')
-    if (s.coverArt) {
-      const filePath = songCoverArtToPath[s.coverArt]
-      if (filePath) {
-        artwork = `file://${filePath}`
-      }
-    }
-
-    return {
-      id: s.id,
-      title: s.title,
-      artist: s.artist || 'Unknown Artist',
-      album: s.album || 'Unknown Album',
-      url: buildStreamUri(s.id),
-      userAgent,
-      artwork,
-      coverArt: s.coverArt,
-      duration: s.duration,
-      artistId: s.artistId,
-      albumId: s.albumId,
-      track: s.track,
-      discNumber: s.discNumber,
-    }
-  }
 
   const contextId = `${type}-${songs?.map(s => s.id).join('-')}`
 
   const setQueue = async (options: SetQueueOptions) => {
-    const queue = (songs || []).map(mapSongToTrackExt)
-    return await _setQueue({ queue, type, contextId, ...options })
+    if (!songs || songs.length === 0) {
+      return
+    }
+
+    const queue = songs.map(mapSongToTrackExt)
+    const first = queue[options.playTrack || 0]
+
+    if (!first.albumId) {
+      first.artwork = require('@res/fallback.png')
+    } else {
+      const albumCoverArt = queryClient.getQueryData<string>(qk.albumCoverArt(first.albumId))
+      const existingFile = queryClient.getQueryData<string>(qk.existingFiles('coverArtThumb', albumCoverArt))
+      const downloadFile = queryClient.getQueryData<string>(qk.coverArt(albumCoverArt, 'thumbnail'))
+      if (existingFile || downloadFile) {
+        first.artwork = `file://${existingFile || downloadFile}`
+      }
+    }
+
+    await _setQueue({ queue, type, contextId, ...options })
+    queueService.emit('set', { queue })
   }
 
-  return { setQueue, contextId, isReady: coverArtPaths.every(c => c.isFetched) }
+  return { setQueue, contextId }
 }