From d7e59a10cead45d6ccd53ee28f84a578ed046858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asc=C3=AAnio?= Date: Wed, 12 May 2021 12:15:38 -0300 Subject: [PATCH] Migrates to null safety (#22) --- packages/flutter_image/CHANGELOG.md | 5 + packages/flutter_image/lib/network.dart | 171 ++++++------- packages/flutter_image/pubspec.yaml | 6 +- packages/flutter_image/test/network_test.dart | 226 +++++++++--------- 4 files changed, 217 insertions(+), 191 deletions(-) diff --git a/packages/flutter_image/CHANGELOG.md b/packages/flutter_image/CHANGELOG.md index cfb10e551e..6d9ac91919 100644 --- a/packages/flutter_image/CHANGELOG.md +++ b/packages/flutter_image/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 4.0.0 + +- Migrates to null safety +- **Breaking change**: `NetworkImageWithRetry.load` now throws a `FetchFailure` if the fetched image data is zero bytes. + ## 3.0.0 * **Breaking change**. Updates for Flutter 1.10.15. diff --git a/packages/flutter_image/lib/network.dart b/packages/flutter_image/lib/network.dart index 3f91fcba8c..41db7c3423 100644 --- a/packages/flutter_image/lib/network.dart +++ b/packages/flutter_image/lib/network.dart @@ -29,10 +29,11 @@ class NetworkImageWithRetry extends ImageProvider { /// Creates an object that fetches the image at the given [url]. /// /// The arguments must not be null. - const NetworkImageWithRetry(this.url, { this.scale = 1.0, this.fetchStrategy = defaultFetchStrategy }) - : assert(url != null), - assert(scale != null), - assert(fetchStrategy != null); + const NetworkImageWithRetry( + this.url, { + this.scale = 1.0, + this.fetchStrategy = defaultFetchStrategy, + }); /// The HTTP client used to download images. static final io.HttpClient _client = io.HttpClient(); @@ -57,10 +58,12 @@ class NetworkImageWithRetry extends ImageProvider { /// This indirection is necessary because [defaultFetchStrategy] is used as /// the default constructor argument value, which requires that it be a const /// expression. - static final FetchStrategy _defaultFetchStrategyFunction = const FetchStrategyBuilder().build(); + static final FetchStrategy _defaultFetchStrategyFunction = + const FetchStrategyBuilder().build(); /// The [FetchStrategy] that [NetworkImageWithRetry] uses by default. - static Future defaultFetchStrategy(Uri uri, FetchFailure failure) { + static Future defaultFetchStrategy( + Uri uri, FetchFailure? failure) { return _defaultFetchStrategyFunction(uri, failure); } @@ -71,37 +74,34 @@ class NetworkImageWithRetry extends ImageProvider { @override ImageStreamCompleter load(NetworkImageWithRetry key, DecoderCallback decode) { - return OneFrameImageStreamCompleter( - _loadWithRetry(key, decode), + return OneFrameImageStreamCompleter(_loadWithRetry(key, decode), informationCollector: () sync* { - yield ErrorDescription('Image provider: $this'); - yield ErrorDescription('Image key: $key'); - } - ); + yield ErrorDescription('Image provider: $this'); + yield ErrorDescription('Image key: $key'); + }); } - void _debugCheckInstructions(FetchInstructions instructions) { + void _debugCheckInstructions(FetchInstructions? instructions) { assert(() { if (instructions == null) { if (fetchStrategy == defaultFetchStrategy) { throw StateError( - 'The default FetchStrategy returned null FetchInstructions. This\n' - 'is likely a bug in $runtimeType. Please file a bug at\n' - 'https://github.com/flutter/flutter/issues.' - ); + 'The default FetchStrategy returned null FetchInstructions. This\n' + 'is likely a bug in $runtimeType. Please file a bug at\n' + 'https://github.com/flutter/flutter/issues.'); } else { throw StateError( - 'The custom FetchStrategy used to fetch $url returned null\n' - 'FetchInstructions. FetchInstructions must never be null, but\n' - 'instead instruct to either make another fetch attempt or give up.' - ); + 'The custom FetchStrategy used to fetch $url returned null\n' + 'FetchInstructions. FetchInstructions must never be null, but\n' + 'instead instruct to either make another fetch attempt or give up.'); } } return true; }()); } - Future _loadWithRetry(NetworkImageWithRetry key, DecoderCallback decode) async { + Future _loadWithRetry( + NetworkImageWithRetry key, DecoderCallback decode) async { assert(key == this); final Stopwatch stopwatch = Stopwatch()..start(); @@ -109,16 +109,19 @@ class NetworkImageWithRetry extends ImageProvider { FetchInstructions instructions = await fetchStrategy(resolved, null); _debugCheckInstructions(instructions); int attemptCount = 0; - FetchFailure lastFailure; + FetchFailure? lastFailure; while (!instructions.shouldGiveUp) { attemptCount += 1; - io.HttpClientRequest request; + io.HttpClientRequest? request; try { - request = await _client.getUrl(instructions.uri).timeout(instructions.timeout); - final io.HttpClientResponse response = await request.close().timeout(instructions.timeout); + request = await _client + .getUrl(instructions.uri) + .timeout(instructions.timeout); + final io.HttpClientResponse response = + await request.close().timeout(instructions.timeout); - if (response == null || response.statusCode != 200) { + if (response.statusCode != 200) { throw FetchFailure._( totalDuration: stopwatch.elapsed, attemptCount: attemptCount, @@ -126,21 +129,25 @@ class NetworkImageWithRetry extends ImageProvider { ); } - final _Uint8ListBuilder builder = await response.fold( - _Uint8ListBuilder(), + final _Uint8ListBuilder builder = await response + .fold( + _Uint8ListBuilder(), (_Uint8ListBuilder buffer, List bytes) => buffer..add(bytes), - ).timeout(instructions.timeout); + ) + .timeout(instructions.timeout); final Uint8List bytes = builder.data; - if (bytes.lengthInBytes == 0) - return null; + if (bytes.lengthInBytes == 0) { + throw FetchFailure._( + totalDuration: stopwatch.elapsed, + attemptCount: attemptCount, + httpStatusCode: response.statusCode, + ); + } final ui.Codec codec = await decode(bytes); final ui.Image image = (await codec.getNextFrame()).image; - if (image == null) - return null; - return ImageInfo( image: image, scale: key.scale, @@ -159,27 +166,31 @@ class NetworkImageWithRetry extends ImageProvider { } } - if (instructions.alternativeImage != null) - return instructions.alternativeImage; + if (instructions.alternativeImage != null) { + return instructions.alternativeImage!; + } assert(lastFailure != null); - FlutterError.onError(FlutterErrorDetails( - exception: lastFailure, - library: 'package:flutter_image', - context: ErrorDescription('$runtimeType failed to load ${instructions.uri}'), - )); + if (FlutterError.onError != null) { + FlutterError.onError!(FlutterErrorDetails( + exception: lastFailure!, + library: 'package:flutter_image', + context: + ErrorDescription('$runtimeType failed to load ${instructions.uri}'), + )); + } - return null; + throw lastFailure!; } @override bool operator ==(dynamic other) { - if (other.runtimeType != runtimeType) + if (other.runtimeType != runtimeType) { return false; + } final NetworkImageWithRetry typedOther = other; - return url == typedOther.url - && scale == typedOther.scale; + return url == typedOther.url && scale == typedOther.scale; } @override @@ -207,26 +218,26 @@ class NetworkImageWithRetry extends ImageProvider { /// [NetworkImageWithRetry] to try again. /// /// See [NetworkImageWithRetry.defaultFetchStrategy] for an example. -typedef FetchStrategy = Future Function(Uri uri, FetchFailure failure); +typedef FetchStrategy = Future Function( + Uri uri, FetchFailure? failure); /// Instructions [NetworkImageWithRetry] uses to fetch the image. @immutable class FetchInstructions { /// Instructs [NetworkImageWithRetry] to give up trying to download the image. const FetchInstructions.giveUp({ - @required this.uri, + required this.uri, this.alternativeImage, - }) - : shouldGiveUp = true, - timeout = null; + }) : shouldGiveUp = true, + timeout = Duration.zero; /// Instructs [NetworkImageWithRetry] to attempt to download the image from /// the given [uri] and [timeout] if it takes too long. const FetchInstructions.attempt({ - @required this.uri, - @required this.timeout, - }) : shouldGiveUp = false, - alternativeImage = null; + required this.uri, + required this.timeout, + }) : shouldGiveUp = false, + alternativeImage = null; /// Instructs to give up trying. /// @@ -241,16 +252,16 @@ class FetchInstructions { final Uri uri; /// Instructs to give up and use this image instead. - final Future alternativeImage; + final Future? alternativeImage; @override String toString() { return '$runtimeType(\n' - ' shouldGiveUp: $shouldGiveUp\n' - ' timeout: $timeout\n' - ' uri: $uri\n' - ' alternativeImage?: ${alternativeImage != null ? 'yes' : 'no'}\n' - ')'; + ' shouldGiveUp: $shouldGiveUp\n' + ' timeout: $timeout\n' + ' uri: $uri\n' + ' alternativeImage?: ${alternativeImage != null ? 'yes' : 'no'}\n' + ')'; } } @@ -258,12 +269,11 @@ class FetchInstructions { @immutable class FetchFailure implements Exception { const FetchFailure._({ - @required this.totalDuration, - @required this.attemptCount, + required this.totalDuration, + required this.attemptCount, this.httpStatusCode, this.originalException, - }) : assert(totalDuration != null), - assert(attemptCount > 0); + }) : assert(attemptCount > 0); /// The total amount of time it has taken so far to download the image. final Duration totalDuration; @@ -276,7 +286,7 @@ class FetchFailure implements Exception { final int attemptCount; /// HTTP status code, such as 500. - final int httpStatusCode; + final int? httpStatusCode; /// The exception that caused the fetch failure. final dynamic originalException; @@ -294,7 +304,7 @@ class FetchFailure implements Exception { /// An indefinitely growing builder of a [Uint8List]. class _Uint8ListBuilder { - static const int _kInitialSize = 100000; // 100KB-ish + static const int _kInitialSize = 100000; // 100KB-ish int _usedLength = 0; Uint8List _buffer = Uint8List(_kInitialSize); @@ -344,20 +354,16 @@ class FetchStrategyBuilder { this.maxAttempts = 5, this.initialPauseBetweenRetries = const Duration(seconds: 1), this.exponentialBackoffMultiplier = 2, - this.transientHttpStatusCodePredicate = defaultTransientHttpStatusCodePredicate, - }) : assert(timeout != null), - assert(totalFetchTimeout != null), - assert(maxAttempts != null), - assert(initialPauseBetweenRetries != null), - assert(exponentialBackoffMultiplier != null), - assert(transientHttpStatusCodePredicate != null); + this.transientHttpStatusCodePredicate = + defaultTransientHttpStatusCodePredicate, + }); /// A list of HTTP status codes that can generally be retried. /// /// You may want to use a different list depending on the needs of your /// application. static const List defaultTransientHttpStatusCodes = [ - 0, // Network error + 0, // Network error 408, // Request timeout 500, // Internal server error 502, // Bad gateway @@ -395,7 +401,7 @@ class FetchStrategyBuilder { /// Builds a [FetchStrategy] that operates using the properties of this /// builder. FetchStrategy build() { - return (Uri uri, FetchFailure failure) async { + return (Uri uri, FetchFailure? failure) async { if (failure == null) { // First attempt. Just load. return FetchInstructions.attempt( @@ -404,18 +410,21 @@ class FetchStrategyBuilder { ); } - final bool isRetriableFailure = transientHttpStatusCodePredicate(failure.httpStatusCode) || + final bool isRetriableFailure = (failure.httpStatusCode != null && + transientHttpStatusCodePredicate(failure.httpStatusCode!)) || failure.originalException is io.SocketException; // If cannot retry, give up. - if (!isRetriableFailure || // retrying will not help - failure.totalDuration > totalFetchTimeout || // taking too long - failure.attemptCount > maxAttempts) { // too many attempts + if (!isRetriableFailure || // retrying will not help + failure.totalDuration > totalFetchTimeout || // taking too long + failure.attemptCount > maxAttempts) { + // too many attempts return FetchInstructions.giveUp(uri: uri); } // Exponential back-off. - final Duration pauseBetweenRetries = initialPauseBetweenRetries * math.pow(exponentialBackoffMultiplier, failure.attemptCount - 1); + final Duration pauseBetweenRetries = initialPauseBetweenRetries * + math.pow(exponentialBackoffMultiplier, failure.attemptCount - 1); await Future.delayed(pauseBetweenRetries); // Retry. diff --git a/packages/flutter_image/pubspec.yaml b/packages/flutter_image/pubspec.yaml index f930dee4b3..399114cef0 100644 --- a/packages/flutter_image/pubspec.yaml +++ b/packages/flutter_image/pubspec.yaml @@ -1,5 +1,5 @@ name: flutter_image -version: 3.0.0 +version: 4.0.0 description: > Image utilities for Flutter: providers, effects, etc author: Flutter Team @@ -12,9 +12,9 @@ dependencies: dev_dependencies: flutter_test: sdk: flutter - quiver: '>=0.24.0 <3.0.0' + quiver: ^3.0.0 test: any environment: - sdk: ">=2.0.0-dev.28.0 <3.0.0" + sdk: ">=2.12.0 <3.0.0" flutter: ">=1.10.15-pre.144" diff --git a/packages/flutter_image/test/network_test.dart b/packages/flutter_image/test/network_test.dart index d9dd426973..4652ac307f 100644 --- a/packages/flutter_image/test/network_test.dart +++ b/packages/flutter_image/test/network_test.dart @@ -19,122 +19,134 @@ void main() { HttpOverrides.global = null; group('NetworkImageWithRetry', () { - setUp(() { - FlutterError.onError = (FlutterErrorDetails error) { - fail('$error'); - }; + group('succeeds', () { + setUp(() { + FlutterError.onError = (FlutterErrorDetails error) { + fail('$error'); + }; + }); + + tearDown(() { + FlutterError.onError = FlutterError.dumpErrorToConsole; + }); + + test('loads image from network', () async { + final NetworkImageWithRetry subject = NetworkImageWithRetry( + _imageUrl('immediate_success.png'), + ); + + assertThatImageLoadingSucceeds(subject); + }); + + test('succeeds on successful retry', () async { + final NetworkImageWithRetry subject = NetworkImageWithRetry( + _imageUrl('error.png'), + fetchStrategy: (Uri uri, FetchFailure? failure) async { + if (failure == null) { + return FetchInstructions.attempt( + uri: uri, + timeout: const Duration(minutes: 1), + ); + } else { + expect(failure.attemptCount, lessThan(2)); + return FetchInstructions.attempt( + uri: Uri.parse(_imageUrl('immediate_success.png')), + timeout: const Duration(minutes: 1), + ); + } + }, + ); + assertThatImageLoadingSucceeds(subject); + }); }); - tearDown(() { - FlutterError.onError = FlutterError.dumpErrorToConsole; - }); - - test('loads image from network', () async { - final NetworkImageWithRetry subject = NetworkImageWithRetry( - _imageUrl('immediate_success.png'), - ); - - subject.load(subject, PaintingBinding.instance.instantiateImageCodec).addListener( - ImageStreamListener(expectAsync2((ImageInfo image, bool synchronousCall) { - expect(image.image.height, 1); - expect(image.image.width, 1); - })), - ); - }); - - test('retries 6 times then gives up', () async { + group('fails', () { final List errorLog = []; - FlutterError.onError = errorLog.add; + FakeAsync fakeAsync = FakeAsync(); - final FakeAsync fakeAsync = FakeAsync(); - final dynamic maxAttemptCountReached = expectAsync0(() {}); + setUp(() { + FlutterError.onError = errorLog.add; + }); - int attemptCount = 0; - Future onAttempt() async { - expect(attemptCount, lessThan(7)); - if (attemptCount == 6) { - maxAttemptCountReached(); - } - await Future.delayed(Duration.zero); - fakeAsync.elapse(const Duration(seconds: 60)); - attemptCount++; - } + tearDown(() { + fakeAsync = FakeAsync(); + errorLog.clear(); + FlutterError.onError = FlutterError.dumpErrorToConsole; + }); - final NetworkImageWithRetry subject = NetworkImageWithRetry( - _imageUrl('error.png'), - fetchStrategy: (Uri uri, FetchFailure failure) { - Timer.run(onAttempt); - return fakeAsync.run((FakeAsync fakeAsync) { - return NetworkImageWithRetry.defaultFetchStrategy(uri, failure); - }); - }, - ); + test('retries 6 times then gives up', () async { + final dynamic maxAttemptCountReached = expectAsync0(() {}); - subject.load(subject, PaintingBinding.instance.instantiateImageCodec).addListener( - ImageStreamListener(expectAsync2((ImageInfo image, bool synchronousCall) { - expect(errorLog.single.exception, isInstanceOf()); - expect(image, null); - })), - ); - }); - - test('gives up immediately on non-retriable errors (HTTP 404)', () async { - final List errorLog = []; - FlutterError.onError = errorLog.add; - - final FakeAsync fakeAsync = FakeAsync(); - - int attemptCount = 0; - Future onAttempt() async { - expect(attemptCount, lessThan(2)); - await Future.delayed(Duration.zero); - fakeAsync.elapse(const Duration(seconds: 60)); - attemptCount++; - } - - final NetworkImageWithRetry subject = NetworkImageWithRetry( - _imageUrl('does_not_exist.png'), - fetchStrategy: (Uri uri, FetchFailure failure) { - Timer.run(onAttempt); - return fakeAsync.run((FakeAsync fakeAsync) { - return NetworkImageWithRetry.defaultFetchStrategy(uri, failure); - }); - }, - ); - - subject.load(subject, PaintingBinding.instance.instantiateImageCodec).addListener( - ImageStreamListener(expectAsync2((ImageInfo image, bool synchronousCall) { - expect(errorLog.single.exception, isInstanceOf()); - expect(image, null); - })), - ); - }); - - test('succeeds on successful retry', () async { - final NetworkImageWithRetry subject = NetworkImageWithRetry( - _imageUrl('error.png'), - fetchStrategy: (Uri uri, FetchFailure failure) async { - if (failure == null) { - return FetchInstructions.attempt( - uri: uri, - timeout: const Duration(minutes: 1), - ); - } else { - expect(failure.attemptCount, lessThan(2)); - return FetchInstructions.attempt( - uri: Uri.parse(_imageUrl('immediate_success.png')), - timeout: const Duration(minutes: 1), - ); + int attemptCount = 0; + Future onAttempt() async { + expect(attemptCount, lessThan(7)); + if (attemptCount == 6) { + maxAttemptCountReached(); } - }, - ); + await Future.delayed(Duration.zero); + fakeAsync.elapse(const Duration(seconds: 60)); + attemptCount++; + } - subject.load(subject, PaintingBinding.instance.instantiateImageCodec).addListener( - ImageStreamListener(expectAsync2((ImageInfo image, bool synchronousCall) { - expect(image.image.height, 1); - expect(image.image.width, 1); - })), - ); + final NetworkImageWithRetry subject = NetworkImageWithRetry( + _imageUrl('error.png'), + fetchStrategy: (Uri uri, FetchFailure? failure) { + Timer.run(onAttempt); + return fakeAsync.run((FakeAsync fakeAsync) { + return NetworkImageWithRetry.defaultFetchStrategy(uri, failure); + }); + }, + ); + + assertThatImageLoadingFails(subject, errorLog); + }); + + test('gives up immediately on non-retriable errors (HTTP 404)', () async { + int attemptCount = 0; + Future onAttempt() async { + expect(attemptCount, lessThan(2)); + await Future.delayed(Duration.zero); + fakeAsync.elapse(const Duration(seconds: 60)); + attemptCount++; + } + + final NetworkImageWithRetry subject = NetworkImageWithRetry( + _imageUrl('does_not_exist.png'), + fetchStrategy: (Uri uri, FetchFailure? failure) { + Timer.run(onAttempt); + return fakeAsync.run((FakeAsync fakeAsync) { + return NetworkImageWithRetry.defaultFetchStrategy(uri, failure); + }); + }, + ); + + assertThatImageLoadingFails(subject, errorLog); + }); }); }); } + +void assertThatImageLoadingFails( + NetworkImageWithRetry subject, List errorLog) { + subject + .load(subject, PaintingBinding.instance!.instantiateImageCodec) + .addListener(ImageStreamListener( + (ImageInfo image, bool synchronousCall) {}, + onError: expectAsync2((Object error, StackTrace? _) { + expect(errorLog.single.exception, isInstanceOf()); + expect(error, isInstanceOf()); + expect(error, equals(errorLog.single.exception)); + }), + )); +} + +void assertThatImageLoadingSucceeds(NetworkImageWithRetry subject) { + subject + .load(subject, PaintingBinding.instance!.instantiateImageCodec) + .addListener( + ImageStreamListener(expectAsync2((ImageInfo image, bool synchronousCall) { + expect(image.image.height, 1); + expect(image.image.width, 1); + })), + ); +}