Fix cloning MultipartFile (cfug#1903)

- Fixes cfug#902
- Adds `FormData.clone()`

Signed-off-by: Gabriel Araujo <[email protected]>
Signed-off-by: Peter Leibiger <[email protected]>
Signed-off-by: Alex Li <[email protected]>
Co-authored-by: Peter Leibiger <[email protected]>
Co-authored-by: Alex Li <[email protected]>

dio/CHANGELOG.md

@@ -6,6 +6,9 @@ See the [Migration Guide][] for the complete breaking changes list.**
 ## Unreleased
 
 - Improve comments.
+- Fix error when cloning `MultipartFile` from `FormData` with regression test.
+- Deprecate `MulitpartFile` constructor in favor `MultipartFile.fromStream`.
+- Add `FormData.clone`.
 
 ## 5.3.0
 

dio/lib/src/form_data.dart

@@ -176,4 +176,14 @@ class FormData {
   Future<List<int>> readAsBytes() {
     return Future(() => finalize().reduce((a, b) => [...a, ...b]));
   }
+
+  // Convenience method to clone finalized FormData when retrying requests.
+  FormData clone() {
+    final clone = FormData();
+    clone.fields.addAll(fields);
+    for (final file in files) {
+      clone.files.add(MapEntry(file.key, file.value.clone()));
+    }
+    return clone;
+  }
 }

dio/lib/src/multipart_file.dart

@@ -18,13 +18,33 @@ class MultipartFile {
   ///
   /// [contentType] currently defaults to `application/octet-stream`, but in the
   /// future may be inferred from [filename].
+  @Deprecated(
+    'MultipartFile.clone() will not work when the stream is provided, use the MultipartFile.fromStream instead.'
+    'This will be removed in 6.0.0',
+  )
   MultipartFile(
     Stream<List<int>> stream,
     this.length, {
     this.filename,
     MediaType? contentType,
     Map<String, List<String>>? headers,
-  })  : _stream = stream,
+  })  : _data = (() => stream),
+        headers = caseInsensitiveKeyMap(headers),
+        contentType = contentType ?? MediaType('application', 'octet-stream');
+
+  /// Creates a new [MultipartFile] from a chunked [Stream] of bytes. The length
+  /// of the file in bytes must be known in advance. If it's not, read the data
+  /// from the stream and use [MultipartFile.fromBytes] instead.
+  ///
+  /// [contentType] currently defaults to `application/octet-stream`, but in the
+  /// future may be inferred from [filename].
+  MultipartFile.fromStream(
+    Stream<List<int>> Function() data,
+    this.length, {
+    this.filename,
+    MediaType? contentType,
+    Map<String, List<String>>? headers,
+  })  : _data = data,
         headers = caseInsensitiveKeyMap(headers),
         contentType = contentType ?? MediaType('application', 'octet-stream');
 
