From 88a0fda52b804a7a414809c05fb1cf76a3b6385f Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Thu, 28 Dec 2023 14:07:47 +0100 Subject: [PATCH] [dio] Test typed responses (#1755) --- dio/CHANGELOG.md | 6 +++-- dio/doc/migration_guide.md | 23 +++++++++++++++++++ dio/lib/src/dio/dio_for_native.dart | 2 +- dio/lib/src/dio_mixin.dart | 3 ++- dio/lib/src/response.dart | 4 ++-- dio/test/options_test.dart | 4 ++-- dio/test/request_integration_test.dart | 2 +- dio/test/timeout_test.dart | 14 ++++++----- dio/test/utils.dart | 19 +++++++++++++++ example/lib/generic.dart | 2 +- example_flutter_app/lib/main.dart | 2 +- example_flutter_app/lib/routes/request.dart | 2 +- plugins/cookie_manager/test/cookies_test.dart | 2 ++ .../native_dio_adapter/example/lib/main.dart | 4 ++-- 14 files changed, 69 insertions(+), 20 deletions(-) diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index d3651a8d0..2d302bd1f 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -7,12 +7,14 @@ See the [Migration Guide][] for the complete breaking changes list.** - The minimum supported Dart version has been bumped from `2.15.0` to `2.19.0`. - Remove `DefaultHttpClientAdapter` which was deprecated in `5.0.0`. -- Remove `IOHttpClientAdapter.onHttpClientCreate` which was deprecated in `5.2.0` +- Remove `IOHttpClientAdapter.onHttpClientCreate` which was deprecated in `5.2.0`. - Remove `DioError` and `DioErrorType` which was deprecated in `5.2.0`. - Remove `DefaultTransformer` which was deprecated in `5.0.0`. - `IOHttpClientAdapter` no longer sets a custom `HttpClient.idleTimeout`. A custom `HttpClient` can be provided via `IOHttpClientAdapter.createHttpClient` if customisation is required. -- Make use of Isolate.run for the BackgroundTransformer +- Make use of `Isolate.run` in the BackgroundTransformer. +- Respect nullability of generic type parameter in `dio`s request methods like `get()`. + Follow the [Migration Guide][] for more details. ## Unreleased diff --git a/dio/doc/migration_guide.md b/dio/doc/migration_guide.md index d9c8ce98a..5f6396206 100644 --- a/dio/doc/migration_guide.md +++ b/dio/doc/migration_guide.md @@ -23,6 +23,29 @@ When new content need to be added to the migration guide, make sure they're foll - `IOHttpClientAdapter.onHttpClientCreate` which was deprecated in `5.2.0` has been removed - use `IOHttpClientAdapter.createHttpClient` instead. - `DioError` and `DioErrorType` which was deprecated in `5.2.0` has been removed - use `DioException` and `DioExceptionType` instead. - `DefaultTransformer` which was deprecated in `5.0.0` has been removed - use `BackgroundTransformer` instead. +- The nullability of the generic type parameter in `dio`s request methods is now respected for the responses' `data`. + +> [!WARNING] +> The migration depends on your use case and API responses. +> If in doubt make the type nullable and correctly handle the null case. +> DO NOT just delete the null checks if they are shown to be redundant, but think about the impact. + +Before: +```dart +Response response = await Dio().get('https://example.com'); +String data = response.data!; +``` + +After: +```dart +Response response = await Dio().get('https://example.com'); +String data = response.data; +``` +Or: +```dart +Response response = await Dio().get('https://example.com'); +String? data = response.data; +``` ## 5.0.0 diff --git a/dio/lib/src/dio/dio_for_native.dart b/dio/lib/src/dio/dio_for_native.dart index 38cc093c3..7857305bc 100644 --- a/dio/lib/src/dio/dio_for_native.dart +++ b/dio/lib/src/dio/dio_for_native.dart @@ -102,7 +102,7 @@ class DioForNative extends DioMixin implements Dio { int received = 0; // Stream - final stream = response.data!.stream; + final stream = response.data.stream; bool compressed = false; int total = 0; final contentEncoding = response.headers.value( diff --git a/dio/lib/src/dio_mixin.dart b/dio/lib/src/dio_mixin.dart index aae150c23..de3f21752 100644 --- a/dio/lib/src/dio_mixin.dart +++ b/dio/lib/src/dio_mixin.dart @@ -525,6 +525,7 @@ abstract class DioMixin implements Dio { // Make sure headers and [ResponseBody.headers] are the same instance. responseBody.headers = headers.map; final ret = Response( + data: null, headers: headers, requestOptions: reqOpt, redirects: responseBody.redirects ?? [], @@ -713,7 +714,7 @@ abstract class DioMixin implements Dio { requestOptions: requestOptions, ); } else if (response is! Response) { - final T? data = response.data as T?; + final T data = response.data as T; final Headers headers; if (data is ResponseBody) { headers = Headers.fromMap( diff --git a/dio/lib/src/response.dart b/dio/lib/src/response.dart index 4713330f2..a98915bd7 100644 --- a/dio/lib/src/response.dart +++ b/dio/lib/src/response.dart @@ -11,7 +11,7 @@ import 'redirect_record.dart'; /// in anytime, typically by [Interceptor] and [Transformer]. class Response { Response({ - this.data, + required this.data, required this.requestOptions, this.statusCode, this.statusMessage, @@ -27,7 +27,7 @@ class Response { /// /// The content could have been transformed by the [Transformer] /// before it can use eventually. - T? data; + T data; /// The [RequestOptions] used for the corresponding request. RequestOptions requestOptions; diff --git a/dio/test/options_test.dart b/dio/test/options_test.dart index f7d7a8464..8c59d4b4a 100644 --- a/dio/test/options_test.dart +++ b/dio/test/options_test.dart @@ -357,9 +357,9 @@ void main() { // Regression: https://github.com/cfug/dio/issues/1834 final r11 = await dio.get(''); expect(r11.data, ''); - final r12 = await dio.get(''); + final r12 = await dio.get(''); expect(r12.data, null); - final r13 = await dio.get>(''); + final r13 = await dio.get?>(''); expect(r13.data, null); }); diff --git a/dio/test/request_integration_test.dart b/dio/test/request_integration_test.dart index e95b409dd..6507b4b96 100644 --- a/dio/test/request_integration_test.dart +++ b/dio/test/request_integration_test.dart @@ -327,7 +327,7 @@ void main() { ); expect(response.data, isA()); expect(response.data, isNotEmpty); - expect(response.data![0], 1); + expect(response.data[0], 1); }); }); diff --git a/dio/test/timeout_test.dart b/dio/test/timeout_test.dart index 5735da650..bd9e6d73c 100644 --- a/dio/test/timeout_test.dart +++ b/dio/test/timeout_test.dart @@ -37,14 +37,16 @@ void main() { ); dio.options.connectTimeout = Duration(milliseconds: 5); - await dio - .get('/') - .catchError((e) => Response(requestOptions: RequestOptions())); + await dio.get('/').catchError((e) => Response( + requestOptions: RequestOptions(), + data: null, + )); expect(client.connectionTimeout, dio.options.connectTimeout); dio.options.connectTimeout = Duration(milliseconds: 10); - await dio - .get('/') - .catchError((e) => Response(requestOptions: RequestOptions())); + await dio.get('/').catchError((e) => Response( + requestOptions: RequestOptions(), + data: null, + )); expect(client.connectionTimeout, dio.options.connectTimeout); }, testOn: 'vm'); }); diff --git a/dio/test/utils.dart b/dio/test/utils.dart index f260bb416..101d57e0b 100644 --- a/dio/test/utils.dart +++ b/dio/test/utils.dart @@ -112,6 +112,25 @@ Future startServer() async { return; } + if (path == '/null-response') { + response.headers.contentType = ContentType('text', 'plain'); + response + ..statusCode = 200 + ..contentLength = -1; + response.close(); + return; + } + + if (path == '/non-null-response') { + response.headers.contentType = ContentType('text', 'plain'); + response + ..statusCode = 200 + ..contentLength = -1 + ..write('response'); + response.close(); + return; + } + final requestBodyBytes = await ByteStream(request).toBytes(); final encodingName = request.uri.queryParameters['response-encoding']; final outputEncoding = encodingName == null diff --git a/example/lib/generic.dart b/example/lib/generic.dart index 75c269d27..05e58d2ea 100644 --- a/example/lib/generic.dart +++ b/example/lib/generic.dart @@ -42,5 +42,5 @@ void main() async { // This is the recommended way. final r = await dio.get('https://baidu.com'); - print(r.data?.length); + print(r.data.length); } diff --git a/example_flutter_app/lib/main.dart b/example_flutter_app/lib/main.dart index c745efb5c..5a5c13570 100644 --- a/example_flutter_app/lib/main.dart +++ b/example_flutter_app/lib/main.dart @@ -54,7 +54,7 @@ class _MyHomePageState extends State { .then((r) { setState(() { print(r.data); - _text = r.data!.replaceAll(RegExp(r'\s'), ''); + _text = r.data.replaceAll(RegExp(r'\s'), ''); }); }); } catch (e) { diff --git a/example_flutter_app/lib/routes/request.dart b/example_flutter_app/lib/routes/request.dart index d44d88eae..eeb5d7a6b 100644 --- a/example_flutter_app/lib/routes/request.dart +++ b/example_flutter_app/lib/routes/request.dart @@ -26,7 +26,7 @@ class _RequestRouteState extends State { onPressed: () { dio.get('https://httpbin.org/get').then((r) { setState(() { - _text = r.data!; + _text = r.data; }); }); }, diff --git a/plugins/cookie_manager/test/cookies_test.dart b/plugins/cookie_manager/test/cookies_test.dart index 931c12a72..aea70fea2 100644 --- a/plugins/cookie_manager/test/cookies_test.dart +++ b/plugins/cookie_manager/test/cookies_test.dart @@ -42,6 +42,7 @@ void main() { final firstRequestOptions = RequestOptions(baseUrl: exampleUrl); final mockResponse = Response( + data: null, requestOptions: firstRequestOptions, headers: Headers.fromMap( {HttpHeaders.setCookieHeader: mockFirstRequestCookies}, @@ -77,6 +78,7 @@ void main() { final requestOptions = RequestOptions(baseUrl: exampleUrl); final mockResponse = Response( + data: null, requestOptions: requestOptions, headers: Headers.fromMap( {HttpHeaders.setCookieHeader: mockResponseCookies}, diff --git a/plugins/native_dio_adapter/example/lib/main.dart b/plugins/native_dio_adapter/example/lib/main.dart index fe0391dd5..24fe25cb1 100644 --- a/plugins/native_dio_adapter/example/lib/main.dart +++ b/plugins/native_dio_adapter/example/lib/main.dart @@ -84,7 +84,7 @@ class _MyHomePageState extends State { return AlertDialog( title: Text('Response ${response.statusCode}'), content: SingleChildScrollView( - child: Text(response.data ?? 'No response'), + child: Text(response.data), ), actions: [ MaterialButton( @@ -120,7 +120,7 @@ class _MyHomePageState extends State { return AlertDialog( title: Text('Response ${response.statusCode}'), content: SingleChildScrollView( - child: Text(response.data ?? 'No response'), + child: Text(response.data), ), actions: [ MaterialButton(