diff --git a/.gitignore b/.gitignore index 47ad5f99db..c1d60d6e9c 100644 --- a/.gitignore +++ b/.gitignore @@ -1041,6 +1041,7 @@ packages/app-mobile/utils/database-driver-react-native.web.js packages/app-mobile/utils/debounce.js packages/app-mobile/utils/focusView.js packages/app-mobile/utils/fs-driver/constants.js +packages/app-mobile/utils/fs-driver/fs-driver-rn.test.js packages/app-mobile/utils/fs-driver/fs-driver-rn.js packages/app-mobile/utils/fs-driver/fs-driver-rn.web.js packages/app-mobile/utils/fs-driver/fs-driver-rn.web.worker.js diff --git a/.ignore.eslint b/.ignore.eslint index f7e1c293ec..4c15fa89d1 100644 --- a/.ignore.eslint +++ b/.ignore.eslint @@ -1067,6 +1067,7 @@ packages/app-mobile/utils/database-driver-react-native.web.js packages/app-mobile/utils/debounce.js packages/app-mobile/utils/focusView.js packages/app-mobile/utils/fs-driver/constants.js +packages/app-mobile/utils/fs-driver/fs-driver-rn.test.js packages/app-mobile/utils/fs-driver/fs-driver-rn.js packages/app-mobile/utils/fs-driver/fs-driver-rn.web.js packages/app-mobile/utils/fs-driver/fs-driver-rn.web.worker.js diff --git a/packages/app-mobile/utils/fs-driver/fs-driver-rn.test.ts b/packages/app-mobile/utils/fs-driver/fs-driver-rn.test.ts new file mode 100644 index 0000000000..24cc0e0e9e --- /dev/null +++ b/packages/app-mobile/utils/fs-driver/fs-driver-rn.test.ts @@ -0,0 +1,234 @@ +const mockSaf = { + copyFile: jest.fn(), + copyFileToDirectory: jest.fn(), + listFiles: jest.fn(), + stat: jest.fn(), + writeFile: jest.fn(), + writeFileInDirectory: jest.fn(), +}; + +jest.mock('rn-fetch-blob', () => ({ + __esModule: true, + default: { fs: {} }, +})); + +jest.mock('@joplin/react-native-saf-x', () => ({ + __esModule: true, + default: mockSaf, + openDocumentTree: jest.fn(), +})); + +import FsDriverRN from './fs-driver-rn'; + +describe('FsDriverRN SAF destination lookup', () => { + const parentPath = 'content://provider/tree/root'; + const destinationPath = `${parentPath}/existing.md`; + const destinationUri = 'content://provider/document/existing'; + + beforeEach(() => { + jest.clearAllMocks(); + mockSaf.stat.mockResolvedValue({ uri: parentPath, documentUri: parentPath }); + mockSaf.listFiles.mockResolvedValue([{ + name: 'existing.md', + uri: destinationPath, + documentUri: destinationUri, + }]); + mockSaf.writeFile.mockResolvedValue(undefined); + mockSaf.copyFile.mockResolvedValue(undefined); + mockSaf.writeFileInDirectory.mockResolvedValue({ uri: `${parentPath}/new.md` }); + mockSaf.copyFileToDirectory.mockResolvedValue({ uri: `${parentPath}/new.md` }); + }); + + it('should overwrite an existing file discovered while warming an empty cache', async () => { + const driver = new FsDriverRN(); + + await driver.writeFile(destinationPath, 'updated', 'utf8'); + await driver.writeFile(destinationPath, 'updated again', 'utf8'); + + expect(mockSaf.listFiles).toHaveBeenCalledTimes(1); + expect(mockSaf.writeFile).toHaveBeenNthCalledWith(1, destinationUri, 'updated', { encoding: 'utf8' }); + expect(mockSaf.writeFile).toHaveBeenNthCalledWith(2, destinationUri, 'updated again', { encoding: 'utf8' }); + expect(mockSaf.writeFileInDirectory).not.toHaveBeenCalled(); + }); + + it('should overwrite an existing copy destination discovered while warming an empty cache', async () => { + const driver = new FsDriverRN(); + const sourcePath = '/local/source.md'; + + await driver.copy(sourcePath, destinationPath); + + expect(mockSaf.listFiles).toHaveBeenCalledWith(parentPath); + expect(mockSaf.copyFile).toHaveBeenCalledWith(sourcePath, destinationUri, { replaceIfDestinationExists: true }); + expect(mockSaf.copyFileToDirectory).not.toHaveBeenCalled(); + }); + + it('should overwrite a destination discovered by stat after an older listing', async () => { + const driver = new FsDriverRN(); + mockSaf.listFiles.mockResolvedValueOnce([]); + await driver.readDirStats(parentPath); + mockSaf.stat.mockResolvedValueOnce({ + name: 'existing.md', + uri: destinationPath, + documentUri: destinationUri, + type: 'file', + size: 0, + lastModified: 0, + }); + + await driver.stat(destinationPath); + await driver.writeFile(destinationPath, 'updated', 'utf8'); + + expect(mockSaf.listFiles).toHaveBeenCalledTimes(1); + expect(mockSaf.writeFile).toHaveBeenCalledWith(destinationUri, 'updated', { encoding: 'utf8' }); + expect(mockSaf.writeFileInDirectory).not.toHaveBeenCalled(); + }); + + it('should obtain fresh metadata through the cached document URI', async () => { + const driver = new FsDriverRN(); + await driver.writeFile(destinationPath, 'first', 'utf8'); + mockSaf.stat.mockResolvedValueOnce({ + name: 'existing.md', + uri: destinationUri, + documentUri: destinationUri, + type: 'file', + size: 5, + lastModified: 123, + }); + + const stat = await driver.stat(destinationPath); + + expect(mockSaf.stat).toHaveBeenLastCalledWith(destinationUri); + expect(stat.size).toBe(5); + expect(stat.mtime.getTime()).toBe(123); + }); + + it('should retry a stat through the logical path after a cached URI becomes stale', async () => { + const driver = new FsDriverRN(); + const replacementUri = 'content://provider/document/replacement'; + await driver.writeFile(destinationPath, 'first', 'utf8'); + mockSaf.stat + .mockRejectedValueOnce(Object.assign(new Error('Document no longer exists'), { code: 'ENOENT' })) + .mockResolvedValueOnce({ + name: 'existing.md', + uri: destinationPath, + documentUri: replacementUri, + type: 'file', + size: 7, + lastModified: 456, + }); + + await driver.stat(destinationPath); + await driver.writeFile(destinationPath, 'updated', 'utf8'); + + expect(mockSaf.stat).toHaveBeenNthCalledWith(2, destinationUri); + expect(mockSaf.stat).toHaveBeenNthCalledWith(3, destinationPath); + expect(mockSaf.writeFile).toHaveBeenLastCalledWith(replacementUri, 'updated', { encoding: 'utf8' }); + }); + + it('should explicitly enable replacement for directory-based writes and copies', async () => { + const driver = new FsDriverRN(); + mockSaf.listFiles.mockResolvedValue([]); + + await driver.writeFile(`${parentPath}/new.md`, 'new', 'utf8'); + await driver.copy('/local/source.md', `${parentPath}/copied.md`); + + expect(mockSaf.writeFileInDirectory).toHaveBeenCalledWith( + parentPath, 'new.md', 'new', { encoding: 'utf8', replaceIfDestinationExists: true }, + ); + expect(mockSaf.copyFileToDirectory).toHaveBeenCalledWith( + '/local/source.md', parentPath, 'copied.md', { replaceIfDestinationExists: true }, + ); + }); + + it('should refresh a cached destination after a non-ENOENT write failure without replaying it', async () => { + const driver = new FsDriverRN(); + await driver.writeFile(destinationPath, 'first', 'utf8'); + mockSaf.writeFile.mockRejectedValueOnce(Object.assign(new Error('Provider failure'), { code: 'EIO' })); + + await expect(driver.writeFile(destinationPath, 'failed', 'utf8')).rejects.toThrow('Provider failure'); + expect(mockSaf.writeFile).toHaveBeenCalledTimes(2); + + await driver.writeFile(destinationPath, 'retried', 'utf8'); + expect(mockSaf.listFiles).toHaveBeenCalledTimes(2); + expect(mockSaf.writeFile).toHaveBeenLastCalledWith(destinationUri, 'retried', { encoding: 'utf8' }); + }); + + it('should resolve a replacement document after a cached URI becomes stale', async () => { + const driver = new FsDriverRN(); + const replacementUri = 'content://provider/document/replacement'; + await driver.writeFile(destinationPath, 'first', 'utf8'); + mockSaf.writeFile.mockRejectedValueOnce(Object.assign(new Error('Document no longer exists'), { code: 'ENOENT' })); + + await expect(driver.writeFile(destinationPath, 'failed', 'utf8')).rejects.toThrow('Document no longer exists'); + expect(mockSaf.writeFile).toHaveBeenCalledTimes(2); + expect(mockSaf.writeFileInDirectory).not.toHaveBeenCalled(); + + mockSaf.listFiles.mockResolvedValueOnce([{ + name: 'existing.md', + uri: destinationPath, + documentUri: replacementUri, + }]); + await driver.writeFile(destinationPath, 'retried', 'utf8'); + + expect(mockSaf.listFiles).toHaveBeenCalledTimes(2); + expect(mockSaf.writeFile).toHaveBeenLastCalledWith(replacementUri, 'retried', { encoding: 'utf8' }); + expect(mockSaf.writeFileInDirectory).not.toHaveBeenCalled(); + }); + + it('should not replay or fall back after an ambiguous cached copy failure', async () => { + const driver = new FsDriverRN(); + const sourcePath = '/local/source.md'; + await driver.copy(sourcePath, destinationPath); + mockSaf.copyFile.mockRejectedValueOnce(Object.assign(new Error('Copy close failed'), { code: 'EIO' })); + + await expect(driver.copy(sourcePath, destinationPath)).rejects.toThrow('Copy close failed'); + + expect(mockSaf.copyFile).toHaveBeenCalledTimes(2); + expect(mockSaf.copyFileToDirectory).not.toHaveBeenCalled(); + + await driver.copy(sourcePath, destinationPath); + expect(mockSaf.listFiles).toHaveBeenCalledTimes(2); + expect(mockSaf.copyFile).toHaveBeenLastCalledWith(sourcePath, destinationUri, { replaceIfDestinationExists: true }); + expect(mockSaf.copyFileToDirectory).not.toHaveBeenCalled(); + }); + + it('should invalidate a failed directory mutation without retrying it through another route', async () => { + const driver = new FsDriverRN(); + const newPath = `${parentPath}/new.md`; + mockSaf.listFiles.mockResolvedValue([]); + mockSaf.writeFileInDirectory.mockRejectedValueOnce(Object.assign(new Error('Close failed'), { code: 'EIO' })); + + await expect(driver.writeFile(newPath, 'new', 'utf8')).rejects.toThrow('Close failed'); + + expect(mockSaf.writeFileInDirectory).toHaveBeenCalledTimes(1); + expect(mockSaf.writeFile).not.toHaveBeenCalled(); + + mockSaf.listFiles.mockResolvedValueOnce([{ + name: 'new.md', + uri: newPath, + documentUri: 'content://provider/document/new', + }]); + await driver.writeFile(newPath, 'retried', 'utf8'); + + expect(mockSaf.listFiles).toHaveBeenCalledTimes(2); + expect(mockSaf.writeFile).toHaveBeenCalledWith('content://provider/document/new', 'retried', { encoding: 'utf8' }); + expect(mockSaf.writeFileInDirectory).toHaveBeenCalledTimes(1); + }); + + it('should reject a directory entry whose URI is not relative to the listed directory', async () => { + const driver = new FsDriverRN(); + mockSaf.listFiles.mockResolvedValueOnce([{ + name: 'existing.md', + uri: destinationUri, + documentUri: destinationUri, + }]); + + await expect(driver.readDirStats(parentPath)).rejects.toThrow('Document URI does not start with directory path'); + await driver.writeFile(destinationPath, 'updated', 'utf8'); + + // The invalid listing must not be treated as complete or populate the + // document cache. A subsequent operation obtains a fresh listing. + expect(mockSaf.listFiles).toHaveBeenCalledTimes(2); + expect(mockSaf.writeFile).toHaveBeenCalledWith(destinationUri, 'updated', { encoding: 'utf8' }); + }); +}); diff --git a/packages/app-mobile/utils/fs-driver/fs-driver-rn.ts b/packages/app-mobile/utils/fs-driver/fs-driver-rn.ts index 7dbb96d3a4..eeb979e556 100644 --- a/packages/app-mobile/utils/fs-driver/fs-driver-rn.ts +++ b/packages/app-mobile/utils/fs-driver/fs-driver-rn.ts @@ -53,6 +53,201 @@ const normalizeEncoding = (encoding: string): SupportedEncoding => { }; export default class FsDriverRN extends FsDriverBase { + private safDocumentUris_ = new Map(); + private safDirectoryUris_ = new Map(); + private listedSafDirectories_ = new Set(); + + private cachedSafUri_(path: string) { + const normalizedPath = this.normalizeSafPath_(path); + return this.safDocumentUris_.get(normalizedPath) ?? normalizedPath; + } + + private normalizeSafPath_(path: string) { + return path.replace(/\/$/, ''); + } + + private safParentPath_(path: string) { + const normalizedPath = this.normalizeSafPath_(path); + return normalizedPath.substring(0, normalizedPath.lastIndexOf('/')); + } + + private safRelativePath_(directoryPath: string, document: DocumentFileDetail) { + directoryPath = this.normalizeSafPath_(directoryPath); + const pathPrefix = `${directoryPath}/`; + if (!document.uri.startsWith(pathPrefix)) { + throw new Error(`Cannot derive SAF relative path: Document URI does not start with directory path: ${document.uri}`); + } + return document.uri.substring(pathPrefix.length); + } + + private invalidateSafDirectory_(path: string) { + const normalizedPath = this.normalizeSafPath_(path); + this.safDirectoryUris_.delete(normalizedPath); + this.listedSafDirectories_.delete(normalizedPath); + } + + private invalidateSafPath_(path: string) { + const normalizedPath = this.normalizeSafPath_(path); + const pathPrefix = `${normalizedPath}/`; + for (const cachedPath of this.safDocumentUris_.keys()) { + if (cachedPath === normalizedPath || cachedPath.startsWith(pathPrefix)) this.safDocumentUris_.delete(cachedPath); + } + for (const cachedPath of this.safDirectoryUris_.keys()) { + if (cachedPath === normalizedPath || cachedPath.startsWith(pathPrefix)) this.safDirectoryUris_.delete(cachedPath); + } + for (const cachedPath of this.listedSafDirectories_) { + if (cachedPath === normalizedPath || cachedPath.startsWith(pathPrefix)) this.listedSafDirectories_.delete(cachedPath); + } + this.invalidateSafDirectory_(this.safParentPath_(normalizedPath)); + } + + private async withCachedSafUri_(path: string, callback: (resolvedPath: string)=> Promise, retryIfStale = false): Promise { + path = this.normalizeSafPath_(path); + const cachedUri = this.cachedSafUri_(path); + try { + return await callback(cachedUri); + } catch (error) { + // Only non-mutating operations may retry a stale URI immediately. A + // mutating native operation can report ENOENT during its final metadata + // check, after it has already changed the destination. + if (cachedUri === path) throw error; + this.invalidateSafPath_(path); + // Providers do not consistently report stale documents as ENOENT. Clear + // the cached URI after any failure so a later operation resolves fresh + // state, but only replay this operation when staleness is confirmed and + // the caller has declared an immediate retry safe. + if (error?.code !== 'ENOENT' || !retryIfStale) throw error; + return callback(path); + } + } + + private async safDirectoryUri_(path: string) { + const normalizedPath = path.replace(/\/$/, ''); + const cachedUri = this.safDirectoryUris_.get(normalizedPath); + if (cachedUri) return cachedUri; + + const directory = await RNSAF.stat(normalizedPath); + const directoryUri = directory.documentUri ?? directory.uri; + this.safDirectoryUris_.set(normalizedPath, directoryUri); + return directoryUri; + } + + private cacheSafDirectoryListing_(path: string, documents: DocumentFileDetail[], relativePaths: string[] = null) { + const normalizedPath = path.replace(/\/$/, ''); + const pathPrefix = `${normalizedPath}/`; + for (const cachedPath of this.safDocumentUris_.keys()) { + if (cachedPath.startsWith(pathPrefix) && !cachedPath.substring(pathPrefix.length).includes('/')) { + this.safDocumentUris_.delete(cachedPath); + } + } + + for (let i = 0; i < documents.length; i++) { + const document = documents[i]; + const relativePath = relativePaths?.[i] ?? this.safRelativePath_(normalizedPath, document); + this.safDocumentUris_.set(`${pathPrefix}${relativePath}`, document.documentUri ?? document.uri); + } + this.listedSafDirectories_.add(normalizedPath); + } + + private async ensureSafDirectoryListed_(path: string) { + const normalizedPath = this.normalizeSafPath_(path); + if (this.listedSafDirectories_.has(normalizedPath)) return; + + try { + await this.safDirectoryUri_(normalizedPath); + const documents = await RNSAF.listFiles(normalizedPath); + this.cacheSafDirectoryListing_(normalizedPath, documents); + } catch (error) { + this.invalidateSafDirectory_(normalizedPath); + throw error; + } + } + + private async writeSafFile_(path: string, content: string, encoding: SupportedEncoding) { + path = this.normalizeSafPath_(path); + if (this.safDocumentUris_.has(path)) { + return this.withCachedSafUri_(path, resolvedPath => RNSAF.writeFile(resolvedPath, content, { encoding })); + } + + const lastSlashIndex = path.lastIndexOf('/'); + const parentPath = path.substring(0, lastSlashIndex); + try { + await this.ensureSafDirectoryListed_(parentPath); + } catch (error) { + if (error?.code !== 'ENOENT') throw error; + return RNSAF.writeFile(path, content, { encoding }); + } + if (this.safDocumentUris_.has(path)) { + return this.withCachedSafUri_(path, resolvedPath => RNSAF.writeFile(resolvedPath, content, { encoding })); + } + + const parentUri = this.safDirectoryUris_.get(parentPath); + if (parentUri) { + try { + const document = await RNSAF.writeFileInDirectory(parentUri, path.substring(lastSlashIndex + 1), content, { encoding, replaceIfDestinationExists: true }); + this.safDocumentUris_.set(path, document.documentUri ?? document.uri); + return; + } catch (error) { + // The operation may have created and written the destination before + // failing during close or its final metadata check. Never replay it here. + this.invalidateSafDirectory_(parentPath); + throw error; + } + } + + return RNSAF.writeFile(path, content, { encoding }); + } + + private async copyToSaf_(source: string, dest: string): Promise { + dest = this.normalizeSafPath_(dest); + if (this.safDocumentUris_.has(dest)) { + await this.withCachedSafUri_(dest, resolvedDest => RNSAF.copyFile(source, resolvedDest, { replaceIfDestinationExists: true })); + return; + } + + const lastSlashIndex = dest.lastIndexOf('/'); + const parentPath = dest.substring(0, lastSlashIndex); + try { + await this.ensureSafDirectoryListed_(parentPath); + } catch (error) { + if (error?.code !== 'ENOENT') throw error; + await RNSAF.copyFile(source, dest, { replaceIfDestinationExists: true }); + return; + } + if (this.safDocumentUris_.has(dest)) { + await this.withCachedSafUri_(dest, resolvedDest => RNSAF.copyFile(source, resolvedDest, { replaceIfDestinationExists: true })); + return; + } + + const parentUri = this.safDirectoryUris_.get(parentPath); + if (parentUri) { + try { + const document = await RNSAF.copyFileToDirectory(source, parentUri, dest.substring(lastSlashIndex + 1), { replaceIfDestinationExists: true }); + this.safDocumentUris_.set(dest, document.documentUri ?? document.uri); + return; + } catch (error) { + // The operation may have created and copied the destination before + // failing during close or its final metadata check. Never replay it here. + this.invalidateSafDirectory_(parentPath); + throw error; + } + } + + await RNSAF.copyFile(source, dest, { replaceIfDestinationExists: true }); + } + + private async appendSafFile_(path: string, content: string, encoding: SupportedEncoding) { + path = this.normalizeSafPath_(path); + const hasCachedUri = this.safDocumentUris_.has(path); + try { + return await this.withCachedSafUri_(path, resolvedPath => RNSAF.writeFile(resolvedPath, content, { encoding, append: true })); + } finally { + // A cache-cold append resolves by path and may create the destination. + // Force the parent listing to be refreshed before a later create/update. + if (!hasCachedUri) this.invalidateSafPath_(path); + } + } + public appendFileSync() { throw new Error('Not implemented: appendFileSync'); } @@ -64,7 +259,7 @@ export default class FsDriverRN extends FsDriverBase { const encoding = normalizeEncoding(rawEncoding); if (isScopedUri(path)) { - return RNSAF.writeFile(path, content, { encoding, append: true }); + return this.appendSafFile_(path, content, encoding); } return RNFS.appendFile(path, content, encoding); } @@ -74,7 +269,7 @@ export default class FsDriverRN extends FsDriverBase { const encoding = normalizeEncoding(rawEncoding); if (isScopedUri(path)) { - return RNSAF.writeFile(path, content, { encoding: encoding }); + return this.writeSafFile_(path, content, encoding); } // We need to use rn-fetch-blob here due to this bug: @@ -114,20 +309,25 @@ export default class FsDriverRN extends FsDriverBase { if (!options) options = { recursive: false }; const isScoped = isScopedUri(path); + if (isScoped) path = this.normalizeSafPath_(path); let stats: RnfsStatLike[] = []; try { if (isScoped) { + await this.safDirectoryUri_(path); stats = await RNSAF.listFiles(path); } else { stats = await RNFS.readDir(path); } } catch (error) { + if (isScoped) this.invalidateSafDirectory_(path); throw new Error(`Could not read directory: ${path}: ${error.message}`); } const toRelativePath = (stat: RnfsStatLike) => { - let relativePath = isScoped ? (stat as DocumentFileDetail).uri : (stat as StatResultT | ReadDirResItemT).path; + if (isScoped) return this.safRelativePath_(path, stat as DocumentFileDetail); + + let relativePath = (stat as StatResultT | ReadDirResItemT).path; // Workaround: Paths returned by RNFS.readDir can include a leading /private/, when this isn't included // in the original path variable: @@ -142,13 +342,15 @@ export default class FsDriverRN extends FsDriverBase { relativePath = relativePath.substring(path.length + 1); return relativePath; }; + const relativePaths = stats.map(toRelativePath); + if (isScoped) this.cacheSafDirectoryListing_(path, stats as DocumentFileDetail[], relativePaths); // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Output combines DocumentFileDetail (SAF) or normalized Stat (RNFS) entries let output: any[] = []; for (let i = 0; i < stats.length; i++) { const stat = stats[i]; - const relativePath = toRelativePath(stat); + const relativePath = relativePaths[i]; const standardStat = this.rnfsStatToStd_(stat, relativePath); output.push(standardStat); @@ -177,14 +379,26 @@ export default class FsDriverRN extends FsDriverBase { public async move(source: string, dest: string) { if (isScopedUri(source) || isScopedUri(dest)) { - await RNSAF.moveFile(source, dest, { replaceIfDestinationExists: true }); + try { + await RNSAF.moveFile(source, dest, { replaceIfDestinationExists: true }); + } finally { + this.invalidateSafPath_(source); + this.invalidateSafPath_(dest); + } + return; } return RNFS.moveFile(source, dest); } public async rename(source: string, dest: string) { if (isScopedUri(source) || isScopedUri(dest)) { - await RNSAF.rename(source, dest); + try { + await RNSAF.rename(source, dest); + } finally { + this.invalidateSafPath_(source); + this.invalidateSafPath_(dest); + } + return; } return RNFS.moveFile(source, dest); } @@ -198,7 +412,11 @@ export default class FsDriverRN extends FsDriverBase { public async mkdir(path: string) { if (isScopedUri(path)) { - await RNSAF.mkdir(path); + try { + await RNSAF.mkdir(path); + } finally { + this.invalidateSafPath_(path); + } return; } @@ -207,10 +425,25 @@ export default class FsDriverRN extends FsDriverBase { } public async stat(path: string) { + const isScoped = isScopedUri(path); try { let r; - if (isScopedUri(path)) { - r = await RNSAF.stat(path); + if (isScoped) { + const normalizedPath = this.normalizeSafPath_(path); + // A direct document URI avoids resolving the filename by walking the + // parent directory. stat() still queries the provider for fresh metadata, + // and a confirmed stale URI is safely retried through the logical path. + r = await this.withCachedSafUri_(normalizedPath, resolvedPath => RNSAF.stat(resolvedPath), true); + const documentUri = r.documentUri ?? r.uri; + if (r.type === 'directory') { + this.safDirectoryUris_.set(normalizedPath, documentUri); + } else { + // stat() is used immediately before upload conflict resolution. + // Retain the document found by that live lookup so an accepted + // upload overwrites it rather than creating another document from + // an older parent listing. + this.safDocumentUris_.set(normalizedPath, documentUri); + } } else { r = await RNFS.stat(path); } @@ -220,6 +453,7 @@ export default class FsDriverRN extends FsDriverBase { // Probably { [Error: File does not exist] framesToPop: 1, code: 'EUNSPECIFIED' } // or { [Error: The file {file} couldn’t be opened because there is no such file.], code: 'ENSCOCOAERRORDOMAIN260' } // which unfortunately does not have a proper error code. Can be ignored. + if (isScoped) this.invalidateSafPath_(path); return null; } else { throw error; @@ -260,19 +494,28 @@ export default class FsDriverRN extends FsDriverBase { const encoding = normalizeEncoding(rawEncoding); if (isScopedUri(path)) { - return RNSAF.readFile(path, { encoding: encoding }); + return this.withCachedSafUri_(path, resolvedPath => RNSAF.readFile(resolvedPath, { encoding: encoding }), true); } return RNFS.readFile(path, encoding); } // Always overwrite destination public async copy(source: string, dest: string) { + if (isScopedUri(source) || isScopedUri(dest)) { + if (isScopedUri(source)) { + try { + await this.withCachedSafUri_(source, resolvedSource => RNSAF.copyFile(resolvedSource, dest, { replaceIfDestinationExists: true })); + } finally { + if (isScopedUri(dest)) this.invalidateSafPath_(dest); + } + } else { + await this.copyToSaf_(source, dest); + } + return; + } + let retry = false; try { - if (isScopedUri(source) || isScopedUri(dest)) { - await RNSAF.copyFile(source, dest, { replaceIfDestinationExists: true }); - return; - } await RNFS.copyFile(source, dest); } catch (error) { // On iOS it will throw an error if the file already exist @@ -281,18 +524,16 @@ export default class FsDriverRN extends FsDriverBase { } if (retry) { - if (isScopedUri(source) || isScopedUri(dest)) { - await RNSAF.copyFile(source, dest, { replaceIfDestinationExists: true }); - } else { - await RNFS.copyFile(source, dest); - } + await RNFS.copyFile(source, dest); } } public async unlink(path: string) { try { if (isScopedUri(path)) { - await RNSAF.unlink(path); + // Deletion is idempotent and unlink does not perform a post-delete stat, + // so retrying a confirmed stale document URI is safe here. + await this.withCachedSafUri_(path, resolvedPath => RNSAF.unlink(resolvedPath), true); return; } await RNFS.unlink(path); @@ -303,6 +544,8 @@ export default class FsDriverRN extends FsDriverBase { } else { throw error; } + } finally { + if (isScopedUri(path)) this.invalidateSafPath_(path); } } diff --git a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/SafXModule.java b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/SafXModule.java index 70e664ae03..51ff26642f 100644 --- a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/SafXModule.java +++ b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/SafXModule.java @@ -82,6 +82,32 @@ public class SafXModule extends ReactContextBaseJavaModule { ); } + @ReactMethod + public void writeFileInDirectory( + String directoryUri, + String fileName, + String data, + String encoding, + String mimeType, + boolean replaceIfDestinationExists, + final Promise promise) { + this.efficientDocumentHelper.writeFileInDirectory( + directoryUri, fileName, data, encoding, mimeType, replaceIfDestinationExists, promise + ); + } + + @ReactMethod + public void copyFileToDirectory( + String sourceUri, + String directoryUri, + String fileName, + boolean replaceIfDestinationExists, + final Promise promise) { + this.efficientDocumentHelper.copyFileToDirectory( + sourceUri, directoryUri, fileName, replaceIfDestinationExists, promise + ); + } + @ReactMethod public void transferFile( String srcUri, String destUri, boolean replaceIfDestExists, boolean copy, Promise promise) { diff --git a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/DocumentStat.java b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/DocumentStat.java index 59e0b5fa00..1c2112761b 100644 --- a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/DocumentStat.java +++ b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/DocumentStat.java @@ -93,6 +93,7 @@ public class DocumentStat { public WritableMap getWritableMap() { WritableMap fileMap = Arguments.createMap(); fileMap.putString("uri", UriHelper.denormalize(uri)); + fileMap.putString("documentUri", internalUri.toString()); fileMap.putString("name", displayName); if (isDirectory) { fileMap.putString("type", "directory"); diff --git a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/EfficientDocumentHelper.java b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/EfficientDocumentHelper.java index 7653b7dc16..601712cdb5 100644 --- a/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/EfficientDocumentHelper.java +++ b/packages/react-native-saf-x/android/src/main/java/com/reactnativesafx/utils/EfficientDocumentHelper.java @@ -386,7 +386,9 @@ public class EfficientDocumentHelper { return newUri; } - private Uri createFile(final String unknownStr, final String mimeType) throws IOException { + private Uri createFile( + final String unknownStr, final String mimeType, final boolean replaceIfDestinationExists + ) throws IOException { Uri uri = null; try { @@ -395,6 +397,7 @@ public class EfficientDocumentHelper { } if (uri != null) { + if (replaceIfDestinationExists) return uri; throw new IOExceptionFast("a file or directory already exist at: " + uri); } @@ -405,6 +408,7 @@ public class EfficientDocumentHelper { file.getParentFile().mkdirs(); boolean created = file.createNewFile(); if (!created) { + if (replaceIfDestinationExists && file.exists() && file.isFile()) return uri; throw new IOExceptionFast("could not create file at: " + unknownStr); } return uri; @@ -412,30 +416,48 @@ public class EfficientDocumentHelper { Uri parentDirOfFile = getDocumentUri(unknownStr, true, false); - // it should be safe because user cannot select sd root or primary root - // and any other path would have at least one '/' to provide a file name in a folder String fileName = UriHelper.getFileName(unknownStr); + return createFileInDirectory(parentDirOfFile, fileName, mimeType, replaceIfDestinationExists); + } + + private Uri createFileInDirectory( + final Uri parentDirOfFile, + final String fileName, + final String mimeType, + final boolean replaceIfDestinationExists + ) throws IOException { if (fileName.indexOf(':') != -1) { throw new IOExceptionFast( "Invalid file name: Could not extract filename from uri string provided"); } - // maybe edited maybe not + String creationMimeType = "*/*"; + + // Some providers append the extension associated with the MIME type. Pass + // the MIME type only when the requested name already has an extension; + // extensionless Joplin resource IDs must remain extensionless. String correctFileName = fileName; - // only files with mime type are special, so we treat it special if (mimeType != null && !mimeType.equals("")) { - int indexOfDot = fileName.indexOf('.'); + int indexOfDot = fileName.lastIndexOf('.'); // len - 1 because there should be an extension that has at least 1 letter - if (indexOfDot != -1 && indexOfDot < fileName.length() - 1) { + if (indexOfDot > 0 && indexOfDot < fileName.length() - 1) { correctFileName = fileName.substring(0, indexOfDot); + creationMimeType = mimeType; } } + // Directory-based callers have already listed the parent and rechecked its + // document cache. Querying the directory again here would make bulk creation + // quadratic on providers that cannot filter children. A provider that permits + // exact same-name siblings could still race with an unrelated external writer + // between that listing and createDocument(). This is an inherent, pre-existing + // SAF limitation shared by other createDocument call sites; provider-renamed + // collisions are detected and cleaned up below. Uri createdFile = DocumentsContract.createDocument( context.getContentResolver(), parentDirOfFile, - mimeType != null && !mimeType.equals("") ? mimeType : "*/*", + creationMimeType, correctFileName ); @@ -447,23 +469,58 @@ public class EfficientDocumentHelper { String createdFileName = UriHelper.getFileName(createdFile.toString()); if (!createdFileName.equals(fileName)) { - // some times setting mimetypes causes name changes, this is to prevent that. - try { - createdFile = renameTo(createdFile, fileName); - } catch (RenameFailedException e) { - unlink(e.getResultUri()); + // A provider can change the name when a document with the requested name + // already exists. Do not try to rename this document into the occupied + // destination: a rename can partially succeed before reporting an error. + // Delete the document returned by this create call. Replacement-capable + // callers can then resolve and overwrite the intended destination; + // create-only callers retain their collision failure semantics. + unlink(createdFile); + if (!replaceIfDestinationExists) { throw new IOExceptionFast( "The created file name was not as expected: input name was '" - + e.getInputName() - + "' " - + "but got: '" - + e.getResultName()); + + fileName + + "' but got: '" + + createdFileName + + "'"); } + DocumentStat existingFile = findFile(parentDirOfFile, fileName); + if (existingFile == null || existingFile.isDirectory()) { + throw new IOExceptionFast( + "The created file name was not as expected and the existing destination could not be resolved: input name was '" + + fileName + + "' but got: '" + + createdFileName + + "'"); + } + return existingFile.getInternalUri(); } return createdFile; } + private void writeToFile(final Uri uri, final String data, final String encoding, final boolean append) throws IOException { + byte[] bytes = GeneralHelper.stringToBytes(data, encoding); + + try (OutputStream out = + context + .getContentResolver() + .openOutputStream(uri, append ? "wa" : "wt")) { + out.write(bytes); + } + } + + private void copyFileContents(final Uri sourceUri, final Uri destinationUri) throws IOException { + try (InputStream inStream = context.getContentResolver().openInputStream(sourceUri); + OutputStream outStream = context.getContentResolver().openOutputStream(destinationUri, "wt")) { + byte[] buffer = new byte[1024 * 4]; + int length; + while ((length = inStream.read(buffer)) > 0) { + outStream.write(buffer, 0, length); + } + } + } + // all public methods SHOULD be below here // and MUST start an Async task @@ -575,7 +632,7 @@ public class EfficientDocumentHelper { @Override public Object doAsync() { try { - Uri uri = createFile(unknownStr, mimeType); + Uri uri = createFile(unknownStr, mimeType, false); return getStat(uri).getWritableMap(); } catch (Exception e) { return e; @@ -734,24 +791,25 @@ public class EfficientDocumentHelper { public Object doAsync() { try { Uri uri; + Uri suppliedUri = UriHelper.getUnifiedUri(unknownStr); - try { - uri = getDocumentUri(unknownStr, false, true); - if (uri == null) { - throw new FileNotFoundExceptionFast(); + if (UriHelper.isDocumentUri(suppliedUri)) { + // A direct document URI already identifies the destination. Opening its + // output stream validates it, so querying it first only adds provider + // overhead to every cached overwrite. + uri = suppliedUri; + } else { + try { + uri = getDocumentUri(unknownStr, false, true); + if (uri == null) { + throw new FileNotFoundExceptionFast(); + } + } catch (FileNotFoundException e) { + uri = createFile(unknownStr, mimeType, true); } - } catch (FileNotFoundException e) { - uri = createFile(unknownStr, mimeType); } - byte[] bytes = GeneralHelper.stringToBytes(data, encoding); - - try (OutputStream out = - context - .getContentResolver() - .openOutputStream(uri, append ? "wa" : "wt")) { - out.write(bytes); - } + writeToFile(uri, data, encoding, append); return null; } catch (Exception e) { @@ -770,6 +828,82 @@ public class EfficientDocumentHelper { }); } + public void writeFileInDirectory( + String directoryUriString, + String fileName, + String data, + String encoding, + String mimeType, + boolean replaceIfDestinationExists, + final Promise promise) { + Async.execute(new Async.Task() { + @Override + public Object doAsync() { + try { + Uri directoryUri = getDocumentUri(directoryUriString, false, true); + Uri fileUri = createFileInDirectory( + directoryUri, fileName, mimeType, replaceIfDestinationExists + ); + writeToFile(fileUri, data, encoding, false); + return getStat(fileUri).getWritableMap(); + } catch (Exception e) { + return e; + } + } + + @Override + public void doSync(Object o) { + if (o instanceof Exception) { + rejectWithException((Exception) o, promise); + } else { + promise.resolve(o); + } + } + }); + } + + public void copyFileToDirectory( + String sourceUriString, + String directoryUriString, + String fileName, + boolean replaceIfDestinationExists, + final Promise promise) { + Async.execute(new Async.Task() { + @Override + public Object doAsync() { + try { + Uri sourceUri = getDocumentUri(sourceUriString, false, true); + String sourceMimeType = null; + // Extensionless Joplin resources are deliberately created with */* so + // providers do not append an extension. A source stat cannot affect the + // destination in that case and only adds another provider round trip. + int extensionIndex = fileName.lastIndexOf('.'); + if (extensionIndex > 0 && extensionIndex < fileName.length() - 1) { + DocumentStat sourceStat = getStat(sourceUri); + sourceMimeType = sourceStat.getMimeType(); + } + Uri directoryUri = getDocumentUri(directoryUriString, false, true); + Uri destinationUri = createFileInDirectory( + directoryUri, fileName, sourceMimeType, replaceIfDestinationExists + ); + copyFileContents(sourceUri, destinationUri); + return getStat(destinationUri).getWritableMap(); + } catch (Exception e) { + return e; + } + } + + @Override + public void doSync(Object o) { + if (o instanceof Exception) { + rejectWithException((Exception) o, promise); + } else { + promise.resolve(o); + } + } + }); + } + public void transferFile( String unknownSrcUri, String unknownDestUri, boolean replaceIfDestExists, boolean copy, Promise promise) { @@ -799,24 +933,15 @@ public class EfficientDocumentHelper { } } catch (FileNotFoundException e) { if (srcStats != null) { - destUri = createFile(unknownDestUri, srcStats.getMimeType()); + destUri = createFile(unknownDestUri, srcStats.getMimeType(), replaceIfDestExists); } else { // createFile() will treat mimetype null as "*/*" - destUri = createFile(unknownDestUri, null); + destUri = createFile(unknownDestUri, null, replaceIfDestExists); } } - try (InputStream inStream = - context.getContentResolver().openInputStream(srcUri); - OutputStream outStream = - context.getContentResolver().openOutputStream(destUri, "wt")) { - byte[] buffer = new byte[1024 * 4]; - int length; - while ((length = inStream.read(buffer)) > 0) { - outStream.write(buffer, 0, length); - } - } + copyFileContents(srcUri, destUri); if (!copy) { unlink(srcUri); diff --git a/packages/react-native-saf-x/src/index.ts b/packages/react-native-saf-x/src/index.ts index 8b911c265f..54179c5ec9 100644 --- a/packages/react-native-saf-x/src/index.ts +++ b/packages/react-native-saf-x/src/index.ts @@ -50,6 +50,15 @@ interface SafxInterface { mimeType?: string, append?: boolean, ): Promise; + writeFileInDirectory( + directoryUri: string, + fileName: string, + data: string, + encoding?: Encoding, + mimeType?: string, + replaceIfDestinationExists?: boolean, + ): Promise; + copyFileToDirectory(sourceUri: string, directoryUri: string, fileName: string, replaceIfDestinationExists?: boolean): Promise; createFile(uriString: string, mimeType?: string): Promise; unlink(uriString: string): Promise; mkdir(uriString: string): Promise; @@ -68,6 +77,7 @@ interface SafxInterface { export type DocumentFileDetail = { uri: string; + documentUri?: string; name: string; type: 'directory' | 'file'; lastModified: number; @@ -153,6 +163,21 @@ export function writeFile( return SafX.writeFile(uriString, data, encoding, mimeType, !!append); } +export function writeFileInDirectory( + directoryUri: string, + fileName: string, + data: string, + options?: Pick & FileTransferOptions, +) { + if (!options) options = {}; + const { encoding, mimeType, replaceIfDestinationExists = false } = options; + return SafX.writeFileInDirectory(directoryUri, fileName, data, encoding, mimeType, replaceIfDestinationExists); +} + +export function copyFileToDirectory(sourceUri: string, directoryUri: string, fileName: string, options: FileTransferOptions = {}) { + return SafX.copyFileToDirectory(sourceUri, directoryUri, fileName, !!options.replaceIfDestinationExists); +} + // Creates an empty file at given uri. // Rejects if a file or directory exist at given uri. export function createFile( @@ -243,6 +268,8 @@ export default { exists, readFile, writeFile, + writeFileInDirectory, + copyFileToDirectory, createFile, unlink, mkdir,