Skip to content

Commit

Permalink
Make it possible to use a custom CronetEngine with runWithClient (#843)
Browse files Browse the repository at this point in the history
  • Loading branch information
brianquinlan authored Jan 3, 2023
1 parent 38d5dd9 commit d434d42
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 40 deletions.
6 changes: 6 additions & 0 deletions pkgs/cronet_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.1.0

* Add a CronetClient that accepts a `Future<CronetEngine>`.
* Modify the example application to create a `CronetClient` using a
`Future<CronetEngine>`.

## 0.0.4

* Fix a bug where the example would not use the configured `package:http`
Expand Down

This file was deleted.

68 changes: 68 additions & 0 deletions pkgs/cronet_http/example/integration_test/client_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:cronet_http/cronet_client.dart';
import 'package:http/http.dart';
import 'package:http_client_conformance_tests/http_client_conformance_tests.dart';
import 'package:integration_test/integration_test.dart';
import 'package:test/test.dart';

void testClientConformance(CronetClient Function() clientFactory) {
// TODO: Use `testAll` when `testServerErrors` passes i.e.
// testAll(CronetClient(), canStreamRequestBody: false);

final client = clientFactory();
testRequestBody(client);
testRequestBodyStreamed(client, canStreamRequestBody: false);
testResponseBody(client);
testResponseBodyStreamed(client);
testRequestHeaders(client);
testResponseHeaders(client);
testRedirect(client);
testCompressedResponseBody(client);
testMultipleClients(clientFactory);
}

Future<void> testConformance() async {
group('default cronet engine',
() => testClientConformance(CronetClient.defaultCronetEngine));

final engine = await CronetEngine.build(
cacheMode: CacheMode.disabled, userAgent: 'Test Agent (Engine)');

group('from cronet engine', () {
testClientConformance(() => CronetClient.fromCronetEngine(engine));
});

group('from cronet engine future', () {
final engineFuture = CronetEngine.build(
cacheMode: CacheMode.disabled, userAgent: 'Test Agent (Future)');
testClientConformance(
() => CronetClient.fromCronetEngineFuture(engineFuture));
});
}

Future<void> testClientFromFutureFails() async {
test('cronet engine future fails', () async {
final engineFuture = CronetEngine.build(
cacheMode: CacheMode.disk,
storagePath: '/non-existant-path/', // Will cause `build` to throw.
userAgent: 'Test Agent (Future)');

final client = CronetClient.fromCronetEngineFuture(engineFuture);
await expectLater(
client.get(Uri.http('example.com', '/')),
throwsA((Exception e) =>
e is ClientException &&
e.message.contains('Exception building CronetEngine: '
'Invalid argument(s): Storage path must')));
});
}

void main() async {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

await testConformance();
await testClientFromFutureFails();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void testCache() {

test('disabled', () async {
final engine = await CronetEngine.build(cacheMode: CacheMode.disabled);
final client = CronetClient(engine);
final client = CronetClient.fromCronetEngine(engine);
await client.get(Uri.parse('http://localhost:${server.port}'));
await client.get(Uri.parse('http://localhost:${server.port}'));
expect(numRequests, 2);
Expand All @@ -43,7 +43,7 @@ void testCache() {
test('memory', () async {
final engine = await CronetEngine.build(
cacheMode: CacheMode.memory, cacheMaxSize: 1024 * 1024);
final client = CronetClient(engine);
final client = CronetClient.fromCronetEngine(engine);
await client.get(Uri.parse('http://localhost:${server.port}'));
await client.get(Uri.parse('http://localhost:${server.port}'));
expect(numRequests, 1);
Expand All @@ -54,7 +54,7 @@ void testCache() {
cacheMode: CacheMode.disk,
cacheMaxSize: 1024 * 1024,
storagePath: (await Directory.systemTemp.createTemp()).absolute.path);
final client = CronetClient(engine);
final client = CronetClient.fromCronetEngine(engine);
await client.get(Uri.parse('http://localhost:${server.port}'));
await client.get(Uri.parse('http://localhost:${server.port}'));
expect(numRequests, 1);
Expand All @@ -66,7 +66,7 @@ void testCache() {
cacheMaxSize: 1024 * 1024,
storagePath: (await Directory.systemTemp.createTemp()).absolute.path);

final client = CronetClient(engine);
final client = CronetClient.fromCronetEngine(engine);
await client.get(Uri.parse('http://localhost:${server.port}'));
await client.get(Uri.parse('http://localhost:${server.port}'));
expect(numRequests, 2);
Expand Down Expand Up @@ -115,7 +115,7 @@ void testUserAgent() {

test('userAgent', () async {
final engine = await CronetEngine.build(userAgent: 'fake-agent');
await CronetClient(engine)
await CronetClient.fromCronetEngine(engine)
.get(Uri.parse('http://localhost:${server.port}'));
expect(requestHeaders['user-agent'], ['fake-agent']);
});
Expand Down
7 changes: 6 additions & 1 deletion pkgs/cronet_http/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import 'book.dart';
void main() {
var clientFactory = Client.new; // Constructs the default client.
if (Platform.isAndroid) {
clientFactory = CronetClient.new;
Future<CronetEngine>? engine;
clientFactory = () {
engine ??= CronetEngine.build(
cacheMode: CacheMode.memory, userAgent: 'Book Agent');
return CronetClient.fromCronetEngineFuture(engine!);
};
}
runWithClient(() => runApp(const BookSearchApp()), clientFactory);
}
Expand Down
68 changes: 60 additions & 8 deletions pkgs/cronet_http/lib/cronet_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

/// An Android Flutter plugin that provides access to the
/// [Cronet](https://developer.android.com/guide/topics/connectivity/cronet/reference/org/chromium/net/package-summary)
/// HTTP client.
///
/// The platform interface must be initialized before using this plugin e.g. by
/// calling
/// [`WidgetsFlutterBinding.ensureInitialized`](https://api.flutter.dev/flutter/widgets/WidgetsFlutterBinding/ensureInitialized.html)
/// or
/// [`runApp`](https://api.flutter.dev/flutter/widgets/runApp.html).
import 'dart:async';

import 'package:flutter/services.dart';
Expand Down Expand Up @@ -123,11 +133,40 @@ class CronetEngine {
/// ```
class CronetClient extends BaseClient {
CronetEngine? _engine;
Future<CronetEngine>? _engineFuture;

/// Indicates that [_engine] was constructed as an implementation detail for
/// this [CronetClient] (i.e. was not provided as a constructor argument) and
/// should be closed when this [CronetClient] is closed.
final bool _ownedEngine;

CronetClient([CronetEngine? engine])
: _engine = engine,
_ownedEngine = engine == null;
CronetClient._(this._engineFuture, this._ownedEngine);

/// A [CronetClient] that will be initialized with a new [CronetEngine].
factory CronetClient.defaultCronetEngine() => CronetClient._(null, true);

/// A [CronetClient] configured with a [CronetEngine].
factory CronetClient.fromCronetEngine(CronetEngine engine) =>
CronetClient._(Future.value(engine), false);

/// A [CronetClient] configured with a [Future] containing a [CronetEngine].
///
/// This can be useful in circumstances where a non-Future [CronetClient] is
/// required but you want to configure the [CronetClient] with a custom
/// [CronetEngine]. For example:
/// ```
/// void main() {
/// Client clientFactory() {
/// final engine = CronetEngine.build(
/// cacheMode: CacheMode.memory, userAgent: 'Book Agent');
/// return CronetClient.fromCronetEngineFuture(engine);
/// }
///
/// runWithClient(() => runApp(const BookSearchApp()), clientFactory);
/// }
/// ```
factory CronetClient.fromCronetEngineFuture(Future<CronetEngine> engine) =>
CronetClient._(engine, false);

@override
void close() {
Expand All @@ -138,11 +177,23 @@ class CronetClient extends BaseClient {

@override
Future<StreamedResponse> send(BaseRequest request) async {
try {
_engine ??= await CronetEngine.build();
} catch (e) {
throw ClientException(e.toString(), request.url);
if (_engine == null) {
// Create the future here rather than in the [fromCronetEngineFuture]
// factory so that [close] does not have to await the future just to
// close it in the case where [send] is never called.
//
// Assign to _engineFuture instead of just `await`ing the result of
// `CronetEngine.build()` to prevent concurrent executions of `send`
// from creating multiple [CronetEngine]s.
_engineFuture ??= CronetEngine.build();
try {
_engine = await _engineFuture;
} catch (e) {
throw ClientException(
'Exception building CronetEngine: ${e.toString()}', request.url);
}
}

final stream = request.finalize();

final body = await stream.toBytes();
Expand Down Expand Up @@ -203,7 +254,8 @@ class CronetClient extends BaseClient {
});

final result = await responseCompleter.future;
final responseHeaders = (result.headers.cast<String, List<Object?>>())
final responseHeaders = result.headers
.cast<String, List<Object?>>()
.map((key, value) => MapEntry(key.toLowerCase(), value.join(',')));

final contentLengthHeader = responseHeaders['content-length'];
Expand Down
2 changes: 1 addition & 1 deletion pkgs/cronet_http/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: cronet_http
description: >
An Android Flutter plugin that provides access to the Cronet HTTP client.
version: 0.0.3
version: 0.1.0
repository: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http

environment:
Expand Down

0 comments on commit d434d42

Please sign in to comment.