diff --git a/__tests__/lib/images.test.ts b/__tests__/lib/images.test.ts index f0f948365c..0ba18ecc0e 100644 --- a/__tests__/lib/images.test.ts +++ b/__tests__/lib/images.test.ts @@ -1,5 +1,9 @@ -import {createDownloadResumable, deleteAsync} from 'expo-file-system/legacy' -import {manipulateAsync, SaveFormat} from 'expo-image-manipulator' +import { + createDownloadResumable, + deleteAsync, + getInfoAsync, +} from 'expo-file-system/legacy' +import {ImageManipulator, SaveFormat} from 'expo-image-manipulator' import {IMAGE_SIZE_CONFIG_2K_1MB} from '../../src/lib/constants' import { @@ -9,7 +13,6 @@ import { import {getResizedDimensions} from '../../src/lib/media/util' const mockResizedImage = { - path: 'file://resized-image.jpg', size: 100, width: 100, height: 100, @@ -20,10 +23,26 @@ describe('downloadAndResize', () => { const errorSpy = jest.spyOn(global.console, 'error') beforeEach(() => { - const mockedCreateResizedImage = manipulateAsync as jest.Mock - mockedCreateResizedImage.mockResolvedValue({ - uri: 'file://resized-image.jpg', - ...mockResizedImage, + let savedImageCount = 0 + const mockedManipulate = ImageManipulator.manipulate as jest.Mock + mockedManipulate.mockImplementation(() => { + const image = { + ...mockResizedImage, + release: jest.fn(), + uri: 'file://rendered-image.jpg', + saveAsync: jest.fn().mockImplementation(() => { + savedImageCount += 1 + return Promise.resolve({ + uri: `file://resized-image-${savedImageCount}.jpg`, + ...mockResizedImage, + }) + }), + } + return { + release: jest.fn(), + renderAsync: jest.fn().mockResolvedValue(image), + resize: jest.fn(), + } }) }) @@ -48,7 +67,10 @@ describe('downloadAndResize', () => { } const result = await downloadAndResize(opts) - expect(result).toEqual(mockResizedImage) + expect(result).toEqual({ + ...mockResizedImage, + path: 'file://resized-image-7.jpg', + }) expect(createDownloadResumable).toHaveBeenCalledWith( opts.uri, expect.anything(), @@ -57,20 +79,98 @@ describe('downloadAndResize', () => { }, ) - // First time it gets called is to get dimensions - expect(manipulateAsync).toHaveBeenCalledWith(expect.any(String), [], {}) + // First time it gets called is to get dimensions. + expect(ImageManipulator.manipulate).toHaveBeenNthCalledWith( + 1, + expect.any(String), + ) + const firstContext = (ImageManipulator.manipulate as jest.Mock).mock + .results[0].value + expect(firstContext.resize).not.toHaveBeenCalled() + // The mocked source image is 100x100, below maxDimension, so it is not // downsized. - expect(manipulateAsync).toHaveBeenCalledWith( - expect.any(String), - [{resize: {height: 100, width: 100}}], - {format: SaveFormat.JPEG, compress: 1.0}, + const secondContext = (ImageManipulator.manipulate as jest.Mock).mock + .results[1].value + expect(secondContext.resize).toHaveBeenCalledWith({ + height: 100, + width: 100, + }) + const lastContext = ( + ImageManipulator.manipulate as jest.Mock + ).mock.results.at(-1)!.value + const resizedImage = await lastContext.renderAsync.mock.results[0].value + expect(resizedImage.saveAsync).toHaveBeenCalledWith( + expect.objectContaining({format: SaveFormat.JPEG, compress: 1.0}), ) - expect(deleteAsync).toHaveBeenCalledWith(expect.any(String), { + const deletedPaths = (deleteAsync as jest.Mock).mock.calls.map( + ([path]) => path, + ) + expect(deletedPaths).toEqual( + expect.arrayContaining([ + 'file://resized-image-1.jpg', + 'file://resized-image-2.jpg', + 'file://resized-image-3.jpg', + 'file://resized-image-4.jpg', + 'file://resized-image-5.jpg', + 'file://resized-image-6.jpg', + ]), + ) + expect(deletedPaths).not.toContain('file://resized-image-7.jpg') + }) + + it('deletes a partial download when downloading fails', async () => { + const mockedFetch = createDownloadResumable as jest.Mock + mockedFetch.mockReturnValue({ + cancelAsync: jest.fn(), + downloadAsync: jest.fn().mockRejectedValue(new Error('download failed')), + }) + + const opts: DownloadAndResizeOpts = { + uri: 'https://example.com/image.jpg', + maxDimension: 2000, + maxSize: 500000, + timeout: 10000, + } + + await expect(downloadAndResize(opts)).rejects.toThrow('download failed') + expect(deleteAsync).toHaveBeenCalledWith(expect.stringMatching(/\.bin$/), { idempotent: true, }) }) + it('deletes every intermediate image when resizing fails', async () => { + const mockedFetch = createDownloadResumable as jest.Mock + mockedFetch.mockReturnValue({ + cancelAsync: jest.fn(), + downloadAsync: jest + .fn() + .mockResolvedValue({uri: 'file://downloaded-image.jpg'}), + }) + ;(getInfoAsync as jest.Mock) + .mockResolvedValueOnce({exists: true, size: 100}) + .mockRejectedValueOnce(new Error('stat failed')) + + const opts: DownloadAndResizeOpts = { + uri: 'https://example.com/image.jpg', + maxDimension: 2000, + maxSize: 500000, + timeout: 10000, + } + + await expect(downloadAndResize(opts)).rejects.toThrow('stat failed') + const deletedPaths = (deleteAsync as jest.Mock).mock.calls.map( + ([path]) => path, + ) + expect(deletedPaths).toEqual( + expect.arrayContaining([ + 'file://resized-image-1.jpg', + 'file://resized-image-2.jpg', + 'file://resized-image-3.jpg', + ]), + ) + }) + it('should return undefined for invalid URI', async () => { const opts: DownloadAndResizeOpts = { uri: 'invalid-uri', diff --git a/jest/jestSetup.js b/jest/jestSetup.js index 44a2c985f5..562c1063c7 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -36,15 +36,36 @@ jest.mock('expo-file-system/legacy', () => ({ createDownloadResumable: jest.fn(), })) -jest.mock('expo-image-manipulator', () => ({ - manipulateAsync: jest.fn().mockResolvedValue({ - uri: 'file://resized-image', - }), - SaveFormat: { - JPEG: 'jpeg', - WEBP: 'webp', - }, -})) +jest.mock('expo-image-manipulator', () => { + const createContext = () => { + const image = { + height: 100, + release: jest.fn(), + saveAsync: jest.fn().mockResolvedValue({ + height: 100, + uri: 'file://resized-image', + width: 100, + }), + width: 100, + } + return { + crop: jest.fn(), + release: jest.fn(), + renderAsync: jest.fn().mockResolvedValue(image), + resize: jest.fn(), + } + } + + return { + ImageManipulator: { + manipulate: jest.fn(createContext), + }, + SaveFormat: { + JPEG: 'jpeg', + WEBP: 'webp', + }, + } +}) jest.mock('expo-camera', () => ({ Camera: { diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 5dc218ca4b..e0435c5a4c 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -638,7 +638,7 @@ "count": 1 }, "typescript/no-floating-promises": { - "count": 5 + "count": 3 } }, "src/lib/media/manip.web.ts": { diff --git a/src/lib/media/image-manipulator.ts b/src/lib/media/image-manipulator.ts new file mode 100644 index 0000000000..41fde37c81 --- /dev/null +++ b/src/lib/media/image-manipulator.ts @@ -0,0 +1,27 @@ +import { + ImageManipulator, + type ImageManipulatorContext, + type ImageResult, + type SaveOptions, +} from 'expo-image-manipulator' + +export async function renderImage( + source: string, + manipulate?: (context: ImageManipulatorContext) => void, + saveOptions?: SaveOptions, +): Promise { + const context = ImageManipulator.manipulate(source) + + try { + manipulate?.(context) + const image = await context.renderAsync() + + try { + return await image.saveAsync(saveOptions) + } finally { + image.release() + } + } finally { + context.release() + } +} diff --git a/src/lib/media/manip.ts b/src/lib/media/manip.ts index 904e3c48ca..3977a861e3 100644 --- a/src/lib/media/manip.ts +++ b/src/lib/media/manip.ts @@ -12,12 +12,13 @@ import { StorageAccessFramework, writeAsStringAsync, } from 'expo-file-system/legacy' -import {manipulateAsync, SaveFormat} from 'expo-image-manipulator' +import {SaveFormat} from 'expo-image-manipulator' import * as MediaLibrary from 'expo-media-library/legacy' import * as Sharing from 'expo-sharing' import {logger} from '#/logger' import {IS_ANDROID, IS_IOS} from '#/env' +import {renderImage} from './image-manipulator' import {type PickerImage} from './picker.shared' import {type Dimensions} from './types' import {convertCdnPreset, getResizedDimensions} from './util' @@ -78,16 +79,25 @@ export async function shareImageModal({uri}: {uri: string}) { } const downloadedPath = await downloadImage(uri, String(uuid.v4()), 15e3) - const {uri: jpegUri} = await manipulateAsync(downloadedPath, [], { - format: SaveFormat.JPEG, - compress: 1.0, - }) - void safeDeleteAsync(downloadedPath) - const imagePath = await moveToPermanentPath(jpegUri, '.jpg') - await Sharing.shareAsync(imagePath, { - mimeType: 'image/jpeg', - UTI: 'image/jpeg', - }) + let jpegUri: string | undefined + let imagePath: string | undefined + + try { + const jpeg = await renderImage(downloadedPath, undefined, { + format: SaveFormat.JPEG, + compress: 1.0, + }) + jpegUri = jpeg.uri + imagePath = await moveToPermanentPath(jpegUri, '.jpg') + await Sharing.shareAsync(imagePath, { + mimeType: 'image/jpeg', + UTI: 'image/jpeg', + }) + } finally { + await safeDeleteAsync(downloadedPath) + if (jpegUri) await safeDeleteAsync(jpegUri) + if (imagePath) await safeDeleteAsync(imagePath) + } } const ALBUM_NAME = 'Bluesky' @@ -199,7 +209,7 @@ async function doResize( // Now instead, we have to supply the final dimensions to the manipulation function instead. // Performing an "empty" manipulation lets us get the dimensions of the original image. React Native's Image.getSize() // does not work for local files... - const imageRes = await manipulateAsync(localUri, [], {}) + const imageRes = await renderImage(localUri) const newDimensions = getResizedDimensions( { width: imageRes.width, @@ -210,59 +220,64 @@ async function doResize( let minQualityPercentage = 0 let maxQualityPercentage = 101 // exclusive - let newDataUri + let newDataUri: PickerImage | undefined const intermediateUris = [] - while (maxQualityPercentage - minQualityPercentage > 1) { - const qualityPercentage = Math.round( - (maxQualityPercentage + minQualityPercentage) / 2, - ) - const resizeRes = await manipulateAsync( - localUri, - [{resize: newDimensions}], - { - format: SaveFormat.JPEG, - compress: qualityPercentage / 100, - }, - ) - - intermediateUris.push(resizeRes.uri) - - const fileInfo = await getInfoAsync(resizeRes.uri) - if (!fileInfo.exists) { - throw new Error( - 'The image manipulation library failed to create a new image.', + try { + while (maxQualityPercentage - minQualityPercentage > 1) { + const qualityPercentage = Math.round( + (maxQualityPercentage + minQualityPercentage) / 2, + ) + const resizeRes = await renderImage( + localUri, + context => context.resize(newDimensions), + { + format: SaveFormat.JPEG, + compress: qualityPercentage / 100, + }, ) - } - if (fileInfo.size < opts.maxSize) { - minQualityPercentage = qualityPercentage - newDataUri = { - path: normalizePath(resizeRes.uri), - mime: 'image/jpeg', - size: fileInfo.size, - width: resizeRes.width, - height: resizeRes.height, + intermediateUris.push(resizeRes.uri) + + const fileInfo = await getInfoAsync(resizeRes.uri) + if (!fileInfo.exists) { + throw new Error( + 'The image manipulation library failed to create a new image.', + ) + } + + if (fileInfo.size < opts.maxSize) { + minQualityPercentage = qualityPercentage + newDataUri = { + path: normalizePath(resizeRes.uri), + mime: 'image/jpeg', + size: fileInfo.size, + width: resizeRes.width, + height: resizeRes.height, + } + } else { + maxQualityPercentage = qualityPercentage } - } else { - maxQualityPercentage = qualityPercentage } - } - for (const intermediateUri of intermediateUris) { - if (newDataUri?.path !== normalizePath(intermediateUri)) { - safeDeleteAsync(intermediateUri) + if (newDataUri) { + return newDataUri } - } - if (newDataUri) { - safeDeleteAsync(imageRes.uri) - return newDataUri + throw new Error( + `This image is too big! We couldn't compress it down to ${opts.maxSize} bytes`, + ) + } catch (err) { + newDataUri = undefined + throw err + } finally { + await safeDeleteAsync(imageRes.uri) + await Promise.all( + intermediateUris + .filter(uri => newDataUri?.path !== normalizePath(uri)) + .map(safeDeleteAsync), + ) } - - throw new Error( - `This image is too big! We couldn't compress it down to ${opts.maxSize} bytes`, - ) } async function moveToPermanentPath(path: string, ext: string): Promise { @@ -393,27 +408,43 @@ async function downloadImage(uri: string, destName: string, timeout: number) { const tempPath = `${cacheDirectory ?? ''}/${destName}.bin` const dlResumable = createDownloadResumable(uri, tempPath, {cache: true}) let timedOut = false + let downloadedPath: string | undefined + let finalPath: string | undefined const to1 = setTimeout(() => { timedOut = true - void dlResumable.cancelAsync() + void dlResumable.cancelAsync().catch(() => undefined) }, timeout) - const dlRes = await dlResumable.downloadAsync() - clearTimeout(to1) - - if (!dlRes?.uri) { - if (timedOut) { - throw new Error('Failed to download image - timed out') - } else { - throw new Error('Failed to download image - dlRes is undefined') + try { + let dlRes + try { + dlRes = await dlResumable.downloadAsync() + } finally { + clearTimeout(to1) } + + if (!dlRes?.uri) { + if (timedOut) { + throw new Error('Failed to download image - timed out') + } else { + throw new Error('Failed to download image - dlRes is undefined') + } + } + + downloadedPath = dlRes.uri + const ext = extFromMime(dlRes.mimeType) + finalPath = `${cacheDirectory ?? ''}/${destName}.${ext}` + await moveAsync({from: downloadedPath, to: finalPath}) + + return normalizePath(finalPath) + } catch (err) { + await Promise.all( + [...new Set([tempPath, downloadedPath, finalPath])] + .filter(path => path !== undefined) + .map(safeDeleteAsync), + ) + throw err } - - const ext = extFromMime(dlRes.mimeType) - const finalPath = `${cacheDirectory ?? ''}/${destName}.${ext}` - await moveAsync({from: dlRes.uri, to: finalPath}) - - return normalizePath(finalPath) } const MIME_TO_EXT: Record = { diff --git a/src/platform/misc.web-check.d.ts b/src/platform/misc.web-check.d.ts index d3eaf3e971..dfa41bf6a4 100644 --- a/src/platform/misc.web-check.d.ts +++ b/src/platform/misc.web-check.d.ts @@ -113,17 +113,8 @@ declare module 'expo-file-system' { declare module 'expo-image-manipulator' { import {type ImageManipulator as ImageManipulatorModule} from 'expo-image-manipulator/build/ImageManipulator.types' export const ImageManipulator: ImageManipulatorModule + export {useImageManipulator} from 'expo-image-manipulator/build/ImageManipulator' export { - manipulateAsync, - useImageManipulator, - } from 'expo-image-manipulator/build/ImageManipulator' - export { - type Action, - type ActionCrop, - type ActionExtent, - type ActionFlip, - type ActionResize, - type ActionRotate, FlipType, type ImageResult, SaveFormat, diff --git a/src/state/gallery.ts b/src/state/gallery.ts index fbb5edd4c2..15c3a9e5b6 100644 --- a/src/state/gallery.ts +++ b/src/state/gallery.ts @@ -5,14 +5,10 @@ import { makeDirectoryAsync, moveAsync, } from 'expo-file-system/legacy' -import { - type Action, - type ActionCrop, - manipulateAsync, - SaveFormat, -} from 'expo-image-manipulator' +import {type ImageManipulatorContext, SaveFormat} from 'expo-image-manipulator' import {nanoid} from 'nanoid/non-secure' +import {renderImage} from '#/lib/media/image-manipulator' import {getImageDim} from '#/lib/media/manip' import {openCropper} from '#/lib/media/picker' import {type PickerImage} from '#/lib/media/picker.shared' @@ -22,7 +18,7 @@ import {logger} from '#/logger' import {IS_NATIVE, IS_WEB} from '#/env' export type ImageTransformation = { - crop?: ActionCrop['crop'] + crop?: Parameters[0] } export type ImageMeta = { @@ -160,11 +156,8 @@ export async function manipulateImage( img: ComposerImage, trans: ImageTransformation, ): Promise { - const rawActions: (Action | undefined)[] = [trans.crop && {crop: trans.crop}] - - const actions = rawActions.filter((a): a is Action => a !== undefined) - - if (actions.length === 0) { + const crop = trans.crop + if (!crop) { if (img.transformed === undefined) { return img } @@ -173,7 +166,7 @@ export async function manipulateImage( } const source = img.source - const result = await manipulateAsync(source.path, actions, { + const result = await renderImage(source.path, context => context.crop(crop), { format: SaveFormat.PNG, }) @@ -244,9 +237,9 @@ export async function compressImage( continue } - const res = await manipulateAsync( + const res = await renderImage( source.path, - [{resize: {width: w, height: h}}], + context => context.resize({width: w, height: h}), { compress: qualityPercentage / 100, format: SaveFormat.JPEG, @@ -362,8 +355,8 @@ function blobToDataUri(blob: Blob): Promise { * media to a post. They live alongside our own `bsky-composer` dir under the OS * cache directory. expo-image-picker copies every originally selected photo and * video here, and expo-image-manipulator leaves intermediate full-resolution - * outputs here (compressImage makes several manipulateAsync passes, only the - * last of which gets moved into `bsky-composer`). Nothing else cleans these up, + * outputs here (compressImage makes several rendering passes, only the last of + * which gets moved into `bsky-composer`). Nothing else cleans these up, * so on iOS - where the OS exposes no "clear cache" - they accumulate * indefinitely, one full-resolution copy per attached item. */