From d1824a70becb4bfd3ce75b0eaa3cf68f29d59f90 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 13 Aug 2021 14:41:28 +0900 Subject: [PATCH] fix artist image render error separate cache state into its own slice --- app/components/ContextMenu.tsx | 12 +- app/components/CoverArt.tsx | 48 +++---- app/hooks/music.ts | 54 ++++---- app/hooks/trackplayer.ts | 4 +- app/state/cache.ts | 234 +++++++++++++++++++++++++++++++++ app/state/music.ts | 216 ------------------------------ app/state/store.ts | 5 +- app/subsonic/api.ts | 2 +- 8 files changed, 300 insertions(+), 275 deletions(-) create mode 100644 app/state/cache.ts diff --git a/app/components/ContextMenu.tsx b/app/components/ContextMenu.tsx index 560380d..bd9219a 100644 --- a/app/components/ContextMenu.tsx +++ b/app/components/ContextMenu.tsx @@ -118,7 +118,17 @@ const MenuHeader = React.memo<{ subtitle?: string }>(({ coverArt, artistId, title, subtitle }) => ( - + {artistId ? ( + + ) : ( + + )} {title} diff --git a/app/components/CoverArt.tsx b/app/components/CoverArt.tsx index 54167bc..fa69875 100644 --- a/app/components/CoverArt.tsx +++ b/app/components/CoverArt.tsx @@ -1,7 +1,7 @@ import { useArtistCoverArtFile, useCoverArtFile } from '@app/hooks/music' -import { DownloadFile } from '@app/state/music' +import { DownloadFile } from '@app/state/cache' import colors from '@app/styles/colors' -import React, { useCallback, useEffect, useState } from 'react' +import React, { useState } from 'react' import { ActivityIndicator, StyleSheet, View, ViewStyle } from 'react-native' import FastImage, { ImageStyle } from 'react-native-fast-image' @@ -22,16 +22,15 @@ type CoverArtProps = BaseProps & { coverArt?: string } -const Image: React.FC<{ file?: DownloadFile } & BaseProps> = ({ file, style, imageStyle, resizeMode }) => { - const [source, setSource] = useState( - file && file.progress === 1 ? { uri: `file://${file.path}` } : require('@res/fallback.png'), - ) +const Image = React.memo<{ file?: DownloadFile } & BaseProps>(({ file, style, imageStyle, resizeMode }) => { + const [error, setError] = useState(false) - useEffect(() => { - if (file && file.progress === 1) { - setSource({ uri: `file://${file.path}` }) - } - }, [file]) + let source + if (!error && file && file.progress === 1) { + source = { uri: `file://${file.path}` } + } else { + source = require('@res/fallback.png') + } return ( <> @@ -39,9 +38,7 @@ const Image: React.FC<{ file?: DownloadFile } & BaseProps> = ({ file, style, ima source={source} resizeMode={resizeMode || FastImage.resizeMode.contain} style={[{ height: style?.height, width: style?.width }, imageStyle]} - onError={() => { - setSource(require('@res/fallback.png')) - }} + onError={() => setError(true)} /> = ({ file, style, ima /> ) -} +}) const ArtistImage = React.memo(props => { const file = useArtistCoverArtFile(props.artistId) @@ -65,31 +62,24 @@ const CoverArtImage = React.memo(props => { return }) -const CoverArt: React.FC = props => { +const CoverArt = React.memo(props => { const viewStyles = [props.style] if (props.round) { viewStyles.push(styles.round) } - const coverArtImage = useCallback(() => , [props]) - const artistImage = useCallback(() => , [props]) - - let ImageComponent + let imageComponent switch (props.type) { case 'artist': - ImageComponent = artistImage + imageComponent = break default: - ImageComponent = coverArtImage + imageComponent = break } - return ( - - - - ) -} + return {imageComponent} +}) const styles = StyleSheet.create({ round: { @@ -103,4 +93,4 @@ const styles = StyleSheet.create({ }, }) -export default React.memo(CoverArt) +export default CoverArt diff --git a/app/hooks/music.ts b/app/hooks/music.ts index 3b2962b..b9dee9c 100644 --- a/app/hooks/music.ts +++ b/app/hooks/music.ts @@ -1,15 +1,17 @@ +import { selectCache } from '@app/state/cache' import { selectMusic } from '@app/state/music' import { Store, useStore } from '@app/state/store' -import { useCallback } from 'react' +import { useCallback, useEffect } from 'react' export const useArtistInfo = (id: string) => { const artistInfo = useStore(useCallback((state: Store) => state.artistInfo[id], [id])) const fetchArtistInfo = useStore(selectMusic.fetchArtistInfo) - if (!artistInfo) { - fetchArtistInfo(id) - return undefined - } + useEffect(() => { + if (!artistInfo) { + fetchArtistInfo(id) + } + }) return artistInfo } @@ -18,9 +20,11 @@ export const useAlbumWithSongs = (id: string) => { const album = useStore(useCallback((state: Store) => state.albumsWithSongs[id], [id])) const fetchAlbum = useStore(selectMusic.fetchAlbumWithSongs) - if (!album) { - fetchAlbum(id) - } + useEffect(() => { + if (!album) { + fetchAlbum(id) + } + }) return album } @@ -29,9 +33,11 @@ export const usePlaylistWithSongs = (id: string) => { const playlist = useStore(useCallback((state: Store) => state.playlistsWithSongs[id], [id])) const fetchPlaylist = useStore(selectMusic.fetchPlaylistWithSongs) - if (!playlist) { - fetchPlaylist(id) - } + useEffect(() => { + if (!playlist) { + fetchPlaylist(id) + } + }) return playlist } @@ -58,12 +64,13 @@ export const useStarred = (id: string, type: string) => { export const useCoverArtFile = (coverArt: string = '-1') => { const file = useStore(useCallback((state: Store) => state.cachedCoverArt[coverArt], [coverArt])) - const cacheCoverArt = useStore(selectMusic.cacheCoverArt) + const cacheCoverArt = useStore(selectCache.cacheCoverArt) - if (!file) { - cacheCoverArt(coverArt) - return undefined - } + useEffect(() => { + if (!file) { + cacheCoverArt(coverArt) + } + }) return file } @@ -71,16 +78,13 @@ export const useCoverArtFile = (coverArt: string = '-1') => { export const useArtistCoverArtFile = (artistId: string) => { const artistInfo = useArtistInfo(artistId) const file = useStore(useCallback((state: Store) => state.cachedArtistArt[artistId], [artistId])) - const cacheArtistArt = useStore(selectMusic.cacheArtistArt) + const cacheArtistArt = useStore(selectCache.cacheArtistArt) - if (!artistInfo) { - return undefined - } - - if (!file) { - cacheArtistArt(artistId, artistInfo.largeImageUrl) - return undefined - } + useEffect(() => { + if (!file && artistInfo) { + cacheArtistArt(artistId, artistInfo.largeImageUrl) + } + }) return file } diff --git a/app/hooks/trackplayer.ts b/app/hooks/trackplayer.ts index 1f72fe1..55cbe3d 100644 --- a/app/hooks/trackplayer.ts +++ b/app/hooks/trackplayer.ts @@ -1,5 +1,5 @@ import { Song } from '@app/models/music' -import { selectMusic } from '@app/state/music' +import { selectCache } from '@app/state/cache' import { useStore } from '@app/state/store' import { getCurrentTrack, @@ -191,7 +191,7 @@ export const useSetQueue = () => { const getQueueShuffled = useCallback(() => !!useStore.getState().shuffleOrder, []) const setQueueContextType = useStore(selectTrackPlayer.setQueueContextType) const setQueueContextId = useStore(selectTrackPlayer.setQueueContextId) - const getCoverArtPath = useStore(selectMusic.getCoverArtPath) + const getCoverArtPath = useStore(selectCache.getCoverArtPath) return async ( songs: Song[], diff --git a/app/state/cache.ts b/app/state/cache.ts new file mode 100644 index 0000000..7f5614c --- /dev/null +++ b/app/state/cache.ts @@ -0,0 +1,234 @@ +import { Song } from '@app/models/music' +import PromiseQueue from '@app/util/PromiseQueue' +import { SetState, GetState } from 'zustand' +import { Store } from './store' +import produce from 'immer' +import RNFS from 'react-native-fs' + +const imageDownloadQueue = new PromiseQueue(10) + +export type DownloadFile = { + path: string + date: number + progress: number + promise?: Promise +} + +export type CacheSlice = { + coverArtDir?: string + artistArtDir?: string + songsDir?: string + + cachedCoverArt: { [coverArt: string]: DownloadFile } + downloadedCoverArt: { [coverArt: string]: DownloadFile } + + cacheCoverArt: (coverArt: string) => Promise + getCoverArtPath: (coverArt: string) => Promise + + cachedArtistArt: { [artistId: string]: DownloadFile } + downloadedArtistArt: { [artistId: string]: DownloadFile } + + cacheArtistArt: (artistId: string, url?: string) => Promise + + cachedSongs: { [id: string]: DownloadFile } + downloadedSongs: { [id: string]: DownloadFile } + + albumCoverArt: { [id: string]: string | undefined } + albumCoverArtRequests: { [id: string]: Promise } + fetchAlbumCoverArt: (id: string) => Promise + getAlbumCoverArt: (id: string | undefined) => Promise + mapSongCoverArtFromAlbum: (songs: Song[]) => Promise +} + +export const selectCache = { + cacheCoverArt: (store: CacheSlice) => store.cacheCoverArt, + getCoverArtPath: (store: CacheSlice) => store.getCoverArtPath, + cacheArtistArt: (store: CacheSlice) => store.cacheArtistArt, + + fetchAlbumCoverArt: (store: CacheSlice) => store.fetchAlbumCoverArt, +} + +export const createCacheSlice = (set: SetState, get: GetState): CacheSlice => ({ + cachedCoverArt: {}, + downloadedCoverArt: {}, + + cacheCoverArt: async coverArt => { + const client = get().client + if (!client) { + return + } + + const path = `${get().coverArtDir}/${coverArt}` + + const existing = get().cachedCoverArt[coverArt] + if (existing) { + if (existing.promise !== undefined) { + return await existing.promise + } else { + return + } + } + + const promise = imageDownloadQueue + .enqueue( + () => + RNFS.downloadFile({ + fromUrl: client.getCoverArtUri({ id: coverArt }), + toFile: path, + }).promise, + ) + .then(() => { + set( + produce(state => { + state.cachedCoverArt[coverArt].progress = 1 + delete state.cachedCoverArt[coverArt].promise + }), + ) + }) + set( + produce(state => { + state.cachedCoverArt[coverArt] = { + path, + date: Date.now(), + progress: 0, + promise, + } + }), + ) + return await promise + }, + + getCoverArtPath: async coverArt => { + const existing = get().cachedCoverArt[coverArt] + if (existing) { + if (existing.promise) { + await existing.promise + } + return existing.path + } + + await get().cacheCoverArt(coverArt) + return get().cachedCoverArt[coverArt].path + }, + + cachedArtistArt: {}, + downloadedArtistArt: {}, + + cacheArtistArt: async (artistId, url) => { + if (!url) { + return + } + + const client = get().client + if (!client) { + return + } + + const path = `${get().artistArtDir}/${artistId}` + + const existing = get().cachedArtistArt[artistId] + if (existing) { + if (existing.promise !== undefined) { + return await existing.promise + } else { + return + } + } + + const promise = imageDownloadQueue + .enqueue( + () => + RNFS.downloadFile({ + fromUrl: url, + toFile: path, + }).promise, + ) + .then(() => { + set( + produce(state => { + state.cachedArtistArt[artistId].progress = 1 + delete state.cachedArtistArt[artistId].promise + }), + ) + }) + set( + produce(state => { + state.cachedArtistArt[artistId] = { + path, + date: Date.now(), + progress: 0, + promise, + } + }), + ) + }, + + cachedSongs: {}, + downloadedSongs: {}, + + albumCoverArt: {}, + albumCoverArtRequests: {}, + + fetchAlbumCoverArt: async id => { + const client = get().client + if (!client) { + return + } + + const inProgress = get().albumCoverArtRequests[id] + if (inProgress !== undefined) { + return await inProgress + } + + const promise = new Promise(async resolve => { + try { + const response = await client.getAlbum({ id }) + set( + produce(state => { + state.albumCoverArt[id] = response.data.album.coverArt + }), + ) + } finally { + resolve() + } + }).then(() => { + set( + produce(state => { + delete state.albumCoverArtRequests[id] + }), + ) + }) + set( + produce(state => { + state.albumCoverArtRequests[id] = promise + }), + ) + + return await promise + }, + + getAlbumCoverArt: async id => { + if (!id) { + return + } + + const existing = get().albumCoverArt[id] + if (existing) { + return existing + } + + await get().fetchAlbumCoverArt(id) + return get().albumCoverArt[id] + }, + + mapSongCoverArtFromAlbum: async songs => { + const mapped: Song[] = [] + for (const s of songs) { + mapped.push({ + ...s, + coverArt: await get().getAlbumCoverArt(s.albumId), + }) + } + return mapped + }, +}) diff --git a/app/state/music.ts b/app/state/music.ts index 05586f9..56bf653 100644 --- a/app/state/music.ts +++ b/app/state/music.ts @@ -14,24 +14,12 @@ import { PlaylistListItem, PlaylistWithSongs, SearchResults, - Song, } from '@app/models/music' import { Store } from '@app/state/store' import { GetAlbumList2Type, StarParams } from '@app/subsonic/params' -import PromiseQueue from '@app/util/PromiseQueue' import produce from 'immer' -import RNFS from 'react-native-fs' import { GetState, SetState } from 'zustand' -const imageDownloadQueue = new PromiseQueue(5) - -export type DownloadFile = { - path: string - date: number - progress: number - promise?: Promise -} - export type MusicSlice = { // // family-style state @@ -70,29 +58,6 @@ export type MusicSlice = { fetchHomeLists: () => Promise clearHomeLists: () => void - // - // downloads - // - coverArtDir?: string - artistArtDir?: string - songsDir?: string - - cachedCoverArt: { [coverArt: string]: DownloadFile } - downloadedCoverArt: { [coverArt: string]: DownloadFile } - - coverArtRequests: { [coverArt: string]: Promise } - - cacheCoverArt: (coverArt: string) => Promise - getCoverArtPath: (coverArt: string) => Promise - - cachedArtistArt: { [artistId: string]: DownloadFile } - downloadedArtistArt: { [artistId: string]: DownloadFile } - - cacheArtistArt: (artistId: string, url?: string) => Promise - - cachedSongs: { [id: string]: DownloadFile } - downloadedSongs: { [id: string]: DownloadFile } - // // actions, etc. // @@ -100,12 +65,6 @@ export type MusicSlice = { starredAlbums: { [id: string]: boolean } starredArtists: { [id: string]: boolean } starItem: (id: string, type: string, unstar?: boolean) => Promise - - albumCoverArt: { [id: string]: string | undefined } - albumCoverArtRequests: { [id: string]: Promise } - fetchAlbumCoverArt: (id: string) => Promise - getAlbumCoverArt: (id: string | undefined) => Promise - mapSongCoverArtFromAlbum: (songs: Song[]) => Promise } export const selectMusic = { @@ -135,12 +94,7 @@ export const selectMusic = { fetchHomeLists: (store: MusicSlice) => store.fetchHomeLists, clearHomeLists: (store: MusicSlice) => store.clearHomeLists, - cacheCoverArt: (store: MusicSlice) => store.cacheCoverArt, - getCoverArtPath: (store: MusicSlice) => store.getCoverArtPath, - cacheArtistArt: (store: MusicSlice) => store.cacheArtistArt, - starItem: (store: MusicSlice) => store.starItem, - fetchAlbumCoverArt: (store: MusicSlice) => store.fetchAlbumCoverArt, } function reduceStarred( @@ -416,110 +370,6 @@ export const createMusicSlice = (set: SetState, get: GetState): Mu set({ homeLists: {} }) }, - cachedCoverArt: {}, - downloadedCoverArt: {}, - - coverArtRequests: {}, - - cacheCoverArt: async coverArt => { - const client = get().client - if (!client) { - return - } - - const path = `${get().coverArtDir}/${coverArt}` - - const existing = get().cachedCoverArt[coverArt] - if (existing) { - if (existing.promise !== undefined) { - return await existing.promise - } else { - return - } - } - - const promise = imageDownloadQueue - .enqueue(() => - RNFS.downloadFile({ - fromUrl: client.getCoverArtUri({ id: coverArt }), - toFile: path, - }).promise.then(() => new Promise(resolve => setTimeout(resolve, 100))), - ) - .then(() => { - set( - produce(state => { - state.cachedCoverArt[coverArt].progress = 1 - delete state.cachedCoverArt[coverArt].promise - }), - ) - }) - set( - produce(state => { - state.cachedCoverArt[coverArt] = { - path, - date: Date.now(), - progress: 0, - promise, - } - }), - ) - return await promise - }, - - getCoverArtPath: async coverArt => { - const existing = get().cachedCoverArt[coverArt] - if (existing) { - if (existing.promise) { - await existing.promise - } - return existing.path - } - - await get().cacheCoverArt(coverArt) - return get().cachedCoverArt[coverArt].path - }, - - cachedArtistArt: {}, - downloadedArtistArt: {}, - - cacheArtistArt: async (artistId, url) => { - if (!url) { - return - } - - const client = get().client - if (!client) { - return - } - - const path = `${get().artistArtDir}/${artistId}` - - set( - produce(state => { - state.cachedArtistArt[artistId] = { - path, - date: Date.now(), - progress: 0, - } - }), - ) - await imageDownloadQueue.enqueue( - () => - RNFS.downloadFile({ - fromUrl: url, - toFile: path, - }).promise, - ) - set( - produce(state => { - state.cachedArtistArt[artistId].progress = 1 - }), - ) - }, - - cachedSongs: {}, - downloadedSongs: {}, - starredSongs: {}, starredAlbums: {}, starredArtists: {}, @@ -579,70 +429,4 @@ export const createMusicSlice = (set: SetState, get: GetState): Mu setStarred(unstar) } }, - - albumCoverArt: {}, - albumCoverArtRequests: {}, - - fetchAlbumCoverArt: async id => { - const client = get().client - if (!client) { - return - } - - const inProgress = get().albumCoverArtRequests[id] - if (inProgress !== undefined) { - return await inProgress - } - - const promise = new Promise(async resolve => { - try { - const response = await client.getAlbum({ id }) - set( - produce(state => { - state.albumCoverArt[id] = response.data.album.coverArt - }), - ) - } finally { - resolve() - } - }).then(() => { - set( - produce(state => { - delete state.albumCoverArtRequests[id] - }), - ) - }) - set( - produce(state => { - state.albumCoverArtRequests[id] = promise - }), - ) - - return await promise - }, - - getAlbumCoverArt: async id => { - if (!id) { - return - } - - const existing = get().albumCoverArt[id] - if (existing) { - return existing - } - - await get().fetchAlbumCoverArt(id) - return get().albumCoverArt[id] - }, - - mapSongCoverArtFromAlbum: async songs => { - const mapped: Song[] = [] - for (const s of songs) { - mapped.push({ - ...s, - coverArt: await get().getAlbumCoverArt(s.albumId), - }) - } - return mapped - }, }) diff --git a/app/state/store.ts b/app/state/store.ts index 05a8be5..80a05cf 100644 --- a/app/state/store.ts +++ b/app/state/store.ts @@ -3,11 +3,13 @@ import { createSettingsSlice, SettingsSlice } from '@app/state/settings' import AsyncStorage from '@react-native-async-storage/async-storage' import create from 'zustand' import { persist, StateStorage } from 'zustand/middleware' +import { CacheSlice, createCacheSlice } from './cache' import { createTrackPlayerSlice, TrackPlayerSlice } from './trackplayer' export type Store = SettingsSlice & MusicSlice & - TrackPlayerSlice & { + TrackPlayerSlice & + CacheSlice & { hydrated: boolean setHydrated: (hydrated: boolean) => void } @@ -36,6 +38,7 @@ export const useStore = create( ...createSettingsSlice(set, get), ...createMusicSlice(set, get), ...createTrackPlayerSlice(set, get), + ...createCacheSlice(set, get), hydrated: false, setHydrated: hydrated => set({ hydrated }), diff --git a/app/subsonic/api.ts b/app/subsonic/api.ts index e8634b4..831855c 100644 --- a/app/subsonic/api.ts +++ b/app/subsonic/api.ts @@ -83,7 +83,7 @@ export class SubsonicApiClient { } const url = `${this.address}/rest/${method}?${query}` - console.log(`${method}: ${url}`) + // console.log(`${method}: ${url}`) return url }