Skip to content

Commit

Permalink
Support BaseResponseWithUrl in package:cupertino_http and `packag…
Browse files Browse the repository at this point in the history
…e:cronet_http` (#1110)
  • Loading branch information
brianquinlan authored Jan 17, 2024
1 parent e7a8e25 commit ccefa7c
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cupertino.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
30 changes: 15 additions & 15 deletions .github/workflows/dart.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkgs/cronet_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion pkgs/cronet_http/example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
19 changes: 18 additions & 1 deletion pkgs/cronet_http/lib/src/cronet_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions pkgs/cronet_http/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion pkgs/cupertino_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
4 changes: 2 additions & 2 deletions pkgs/cupertino_http/example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 19 additions & 1 deletion pkgs/cupertino_http/lib/src/cupertino_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions pkgs/cupertino_http/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
23 changes: 23 additions & 0 deletions pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkgs/http_client_conformance_tests/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit ccefa7c

Please sign in to comment.