@@ -38,9 +58,8 @@ class MultipartFile {
     MediaType? contentType,
     final Map<String, List<String>>? headers,
   }) {
-    final stream = Stream.fromIterable([value]);
-    return MultipartFile(
-      stream,
+    return MultipartFile.fromStream(
+      () => Stream.fromIterable([value]),
       value.length,
       filename: filename,
       contentType: contentType,
@@ -88,12 +107,11 @@ class MultipartFile {
   /// The content-type of the file. Defaults to `application/octet-stream`.
   final MediaType? contentType;
 
-  /// The stream that will emit the file's contents.
-  final Stream<List<int>> _stream;
+  /// The stream builder that will emit the file's contents for every call.
+  final Stream<List<int>> Function() _data;
 
   /// Whether [finalize] has been called.
   bool get isFinalized => _isFinalized;
-  bool _isFinalized = false;
 
   /// Creates a new [MultipartFile] from a path to a file on disk.
   ///
@@ -129,6 +147,8 @@ class MultipartFile {
         headers: headers,
       );
 
+  bool _isFinalized = false;
+
   Stream<List<int>> finalize() {
     if (isFinalized) {
       throw StateError(
@@ -138,17 +158,19 @@ class MultipartFile {
       );
     }
     _isFinalized = true;
-    return _stream;
+    return _data.call();
   }
 
   /// Clone MultipartFile, returning a new instance of the same object.
   /// This is useful if your request failed and you wish to retry it,
   /// such as an unauthorized exception can be solved by refreshing the token.
-  MultipartFile clone() => MultipartFile(
-        _stream,
-        length,
-        filename: filename,
-        contentType: contentType,
-        headers: headers,
-      );
+  MultipartFile clone() {
+    return MultipartFile.fromStream(
+      _data,
+      length,
+      filename: filename,
+      contentType: contentType,
+      headers: headers,
+    );
+  }
 }

dio/lib/src/multipart_file/io_multipart_file.dart

@@ -15,9 +15,8 @@ Future<MultipartFile> multipartFileFromPath(
   filename ??= p.basename(filePath);
   final file = File(filePath);
   final length = await file.length();
-  final stream = file.openRead();
-  return MultipartFile(
-    stream,
+  return MultipartFile.fromStream(
+    () => _getStreamFromFilepath(file),
     length,
     filename: filename,
     contentType: contentType,
@@ -34,12 +33,16 @@ MultipartFile multipartFileFromPathSync(
   filename ??= p.basename(filePath);
   final file = File(filePath);
   final length = file.lengthSync();
-  final stream = file.openRead();
-  return MultipartFile(
-    stream,
+  return MultipartFile.fromStream(
+    () => _getStreamFromFilepath(file),
     length,
     filename: filename,
     contentType: contentType,
     headers: headers,
   );
 }
+
+Stream<List<int>> _getStreamFromFilepath(File file) {
+  final stream = file.openRead();
+  return stream;
+}

dio/test/formdata_test.dart

@@ -94,10 +94,64 @@ void main() async {
       testOn: 'vm',
     );
 
+    test(
+      'complex cloning FormData object',
+      () async {
+        final fm = FormData.fromMap({
+          'name': 'wendux',
+          'age': 25,
+          'path': '/图片空间/地址',
+          'file': MultipartFile.fromString(
+            'hello world.',
+            headers: {
+              'test': <String>['a']
+            },
+          ),
+          'files': [
+            await MultipartFile.fromFile(
+              'test/mock/_testfile',
+              filename: '1.txt',
+              headers: {
+                'test': <String>['b']
+              },
+            ),
+            MultipartFile.fromFileSync(
+              'test/mock/_testfile',
+              filename: '2.txt',
+              headers: {
+                'test': <String>['c']
+              },
+            ),
+          ]
+        });
+        final fmStr = await fm.readAsBytes();
+        final f = File('test/mock/_formdata');
+        String content = f.readAsStringSync();
+        content = content.replaceAll('--dio-boundary-3788753558', fm.boundary);
+        String actual = utf8.decode(fmStr, allowMalformed: true);
+
+        actual = actual.replaceAll('\r\n', '\n');
+        content = content.replaceAll('\r\n', '\n');
+
+        expect(actual, content);
+        expect(fm.readAsBytes(), throwsA(const TypeMatcher<StateError>()));
+
+        final fm1 = fm.clone();
+        expect(fm1.isFinalized, false);
+        final fm1Str = await fm1.readAsBytes();
+        expect(fmStr.length, fm1Str.length);
+        expect(fm1.isFinalized, true);
+        expect(fm1 != fm, true);
+        expect(fm1.files[0].value.filename, fm.files[0].value.filename);
+        expect(fm1.fields, fm.fields);
+      },
+      testOn: 'vm',
+    );
+
     // Cloned multipart files should be able to be read again and be the same
     // as the original ones.
     test(
-      'complex with cloning',
+      'complex cloning MultipartFile',
       () async {
         final multipartFile1 = MultipartFile.fromString(
           'hello world.',