Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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