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/manip.ts b/src/lib/media/manip.ts index 49136b7e96..3977a861e3 100644 --- a/src/lib/media/manip.ts +++ b/src/lib/media/manip.ts @@ -79,16 +79,25 @@ export async function shareImageModal({uri}: {uri: string}) { } const downloadedPath = await downloadImage(uri, String(uuid.v4()), 15e3) - const {uri: jpegUri} = await renderImage(downloadedPath, undefined, { - 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' @@ -211,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 renderImage( - localUri, - context => context.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 { @@ -394,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 = {