feat: support concurrent chunk uploads#313
Conversation
|
Closing in favor of focused PR #314. |
Greptile SummaryThis PR adds concurrent chunk uploads to the Flutter SDK, allowing up to 8 parallel HTTP requests when uploading large files, and bumps the version to 24.2.0.
Confidence Score: 4/5Safe to merge; the concurrent worker-pool logic is correct for Dart's cooperative async model and the upload-session ID is properly forwarded to all chunks. The core concurrency logic is sound — Dart's single-threaded event loop makes the shared nextChunk/completedChunks/uploadedBytes state safe between awaits. The main concern is that lastResponse is only updated when isUploadComplete fires; if the server ever omits chunksUploaded from intermediate responses, callers receive the first-chunk response as the upload result. There is also a minor dead parameter (index in uploadChunk) in both client files. lib/src/client_browser.dart and lib/src/client_io.dart warrant a second look around the lastResponse update logic and the unused index parameter. Important Files Changed
Reviews (1): Last reviewed commit: "Commit from GitHub Actions (Format and p..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Future<Response> uploadChunk( | ||
| int index, int start, int end, String? id) async { | ||
| List<int> chunk = []; | ||
| final end = min(offset + chunkSize, size); | ||
| chunk = file.bytes!.getRange(offset, end).toList(); | ||
| params[paramName] = http.MultipartFile.fromBytes( | ||
| chunk = file.bytes!.getRange(start, end).toList(); | ||
|
|
||
| final chunkParams = Map<String, dynamic>.from(params); | ||
| chunkParams[paramName] = http.MultipartFile.fromBytes( | ||
| paramName, | ||
| chunk, | ||
| filename: file.filename, | ||
| ); | ||
| headers['content-range'] = | ||
| 'bytes $offset-${min<int>((offset + chunkSize - 1), size - 1)}/$size'; | ||
| res = await call( | ||
| final chunkHeaders = Map<String, String>.from(headers); | ||
| if (id != null && id.isNotEmpty) { | ||
| chunkHeaders['x-appwrite-id'] = id; | ||
| } | ||
| chunkHeaders['content-range'] = 'bytes $start-${end - 1}/$size'; | ||
|
|
||
| return call( | ||
| HttpMethod.post, | ||
| path: path, | ||
| headers: headers, | ||
| params: params, | ||
| headers: chunkHeaders, | ||
| params: chunkParams, | ||
| ); | ||
| offset += chunkSize; | ||
| if (offset < size) { | ||
| headers['x-appwrite-id'] = res.data['\$id']; | ||
| } |
There was a problem hiding this comment.
index parameter in uploadChunk
The index parameter is accepted by uploadChunk in both client_browser.dart and client_io.dart but is never referenced in either function body. All positioning is done via start/end, so the parameter appears to be dead code. Remove it or document its intended use to avoid confusion.
| if (isUploadComplete(chunkResponse)) { | ||
| lastResponse = chunkResponse; | ||
| } |
There was a problem hiding this comment.
lastResponse falls back to the first-chunk response if the server never signals completion
lastResponse is only updated inside uploadNext when isUploadComplete returns true. If every concurrent chunk response has null or a non-num chunksUploaded field (e.g., a transient server regression), lastResponse stays as the first chunk's Response—which has stale metadata such as chunksUploaded: 1. The caller has no way to distinguish this from a genuinely incomplete upload. The old sequential loop always returned the last chunk's response regardless of field values.
1 participant
This PR updates the SDK to support concurrent chunk uploads.