Cut unused API surface, make iOS startUpload idempotent

README.md

@@ -70,10 +70,10 @@ const options = {
 
 const uploadId = await Upload.startUpload(options);
 
-Upload.addListener('progress', uploadId, ({ progress }) => {});
-Upload.addListener('completed', uploadId, ({ responseCode, responseBody }) => {});
-Upload.addListener('error', uploadId, ({ error, errorKind, responseCode }) => {});
-Upload.addListener('cancelled', uploadId, ({ cancelReason }) => {});
+Upload.addListener('progress', ({ id, progress }) => {});
+Upload.addListener('completed', ({ id, responseCode, responseBody }) => {});
+Upload.addListener('error', ({ id, error, errorKind, responseCode }) => {});
+Upload.addListener('cancelled', ({ id, cancelReason }) => {});
 ```
 
 # Reliable delivery
@@ -116,7 +116,9 @@ All methods are on the default export.
 ### `startUpload(options): Promise<string>`
 Starts an upload; resolves to its id. Rejects only on a bad option (missing/invalid
 `url` or `path`) — transport failures and HTTP error responses arrive later as
-`error` events, not a rejection.
+`error` events, not a rejection. Idempotent for a given `id`: calling
+it again while that upload is pending or running resolves with the same id instead
+of starting a duplicate.
 
 | Option | Type | Notes |
 | --- | --- | --- |
@@ -125,10 +127,10 @@ Starts an upload; resolves to its id. Rejects only on a bad option (missing/inva
 | `type` | `'raw'` | Only `raw` is supported. |
 | `method` | string | Default `POST`. |
 | `headers` | object | HTTP headers. |
-| `customUploadId` | string | Defaults to a generated UUID. |
+| `id` | string | Defaults to a generated UUID. |
 | `wifiOnly` | boolean | Wait for wifi before/while uploading. |
 | `acceptStatus` | number[] | Non-2xx statuses to treat as success. |
-| `android` | object | Optional. `notificationId/Title/TitleNoWifi/TitleNoInternet/Channel`, `maxRetries` (default 5), `noNotification` (default false). Sensible defaults + auto-created channel if omitted. |
+| `android` | object | Optional. `notificationId/Title/TitleNoWifi/TitleNoInternet/Channel`, `noNotification` (default false). Sensible defaults + auto-created channel if omitted. |
 
 #### Silent uploads (Android)
 
@@ -147,9 +149,9 @@ reports every in-flight upload — silent ones included.
 ### `cancelUpload(uploadId): Promise<boolean>`
 Cancels an upload. Fires a `cancelled` event with `cancelReason: 'user'`.
 
-### `addListener(eventType, uploadId | null, listener): EventSubscription`
-Listen for `'progress' | 'error' | 'completed' | 'cancelled'`. Pass `null` for
-`uploadId` to receive events for all uploads. Call `.remove()` on the result to
+### `addListener(eventType, listener): EventSubscription`
+Listen for `'progress' | 'error' | 'completed' | 'cancelled'` across all uploads;
+every event carries the upload's `id`. Call `.remove()` on the result to
 unsubscribe.
 
 ### `getUnacknowledgedEvents(): Promise<JournaledEvent[]>`
@@ -161,10 +163,6 @@ Removes journaled events once processed.
 ### `getAllUploads(): Promise<UploadSnapshot[]>`
 Uploads the OS still knows about, for boot-time reconciliation.
 
-### `ios.getUploadStatus(uploadId)`
-iOS-only live task state (`running | suspended | canceling`, plus byte counts), or
-`undefined` if the task isn't active.
-
 ### `android.addNotificationListener(listener)`
 Fires when the Android progress notification is pressed. No event data.
 

android/src/main/java/ai/openspace/backgroundupload/Upload.kt

@@ -11,7 +11,6 @@ data class Upload(
   val url: String,
   val path: String,
   val method: String,
-  val maxRetries: Int,
   val wifiOnly: Boolean,
   // Non-2xx statuses to treat as a successful completion (e.g. [409] when
   // duplicate-create conflicts are expected). Everything else non-2xx is a
@@ -47,11 +46,10 @@ data class Upload(
     const val DEFAULT_NOTIFICATION_CHANNEL = "background-upload"
 
     fun fromReadableMap(map: ReadableMap) = Upload(
-      id = map.getString("customUploadId") ?: UUID.randomUUID().toString(),
+      id = map.getString(Upload::id.name) ?: UUID.randomUUID().toString(),
       url = map.getString(Upload::url.name) ?: throw MissingOptionException(Upload::url.name),
       path = map.getString(Upload::path.name) ?: throw MissingOptionException(Upload::path.name),
       method = map.getString(Upload::method.name) ?: "POST",
-      maxRetries = if (map.hasKey(Upload::maxRetries.name)) map.getInt(Upload::maxRetries.name) else 5,
       wifiOnly = if (map.hasKey(Upload::wifiOnly.name)) map.getBoolean(Upload::wifiOnly.name) else false,
       acceptStatus = map.getArray(Upload::acceptStatus.name)?.let { arr ->
         (0 until arr.size()).map { i -> arr.getInt(i) }

android/src/main/java/ai/openspace/backgroundupload/UploadWorker.kt

@@ -34,6 +34,11 @@ private const val MAX_CONCURRENCY = 1
 // Retry delay
 private val RETRY_DELAY = TimeUnit.SECONDS.toMillis(10L)
 
+// The retry budget for errors that count (see checkRetry). A connectivity gap
+// or flaky-network IO resets the budget. The retry policy is internal to the
+// library. It is not an option.
+private const val MAX_RETRIES = 5
+
 // Max total time for a single request to complete
 // This is 24hrs so plenty of time for large uploads
 // Worst case is the time maxes out and the upload gets restarted.
@@ -287,7 +292,7 @@ class UploadWorker(private val context: Context, params: WorkerParameters) :
     }
 
     retries = if (unlimitedRetry) 0 else retries + 1
-    return retries <= upload.maxRetries
+    return retries <= MAX_RETRIES
   }
 
   // Checks connection and alerts connection issues

android/src/main/java/ai/openspace/backgroundupload/UploaderModule.kt

@@ -158,16 +158,6 @@ class UploaderModule(context: ReactApplicationContext) :
   }
 
 
-  /**
-   * iOS-only: there is no per-task byte counter to read on Android, where uploads
-   * are WorkManager jobs rather than URLSession tasks. Use getAllUploads for
-   * liveness and the progress event for bytes.
-   */
-  override fun getUploadStatus(id: String, promise: Promise) {
-    promise.resolve(null)
-  }
-
-
   /*
    * Starts a file upload.
    * Returns a promise with the string ID of the upload.
@@ -192,7 +182,7 @@ class UploaderModule(context: ReactApplicationContext) :
     val upload = Upload.fromReadableMap(options)
     val data = Gson().toJson(upload)
 
-    // Clear any stale user-cancel mark for this (possibly reused customUploadId)
+    // Clear any stale user-cancel mark for this (possibly reused) id
     // from a prior life, so a later system stop of this fresh upload isn't
     // misreported as a user cancel. Done here (before enqueue), never in the
     // worker, so a real cancel arriving as the worker starts can't be erased.
@@ -227,8 +217,8 @@ class UploaderModule(context: ReactApplicationContext) :
         .firstOrNull { !it.state.isFinished }
 
       if (active == null) {
-        // Nothing to cancel. Drop any mark so a later upload reusing this
-        // customUploadId can't be misreported as a user cancel.
+        // Nothing to cancel. Drop any mark so a later upload reusing this id
+        // can't be misreported as a user cancel.
         UserCancellations.consume(id)
         promise.resolve(false)
 

android/src/test/java/ai/openspace/backgroundupload/UploadTest.kt

@@ -13,7 +13,6 @@ class UploadTest {
     url = "https://example.com/upload",
     path = "/tmp/file",
     method = "POST",
-    maxRetries = 5,
     wifiOnly = false,
     acceptStatus = listOf(),
     headers = mapOf(),

example/RNBGUExample/App.tsx

@@ -17,7 +17,7 @@
   Button,
 } from 'react-native';
 import notifee, {AndroidImportance} from '@notifee/react-native';
 import {Colors} from 'react-native/Libraries/NewAppScreen';
 
 import Upload, {UploadOptions} from 'react-native-background-upload';
 
@@ -36,16 +36,16 @@
   >();
 
   useEffect(() => {
-    Upload.addListener('progress', null, data => {
+    Upload.addListener('progress', data => {
       setProgress(data.progress);
     });
-    Upload.addListener('error', null, data => {
+    Upload.addListener('error', data => {
       console.log('Error!', JSON.stringify(data));
     });
-    Upload.addListener('completed', null, data => {
+    Upload.addListener('completed', data => {
       console.log('Completed!', JSON.stringify(data));
     });
-    Upload.addListener('cancelled', null, data => {
+    Upload.addListener('cancelled', data => {
       console.log('Cancelled!', JSON.stringify(data));
     });
   }, []);
@@ -88,7 +88,7 @@
     };
 
     Upload.startUpload(uploadOpts)
       .then(uploadId => {
         console.log(
           `Upload started with options: ${JSON.stringify(uploadOpts)}`,
         );

ios/RNBackgroundUpload.swift

@@ -183,16 +183,47 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate {
     // RN bridges a JS number[] to NSArray<NSNumber>; map explicitly rather than
     // rely on an [Int] bridging cast that can yield nil and silently drop it.
     let acceptStatus = (options["acceptStatus"] as? [NSNumber])?.map { $0.intValue } ?? []
-    let uploadId = (options["customUploadId"] as? String) ?? UUID().uuidString
+    let uploadId = (options["id"] as? String) ?? UUID().uuidString
     let fileURL = URL(string: path) ?? URL(fileURLWithPath: path)
 
     let session = self.session(wifiOnly: wifiOnly)
-    let task = session.uploadTask(with: request, fromFile: fileURL)
-    task.taskDescription = uploadId
-    TaskMap.set(TaskMap.Meta(id: uploadId, acceptStatus: acceptStatus),
-                forKey: taskMapKey(session, task))
-    task.resume()
-    resolve(uploadId)
+    let startNew = {
+      let task = session.uploadTask(with: request, fromFile: fileURL)
+      task.taskDescription = uploadId
+      TaskMap.set(TaskMap.Meta(id: uploadId, acceptStatus: acceptStatus),
+                  forKey: self.taskMapKey(session, task))
+      task.resume()
+      resolve(uploadId)
+    }
+
+    // A consumer-supplied id makes startUpload idempotent. This is the same
+    // behavior as Android's ExistingWorkPolicy.KEEP. If a task with this id is
+    // already pending or running, we resolve with that id. We do not enqueue a
+    // second task. We examine both sessions, because a new call can set a
+    // different wifiOnly value while the first task continues in its first
+    // session. A generated id cannot collide, so that path does not do the
+    // (asynchronous) task enumeration.
+    guard options["id"] != nil else { startNew(); return }
+
+    let group = DispatchGroup()
+    let foundLock = NSLock()
+    var exists = false
+    for s in activeSessions {
+      group.enter()
+      s.getAllTasks { tasks in
+        for task in tasks
+        where self.uploadId(s, task) == uploadId
+          && (task.state == .running || task.state == .suspended) {
+          foundLock.lock()
+          exists = true
+          foundLock.unlock()
+        }
+        group.leave()
+      }
+    }
+    group.notify(queue: .main) {
+      if exists { resolve(uploadId) } else { startNew() }
+    }
   }
 
   @objc(cancelUpload:resolve:reject:)
@@ -228,7 +259,7 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate {
       foundLock.unlock()
       if !matched {
         // Nothing to cancel: drop the intent again so a later upload reusing this
-        // customUploadId isn't misattributed as a user cancel.
+        // id isn't misattributed as a user cancel.
         RNBackgroundUpload.lock.lock()
         RNBackgroundUpload.userCancelledIds.remove(cancelUploadId)
         RNBackgroundUpload.lock.unlock()
@@ -237,32 +268,6 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate {
     }
   }
 
-  @objc(getUploadStatus:resolve:reject:)
-  public func getUploadStatus(_ uploadId: String,
-                              resolve: @escaping RCTPromiseResolveBlock,
-                              reject: @escaping RCTPromiseRejectBlock) {
-    let sessions = activeSessions
-    let group = DispatchGroup()
-    let lock = NSLock()
-    var result: [String: Any]?
-    for session in sessions {
-      group.enter()
-      session.getAllTasks { tasks in
-        for task in tasks where self.uploadId(session, task) == uploadId {
-          lock.lock()
-          if result == nil {
-            result = ["state": self.stateString(task.state),
-                      "bytesSent": task.countOfBytesSent,
-                      "totalBytes": task.countOfBytesExpectedToSend]
-          }
-          lock.unlock()
-        }
-        group.leave()
-      }
-    }
-    group.notify(queue: .main) { resolve(result) }
-  }
-
   @objc(getUnacknowledgedEvents:reject:)
   public func getUnacknowledgedEvents(_ resolve: @escaping RCTPromiseResolveBlock,
                                       reject: @escaping RCTPromiseRejectBlock) {
@@ -388,7 +393,7 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate {
     // Consume the user-cancel intent on EVERY terminal outcome, not only the
     // cancelled branch. If cancelUpload lost the race with completion, the id
     // would otherwise linger for the life of the process and a later upload
-    // reusing that customUploadId would report a system cancel as a user cancel.
+    // reusing that id would report a system cancel as a user cancel.
     let userCancelled = RNBackgroundUpload.userCancelledIds.remove(id) != nil
     RNBackgroundUpload.lock.unlock()
 
@@ -468,14 +473,4 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate {
       return "unknown"
     }
   }
-
-  private func stateString(_ state: URLSessionTask.State) -> String {
-    switch state {
-    case .running: return "running"
-    case .suspended: return "suspended"
-    case .canceling: return "canceling"
-    case .completed: return "completed"
-    @unknown default: return "running"
-    }
-  }
 }

ios/RNFileUploader.mm

@@ -80,13 +80,6 @@ - (void)cancelUpload:(NSString *)id
   [RNBackgroundUpload.shared cancelUpload:id resolve:resolve reject:reject];
 }
 
-- (void)getUploadStatus:(NSString *)id
-                resolve:(RCTPromiseResolveBlock)resolve
-                 reject:(RCTPromiseRejectBlock)reject
-{
-  [RNBackgroundUpload.shared getUploadStatus:id resolve:resolve reject:reject];
-}
-
 - (void)getUnacknowledgedEvents:(RCTPromiseResolveBlock)resolve
                          reject:(RCTPromiseRejectBlock)reject
 {

src/NativeRNFileUploader.ts

@@ -10,8 +10,6 @@ import { TurboModuleRegistry } from 'react-native';
 export interface Spec extends TurboModule {
   startUpload(options: CodegenTypes.UnsafeObject): Promise<string>;
   cancelUpload(id: string): Promise<boolean>;
-  // iOS returns { state, bytesSent, totalBytes }; Android returns null.
-  getUploadStatus(id: string): Promise<CodegenTypes.UnsafeObject | null>;
   getUnacknowledgedEvents(): Promise<CodegenTypes.UnsafeObject[]>;
   ackEvents(ids: string[]): Promise<boolean>;
   getAllUploads(): Promise<CodegenTypes.UnsafeObject[]>;

src/__tests__/index.test.ts

@@ -8,7 +8,6 @@ jest.mock('react-native', () => {
   const nativeModule = {
     startUpload: jest.fn(async () => 'id-1'),
     cancelUpload: jest.fn(async () => true),
-    getUploadStatus: jest.fn(async () => null),
     getUnacknowledgedEvents: jest.fn(async () => [
       {
         eventId: 'e1',
@@ -58,10 +57,6 @@ describe('journal + query API', () => {
     const uploads = await Upload.getAllUploads();
     expect(uploads[0]).toEqual({ id: 'u1', state: 'running' });
   });
-
-  it('getUploadStatus maps a null result to undefined', async () => {
-    await expect(Upload.ios.getUploadStatus('u1')).resolves.toBeUndefined();
-  });
 });
 
 describe('startUpload', () => {
@@ -85,18 +80,18 @@ describe('startUpload', () => {
 
 describe('addListener', () => {
   it('subscribes to the matching codegen emitter', () => {
-    Upload.addListener('progress', null, jest.fn());
+    Upload.addListener('progress', jest.fn());
     expect(native.onProgress).toHaveBeenCalled();
   });
 
-  it('only invokes the listener for the matching upload id', () => {
+  it('delivers events for every upload', () => {
     const cb = jest.fn();
-    Upload.addListener('completed', 'u1', cb);
+    Upload.addListener('completed', cb);
     const handler = native.onCompleted.mock.calls.at(-1)![0] as (
       data: unknown,
     ) => void;
     handler({ id: 'u1', responseCode: 200 });
     handler({ id: 'someone-else', responseCode: 200 });
-    expect(cb).toHaveBeenCalledTimes(1);
+    expect(cb).toHaveBeenCalledTimes(2);
   });
 });