From 4066fc37c60b2a7bde8fde4ef3dc7218ebf08d30 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 13:10:49 -0800 Subject: [PATCH 1/9] Add `BaseResponseV2.url` --- pkgs/http/CHANGELOG.md | 3 +- pkgs/http/lib/src/base_response.dart | 34 +++++++++++++++++++ pkgs/http/lib/src/browser_client.dart | 17 +++++++++- pkgs/http/lib/src/io_client.dart | 21 +++++++++++- pkgs/http/pubspec.yaml | 2 +- pkgs/http/test/io/request_test.dart | 3 ++ .../lib/src/redirect_tests.dart | 23 +++++++++++++ 7 files changed, 99 insertions(+), 4 deletions(-) diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index 7b8dec4fdd..c2987adc38 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,6 +1,7 @@ -## 1.1.3-wip +## 1.2.0 * Add `MockClient.pngResponse`, which makes it easier to fake image responses. +* Added the ability to fetch the URL of the response through `BaseResponseV2`. ## 1.1.2 diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index ed95f6cdb2..89d66e80f1 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -4,6 +4,9 @@ import 'base_client.dart'; import 'base_request.dart'; +import 'client.dart'; +import 'response.dart'; +import 'streamed_response.dart'; /// The base class for HTTP responses. /// @@ -68,3 +71,34 @@ abstract class BaseResponse { } } } + +/// Fields and methods that will be added to [BaseResponse] when `package:http` +/// version 2 is released. +/// +/// [Client] methods that return a [BaseResponse] subclass, such as [Response] +/// or [StreamedResponse], **may** return a [BaseResponseV2]. +/// +/// For example: +/// +/// ```dart +/// final client = Client(); +/// final response = client.get(Uri.https('example.com', '/')); +/// Uri? finalUri; +/// if (response is BaseResponseV2) { +/// finalUri = (response as BaseResponseV2).uri; +/// } +/// // Do something with `finalUri`. +/// client.close(); +/// ``` +mixin BaseResponseV2 on BaseResponse { + /// The [Uri] of the response returned by the server. + /// + /// If no redirects were followed, [url] will be the same as the requested + /// [Uri]. + /// + /// If redirects were followed, [url] will be the [Uri] of the last redirect + /// that was followed. + /// + /// May be `null` when the response is for a special URL such as "about:". + abstract final Uri? url; +} diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 80db8b1291..0b0a5d365f 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -9,6 +9,7 @@ import 'package:web/helpers.dart'; import 'base_client.dart'; import 'base_request.dart'; +import 'base_response.dart'; import 'byte_stream.dart'; import 'exception.dart'; import 'streamed_response.dart'; @@ -26,6 +27,17 @@ BaseClient createClient() { return BrowserClient(); } +class _StreamedResponseV2 extends StreamedResponse with BaseResponseV2 { + @override + final Uri? url; + _StreamedResponseV2(super.stream, super.statusCode, + {super.contentLength, + super.request, + super.headers, + this.url, + super.reasonPhrase}); +} + /// A `package:web`-based HTTP client that runs in the browser and is backed by /// [XMLHttpRequest]. /// @@ -79,10 +91,13 @@ class BrowserClient extends BaseClient { return; } var body = (xhr.response as JSArrayBuffer).toDart.asUint8List(); - completer.complete(StreamedResponse( + var responseUrl = xhr.responseURL; + var url = responseUrl.isNotEmpty ? Uri.parse(responseUrl) : null; + completer.complete(_StreamedResponseV2( ByteStream.fromBytes(body), xhr.status, contentLength: body.length, request: request, + url: url, headers: xhr.responseHeaders, reasonPhrase: xhr.statusText)); })); diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index db66b028c4..70164090e9 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'base_client.dart'; import 'base_request.dart'; +import 'base_response.dart'; import 'client.dart'; import 'exception.dart'; import 'io_streamed_response.dart'; @@ -46,6 +47,21 @@ class _ClientSocketException extends ClientException String toString() => 'ClientException with $cause, uri=$uri'; } +class _IOStreamedResponseV2 extends IOStreamedResponse with BaseResponseV2 { + @override + final Uri? url; + + _IOStreamedResponseV2(super.stream, super.statusCode, + {super.contentLength, + super.request, + super.headers, + super.isRedirect, + this.url, + super.persistentConnection, + super.reasonPhrase, + super.inner}); +} + /// A `dart:io`-based HTTP [Client]. /// /// If there is a socket-level failure when communicating with the server @@ -116,7 +132,7 @@ class IOClient extends BaseClient { headers[key] = values.map((value) => value.trimRight()).join(','); }); - return IOStreamedResponse( + return _IOStreamedResponseV2( response.handleError((Object error) { final httpException = error as HttpException; throw ClientException(httpException.message, httpException.uri); @@ -127,6 +143,9 @@ class IOClient extends BaseClient { request: request, headers: headers, isRedirect: response.isRedirect, + url: response.redirects.isNotEmpty + ? response.redirects.last.location + : request.url, persistentConnection: response.persistentConnection, reasonPhrase: response.reasonPhrase, inner: response); diff --git a/pkgs/http/pubspec.yaml b/pkgs/http/pubspec.yaml index 1645f96048..a531a6373c 100644 --- a/pkgs/http/pubspec.yaml +++ b/pkgs/http/pubspec.yaml @@ -1,5 +1,5 @@ name: http -version: 1.1.3-wip +version: 1.2.0 description: A composable, multi-platform, Future-based API for HTTP requests. repository: https://github.com/dart-lang/http/tree/master/pkgs/http diff --git a/pkgs/http/test/io/request_test.dart b/pkgs/http/test/io/request_test.dart index ac6b44c3fd..ee95fb45be 100644 --- a/pkgs/http/test/io/request_test.dart +++ b/pkgs/http/test/io/request_test.dart @@ -6,6 +6,7 @@ library; import 'package:http/http.dart' as http; +import 'package:http/src/base_response.dart'; import 'package:test/test.dart'; import '../utils.dart'; @@ -46,6 +47,7 @@ void main() { final response = await request.send(); expect(response.statusCode, equals(302)); + expect((response as BaseResponseV2).url, serverUrl.resolve('/redirect')); }); test('with redirects', () async { @@ -55,6 +57,7 @@ void main() { expect(response.statusCode, equals(200)); final bytesString = await response.stream.bytesToString(); expect(bytesString, parse(containsPair('path', '/'))); + expect((response as BaseResponseV2).url, serverUrl.resolve('/')); }); test('exceeding max redirects', () async { diff --git a/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart b/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart index 47a77a7dbf..0d6a0a1129 100644 --- 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 is BaseResponseV2) { + expect((response as BaseResponseV2).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 is BaseResponseV2) { + expect((response as BaseResponseV2).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 is BaseResponseV2) { + expect((response as BaseResponseV2).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 is BaseResponseV2) { + expect((response as BaseResponseV2).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 is BaseResponseV2) { + expect((response as BaseResponseV2).url, Uri.http(host, '/')); + } }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false); test('too many redirects', () async { From 83b360b9c22662e23419393ab6948deec3a50d67 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 13:27:42 -0800 Subject: [PATCH 2/9] Update redirect_tests.dart --- .../lib/src/redirect_tests.dart | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart b/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart index 0d6a0a1129..47a77a7dbf 100644 --- a/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/redirect_tests.dart @@ -27,26 +27,12 @@ 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 is BaseResponseV2) { - expect((response as BaseResponseV2).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 is BaseResponseV2) { - expect((response as BaseResponseV2).url, Uri.http(host, '/1')); - } }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false); test('disallow redirect, 0 maxRedirects', () async { @@ -56,9 +42,6 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async { final response = await client.send(request); expect(response.statusCode, 302); expect(response.isRedirect, true); - if (response is BaseResponseV2) { - expect((response as BaseResponseV2).url, Uri.http(host, '/1')); - } }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false); test('allow redirect', () async { @@ -67,9 +50,6 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async { final response = await client.send(request); expect(response.statusCode, 200); expect(response.isRedirect, false); - if (response is BaseResponseV2) { - expect((response as BaseResponseV2).url, Uri.http(host, '/')); - } }); test('allow redirect, 0 maxRedirects', () async { @@ -89,9 +69,6 @@ void testRedirect(Client client, {bool redirectAlwaysAllowed = false}) async { final response = await client.send(request); expect(response.statusCode, 200); expect(response.isRedirect, false); - if (response is BaseResponseV2) { - expect((response as BaseResponseV2).url, Uri.http(host, '/')); - } }, skip: redirectAlwaysAllowed ? 'redirects always allowed' : false); test('too many redirects', () async { From 40d572232c297929257fad351efd05d7b594ddf4 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 13:56:21 -0800 Subject: [PATCH 3/9] Make tests pass --- pkgs/http/lib/http.dart | 2 +- pkgs/http/lib/src/base_request.dart | 26 +++++++++++++++++------- pkgs/http/lib/src/browser_client.dart | 13 +----------- pkgs/http/lib/src/streamed_response.dart | 16 +++++++++++++++ pkgs/http/test/io/request_test.dart | 6 +++--- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index 62004240c7..404f3d2e2f 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -25,7 +25,7 @@ export 'src/multipart_request.dart'; export 'src/request.dart'; export 'src/response.dart'; export 'src/streamed_request.dart'; -export 'src/streamed_response.dart'; +export 'src/streamed_response.dart' hide StreamedResponseV2; /// Sends an HTTP HEAD request with the given headers to the given URL. /// diff --git a/pkgs/http/lib/src/base_request.dart b/pkgs/http/lib/src/base_request.dart index 70a78695aa..ece1541783 100644 --- a/pkgs/http/lib/src/base_request.dart +++ b/pkgs/http/lib/src/base_request.dart @@ -132,13 +132,25 @@ abstract class BaseRequest { try { var response = await client.send(this); var stream = onDone(response.stream, client.close); - return StreamedResponse(ByteStream(stream), response.statusCode, - contentLength: response.contentLength, - request: response.request, - headers: response.headers, - isRedirect: response.isRedirect, - persistentConnection: response.persistentConnection, - reasonPhrase: response.reasonPhrase); + + if (response is BaseResponseV2) { + return StreamedResponseV2(ByteStream(stream), response.statusCode, + contentLength: response.contentLength, + request: response.request, + headers: response.headers, + isRedirect: response.isRedirect, + url: (response as BaseResponseV2).url, + persistentConnection: response.persistentConnection, + reasonPhrase: response.reasonPhrase); + } else { + return StreamedResponse(ByteStream(stream), response.statusCode, + contentLength: response.contentLength, + request: response.request, + headers: response.headers, + isRedirect: response.isRedirect, + persistentConnection: response.persistentConnection, + reasonPhrase: response.reasonPhrase); + } } catch (_) { client.close(); rethrow; diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 0b0a5d365f..48849d7b9d 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -27,17 +27,6 @@ BaseClient createClient() { return BrowserClient(); } -class _StreamedResponseV2 extends StreamedResponse with BaseResponseV2 { - @override - final Uri? url; - _StreamedResponseV2(super.stream, super.statusCode, - {super.contentLength, - super.request, - super.headers, - this.url, - super.reasonPhrase}); -} - /// A `package:web`-based HTTP client that runs in the browser and is backed by /// [XMLHttpRequest]. /// @@ -93,7 +82,7 @@ class BrowserClient extends BaseClient { var body = (xhr.response as JSArrayBuffer).toDart.asUint8List(); var responseUrl = xhr.responseURL; var url = responseUrl.isNotEmpty ? Uri.parse(responseUrl) : null; - completer.complete(_StreamedResponseV2( + completer.complete(StreamedResponseV2( ByteStream.fromBytes(body), xhr.status, contentLength: body.length, request: request, diff --git a/pkgs/http/lib/src/streamed_response.dart b/pkgs/http/lib/src/streamed_response.dart index 8cc0c76f75..2d6f61ff35 100644 --- a/pkgs/http/lib/src/streamed_response.dart +++ b/pkgs/http/lib/src/streamed_response.dart @@ -26,3 +26,19 @@ class StreamedResponse extends BaseResponse { super.reasonPhrase}) : stream = toByteStream(stream); } + +/// This class is private to `package:http` and will be removed when +/// `package:http` v2 is released. +class StreamedResponseV2 extends StreamedResponse with BaseResponseV2 { + @override + final Uri? url; + + StreamedResponseV2(super.stream, super.statusCode, + {super.contentLength, + super.request, + super.headers, + super.isRedirect, + this.url, + super.persistentConnection, + super.reasonPhrase}); +} diff --git a/pkgs/http/test/io/request_test.dart b/pkgs/http/test/io/request_test.dart index ee95fb45be..9d5c0b6d04 100644 --- a/pkgs/http/test/io/request_test.dart +++ b/pkgs/http/test/io/request_test.dart @@ -47,17 +47,17 @@ void main() { final response = await request.send(); expect(response.statusCode, equals(302)); - expect((response as BaseResponseV2).url, serverUrl.resolve('/redirect')); + expect( + (response as http.BaseResponseV2).url, serverUrl.resolve('/redirect')); }); test('with redirects', () async { final request = http.Request('GET', serverUrl.resolve('/redirect')); final response = await request.send(); - expect(response.statusCode, equals(200)); final bytesString = await response.stream.bytesToString(); expect(bytesString, parse(containsPair('path', '/'))); - expect((response as BaseResponseV2).url, serverUrl.resolve('/')); + expect((response as http.BaseResponseV2).url, serverUrl.resolve('/')); }); test('exceeding max redirects', () async { From 4ddd4b095fbde169503da99ae41b211d2b7e524a Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 13:59:35 -0800 Subject: [PATCH 4/9] Remove extra imports --- pkgs/http/lib/src/browser_client.dart | 1 - pkgs/http/test/io/request_test.dart | 1 - 2 files changed, 2 deletions(-) diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index 48849d7b9d..b61ae5d0ed 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -9,7 +9,6 @@ import 'package:web/helpers.dart'; import 'base_client.dart'; import 'base_request.dart'; -import 'base_response.dart'; import 'byte_stream.dart'; import 'exception.dart'; import 'streamed_response.dart'; diff --git a/pkgs/http/test/io/request_test.dart b/pkgs/http/test/io/request_test.dart index 9d5c0b6d04..b07a0f4898 100644 --- a/pkgs/http/test/io/request_test.dart +++ b/pkgs/http/test/io/request_test.dart @@ -6,7 +6,6 @@ library; import 'package:http/http.dart' as http; -import 'package:http/src/base_response.dart'; import 'package:test/test.dart'; import '../utils.dart'; From b8bed260d7e0779fc87f4757ab4ee28479fe7e53 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 18:22:58 -0800 Subject: [PATCH 5/9] s/BaseResposeV2/BaseResponseWithUrl --- pkgs/http/CHANGELOG.md | 2 +- pkgs/http/lib/src/base_request.dart | 4 ++-- pkgs/http/lib/src/base_response.dart | 14 ++++++++------ pkgs/http/lib/src/io_client.dart | 3 ++- pkgs/http/lib/src/streamed_response.dart | 2 +- pkgs/http/test/io/request_test.dart | 6 +++--- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index c2987adc38..a52b584c0e 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,7 +1,7 @@ ## 1.2.0 * Add `MockClient.pngResponse`, which makes it easier to fake image responses. -* Added the ability to fetch the URL of the response through `BaseResponseV2`. +* Added the ability to fetch the URL of the response through `BaseResponseWithUrl`. ## 1.1.2 diff --git a/pkgs/http/lib/src/base_request.dart b/pkgs/http/lib/src/base_request.dart index ece1541783..3125178a93 100644 --- a/pkgs/http/lib/src/base_request.dart +++ b/pkgs/http/lib/src/base_request.dart @@ -133,13 +133,13 @@ abstract class BaseRequest { var response = await client.send(this); var stream = onDone(response.stream, client.close); - if (response is BaseResponseV2) { + if (response is BaseResponseWithUrl) { return StreamedResponseV2(ByteStream(stream), response.statusCode, contentLength: response.contentLength, request: response.request, headers: response.headers, isRedirect: response.isRedirect, - url: (response as BaseResponseV2).url, + url: (response as BaseResponseWithUrl).url, persistentConnection: response.persistentConnection, reasonPhrase: response.reasonPhrase); } else { diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index 89d66e80f1..0e7a0ee9a3 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -72,11 +72,10 @@ abstract class BaseResponse { } } -/// Fields and methods that will be added to [BaseResponse] when `package:http` -/// version 2 is released. +/// A [BaseResponse] with a [url] field. /// /// [Client] methods that return a [BaseResponse] subclass, such as [Response] -/// or [StreamedResponse], **may** return a [BaseResponseV2]. +/// or [StreamedResponse], **may** return a [BaseResponseWithUrl]. /// /// For example: /// @@ -84,13 +83,16 @@ abstract class BaseResponse { /// final client = Client(); /// final response = client.get(Uri.https('example.com', '/')); /// Uri? finalUri; -/// if (response is BaseResponseV2) { -/// finalUri = (response as BaseResponseV2).uri; +/// if (response is BaseResponseWithUrl) { +/// finalUri = (response as BaseResponseWithUrl).url; /// } /// // Do something with `finalUri`. /// client.close(); /// ``` -mixin BaseResponseV2 on BaseResponse { +/// +/// [url] will be added to [BaseResponse] when `package:http` version 2 is +/// released and this mixin will be deprecated. +mixin BaseResponseWithUrl on BaseResponse { /// The [Uri] of the response returned by the server. /// /// If no redirects were followed, [url] will be the same as the requested diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 70164090e9..ef9d7318f8 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -47,7 +47,8 @@ class _ClientSocketException extends ClientException String toString() => 'ClientException with $cause, uri=$uri'; } -class _IOStreamedResponseV2 extends IOStreamedResponse with BaseResponseV2 { +class _IOStreamedResponseV2 extends IOStreamedResponse + with BaseResponseWithUrl { @override final Uri? url; diff --git a/pkgs/http/lib/src/streamed_response.dart b/pkgs/http/lib/src/streamed_response.dart index 2d6f61ff35..4b81f67fb4 100644 --- a/pkgs/http/lib/src/streamed_response.dart +++ b/pkgs/http/lib/src/streamed_response.dart @@ -29,7 +29,7 @@ class StreamedResponse extends BaseResponse { /// This class is private to `package:http` and will be removed when /// `package:http` v2 is released. -class StreamedResponseV2 extends StreamedResponse with BaseResponseV2 { +class StreamedResponseV2 extends StreamedResponse with BaseResponseWithUrl { @override final Uri? url; diff --git a/pkgs/http/test/io/request_test.dart b/pkgs/http/test/io/request_test.dart index b07a0f4898..90caaaa718 100644 --- a/pkgs/http/test/io/request_test.dart +++ b/pkgs/http/test/io/request_test.dart @@ -46,8 +46,8 @@ void main() { final response = await request.send(); expect(response.statusCode, equals(302)); - expect( - (response as http.BaseResponseV2).url, serverUrl.resolve('/redirect')); + expect((response as http.BaseResponseWithUrl).url, + serverUrl.resolve('/redirect')); }); test('with redirects', () async { @@ -56,7 +56,7 @@ void main() { expect(response.statusCode, equals(200)); final bytesString = await response.stream.bytesToString(); expect(bytesString, parse(containsPair('path', '/'))); - expect((response as http.BaseResponseV2).url, serverUrl.resolve('/')); + expect((response as http.BaseResponseWithUrl).url, serverUrl.resolve('/')); }); test('exceeding max redirects', () async { From 35dfa26888dfc18cbd258eea44b637e648516260 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 18:36:44 -0800 Subject: [PATCH 6/9] Review updates --- pkgs/http/lib/http.dart | 2 +- pkgs/http/lib/src/base_request.dart | 4 ++-- pkgs/http/lib/src/base_response.dart | 4 +--- pkgs/http/lib/src/browser_client.dart | 2 +- pkgs/http/lib/src/io_client.dart | 4 ++-- pkgs/http/lib/src/streamed_response.dart | 4 ++-- pkgs/http/test/io/request_test.dart | 11 ++++++++--- 7 files changed, 17 insertions(+), 14 deletions(-) diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index 404f3d2e2f..c9d9d7e815 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -25,7 +25,7 @@ export 'src/multipart_request.dart'; export 'src/request.dart'; export 'src/response.dart'; export 'src/streamed_request.dart'; -export 'src/streamed_response.dart' hide StreamedResponseV2; +export 'src/streamed_response.dart' show StreamedResponse; /// Sends an HTTP HEAD request with the given headers to the given URL. /// diff --git a/pkgs/http/lib/src/base_request.dart b/pkgs/http/lib/src/base_request.dart index 3125178a93..4b165c7587 100644 --- a/pkgs/http/lib/src/base_request.dart +++ b/pkgs/http/lib/src/base_request.dart @@ -133,13 +133,13 @@ abstract class BaseRequest { var response = await client.send(this); var stream = onDone(response.stream, client.close); - if (response is BaseResponseWithUrl) { + if (response case BaseResponseWithUrl(:final url)) { return StreamedResponseV2(ByteStream(stream), response.statusCode, contentLength: response.contentLength, request: response.request, headers: response.headers, isRedirect: response.isRedirect, - url: (response as BaseResponseWithUrl).url, + url: url, persistentConnection: response.persistentConnection, reasonPhrase: response.reasonPhrase); } else { diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index 0e7a0ee9a3..967c688903 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -100,7 +100,5 @@ mixin BaseResponseWithUrl on BaseResponse { /// /// If redirects were followed, [url] will be the [Uri] of the last redirect /// that was followed. - /// - /// May be `null` when the response is for a special URL such as "about:". - abstract final Uri? url; + abstract final Uri url; } diff --git a/pkgs/http/lib/src/browser_client.dart b/pkgs/http/lib/src/browser_client.dart index b61ae5d0ed..cbbada65f8 100644 --- a/pkgs/http/lib/src/browser_client.dart +++ b/pkgs/http/lib/src/browser_client.dart @@ -80,7 +80,7 @@ class BrowserClient extends BaseClient { } var body = (xhr.response as JSArrayBuffer).toDart.asUint8List(); var responseUrl = xhr.responseURL; - var url = responseUrl.isNotEmpty ? Uri.parse(responseUrl) : null; + var url = responseUrl.isNotEmpty ? Uri.parse(responseUrl) : request.url; completer.complete(StreamedResponseV2( ByteStream.fromBytes(body), xhr.status, contentLength: body.length, diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index ef9d7318f8..2e7d9a7059 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -50,14 +50,14 @@ class _ClientSocketException extends ClientException class _IOStreamedResponseV2 extends IOStreamedResponse with BaseResponseWithUrl { @override - final Uri? url; + final Uri url; _IOStreamedResponseV2(super.stream, super.statusCode, {super.contentLength, super.request, super.headers, super.isRedirect, - this.url, + required this.url, super.persistentConnection, super.reasonPhrase, super.inner}); diff --git a/pkgs/http/lib/src/streamed_response.dart b/pkgs/http/lib/src/streamed_response.dart index 4b81f67fb4..e7aeb8e47c 100644 --- a/pkgs/http/lib/src/streamed_response.dart +++ b/pkgs/http/lib/src/streamed_response.dart @@ -31,14 +31,14 @@ class StreamedResponse extends BaseResponse { /// `package:http` v2 is released. class StreamedResponseV2 extends StreamedResponse with BaseResponseWithUrl { @override - final Uri? url; + final Uri url; StreamedResponseV2(super.stream, super.statusCode, {super.contentLength, super.request, super.headers, super.isRedirect, - this.url, + required this.url, super.persistentConnection, super.reasonPhrase}); } diff --git a/pkgs/http/test/io/request_test.dart b/pkgs/http/test/io/request_test.dart index 90caaaa718..226781fbeb 100644 --- a/pkgs/http/test/io/request_test.dart +++ b/pkgs/http/test/io/request_test.dart @@ -46,8 +46,10 @@ void main() { final response = await request.send(); expect(response.statusCode, equals(302)); - expect((response as http.BaseResponseWithUrl).url, - serverUrl.resolve('/redirect')); + expect( + response, + isA() + .having((r) => r.url, 'url', serverUrl.resolve('/redirect'))); }); test('with redirects', () async { @@ -56,7 +58,10 @@ void main() { expect(response.statusCode, equals(200)); final bytesString = await response.stream.bytesToString(); expect(bytesString, parse(containsPair('path', '/'))); - expect((response as http.BaseResponseWithUrl).url, serverUrl.resolve('/')); + expect( + response, + isA() + .having((r) => r.url, 'url', serverUrl.resolve('/'))); }); test('exceeding max redirects', () async { From fdb860590e78d116f7189521dfc4638319d9168c Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 9 Jan 2024 18:41:07 -0800 Subject: [PATCH 7/9] Update base_response.dart --- pkgs/http/lib/src/base_response.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index 967c688903..66b7749e4e 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -83,8 +83,8 @@ abstract class BaseResponse { /// final client = Client(); /// final response = client.get(Uri.https('example.com', '/')); /// Uri? finalUri; -/// if (response is BaseResponseWithUrl) { -/// finalUri = (response as BaseResponseWithUrl).url; +/// if (response case BaseResponseWithUrl(:final url)) { +/// finalUri = url; /// } /// // Do something with `finalUri`. /// client.close(); From 58a8c6e009b0cc5b138b2f870c40e2764b040175 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 16 Jan 2024 14:32:21 -0800 Subject: [PATCH 8/9] Review fixes --- pkgs/http/lib/src/base_response.dart | 2 +- pkgs/http/lib/src/io_client.dart | 6 +++--- pkgs/http/lib/src/streamed_response.dart | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index 66b7749e4e..4b1382bbb0 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -92,7 +92,7 @@ abstract class BaseResponse { /// /// [url] will be added to [BaseResponse] when `package:http` version 2 is /// released and this mixin will be deprecated. -mixin BaseResponseWithUrl on BaseResponse { +abstract interface class BaseResponseWithUrl implements BaseResponse { /// The [Uri] of the response returned by the server. /// /// If no redirects were followed, [url] will be the same as the requested diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index 2e7d9a7059..fe4834bb24 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -48,16 +48,16 @@ class _ClientSocketException extends ClientException } class _IOStreamedResponseV2 extends IOStreamedResponse - with BaseResponseWithUrl { + implements BaseResponseWithUrl { @override final Uri url; _IOStreamedResponseV2(super.stream, super.statusCode, - {super.contentLength, + {required this.url, + super.contentLength, super.request, super.headers, super.isRedirect, - required this.url, super.persistentConnection, super.reasonPhrase, super.inner}); diff --git a/pkgs/http/lib/src/streamed_response.dart b/pkgs/http/lib/src/streamed_response.dart index e7aeb8e47c..44389d7061 100644 --- a/pkgs/http/lib/src/streamed_response.dart +++ b/pkgs/http/lib/src/streamed_response.dart @@ -29,16 +29,17 @@ class StreamedResponse extends BaseResponse { /// This class is private to `package:http` and will be removed when /// `package:http` v2 is released. -class StreamedResponseV2 extends StreamedResponse with BaseResponseWithUrl { +class StreamedResponseV2 extends StreamedResponse + implements BaseResponseWithUrl { @override final Uri url; StreamedResponseV2(super.stream, super.statusCode, - {super.contentLength, + {required this.url, + super.contentLength, super.request, super.headers, super.isRedirect, - required this.url, super.persistentConnection, super.reasonPhrase}); } From 7b49498545d4e7daef0fcb4a3b3751a5faed8199 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 16 Jan 2024 14:35:35 -0800 Subject: [PATCH 9/9] Show base response --- pkgs/http/lib/http.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index 11bb388af8..da35b23a87 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -16,7 +16,8 @@ import 'src/streamed_request.dart'; export 'src/base_client.dart'; export 'src/base_request.dart'; -export 'src/base_response.dart' show BaseResponse, HeadersWithSplitValues; +export 'src/base_response.dart' + show BaseResponse, BaseResponseWithUrl, HeadersWithSplitValues; export 'src/byte_stream.dart'; export 'src/client.dart' hide zoneClient; export 'src/exception.dart';