From c3fcd486b3af97a2493190f9ec35febf6675f1ce Mon Sep 17 00:00:00 2001 From: Hailey Date: Wed, 24 Apr 2024 17:12:36 -0700 Subject: [PATCH] Cleanup files after each iteration of compression and downloading (#3599) * delete image on each iteration of compression * replace a few other instances of `unlink()` * ensure that moving to the permanent path will succeed * use `cacheDirectory` * missing file extension? * assert * Remove extra . * Extract safeDeleteAsync, fix normalization * Normalize everywhere * Use safeDeleteAsync in more places * Delete .bin too --------- Co-authored-by: Dan Abramov --- src/lib/api/index.ts | 19 ++++------------ src/lib/media/manip.ts | 51 ++++++++++++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/lib/api/index.ts b/src/lib/api/index.ts index cc86ce33..bc50f9cb 100644 --- a/src/lib/api/index.ts +++ b/src/lib/api/index.ts @@ -1,4 +1,3 @@ -import {deleteAsync} from 'expo-file-system' import { AppBskyEmbedExternal, AppBskyEmbedImages, @@ -20,6 +19,7 @@ import {shortenLinks} from 'lib/strings/rich-text-manip' import {isNative, isWeb} from 'platform/detection' import {ImageModel} from 'state/models/media/image' import {LinkMeta} from '../link-meta/link-meta' +import {safeDeleteAsync} from '../media/manip' export interface ExternalEmbedDraft { uri: string @@ -119,15 +119,9 @@ export async function post(agent: BskyAgent, opts: PostOpts) { const {width, height} = image.compressed || image logger.debug(`Uploading image`) const res = await uploadBlob(agent, path, 'image/jpeg') - if (isNative) { - try { - deleteAsync(path) - } catch (e) { - console.error(e) - } + safeDeleteAsync(path) } - images.push({ image: res.data.blob, alt: image.altText ?? '', @@ -182,13 +176,8 @@ export async function post(agent: BskyAgent, opts: PostOpts) { encoding, ) thumb = thumbUploadRes.data.blob - - try { - if (isNative) { - deleteAsync(opts.extLink.localThumb.path) - } - } catch (e) { - console.error(e) + if (isNative) { + safeDeleteAsync(opts.extLink.localThumb.path) } } } diff --git a/src/lib/media/manip.ts b/src/lib/media/manip.ts index e0adf0dd..9cd4abc6 100644 --- a/src/lib/media/manip.ts +++ b/src/lib/media/manip.ts @@ -1,7 +1,7 @@ import {Image as RNImage, Share as RNShare} from 'react-native' -import * as RNFS from 'react-native-fs' import {Image} from 'react-native-image-crop-picker' import uuid from 'react-native-uuid' +import {cacheDirectory, copyAsync, deleteAsync} from 'expo-file-system' import * as MediaLibrary from 'expo-media-library' import * as Sharing from 'expo-sharing' import ImageResizer from '@bam.tech/react-native-image-resizer' @@ -24,7 +24,10 @@ export async function compressIfNeeded( mode: 'stretch', maxSize, }) - const finalImageMovedPath = await moveToPermanentPath(resizedImage.path) + const finalImageMovedPath = await moveToPermanentPath( + resizedImage.path, + '.jpg', + ) const finalImg = { ...resizedImage, path: finalImageMovedPath, @@ -69,13 +72,10 @@ export async function downloadAndResize(opts: DownloadAndResizeOpts) { return } - let localUri = downloadRes.path() - if (!localUri.startsWith('file://')) { - localUri = `file://${localUri}` - } - + const localUri = normalizePath(downloadRes.path(), true) return await doResize(localUri, opts) } finally { + // TODO Whenever we remove `rn-fetch-blob`, we will need to replace this `flush()` with a `deleteAsync()` -hailey if (downloadRes) { downloadRes.flush() } @@ -111,7 +111,8 @@ export async function shareImageModal({uri}: {uri: string}) { UTI: 'image/png', }) } - RNFS.unlink(imagePath) + + safeDeleteAsync(imagePath) } export async function saveImageToMediaLibrary({uri}: {uri: string}) { @@ -128,6 +129,7 @@ export async function saveImageToMediaLibrary({uri}: {uri: string}) { // save await MediaLibrary.createAssetAsync(imagePath) + safeDeleteAsync(imagePath) } export function getImageDim(path: string): Promise { @@ -174,6 +176,8 @@ async function doResize(localUri: string, opts: DoResizeOpts): Promise { width: resizeRes.width, height: resizeRes.height, } + } else { + safeDeleteAsync(resizeRes.path) } } throw new Error( @@ -181,7 +185,7 @@ async function doResize(localUri: string, opts: DoResizeOpts): Promise { ) } -async function moveToPermanentPath(path: string, ext = ''): Promise { +async function moveToPermanentPath(path: string, ext = 'jpg'): Promise { /* Since this package stores images in a temp directory, we need to move the file to a permanent location. Relevant: IOS bug when trying to open a second time: @@ -189,14 +193,33 @@ async function moveToPermanentPath(path: string, ext = ''): Promise { */ const filename = uuid.v4() - const destinationPath = joinPath( - RNFS.TemporaryDirectoryPath, - `${filename}${ext}`, - ) - await RNFS.moveFile(path, destinationPath) + // cacheDirectory will not ever be null on native, but it could be on web. This function only ever gets called on + // native so we assert as a string. + const destinationPath = joinPath(cacheDirectory as string, filename + ext) + await copyAsync({ + from: normalizePath(path), + to: normalizePath(destinationPath), + }) + safeDeleteAsync(path) return normalizePath(destinationPath) } +export async function safeDeleteAsync(path: string) { + // Normalize is necessary for Android, otherwise it doesn't delete. + const normalizedPath = normalizePath(path) + try { + await Promise.allSettled([ + deleteAsync(normalizedPath, {idempotent: true}), + // HACK: Try this one too. Might exist due to api-polyfill hack. + deleteAsync(normalizedPath.replace(/\.jpe?g$/, '.bin'), { + idempotent: true, + }), + ]) + } catch (e) { + console.error('Failed to delete file', e) + } +} + function joinPath(a: string, b: string) { if (a.endsWith('/')) { if (b.startsWith('/')) {