From 94b8de7ca3c95eb496ebfd79e4ba3430abe6008c Mon Sep 17 00:00:00 2001 From: JaffaKetchup Date: Sun, 7 Apr 2024 23:35:29 +0100 Subject: [PATCH] Fixed "User-Agent" "FMTC" identifier injection Added loading monitoring between `ImageProvider` & `TileProvider` to reduce chance of early `HttpClient` closure Minor miscellaneous improvements --- lib/flutter_map_tile_caching.dart | 1 + lib/src/bulk_download/manager.dart | 18 +++-- lib/src/providers/image_provider.dart | 40 ++++++---- lib/src/providers/tile_provider.dart | 108 +++++++++++++++++++------- lib/src/store/store.dart | 10 +-- 5 files changed, 120 insertions(+), 57 deletions(-) diff --git a/lib/flutter_map_tile_caching.dart b/lib/flutter_map_tile_caching.dart index 0aec5424..d1b714bc 100644 --- a/lib/flutter_map_tile_caching.dart +++ b/lib/flutter_map_tile_caching.dart @@ -11,6 +11,7 @@ library flutter_map_tile_caching; import 'dart:async'; +import 'dart:collection'; import 'dart:io'; import 'dart:isolate'; import 'dart:math' as math; diff --git a/lib/src/bulk_download/manager.dart b/lib/src/bulk_download/manager.dart index c27e9a46..f3f92593 100644 --- a/lib/src/bulk_download/manager.dart +++ b/lib/src/bulk_download/manager.dart @@ -18,15 +18,21 @@ Future _downloadManager( FMTCBackendInternalThreadSafe backend, }) input, ) async { - // Precalculate shared inputs for all threads + // Precalculate how large the tile buffers should be for each thread final threadBufferLength = (input.maxBufferLength / input.parallelThreads).floor(); + + // Generate appropriate headers for network requests + final inputHeaders = input.region.options.tileProvider.headers; final headers = { - ...input.region.options.tileProvider.headers, - 'User-Agent': input.region.options.tileProvider.headers['User-Agent'] == - null - ? 'flutter_map_tile_caching for flutter_map (unknown)' - : 'flutter_map_tile_caching for ${input.region.options.tileProvider.headers['User-Agent']}', + ...inputHeaders, + 'User-Agent': inputHeaders['User-Agent'] == null + ? 'flutter_map (unknown)' + : 'flutter_map + FMTC ${inputHeaders['User-Agent']!.replaceRange( + 0, + inputHeaders['User-Agent']!.length.clamp(0, 12), + '', + )}', }; // Count number of tiles diff --git a/lib/src/providers/image_provider.dart b/lib/src/providers/image_provider.dart index 17065e71..fac44747 100644 --- a/lib/src/providers/image_provider.dart +++ b/lib/src/providers/image_provider.dart @@ -15,19 +15,19 @@ import '../../flutter_map_tile_caching.dart'; import '../backend/export_internal.dart'; import '../misc/obscure_query_params.dart'; -/// A specialised [ImageProvider] dedicated to 'flutter_map_tile_caching' +/// A specialised [ImageProvider] that uses FMTC internals to enable browse +/// caching class FMTCImageProvider extends ImageProvider { - /// Create a specialised [ImageProvider] dedicated to 'flutter_map_tile_caching' + /// Create a specialised [ImageProvider] that uses FMTC internals to enable + /// browse caching FMTCImageProvider({ - required this.storeName, required this.provider, required this.options, required this.coords, + required this.startedLoading, + required this.finishedLoadingBytes, }); - /// The name of the store associated with this provider - final String storeName; - /// An instance of the [FMTCTileProvider] in use final FMTCTileProvider provider; @@ -37,6 +37,18 @@ class FMTCImageProvider extends ImageProvider { /// The coordinates of the tile to be fetched final TileCoordinates coords; + /// Function invoked when the image starts loading (not from cache) + /// + /// Used with [finishedLoadingBytes] to safely dispose of the `httpClient` only + /// after all tiles have loaded. + final void Function() startedLoading; + + /// Function invoked when the image completes loading bytes from the network + /// + /// Used with [finishedLoadingBytes] to safely dispose of the `httpClient` only + /// after all tiles have loaded. + final void Function() finishedLoadingBytes; + @override ImageStreamCompleter loadImage( FMTCImageProvider key, @@ -49,7 +61,7 @@ class FMTCImageProvider extends ImageProvider { scale: 1, debugLabel: coords.toString(), informationCollector: () => [ - DiagnosticsProperty('Store name', storeName), + DiagnosticsProperty('Store name', provider.storeName), DiagnosticsProperty('Tile coordinates', coords), DiagnosticsProperty('Current provider', key), ], @@ -64,7 +76,7 @@ class FMTCImageProvider extends ImageProvider { Future finishWithError(FMTCBrowsingError err) async { scheduleMicrotask(() => PaintingBinding.instance.imageCache.evict(key)); unawaited(chunkEvents.close()); - await evict(); + finishedLoadingBytes(); provider.settings.errorHandler?.call(err); throw err; @@ -76,11 +88,11 @@ class FMTCImageProvider extends ImageProvider { }) async { scheduleMicrotask(() => PaintingBinding.instance.imageCache.evict(key)); unawaited(chunkEvents.close()); - await evict(); + finishedLoadingBytes(); unawaited( FMTCBackendAccess.internal - .registerHitOrMiss(storeName: storeName, hit: cacheHit), + .registerHitOrMiss(storeName: provider.storeName, hit: cacheHit), ); return decode(await ImmutableBuffer.fromUint8List(bytes)); } @@ -98,6 +110,8 @@ class FMTCImageProvider extends ImageProvider { return null; } + startedLoading(); + final networkUrl = provider.getTileUrl(coords, options); final matcherUrl = obscureQueryParams( url: networkUrl, @@ -106,7 +120,7 @@ class FMTCImageProvider extends ImageProvider { final existingTile = await FMTCBackendAccess.internal.readTile( url: matcherUrl, - storeName: storeName, + storeName: provider.storeName, ); final needsCreating = existingTile == null; @@ -241,7 +255,7 @@ class FMTCImageProvider extends ImageProvider { // Cache the tile retrieved from the network response unawaited( FMTCBackendAccess.internal.writeTile( - storeName: storeName, + storeName: provider.storeName, url: matcherUrl, bytes: responseBytes, ), @@ -251,7 +265,7 @@ class FMTCImageProvider extends ImageProvider { if (needsCreating && provider.settings.maxStoreLength != 0) { unawaited( FMTCBackendAccess.internal.removeOldestTilesAboveLimit( - storeName: storeName, + storeName: provider.storeName, tilesLimit: provider.settings.maxStoreLength, ), ); diff --git a/lib/src/providers/tile_provider.dart b/lib/src/providers/tile_provider.dart index c3fc0120..c25557fa 100644 --- a/lib/src/providers/tile_provider.dart +++ b/lib/src/providers/tile_provider.dart @@ -3,56 +3,74 @@ part of '../../flutter_map_tile_caching.dart'; -/// FMTC's custom [TileProvider] for use in a [TileLayer] +/// Specialised [TileProvider] that uses a specialised [ImageProvider] to connect +/// to FMTC internals +/// +/// An "FMTC" identifying mark is injected into the "User-Agent" header generated +/// by flutter_map, except if specified in the constructor. For technical +/// details, see [_CustomUserAgentCompatMap]. /// /// Create from the store directory chain, eg. [FMTCStore.getTileProvider]. class FMTCTileProvider extends TileProvider { FMTCTileProvider._( - this._store, { - required FMTCTileProviderSettings? settings, - required Map headers, - required http.Client? httpClient, - }) : settings = settings ?? FMTCTileProviderSettings.instance, + this.storeName, + FMTCTileProviderSettings? settings, + Map? headers, + http.Client? httpClient, + ) : settings = settings ?? FMTCTileProviderSettings.instance, httpClient = httpClient ?? IOClient(HttpClient()..userAgent = null), super( - headers: { - ...headers, - 'User-Agent': headers['User-Agent'] == null - ? 'flutter_map_tile_caching for flutter_map (unknown)' - : 'flutter_map_tile_caching for ${headers['User-Agent']}', - }, + headers: (headers?.containsKey('User-Agent') ?? false) + ? headers + : _CustomUserAgentCompatMap(headers ?? {}), ); - /// The store directory attached to this provider - final FMTCStore _store; + /// The store name of the [FMTCStore] used when generating this provider + final String storeName; /// The tile provider settings to use + /// + /// Defaults to the ambient [FMTCTileProviderSettings.instance]. final FMTCTileProviderSettings settings; /// [http.Client] (such as a [IOClient]) used to make all network requests /// - /// Defaults to a standard [IOClient]/[HttpClient] for HTTP/1.1 servers. + /// Do not close manually. + /// + /// Defaults to a standard [IOClient]/[HttpClient]. final http.Client httpClient; - /// Closes the open [httpClient] - this will make the provider unable to - /// perform network requests - @override - void dispose() { - httpClient.close(); - super.dispose(); - } + /// Each [Completer] is completed once the corresponding tile has finished + /// loading + /// + /// Used to avoid disposing of [httpClient] whilst HTTP requests are still + /// underway. + /// + /// Does not include tiles loaded from session cache. + final _tilesInProgress = HashMap>(); - /// Get a browsed tile as an image, paint it on the map and save it's bytes to - /// cache for later (dependent on the [CacheBehavior]) @override - ImageProvider getImage(TileCoordinates coords, TileLayer options) => + ImageProvider getImage(TileCoordinates coordinates, TileLayer options) => FMTCImageProvider( - storeName: _store.storeName, provider: this, options: options, - coords: coords, + coords: coordinates, + startedLoading: () => _tilesInProgress[coordinates] = Completer(), + finishedLoadingBytes: () { + _tilesInProgress[coordinates]?.complete(); + _tilesInProgress.remove(coordinates); + }, ); + @override + Future dispose() async { + if (_tilesInProgress.isNotEmpty) { + await Future.wait(_tilesInProgress.values.map((c) => c.future)); + } + httpClient.close(); + super.dispose(); + } + /// Check whether a specified tile is cached in the current store @Deprecated(''' Migrate to `checkTileCached`. @@ -72,7 +90,7 @@ member will be removed in a future version.''') required TileLayer options, }) => FMTCBackendAccess.internal.tileExistsInStore( - storeName: _store.storeName, + storeName: storeName, url: obscureQueryParams( url: getTileUrl(coords, options), obscuredQueryParams: settings.obscuredQueryParams, @@ -83,11 +101,41 @@ member will be removed in a future version.''') bool operator ==(Object other) => identical(this, other) || (other is FMTCTileProvider && - other._store == _store && + other.storeName == storeName && other.headers == headers && other.settings == settings && other.httpClient == httpClient); @override - int get hashCode => Object.hash(_store, settings, headers, httpClient); + int get hashCode => Object.hash(storeName, settings, headers, httpClient); +} + +/// Custom override of [Map] that only overrides the [MapView.putIfAbsent] +/// method, to enable injection of an identifying mark ("FMTC") +class _CustomUserAgentCompatMap extends MapView { + const _CustomUserAgentCompatMap(super.map); + + /// Modified implementation of [MapView.putIfAbsent], that overrides behaviour + /// only when [key] is "User-Agent" + /// + /// flutter_map's [TileLayer] constructor calls this method after the + /// [TileLayer.tileProvider] has been constructed to customize the + /// "User-Agent" header with `TileLayer.userAgentPackageName`. + /// This method intercepts any call with [key] equal to "User-Agent" and + /// replacement value that matches the expected format, and adds an "FMTC" + /// identifying mark. + /// + /// The identifying mark is injected to seperate traffic sent via FMTC from + /// standard flutter_map traffic, as it significantly changes the behaviour of + /// tile retrieval, and could generate more traffic. + @override + String putIfAbsent(String key, String Function() ifAbsent) { + if (key != 'User-Agent') return super.putIfAbsent(key, ifAbsent); + + final replacementValue = ifAbsent(); + if (!RegExp(r'flutter_map \(.+\)').hasMatch(replacementValue)) { + return super.putIfAbsent(key, ifAbsent); + } + return this[key] = replacementValue.replaceRange(11, 12, ' + FMTC '); + } } diff --git a/lib/src/store/store.dart b/lib/src/store/store.dart index 0031c624..ef0f8233 100644 --- a/lib/src/store/store.dart +++ b/lib/src/store/store.dart @@ -57,8 +57,7 @@ class FMTCStore { /// background downloading functionality. DownloadManagement get download => DownloadManagement._(this); - /// Get the [TileProvider] suitable to connect the [TileLayer] to FMTC's - /// internals + /// Generate a [TileProvider] that connects to FMTC internals /// /// [settings] defaults to the current ambient /// [FMTCTileProviderSettings.instance], which defaults to the initial @@ -68,12 +67,7 @@ class FMTCStore { Map? headers, http.Client? httpClient, }) => - FMTCTileProvider._( - this, - settings: settings, - headers: headers ?? {}, - httpClient: httpClient, - ); + FMTCTileProvider._(storeName, settings, headers, httpClient); @override bool operator ==(Object other) =>