diff --git a/Mobile-Expensify b/Mobile-Expensify index 2c5d33674f1f..204f5bdfbbaa 160000 --- a/Mobile-Expensify +++ b/Mobile-Expensify @@ -1 +1 @@ -Subproject commit 2c5d33674f1fc854c79da7b1786e872ee245ca6e +Subproject commit 204f5bdfbbaac71c917a98f6f49baf3b254116f1 diff --git a/jest/setup.ts b/jest/setup.ts index 76b88f58d8e6..519a34e53e75 100644 --- a/jest/setup.ts +++ b/jest/setup.ts @@ -121,7 +121,14 @@ jest.mock('react-native-fs', () => ({ res([]); }), ), - CachesDirectoryPath: jest.fn(), + exists: jest.fn(() => Promise.resolve(false)), + mkdir: jest.fn(() => Promise.resolve()), + moveFile: jest.fn(() => Promise.resolve()), + copyFile: jest.fn(() => Promise.resolve()), + writeFile: jest.fn(() => Promise.resolve()), + DocumentDirectoryPath: '/mock/documents', + CachesDirectoryPath: '/mock/caches', + LibraryDirectoryPath: '/mock/library', })); jest.mock('react-native-share', () => ({ diff --git a/patches/react-native-nitro-sqlite/details.md b/patches/react-native-nitro-sqlite/details.md new file mode 100644 index 000000000000..95fa8458365e --- /dev/null +++ b/patches/react-native-nitro-sqlite/details.md @@ -0,0 +1,16 @@ +# `react-native-nitro-sqlite` patches + +### [react-native-nitro-sqlite+9.6.0+001+store-database-outside-documents.patch](react-native-nitro-sqlite+9.6.0+001+store-database-outside-documents.patch) + +- Reason: + + ``` + The library stores SQLite databases in the iOS Documents directory, which is exposed to users + via the Files app when file sharing is enabled. This patch stores databases in + Library/Application Support instead (persistent, backed up, never user-visible) and migrates + databases created by older app versions out of Documents on first launch. + ``` + +- Upstream PR/issue: https://github.com/margelo/react-native-nitro-sqlite/issues/289 +- E/App issue: https://github.com/Expensify/App/issues/96649 +- PR introducing patch: https://github.com/Expensify/App/pull/96531 diff --git a/patches/react-native-nitro-sqlite/react-native-nitro-sqlite+9.6.0+001+store-database-outside-documents.patch b/patches/react-native-nitro-sqlite/react-native-nitro-sqlite+9.6.0+001+store-database-outside-documents.patch new file mode 100644 index 000000000000..ec70f626a216 --- /dev/null +++ b/patches/react-native-nitro-sqlite/react-native-nitro-sqlite+9.6.0+001+store-database-outside-documents.patch @@ -0,0 +1,59 @@ +diff --git a/node_modules/react-native-nitro-sqlite/ios/OnLoad.mm b/node_modules/react-native-nitro-sqlite/ios/OnLoad.mm +index 6ce7258..38ea210 100644 +--- a/node_modules/react-native-nitro-sqlite/ios/OnLoad.mm ++++ b/node_modules/react-native-nitro-sqlite/ios/OnLoad.mm +@@ -10,6 +10,33 @@ @implementation OnLoad + using namespace margelo::nitro; + using namespace margelo::nitro::rnnitrosqlite; + ++// The Documents directory can be exposed to the user (Files app) when file sharing ++// is enabled, so databases are stored in Library/Application Support instead. ++// Databases created by older app versions are moved out of Documents on first launch. ++static void migrateDatabaseFiles(NSString *fromDirectory, NSString *toDirectory) { ++ NSFileManager *fileManager = [NSFileManager defaultManager]; ++ NSArray *files = [fileManager contentsOfDirectoryAtPath:fromDirectory error:nil]; ++ ++ for (NSString *file in files) { ++ // Covers the database itself plus its -wal/-shm journal files ++ if (![file hasPrefix:@"OnyxDB"]) { ++ continue; ++ } ++ ++ NSString *sourcePath = [fromDirectory stringByAppendingPathComponent:file]; ++ NSString *destinationPath = [toDirectory stringByAppendingPathComponent:file]; ++ ++ if ([fileManager fileExistsAtPath:destinationPath]) { ++ continue; ++ } ++ ++ NSError *error = nil; ++ if (![fileManager moveItemAtPath:sourcePath toPath:destinationPath error:&error]) { ++ NSLog(@"Failed to migrate database file %@: %@", file, error.localizedDescription); ++ } ++ } ++} ++ + + (void)load { + // Get appGroupID value from Info.plist using key "AppGroup" + NSString *appGroupID = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"RNNitroSQLite_AppGroup"]; +@@ -30,9 +57,18 @@ + (void)load { + + documentPath = [storeUrl path]; + } else { +- // Get iOS app's document directory (to safely store database .sqlite3 file) +- NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, true); ++ // Store databases in Library/Application Support, which is persistent, backed up, ++ // and never exposed to the user via the Files app (unlike the Documents directory) ++ NSArray *paths = NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, true); + documentPath = [paths objectAtIndex:0]; ++ ++ NSFileManager *fileManager = [NSFileManager defaultManager]; ++ if (![fileManager fileExistsAtPath:documentPath]) { ++ [fileManager createDirectoryAtPath:documentPath withIntermediateDirectories:YES attributes:nil error:nil]; ++ } ++ ++ NSArray *documentPaths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, true); ++ migrateDatabaseFiles([documentPaths objectAtIndex:0], documentPath); + } + + HybridNitroSQLite::docPath = [documentPath UTF8String]; diff --git a/src/libs/ExportOnyxState/index.native.ts b/src/libs/ExportOnyxState/index.native.ts index 74a6eef80571..967b6c05b956 100644 --- a/src/libs/ExportOnyxState/index.native.ts +++ b/src/libs/ExportOnyxState/index.native.ts @@ -40,7 +40,10 @@ const shareAsFile: ShareAsFile = (fileContent) => { try { // Define new filename and path for the app info file const infoFileName = CONST.DEFAULT_ONYX_DUMP_FILE_NAME; - const infoFilePath = `${RNFS.DocumentDirectoryPath}/${infoFileName}`; + // The dump only needs to live long enough to be shared, so it goes in Caches, which + // is never exposed to the user (unlike Documents, which the iOS Files app shows when + // file sharing is enabled) and which the OS can reclaim afterwards + const infoFilePath = `${RNFS.CachesDirectoryPath}/${infoFileName}`; const actualInfoFile = `file://${infoFilePath}`; RNFS.writeFile(infoFilePath, fileContent, 'utf8').then(() => { diff --git a/src/libs/actions/Attachment/index.native.ts b/src/libs/actions/Attachment/index.native.ts index dd6cb92c7559..8702893368c4 100644 --- a/src/libs/actions/Attachment/index.native.ts +++ b/src/libs/actions/Attachment/index.native.ts @@ -10,7 +10,9 @@ import Onyx from 'react-native-onyx'; import type {CacheAttachmentProps, GetCachedAttachmentProps, RemoveCachedAttachmentProps} from './types'; -const ATTACHMENT_DIR = `${RNFS.DocumentDirectoryPath}/attachments`; +// Cached attachments are re-downloadable, so they live in Caches, which the OS may purge +// and which is never exposed to the user via the iOS Files app (unlike Documents) +const ATTACHMENT_DIR = `${RNFS.CachesDirectoryPath}/attachments`; async function cacheAttachment({attachmentID, uri, mimeType}: CacheAttachmentProps) { const isLocalFile = uri.startsWith('file://'); diff --git a/src/libs/fileDownload/index.ios.ts b/src/libs/fileDownload/index.ios.ts index 45577c54ba31..0a0f38155461 100644 --- a/src/libs/fileDownload/index.ios.ts +++ b/src/libs/fileDownload/index.ios.ts @@ -30,26 +30,41 @@ const isUserCancelled = (err: unknown) => { }; /** - * Downloads the file to Documents section in iOS + * Downloads the file to the Documents directory, which the iOS Files app shows to the user + * as the app's folder because file sharing is enabled. Only files the user asked to download + * belong there; internal files must go to a directory the Files app does not expose. */ function downloadFile(fileUrl: string, fileName: string) { const dirs = RNFetchBlob.fs.dirs; - // The iOS files will download to documents directory - const path = dirs.DocumentDir; + return RNFetchBlob.config({ + fileCache: true, + path: `${dirs.DocumentDir}/${fileName}`, + }).fetch('GET', fileUrl); +} + +/** + * Downloads the file to the cache directory, for flows that only need a temporary local + * copy (e.g. saving to Photos or handing off to the share sheet). Unlike Documents, the + * cache directory is never shown to the user in the iOS Files app. + */ +function downloadFileToCache(fileUrl: string, fileName: string) { + const dirs = RNFetchBlob.fs.dirs; - // Fetching the attachment return RNFetchBlob.config({ fileCache: true, - path: `${path}/${fileName}`, - addAndroidDownloads: { - useDownloadManager: true, - notification: true, - path: `${path}/Expensify/${fileName}`, - }, + path: `${dirs.CacheDir}/${fileName}`, }).fetch('GET', fileUrl); } +/** + * Presents the iOS share sheet so the user can save the file to the Files app, + * then removes the local copy. + */ +function shareFileToFilesApp(localPath: string) { + return Share.open({url: localPath, failOnCancel: false, saveToFiles: true}).then(() => RNFS.unlink(localPath)); +} + const postDownloadFile = (translate: LocalizedTranslate, url: string, fileName?: string, formData?: FormData, onDownloadFailed?: () => void, appendTimestamp = true) => { const fetchOptions: RequestInit = { method: 'POST', @@ -70,12 +85,12 @@ const postDownloadFile = (translate: LocalizedTranslate, url: string, fileName?: .then((fileData) => { const resolvedFileName = fileName ?? 'Expensify'; const finalFileName = appendTimestamp ? appendTimeToFileName(resolvedFileName) : resolvedFileName; - const expensifyDir = `${RNFS.DocumentDirectoryPath}/Expensify`; + // The file only exists to be handed to the share sheet, so it is written to the + // cache directory, which the iOS Files app never shows to the user + const expensifyDir = `${RNFS.CachesDirectoryPath}/Expensify`; const localPath = `${expensifyDir}/${finalFileName}`; return RNFS.mkdir(expensifyDir).then(() => { - return RNFS.writeFile(localPath, fileData, 'utf8') - .then(() => Share.open({url: localPath, failOnCancel: false, saveToFiles: true})) - .then(() => RNFS.unlink(localPath)); + return RNFS.writeFile(localPath, fileData, 'utf8').then(() => shareFileToFilesApp(localPath)); }); }) .catch((error) => { @@ -102,24 +117,24 @@ function downloadImage(fileUrl: string) { */ function downloadVideo(fileUrl: string, fileName: string): Promise { return new Promise((resolve, reject) => { - let documentPathUri: string | null = null; + let tempPathUri: string | null = null; let cameraRollAsset: PhotoIdentifier; - // Because CameraRoll doesn't allow direct downloads of video with remote URIs, we first download as documents, then copy to photo lib and unlink the original file. - downloadFile(fileUrl, fileName) + // Because CameraRoll doesn't allow direct downloads of video with remote URIs, we first download to the cache, then copy to photo lib and unlink the temporary file. + downloadFileToCache(fileUrl, fileName) .then((attachment) => { - documentPathUri = attachment.data as string | null; - if (!documentPathUri) { + tempPathUri = attachment.data as string | null; + if (!tempPathUri) { throw new Error('Error downloading video'); } - return CameraRoll.saveAsset(documentPathUri); + return CameraRoll.saveAsset(tempPathUri); }) .then((attachment) => { cameraRollAsset = attachment; - if (!documentPathUri) { + if (!tempPathUri) { throw new Error('Error downloading video'); } - return RNFetchBlob.fs.unlink(documentPathUri); + return RNFetchBlob.fs.unlink(tempPathUri); }) .then(() => { resolve(cameraRollAsset); diff --git a/src/libs/localFileCreate/index.native.ts b/src/libs/localFileCreate/index.native.ts index b1a8c893e500..a568874b3c2f 100644 --- a/src/libs/localFileCreate/index.native.ts +++ b/src/libs/localFileCreate/index.native.ts @@ -14,7 +14,10 @@ const localFileCreate: LocalFileCreate = (fileName, textContent, appendTimestamp const {fileExtension} = splitExtensionFromFileName(fileName); const fileNameWithExtension = fileExtension ? fileName : `${fileName}.txt`; const newFileName = appendTimestamp ? appendTimeToFileName(fileNameWithExtension) : fileNameWithExtension; - const dir = RNFetchBlob.fs.dirs.DocumentDir; + // These files are temporary hand-offs to a share/copy flow that deletes them afterwards, + // so they belong in the cache directory, which is never exposed to the user (unlike + // Documents, which the iOS Files app shows when file sharing is enabled) + const dir = RNFetchBlob.fs.dirs.CacheDir; const path = `${dir}/${newFileName}`; return RNFetchBlob.fs.writeFile(path, textContent, 'utf8').then(() => RNFetchBlob.fs.stat(path).then(({size}) => ({path, newFileName, size}))); diff --git a/src/libs/migrateOnyx.ts b/src/libs/migrateOnyx.ts index 21c828349228..c19112f0c855 100644 --- a/src/libs/migrateOnyx.ts +++ b/src/libs/migrateOnyx.ts @@ -2,6 +2,7 @@ import CONST from '@src/CONST'; import Log from './Log'; import ConvertGpsPointsTo2DArray from './migrations/ConvertGpsPointsTo2DArray'; +import MoveFilesOutOfDocuments from './migrations/MoveFilesOutOfDocuments'; import {endSpan, getSpan, startSpan} from './telemetry/activeSpans'; export default function () { @@ -16,7 +17,7 @@ export default function () { }); // Add all migrations to an array so they are executed in order - const migrationPromises: Array<() => Promise> = [ConvertGpsPointsTo2DArray]; + const migrationPromises: Array<() => Promise> = [ConvertGpsPointsTo2DArray, MoveFilesOutOfDocuments]; // Reduce all promises down to a single promise. All promises run in a linear fashion, waiting for the // previous promise to finish before moving onto the next one. diff --git a/src/libs/migrations/MoveFilesOutOfDocuments/index.ios.ts b/src/libs/migrations/MoveFilesOutOfDocuments/index.ios.ts new file mode 100644 index 000000000000..597701f98168 --- /dev/null +++ b/src/libs/migrations/MoveFilesOutOfDocuments/index.ios.ts @@ -0,0 +1,61 @@ +import Log from '@libs/Log'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import RNFS from 'react-native-fs'; +import Onyx from 'react-native-onyx'; + +const OLD_ATTACHMENT_DIR = `${RNFS.DocumentDirectoryPath}/attachments`; + +/** + * The attachment cache now lives in Library/Caches. The old copies in Documents are + * deleted rather than moved because cached attachments re-download on demand, and the + * Onyx attachment collection is cleared since its sources point at the old directory. + */ +function migrateAttachmentCache(): Promise { + return RNFS.exists(OLD_ATTACHMENT_DIR).then((exists) => { + if (!exists) { + return; + } + return RNFS.unlink(OLD_ATTACHMENT_DIR) + .then(() => Onyx.setCollection(ONYXKEYS.COLLECTION.ATTACHMENT, {})) + .then(() => { + Log.info('[Migrate Onyx] MoveFilesOutOfDocuments removed the old attachment cache'); + }); + }); +} + +/** + * Onyx state dumps were previously written to Documents and never deleted after sharing, + * so a stale dump may still sit there. It is an internal debug file, so it is removed. + */ +function removeStaleOnyxDump(): Promise { + const dumpPath = `${RNFS.DocumentDirectoryPath}/${CONST.DEFAULT_ONYX_DUMP_FILE_NAME}`; + return RNFS.exists(dumpPath).then((exists) => { + if (!exists) { + return; + } + return RNFS.unlink(dumpPath).then(() => { + Log.info('[Migrate Onyx] MoveFilesOutOfDocuments removed a stale Onyx state dump'); + }); + }); +} + +/** + * Internal app files used to live in the Documents directory, which iOS shows to the + * user (and other apps) through the Files app because file sharing is enabled. This + * removes the ones older app versions left behind, so the directory only holds files + * the user expects to see there: their downloads and queued receipt uploads. + */ +export default function (): Promise { + return ( + Promise.resolve() + .then(() => Promise.all([migrateAttachmentCache(), removeStaleOnyxDump()])) + .then(() => undefined) + // A failed cleanup must never block app startup; new files already go to the new locations + .catch((error) => { + Log.warn('[Migrate Onyx] MoveFilesOutOfDocuments failed', {error: error instanceof Error ? error.message : String(error)}); + }) + ); +} diff --git a/src/libs/migrations/MoveFilesOutOfDocuments/index.ts b/src/libs/migrations/MoveFilesOutOfDocuments/index.ts new file mode 100644 index 000000000000..79cd559856e7 --- /dev/null +++ b/src/libs/migrations/MoveFilesOutOfDocuments/index.ts @@ -0,0 +1,5 @@ +// This migration only applies to iOS, where internal files previously lived in the +// user-visible Documents directory. On other platforms it is a no-op. +export default function (): Promise { + return Promise.resolve(); +} diff --git a/tests/actions/AttachmentTest.ts b/tests/actions/AttachmentTest.ts index 39445c7c51f3..2c0dc33b72d0 100644 --- a/tests/actions/AttachmentTest.ts +++ b/tests/actions/AttachmentTest.ts @@ -16,6 +16,7 @@ import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; jest.mock('react-native-fs', () => ({ DocumentDirectoryPath: '/mock/documents', + CachesDirectoryPath: '/mock/caches', copyFile: jest.fn(() => Promise.resolve()), exists: jest.fn(() => Promise.resolve(true)), unlink: jest.fn(() => Promise.resolve()), @@ -104,7 +105,7 @@ describe('AttachmentStorage', () => { expect(attachmentID).toBeDefined(); expect(attachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, }); }); it('should cache markdown attachment', async () => { @@ -144,7 +145,7 @@ describe('AttachmentStorage', () => { expect(attachmentID).toBeDefined(); expect(attachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: sourceURL, }); }); @@ -188,7 +189,7 @@ describe('AttachmentStorage', () => { expect(attachmentID).toBeDefined(); expect(attachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: sourceURL, }); @@ -207,7 +208,7 @@ describe('AttachmentStorage', () => { // Then the attachment should be updated with new attachment link expect(newAttachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: newSourceURL, }); }); @@ -260,7 +261,7 @@ describe('AttachmentStorage', () => { expect(attachmentID).toBeDefined(); expect(newAttachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: fileData.uri, }); @@ -323,7 +324,7 @@ describe('AttachmentStorage', () => { expect(attachmentID).toBeDefined(); expect(newAttachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: sourceURL, }); @@ -409,14 +410,14 @@ describe('AttachmentStorage', () => { expect(remoteSourceIndex).toBeDefined(); expect(attachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, remoteSource: markdownAttachments.at(remoteSourceIndex), }); continue; } expect(attachment).toEqual({ attachmentID, - source: `/mock/documents/attachments/${attachmentID}.jpg`, + source: `/mock/caches/attachments/${attachmentID}.jpg`, }); } diff --git a/tests/unit/MoveFilesOutOfDocumentsTest.ts b/tests/unit/MoveFilesOutOfDocumentsTest.ts new file mode 100644 index 000000000000..1ac4f6e91dec --- /dev/null +++ b/tests/unit/MoveFilesOutOfDocumentsTest.ts @@ -0,0 +1,79 @@ +import MoveFilesOutOfDocuments from '@libs/migrations/MoveFilesOutOfDocuments/index.ios'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import Onyx from 'react-native-onyx'; + +import getOnyxValue from '../utils/getOnyxValue'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +// Importing the real Log module pulls in the network stack, whose persisted-request +// bookkeeping reacts to the Onyx writes made in these tests. +jest.mock('@libs/Log', () => ({ + __esModule: true, + default: {info: jest.fn(), warn: jest.fn(), alert: jest.fn()}, +})); + +jest.mock('react-native-fs', () => ({ + DocumentDirectoryPath: '/mock/documents', + exists: jest.fn(() => Promise.resolve(false)), + unlink: jest.fn(() => Promise.resolve()), +})); + +const mockRNFS: { + exists: jest.Mock; + unlink: jest.Mock; +} = jest.requireMock('react-native-fs'); + +const OLD_ATTACHMENT_DIR = '/mock/documents/attachments'; +const STALE_ONYX_DUMP = `/mock/documents/${CONST.DEFAULT_ONYX_DUMP_FILE_NAME}`; + +describe('MoveFilesOutOfDocuments migration (iOS)', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + jest.clearAllMocks(); + mockRNFS.exists.mockImplementation(() => Promise.resolve(false)); + mockRNFS.unlink.mockImplementation(() => Promise.resolve()); + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + it('does nothing when no internal files are left in Documents', async () => { + await MoveFilesOutOfDocuments(); + + expect(mockRNFS.unlink).not.toHaveBeenCalled(); + }); + + it('removes the old attachment cache and clears the attachment collection', async () => { + mockRNFS.exists.mockImplementation((path: string) => Promise.resolve(path === OLD_ATTACHMENT_DIR)); + await Onyx.set(`${ONYXKEYS.COLLECTION.ATTACHMENT}source1`, {source: `${OLD_ATTACHMENT_DIR}/file.pdf`}); + await waitForBatchedUpdates(); + + await MoveFilesOutOfDocuments(); + await waitForBatchedUpdates(); + + expect(mockRNFS.unlink).toHaveBeenCalledWith(OLD_ATTACHMENT_DIR); + const attachment = await getOnyxValue(`${ONYXKEYS.COLLECTION.ATTACHMENT}source1`); + expect(attachment).toBeUndefined(); + }); + + it('removes a stale Onyx state dump left by older app versions', async () => { + mockRNFS.exists.mockImplementation((path: string) => Promise.resolve(path === STALE_ONYX_DUMP)); + + await MoveFilesOutOfDocuments(); + + expect(mockRNFS.unlink).toHaveBeenCalledWith(STALE_ONYX_DUMP); + expect(mockRNFS.unlink).not.toHaveBeenCalledWith(OLD_ATTACHMENT_DIR); + }); + + it('does not block startup when the cleanup fails', async () => { + mockRNFS.exists.mockImplementation(() => Promise.resolve(true)); + mockRNFS.unlink.mockImplementation(() => Promise.reject(new Error('unlink failed'))); + + await expect(MoveFilesOutOfDocuments()).resolves.toBeUndefined(); + }); +});