refactor(providers): split upload & media providers out of app_providers.dart#4550
Conversation
…ers.dart First sub-batch of #4506's final wave (9a of 9c). Moves the 10-entity upload/media cluster to `lib/providers/upload_media_providers.dart`: Auth/media chain (preserved through batches 5 and 7): - _BlossomAuthAdapter (private adapter for BlossomAuthProvider) - _FirebasePerformanceAdapter (private adapter for BlossomPerformanceMonitor) - blossomAuthService - mediaViewerAuthServiceProvider (legacy `Provider<X>(...)` style) - mediaAuthInterceptor Upload chain: - blossomUploadService - uploadManager (top of upload chain — many UI consumers) API + audio: - apiService (nip98-backed) - crosspostApiClient (Bluesky-toggle settings) - audioPlaybackService `app_providers.dart` adds `import 'upload_media_providers.dart';` (back-import — `bugReportService` consumes `blossomUploadServiceProvider`) and `export 'upload_media_providers.dart';` (keeps 28 external consumer imports unchanged). No mutual import with `app_providers.dart` from the new file — all outbound deps reach already-split files: `auth_providers.dart` (authService, oauthClient, oauthConfig, nip98AuthService), `moderation_providers.dart` (ageVerificationService, contentFilterService), `environment_provider.dart` (currentEnvironmentProvider). Discovered + fixed during implementation: - AudioPlaybackService lives in the `sound_service` package, not `openvine/services/audio_playback_service.dart` (path that didn't exist) — corrected the import. Removed 9 now-unused imports from `app_providers.dart` via `dart fix`: blossom_upload_service, api_service, auth_service (no longer needed), crosspost_api_client, media_auth_interceptor, etc. Codegen moved: 6 *Provider chunks (~378 LoC) relocated from `app_providers.g.dart` into `upload_media_providers.g.dart`. This is the cleanest cluster of the wave — no in-line state machines, no bridges, no dead providers, no log-level mismatches. Pure dependency injection. After this batch, `app_providers.dart` is 1120 → 978 LoC. Two sub-batches remain (9b: Repositories, 9c: Social + foundation). Refs #4506, #4339 Wave 2
Mobile PR PreviewPreview refreshed for Last refresh:
|
rabble
left a comment
There was a problem hiding this comment.
Code Review
Overview
Pure mechanical extraction of the upload/media provider cluster (10 entities + 2 private adapters) from app_providers.dart to a new upload_media_providers.dart. Net diff: 4 files, +534/-513, all under mobile/lib/providers/. No behavior changes.
Verification performed
Pure move confirmed. Diffed the deleted blocks in app_providers.dart against the new file — _BlossomAuthAdapter, _FirebasePerformanceAdapter, and all 8 providers (blossomAuthService, mediaViewerAuthServiceProvider, mediaAuthInterceptor, blossomUploadService, uploadManager, apiService, crosspostApiClient, audioPlaybackService) are byte-identical in their new home. No drift, no "while I'm here" edits.
Codegen relocation verified. app_providers.g.dart shows 0 leftover references to any of the 8 moved provider symbols; corresponding @ProviderFor blocks all moved cleanly into upload_media_providers.g.dart. No duplication.
Consumer stability confirmed. Re-export from app_providers.dart (export 'upload_media_providers.dart';) means all 20+ consumer files (main.dart, global_upload_indicator.dart, video_providers.dart, sounds_screen.dart, test fixtures, etc.) continue to import via package:openvine/providers/app_providers.dart unchanged. Test overrides in nip05_settings_navigation_test.dart etc. still resolve through the re-export.
Bridge import justified. The kept import 'package:openvine/providers/upload_media_providers.dart'; in app_providers.dart is required because bugReportService (PR line 840) still consumes blossomUploadServiceProvider. PR description correctly flags this as a temporary bridge until batch 9c moves bugReportService.
Bloc-bridge identity rule (state_management.md). Searched for ref.read(<moved provider>) calls inside BlocProvider.create: factories — none found. The ref.read sites in individual_video_providers.dart and main.dart are inside Riverpod provider bodies and a Riverpod container init respectively, where the bridge rule doesn't apply. No regression introduced.
Lazy-create gotcha (state_management.md). None of the moved providers had side effects in their create: factory that depended on eager construction; the relocation doesn't change initialization timing.
Outbound deps. The new file imports only already-split sibling files (auth_providers.dart, moderation_providers.dart, environment_provider.dart) — no mutual import with app_providers.dart, matching the pattern claimed in the PR description.
Scope discipline. Diff is exactly the relocation; no unrelated changes, no new providers, no logic edits. dart fix --apply cleanup of 9 newly-unused imports is appropriate.
Findings
No blocking issues, no nits.
Verdict
LGTM. Cleanly-scoped mechanical refactor with no behavior change, no regressions to the rules-of-concern (lazy create, bridge identity), no dead code, no scope creep. Approve.
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
Reviewed the extraction locally and against the PR diff. Safe mechanical split; approving. Follow-up issue filed for the remaining transitive provider dependency cleanup.
NotThatKindOfDrLiz
left a comment
There was a problem hiding this comment.
Reviewed the extraction locally and against the PR diff. Safe mechanical split; approving. Follow-up issue filed for the remaining transitive provider dependency cleanup.
Summary
First sub-batch (9a of 3) of #4506's final wave. Refs #4339 Wave 2.
Moves the 10-entity upload/media cluster to
lib/providers/upload_media_providers.dart. Cleanest cluster of the entire wave — pure dependency injection, no in-line state machines, no bridges, no follow-up smells.What moved
Private adapters (preserved through batches 5/7 and now moving with their callers):
_BlossomAuthAdapter— implementsBlossomAuthProvider_FirebasePerformanceAdapter— implementsBlossomPerformanceMonitorMedia auth chain:
blossomAuthServicemediaViewerAuthServiceProvider(legacyProvider<X>(...)style)mediaAuthInterceptorUpload chain:
blossomUploadServiceuploadManager(top of upload chain)API + audio:
apiService(nip98-backed)crosspostApiClient(Bluesky-toggle settings)audioPlaybackServiceMigration pattern
app_providers.dartadds:```dart
import 'package:openvine/providers/upload_media_providers.dart'; // back-import for bugReportService
// ...
export 'upload_media_providers.dart'; // keeps 28 consumers' imports unchanged
```
bugReportService(line 945, will move with batch 9c) still consumesblossomUploadServiceProvider.No mutual import — the new file does NOT import
app_providers.dart. All outbound deps reach already-split files:auth_providers.dart(authService, oauthClient, oauthConfig, nip98AuthService),moderation_providers.dart(ageVerificationService, contentFilterService),environment_provider.dart(currentEnvironmentProvider). First batch of the wave with this shape.Discovered during implementation
AudioPlaybackServicelives in thesound_servicepackage, not atopenvine/services/audio_playback_service.dart(a path I assumed in the plan and that doesn't exist). Corrected the import.Scope cleanup
dart fix --applyremoved 9 newly-unused imports fromapp_providers.dart(blossom_upload_service, api_service, auth_service, crosspost_api_client, media_auth_interceptor, media_viewer_auth_service, performance_monitoring_service, upload_manager, audio_playback_service via sound_service).Codegen moved: 6
*Providerchunks (~378 LoC) relocated fromapp_providers.g.dartintoupload_media_providers.g.dart. Zero duplication (verified via grep).Test plan
dart run build_runner build --delete-conflicting-outputs— cleanflutter analyze lib test integration_test— clean (afterdart fix+ EOL newline fix)flutter test test/widgets/upload_failure_sheet_test.dart test/widgets/report_content_dialog_test.dart— 35 passeddart formatappliedCumulative reduction
After 9a:
app_providers.dartis 1120 → 978 LoC (66% reduction from original 2,918). Two sub-batches remain to fully exhaust #4506:Reviewer
Per established pattern, only
NotThatKindOfDrLizwill be added when CI turns green.