address file handling review

__tests__/lib/images.test.ts

@@ -1,12 +1,21 @@
 import {File, Paths} from 'expo-file-system'
 import {ImageManipulator, SaveFormat} from 'expo-image-manipulator'
+import * as Sharing from 'expo-sharing'
 
 import {IMAGE_SIZE_CONFIG_2K_1MB} from '../../src/lib/constants'
 import {
   downloadAndResize,
   type DownloadAndResizeOpts,
+  saveToDevice,
+  shareImageModal,
 } from '../../src/lib/media/manip'
 import {getResizedDimensions} from '../../src/lib/media/util'
+import {logger} from '../../src/logger'
+
+jest.mock('expo-sharing', () => ({
+  isAvailableAsync: jest.fn().mockResolvedValue(true),
+  shareAsync: jest.fn().mockResolvedValue(undefined),
+}))
 
 const mockResizedImage = {
   size: 100,
@@ -221,3 +230,39 @@ describe('downloadAndResize', () => {
     })
   })
 })
+
+describe('temporary file lifecycles', () => {
+  beforeEach(() => {
+    ;(File.downloadFileAsync as jest.Mock).mockResolvedValue(
+      new File('file://downloaded-image.jpg'),
+    )
+  })
+
+  afterEach(() => {
+    jest.clearAllMocks()
+  })
+
+  it('retains a successfully shared image for lazy consumers', async () => {
+    await shareImageModal({uri: 'https://example.com/image.jpg'})
+
+    const sharedPath = (Sharing.shareAsync as jest.Mock).mock.calls[0][0]
+    const deletedPaths = (Paths.info as jest.Mock).mock.calls.map(
+      ([path]) => path,
+    )
+    expect(sharedPath).toContain('/bsky-share/')
+    expect(deletedPaths).not.toContain(sharedPath)
+  })
+
+  it('does not report user cancellation as an error', async () => {
+    const errorSpy = jest.spyOn(logger, 'error')
+    ;(Sharing.shareAsync as jest.Mock).mockRejectedValueOnce(
+      new Error('Picker cancelled'),
+    )
+
+    await expect(
+      saveToDevice('test.txt', 'dGVzdA==', 'text/plain'),
+    ).resolves.toBe(false)
+    expect(errorSpy).not.toHaveBeenCalled()
+    errorSpy.mockRestore()
+  })
+})

src/lib/media/manip.ts

@@ -5,6 +5,7 @@ import {SaveFormat} from 'expo-image-manipulator'
 import * as MediaLibrary from 'expo-media-library/legacy'
 import * as Sharing from 'expo-sharing'
 
+import {isCancelledError} from '#/lib/strings/errors'
 import {logger} from '#/logger'
 import {IS_ANDROID, IS_IOS} from '#/env'
 import {renderImage} from './image-manipulator'
@@ -67,9 +68,11 @@ export async function shareImageModal({uri}: {uri: string}) {
     return
   }
 
+  const shareDirectory = prepareShareDirectory()
   const downloadedPath = await downloadImage(uri, String(uuid.v4()), 15e3)
   let jpegUri: string | undefined
   let imagePath: string | undefined
+  let didShare = false
 
   try {
     const jpeg = await renderImage(downloadedPath, undefined, {
@@ -77,15 +80,16 @@ export async function shareImageModal({uri}: {uri: string}) {
       compress: 1.0,
     })
     jpegUri = jpeg.uri
-    imagePath = await moveToPermanentPath(jpegUri, '.jpg')
+    imagePath = await moveToPermanentPath(jpegUri, '.jpg', shareDirectory)
     await Sharing.shareAsync(imagePath, {
       mimeType: 'image/jpeg',
       UTI: 'image/jpeg',
     })
+    didShare = true
   } finally {
     safeDelete(downloadedPath)
     if (jpegUri) safeDelete(jpegUri)
-    if (imagePath) safeDelete(imagePath)
+    if (imagePath && !didShare) safeDelete(imagePath)
   }
 }
 
@@ -269,7 +273,11 @@ async function doResize(
   }
 }
 
