Fix return type of OnHttpClientCreate (cfug#1812)

The return type of `OnHttpClientCreate` has been changed from `HttpClient?` to `HttpClient`. While technically a breaking change, it is considered a bug because the callback was actually a no-op when the provided client was modified but not returned. Fixes 1. of cfug#1811 ### New Pull Request Checklist - [x] I have read the [Documentation](https://pub.dev/documentation/dio/latest/) - [x] I have searched for a similar pull request in the [project](https://github.com/cfug/dio/pulls) and found none - [x] I have updated this branch with the latest `main` branch to avoid conflicts (via merge from master or rebase) - [ ] I have added the required tests to prove the fix/feature I'm adding - [x] I have updated the documentation (if necessary) - [x] I have run the tests without failures - [x] I have updated the `CHANGELOG.md` in the corresponding package
Frezyx · May 13, 2023 · 690896a · 690896a
1 parent a3f61a9
commit 690896a
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 38 deletions.
diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 - Make `LogInterceptor` prints in DEBUG mode (when the assertion is enabled) by default.
 - Fix `IOHttpClientAdapter.onHttpClientCreate` Repeated calls
+- `IOHttpClientAdapter.onHttpClientCreate` has been deprecated and is scheduled for removal in
+  Dio 6.0.0 - Please use the replacement `IOHttpClientAdapter.createHttpClient` instead.
 
 ## 5.1.2
 

diff --git a/dio/README-ZH.md b/dio/README-ZH.md
@@ -698,14 +698,15 @@ dio.httpClientAdapter = HttpClientAdapter();
 
 ### 设置代理
 
-`IOHttpClientAdapter` 提供了一个 `onHttpClientCreate` 回调来设置底层 `HttpClient` 的代理：
+`IOHttpClientAdapter` 提供了一个 `createHttpClient` 回调来设置底层 `HttpClient` 的代理：
 
 ```dart
 import 'package:dio/io.dart';
 
 void initAdapter() {
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (client) {
+    createHttpClient: () {
+      final client = HttpClient();
       client.findProxy = (uri) {
         // 将请求代理至 localhost:8888。
         // 请注意，代理会在你正在运行应用的设备上生效，而不是在宿主平台生效。
@@ -776,7 +777,8 @@ openssl s_client -servername pinning-test.badssl.com -connect pinning-test.badss
 void initAdapter() {
   String PEM = 'XXXXX'; // root certificate content
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (client) {
+    createHttpClient: () {
+      final client = HttpClient();
       client.badCertificateCallback = (X509Certificate cert, String host, int port) {
         return cert.pem == PEM; // Verify the certificate.
       };

diff --git a/dio/README.md b/dio/README.md
@@ -746,7 +746,8 @@ import 'package:dio/io.dart';
 
 void initAdapter() {
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (client) {
+    createHttpClient: () {
+      final client = HttpClient();
       // Config the client.
       client.findProxy = (uri) {
         // Forward all request to proxy "localhost:8888".
@@ -824,7 +825,8 @@ Suppose the certificate format is PEM, the code like:
 void initAdapter() {
   String PEM = 'XXXXX'; // root certificate content
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (client) {
+    createHttpClient: () {
+      final client = HttpClient();
       client.badCertificateCallback = (X509Certificate cert, String host, int port) {
         return cert.pem == PEM; // Verify the certificate.
       };

diff --git a/dio/lib/src/adapters/io_adapter.dart b/dio/lib/src/adapters/io_adapter.dart
@@ -10,7 +10,12 @@ import '../redirect_record.dart';
 @Deprecated('Use IOHttpClientAdapter instead. This will be removed in 6.0.0')
 typedef DefaultHttpClientAdapter = IOHttpClientAdapter;
 
+@Deprecated('Use CreateHttpClient instead. This will be removed in 6.0.0')
 typedef OnHttpClientCreate = HttpClient? Function(HttpClient client);
+
+/// Can be used to provide a custom [HttpClient] for Dio.
+typedef CreateHttpClient = HttpClient Function();
+
 typedef ValidateCertificate = bool Function(
   X509Certificate? certificate,
   String host,
@@ -21,12 +26,22 @@ HttpClientAdapter createAdapter() => IOHttpClientAdapter();
 
 /// The default [HttpClientAdapter] for native platforms.
 class IOHttpClientAdapter implements HttpClientAdapter {
-  IOHttpClientAdapter({this.onHttpClientCreate, this.validateCertificate});
+  IOHttpClientAdapter({
+    @Deprecated('Use createHttpClient instead. This will be removed in 6.0.0')
+        this.onHttpClientCreate,
+    this.createHttpClient,
+    this.validateCertificate,
+  });
 
   /// [Dio] will create [HttpClient] when it is needed. If [onHttpClientCreate]
   /// has provided, [Dio] will call it when a [HttpClient] created.
+  @Deprecated('Use createHttpClient instead. This will be removed in 6.0.0')
   OnHttpClientCreate? onHttpClientCreate;
 
+  /// When this callback is set, [Dio] will call it every
+  /// time it needs a [HttpClient].
+  CreateHttpClient? createHttpClient;
+
   /// Allows the user to decide if the response certificate is good.
   /// If this function is missing, then the certificate is allowed.
   /// This method is called only if both the [SecurityContext] and
@@ -188,25 +203,27 @@ class IOHttpClientAdapter implements HttpClientAdapter {
     Duration? connectionTimeout,
   ) {
     if (cancelFuture != null) {
-      final HttpClient client =
-          onHttpClientCreate?.call(HttpClient()) ?? HttpClient();
+      final client = _createHttpClient();
       client.userAgent = null;
       client.idleTimeout = Duration(seconds: 0);
       cancelFuture.whenComplete(() => client.close(force: true));
       return client..connectionTimeout = connectionTimeout;
     }
-
-    if (_cachedHttpClient == null) {
-      final HttpClient client = HttpClient()
-        ..idleTimeout = Duration(seconds: 3);
-      _cachedHttpClient = onHttpClientCreate?.call(client) ?? client;
-    }
-    return _cachedHttpClient!..connectionTimeout = connectionTimeout;
+    return (_cachedHttpClient ??= _createHttpClient())
+      ..connectionTimeout = connectionTimeout;
   }
 
   @override
   void close({bool force = false}) {
     _closed = true;
     _cachedHttpClient?.close(force: force);
   }
+
+  HttpClient _createHttpClient() {
+    if (createHttpClient != null) {
+      return createHttpClient!();
+    }
+    final client = HttpClient()..idleTimeout = Duration(seconds: 3);
+    return onHttpClientCreate?.call(client) ?? client;
+  }
 }
diff --git a/dio/test/adapters_test.dart b/dio/test/adapters_test.dart
@@ -1,20 +1,39 @@
+import 'dart:io';
+
 import 'package:dio/dio.dart';
 import 'package:dio/io.dart';
 import 'package:test/test.dart';
 
 void main() {
-  group(HttpClientAdapter, () {
-    test(
-        'IOHttpClientAdapter.onHttpClientCreate is only executed once per request',
-        () async {
-      int onHttpClientCreateInvokeCount = 0;
-      final dio = Dio();
-      dio.httpClientAdapter = IOHttpClientAdapter(onHttpClientCreate: (client) {
-        onHttpClientCreateInvokeCount++;
-        return client;
+  group(
+    IOHttpClientAdapter,
+    () {
+      test('onHttpClientCreate is only executed once per request', () async {
+        int onHttpClientCreateInvokeCount = 0;
+        final dio = Dio();
+        dio.httpClientAdapter = IOHttpClientAdapter(
+          onHttpClientCreate: (client) {
+            onHttpClientCreateInvokeCount++;
+            return client;
+          },
+        );
+        await dio.get('https://pub.dev');
+        expect(onHttpClientCreateInvokeCount, 1);
+      });
+
+      test('createHttpClientCount is only executed once per request', () async {
+        int createHttpClientCount = 0;
+        final dio = Dio();
+        dio.httpClientAdapter = IOHttpClientAdapter(
+          createHttpClient: () {
+            createHttpClientCount++;
+            return HttpClient();
+          },
+        );
+        await dio.get('https://pub.dev');
+        expect(createHttpClientCount, 1);
       });
-      await dio.get('https://pub.dev');
-      expect(onHttpClientCreateInvokeCount, 1);
-    });
-  });
+    },
+    testOn: 'vm',
+  );
 }
diff --git a/dio/test/exception_test.dart b/dio/test/exception_test.dart
@@ -56,8 +56,9 @@ void main() {
     () async {
       final dio = Dio();
       dio.httpClientAdapter = IOHttpClientAdapter(
-        onHttpClientCreate: (client) {
-          return client..badCertificateCallback = (cert, host, port) => true;
+        createHttpClient: () {
+          return HttpClient()
+            ..badCertificateCallback = (cert, host, port) => true;
         },
       );
       Response response = await dio.get('https://wrong.host.badssl.com/');

diff --git a/dio/test/pinning_test.dart b/dio/test/pinning_test.dart
@@ -67,8 +67,9 @@ void main() {
     final dio = Dio();
     // badCertificateCallback must allow the untrusted certificate through
     dio.httpClientAdapter = IOHttpClientAdapter(
-      onHttpClientCreate: (client) {
-        return client..badCertificateCallback = (cert, host, port) => true;
+      createHttpClient: () {
+        return HttpClient()
+          ..badCertificateCallback = (cert, host, port) => true;
       },
       validateCertificate: (cert, host, port) {
         return fingerprint == sha256.convert(cert!.der).toString();
@@ -88,7 +89,7 @@ void main() {
       try {
         final dio = Dio();
         dio.httpClientAdapter = IOHttpClientAdapter(
-          onHttpClientCreate: (client) {
+          createHttpClient: () {
             return HttpClient(
               context: SecurityContext(withTrustedRoots: false),
             );
@@ -113,7 +114,7 @@ void main() {
     try {
       final dio = Dio();
       dio.httpClientAdapter = IOHttpClientAdapter(
-        onHttpClientCreate: (HttpClient client) {
+        createHttpClient: () {
           final effectiveClient = HttpClient(
             context: SecurityContext(withTrustedRoots: false),
           );

diff --git a/dio/test/readtimeout_test.dart b/dio/test/readtimeout_test.dart
@@ -105,7 +105,7 @@ void main() {
     final adapter = IOHttpClientAdapter();
     final http = HttpClient();
 
-    adapter.onHttpClientCreate = (_) => http;
+    adapter.createHttpClient = () => http;
     dio.httpClientAdapter = adapter;
     dio.options
       ..baseUrl = serverUrl.toString()

diff --git a/example/lib/certificate_pinning.dart b/example/lib/certificate_pinning.dart
@@ -18,7 +18,7 @@ void main() async {
 
   // Don't trust any certificate just because their root cert is trusted
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (_) {
+    createHttpClient: () {
       final client = HttpClient(
         context: SecurityContext(withTrustedRoots: false),
       );

diff --git a/example/lib/formdata.dart b/example/lib/formdata.dart
@@ -1,4 +1,5 @@
 import 'dart:convert';
+import 'dart:io';
 
 import 'package:dio/dio.dart';
 import 'package:dio/io.dart';
@@ -93,7 +94,8 @@ void main() async {
   dio.interceptors.add(LogInterceptor());
   // dio.interceptors.add(LogInterceptor(requestBody: true));
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (client) {
+    createHttpClient: () {
+      final client = HttpClient();
       client.findProxy = (uri) {
         // Proxy all request to localhost:8888
         return 'PROXY localhost:8888';

diff --git a/example/lib/proxy.dart b/example/lib/proxy.dart
@@ -9,7 +9,8 @@ void main() async {
     ..headers['user-agent'] = 'xxx'
     ..contentType = 'text';
   dio.httpClientAdapter = IOHttpClientAdapter(
-    onHttpClientCreate: (HttpClient client) {
+    createHttpClient: () {
+      final client = HttpClient();
       client.findProxy = (uri) {
         // Proxy all request to localhost:8888.
         // Be aware, the proxy should went through you running device,