feat: add multipart upload support to DioService with progress tracking#19
feat: add multipart upload support to DioService with progress tracking#19MohammedAEzz wants to merge 4 commits intoArjun544:mainfrom
Conversation
|
@MohammedAEzz is attempting to deploy a commit to the arjun544's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdded multipart file upload support to the Flutter Dio networking overlay: new Changes
Sequence DiagramsequenceDiagram
participant Client
participant DioService
participant Payload as UploadFilePayload
participant FormData
participant Dio as AppConfig.dio
participant Callback as onProgress
Client->>DioService: uploadFile(path, payload, onProgress)
DioService->>Payload: toMultipartFile()
Payload-->>DioService: MultipartFile
DioService->>FormData: _buildFormData(files, fields)
FormData-->>DioService: FormData ready
DioService->>Dio: post(path, formData, options, onSendProgress)
Dio->>Callback: onSendProgress(sent, total)
Callback->>Client: UploadProgress(sent, total)
Dio-->>DioService: Response
DioService-->>Client: FutureEither<Response>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs (1)
36-82: Consider collapsing single-file upload into the multi-file path.
uploadFileanduploadFilescurrently duplicate request assembly logic. DelegatinguploadFiletouploadFileswill reduce drift risk.Proposed refactor
FutureEither<Response> uploadFile( String path, UploadFilePayload payload, { void Function(UploadProgress)? onProgress, Map<String, dynamic>? fields, Map<String, dynamic>? queryParameters, }) { - return runTask( - () async { - final formData = await _buildFormData([payload], fields: fields); - return AppConfig.dio.post( - path, - data: formData, - queryParameters: queryParameters, - options: _multipartOptions(), - onSendProgress: (sent, total) { - onProgress?.call(UploadProgress(sent: sent, total: total)); - }, - ); - }, - requiresNetwork: true, - ); + return uploadFiles( + path, + [payload], + onProgress: onProgress, + fields: fields, + queryParameters: queryParameters, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs` around lines 36 - 82, Replace duplicated logic by delegating uploadFile to uploadFiles: in uploadFile, call uploadFiles(path, [payload], onProgress: onProgress, fields: fields, queryParameters: queryParameters) and return its FutureEither<Response> instead of rebuilding form data and posting; this reuses the existing _buildFormData and runTask flow in uploadFiles (referencing uploadFile, uploadFiles, _buildFormData, UploadFilePayload) and preserves the original signature and onProgress behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs`:
- Around line 95-100: The assert in _buildFormData is debug-only and won't
prevent empty uploads in release; replace the assert(payloads.isNotEmpty, ...)
with an explicit runtime validation that throws a clear exception (e.g.,
ArgumentError or StateError) when payloads.isEmpty, and ensure callers like
uploadFiles receive/propagate that error; update the _buildFormData signature to
perform this check at the top and include a descriptive message so invalid calls
fail reliably in all builds.
In
`@templates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs`:
- Around line 12-21: The constructor for UploadFilePayload currently uses an
assert to require filePath or bytes which is stripped in release builds; replace
the assert by removing the const constructor and adding a runtime check in the
UploadFilePayload constructor body that throws an ArgumentError (or similar) if
both filePath and bytes are null, or alternatively implement a factory that
validates and returns a const instance; ensure references to filePath! usages
(e.g., where filePath is dereferenced) remain safe after this change.
---
Nitpick comments:
In
`@templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs`:
- Around line 36-82: Replace duplicated logic by delegating uploadFile to
uploadFiles: in uploadFile, call uploadFiles(path, [payload], onProgress:
onProgress, fields: fields, queryParameters: queryParameters) and return its
FutureEither<Response> instead of rebuilding form data and posting; this reuses
the existing _buildFormData and runTask flow in uploadFiles (referencing
uploadFile, uploadFiles, _buildFormData, UploadFilePayload) and preserves the
original signature and onProgress behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e71bb0f-25eb-40d8-98f5-51c264bd965c
📒 Files selected for processing (4)
templates/flutter/base/lib/src/services/services.dart.hbstemplates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbstemplates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbstemplates/flutter/overlays/networking/dio/lib/src/services/upload_progress.dart.hbs
|
@coderabbitai @Arjun544 |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs (1)
80-83: Add a test template for multipart upload Content-Type handling in DioService.
AppConfig.diosets a globalContent-Type: application/json; the_multipartOptions()helper relies on Dio treatingheaders: {Headers.contentTypeHeader: null}as removal soFormDatacan set themultipart/form-data; boundary=...header. No test template exists to verify this merge behavior is correct. Consider creating a test template (similar totemplates/flutter/base/test/widget_test.dart.hbs) that exercises theuploadFilesmethod and confirms the Content-Type header is properly overridden for multipart requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs` around lines 80 - 83, Add a test template that verifies Dio merges/removes the global Content-Type when doing multipart uploads: create a new test (patterned after widget_test.dart.hbs) that constructs an AppConfig.dio with a global header Content-Type: application/json, calls the uploadFiles method, and asserts the outgoing request uses FormData's multipart/form-data; boundary=... header (i.e. that _multipartOptions headers: {Headers.contentTypeHeader: null} causes Dio to remove/override the global header). Locate tests exercising uploadFiles and add this template to ensure multipart behavior is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs`:
- Line 3: The template imports package:http_parser/http_parser.dart but
http_parser is not declared as a direct dependency in the Flutter pubspec
template; update the pubspec template's dependencies section to add the
http_parser package (e.g., add "http_parser: ^<latest-compatible-version>") so
generated apps don't rely on Dio's transitive dependency and satisfy the
depend_on_referenced_packages lint—make the change in the pubspec template used
by the project alongside the import in
templates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs.
---
Nitpick comments:
In
`@templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbs`:
- Around line 80-83: Add a test template that verifies Dio merges/removes the
global Content-Type when doing multipart uploads: create a new test (patterned
after widget_test.dart.hbs) that constructs an AppConfig.dio with a global
header Content-Type: application/json, calls the uploadFiles method, and asserts
the outgoing request uses FormData's multipart/form-data; boundary=... header
(i.e. that _multipartOptions headers: {Headers.contentTypeHeader: null} causes
Dio to remove/override the global header). Locate tests exercising uploadFiles
and add this template to ensure multipart behavior is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e70d121-a2e9-403e-b0df-923bcf774f6a
📒 Files selected for processing (2)
templates/flutter/overlays/networking/dio/lib/src/services/dio_service.dart.hbstemplates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs
| @@ -0,0 +1,62 @@ | |||
| import 'dart:typed_data'; | |||
| import 'package:dio/dio.dart'; | |||
| import 'package:http_parser/http_parser.dart'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that generated Flutter pubspec templates declare http_parser directly.
set -eu
printf 'Import sites:\n'
rg -n -C2 "package:http_parser/http_parser.dart" --iglob '*.hbs' --iglob '*.dart' || true
printf '\nDirect dependency declarations expected under dependencies:\n'
rg -n -C3 "^[[:space:]]*http_parser:" --iglob 'pubspec*.hbs' --iglob 'pubspec.yaml' || trueRepository: Arjun544/flutter_init
Length of output: 720
Add http_parser as a direct dependency in the pubspec template.
Line 3 imports package:http_parser/http_parser.dart, but http_parser is not declared as a direct dependency in the pubspec templates. Relying solely on Dio's transitive dependency violates the depend_on_referenced_packages lint rule and makes generated apps fragile if Dio changes its dependency graph. Add http_parser to the dependencies section of the pubspec template.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@templates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs`
at line 3, The template imports package:http_parser/http_parser.dart but
http_parser is not declared as a direct dependency in the Flutter pubspec
template; update the pubspec template's dependencies section to add the
http_parser package (e.g., add "http_parser: ^<latest-compatible-version>") so
generated apps don't rely on Dio's transitive dependency and satisfy the
depend_on_referenced_packages lint—make the change in the pubspec template used
by the project alongside the import in
templates/flutter/overlays/networking/dio/lib/src/services/upload_file_payload.dart.hbs.
1 participant
Adds multipart/form-data upload support to DioService for single and multiple files with optional progress tracking.
Highlights:
Summary by CodeRabbit