diff --git a/README.md b/README.md index 24d87446..de9deaa6 100644 --- a/README.md +++ b/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` 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` 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` @@ -161,10 +163,6 @@ Removes journaled events once processed. ### `getAllUploads(): Promise` 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. diff --git a/android/src/main/java/ai/openspace/backgroundupload/Upload.kt b/android/src/main/java/ai/openspace/backgroundupload/Upload.kt index d4d753a5..6d043f7e 100644 --- a/android/src/main/java/ai/openspace/backgroundupload/Upload.kt +++ b/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) } diff --git a/android/src/main/java/ai/openspace/backgroundupload/UploadWorker.kt b/android/src/main/java/ai/openspace/backgroundupload/UploadWorker.kt index 9ad6380b..039dc7c7 100644 --- a/android/src/main/java/ai/openspace/backgroundupload/UploadWorker.kt +++ b/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 diff --git a/android/src/main/java/ai/openspace/backgroundupload/UploaderModule.kt b/android/src/main/java/ai/openspace/backgroundupload/UploaderModule.kt index aed7f4e3..5136bb09 100644 --- a/android/src/main/java/ai/openspace/backgroundupload/UploaderModule.kt +++ b/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) diff --git a/android/src/test/java/ai/openspace/backgroundupload/UploadTest.kt b/android/src/test/java/ai/openspace/backgroundupload/UploadTest.kt index de856578..070c2bca 100644 --- a/android/src/test/java/ai/openspace/backgroundupload/UploadTest.kt +++ b/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(), diff --git a/example/RNBGUExample/App.tsx b/example/RNBGUExample/App.tsx index d3147e02..90693d49 100644 --- a/example/RNBGUExample/App.tsx +++ b/example/RNBGUExample/App.tsx @@ -36,16 +36,16 @@ const App = () => { >(); 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)); }); }, []); diff --git a/ios/RNBackgroundUpload.swift b/ios/RNBackgroundUpload.swift index 2aa64764..7a11480f 100644 --- a/ios/RNBackgroundUpload.swift +++ b/ios/RNBackgroundUpload.swift @@ -183,16 +183,47 @@ public class RNBackgroundUpload: NSObject, URLSessionDataDelegate { // RN bridges a JS number[] to NSArray; 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" - } - } } diff --git a/ios/RNFileUploader.mm b/ios/RNFileUploader.mm index 0ce76070..6ff3ff24 100644 --- a/ios/RNFileUploader.mm +++ b/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 { diff --git a/src/NativeRNFileUploader.ts b/src/NativeRNFileUploader.ts index ffeec3e8..1c7245f5 100644 --- a/src/NativeRNFileUploader.ts +++ b/src/NativeRNFileUploader.ts @@ -10,8 +10,6 @@ import { TurboModuleRegistry } from 'react-native'; export interface Spec extends TurboModule { startUpload(options: CodegenTypes.UnsafeObject): Promise; cancelUpload(id: string): Promise; - // iOS returns { state, bytesSent, totalBytes }; Android returns null. - getUploadStatus(id: string): Promise; getUnacknowledgedEvents(): Promise; ackEvents(ids: string[]): Promise; getAllUploads(): Promise; diff --git a/src/__tests__/index.test.ts b/src/__tests__/index.test.ts index c81cb5db..5e4e5b5d 100644 --- a/src/__tests__/index.test.ts +++ b/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); }); }); diff --git a/src/index.ts b/src/index.ts index 8a0b21e9..2e763455 100644 --- a/src/index.ts +++ b/src/index.ts @@ -53,8 +53,8 @@ const cancelUpload = (cancelUploadId: string): Promise => NativeRNFileUploader.cancelUpload(cancelUploadId); /** - * Listens for the given event on the given upload ID (resolved from startUpload). - * If you don't supply a value for uploadId, the event will fire for all uploads. + * Listens for one event type across all uploads. Use `data.id` to identify + * the upload. * Events (id is always the upload ID): * progress - { id, progress: 0-100 } * error - { id, error, errorKind?, responseCode?, responseBody?, responseHeaders? } @@ -63,30 +63,20 @@ const cancelUpload = (cancelUploadId: string): Promise => */ const addListener = (( eventType: 'progress' | 'error' | 'completed' | 'cancelled', - uploadId: UploadId | null, // The payload shape varies per event; the public AddListener overloads carry // the precise contract, so the internal forwarder stays untyped. // eslint-disable-next-line @typescript-eslint/no-explicit-any listener: (data: any) => void, ): EventSubscription => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const forMatchingUpload = (data: any) => { - // A scoped subscription drops anything it can't attribute, rather than - // failing open and delivering another upload's (or an unidentified) event. - if (!uploadId || data?.id === uploadId) { - listener(data); - } - }; - switch (eventType) { case 'progress': - return NativeRNFileUploader.onProgress(forMatchingUpload); + return NativeRNFileUploader.onProgress(listener); case 'error': - return NativeRNFileUploader.onError(forMatchingUpload); + return NativeRNFileUploader.onError(listener); case 'cancelled': - return NativeRNFileUploader.onCancelled(forMatchingUpload); + return NativeRNFileUploader.onCancelled(listener); case 'completed': - return NativeRNFileUploader.onCompleted(forMatchingUpload); + return NativeRNFileUploader.onCompleted(listener); default: throw new Error(`Unknown upload event: ${eventType}`); } @@ -117,34 +107,6 @@ const ackEvents = (eventIds: string[]): Promise => const getAllUploads = async (): Promise => (await NativeRNFileUploader.getAllUploads()) as UploadSnapshot[]; -const ios = { - /** - * Directly check the state of a single upload task without using event listeners. - * Note that this method has no way of distinguishing between a task being completed, errored, or non-existent. - * They're all `undefined`. You will need to either rely on the listeners or - * check with the API service you're using to upload. - * - * Android always resolves `undefined`. - */ - getUploadStatus: async ( - jobId: string, - ): Promise< - | { - state: 'running' | 'suspended' | 'canceling' | 'completed'; - bytesSent: number; - totalBytes: number; - } - | undefined - > => - ((await NativeRNFileUploader.getUploadStatus(jobId)) as - | { - state: 'running' | 'suspended' | 'canceling' | 'completed'; - bytesSent: number; - totalBytes: number; - } - | null) ?? undefined, -}; - const android = { /** * When the upload progress notification is pressed, it will open the app and fire this event. @@ -162,6 +124,5 @@ export default { getUnacknowledgedEvents, ackEvents, getAllUploads, - ios, android, }; diff --git a/src/types.ts b/src/types.ts index 6bf31eb2..14fe8f0d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -79,7 +79,7 @@ export type UploadOptions = { url: string; path: string; method: 'POST' | 'GET' | 'PUT' | 'PATCH' | 'DELETE'; - customUploadId?: string; + id?: string; headers?: { [index: string]: string; }; @@ -99,10 +99,6 @@ export type AndroidOnlyUploadOptions = { notificationTitleNoWifi: string; notificationTitleNoInternet: string; notificationChannel: string; - // Does not retry based on http code. - // Only retry IO and other unknown issues. - // Network failure does not count towards retries - maxRetries?: number; /** * Uploads this file without a progress notification. Default false. * @@ -119,37 +115,21 @@ export type RawUploadOptions = { type: 'raw'; }; -// TODO support this to replace netq -// type MultipartUploadOptions = { -// type: 'multipart'; -// field: string; -// parameters?: { -// [index: string]: string; -// }; -// }; - export interface AddListener { ( event: 'progress', - uploadId: UploadId | null, callback: (data: ProgressData) => void, ): EventSubscription; - ( - event: 'error', - uploadId: UploadId | null, - callback: (data: ErrorData) => void, - ): EventSubscription; + (event: 'error', callback: (data: ErrorData) => void): EventSubscription; ( event: 'completed', - uploadId: UploadId | null, callback: (data: CompletedData) => void, ): EventSubscription; ( event: 'cancelled', - uploadId: UploadId | null, callback: (data: CancelledData) => void, ): EventSubscription; }