From 74fb0d9e0c8b1414b6876eb569d55f6d30ef7f86 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Wed, 5 Jul 2023 15:55:22 -0300 Subject: [PATCH 01/20] add restoration capability to multipart file --- dio/lib/src/multipart_file.dart | 36 ++++++++++++++++++- .../src/multipart_file/io_multipart_file.dart | 2 ++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 85fa3e6c0..9dc462bce 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -25,6 +25,12 @@ class MultipartFile { /// The content-type of the file. Defaults to `application/octet-stream`. final MediaType? contentType; + /// The path of the file on disk. May be null. + final String? _filePath; + + /// The bytes of the file. May be null. + final List? _valueBytes; + /// The stream that will emit the file's contents. final Stream> _stream; @@ -44,9 +50,13 @@ class MultipartFile { this.filename, MediaType? contentType, Map>? headers, + String? filePath, + List? value, }) : _stream = stream, headers = caseInsensitiveKeyMap(headers), - contentType = contentType ?? MediaType('application', 'octet-stream'); + contentType = contentType ?? MediaType('application', 'octet-stream'), + _filePath = filePath, + _valueBytes = value; /// Creates a new [MultipartFile] from a byte array. /// @@ -65,6 +75,7 @@ class MultipartFile { filename: filename, contentType: contentType, headers: headers, + value: value, ); } @@ -140,4 +151,27 @@ class MultipartFile { _isFinalized = true; return _stream; } + + /// Restore MultipartFile from path or value after its initial use. This is useful if your request failed and you wish + /// to sistematically retry it, for example a 401 error that can be solved by refreshing the token. + MultipartFile restoreMultipartFile() { + if (_filePath != null) { + return multipartFileFromPathSync( + _filePath!, + filename: filename, + contentType: contentType, + headers: headers, + ); + } else if (_valueBytes != null) { + return MultipartFile.fromBytes( + _valueBytes!, + filename: filename, + contentType: contentType, + headers: headers, + ); + } else { + throw ArgumentError( + 'Operation is not possible, file path or value is not available'); + } + } } diff --git a/dio/lib/src/multipart_file/io_multipart_file.dart b/dio/lib/src/multipart_file/io_multipart_file.dart index b4d79d79d..740066bac 100644 --- a/dio/lib/src/multipart_file/io_multipart_file.dart +++ b/dio/lib/src/multipart_file/io_multipart_file.dart @@ -22,6 +22,7 @@ Future multipartFileFromPath( filename: filename, contentType: contentType, headers: headers, + filePath: filePath, ); } @@ -41,5 +42,6 @@ MultipartFile multipartFileFromPathSync( filename: filename, contentType: contentType, headers: headers, + filePath: filePath, ); } From 8a49b5c23e9600e1bbb9bfc5010750a4038fd97c Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Wed, 5 Jul 2023 15:58:53 -0300 Subject: [PATCH 02/20] add test to ensure behavior --- dio/test/formdata_test.dart | 64 +++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dio/test/formdata_test.dart b/dio/test/formdata_test.dart index b1171ab4b..9a3440204 100644 --- a/dio/test/formdata_test.dart +++ b/dio/test/formdata_test.dart @@ -94,6 +94,70 @@ void main() async { testOn: 'vm', ); + // Restored multipart files should be able to read again and be the same as the original ones. + test( + 'complex with restoration', + () async { + final multipartFile1 = MultipartFile.fromString( + 'hello world.', + headers: { + 'test': ['a'] + }, + ); + final multipartFile2 = await MultipartFile.fromFile( + 'test/mock/_testfile', + filename: '1.txt', + headers: { + 'test': ['b'] + }, + ); + final multipartFile3 = MultipartFile.fromFileSync( + 'test/mock/_testfile', + filename: '2.txt', + headers: { + 'test': ['c'] + }, + ); + + final fm = FormData.fromMap({ + 'name': 'wendux', + 'age': 25, + 'path': '/图片空间/地址', + 'file': multipartFile1, + 'files': [ + multipartFile2, + multipartFile3, + ] + }); + final fmStr = await fm.readAsBytes(); + + final fm1 = FormData(); + fm1.fields.add(MapEntry('name', 'wendux')); + fm1.fields.add(MapEntry('age', '25')); + fm1.fields.add(MapEntry('path', '/图片空间/地址')); + fm1.files.add( + MapEntry( + 'file', + multipartFile1.restoreMultipartFile(), + ), + ); + fm1.files.add( + MapEntry( + 'files', + multipartFile2.restoreMultipartFile(), + ), + ); + fm1.files.add( + MapEntry( + 'files', + multipartFile3.restoreMultipartFile(), + ), + ); + expect(fmStr.length, fm1.length); + }, + testOn: 'vm', + ); + test('encodes maps correctly', () async { final fd = FormData.fromMap( { From 9fa83af292a50cffc39d8769cbebfa5472ffccdc Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Wed, 5 Jul 2023 16:17:46 -0300 Subject: [PATCH 03/20] add CHANGELOG entry --- dio/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index f245beca8..5e621cdad 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -6,6 +6,7 @@ See the [Migration Guide][] for the complete breaking changes list.** ## Unreleased - Remove `http` from `dev_dependencies`. +- Add support for restoring MultipartFile from `FormData`. ## 5.2.1+1 @@ -28,7 +29,7 @@ See the [Migration Guide][] for the complete breaking changes list.** Dio 6.0.0 - Please use the replacement `IOHttpClientAdapter.createHttpClient` instead. - Using `CancelToken` no longer closes and re-creates `HttpClient` for each request when `IOHttpClientAdapter` is used. - Fix timeout handling for browser `receiveTimeout`. -- Improve performance when sending binary data (`List`/`Uint8List`). +- Improve performance when sending binary data (`List`/`Uint8List`). ## 5.1.2 From 9bb916830710bdfcf7d57cd88693bee08f959fd9 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Fri, 7 Jul 2023 13:35:50 -0300 Subject: [PATCH 04/20] apply PR suggestion: Remove extra fields and optimize restore method --- dio/lib/src/multipart_file.dart | 34 +++---------------- .../src/multipart_file/io_multipart_file.dart | 2 -- 2 files changed, 5 insertions(+), 31 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 9dc462bce..a8c3d6107 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -25,12 +25,6 @@ class MultipartFile { /// The content-type of the file. Defaults to `application/octet-stream`. final MediaType? contentType; - /// The path of the file on disk. May be null. - final String? _filePath; - - /// The bytes of the file. May be null. - final List? _valueBytes; - /// The stream that will emit the file's contents. final Stream> _stream; @@ -50,13 +44,9 @@ class MultipartFile { this.filename, MediaType? contentType, Map>? headers, - String? filePath, - List? value, }) : _stream = stream, headers = caseInsensitiveKeyMap(headers), - contentType = contentType ?? MediaType('application', 'octet-stream'), - _filePath = filePath, - _valueBytes = value; + contentType = contentType ?? MediaType('application', 'octet-stream'); /// Creates a new [MultipartFile] from a byte array. /// @@ -75,7 +65,6 @@ class MultipartFile { filename: filename, contentType: contentType, headers: headers, - value: value, ); } @@ -152,26 +141,13 @@ class MultipartFile { return _stream; } - /// Restore MultipartFile from path or value after its initial use. This is useful if your request failed and you wish + /// Restore MultipartFile, returning a new instance of the same object. This is useful if your request failed and you wish /// to sistematically retry it, for example a 401 error that can be solved by refreshing the token. - MultipartFile restoreMultipartFile() { - if (_filePath != null) { - return multipartFileFromPathSync( - _filePath!, - filename: filename, - contentType: contentType, - headers: headers, - ); - } else if (_valueBytes != null) { - return MultipartFile.fromBytes( - _valueBytes!, + MultipartFile restoreMultipartFile() => MultipartFile( + _stream, + length, filename: filename, contentType: contentType, headers: headers, ); - } else { - throw ArgumentError( - 'Operation is not possible, file path or value is not available'); - } - } } diff --git a/dio/lib/src/multipart_file/io_multipart_file.dart b/dio/lib/src/multipart_file/io_multipart_file.dart index 740066bac..b4d79d79d 100644 --- a/dio/lib/src/multipart_file/io_multipart_file.dart +++ b/dio/lib/src/multipart_file/io_multipart_file.dart @@ -22,7 +22,6 @@ Future multipartFileFromPath( filename: filename, contentType: contentType, headers: headers, - filePath: filePath, ); } @@ -42,6 +41,5 @@ MultipartFile multipartFileFromPathSync( filename: filename, contentType: contentType, headers: headers, - filePath: filePath, ); } From d6e736b6656da6fd32e8f9b6d6880ddce1dec973 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Fri, 7 Jul 2023 13:42:10 -0300 Subject: [PATCH 05/20] update test to guarantee multipart file is not finalized --- dio/test/formdata_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dio/test/formdata_test.dart b/dio/test/formdata_test.dart index 9a3440204..5334e7d84 100644 --- a/dio/test/formdata_test.dart +++ b/dio/test/formdata_test.dart @@ -154,6 +154,9 @@ void main() async { ), ); expect(fmStr.length, fm1.length); + expect(fm1.files[0].value.isFinalized, false); + expect(fm1.files[1].value.isFinalized, false); + expect(fm1.files[2].value.isFinalized, false); }, testOn: 'vm', ); From a22208d0d7ae0542ebb4bb0b47db06f0091cc5a8 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 10 Jul 2023 08:05:53 -0300 Subject: [PATCH 06/20] Rename method, apply line length rule and update test --- dio/lib/src/multipart_file.dart | 7 ++++--- dio/test/formdata_test.dart | 34 +++++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index a8c3d6107..f1c3ec347 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -141,9 +141,10 @@ class MultipartFile { return _stream; } - /// Restore MultipartFile, returning a new instance of the same object. This is useful if your request failed and you wish - /// to sistematically retry it, for example a 401 error that can be solved by refreshing the token. - MultipartFile restoreMultipartFile() => MultipartFile( + /// Restore 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 duplicateMultipartFile() => MultipartFile( _stream, length, filename: filename, diff --git a/dio/test/formdata_test.dart b/dio/test/formdata_test.dart index 5334e7d84..2130b6951 100644 --- a/dio/test/formdata_test.dart +++ b/dio/test/formdata_test.dart @@ -94,7 +94,8 @@ void main() async { testOn: 'vm', ); - // Restored multipart files should be able to read again and be the same as the original ones. + // Restored multipart files should be able to be read again and be the same + // as the original ones. test( 'complex with restoration', () async { @@ -131,6 +132,19 @@ void main() async { }); final fmStr = await fm.readAsBytes(); + // Files are finalized after being read. + try { + multipartFile1.finalize(); + fail('Should not be able to finalize a file twice.'); + } catch (e) { + expect(e, isA()); + expect( + (e as StateError).message, + 'The MultipartFile has already been finalized. This typically ' + 'means you are using the same MultipartFile in repeated requests.', + ); + } + final fm1 = FormData(); fm1.fields.add(MapEntry('name', 'wendux')); fm1.fields.add(MapEntry('age', '25')); @@ -138,25 +152,37 @@ void main() async { fm1.files.add( MapEntry( 'file', - multipartFile1.restoreMultipartFile(), + multipartFile1.duplicateMultipartFile(), ), ); fm1.files.add( MapEntry( 'files', - multipartFile2.restoreMultipartFile(), + multipartFile2.duplicateMultipartFile(), ), ); fm1.files.add( MapEntry( 'files', - multipartFile3.restoreMultipartFile(), + multipartFile3.duplicateMultipartFile(), ), ); expect(fmStr.length, fm1.length); + + // The restored multipart files should be able to be read again. + expect(fm.files[0].value.isFinalized, true); + expect(fm.files[1].value.isFinalized, true); + expect(fm.files[2].value.isFinalized, true); expect(fm1.files[0].value.isFinalized, false); expect(fm1.files[1].value.isFinalized, false); expect(fm1.files[2].value.isFinalized, false); + + // The restored multipart files' properties should be the same as the + // original ones. + expect(fm1.files[0].value.filename, multipartFile1.filename); + expect(fm1.files[0].value.contentType, multipartFile1.contentType); + expect(fm1.files[0].value.length, multipartFile1.length); + expect(fm1.files[0].value.headers, multipartFile1.headers); }, testOn: 'vm', ); From 601f09e015161177cee3cbb12e1171a96344a002 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Wed, 12 Jul 2023 07:32:03 -0300 Subject: [PATCH 07/20] rename method as clone --- dio/lib/src/multipart_file.dart | 4 ++-- dio/test/formdata_test.dart | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index f1c3ec347..6bcb42f26 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -141,10 +141,10 @@ class MultipartFile { return _stream; } - /// Restore MultipartFile, returning a new instance of the same object. + /// 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 duplicateMultipartFile() => MultipartFile( + MultipartFile clone() => MultipartFile( _stream, length, filename: filename, diff --git a/dio/test/formdata_test.dart b/dio/test/formdata_test.dart index 2130b6951..c1ae7b216 100644 --- a/dio/test/formdata_test.dart +++ b/dio/test/formdata_test.dart @@ -94,10 +94,10 @@ void main() async { testOn: 'vm', ); - // Restored multipart files should be able to be read again and be the same + // Cloned multipart files should be able to be read again and be the same // as the original ones. test( - 'complex with restoration', + 'complex with cloning', () async { final multipartFile1 = MultipartFile.fromString( 'hello world.', @@ -152,24 +152,24 @@ void main() async { fm1.files.add( MapEntry( 'file', - multipartFile1.duplicateMultipartFile(), + multipartFile1.clone(), ), ); fm1.files.add( MapEntry( 'files', - multipartFile2.duplicateMultipartFile(), + multipartFile2.clone(), ), ); fm1.files.add( MapEntry( 'files', - multipartFile3.duplicateMultipartFile(), + multipartFile3.clone(), ), ); expect(fmStr.length, fm1.length); - // The restored multipart files should be able to be read again. + // The cloned multipart files should be able to be read again. expect(fm.files[0].value.isFinalized, true); expect(fm.files[1].value.isFinalized, true); expect(fm.files[2].value.isFinalized, true); @@ -177,7 +177,7 @@ void main() async { expect(fm1.files[1].value.isFinalized, false); expect(fm1.files[2].value.isFinalized, false); - // The restored multipart files' properties should be the same as the + // The cloned multipart files' properties should be the same as the // original ones. expect(fm1.files[0].value.filename, multipartFile1.filename); expect(fm1.files[0].value.contentType, multipartFile1.contentType); From dfc81e20f1a3a14e414d8e5addc475948742610a Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Wed, 12 Jul 2023 12:54:56 +0200 Subject: [PATCH 08/20] Update dio/CHANGELOG.md Signed-off-by: Peter Leibiger --- dio/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index cf5ef6e96..ceb2726d9 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -6,7 +6,7 @@ See the [Migration Guide][] for the complete breaking changes list.** ## Unreleased - Remove `http` from `dev_dependencies`. -- Add support for restoring MultipartFile from `FormData`. +- Add support for cloning `MultipartFile` from `FormData`. - Only produce null response body when `ResponseType.json`. ## 5.2.1+1 From f76480a7265d1332a47935cf3d07ed491bd16d03 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 08:50:03 -0300 Subject: [PATCH 09/20] [FIX] Fix FileSystemException: File closed when cloning MultipartFile --- dio/lib/src/multipart_file.dart | 32 ++++++++++++++++--- .../src/multipart_file/io_multipart_file.dart | 2 ++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 6bcb42f26..e4e6659d0 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -28,6 +28,12 @@ class MultipartFile { /// The stream that will emit the file's contents. final Stream> _stream; + /// The path of the file on disk. May be null. + final String? _filePath; + + /// The bytes of the file. May be null. + final List? _valueBytes; + /// Whether [finalize] has been called. bool get isFinalized => _isFinalized; bool _isFinalized = false; @@ -44,9 +50,13 @@ class MultipartFile { this.filename, MediaType? contentType, Map>? headers, + String? filePath, + List? value, }) : _stream = stream, headers = caseInsensitiveKeyMap(headers), - contentType = contentType ?? MediaType('application', 'octet-stream'); + contentType = contentType ?? MediaType('application', 'octet-stream'), + _filePath = filePath, + _valueBytes = value; /// Creates a new [MultipartFile] from a byte array. /// @@ -65,6 +75,7 @@ class MultipartFile { filename: filename, contentType: contentType, headers: headers, + value: value, ); } @@ -144,11 +155,24 @@ class MultipartFile { /// 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, + MultipartFile clone() { + if (_filePath != null) { + return multipartFileFromPathSync( + _filePath!, + filename: filename, + contentType: contentType, + headers: headers, + ); + } else if (_valueBytes != null) { + return MultipartFile.fromBytes( + _valueBytes!, filename: filename, contentType: contentType, headers: headers, ); + } else { + throw ArgumentError( + 'Operation is not possible, file path or value is not available'); + } + } } diff --git a/dio/lib/src/multipart_file/io_multipart_file.dart b/dio/lib/src/multipart_file/io_multipart_file.dart index b4d79d79d..740066bac 100644 --- a/dio/lib/src/multipart_file/io_multipart_file.dart +++ b/dio/lib/src/multipart_file/io_multipart_file.dart @@ -22,6 +22,7 @@ Future multipartFileFromPath( filename: filename, contentType: contentType, headers: headers, + filePath: filePath, ); } @@ -41,5 +42,6 @@ MultipartFile multipartFileFromPathSync( filename: filename, contentType: contentType, headers: headers, + filePath: filePath, ); } From da66169e4c62500143f0d3a01e6fbee9e4dcc071 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 08:56:59 -0300 Subject: [PATCH 10/20] add changelog entry --- dio/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index 849037eaf..1b22b1d39 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -6,6 +6,7 @@ See the [Migration Guide][] for the complete breaking changes list.** ## Unreleased - Improve comments. +- Fix error when cloning `MultipartFile` from `FormData`. ## 5.3.0 From 40001daa06156fad854b9d3ca521de6e7b2458d5 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 11:39:40 -0300 Subject: [PATCH 11/20] update clone method to use a StreamBuilder --- dio/lib/src/multipart_file.dart | 46 ++++++------------- .../src/multipart_file/io_multipart_file.dart | 13 +++--- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index e4e6659d0..424847a58 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -26,13 +26,11 @@ class MultipartFile { final MediaType? contentType; /// The stream that will emit the file's contents. + @Deprecated('Use [data] instead.') final Stream> _stream; - /// The path of the file on disk. May be null. - final String? _filePath; - - /// The bytes of the file. May be null. - final List? _valueBytes; + // The stream builder that will emit the file's contents for every call. + final Stream> Function() _data; /// Whether [finalize] has been called. bool get isFinalized => _isFinalized; @@ -45,18 +43,15 @@ class MultipartFile { /// [contentType] currently defaults to `application/octet-stream`, but in the /// future may be inferred from [filename]. MultipartFile( - Stream> stream, + Stream> Function() data, this.length, { this.filename, MediaType? contentType, Map>? headers, - String? filePath, - List? value, - }) : _stream = stream, + }) : _data = data, headers = caseInsensitiveKeyMap(headers), contentType = contentType ?? MediaType('application', 'octet-stream'), - _filePath = filePath, - _valueBytes = value; + _stream = data.call(); /// Creates a new [MultipartFile] from a byte array. /// @@ -68,14 +63,12 @@ class MultipartFile { MediaType? contentType, final Map>? headers, }) { - final stream = Stream.fromIterable([value]); return MultipartFile( - stream, + () => Stream.fromIterable([value]), value.length, filename: filename, contentType: contentType, headers: headers, - value: value, ); } @@ -156,23 +149,12 @@ class MultipartFile { /// 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() { - if (_filePath != null) { - return multipartFileFromPathSync( - _filePath!, - filename: filename, - contentType: contentType, - headers: headers, - ); - } else if (_valueBytes != null) { - return MultipartFile.fromBytes( - _valueBytes!, - filename: filename, - contentType: contentType, - headers: headers, - ); - } else { - throw ArgumentError( - 'Operation is not possible, file path or value is not available'); - } + return MultipartFile( + _data, + length, + filename: filename, + contentType: contentType, + headers: headers, + ); } } diff --git a/dio/lib/src/multipart_file/io_multipart_file.dart b/dio/lib/src/multipart_file/io_multipart_file.dart index 740066bac..6014d0f78 100644 --- a/dio/lib/src/multipart_file/io_multipart_file.dart +++ b/dio/lib/src/multipart_file/io_multipart_file.dart @@ -15,14 +15,12 @@ Future multipartFileFromPath( filename ??= p.basename(filePath); final file = File(filePath); final length = await file.length(); - final stream = file.openRead(); return MultipartFile( - stream, + () => _getStreamFromFilepath(file), length, filename: filename, contentType: contentType, headers: headers, - filePath: filePath, ); } @@ -35,13 +33,16 @@ MultipartFile multipartFileFromPathSync( filename ??= p.basename(filePath); final file = File(filePath); final length = file.lengthSync(); - final stream = file.openRead(); return MultipartFile( - stream, + () => _getStreamFromFilepath(file), length, filename: filename, contentType: contentType, headers: headers, - filePath: filePath, ); } + +Stream> _getStreamFromFilepath(File file) { + final stream = file.openRead(); + return stream; +} From 578ec7863cb7feb012525feaa5b2b43b2d83ba18 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 11:39:59 -0300 Subject: [PATCH 12/20] add convenience method for cloning formdata and regression test for File error --- dio/lib/src/form_data.dart | 10 +++++++ dio/test/formdata_test.dart | 56 ++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/dio/lib/src/form_data.dart b/dio/lib/src/form_data.dart index 512024643..d0e851561 100644 --- a/dio/lib/src/form_data.dart +++ b/dio/lib/src/form_data.dart @@ -176,4 +176,14 @@ class FormData { Future> 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; + } } diff --git a/dio/test/formdata_test.dart b/dio/test/formdata_test.dart index c1ae7b216..2403c8b9a 100644 --- a/dio/test/formdata_test.dart +++ b/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': ['a'] + }, + ), + 'files': [ + await MultipartFile.fromFile( + 'test/mock/_testfile', + filename: '1.txt', + headers: { + 'test': ['b'] + }, + ), + MultipartFile.fromFileSync( + 'test/mock/_testfile', + filename: '2.txt', + headers: { + 'test': ['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())); + + 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.', From 0e7ab5422eaf85e82aab855c96eaea8d37152ede Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 11:40:05 -0300 Subject: [PATCH 13/20] update changelog --- dio/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index 1b22b1d39..3f69505f8 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -6,7 +6,9 @@ See the [Migration Guide][] for the complete breaking changes list.** ## Unreleased - Improve comments. -- Fix error when cloning `MultipartFile` from `FormData`. +- Fix error when cloning `MultipartFile` from `FormData` with regression test. +- Add convenience method for cloning `FormData` as a whole +- Deprecate `stream` parameter in favor of `data` in `MultipartFile`. ## 5.3.0 From 3499783a411d26ee0be7f9b29550fd46e41cef91 Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 14:21:18 -0300 Subject: [PATCH 14/20] move fields after constructor and add deprecation to constructor --- dio/lib/src/multipart_file.dart | 38 +++++++++++++++++---------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 424847a58..408df6fa6 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -12,6 +12,26 @@ import 'multipart_file/io_multipart_file.dart' /// MultipartFile is based on stream, and a stream can be read only once, /// so the same MultipartFile can't be read multiple times. class MultipartFile { + /// 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]. + @Deprecated( + 'Clone will not work with this constructor and it will be removed in 6.0.0', + ) + MultipartFile( + Stream> Function() data, + this.length, { + this.filename, + MediaType? contentType, + Map>? headers, + }) : _data = data, + headers = caseInsensitiveKeyMap(headers), + contentType = contentType ?? MediaType('application', 'octet-stream'), + _stream = data.call(); + /// The size of the file in bytes. This must be known in advance, even if this /// file is created from a [ByteStream]. final int length; @@ -26,7 +46,6 @@ class MultipartFile { final MediaType? contentType; /// The stream that will emit the file's contents. - @Deprecated('Use [data] instead.') final Stream> _stream; // The stream builder that will emit the file's contents for every call. @@ -36,23 +55,6 @@ class MultipartFile { bool get isFinalized => _isFinalized; bool _isFinalized = false; - /// 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( - Stream> Function() data, - this.length, { - this.filename, - MediaType? contentType, - Map>? headers, - }) : _data = data, - headers = caseInsensitiveKeyMap(headers), - contentType = contentType ?? MediaType('application', 'octet-stream'), - _stream = data.call(); - /// Creates a new [MultipartFile] from a byte array. /// /// [contentType] currently defaults to `application/octet-stream`, but in the From a8563c4e67d6a1dbee6939a9e8ec345f5d4ae72b Mon Sep 17 00:00:00 2001 From: gabrielaraujoz Date: Mon, 24 Jul 2023 14:22:14 -0300 Subject: [PATCH 15/20] remove _stream in favor of data --- dio/lib/src/multipart_file.dart | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 408df6fa6..08bf153d5 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -29,8 +29,7 @@ class MultipartFile { Map>? headers, }) : _data = data, headers = caseInsensitiveKeyMap(headers), - contentType = contentType ?? MediaType('application', 'octet-stream'), - _stream = data.call(); + contentType = contentType ?? MediaType('application', 'octet-stream'); /// The size of the file in bytes. This must be known in advance, even if this /// file is created from a [ByteStream]. @@ -45,9 +44,6 @@ 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> _stream; - // The stream builder that will emit the file's contents for every call. final Stream> Function() _data; @@ -144,7 +140,7 @@ class MultipartFile { ); } _isFinalized = true; - return _stream; + return _data.call(); } /// Clone MultipartFile, returning a new instance of the same object. From e7e7b9df30810ebdbb449001baf122fd3307773b Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Mon, 24 Jul 2023 20:13:08 +0200 Subject: [PATCH 16/20] Review/PR adjustments --- dio/CHANGELOG.md | 2 +- dio/lib/src/multipart_file.dart | 25 ++++++++++++++++--- .../src/multipart_file/io_multipart_file.dart | 4 +-- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index 3f69505f8..7e0cbdbab 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -7,8 +7,8 @@ See the [Migration Guide][] for the complete breaking changes list.** - Improve comments. - Fix error when cloning `MultipartFile` from `FormData` with regression test. +- Deprecate `MulitpartFile` constructor in favor `MultipartFile.fromStream`. - Add convenience method for cloning `FormData` as a whole -- Deprecate `stream` parameter in favor of `data` in `MultipartFile`. ## 5.3.0 diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 08bf153d5..c5e142a1d 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -19,9 +19,26 @@ class MultipartFile { /// [contentType] currently defaults to `application/octet-stream`, but in the /// future may be inferred from [filename]. @Deprecated( - 'Clone will not work with this constructor and it will be removed in 6.0.0', + '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>? stream, + this.length, { + this.filename, + MediaType? contentType, + Map>? headers, + }) : _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> Function() data, this.length, { this.filename, @@ -44,7 +61,7 @@ class MultipartFile { /// The content-type of the file. Defaults to `application/octet-stream`. final MediaType? contentType; - // The stream builder that will emit the file's contents for every call. + /// The stream builder that will emit the file's contents for every call. final Stream> Function() _data; /// Whether [finalize] has been called. @@ -61,7 +78,7 @@ class MultipartFile { MediaType? contentType, final Map>? headers, }) { - return MultipartFile( + return MultipartFile.fromStream( () => Stream.fromIterable([value]), value.length, filename: filename, @@ -147,7 +164,7 @@ class MultipartFile { /// 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() { - return MultipartFile( + return MultipartFile.fromStream( _data, length, filename: filename, diff --git a/dio/lib/src/multipart_file/io_multipart_file.dart b/dio/lib/src/multipart_file/io_multipart_file.dart index 6014d0f78..320acfdc4 100644 --- a/dio/lib/src/multipart_file/io_multipart_file.dart +++ b/dio/lib/src/multipart_file/io_multipart_file.dart @@ -15,7 +15,7 @@ Future multipartFileFromPath( filename ??= p.basename(filePath); final file = File(filePath); final length = await file.length(); - return MultipartFile( + return MultipartFile.fromStream( () => _getStreamFromFilepath(file), length, filename: filename, @@ -33,7 +33,7 @@ MultipartFile multipartFileFromPathSync( filename ??= p.basename(filePath); final file = File(filePath); final length = file.lengthSync(); - return MultipartFile( + return MultipartFile.fromStream( () => _getStreamFromFilepath(file), length, filename: filename, From b79474572cd60e993eddbfaf65a4f3044a7e6920 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Mon, 24 Jul 2023 23:20:40 +0200 Subject: [PATCH 17/20] Restore formatting --- dio/lib/src/multipart_file.dart | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index c5e142a1d..1441bd9d8 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -48,26 +48,6 @@ class MultipartFile { headers = caseInsensitiveKeyMap(headers), contentType = contentType ?? MediaType('application', 'octet-stream'); - /// The size of the file in bytes. This must be known in advance, even if this - /// file is created from a [ByteStream]. - final int length; - - /// The basename of the file. May be null. - final String? filename; - - /// The additional headers the file has. May be null. - final Map>? headers; - - /// The content-type of the file. Defaults to `application/octet-stream`. - final MediaType? contentType; - - /// The stream builder that will emit the file's contents for every call. - final Stream> Function() _data; - - /// Whether [finalize] has been called. - bool get isFinalized => _isFinalized; - bool _isFinalized = false; - /// Creates a new [MultipartFile] from a byte array. /// /// [contentType] currently defaults to `application/octet-stream`, but in the @@ -114,6 +94,25 @@ class MultipartFile { ); } + /// The size of the file in bytes. This must be known in advance, even if this + /// file is created from a [ByteStream]. + final int length; + + /// The basename of the file. May be null. + final String? filename; + + /// The additional headers the file has. May be null. + final Map>? headers; + + /// The content-type of the file. Defaults to `application/octet-stream`. + final MediaType? contentType; + + /// The stream builder that will emit the file's contents for every call. + final Stream> Function() _data; + + /// Whether [finalize] has been called. + bool get isFinalized => _isFinalized; + /// Creates a new [MultipartFile] from a path to a file on disk. /// /// [filename] defaults to the basename of [filePath]. [contentType] currently @@ -147,6 +146,7 @@ class MultipartFile { contentType: contentType, headers: headers, ); + bool _isFinalized = false; Stream> finalize() { if (isFinalized) { From 67fe9e6627d182a988a69ea031be1c5caa43a05c Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Tue, 25 Jul 2023 01:27:07 +0200 Subject: [PATCH 18/20] Fix refactoring mistake --- dio/lib/src/multipart_file.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 1441bd9d8..650deae77 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -23,12 +23,12 @@ class MultipartFile { 'This will be removed in 6.0.0', ) MultipartFile( - Stream>? stream, + Stream> stream, this.length, { this.filename, MediaType? contentType, Map>? headers, - }) : _data = (() => stream!), + }) : _data = (() => stream), headers = caseInsensitiveKeyMap(headers), contentType = contentType ?? MediaType('application', 'octet-stream'); From 0d329ca2c209dc0885d649b65d504ed4ddabe83d Mon Sep 17 00:00:00 2001 From: Alex Li Date: Tue, 25 Jul 2023 17:18:14 +0800 Subject: [PATCH 19/20] Update dio/lib/src/multipart_file.dart Signed-off-by: Alex Li --- dio/lib/src/multipart_file.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/dio/lib/src/multipart_file.dart b/dio/lib/src/multipart_file.dart index 650deae77..d7d70acc4 100644 --- a/dio/lib/src/multipart_file.dart +++ b/dio/lib/src/multipart_file.dart @@ -146,6 +146,7 @@ class MultipartFile { contentType: contentType, headers: headers, ); + bool _isFinalized = false; Stream> finalize() { From 528c5d69effdcc6293c451f44b96310cfff70682 Mon Sep 17 00:00:00 2001 From: Alex Li Date: Tue, 25 Jul 2023 17:27:23 +0800 Subject: [PATCH 20/20] Update dio/CHANGELOG.md Signed-off-by: Alex Li --- dio/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index 7e0cbdbab..dac2b6daf 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -8,7 +8,7 @@ See the [Migration Guide][] for the complete breaking changes list.** - Improve comments. - Fix error when cloning `MultipartFile` from `FormData` with regression test. - Deprecate `MulitpartFile` constructor in favor `MultipartFile.fromStream`. -- Add convenience method for cloning `FormData` as a whole +- Add `FormData.clone`. ## 5.3.0