-async function moveToPermanentPath(path: string, ext: string): Promise<string> {
+async function moveToPermanentPath(
+  path: string,
+  ext: string,
+  destinationDirectory = Paths.cache,
+): Promise<string> {
   /*
   Since this package stores images in a temp directory, we need to move the file to a permanent location.
   Relevant: IOS bug when trying to open a second time:
@@ -277,12 +285,30 @@ async function moveToPermanentPath(path: string, ext: string): Promise<string> {
   */
   const filename = uuid.v4()
 
-  const destination = new File(Paths.cache, filename + ext)
+  const destination = new File(destinationDirectory, filename + ext)
   await new File(normalizePath(path)).copy(destination)
   safeDelete(path)
   return normalizePath(destination.uri)
 }
 
+function prepareShareDirectory(): Directory {
+  const directory = new Directory(Paths.cache, 'bsky-share')
+
+  /*
+   * Keep the current file alive after the share sheet closes because Android
+   * targets may consume its content URI lazily. Remove the previous share the
+   * next time a share starts so the cache remains bounded.
+   */
+  if (directory.exists) {
+    for (const entry of directory.list()) {
+      safeDelete(entry.uri)
+    }
+  } else {
+    directory.create({intermediates: true})
+  }
+  return directory
+}
+
 export function safeDelete(path: string) {
   const normalizedPath = normalizePath(path)
   try {
@@ -338,6 +364,9 @@ export async function saveToDevice(
       return true
     }
   } catch (e) {
+    if (isCancelledError(e)) {
+      return false
+    }
     logger.error('Error occurred while saving file', {message: e})
     return false
   }

src/state/shell/composer/index.tsx

@@ -1,4 +1,12 @@
-import {createContext, useContext, useMemo, useState} from 'react'
+import {
+  createContext,
+  useContext,
+  useEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react'
+import {InteractionManager} from 'react-native'
 import {type ModerationDecision} from '@bsky/sdk/moderation'
 import {msg} from '@lingui/core/macro'
 import {useLingui} from '@lingui/react'
@@ -68,6 +76,14 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
   const {_} = useLingui()
   const [state, setState] = useState<StateContext>()
   const queryClient = useQueryClient()
+  const pendingPurgeRef =
+    useRef<ReturnType<typeof InteractionManager.runAfterInteractions>>(
+      undefined,
+    )
+
+  useEffect(() => {
+    return () => pendingPurgeRef.current?.cancel()
+  }, [])
 
   const openComposer = useNonReactiveCallback((opts: ComposerOpts) => {
     if (opts.quote) {
@@ -97,6 +113,12 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         type: 'warning',
       })
     } else {
+      /*
+       * A purge scheduled by the previous composer must not delete files from
+       * a composer that is opened before the callback gets to run.
+       */
+      pendingPurgeRef.current?.cancel()
+      pendingPurgeRef.current = undefined
       setState(prevOpts => {
         if (prevOpts) {
           // Never replace an already open composer.
@@ -111,13 +133,17 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
     let wasOpen = !!state
     if (wasOpen) {
       setState(undefined)
-      purgeTemporaryImageFiles()
+      pendingPurgeRef.current?.cancel()
-      // Purging deletes cached thumbnails on disk, so remove the query
+      pendingPurgeRef.current = InteractionManager.runAfterInteractions(() => {
-      // caches that may hold references to those now-deleted file paths.
+        pendingPurgeRef.current = undefined
-      // Without this, restoring a draft would serve stale ResolvedLink
+        purgeTemporaryImageFiles()
-      // data pointing at missing files, causing "Failed to load blob".
+        // Purging deletes cached thumbnails on disk, so remove the query
-      queryClient.removeQueries({queryKey: [RQKEY_LINK_ROOT]})
+        // caches that may hold references to those now-deleted file paths.
-      queryClient.removeQueries({queryKey: [RQKEY_GIF_ROOT]})
+        // Without this, restoring a draft would serve stale ResolvedLink
+        // data pointing at missing files, causing "Failed to load blob".
+        queryClient.removeQueries({queryKey: [RQKEY_LINK_ROOT]})
+        queryClient.removeQueries({queryKey: [RQKEY_GIF_ROOT]})
+      })
     }
 
     return wasOpen