From 8983939b05332b8829831a17d89a53a1db1f82b3 Mon Sep 17 00:00:00 2001 From: Doug Borg Date: Tue, 24 Feb 2026 19:39:09 -0700 Subject: [PATCH] Delegate network tile fetching to NetworkTileProvider Replace our custom tile pipeline (fetchRemoteTile / _SimpleSemaphore / exponential backoff) with flutter_map's built-in NetworkTileProvider, gaining persistent disk cache, ETag revalidation, RetryClient, and obsolete request aborting for free. DeflockTileProvider now extends NetworkTileProvider and overrides getTileUrl() to route through TileType.getTileUrl() (quadkey, subdomains, API keys). getImageWithCancelLoadingSupport() routes between two paths at runtime: the common network path (super) when no offline areas exist, and a DeflockOfflineTileImageProvider for offline-first when they do. - Delete tiles_from_remote.dart (semaphore, retry loop, spatial helpers) - Simplify MapDataProvider._fetchRemoteTileFromCurrentProvider to plain http.get (only used by offline area downloader now) - Remove dead clearTileQueue/clearTileQueueSelective from MapDataProvider - Remove 7 tile fetch constants from dev_config.dart - TileLayerManager now disposes provider on cache clear and uses actual urlTemplate for cache key generation - 9 new tests covering URL delegation, routing, and equality Closes #87 Phase 2. Co-Authored-By: Claude Opus 4.6 --- lib/dev_config.dart | 9 - lib/services/deflock_tile_provider.dart | 316 ++++++++++++------ lib/services/map_data_provider.dart | 28 +- .../map_data_submodules/tiles_from_local.dart | 22 +- .../tiles_from_remote.dart | 265 --------------- lib/widgets/map/tile_layer_manager.dart | 31 +- test/services/deflock_tile_provider_test.dart | 280 ++++++++++++---- 7 files changed, 465 insertions(+), 486 deletions(-) delete mode 100644 lib/services/map_data_submodules/tiles_from_remote.dart diff --git a/lib/dev_config.dart b/lib/dev_config.dart index 0c495c7..44294c6 100644 --- a/lib/dev_config.dart +++ b/lib/dev_config.dart @@ -155,15 +155,6 @@ const double kPinchZoomThreshold = 0.2; // How much pinch required to start zoom const double kPinchMoveThreshold = 30.0; // How much drag required for two-finger pan (default 40.0) const double kRotationThreshold = 6.0; // Degrees of rotation required before map actually rotates (Google Maps style) -// Tile fetch configuration (brutalist approach: simple, configurable, unlimited retries) -const int kTileFetchConcurrentThreads = 8; // Reduced from 10 to 8 for better cross-platform performance -const int kTileFetchInitialDelayMs = 150; // Reduced from 200ms for faster retries -const double kTileFetchBackoffMultiplier = 1.4; // Slightly reduced for faster recovery -const int kTileFetchMaxDelayMs = 4000; // Reduced from 5000ms for faster max retry -const int kTileFetchRandomJitterMs = 50; // Reduced jitter for more predictable timing -const int kTileFetchMaxQueueSize = 100; // Reasonable queue size to prevent memory bloat -// Note: Removed max attempts - tiles retry indefinitely until they succeed or are canceled - // User download max zoom span (user can download up to kMaxUserDownloadZoomSpan zooms above min) const int kMaxUserDownloadZoomSpan = 7; diff --git a/lib/services/deflock_tile_provider.dart b/lib/services/deflock_tile_provider.dart index a9f4aa8..707f015 100644 --- a/lib/services/deflock_tile_provider.dart +++ b/lib/services/deflock_tile_provider.dart @@ -4,154 +4,268 @@ import 'dart:ui'; import 'package:flutter_map/flutter_map.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; +import 'package:http/http.dart'; +import 'package:http/retry.dart'; import '../app_state.dart'; -import 'map_data_provider.dart'; +import 'http_client.dart'; +import 'map_data_submodules/tiles_from_local.dart'; import 'offline_area_service.dart'; -/// Custom tile provider that integrates with DeFlock's offline/online architecture. -/// -/// This replaces the complex HTTP interception approach with a clean TileProvider -/// implementation that directly interfaces with our MapDataProvider system. -class DeflockTileProvider extends TileProvider { - final MapDataProvider _mapDataProvider = MapDataProvider(); - +/// Custom tile provider that extends NetworkTileProvider to leverage its +/// built-in disk cache, RetryClient, ETag revalidation, and abort support, +/// while routing URLs through our TileType logic and supporting offline tiles. +/// +/// Two runtime paths: +/// 1. **Common path** (no offline areas for current provider): delegates to +/// super.getImageWithCancelLoadingSupport() — full NetworkTileImageProvider +/// pipeline (disk cache, ETag revalidation, RetryClient, abort support). +/// 2. **Offline-first path** (has offline areas or offline mode): returns +/// DeflockOfflineTileImageProvider — checks fetchLocalTile() first, falls +/// back to HTTP via shared RetryClient on miss. +class DeflockTileProvider extends NetworkTileProvider { + /// The shared HTTP client we own. We keep a reference because + /// NetworkTileProvider._httpClient is private and _isInternallyCreatedClient + /// will be false (we passed it in), so super.dispose() won't close it. + final Client _sharedHttpClient; + + DeflockTileProvider._({required Client httpClient}) + : _sharedHttpClient = httpClient, + super( + httpClient: httpClient, + silenceExceptions: true, + ); + + factory DeflockTileProvider() { + final client = UserAgentClient(RetryClient(Client())); + return DeflockTileProvider._(httpClient: client); + } + @override - ImageProvider getImage(TileCoordinates coordinates, TileLayer options) { - // Get current provider info to include in cache key + String getTileUrl(TileCoordinates coordinates, TileLayer options) { + final appState = AppState.instance; + final selectedTileType = appState.selectedTileType; + final selectedProvider = appState.selectedTileProvider; + + if (selectedTileType == null || selectedProvider == null) { + // Fallback to base implementation if no provider configured + return super.getTileUrl(coordinates, options); + } + + return selectedTileType.getTileUrl( + coordinates.z, + coordinates.x, + coordinates.y, + apiKey: selectedProvider.apiKey, + ); + } + + @override + ImageProvider getImageWithCancelLoadingSupport( + TileCoordinates coordinates, + TileLayer options, + Future cancelLoading, + ) { + if (!_shouldCheckOfflineCache()) { + // Common path: no offline areas — delegate to NetworkTileProvider's + // full pipeline (disk cache, ETag, RetryClient, abort support). + return super.getImageWithCancelLoadingSupport( + coordinates, + options, + cancelLoading, + ); + } + + // Offline-first path: check local tiles first, fall back to network. final appState = AppState.instance; final providerId = appState.selectedTileProvider?.id ?? 'unknown'; final tileTypeId = appState.selectedTileType?.id ?? 'unknown'; - - return DeflockTileImageProvider( + + return DeflockOfflineTileImageProvider( coordinates: coordinates, options: options, - mapDataProvider: _mapDataProvider, + httpClient: _sharedHttpClient, + headers: headers, + cancelLoading: cancelLoading, + isOfflineOnly: appState.offlineMode, providerId: providerId, tileTypeId: tileTypeId, + tileUrl: getTileUrl(coordinates, options), ); } + + /// Determine if we should check offline cache for this tile request. + /// Only returns true if: + /// 1. We're in offline mode (forced), OR + /// 2. We have offline areas for the current provider/type + /// + /// This avoids the offline-first path (and its filesystem searches) when + /// browsing online with providers that have no offline areas. + bool _shouldCheckOfflineCache() { + final appState = AppState.instance; + + // Always use offline path in offline mode + if (appState.offlineMode) { + return true; + } + + // For online mode, only use offline path if we have relevant offline data + final currentProvider = appState.selectedTileProvider; + final currentTileType = appState.selectedTileType; + + if (currentProvider == null || currentTileType == null) { + return false; + } + + final offlineService = OfflineAreaService(); + return offlineService.hasOfflineAreasForProvider( + currentProvider.id, + currentTileType.id, + ); + } + + @override + Future dispose() async { + try { + await super.dispose(); + } finally { + _sharedHttpClient.close(); + } + } } -/// Image provider that fetches tiles through our MapDataProvider. -/// -/// This handles the actual tile fetching using our existing offline/online -/// routing logic without any HTTP interception complexity. -class DeflockTileImageProvider extends ImageProvider { +/// Image provider for the offline-first path. +/// +/// Tries fetchLocalTile() first. On miss (and if online), falls back to an +/// HTTP GET via the shared RetryClient. Handles cancelLoading abort and +/// returns transparent tiles on errors (consistent with silenceExceptions). +class DeflockOfflineTileImageProvider + extends ImageProvider { final TileCoordinates coordinates; final TileLayer options; - final MapDataProvider mapDataProvider; + final Client httpClient; + final Map headers; + final Future cancelLoading; + final bool isOfflineOnly; final String providerId; final String tileTypeId; - - const DeflockTileImageProvider({ + final String tileUrl; + + const DeflockOfflineTileImageProvider({ required this.coordinates, required this.options, - required this.mapDataProvider, + required this.httpClient, + required this.headers, + required this.cancelLoading, + required this.isOfflineOnly, required this.providerId, required this.tileTypeId, + required this.tileUrl, }); - + @override - Future obtainKey(ImageConfiguration configuration) { - return SynchronousFuture(this); + Future obtainKey( + ImageConfiguration configuration) { + return SynchronousFuture(this); } - + @override - ImageStreamCompleter loadImage(DeflockTileImageProvider key, ImageDecoderCallback decode) { + ImageStreamCompleter loadImage( + DeflockOfflineTileImageProvider key, ImageDecoderCallback decode) { final chunkEvents = StreamController(); - + final codecFuture = _loadAsync(key, decode, chunkEvents); + + codecFuture.whenComplete(() { + chunkEvents.close(); + }); + return MultiFrameImageStreamCompleter( - codec: _loadAsync(key, decode, chunkEvents), + codec: codecFuture, chunkEvents: chunkEvents.stream, scale: 1.0, ); } - + Future _loadAsync( - DeflockTileImageProvider key, + DeflockOfflineTileImageProvider key, ImageDecoderCallback decode, StreamController chunkEvents, ) async { + Future decodeBytes(Uint8List bytes) => + ImmutableBuffer.fromUint8List(bytes).then(decode); + + Future transparent() => + decodeBytes(TileProvider.transparentImage); + try { - // Get current tile provider and type from app state - final appState = AppState.instance; - final selectedProvider = appState.selectedTileProvider; - final selectedTileType = appState.selectedTileType; - - if (selectedProvider == null || selectedTileType == null) { - throw Exception('No tile provider configured'); + // Track cancellation + bool cancelled = false; + cancelLoading.then((_) => cancelled = true); + + // Try local tile first — pass captured IDs to avoid a race if the + // user switches provider while this async load is in flight. + try { + final localBytes = await fetchLocalTile( + z: coordinates.z, + x: coordinates.x, + y: coordinates.y, + providerId: providerId, + tileTypeId: tileTypeId, + ); + return await decodeBytes(Uint8List.fromList(localBytes)); + } catch (_) { + // Local miss — fall through to network if online } - - // Smart cache routing: only check offline cache when needed - final MapSource source = _shouldCheckOfflineCache(appState) - ? MapSource.auto // Check offline first, then network - : MapSource.remote; // Skip offline cache, go directly to network - - final tileBytes = await mapDataProvider.getTile( - z: coordinates.z, - x: coordinates.x, - y: coordinates.y, - source: source, - ); - - // Decode the image bytes - final buffer = await ImmutableBuffer.fromUint8List(Uint8List.fromList(tileBytes)); - return await decode(buffer); - + + if (cancelled) return await transparent(); + if (isOfflineOnly) return await transparent(); + + // Fall back to network via shared RetryClient. + // Race the download against cancelLoading so we stop waiting if the + // tile is pruned mid-flight (the underlying TCP connection is cleaned + // up naturally by the shared client). + final request = Request('GET', Uri.parse(tileUrl)); + request.headers.addAll(headers); + + final networkFuture = httpClient.send(request).then((response) async { + final bytes = await response.stream.toBytes(); + return (statusCode: response.statusCode, bytes: bytes); + }); + + final result = await Future.any([ + networkFuture, + cancelLoading.then((_) => (statusCode: 0, bytes: Uint8List(0))), + ]); + + if (cancelled || result.statusCode == 0) return await transparent(); + + if (result.statusCode == 200 && result.bytes.isNotEmpty) { + return await decodeBytes(result.bytes); + } + + return await transparent(); } catch (e) { - // Don't log routine offline misses to avoid console spam - if (!e.toString().contains('offline mode is enabled')) { - debugPrint('[DeflockTileProvider] Failed to load tile ${coordinates.z}/${coordinates.x}/${coordinates.y}: $e'); + // Don't log routine offline misses + if (!e.toString().contains('offline')) { + debugPrint( + '[DeflockTileProvider] Offline-first tile failed ' + '${coordinates.z}/${coordinates.x}/${coordinates.y} ' + '(${e.runtimeType})'); } - - // Re-throw the exception and let FlutterMap handle missing tiles gracefully - // This is better than trying to provide fallback images - rethrow; + return await ImmutableBuffer.fromUint8List(TileProvider.transparentImage) + .then(decode); } } - + @override bool operator ==(Object other) { if (other.runtimeType != runtimeType) return false; - return other is DeflockTileImageProvider && - other.coordinates == coordinates && - other.providerId == providerId && - other.tileTypeId == tileTypeId; + return other is DeflockOfflineTileImageProvider && + other.coordinates == coordinates && + other.providerId == providerId && + other.tileTypeId == tileTypeId; } - + @override int get hashCode => Object.hash(coordinates, providerId, tileTypeId); - - /// Determine if we should check offline cache for this tile request. - /// Only check offline cache if: - /// 1. We're in offline mode (forced), OR - /// 2. We have offline areas for the current provider/type - /// - /// This avoids expensive filesystem searches when browsing online - /// with providers that have no offline areas. - bool _shouldCheckOfflineCache(AppState appState) { - // Always check offline cache in offline mode - if (appState.offlineMode) { - return true; - } - - // For online mode, only check if we might actually have relevant offline data - final currentProvider = appState.selectedTileProvider; - final currentTileType = appState.selectedTileType; - - if (currentProvider == null || currentTileType == null) { - return false; - } - - // Quick check: do we have any offline areas for this provider/type? - // This avoids the expensive per-tile filesystem search in fetchLocalTile - final offlineService = OfflineAreaService(); - final hasRelevantAreas = offlineService.hasOfflineAreasForProvider( - currentProvider.id, - currentTileType.id, - ); - - return hasRelevantAreas; - } -} \ No newline at end of file +} diff --git a/lib/services/map_data_provider.dart b/lib/services/map_data_provider.dart index 34c24c8..6f5e99b 100644 --- a/lib/services/map_data_provider.dart +++ b/lib/services/map_data_provider.dart @@ -4,7 +4,7 @@ import 'package:flutter_map/flutter_map.dart'; import '../models/node_profile.dart'; import '../models/osm_node.dart'; import '../app_state.dart'; -import 'map_data_submodules/tiles_from_remote.dart'; +import 'http_client.dart'; import 'map_data_submodules/tiles_from_local.dart'; import 'node_data_manager.dart'; import 'node_spatial_cache.dart'; @@ -24,6 +24,7 @@ class MapDataProvider { MapDataProvider._(); final NodeDataManager _nodeDataManager = NodeDataManager(); + final UserAgentClient _httpClient = UserAgentClient(); bool get isOfflineMode => AppState.instance.offlineMode; void setOfflineMode(bool enabled) { @@ -97,29 +98,24 @@ class MapDataProvider { } } - /// Fetch remote tile using current provider from AppState + /// Fetch remote tile using current provider from AppState. + /// Only used by offline area downloader — the main tile pipeline now goes + /// through NetworkTileProvider (see DeflockTileProvider). Future> _fetchRemoteTileFromCurrentProvider(int z, int x, int y) async { final appState = AppState.instance; final selectedTileType = appState.selectedTileType; final selectedProvider = appState.selectedTileProvider; - - // We guarantee that a provider and tile type are always selected + if (selectedTileType == null || selectedProvider == null) { throw Exception('No tile provider selected - this should never happen'); } - - final tileUrl = selectedTileType.getTileUrl(z, x, y, apiKey: selectedProvider.apiKey); - return fetchRemoteTile(z: z, x: x, y: y, url: tileUrl); - } - /// Clear any queued tile requests (call when map view changes significantly) - void clearTileQueue() { - clearRemoteTileQueue(); - } - - /// Clear only tile requests that are no longer visible in the current bounds - void clearTileQueueSelective(LatLngBounds currentBounds) { - clearRemoteTileQueueSelective(currentBounds); + final tileUrl = selectedTileType.getTileUrl(z, x, y, apiKey: selectedProvider.apiKey); + final resp = await _httpClient.get(Uri.parse(tileUrl)); + if (resp.statusCode == 200 && resp.bodyBytes.isNotEmpty) { + return resp.bodyBytes; + } + throw Exception('Failed to fetch tile $z/$x/$y: status ${resp.statusCode}'); } /// Add or update nodes in cache (for upload queue integration) diff --git a/lib/services/map_data_submodules/tiles_from_local.dart b/lib/services/map_data_submodules/tiles_from_local.dart index 373f20d..003d9a0 100644 --- a/lib/services/map_data_submodules/tiles_from_local.dart +++ b/lib/services/map_data_submodules/tiles_from_local.dart @@ -4,11 +4,21 @@ import '../offline_areas/offline_area_models.dart'; import '../offline_areas/offline_tile_utils.dart'; import '../../app_state.dart'; -/// Fetch a tile from the newest offline area that matches the current provider, or throw if not found. -Future> fetchLocalTile({required int z, required int x, required int y}) async { +/// Fetch a tile from the newest offline area that matches the given provider, or throw if not found. +/// +/// When [providerId] and [tileTypeId] are supplied the lookup is pinned to +/// those values (avoids a race when the user switches provider mid-flight). +/// Otherwise falls back to the current AppState selection. +Future> fetchLocalTile({ + required int z, + required int x, + required int y, + String? providerId, + String? tileTypeId, +}) async { final appState = AppState.instance; - final currentProvider = appState.selectedTileProvider; - final currentTileType = appState.selectedTileType; + final currentProviderId = providerId ?? appState.selectedTileProvider?.id; + final currentTileTypeId = tileTypeId ?? appState.selectedTileType?.id; final offlineService = OfflineAreaService(); await offlineService.ensureInitialized(); @@ -20,7 +30,7 @@ Future> fetchLocalTile({required int z, required int x, required int y if (z < area.minZoom || z > area.maxZoom) continue; // Only consider areas that match the current provider/type - if (area.tileProviderId != currentProvider?.id || area.tileTypeId != currentTileType?.id) continue; + if (area.tileProviderId != currentProviderId || area.tileTypeId != currentTileTypeId) continue; // Get tile coverage for area at this zoom only final coveredTiles = computeTileList(area.bounds, z, z); @@ -35,7 +45,7 @@ Future> fetchLocalTile({required int z, required int x, required int y } } if (candidates.isEmpty) { - throw Exception('Tile $z/$x/$y from current provider ${currentProvider?.id}/${currentTileType?.id} not found in any offline area'); + throw Exception('Tile $z/$x/$y from provider $currentProviderId/$currentTileTypeId not found in any offline area'); } candidates.sort((a, b) => b.modified.compareTo(a.modified)); // newest first return await candidates.first.file.readAsBytes(); diff --git a/lib/services/map_data_submodules/tiles_from_remote.dart b/lib/services/map_data_submodules/tiles_from_remote.dart deleted file mode 100644 index e3e1f1f..0000000 --- a/lib/services/map_data_submodules/tiles_from_remote.dart +++ /dev/null @@ -1,265 +0,0 @@ -import 'dart:math'; -import 'dart:io'; -import 'dart:async'; -import 'package:flutter/foundation.dart'; -import 'package:latlong2/latlong.dart'; -import 'package:flutter_map/flutter_map.dart'; -import 'package:deflockapp/dev_config.dart'; -import 'package:deflockapp/services/http_client.dart'; - -/// Global semaphore to limit simultaneous tile fetches -final _tileFetchSemaphore = _SimpleSemaphore(kTileFetchConcurrentThreads); - -/// Clear queued tile requests when map view changes significantly -void clearRemoteTileQueue() { - final clearedCount = _tileFetchSemaphore.clearQueue(); - // Only log if we actually cleared something significant - if (clearedCount > 5) { - debugPrint('[RemoteTiles] Cleared $clearedCount queued tile requests'); - } -} - -/// Clear only tile requests that are no longer visible in the given bounds -void clearRemoteTileQueueSelective(LatLngBounds currentBounds) { - final clearedCount = _tileFetchSemaphore.clearStaleRequests((z, x, y) { - // Return true if tile should be cleared (i.e., is NOT visible) - return !_isTileVisible(z, x, y, currentBounds); - }); - - if (clearedCount > 0) { - debugPrint('[RemoteTiles] Selectively cleared $clearedCount non-visible tile requests'); - } -} - -/// Calculate retry delay using configurable backoff strategy. -/// Uses: initialDelay * (multiplier ^ (attempt - 1)) + randomJitter, capped at maxDelay -int _calculateRetryDelay(int attempt, Random random) { - // Calculate exponential backoff - final baseDelay = (kTileFetchInitialDelayMs * - pow(kTileFetchBackoffMultiplier, attempt - 1)).round(); - - // Add random jitter to avoid thundering herd - final jitter = random.nextInt(kTileFetchRandomJitterMs + 1); - - // Apply max delay cap - return (baseDelay + jitter).clamp(0, kTileFetchMaxDelayMs); -} - -/// Convert tile coordinates to lat/lng bounds for spatial filtering -class _TileBounds { - final double north, south, east, west; - _TileBounds({required this.north, required this.south, required this.east, required this.west}); -} - -/// Calculate the lat/lng bounds for a given tile -_TileBounds _tileToBounds(int z, int x, int y) { - final n = pow(2, z); - final lon1 = (x / n) * 360.0 - 180.0; - final lon2 = ((x + 1) / n) * 360.0 - 180.0; - final lat1 = _yToLatitude(y, z); - final lat2 = _yToLatitude(y + 1, z); - - return _TileBounds( - north: max(lat1, lat2), - south: min(lat1, lat2), - east: max(lon1, lon2), - west: min(lon1, lon2), - ); -} - -/// Convert tile Y coordinate to latitude -double _yToLatitude(int y, int z) { - final n = pow(2, z); - final latRad = atan(_sinh(pi * (1 - 2 * y / n))); - return latRad * 180.0 / pi; -} - -/// Hyperbolic sine function: sinh(x) = (e^x - e^(-x)) / 2 -double _sinh(double x) { - return (exp(x) - exp(-x)) / 2; -} - -/// Check if a tile intersects with the current view bounds -bool _isTileVisible(int z, int x, int y, LatLngBounds viewBounds) { - final tileBounds = _tileToBounds(z, x, y); - - // Check if tile bounds intersect with view bounds - return !(tileBounds.east < viewBounds.west || - tileBounds.west > viewBounds.east || - tileBounds.north < viewBounds.south || - tileBounds.south > viewBounds.north); -} - - - -final _tileClient = UserAgentClient(); - -/// Fetches a tile from any remote provider with unlimited retries. -/// Returns tile image bytes. Retries forever until success. -/// Brutalist approach: Keep trying until it works - no arbitrary retry limits. -Future> fetchRemoteTile({ - required int z, - required int x, - required int y, - required String url, -}) async { - int attempt = 0; - final random = Random(); - final hostInfo = Uri.parse(url).host; // For logging - - while (true) { - await _tileFetchSemaphore.acquire(z: z, x: x, y: y); - try { - // Only log on first attempt - if (attempt == 0) { - debugPrint('[fetchRemoteTile] Fetching $z/$x/$y from $hostInfo'); - } - attempt++; - final resp = await _tileClient.get(Uri.parse(url)); - - if (resp.statusCode == 200 && resp.bodyBytes.isNotEmpty) { - // Success! - if (attempt > 1) { - debugPrint('[fetchRemoteTile] SUCCESS $z/$x/$y from $hostInfo after $attempt attempts'); - } - return resp.bodyBytes; - } else { - debugPrint('[fetchRemoteTile] FAIL $z/$x/$y from $hostInfo: code=${resp.statusCode}, bytes=${resp.bodyBytes.length}'); - throw HttpException('Failed to fetch tile $z/$x/$y from $hostInfo: status ${resp.statusCode}'); - } - } catch (e) { - // Calculate delay and retry (no attempt limit - keep trying forever) - final delay = _calculateRetryDelay(attempt, random); - if (attempt == 1) { - debugPrint("[fetchRemoteTile] Attempt $attempt for $z/$x/$y from $hostInfo failed: $e. Retrying in ${delay}ms."); - } else if (attempt % 10 == 0) { - // Log every 10th attempt to show we're still working - debugPrint("[fetchRemoteTile] Still trying $z/$x/$y from $hostInfo (attempt $attempt). Retrying in ${delay}ms."); - } - await Future.delayed(Duration(milliseconds: delay)); - } finally { - _tileFetchSemaphore.release(z: z, x: x, y: y); - } - } -} - -/// Legacy function for backward compatibility -@Deprecated('Use fetchRemoteTile instead') -Future> fetchOSMTile({ - required int z, - required int x, - required int y, -}) async { - return fetchRemoteTile( - z: z, - x: x, - y: y, - url: 'https://tile.openstreetmap.org/$z/$x/$y.png', - ); -} - -/// Enhanced tile request entry that tracks coordinates for spatial filtering -class _TileRequest { - final int z, x, y; - final VoidCallback callback; - - _TileRequest({required this.z, required this.x, required this.y, required this.callback}); -} - -/// Spatially-aware counting semaphore for tile requests with deduplication -class _SimpleSemaphore { - final int _max; - int _current = 0; - final List<_TileRequest> _queue = []; - final Set _inFlightTiles = {}; // Track in-flight requests for deduplication - _SimpleSemaphore(this._max); - - Future acquire({int? z, int? x, int? y}) async { - // Create tile key for deduplication - final tileKey = '${z ?? -1}/${x ?? -1}/${y ?? -1}'; - - // If this tile is already in flight, skip the request - if (_inFlightTiles.contains(tileKey)) { - debugPrint('[SimpleSemaphore] Skipping duplicate request for $tileKey'); - return; - } - - // Add to in-flight tracking - _inFlightTiles.add(tileKey); - - if (_current < _max) { - _current++; - return; - } else { - // Check queue size limit to prevent memory bloat - if (_queue.length >= kTileFetchMaxQueueSize) { - // Remove oldest request to make room - final oldRequest = _queue.removeAt(0); - final oldKey = '${oldRequest.z}/${oldRequest.x}/${oldRequest.y}'; - _inFlightTiles.remove(oldKey); - debugPrint('[SimpleSemaphore] Queue full, dropped oldest request: $oldKey'); - } - - final c = Completer(); - final request = _TileRequest( - z: z ?? -1, - x: x ?? -1, - y: y ?? -1, - callback: () => c.complete(), - ); - _queue.add(request); - await c.future; - } - } - - void release({int? z, int? x, int? y}) { - // Remove from in-flight tracking - final tileKey = '${z ?? -1}/${x ?? -1}/${y ?? -1}'; - _inFlightTiles.remove(tileKey); - - if (_queue.isNotEmpty) { - final request = _queue.removeAt(0); - request.callback(); - } else { - _current--; - } - } - - /// Clear all queued requests (call when view changes significantly) - int clearQueue() { - final clearedCount = _queue.length; - _queue.clear(); - _inFlightTiles.clear(); // Also clear deduplication tracking - return clearedCount; - } - - /// Clear only tiles that don't pass the visibility filter - int clearStaleRequests(bool Function(int z, int x, int y) isStale) { - final initialCount = _queue.length; - final initialInFlightCount = _inFlightTiles.length; - - // Remove stale requests from queue - _queue.removeWhere((request) => isStale(request.z, request.x, request.y)); - - // Remove stale tiles from in-flight tracking - _inFlightTiles.removeWhere((tileKey) { - final parts = tileKey.split('/'); - if (parts.length == 3) { - final z = int.tryParse(parts[0]) ?? -1; - final x = int.tryParse(parts[1]) ?? -1; - final y = int.tryParse(parts[2]) ?? -1; - return isStale(z, x, y); - } - return false; - }); - - final queueClearedCount = initialCount - _queue.length; - final inFlightClearedCount = initialInFlightCount - _inFlightTiles.length; - - if (queueClearedCount > 0 || inFlightClearedCount > 0) { - debugPrint('[SimpleSemaphore] Cleared $queueClearedCount stale queue + $inFlightClearedCount stale in-flight, kept ${_queue.length}'); - } - - return queueClearedCount + inFlightClearedCount; - } -} \ No newline at end of file diff --git a/lib/widgets/map/tile_layer_manager.dart b/lib/widgets/map/tile_layer_manager.dart index d436ade..75acc72 100644 --- a/lib/widgets/map/tile_layer_manager.dart +++ b/lib/widgets/map/tile_layer_manager.dart @@ -22,7 +22,7 @@ class TileLayerManager { /// Dispose of resources void dispose() { - // No resources to dispose with the new tile provider + _tileProvider?.dispose(); } /// Check if cache should be cleared and increment rebuild key if needed. @@ -45,7 +45,8 @@ class TileLayerManager { if (shouldClear) { // Force map rebuild with new key to bust flutter_map cache _mapRebuildKey++; - // Also force new tile provider instance to ensure fresh cache + // Dispose old provider before creating a fresh one (closes HTTP client) + _tileProvider?.dispose(); _tileProvider = null; debugPrint('[TileLayerManager] *** CACHE CLEAR *** $reason changed - rebuilding map $_mapRebuildKey'); } @@ -58,7 +59,7 @@ class TileLayerManager { /// Clear the tile request queue (call after cache clear) void clearTileQueue() { - // With the new tile provider, clearing is handled by FlutterMap's internal cache + // With NetworkTileProvider, clearing is handled by FlutterMap's internal cache // We just need to increment the rebuild key to bust the cache _mapRebuildKey++; debugPrint('[TileLayerManager] Cache cleared - rebuilding map $_mapRebuildKey'); @@ -66,14 +67,12 @@ class TileLayerManager { /// Clear tile queue immediately (for zoom changes, etc.) void clearTileQueueImmediate() { - // No immediate clearing needed with the new architecture - // FlutterMap handles this naturally + // No immediate clearing needed — NetworkTileProvider aborts obsolete requests } - + /// Clear only tiles that are no longer visible in the current bounds void clearStaleRequests({required LatLngBounds currentBounds}) { - // No selective clearing needed with the new architecture - // FlutterMap's internal caching is efficient enough + // No selective clearing needed — NetworkTileProvider aborts obsolete requests } /// Build tile layer widget with current provider and type. @@ -84,16 +83,18 @@ class TileLayerManager { }) { // Create a fresh tile provider instance if we don't have one or cache was cleared _tileProvider ??= DeflockTileProvider(); - - // Use provider/type info in URL template for FlutterMap's cache key generation - // This ensures different providers/types get different cache keys - final urlTemplate = '${selectedProvider?.id ?? 'unknown'}/${selectedTileType?.id ?? 'unknown'}/{z}/{x}/{y}'; - + + // Use the actual urlTemplate from the selected tile type. Our getTileUrl() + // override handles the real URL generation; flutter_map uses urlTemplate + // internally for cache key generation. + final urlTemplate = selectedTileType?.urlTemplate + ?? '${selectedProvider?.id ?? 'unknown'}/${selectedTileType?.id ?? 'unknown'}/{z}/{x}/{y}'; + return TileLayer( - urlTemplate: urlTemplate, // Critical for cache key generation + urlTemplate: urlTemplate, userAgentPackageName: 'me.deflock.deflockapp', maxZoom: selectedTileType?.maxZoom.toDouble() ?? 18.0, tileProvider: _tileProvider!, ); } -} \ No newline at end of file +} diff --git a/test/services/deflock_tile_provider_test.dart b/test/services/deflock_tile_provider_test.dart index a09e7b2..ee4cd36 100644 --- a/test/services/deflock_tile_provider_test.dart +++ b/test/services/deflock_tile_provider_test.dart @@ -1,119 +1,251 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_map/flutter_map.dart'; +import 'package:http/http.dart' as http; import 'package:mocktail/mocktail.dart'; import 'package:deflockapp/app_state.dart'; +import 'package:deflockapp/models/tile_provider.dart' as models; import 'package:deflockapp/services/deflock_tile_provider.dart'; -import 'package:deflockapp/services/map_data_provider.dart'; class MockAppState extends Mock implements AppState {} void main() { + late DeflockTileProvider provider; + late MockAppState mockAppState; + + setUp(() { + mockAppState = MockAppState(); + AppState.instance = mockAppState; + + // Default stubs: online, OSM provider selected, no offline areas + when(() => mockAppState.offlineMode).thenReturn(false); + when(() => mockAppState.selectedTileProvider).thenReturn( + const models.TileProvider( + id: 'openstreetmap', + name: 'OpenStreetMap', + tileTypes: [], + ), + ); + when(() => mockAppState.selectedTileType).thenReturn( + const models.TileType( + id: 'osm_street', + name: 'Street Map', + urlTemplate: 'https://tile.openstreetmap.org/{z}/{x}/{y}.png', + attribution: '© OpenStreetMap', + maxZoom: 19, + ), + ); + + provider = DeflockTileProvider(); + }); + + tearDown(() async { + await provider.dispose(); + AppState.instance = MockAppState(); + }); + group('DeflockTileProvider', () { - late DeflockTileProvider provider; - late MockAppState mockAppState; - - setUp(() { - provider = DeflockTileProvider(); - mockAppState = MockAppState(); - when(() => mockAppState.selectedTileProvider).thenReturn(null); - when(() => mockAppState.selectedTileType).thenReturn(null); - AppState.instance = mockAppState; + test('supportsCancelLoading is true', () { + expect(provider.supportsCancelLoading, isTrue); }); - tearDown(() { - // Reset to a clean mock so stubbed state doesn't leak to other tests - AppState.instance = MockAppState(); + test('getTileUrl() delegates to TileType.getTileUrl()', () { + const coords = TileCoordinates(1, 2, 3); + final options = TileLayer(urlTemplate: 'ignored/{z}/{x}/{y}'); + + final url = provider.getTileUrl(coords, options); + + expect(url, equals('https://tile.openstreetmap.org/3/1/2.png')); }); - test('creates image provider for tile coordinates', () { - const coordinates = TileCoordinates(0, 0, 0); - final options = TileLayer( - urlTemplate: 'test/{z}/{x}/{y}', + test('getTileUrl() includes API key when present', () { + when(() => mockAppState.selectedTileProvider).thenReturn( + const models.TileProvider( + id: 'mapbox', + name: 'Mapbox', + apiKey: 'test_key_123', + tileTypes: [], + ), + ); + when(() => mockAppState.selectedTileType).thenReturn( + const models.TileType( + id: 'mapbox_satellite', + name: 'Satellite', + urlTemplate: + 'https://api.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}@2x.jpg90?access_token={api_key}', + attribution: '© Mapbox', + ), ); - final imageProvider = provider.getImage(coordinates, options); + const coords = TileCoordinates(1, 2, 10); + final options = TileLayer(urlTemplate: 'ignored'); - expect(imageProvider, isA()); - expect((imageProvider as DeflockTileImageProvider).coordinates, - equals(coordinates)); + final url = provider.getTileUrl(coords, options); + + expect(url, contains('access_token=test_key_123')); + expect(url, contains('/10/1/2@2x')); + }); + + test('getTileUrl() falls back to super when no provider selected', () { + when(() => mockAppState.selectedTileProvider).thenReturn(null); + when(() => mockAppState.selectedTileType).thenReturn(null); + + const coords = TileCoordinates(1, 2, 3); + final options = TileLayer(urlTemplate: 'https://example.com/{z}/{x}/{y}'); + + final url = provider.getTileUrl(coords, options); + + // Super implementation uses the urlTemplate from TileLayer options + expect(url, equals('https://example.com/3/1/2')); + }); + + test('routes to network path when no offline areas exist', () { + // offlineMode = false, OfflineAreaService not initialized → no offline areas + const coords = TileCoordinates(5, 10, 12); + final options = TileLayer(urlTemplate: 'test/{z}/{x}/{y}'); + final cancelLoading = Future.value(); + + final imageProvider = provider.getImageWithCancelLoadingSupport( + coords, + options, + cancelLoading, + ); + + // Should NOT be a DeflockOfflineTileImageProvider — it should be the + // NetworkTileImageProvider returned by super + expect(imageProvider, isNot(isA())); + }); + + test('routes to offline path when offline mode is enabled', () { + when(() => mockAppState.offlineMode).thenReturn(true); + + const coords = TileCoordinates(5, 10, 12); + final options = TileLayer(urlTemplate: 'test/{z}/{x}/{y}'); + final cancelLoading = Future.value(); + + final imageProvider = provider.getImageWithCancelLoadingSupport( + coords, + options, + cancelLoading, + ); + + expect(imageProvider, isA()); + final offlineProvider = imageProvider as DeflockOfflineTileImageProvider; + expect(offlineProvider.isOfflineOnly, isTrue); + expect(offlineProvider.coordinates, equals(coords)); + expect(offlineProvider.providerId, equals('openstreetmap')); + expect(offlineProvider.tileTypeId, equals('osm_street')); }); }); - group('DeflockTileImageProvider', () { - test('generates consistent keys for same coordinates', () { - const coordinates1 = TileCoordinates(1, 2, 3); - const coordinates2 = TileCoordinates(1, 2, 3); - const coordinates3 = TileCoordinates(1, 2, 4); - + group('DeflockOfflineTileImageProvider', () { + test('equal for same coordinates and provider/type', () { + const coords = TileCoordinates(1, 2, 3); final options = TileLayer(urlTemplate: 'test/{z}/{x}/{y}'); + final cancel = Future.value(); - final mapDataProvider = MapDataProvider(); - - final provider1 = DeflockTileImageProvider( - coordinates: coordinates1, + final a = DeflockOfflineTileImageProvider( + coordinates: coords, options: options, - mapDataProvider: mapDataProvider, - providerId: 'test_provider', - tileTypeId: 'test_type', + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_a', + tileTypeId: 'type_1', + tileUrl: 'https://example.com/3/1/2', ); - final provider2 = DeflockTileImageProvider( - coordinates: coordinates2, + final b = DeflockOfflineTileImageProvider( + coordinates: coords, options: options, - mapDataProvider: mapDataProvider, - providerId: 'test_provider', - tileTypeId: 'test_type', - ); - final provider3 = DeflockTileImageProvider( - coordinates: coordinates3, - options: options, - mapDataProvider: mapDataProvider, - providerId: 'test_provider', - tileTypeId: 'test_type', + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: true, // different — but not in == + providerId: 'prov_a', + tileTypeId: 'type_1', + tileUrl: 'https://other.com/3/1/2', // different — but not in == ); - // Same coordinates should be equal - expect(provider1, equals(provider2)); - expect(provider1.hashCode, equals(provider2.hashCode)); - - // Different coordinates should not be equal - expect(provider1, isNot(equals(provider3))); + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); }); - test('generates different keys for different providers/types', () { - const coordinates = TileCoordinates(1, 2, 3); + test('not equal for different coordinates', () { + const coords1 = TileCoordinates(1, 2, 3); + const coords2 = TileCoordinates(1, 2, 4); final options = TileLayer(urlTemplate: 'test/{z}/{x}/{y}'); - final mapDataProvider = MapDataProvider(); + final cancel = Future.value(); - final provider1 = DeflockTileImageProvider( - coordinates: coordinates, + final a = DeflockOfflineTileImageProvider( + coordinates: coords1, options: options, - mapDataProvider: mapDataProvider, - providerId: 'provider_a', + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_a', tileTypeId: 'type_1', + tileUrl: 'url1', ); - final provider2 = DeflockTileImageProvider( - coordinates: coordinates, + final b = DeflockOfflineTileImageProvider( + coordinates: coords2, options: options, - mapDataProvider: mapDataProvider, - providerId: 'provider_b', + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_a', tileTypeId: 'type_1', + tileUrl: 'url2', ); - final provider3 = DeflockTileImageProvider( - coordinates: coordinates, + + expect(a, isNot(equals(b))); + }); + + test('not equal for different provider or type', () { + const coords = TileCoordinates(1, 2, 3); + final options = TileLayer(urlTemplate: 'test/{z}/{x}/{y}'); + final cancel = Future.value(); + + final base = DeflockOfflineTileImageProvider( + coordinates: coords, options: options, - mapDataProvider: mapDataProvider, - providerId: 'provider_a', + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_a', + tileTypeId: 'type_1', + tileUrl: 'url', + ); + final diffProvider = DeflockOfflineTileImageProvider( + coordinates: coords, + options: options, + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_b', + tileTypeId: 'type_1', + tileUrl: 'url', + ); + final diffType = DeflockOfflineTileImageProvider( + coordinates: coords, + options: options, + httpClient: http.Client(), + headers: const {}, + cancelLoading: cancel, + isOfflineOnly: false, + providerId: 'prov_a', tileTypeId: 'type_2', + tileUrl: 'url', ); - // Different providers should not be equal (even with same coordinates) - expect(provider1, isNot(equals(provider2))); - expect(provider1.hashCode, isNot(equals(provider2.hashCode))); - - // Different tile types should not be equal (even with same coordinates and provider) - expect(provider1, isNot(equals(provider3))); - expect(provider1.hashCode, isNot(equals(provider3.hashCode))); + expect(base, isNot(equals(diffProvider))); + expect(base.hashCode, isNot(equals(diffProvider.hashCode))); + expect(base, isNot(equals(diffType))); + expect(base.hashCode, isNot(equals(diffType.hashCode))); }); }); }