From 989ca826184433cb68dc5286bf68c343d98fb8e8 Mon Sep 17 00:00:00 2001 From: Bretton Date: Fri, 7 Aug 2026 13:18:58 -0700 Subject: [PATCH] refactor(di): share one CovesApiService instance app-wide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously ~14 call sites each constructed their own CovesApiService (each with its own Dio stack and interceptor chain); two providers used a divergent session?.token getter that would silently miss future refresh logic, and create_post_screen leaked a new client per submit. Construct the client once in bootstrapCovesApp(), wired to AuthProvider.getAccessToken/refreshToken/signOut, and inject it everywhere: providers take a required constructor param (the optional `apiService ?? CovesApiService(...)` fallbacks are gone — that path is what let call sites drift), widgets read Provider from the tree. Provider-level dispose() of the service is removed: the instance is app-lifetime, and an LRU-evicted CommentsProvider closing the shared Dio would have broken every other consumer. Also: UserProfileProvider now receives the shared CommentService (required) instead of building its own, its updateAuthProvider no longer disposes/recreates the service (callbacks are bound to the singleton bootstrap AuthProvider, now enforced by an assert in main.dart), and the dead `_apiService == null` error branches are deleted. BREAKING (fail-fast contract): screens now hard-require Provider ancestry and throw ProviderNotFoundException at initState if pumped outside the app's MultiProvider, where they previously rendered a recoverable error state. Widget tests must provide the service (see postCardProviders in fake_providers.dart). Reviewed via multi-model second opinion (6 streams); applied both Important findings (vacuous post_detail_loader default-fetcher test, postCardProviders missing the new provider) and added dispose-invariant tests pinning that providers never dispose the injected shared client. Co-Authored-By: Claude Fable 5 --- lib/main.dart | 48 ++++++++++++--- lib/providers/comments_provider.dart | 20 ++---- .../community_subscription_provider.dart | 10 +-- lib/providers/multi_feed_provider.dart | 24 ++------ lib/providers/user_profile_provider.dart | 44 +++---------- .../community/community_feed_screen.dart | 26 ++------ .../compose/community_picker_screen.dart | 25 +++----- lib/screens/home/communities_admin_panel.dart | 31 ++-------- .../home/communities_discovery_screen.dart | 61 ++----------------- .../home/communities_see_all_screen.dart | 34 ++--------- lib/screens/home/create_post_screen.dart | 9 +-- lib/screens/home/post_detail_loader.dart | 19 +----- lib/services/comments_provider_cache.dart | 7 ++- lib/widgets/post_card_actions.dart | 8 +-- lib/widgets/report_dialog.dart | 15 ++--- test/providers/comments_provider_test.dart | 16 +++++ .../community_subscription_provider_test.dart | 12 ++++ test/screens/main_shell_screen_test.dart | 14 ++++- test/test_helpers/fake_providers.dart | 11 +++- test/widget_test.dart | 11 +++- test/widgets/feed_screen_test.dart | 6 +- test/widgets/post_detail_loader_test.dart | 45 ++++++++++---- 22 files changed, 205 insertions(+), 291 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index c244f18..e3a2c74 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -117,6 +117,19 @@ Future bootstrapCovesApp() async { ); } + // Single app-wide Coves API client (one Dio stack / connection pool). + // Constructed once here and injected everywhere — providers via + // constructor, widgets via Provider. Never construct + // ad-hoc instances elsewhere: they would miss future auth wiring and + // leak their Dio stack. This instance lives for the whole app and is + // intentionally never disposed; CovesApiService.dispose() exists for + // test-local instances only. + final apiService = CovesApiService( + tokenGetter: authProvider.getAccessToken, + tokenRefresher: authProvider.refreshToken, + signOutHandler: authProvider.signOut, + ); + // Initialize vote service with auth callbacks // Votes go through the Coves backend (which proxies to PDS with DPoP) // Includes token refresh and sign-out handlers for automatic 401 recovery @@ -147,18 +160,19 @@ Future bootstrapCovesApp() async { authProvider: authProvider, ), ), + // Expose the shared API client so screens/widgets can context.read it + Provider.value(value: apiService), ChangeNotifierProvider( create: - (_) => CommunitySubscriptionProvider(authProvider: authProvider), + (_) => CommunitySubscriptionProvider( + authProvider: authProvider, + apiService: apiService, + ), ), ChangeNotifierProvider( create: (_) => BlockProvider( - apiService: CovesApiService( - tokenGetter: () async => authProvider.session?.token, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ), + apiService: apiService, authProvider: authProvider, ), ), @@ -171,6 +185,7 @@ Future bootstrapCovesApp() async { create: (context) => MultiFeedProvider( authProvider, + apiService: apiService, voteProvider: context.read(), subscriptionProvider: context.read(), @@ -180,6 +195,7 @@ Future bootstrapCovesApp() async { return previous ?? MultiFeedProvider( auth, + apiService: apiService, voteProvider: vote, subscriptionProvider: subscription, ); @@ -193,6 +209,7 @@ Future bootstrapCovesApp() async { authProvider: authProvider, voteProvider: context.read(), commentService: commentService, + apiService: apiService, ), update: (context, auth, vote, previous) { // Reuse existing cache @@ -201,6 +218,7 @@ Future bootstrapCovesApp() async { authProvider: auth, voteProvider: vote, commentService: commentService, + apiService: apiService, ); }, dispose: (_, cache) => cache.dispose(), @@ -216,12 +234,28 @@ Future bootstrapCovesApp() async { create: (context) => UserProfileProvider( authProvider, + apiService: apiService, voteProvider: context.read(), + commentService: commentService, ), update: (context, auth, vote, previous) { + // The shared apiService/commentService auth callbacks are bound + // to the bootstrap AuthProvider instance; a different instance + // flowing through here would leave them stale. + assert( + identical(auth, authProvider), + 'AuthProvider instance changed: shared service auth callbacks ' + 'are bound to the bootstrap instance', + ); // Propagate auth changes to existing provider previous?.updateAuthProvider(auth); - return previous ?? UserProfileProvider(auth, voteProvider: vote); + return previous ?? + UserProfileProvider( + auth, + apiService: apiService, + voteProvider: vote, + commentService: commentService, + ); }, ), ], diff --git a/lib/providers/comments_provider.dart b/lib/providers/comments_provider.dart index 1087458..b12cdc9 100644 --- a/lib/providers/comments_provider.dart +++ b/lib/providers/comments_provider.dart @@ -28,27 +28,17 @@ class CommentsProvider with ChangeNotifier { this._authProvider, { required String postUri, required String postCid, - CovesApiService? apiService, + required CovesApiService apiService, VoteProvider? voteProvider, CommentService? commentService, List? indexingRetryDelays, }) : _postUri = postUri, _postCid = postCid, + _apiService = apiService, _voteProvider = voteProvider, _commentService = commentService, _indexingRetryDelays = - indexingRetryDelays ?? _defaultIndexingRetryDelays { - // Use injected service (for testing) or create new one (for production) - // Pass token getter, refresh handler, and sign out handler to API service - // for automatic fresh token retrieval and automatic token refresh on 401 - _apiService = - apiService ?? - CovesApiService( - tokenGetter: _authProvider.getAccessToken, - tokenRefresher: _authProvider.refreshToken, - signOutHandler: _authProvider.signOut, - ); - } + indexingRetryDelays ?? _defaultIndexingRetryDelays; /// Maximum comment length in characters (matches backend limit) /// Note: This counts Unicode grapheme clusters, so emojis count correctly @@ -67,7 +57,7 @@ class CommentsProvider with ChangeNotifier { ]; final AuthProvider _authProvider; - late final CovesApiService _apiService; + final CovesApiService _apiService; final VoteProvider? _voteProvider; final CommentService? _commentService; final List _indexingRetryDelays; @@ -970,8 +960,6 @@ class CommentsProvider with ChangeNotifier { _isDisposed = true; // Stop time updates and cancel timer (also sets value to null) stopTimeUpdates(); - // Dispose API service - _apiService.dispose(); // Dispose the ValueNotifier last _currentTimeNotifier.dispose(); super.dispose(); diff --git a/lib/providers/community_subscription_provider.dart b/lib/providers/community_subscription_provider.dart index 62b12e9..dd95eac 100644 --- a/lib/providers/community_subscription_provider.dart +++ b/lib/providers/community_subscription_provider.dart @@ -13,15 +13,9 @@ import 'auth_provider.dart'; class CommunitySubscriptionProvider with ChangeNotifier { CommunitySubscriptionProvider({ required AuthProvider authProvider, - CovesApiService? apiService, + required CovesApiService apiService, }) : _authProvider = authProvider, - _apiService = - apiService ?? - CovesApiService( - tokenGetter: () async => authProvider.session?.token, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ) { + _apiService = apiService { // Listen to auth state changes and clear subscriptions on sign-out _authProvider.addListener(_onAuthChanged); } diff --git a/lib/providers/multi_feed_provider.dart b/lib/providers/multi_feed_provider.dart index d4166b9..e4342a2 100644 --- a/lib/providers/multi_feed_provider.dart +++ b/lib/providers/multi_feed_provider.dart @@ -22,28 +22,17 @@ enum FeedType { /// Manages independent state for multiple feeds (Discover and For You). /// Each feed maintains its own posts, scroll position, and pagination state. /// -/// IMPORTANT: Accepts AuthProvider reference to fetch fresh access -/// tokens before each authenticated request (critical for atProto OAuth -/// token rotation). +/// The [CovesApiService] is injected (shared app-wide, owned by main.dart) +/// and must not be disposed here. class MultiFeedProvider with ChangeNotifier { MultiFeedProvider( this._authProvider, { - CovesApiService? apiService, + required CovesApiService apiService, VoteProvider? voteProvider, CommunitySubscriptionProvider? subscriptionProvider, - }) : _voteProvider = voteProvider, + }) : _apiService = apiService, + _voteProvider = voteProvider, _subscriptionProvider = subscriptionProvider { - // Use injected service (for testing) or create new one (for production) - // Pass token getter, refresh handler, and sign out handler to API service - // for automatic fresh token retrieval and automatic token refresh on 401 - _apiService = - apiService ?? - CovesApiService( - tokenGetter: _authProvider.getAccessToken, - tokenRefresher: _authProvider.refreshToken, - signOutHandler: _authProvider.signOut, - ); - // Track initial auth state _wasAuthenticated = _authProvider.isAuthenticated; @@ -83,7 +72,7 @@ class MultiFeedProvider with ChangeNotifier { } final AuthProvider _authProvider; - late final CovesApiService _apiService; + final CovesApiService _apiService; final VoteProvider? _voteProvider; final CommunitySubscriptionProvider? _subscriptionProvider; @@ -440,7 +429,6 @@ class MultiFeedProvider with ChangeNotifier { stopTimeUpdates(); // Remove auth listener to prevent memory leaks _authProvider.removeListener(_onAuthChanged); - _apiService.dispose(); super.dispose(); } } diff --git a/lib/providers/user_profile_provider.dart b/lib/providers/user_profile_provider.dart index cc3cdec..ec8b892 100644 --- a/lib/providers/user_profile_provider.dart +++ b/lib/providers/user_profile_provider.dart @@ -15,62 +15,37 @@ import 'vote_provider.dart'; /// Manages state for user profile pages including profile data and /// author posts feed. Supports viewing both own profile and other users. /// -/// IMPORTANT: Accepts AuthProvider reference to fetch fresh access -/// tokens before each authenticated request (critical for atProto OAuth -/// token rotation). +/// The [CovesApiService] is injected (shared app-wide, owned by main.dart) +/// and must not be disposed here. Its auth callbacks are bound to the +/// app-level [AuthProvider], so they stay valid across auth state changes. class UserProfileProvider with ChangeNotifier { UserProfileProvider( AuthProvider authProvider, { - CovesApiService? apiService, + required CovesApiService apiService, + required CommentService commentService, VoteProvider? voteProvider, - CommentService? commentService, }) : _authProvider = authProvider, + _apiService = apiService, + _commentService = commentService, _voteProvider = voteProvider { - _apiService = - apiService ?? - CovesApiService( - tokenGetter: _authProvider.getAccessToken, - tokenRefresher: _authProvider.refreshToken, - signOutHandler: _authProvider.signOut, - ); - - // Create CommentService if not provided (for delete functionality) - _commentService = - commentService ?? - CommentService( - sessionGetter: () async => _authProvider.session, - tokenRefresher: _authProvider.refreshToken, - signOutHandler: _authProvider.signOut, - ); - // Listen to auth state changes _authProvider.addListener(_onAuthChanged); } AuthProvider _authProvider; final VoteProvider? _voteProvider; - late final CommentService _commentService; + final CommentService _commentService; /// Update auth provider reference (called by ChangeNotifierProxyProvider) - /// - /// This ensures token refresh and sign-out handlers stay in sync when - /// auth state changes propagate through the provider tree. void updateAuthProvider(AuthProvider newAuth) { if (_authProvider != newAuth) { _authProvider.removeListener(_onAuthChanged); _authProvider = newAuth; _authProvider.addListener(_onAuthChanged); - // Recreate API service with new auth callbacks - _apiService.dispose(); - _apiService = CovesApiService( - tokenGetter: _authProvider.getAccessToken, - tokenRefresher: _authProvider.refreshToken, - signOutHandler: _authProvider.signOut, - ); } } - late CovesApiService _apiService; + final CovesApiService _apiService; // Profile state UserProfile? _profile; @@ -677,7 +652,6 @@ class UserProfileProvider with ChangeNotifier { @override void dispose() { _authProvider.removeListener(_onAuthChanged); - _apiService.dispose(); super.dispose(); } } diff --git a/lib/screens/community/community_feed_screen.dart b/lib/screens/community/community_feed_screen.dart index 13a8cd3..af7a2c4 100644 --- a/lib/screens/community/community_feed_screen.dart +++ b/lib/screens/community/community_feed_screen.dart @@ -45,7 +45,8 @@ class CommunityFeedScreen extends StatefulWidget { } class _CommunityFeedScreenState extends State { - CovesApiService? _apiService; + // Shared app-wide API client (owned by main.dart) — do not dispose here + late final CovesApiService _apiService; final ScrollController _scrollController = ScrollController(); // Tab state @@ -75,6 +76,7 @@ class _CommunityFeedScreenState extends State { @override void initState() { super.initState(); + _apiService = context.read(); _community = widget.community; _scrollController.addListener(_onScroll); @@ -88,22 +90,9 @@ class _CommunityFeedScreenState extends State { @override void dispose() { _scrollController.dispose(); - _apiService?.dispose(); super.dispose(); } - CovesApiService _getApiService() { - if (_apiService == null) { - final authProvider = context.read(); - _apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - } - return _apiService!; - } - void _onScroll() { if (_scrollController.position.pixels >= _scrollController.position.maxScrollExtent - 200) { @@ -142,8 +131,7 @@ class _CommunityFeedScreenState extends State { }); try { - final apiService = _getApiService(); - final community = await apiService.getCommunity( + final community = await _apiService.getCommunity( community: widget.identifier, ); @@ -192,8 +180,7 @@ class _CommunityFeedScreenState extends State { }); try { - final apiService = _getApiService(); - final response = await apiService.getCommunityFeed( + final response = await _apiService.getCommunityFeed( community: widget.identifier, sort: _feedSort, cursor: refresh ? null : _cursor, @@ -236,8 +223,7 @@ class _CommunityFeedScreenState extends State { }); try { - final apiService = _getApiService(); - final response = await apiService.getCommunityFeed( + final response = await _apiService.getCommunityFeed( community: widget.identifier, sort: _feedSort, cursor: _cursor, diff --git a/lib/screens/compose/community_picker_screen.dart b/lib/screens/compose/community_picker_screen.dart index 4db2548..812812d 100644 --- a/lib/screens/compose/community_picker_screen.dart +++ b/lib/screens/compose/community_picker_screen.dart @@ -6,7 +6,6 @@ import 'package:provider/provider.dart'; import '../../constants/app_colors.dart'; import '../../models/community.dart'; -import '../../providers/auth_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; @@ -46,35 +45,25 @@ class _CommunityPickerScreenState extends State { String? _cursor; bool _hasMore = true; Timer? _searchDebounce; - CovesApiService? _apiService; + // Shared app-wide API client (owned by main.dart) — do not dispose here + late final CovesApiService _apiService; @override void initState() { super.initState(); + _apiService = context.read(); _searchController.addListener(_onSearchChanged); _scrollController.addListener(_onScroll); - // Defer API initialization to first frame to access context WidgetsBinding.instance.addPostFrameCallback((_) { - _initApiService(); _loadCommunities(); }); } - void _initApiService() { - final authProvider = context.read(); - _apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - } - @override void dispose() { _searchController.dispose(); _scrollController.dispose(); _searchDebounce?.cancel(); - _apiService?.dispose(); super.dispose(); } @@ -120,7 +109,7 @@ class _CommunityPickerScreenState extends State { } Future _loadCommunities() async { - if (_isLoading || _apiService == null) { + if (_isLoading) { return; } @@ -130,7 +119,7 @@ class _CommunityPickerScreenState extends State { }); try { - final response = await _apiService!.listCommunities( + final response = await _apiService.listCommunities( limit: 50, ); @@ -161,7 +150,7 @@ class _CommunityPickerScreenState extends State { } Future _loadMoreCommunities() async { - if (_isLoadingMore || !_hasMore || _cursor == null || _apiService == null) { + if (_isLoadingMore || !_hasMore || _cursor == null) { return; } @@ -170,7 +159,7 @@ class _CommunityPickerScreenState extends State { }); try { - final response = await _apiService!.listCommunities( + final response = await _apiService.listCommunities( limit: 50, cursor: _cursor, ); diff --git a/lib/screens/home/communities_admin_panel.dart b/lib/screens/home/communities_admin_panel.dart index 42d3b17..ed0d003 100644 --- a/lib/screens/home/communities_admin_panel.dart +++ b/lib/screens/home/communities_admin_panel.dart @@ -8,7 +8,6 @@ import 'package:provider/provider.dart'; import '../../constants/app_colors.dart'; import '../../models/community.dart'; import '../../models/picked_image.dart'; -import '../../providers/auth_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; import '../../utils/image_crop_utils.dart'; @@ -57,8 +56,8 @@ class _CommunitiesAdminPanelState extends State { final TextEditingController _communityHandleController = TextEditingController(); - // API service (cached to avoid repeated instantiation) - CovesApiService? _apiService; + // Shared app-wide API client (owned by main.dart) — do not dispose here + late final CovesApiService _apiService; // Form state bool _isSubmitting = false; @@ -88,6 +87,7 @@ class _CommunitiesAdminPanelState extends State { @override void initState() { super.initState(); + _apiService = context.read(); _nameController.addListener(_onTextChanged); _displayNameController.addListener(_onTextChanged); _descriptionController.addListener(_onTextChanged); @@ -103,7 +103,6 @@ class _CommunitiesAdminPanelState extends State { _displayNameController.dispose(); _descriptionController.dispose(); _communityHandleController.dispose(); - _apiService?.dispose(); super.dispose(); } @@ -144,19 +143,6 @@ class _CommunitiesAdminPanelState extends State { return true; } - /// Gets or creates the cached API service - CovesApiService _getApiService() { - if (_apiService == null) { - final authProvider = context.read(); - _apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - } - return _apiService!; - } - Future _createCommunity() async { if (!_isFormValid || _isSubmitting) return; @@ -168,9 +154,7 @@ class _CommunitiesAdminPanelState extends State { }); try { - final apiService = _getApiService(); - - final response = await apiService.createCommunity( + final response = await _apiService.createCommunity( name: _nameController.text.trim().toLowerCase(), displayName: _displayNameController.text.trim(), description: _descriptionController.text.trim(), @@ -292,8 +276,7 @@ class _CommunitiesAdminPanelState extends State { }); try { - final apiService = _getApiService(); - final response = await apiService.listCommunities(); + final response = await _apiService.listCommunities(); if (mounted) { if (kDebugMode) { @@ -1053,9 +1036,7 @@ class _CommunitiesAdminPanelState extends State { ); } - final apiService = _getApiService(); - - await apiService.updateCommunity( + await _apiService.updateCommunity( communityDid: _selectedCommunity!.did, imageBytes: imageBytes, mimeType: mimeType, diff --git a/lib/screens/home/communities_discovery_screen.dart b/lib/screens/home/communities_discovery_screen.dart index 48155ef..0af1920 100644 --- a/lib/screens/home/communities_discovery_screen.dart +++ b/lib/screens/home/communities_discovery_screen.dart @@ -64,32 +64,23 @@ class _CommunitiesDiscoveryScreenState String? _newError; bool _hasLoaded = false; - CovesApiService? _apiService; + // Shared app-wide API client (owned by main.dart) — do not dispose here + late final CovesApiService _apiService; @override void initState() { super.initState(); + _apiService = context.read(); _searchController.addListener(_onSearchChanged); WidgetsBinding.instance.addPostFrameCallback((_) { - _initApiService(); _loadAllSections(); }); } - void _initApiService() { - final authProvider = context.read(); - _apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - } - @override void dispose() { _searchController.dispose(); _searchDebounce?.cancel(); - _apiService?.dispose(); super.dispose(); } @@ -117,19 +108,6 @@ class _CommunitiesDiscoveryScreenState if (isAuthenticated) _isLoadingSubscribed = true; }); - // Guard against null API service (initialization may have failed) - if (_apiService == null) { - setState(() { - _subscribedError = 'Service not initialized. Pull to retry.'; - _popularError = 'Service not initialized. Pull to retry.'; - _newError = 'Service not initialized. Pull to retry.'; - _isLoadingSubscribed = false; - _isLoadingPopular = false; - _isLoadingNew = false; - }); - return; - } - // Fire all requests in parallel await Future.wait([ if (isAuthenticated) _loadSubscribed(), @@ -159,7 +137,7 @@ class _CommunitiesDiscoveryScreenState required String fallbackError, }) async { try { - final response = await _apiService!.listCommunities( + final response = await _apiService.listCommunities( limit: limit, sort: sort, subscribed: subscribed, @@ -272,42 +250,13 @@ class _CommunitiesDiscoveryScreenState } Future _loadFullCommunityList(String query) async { - if (_apiService == null) { - // Fall back to partial data if API service isn't initialized - if (kDebugMode) { - debugPrint( - 'CommunitiesDiscoveryScreen: _apiService is null during search, ' - 'falling back to partial data', - ); - } - unawaited( - Sentry.addBreadcrumb( - Breadcrumb( - message: 'Search fell back to partial data: _apiService was null', - category: 'communities.search', - level: SentryLevel.warning, - ), - ), - ); - _allCommunities = _deduplicateCommunities([ - ..._subscribedCommunities, - ..._popularCommunities, - ..._newCommunities, - ]); - setState(() { - _isUsingPartialData = _allCommunities.isNotEmpty; - }); - _applySearchFilter(query); - return; - } - setState(() { _isLoadingFullList = true; _searchQuery = query; }); try { - final response = await _apiService!.listCommunities(limit: 100); + final response = await _apiService.listCommunities(limit: 100); if (mounted) { _allCommunities = response.communities; diff --git a/lib/screens/home/communities_see_all_screen.dart b/lib/screens/home/communities_see_all_screen.dart index 538f08e..fae7378 100644 --- a/lib/screens/home/communities_see_all_screen.dart +++ b/lib/screens/home/communities_see_all_screen.dart @@ -8,7 +8,6 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import '../../constants/app_colors.dart'; import '../../models/community.dart'; -import '../../providers/auth_provider.dart'; import '../../services/api_exceptions.dart'; import '../../services/coves_api_service.dart'; import '../../utils/community_search_utils.dart'; @@ -47,34 +46,25 @@ class _CommunitiesSeeAllScreenState extends State { String? _cursor; bool _hasMore = true; Timer? _searchDebounce; - CovesApiService? _apiService; + // Shared app-wide API client (owned by main.dart) — do not dispose here + late final CovesApiService _apiService; @override void initState() { super.initState(); + _apiService = context.read(); _searchController.addListener(_onSearchChanged); _scrollController.addListener(_onScroll); WidgetsBinding.instance.addPostFrameCallback((_) { - _initApiService(); _loadCommunities(); }); } - void _initApiService() { - final authProvider = context.read(); - _apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - } - @override void dispose() { _searchController.dispose(); _scrollController.dispose(); _searchDebounce?.cancel(); - _apiService?.dispose(); super.dispose(); } @@ -108,13 +98,6 @@ class _CommunitiesSeeAllScreenState extends State { Future _loadCommunities() async { if (_isLoading) return; - if (_apiService == null) { - setState(() { - _error = 'Unable to connect. Please try again.'; - _isLoading = false; - }); - return; - } setState(() { _isLoading = true; @@ -122,7 +105,7 @@ class _CommunitiesSeeAllScreenState extends State { }); try { - final response = await _apiService!.listCommunities( + final response = await _apiService.listCommunities( limit: 50, sort: widget.sort, subscribed: widget.subscribed, @@ -160,20 +143,13 @@ class _CommunitiesSeeAllScreenState extends State { Future _loadMoreCommunities() async { if (_isLoadingMore || !_hasMore || _cursor == null) return; - if (_apiService == null) { - setState(() { - _error = 'Unable to connect. Please try again.'; - _isLoadingMore = false; - }); - return; - } setState(() { _isLoadingMore = true; }); try { - final response = await _apiService!.listCommunities( + final response = await _apiService.listCommunities( limit: 50, cursor: _cursor, sort: widget.sort, diff --git a/lib/screens/home/create_post_screen.dart b/lib/screens/home/create_post_screen.dart index a183a2d..b70f9c2 100644 --- a/lib/screens/home/create_post_screen.dart +++ b/lib/screens/home/create_post_screen.dart @@ -171,13 +171,8 @@ class _CreatePostScreenState extends State try { final authProvider = context.read(); - - // Create API service with auth - final apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); + // Shared app-wide API client (owned by main.dart) + final apiService = context.read(); // Build embed if URL is provided ExternalEmbedInput? embed; diff --git a/lib/screens/home/post_detail_loader.dart b/lib/screens/home/post_detail_loader.dart index c3ff54a..c508ade 100644 --- a/lib/screens/home/post_detail_loader.dart +++ b/lib/screens/home/post_detail_loader.dart @@ -46,9 +46,6 @@ class PostDetailLoader extends StatefulWidget { } class _PostDetailLoaderState extends State { - /// API service created for the default fetcher (null when injected) - CovesApiService? _apiService; - /// Result of the fetch, null while loading or on error PostGetResult? _result; @@ -76,12 +73,6 @@ class _PostDetailLoaderState extends State { } } - @override - void dispose() { - _apiService?.dispose(); - super.dispose(); - } - /// Resets state, invalidates in-flight fetches, and starts a new load. /// /// Callers must ensure a rebuild is already scheduled (initState, @@ -105,7 +96,7 @@ class _PostDetailLoaderState extends State { setState(_startLoad); } - /// Resolves the fetcher: injected override or a lazily created API service + /// Resolves the fetcher: injected override or the shared API service PostFetcher _resolveFetcher() { final injected = widget.fetchPost; if (injected != null) { @@ -113,13 +104,7 @@ class _PostDetailLoaderState extends State { } // context.read doesn't subscribe, so it's safe outside of build - final authProvider = context.read(); - _apiService ??= CovesApiService( - tokenGetter: () async => authProvider.session?.token, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - return _apiService!.getPost; + return context.read().getPost; } Future _fetch() async { diff --git a/lib/services/comments_provider_cache.dart b/lib/services/comments_provider_cache.dart index 436cfd5..e084c1c 100644 --- a/lib/services/comments_provider_cache.dart +++ b/lib/services/comments_provider_cache.dart @@ -5,6 +5,7 @@ import '../providers/auth_provider.dart'; import '../providers/comments_provider.dart'; import '../providers/vote_provider.dart'; import 'comment_service.dart'; +import 'coves_api_service.dart'; /// Comments Provider Cache /// @@ -29,10 +30,12 @@ class CommentsProviderCache { required AuthProvider authProvider, required VoteProvider voteProvider, required CommentService commentService, + required CovesApiService apiService, this.maxSize = 15, }) : _authProvider = authProvider, _voteProvider = voteProvider, - _commentService = commentService { + _commentService = commentService, + _apiService = apiService { _wasAuthenticated = _authProvider.isAuthenticated; _authProvider.addListener(_onAuthChanged); } @@ -40,6 +43,7 @@ class CommentsProviderCache { final AuthProvider _authProvider; final VoteProvider _voteProvider; final CommentService _commentService; + final CovesApiService _apiService; /// Maximum number of providers to cache final int maxSize; @@ -125,6 +129,7 @@ class CommentsProviderCache { // Create new provider final provider = CommentsProvider( _authProvider, + apiService: _apiService, voteProvider: _voteProvider, commentService: _commentService, postUri: postUri, diff --git a/lib/widgets/post_card_actions.dart b/lib/widgets/post_card_actions.dart index 1a0e829..b3b9947 100644 --- a/lib/widgets/post_card_actions.dart +++ b/lib/widgets/post_card_actions.dart @@ -213,11 +213,8 @@ class _PostCardActionsState extends State { if (!context.mounted) return; final messenger = ScaffoldMessenger.of(context); - final apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); + // Shared app-wide API client (owned by main.dart) — do not dispose + final apiService = context.read(); try { await apiService.deletePost(uri: post.post.uri); @@ -288,7 +285,6 @@ class _PostCardActionsState extends State { ); } } finally { - apiService.dispose(); if (mounted) { setState(() => _isDeleting = false); } diff --git a/lib/widgets/report_dialog.dart b/lib/widgets/report_dialog.dart index dc43289..73511f5 100644 --- a/lib/widgets/report_dialog.dart +++ b/lib/widgets/report_dialog.dart @@ -4,7 +4,6 @@ import 'package:flutter/services.dart'; import 'package:provider/provider.dart'; import '../constants/app_colors.dart'; -import '../providers/auth_provider.dart'; import '../services/api_exceptions.dart'; import '../services/coves_api_service.dart'; @@ -91,19 +90,17 @@ class _ReportDialogState extends State { _error = null; }); + // Shared app-wide API client (owned by main.dart) — do not dispose. + // Read before the first await: the dialog can be dismissed during the + // haptic call, deactivating this context. + final apiService = context.read(); + try { await HapticFeedback.lightImpact(); } on PlatformException { // Haptics not supported } - final authProvider = context.read(); - final apiService = CovesApiService( - tokenGetter: authProvider.getAccessToken, - tokenRefresher: authProvider.refreshToken, - signOutHandler: authProvider.signOut, - ); - try { await apiService.submitReport( targetUri: widget.targetUri, @@ -136,8 +133,6 @@ class _ReportDialogState extends State { _isSubmitting = false; }); } - } finally { - apiService.dispose(); } } diff --git a/test/providers/comments_provider_test.dart b/test/providers/comments_provider_test.dart index d211d3e..97f46f6 100644 --- a/test/providers/comments_provider_test.dart +++ b/test/providers/comments_provider_test.dart @@ -52,6 +52,22 @@ void main() { commentsProvider.dispose(); }); + test('dispose does not dispose the injected shared api service', () { + // The api service is app-lifetime and owned by main.dart; an + // LRU-evicted CommentsProvider closing it would break every other + // consumer's requests. + commentsProvider.dispose(); + verifyNever(mockApiService.dispose()); + // Recreate so the group tearDown's dispose() targets a live provider + commentsProvider = CommentsProvider( + mockAuthProvider, + postUri: testPostUri, + postCid: testPostCid, + apiService: mockApiService, + voteProvider: mockVoteProvider, + ); + }); + group('loadComments', () { test('should load comments successfully', () async { final mockComments = [ diff --git a/test/providers/community_subscription_provider_test.dart b/test/providers/community_subscription_provider_test.dart index 27c4a18..e051185 100644 --- a/test/providers/community_subscription_provider_test.dart +++ b/test/providers/community_subscription_provider_test.dart @@ -73,6 +73,18 @@ void main() { authProvider.dispose(); }); + test('dispose does not dispose the injected shared api service', () { + // The api service is app-lifetime and owned by main.dart; disposing + // it here would close the shared Dio stack for the whole app. + provider.dispose(); + verifyNever(mockApiService.dispose()); + // Recreate so the group tearDown's dispose() targets a live provider + provider = CommunitySubscriptionProvider( + authProvider: authProvider, + apiService: mockApiService, + ); + }); + test('toggle-then-refetch: user toggle beats a stale server seed', () async { await provider.toggleSubscription(communityDid: did); diff --git a/test/screens/main_shell_screen_test.dart b/test/screens/main_shell_screen_test.dart index 2932919..0c4ccbc 100644 --- a/test/screens/main_shell_screen_test.dart +++ b/test/screens/main_shell_screen_test.dart @@ -8,6 +8,7 @@ import 'package:coves_flutter/providers/multi_feed_provider.dart'; import 'package:coves_flutter/providers/user_profile_provider.dart'; import 'package:coves_flutter/providers/vote_provider.dart'; import 'package:coves_flutter/screens/home/main_shell_screen.dart'; +import 'package:coves_flutter/services/comment_service.dart'; import 'package:coves_flutter/services/coves_api_service.dart'; import 'package:coves_flutter/services/vote_service.dart'; import 'package:flutter/material.dart'; @@ -42,7 +43,8 @@ class FakeVoteProvider extends VoteProvider { // Fake CommunitySubscriptionProvider that never touches the network class FakeCommunitySubscriptionProvider extends CommunitySubscriptionProvider { - FakeCommunitySubscriptionProvider({required super.authProvider}); + FakeCommunitySubscriptionProvider({required super.authProvider}) + : super(apiService: CovesApiService()); @override Future loadSubscribedCommunities() async { @@ -52,7 +54,8 @@ class FakeCommunitySubscriptionProvider extends CommunitySubscriptionProvider { // Fake MultiFeedProvider that never touches the network class FakeMultiFeedProvider extends MultiFeedProvider { - FakeMultiFeedProvider() : super(FakeAuthProvider()); + FakeMultiFeedProvider() + : super(FakeAuthProvider(), apiService: CovesApiService()); @override FeedState getState(FeedType type) => FeedState.initial(); @@ -91,7 +94,11 @@ void main() { apiService: CovesApiService(), authProvider: fakeAuthProvider, ); - profileProvider = UserProfileProvider(fakeAuthProvider); + profileProvider = UserProfileProvider( + fakeAuthProvider, + apiService: CovesApiService(), + commentService: CommentService(), + ); navigatorKey = GlobalKey(); }); @@ -117,6 +124,7 @@ void main() { await tester.pumpWidget( MultiProvider( providers: [ + Provider(create: (_) => CovesApiService()), ChangeNotifierProvider.value(value: fakeAuthProvider), ChangeNotifierProvider.value( value: fakeFeedProvider, diff --git a/test/test_helpers/fake_providers.dart b/test/test_helpers/fake_providers.dart index 81c0efa..46ce22e 100644 --- a/test/test_helpers/fake_providers.dart +++ b/test/test_helpers/fake_providers.dart @@ -42,7 +42,10 @@ class FakeVoteProvider extends VoteProvider { /// CommunitySubscriptionProvider whose initial load is a no-op, so no /// pending timers or network calls leak into a test. class FakeSubscriptionProvider extends CommunitySubscriptionProvider { - FakeSubscriptionProvider({required super.authProvider, super.apiService}); + FakeSubscriptionProvider({ + required super.authProvider, + CovesApiService? apiService, + }) : super(apiService: apiService ?? tokenlessApiService()); @override Future loadSubscribedCommunities() async {} @@ -62,6 +65,12 @@ List postCardProviders({ StreamableService? streamableService, }) { return [ + // PostCardActions' delete flow and ReportDialog read the shared API + // client from the tree, mirroring the app-level wiring in main.dart. + Provider( + create: (_) => tokenlessApiService(), + dispose: (_, service) => service.dispose(), + ), ChangeNotifierProvider.value(value: auth), ChangeNotifierProvider(create: (_) => FakeVoteProvider(auth)), ChangeNotifierProvider( diff --git a/test/widget_test.dart b/test/widget_test.dart index 5dabcb9..601ac71 100644 --- a/test/widget_test.dart +++ b/test/widget_test.dart @@ -3,6 +3,7 @@ import 'package:coves_flutter/providers/auth_provider.dart'; import 'package:coves_flutter/providers/community_guidelines_provider.dart'; import 'package:coves_flutter/providers/eula_provider.dart'; import 'package:coves_flutter/providers/multi_feed_provider.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:provider/provider.dart'; @@ -47,8 +48,16 @@ void main() { ChangeNotifierProvider( create: (_) => FakeGuidelinesProvider(), ), + Provider( + create: (_) => CovesApiService(tokenGetter: () async => null), + dispose: (_, service) => service.dispose(), + ), ChangeNotifierProvider( - create: (_) => MultiFeedProvider(authProvider), + create: + (context) => MultiFeedProvider( + authProvider, + apiService: context.read(), + ), ), ], child: const CovesApp(), diff --git a/test/widgets/feed_screen_test.dart b/test/widgets/feed_screen_test.dart index 62e36ed..2f245fc 100644 --- a/test/widgets/feed_screen_test.dart +++ b/test/widgets/feed_screen_test.dart @@ -58,7 +58,8 @@ class FakeVoteProvider extends VoteProvider { // Fake CommunitySubscriptionProvider that never touches the network class FakeCommunitySubscriptionProvider extends CommunitySubscriptionProvider { - FakeCommunitySubscriptionProvider({required super.authProvider}); + FakeCommunitySubscriptionProvider({required super.authProvider}) + : super(apiService: CovesApiService()); @override Future loadSubscribedCommunities() async { @@ -68,7 +69,8 @@ class FakeCommunitySubscriptionProvider extends CommunitySubscriptionProvider { // Fake MultiFeedProvider for testing class FakeMultiFeedProvider extends MultiFeedProvider { - FakeMultiFeedProvider() : super(FakeAuthProvider()); + FakeMultiFeedProvider() + : super(FakeAuthProvider(), apiService: CovesApiService()); final Map _states = { FeedType.discover: FeedState.initial(), diff --git a/test/widgets/post_detail_loader_test.dart b/test/widgets/post_detail_loader_test.dart index 76a6686..75c33e5 100644 --- a/test/widgets/post_detail_loader_test.dart +++ b/test/widgets/post_detail_loader_test.dart @@ -9,6 +9,7 @@ import 'package:coves_flutter/screens/home/post_detail_screen.dart'; import 'package:coves_flutter/services/api_exceptions.dart'; import 'package:coves_flutter/services/comment_service.dart'; import 'package:coves_flutter/services/comments_provider_cache.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; import 'package:coves_flutter/services/vote_service.dart'; import 'package:coves_flutter/widgets/loading_error_states.dart'; import 'package:flutter/material.dart'; @@ -135,6 +136,7 @@ void main() { authProvider: fakeAuthProvider, voteProvider: voteProvider, commentService: CommentService(), + apiService: CovesApiService(), ); await tester.pumpWidget( @@ -187,6 +189,7 @@ void main() { authProvider: fakeAuthProvider, voteProvider: voteProvider, commentService: CommentService(), + apiService: CovesApiService(), ); await tester.pumpWidget( @@ -429,18 +432,22 @@ void main() { expect(find.text('Post Unavailable'), findsNothing); }); - testWidgets('default fetcher resolves AuthProvider without throwing', ( - tester, - ) async { - // No injected fetcher: the loader must build its own CovesApiService - // from AuthProvider. The test HTTP client fails every request, so the - // loader should land in a terminal (error or not-found) state - the - // point is that it never throws ProviderNotFoundException or hangs. + testWidgets('default fetcher resolves the shared CovesApiService ' + 'from the widget tree', (tester) async { + // No injected fetcher: the loader must read the app-wide + // Provider. The test HTTP client fails every request, + // so the loader should land in a terminal (error or not-found) state + // via a real fetch attempt - never hang on the loading state. final fakeAuthProvider = FakeAuthProvider(); + final apiService = CovesApiService(tokenGetter: () async => null); + addTearDown(apiService.dispose); await tester.pumpWidget( - ChangeNotifierProvider.value( - value: fakeAuthProvider, + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: fakeAuthProvider), + Provider.value(value: apiService), + ], child: const MaterialApp(home: PostDetailLoader(postUri: testUri)), ), ); @@ -452,10 +459,26 @@ void main() { tester.any(find.byType(NotFoundError)) || tester.any(find.byType(FullScreenError)); expect(reachedTerminalState, isTrue); + }); - // Unmount to exercise disposal of the lazily created API service - await tester.pumpWidget(const MaterialApp(home: Scaffold())); + testWidgets('missing Provider degrades to the error ' + 'state instead of crashing', (tester) async { + // Documents (rather than accidentally blesses) the DI-misconfiguration + // mode: without the provider, _resolveFetcher throws + // ProviderNotFoundException, which _fetch's broad catch converts into + // the terminal error UI. + final fakeAuthProvider = FakeAuthProvider(); + + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: fakeAuthProvider, + child: const MaterialApp(home: PostDetailLoader(postUri: testUri)), + ), + ); await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(find.byType(FullScreenError), findsOneWidget); }); group('PostDetailScreen.displayedCommentCount', () { -- 2.51.2