From a6ce7ecb31f0d081cd03522a2a5482d75cb7d989 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Wed, 22 Mar 2023 13:46:35 +0100 Subject: [PATCH 1/6] Test typed response --- dio/lib/src/dio_mixin.dart | 21 ++----------------- dio/lib/src/response.dart | 4 ++-- dio/test/request_test.dart | 41 ++++++++++++++++++++++++++++++++++++++ dio/test/utils.dart | 19 ++++++++++++++++++ 4 files changed, 64 insertions(+), 21 deletions(-) diff --git a/dio/lib/src/dio_mixin.dart b/dio/lib/src/dio_mixin.dart index 0571796e8..d86b95f60 100644 --- a/dio/lib/src/dio_mixin.dart +++ b/dio/lib/src/dio_mixin.dart @@ -549,6 +549,7 @@ abstract class DioMixin implements Dio { // Make sure headers and responseBody.headers point to a same Map responseBody.headers = headers.map; final ret = Response( + data: null, headers: headers, requestOptions: reqOpt, redirects: responseBody.redirects ?? [], @@ -579,7 +580,6 @@ abstract class DioMixin implements Dio { } bool _isValidToken(String token) { - _checkNotNullable(token, 'token'); // from https://www.rfc-editor.org/rfc/rfc2616#page-15 // // CTL = ) { - final T? data = response.data as T?; + final T data = response.data as T; final Headers headers; if (data is ResponseBody) { headers = Headers.fromMap(data.headers); @@ -737,23 +737,6 @@ abstract class DioMixin implements Dio { } } -/// A null-check function for function parameters in Null Safety enabled code. -/// -/// Because Dart does not have full null safety -/// until all legacy code has been removed from a program, -/// a non-nullable parameter can still end up with a `null` value. -/// This function can be used to guard those functions against null arguments. -/// It throws a [TypeError] because we are really seeing the failure to -/// assign `null` to a non-nullable type. -/// -/// See http://dartbug.com/40614 for context. -T _checkNotNullable(T value, String name) { - if ((value as dynamic) == null) { - throw NotNullableError(name); - } - return value; -} - /// A [TypeError] thrown by [_checkNotNullable]. class NotNullableError extends Error implements TypeError { NotNullableError(this._name); diff --git a/dio/lib/src/response.dart b/dio/lib/src/response.dart index 23de0f11d..ad6093547 100644 --- a/dio/lib/src/response.dart +++ b/dio/lib/src/response.dart @@ -6,7 +6,7 @@ import 'redirect_record.dart'; /// Response describes the http Response info. class Response { Response({ - this.data, + required this.data, required this.requestOptions, this.statusCode, this.statusMessage, @@ -17,7 +17,7 @@ class Response { }) : headers = headers ?? Headers(); /// Response body. may have been transformed, please refer to [ResponseType]. - T? data; + T data; /// The corresponding request info. RequestOptions requestOptions; diff --git a/dio/test/request_test.dart b/dio/test/request_test.dart index cd49e293b..3f33aca57 100644 --- a/dio/test/request_test.dart +++ b/dio/test/request_test.dart @@ -191,5 +191,46 @@ void main() { expect(response.data, isA()); expect(response.data[0], 1); }); + + group('response type is', () { + group('nullable:', () { + test(' => null', () async { + final response = await dio.get('/null-response'); + expect(response.data, isNull); + expect(response.data?.isEmpty, isNull); + }); + + test(' => non-null', () async { + final response = await dio.get('/non-null-response'); + expect(response.data, isNotNull); + expect(response.data.runtimeType, String); + expect(response.data?.isEmpty, false); + expect(response.data, 'response'); + }); + }); + + group('non-nullable:', () { + test(' => null', () async { + await expectLater( + dio.get('/null-response'), + throwsA(allOf([ + isA(), + (DioError e) => e.type == DioErrorType.unknown, + (DioError e) => + e.error.toString() == + "type 'Null' is not a subtype of type 'String' in type cast", + ])), + ); + }); + + test(' => non-null', () async { + final response = await dio.get('/non-null-response'); + expect(response.data, isNotNull); + expect(response.data.runtimeType, String); + expect(response.data.isEmpty, false); + expect(response.data, 'response'); + }); + }); + }); }); } diff --git a/dio/test/utils.dart b/dio/test/utils.dart index 0771f0719..2df6c25df 100644 --- a/dio/test/utils.dart +++ b/dio/test/utils.dart @@ -102,6 +102,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 From 3f9e941c21524e986119d09d6a3e1b2ef6846828 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Wed, 22 Mar 2023 14:41:54 +0100 Subject: [PATCH 2/6] Nullablity fixes --- dio/lib/src/dio/dio_for_native.dart | 2 +- 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 ++ plugins/native_dio_adapter/example/lib/main.dart | 4 ++-- 6 files changed, 8 insertions(+), 6 deletions(-) diff --git a/dio/lib/src/dio/dio_for_native.dart b/dio/lib/src/dio/dio_for_native.dart index f256bc8f5..812aeaaab 100644 --- a/dio/lib/src/dio/dio_for_native.dart +++ b/dio/lib/src/dio/dio_for_native.dart @@ -94,7 +94,7 @@ class DioForNative with 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/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 43bb79349..117624921 100644 --- a/example_flutter_app/lib/main.dart +++ b/example_flutter_app/lib/main.dart @@ -66,7 +66,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 6915bc49d..9cb7bd910 100644 --- a/plugins/cookie_manager/test/cookies_test.dart +++ b/plugins/cookie_manager/test/cookies_test.dart @@ -41,6 +41,7 @@ void main() { final firstRequestOptions = RequestOptions(baseUrl: exampleUrl); final mockResponse = Response( + data: null, requestOptions: firstRequestOptions, headers: Headers.fromMap( {HttpHeaders.setCookieHeader: mockFirstRequestCookies}, @@ -76,6 +77,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 69a69cab2..6f61eadec 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( From 5853698175016f9de482dceae4a7a26580d015e8 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Fri, 2 Jun 2023 00:39:00 +0200 Subject: [PATCH 3/6] Fix tests after rebase --- dio/test/request_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dio/test/request_test.dart b/dio/test/request_test.dart index 3f33aca57..86169b356 100644 --- a/dio/test/request_test.dart +++ b/dio/test/request_test.dart @@ -214,9 +214,9 @@ void main() { await expectLater( dio.get('/null-response'), throwsA(allOf([ - isA(), - (DioError e) => e.type == DioErrorType.unknown, - (DioError e) => + isA(), + (DioException e) => e.type == DioExceptionType.unknown, + (DioException e) => e.error.toString() == "type 'Null' is not a subtype of type 'String' in type cast", ])), From cd82f184ef78bd7bd4b4551e198f1c2420d3fc98 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Thu, 21 Dec 2023 23:18:01 +0100 Subject: [PATCH 4/6] Update tests after merge --- dio/test/options_test.dart | 4 ++-- dio/test/timeout_test.dart | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) 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/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'); }); From f0360021469912b44af730a74fc5ec2d0a5cd3a2 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Thu, 21 Dec 2023 23:14:44 +0100 Subject: [PATCH 5/6] Add changelog and migration notes --- dio/CHANGELOG.md | 6 ++++-- dio/doc/migration_guide.md | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 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 From 2b6ced84dd7167062aa6dba41dffb80c35951ea4 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Thu, 21 Dec 2023 23:22:24 +0100 Subject: [PATCH 6/6] Fix test --- dio/test/request_integration_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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); }); });