diff --git a/src/components/AttachmentPicker/index.native.tsx b/src/components/AttachmentPicker/index.native.tsx index 551db86d6a73..0593cd1c4ace 100644 --- a/src/components/AttachmentPicker/index.native.tsx +++ b/src/components/AttachmentPicker/index.native.tsx @@ -11,9 +11,9 @@ import useStyleUtils from '@hooks/useStyleUtils'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; -import {cleanFileName, showCameraPermissionsAlert, verifyFileFormat} from '@libs/fileDownload/FileUtils'; +import {cleanFileName, showCameraPermissionsAlert} from '@libs/fileDownload/FileUtils'; +import processPickedAssetsSequentially from '@libs/fileDownload/processPickedAssets'; import fileURIToPath from '@libs/fileURIToPath'; -import Log from '@libs/Log'; import ReceiptStorage from '@libs/ReceiptStorage'; import {getPickerCaptureSource, logReceiptAdoptFailed} from '@libs/telemetry/ReceiptObservability'; @@ -27,7 +27,6 @@ import type {Asset, Callback, CameraOptions, ImageLibraryOptions, ImagePickerRes import {keepLocalCopy, pick, types} from '@react-native-documents/picker'; import {Str} from 'expensify-common'; -import {ImageManipulator, SaveFormat} from 'expo-image-manipulator'; import React, {useCallback, useMemo, useRef, useState} from 'react'; import {Alert, View} from 'react-native'; import RNFetchBlob from 'react-native-blob-util'; @@ -72,27 +71,6 @@ type Item = { /** Function to call when the user clicks the item */ pickAttachment: () => Promise; }; - -/** - * Ensures asset has proper fileName and type properties - */ -const processAssetWithFallbacks = (asset: Asset): Asset => { - // Generate fallback name: extract from URI if available, otherwise use timestamped default - const fallbackName = asset.uri - ? asset.uri - .substring(asset.uri.lastIndexOf('/') + 1) - .split('?') - .at(0) - : `image_${Date.now()}.jpeg`; - const fileName = asset.fileName ?? fallbackName; - return { - ...asset, - fileName, - // Default to JPEG if no type specified - type: asset.type ?? 'image/jpeg', - }; -}; - /** * Return imagePickerOptions based on the type */ @@ -225,68 +203,7 @@ function AttachmentPicker({ return resolve(); } - const processedAssets: Asset[] = []; - let processedCount = 0; - - const checkAllProcessed = () => { - processedCount++; - if (processedCount === assets.length) { - resolve(processedAssets.length > 0 ? processedAssets : undefined); - } - }; - - for (const asset of assets) { - if (!asset.uri) { - checkAllProcessed(); - continue; - } - - if (asset.type?.startsWith('image')) { - verifyFileFormat({fileUri: asset.uri, formatSignatures: CONST.HEIC_SIGNATURES}) - .then((isHEIC) => { - // react-native-image-picker incorrectly changes file extension without transcoding the HEIC file, so we are doing it manually if we detect HEIC signature - if (isHEIC && asset.uri) { - ImageManipulator.manipulate(asset.uri) - .renderAsync() - .then((manipulatedImage) => manipulatedImage.saveAsync({format: SaveFormat.JPEG})) - .then((manipulationResult) => { - const uri = manipulationResult.uri; - const convertedAsset = { - uri, - name: uri - .substring(uri.lastIndexOf('/') + 1) - .split('?') - .at(0), - type: 'image/jpeg', - width: manipulationResult.width, - height: manipulationResult.height, - }; - processedAssets.push(convertedAsset); - checkAllProcessed(); - }) - .catch((error: Error) => { - Log.warn('Failed to convert HEIC image, skipping asset', {error: error.message}); - showGeneralAlert(translate('attachmentPicker.errorWhileConvertingHeic')); - checkAllProcessed(); - }); - } else { - // Ensure the asset has proper fileName and type for non-HEIC images - const processedAsset = processAssetWithFallbacks(asset); - processedAssets.push(processedAsset); - checkAllProcessed(); - } - }) - .catch((error: Error) => { - showGeneralAlert(error.message ?? 'An unknown error occurred'); - checkAllProcessed(); - }); - } else { - // Ensure the asset has proper fileName and type - const processedAsset = processAssetWithFallbacks(asset); - processedAssets.push(processedAsset); - checkAllProcessed(); - } - } + processPickedAssetsSequentially(assets, showGeneralAlert, translate).then(resolve).catch(reject); }); }), [fileLimit, showGeneralAlert, translate, type], diff --git a/src/libs/fileDownload/processPickedAssets.ts b/src/libs/fileDownload/processPickedAssets.ts new file mode 100644 index 000000000000..5e93d4bb5ff7 --- /dev/null +++ b/src/libs/fileDownload/processPickedAssets.ts @@ -0,0 +1,133 @@ +import type {LocaleContextProps} from '@components/LocaleContextProvider'; + +import Log from '@libs/Log'; + +import CONST from '@src/CONST'; + +import type {Asset} from 'react-native-image-picker'; + +import {ImageManipulator, SaveFormat} from 'expo-image-manipulator'; + +import {verifyFileFormat} from './FileUtils'; + +/** + * Ensures asset has proper fileName and type properties + */ +const processAssetWithFallbacks = (asset: Asset): Asset => { + // Generate fallback name: extract from URI if available, otherwise use timestamped default + const fallbackName = asset.uri + ? asset.uri + .substring(asset.uri.lastIndexOf('/') + 1) + .split('?') + .at(0) + : `image_${Date.now()}.jpeg`; + const fileName = asset.fileName ?? fallbackName; + return { + ...asset, + fileName, + // Default to JPEG if no type specified + type: asset.type ?? 'image/jpeg', + }; +}; + +const getErrorMessage = (error: unknown, fallback: string): string => (error instanceof Error && error.message ? error.message : fallback); + +/** + * Frees a native image resource. Releasing is best-effort cleanup, so a failure here must never change + * whether the converted asset is kept. + */ +const releaseQuietly = (releasable: {release: () => void}) => { + try { + releasable.release(); + } catch (error) { + Log.warn('Failed to release native image resource', {error: getErrorMessage(error, 'An unknown error occurred')}); + } +}; + +/** + * Convert the picked assets one at a time, transcoding any HEIC images to JPEG. + * + * The conversion is deliberately sequential: `ImageManipulator` decodes each image into a full-size + * bitmap in native memory, so converting a whole selection at once (the picker allows up to + * `CONST.API_ATTACHMENT_VALIDATIONS.MAX_FILE_LIMIT` files) holds every bitmap simultaneously and the + * OS terminates the app for exceeding its memory limit. Processing one image at a time keeps the peak + * at a single bitmap regardless of how many files were picked. + */ +const processPickedAssetsSequentially = async (assets: Asset[], showGeneralAlert: (message?: string) => void, translate: LocaleContextProps['translate']): Promise => { + const processedAssets: Asset[] = []; + // Collected instead of alerted inline so the whole selection produces a single alert: alerting per + // asset would leave the user dismissing one native modal after another. + const failureMessages = new Set(); + + for (const asset of assets) { + if (!asset.uri) { + continue; + } + + if (!asset.type?.startsWith('image')) { + // Ensure the asset has proper fileName and type + processedAssets.push(processAssetWithFallbacks(asset)); + continue; + } + + try { + // eslint-disable-next-line no-await-in-loop -- converting one image at a time is the point, see the doc comment above + const isHEIC = await verifyFileFormat({fileUri: asset.uri, formatSignatures: CONST.HEIC_SIGNATURES}); + + if (!isHEIC) { + // Ensure the asset has proper fileName and type for non-HEIC images + processedAssets.push(processAssetWithFallbacks(asset)); + continue; + } + + // react-native-image-picker incorrectly changes file extension without transcoding the HEIC file, so we are doing it manually if we detect HEIC signature + const imageManipulatorContext = ImageManipulator.manipulate(asset.uri); + // Decided on the conversion result rather than inside the try, so cleanup can never determine + // whether the asset is kept. + let convertedAsset: Asset | undefined; + try { + // eslint-disable-next-line no-await-in-loop -- converting one image at a time is the point, see the doc comment above + const manipulatedImage = await imageManipulatorContext.renderAsync(); + try { + // eslint-disable-next-line no-await-in-loop -- converting one image at a time is the point, see the doc comment above + const manipulationResult = await manipulatedImage.saveAsync({format: SaveFormat.JPEG}); + const uri = manipulationResult.uri; + const fileName = + uri + .substring(uri.lastIndexOf('/') + 1) + .split('?') + .at(0) ?? ''; + convertedAsset = { + uri, + fileName, + type: 'image/jpeg', + width: manipulationResult.width, + height: manipulationResult.height, + }; + } finally { + releaseQuietly(manipulatedImage); + } + } catch (error) { + Log.warn('Failed to convert HEIC image, skipping asset', {error: getErrorMessage(error, 'An unknown error occurred')}); + } finally { + releaseQuietly(imageManipulatorContext); + } + + if (convertedAsset) { + processedAssets.push(convertedAsset); + } else { + failureMessages.add(translate('attachmentPicker.errorWhileConvertingHeic')); + } + } catch (error) { + failureMessages.add(getErrorMessage(error, translate('attachmentPicker.errorWhileSelectingAttachment'))); + } + } + + if (failureMessages.size > 0) { + showGeneralAlert([...failureMessages].join('\n')); + } + + return processedAssets.length > 0 ? processedAssets : undefined; +}; + +export default processPickedAssetsSequentially; diff --git a/tests/unit/AttachmentPickerAssetProcessingTest.ts b/tests/unit/AttachmentPickerAssetProcessingTest.ts new file mode 100644 index 000000000000..d77513eac74d --- /dev/null +++ b/tests/unit/AttachmentPickerAssetProcessingTest.ts @@ -0,0 +1,196 @@ +import type {LocaleContextProps} from '@components/LocaleContextProvider'; + +import processPickedAssetsSequentially from '@libs/fileDownload/processPickedAssets'; + +import type {Asset} from 'react-native-image-picker'; + +const mockVerifyFileFormat = jest.fn(); +const mockRenderAsync = jest.fn(); +const mockSaveAsync = jest.fn(); +const mockRelease = jest.fn(); +const mockImageRelease = jest.fn(); + +jest.mock('@libs/fileDownload/FileUtils', () => ({ + verifyFileFormat: () => mockVerifyFileFormat() as unknown, +})); + +jest.mock('expo-image-manipulator', () => ({ + ImageManipulator: { + manipulate: () => ({ + renderAsync: () => mockRenderAsync() as unknown, + release: () => { + mockRelease(); + }, + }), + }, + SaveFormat: {JPEG: 'jpeg'}, +})); + +jest.mock('@libs/Log', () => ({ + info: jest.fn(), + warn: jest.fn(), +})); + +const buildHeicAssets = (count: number): Asset[] => + Array.from({length: count}, (value, index) => ({ + uri: `file:///photo-${index}.heic`, + fileName: `photo-${index}.heic`, + type: 'image/heic', + })); + +const showGeneralAlert = jest.fn(); +// Returns the key itself so assertions can tell the different failure messages apart. +const translate: LocaleContextProps['translate'] = (path, ...parameters): string => (parameters.length > 0 ? `${path}:${parameters.length}` : path); + +describe('processPickedAssetsSequentially', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockVerifyFileFormat.mockResolvedValue(true); + mockSaveAsync.mockResolvedValue({uri: 'file:///photo.jpg', width: 100, height: 200}); + mockRenderAsync.mockImplementation(() => + Promise.resolve({ + saveAsync: () => mockSaveAsync() as unknown, + release: () => { + mockImageRelease(); + }, + }), + ); + }); + + it('decodes only one image at a time', async () => { + let inFlight = 0; + let peakInFlight = 0; + + // Hold each decode open long enough that any overlap would be observable in `peakInFlight`. + mockRenderAsync.mockImplementation(() => { + inFlight++; + peakInFlight = Math.max(peakInFlight, inFlight); + return new Promise((resolve) => { + setImmediate(() => { + inFlight--; + resolve({ + saveAsync: () => mockSaveAsync() as unknown, + release: () => { + mockImageRelease(); + }, + }); + }); + }); + }); + + const result = await processPickedAssetsSequentially(buildHeicAssets(30), showGeneralAlert, translate); + + expect(peakInFlight).toBe(1); + expect(mockRenderAsync).toHaveBeenCalledTimes(30); + expect(result).toHaveLength(30); + }); + + it('releases the native image resources for every converted asset', async () => { + await processPickedAssetsSequentially(buildHeicAssets(5), showGeneralAlert, translate); + + expect(mockRelease).toHaveBeenCalledTimes(5); + expect(mockImageRelease).toHaveBeenCalledTimes(5); + }); + + it('releases the manipulator context even when the conversion fails', async () => { + mockRenderAsync.mockRejectedValue(new Error('decode failed')); + + await processPickedAssetsSequentially(buildHeicAssets(3), showGeneralAlert, translate); + + expect(mockRelease).toHaveBeenCalledTimes(3); + }); + + it('skips assets that fail to convert instead of uploading the raw HEIC', async () => { + mockRenderAsync.mockRejectedValue(new Error('decode failed')); + + const result = await processPickedAssetsSequentially(buildHeicAssets(3), showGeneralAlert, translate); + + expect(result).toBeUndefined(); + }); + + it('shows a single alert when the whole selection fails the same way', async () => { + mockRenderAsync.mockRejectedValue(new Error('decode failed')); + + await processPickedAssetsSequentially(buildHeicAssets(30), showGeneralAlert, translate); + + expect(showGeneralAlert).toHaveBeenCalledTimes(1); + }); + + it('passes non-HEIC images through without decoding them', async () => { + mockVerifyFileFormat.mockResolvedValue(false); + + const result = await processPickedAssetsSequentially([{uri: 'file:///photo.jpg', fileName: 'photo.jpg', type: 'image/jpeg'}], showGeneralAlert, translate); + + expect(mockRenderAsync).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + }); + it('preserves selection order across mixed HEIC and non-HEIC assets', async () => { + mockVerifyFileFormat.mockResolvedValueOnce(true).mockResolvedValueOnce(false).mockResolvedValueOnce(true); + mockSaveAsync.mockResolvedValueOnce({uri: 'file:///a-converted.jpg', width: 1, height: 1}).mockResolvedValueOnce({uri: 'file:///c-converted.jpg', width: 1, height: 1}); + + const result = await processPickedAssetsSequentially( + [ + {uri: 'file:///a.heic', fileName: 'a.heic', type: 'image/heic'}, + {uri: 'file:///b.jpg', fileName: 'b.jpg', type: 'image/jpeg'}, + {uri: 'file:///c.heic', fileName: 'c.heic', type: 'image/heic'}, + ], + showGeneralAlert, + translate, + ); + + expect(result?.map((asset) => asset.fileName)).toEqual(['a-converted.jpg', 'b.jpg', 'c-converted.jpg']); + }); + + it('skips assets that have no uri', async () => { + const result = await processPickedAssetsSequentially([{fileName: 'no-uri.heic', type: 'image/heic'}], showGeneralAlert, translate); + + expect(mockVerifyFileFormat).not.toHaveBeenCalled(); + expect(result).toBeUndefined(); + }); + + it('passes non-image assets through without checking the file format', async () => { + const result = await processPickedAssetsSequentially([{uri: 'file:///doc.pdf', fileName: 'doc.pdf', type: 'application/pdf'}], showGeneralAlert, translate); + + expect(mockVerifyFileFormat).not.toHaveBeenCalled(); + expect(result?.at(0)?.fileName).toBe('doc.pdf'); + }); + + it('surfaces the underlying message when the format check fails', async () => { + mockVerifyFileFormat.mockRejectedValueOnce(new Error('format check failed')); + + await processPickedAssetsSequentially(buildHeicAssets(1), showGeneralAlert, translate); + + expect(showGeneralAlert).toHaveBeenCalledWith('format check failed'); + }); + + it('falls back to localized copy when the failure is not an Error', async () => { + mockVerifyFileFormat.mockRejectedValueOnce('not an error object'); + + await processPickedAssetsSequentially(buildHeicAssets(1), showGeneralAlert, translate); + + expect(showGeneralAlert).toHaveBeenCalledWith('attachmentPicker.errorWhileSelectingAttachment'); + }); + + it('shows one alert even when the selection fails in different ways', async () => { + mockVerifyFileFormat.mockRejectedValueOnce(new Error('format check failed')); + mockRenderAsync.mockRejectedValue(new Error('decode failed')); + + await processPickedAssetsSequentially(buildHeicAssets(2), showGeneralAlert, translate); + + expect(showGeneralAlert).toHaveBeenCalledTimes(1); + expect(showGeneralAlert).toHaveBeenCalledWith('format check failed\nattachmentPicker.errorWhileConvertingHeic'); + }); + it.each([ + ['the rendered image', () => mockImageRelease], + ['the manipulator context', () => mockRelease], + ])('keeps a converted asset even if releasing %s throws', async (name, getMock) => { + getMock().mockImplementation(() => { + throw new Error('release blew up'); + }); + + const result = await processPickedAssetsSequentially(buildHeicAssets(1), showGeneralAlert, translate); + + expect(result).toHaveLength(1); + expect(showGeneralAlert).not.toHaveBeenCalled(); + }); +});