Skip to content

Commit

Permalink
Add tests, readme, changelog
Browse files Browse the repository at this point in the history
  • Loading branch information
brianquinlan committed Feb 20, 2024
1 parent 8c020b3 commit a81ecf6
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 12 deletions.
5 changes: 5 additions & 0 deletions pkgs/cronet_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.1.1

* Make it possible to construct `CronetClient` with custom a `CronetEngine`
while still allowing `CronetClient` to close the `CronetEngine`.

## 1.1.0

* Use `package:http_image_provider` in the example application.
Expand Down
3 changes: 2 additions & 1 deletion pkgs/cronet_http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void main() async {
cacheMode: CacheMode.memory,
cacheMaxSize: 2 * 1024 * 1024,
userAgent: 'Book Agent');
httpClient = CronetClient.fromCronetEngine(engine);
httpClient = CronetClient.fromCronetEngine(engine, isOwned: true);
} else {
httpClient = IOClient(HttpClient()..userAgent = 'Book Agent');
}
Expand All @@ -52,6 +52,7 @@ void main() async {
'www.googleapis.com',
'/books/v1/volumes',
{'q': 'HTTP', 'maxResults': '40', 'printType': 'books'}));
httpClient.close();
}
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:io';

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

Expand Down Expand Up @@ -120,10 +121,39 @@ void testUserAgent() {
});
}

void testEngineClose() {
group('engine close', () {
test('multiple close', () {
CronetEngine.build()
..close()
..close();
});

test('request after close', () async {
final closedEngine = CronetEngine.build()..close();
final client = CronetClient.fromCronetEngine(closedEngine);
await expectLater(() => client.get(Uri.https('example.com', '/')),
throwsA(isA<ClientException>()));
});

test('engine owned close', () {
final engine = CronetEngine.build();
CronetClient.fromCronetEngine(engine, isOwned: true).close();
});

test('engine not owned close', () {
final engine = CronetEngine.build();
CronetClient.fromCronetEngine(engine, isOwned: false).close();
engine.close();
});
});
}

void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

testCache();
testInvalidConfigurations();
testUserAgent();
testEngineClose();
}
2 changes: 1 addition & 1 deletion pkgs/cronet_http/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void main() {
cacheMode: CacheMode.memory,
cacheMaxSize: 2 * 1024 * 1024,
userAgent: 'Book Agent');
httpClient = CronetClient.fromCronetEngine(engine);
httpClient = CronetClient.fromCronetEngine(engine, isOwned: true);
} else {
httpClient = IOClient(HttpClient()..userAgent = 'Book Agent');
}
Expand Down
29 changes: 19 additions & 10 deletions pkgs/cronet_http/lib/src/cronet_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ class CronetEngine {

void close() {
if (!_isClosed) {
_engine.shutdown();
_engine.release();
_engine
..shutdown()
..release();
}
_isClosed = true;
}
Expand Down Expand Up @@ -280,9 +281,7 @@ class CronetClient extends BaseClient {
CronetEngine? _engine;
bool _isClosed = false;

/// 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.
/// Indicates that [CronetClient] is responsible for closing [_engine].
final bool _ownedEngine;

CronetClient._(this._engine, this._ownedEngine) {
Expand All @@ -293,8 +292,13 @@ class CronetClient extends BaseClient {
factory CronetClient.defaultCronetEngine() => CronetClient._(null, true);

/// A [CronetClient] configured with a [CronetEngine].
factory CronetClient.fromCronetEngine(CronetEngine engine) =>
CronetClient._(engine, false);
///
/// If [isOwned] is `true`, then [engine] will be closed when [close] is
/// called. This can simplify lifetime management if [engine] is only used
/// in one [CronetClient].
factory CronetClient.fromCronetEngine(CronetEngine engine,
{bool isOwned = false}) =>
CronetClient._(engine, isOwned);

/// A [CronetClient] configured with a [Future] containing a [CronetEngine].
///
Expand Down Expand Up @@ -327,14 +331,19 @@ class CronetClient extends BaseClient {
'HTTP request failed. Client is already closed.', request.url);
}

_engine ??= CronetEngine.build();
final engine = _engine ?? CronetEngine.build();
_engine = engine;

if (engine._isClosed) {
throw ClientException(
'HTTP request failed. CronetEngine is already closed.', request.url);
}

final stream = request.finalize();
final body = await stream.toBytes();
final responseCompleter = Completer<StreamedResponse>();
final engine = _engine!._engine;

final builder = engine.newUrlRequestBuilder(
final builder = engine._engine.newUrlRequestBuilder(
request.url.toString().toJString(),
jb.UrlRequestCallbackProxy.new1(
_urlRequestCallbacks(request, responseCompleter)),
Expand Down

0 comments on commit a81ecf6

Please sign in to comment.