Support BaseResponseWithUrl in package:cupertino_http and package:cronet_http (#1110)

brianquinlan · web-flow · commit ccefa7c46212 · 2024-01-17T15:29:42.000-08:00
diff --git a/.github/workflows/cupertino.yml b/.github/workflows/cupertino.yml
@@ -57,7 +57,7 @@ jobs:
       matrix:
         # Test on the minimum supported flutter version and the latest
         # version.
-        flutter-version: ["3.10.0", "any"]
+        flutter-version: ["3.16.0", "any"]
     runs-on: macos-latest
     defaults:
       run:
diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml
diff --git a/pkgs/cronet_http/CHANGELOG.md b/pkgs/cronet_http/CHANGELOG.md
@@ -1,7 +1,8 @@
-## 1.0.1-wip
+## 1.1.0
 
 * Use `package:http_image_provider` in the example application.
 * Support Android API 21+.
+* Support `BaseResponseWithUrl`.
 
 ## 1.0.0
 
diff --git a/pkgs/cronet_http/example/pubspec.yaml b/pkgs/cronet_http/example/pubspec.yaml
@@ -4,7 +4,7 @@ description: Demonstrates how to use the cronet_http plugin.
 publish_to: 'none'
 
 environment:
-  sdk: ^3.0.0
+  sdk: ^3.2.0
 
 dependencies:
   cronet_http:
diff --git a/pkgs/cronet_http/lib/src/cronet_client.dart b/pkgs/cronet_http/lib/src/cronet_client.dart
@@ -23,6 +23,21 @@ import 'jni/jni_bindings.dart' as jb;
 final _digitRegex = RegExp(r'^\d+$');
 const _bufferSize = 10 * 1024; // The size of the Cronet read buffer.
 
+/// This class can be removed when `package:http` v2 is released.
+class _StreamedResponseWithUrl extends StreamedResponse
+    implements BaseResponseWithUrl {
+  @override
+  final Uri url;
+
+  _StreamedResponseWithUrl(super.stream, super.statusCode,
+      {required this.url,
+      super.contentLength,
+      super.request,
+      super.headers,
+      super.isRedirect,
+      super.reasonPhrase});
+}
+
 /// The type of caching to use when making HTTP requests.
 enum CacheMode {
   disabled,
@@ -163,9 +178,11 @@ jb.UrlRequestCallbackProxy_UrlRequestCallbackInterface _urlRequestCallbacks(
         case final contentLengthHeader?:
           contentLength = int.parse(contentLengthHeader);
       }
-      responseCompleter.complete(StreamedResponse(
+      responseCompleter.complete(_StreamedResponseWithUrl(
         responseStream!.stream,
         responseInfo.getHttpStatusCode(),
+        url: Uri.parse(
+            responseInfo.getUrl().toDartString(releaseOriginal: true)),
         contentLength: contentLength,
         reasonPhrase: responseInfo
             .getHttpStatusText()
diff --git a/pkgs/cronet_http/pubspec.yaml b/pkgs/cronet_http/pubspec.yaml
@@ -1,5 +1,5 @@
 name: cronet_http
-version: 1.0.1-wip
+version: 1.1.0
 description: >-
   An Android Flutter plugin that provides access to the Cronet HTTP client.
 repository: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http
@@ -11,7 +11,7 @@ environment:
 dependencies:
   flutter:
     sdk: flutter
-  http: '>=0.13.4 <2.0.0'
+  http: ^1.2.0
   jni: ^0.7.2
 
 dev_dependencies:
diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md
@@ -1,6 +1,7 @@
-## 1.2.1-wip
+## 1.3.0
 
 * Use `package:http_image_provider` in the example application.
+* Support `BaseResponseWithUrl`.
 
 ## 1.2.0
 
diff --git a/pkgs/cupertino_http/example/pubspec.yaml b/pkgs/cupertino_http/example/pubspec.yaml
@@ -6,8 +6,8 @@ publish_to: 'none'
 version: 1.0.0+1
 
 environment:
-  sdk: ^3.0.0
-  flutter: '>=3.10.0'
+  sdk: ^3.2.0
+  flutter: ^3.16.0
 
 dependencies:
   cupertino_http:
diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart
@@ -17,11 +17,27 @@ import 'cupertino_api.dart';
 
 final _digitRegex = RegExp(r'^\d+$');
 
+/// This class can be removed when `package:http` v2 is released.
+class _StreamedResponseWithUrl extends StreamedResponse
+    implements BaseResponseWithUrl {
+  @override
+  final Uri url;
+
+  _StreamedResponseWithUrl(super.stream, super.statusCode,
+      {required this.url,
+      super.contentLength,
+      super.request,
+      super.headers,
+      super.isRedirect,
+      super.reasonPhrase});
+}
+
 class _TaskTracker {
   final responseCompleter = Completer<URLResponse>();
   final BaseRequest request;
   final responseController = StreamController<Uint8List>();
   int numRedirects = 0;
+  Uri? lastUrl; // The last URL redirected to.
 
   _TaskTracker(this.request);
 
@@ -180,6 +196,7 @@ class CupertinoClient extends BaseClient {
     ++taskTracker.numRedirects;
     if (taskTracker.request.followRedirects &&
         taskTracker.numRedirects <= taskTracker.request.maxRedirects) {
+      taskTracker.lastUrl = request.url;
       return request;
     }
     return null;
@@ -292,9 +309,10 @@ class CupertinoClient extends BaseClient {
       );
     }
 
-    return StreamedResponse(
+    return _StreamedResponseWithUrl(
       taskTracker.responseController.stream,
       response.statusCode,
+      url: taskTracker.lastUrl ?? request.url,
       contentLength: response.expectedContentLength == -1
           ? null
           : response.expectedContentLength,
diff --git a/pkgs/cupertino_http/pubspec.yaml b/pkgs/cupertino_http/pubspec.yaml
@@ -1,20 +1,20 @@
 name: cupertino_http
-version: 1.2.1-wip
+version: 1.3.0
 description: >-
   A macOS/iOS Flutter plugin that provides access to the Foundation URL
   Loading System.
 repository: https://github.com/dart-lang/http/tree/master/pkgs/cupertino_http
 
 environment:
-  sdk: ^3.0.0
-  flutter: '>=3.10.0'  # If changed, update test matrix.
+  sdk: ^3.2.0
+  flutter: ^3.16.0  # If changed, update test matrix.
 
 dependencies:
   async: ^2.5.0
   ffi: ^2.1.0
   flutter:
     sdk: flutter
-  http: '>=0.13.4 <2.0.0'
+  http: ^1.2.0
 
 dev_dependencies:
   dart_flutter_team_lints: ^2.0.0
diff --git a/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart b/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart
@@ -27,12 +27,26 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async {
     });
     tearDownAll(() => httpServerChannel.sink.add(null));
 
+    test('no redirect', () async {
+      final request = Request('GET', Uri.http(host, '/'))
+        ..followRedirects = false;
+      final response = await client.send(request);
+      expect(response.statusCode, 200);
+      expect(response.isRedirect, false);
+      if (response case BaseResponseWithUrl(url: final url)) {
+        expect(url, Uri.http(host, '/'));
+      }
+    });
+
     test('disallow redirect', () async {
       final request = Request('GET', Uri.http(host, '/1'))
         ..followRedirects = false;
       final response = await client.send(request);
       expect(response.statusCode, 302);
       expect(response.isRedirect, true);
+      if (response case BaseResponseWithUrl(url: final url)) {
+        expect(url, Uri.http(host, '/1'));
+      }
     }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false);
 
     test('disallow redirect, 0 maxRedirects', () async {
@@ -42,6 +56,9 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async {
       final response = await client.send(request);
       expect(response.statusCode, 302);
       expect(response.isRedirect, true);
+      if (response case BaseResponseWithUrl(url: final url)) {
+        expect(url, Uri.http(host, '/1'));
+      }
     }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false);
 
     test('allow redirect', () async {
@@ -50,6 +67,9 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async {
       final response = await client.send(request);
       expect(response.statusCode, 200);
       expect(response.isRedirect, false);
+      if (response case BaseResponseWithUrl(url: final url)) {
+        expect(url, Uri.http(host, '/'));
+      }
     });
 
     test('allow redirect, 0 maxRedirects', () async {
@@ -69,6 +89,9 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async {
       final response = await client.send(request);
       expect(response.statusCode, 200);
       expect(response.isRedirect, false);
+      if (response case BaseResponseWithUrl(url: final url)) {
+        expect(url, Uri.http(host, '/'));
+      }
     }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false);
 
     test('too many redirects', () async {
diff --git a/pkgs/http_client_conformance_tests/pubspec.yaml b/pkgs/http_client_conformance_tests/pubspec.yaml
@@ -7,12 +7,12 @@ repository: https://github.com/dart-lang/http/tree/master/pkgs/http_client_confo
 publish_to: none
 
 environment:
-  sdk: ^3.0.0
+  sdk: ^3.2.0
 
 dependencies:
   async: ^2.8.2
   dart_style: ^2.2.3
-  http: ^1.0.0
+  http: ^1.2.0
   stream_channel: ^2.1.1
   test: ^1.21.2
 
