AppServices.md (5193B)
1 # AppServices Architecture Note 2 3 I would not try to "remove the god object" by scattering construction 4 everywhere. `AppServices` is doing something valuable: it gives the app one 5 obvious ownership point for long-lived services, keeps SwiftUI environment setup 6 simple, and makes startup ordering explicit. Those are real advantages. 7 8 The thing I would change is not the centralisation itself, but the number of 9 responsibilities inside that central class. 10 11 Right now `AppServices` is both: 12 13 1. The composition root: constructs `PersistenceController`, `GameStore`, 14 `SyncEngine`, `NYTAuthService`, etc. 15 2. The lifecycle coordinator: handles startup, foreground/background sync, 16 remote notifications, share acceptance, reset, and import URLs. 17 3. The wiring layer: connects `GameStore` callbacks to `SyncEngine`, connects 18 `SyncEngine` callbacks back to `GameStore`, installs tracing, and refreshes 19 identity. 20 4. A small service locator used by views. 21 22 That is why it feels dense. The problem is less "many properties" and more 23 "many behavior loops in one file." 24 25 ## Recommendation 26 27 Keep `AppServices` as the composition root. 28 29 I would still have something like: 30 31 ```swift 32 @MainActor 33 final class AppServices { 34 let persistence: PersistenceController 35 let store: GameStore 36 let sync: SyncCoordinator 37 let sharing: SharingCoordinator 38 let imports: ImportCoordinator 39 let nytAuth: NYTAuthService 40 let nytFetcher: NYTPuzzleFetcher 41 let driveMonitor: DriveMonitor 42 } 43 ``` 44 45 So the app still has one root object. SwiftUI can still receive `services`. 46 Tests and previews still have a single place to swap dependencies later. 47 Startup is still discoverable. 48 49 ## Extract Orchestration By Domain 50 51 The first extraction I would make is a `SyncCoordinator`. 52 53 It would own the current sync lifecycle behavior: 54 55 - `start(preferences:)` 56 - `syncOnForeground()` 57 - `syncOnBackground()` 58 - `handleRemoteNotification()` 59 - identity refresh 60 - `SyncMonitor` tracing 61 - `SyncEngine` callback wiring 62 - `MoveBuffer` construction/wiring, if you want to group move flushing with sync 63 64 That would remove the densest closure wiring from `AppServices` while preserving 65 the existing behavior. 66 67 Then `AppServices.start(...)` becomes mostly: 68 69 ```swift 70 func start(appDelegate: AppDelegate, preferences: PlayerPreferences) async { 71 guard !started else { return } 72 started = true 73 74 nytAuth.loadStoredSession() 75 driveMonitor.start() 76 77 appDelegate.onRemoteNotification = { await self.sync.handleRemoteNotification() } 78 appDelegate.onAcceptShare = { await self.sharing.acceptShare(metadata: $0) } 79 80 await sync.start(preferences: preferences) 81 } 82 ``` 83 84 That is a good shape for a composition root: it says what starts, not how every 85 subsystem talks internally. 86 87 ## Extract CloudKit Share And Reset Operations 88 89 `acceptShare(metadata:)`, `deleteAllPrivateZones()`, and `leaveAllSharedZones()` 90 are a coherent CloudKit-sharing/admin concern. I would put those in something 91 like `SharingCoordinator` or `CloudKitMaintenanceService`. 92 93 That class would depend on: 94 95 - `CKContainer` 96 - `SyncEngine` 97 - `SyncMonitor` 98 - maybe `GameStore` for reset 99 100 This keeps the dangerous operations, especially reset and zone deletion, out of 101 the general app service bag. 102 103 ## Extract URL Import 104 105 `handleOpenURL(_:)` is also separable. It coordinates: 106 107 - file security scope 108 - XD file loading 109 - Drive import 110 - duplicate game lookup 111 - game creation 112 113 That could become: 114 115 ```swift 116 @MainActor 117 final class GameImportService { 118 func importGame(from url: URL) -> UUID? 119 } 120 ``` 121 122 This is a nice extraction because it has a clear input/output and little 123 lifecycle complexity. 124 125 ## Avoid Over-Abstracting The Simple Services 126 127 I would not introduce protocols everywhere yet. For example, I would not 128 immediately create `GameStoreProtocol`, `SyncEngineProtocol`, `DriveMonitoring`, 129 `PuzzleFetching`, etc. unless tests or previews need them. That would make the 130 code feel more "architected" without reducing the actual complexity. 131 132 I would also avoid splitting into tiny factories like `PersistenceFactory`, 133 `SyncFactory`, or `AuthFactory`. The construction code is not the main problem. 134 The callback graph is. 135 136 ## Target Shape 137 138 A reasonable end state would be: 139 140 - `AppServices`: owns and exposes app-wide services; starts coordinators. 141 - `SyncCoordinator`: owns sync lifecycle, move buffering, sync callbacks, 142 identity, and monitoring. 143 - `SharingCoordinator`: accepts shares and maybe handles CloudKit zone 144 cleanup/reset support. 145 - `GameImportService`: handles `.xd` URLs and Drive import. 146 - `PlayerNamePublisher`: stays as-is. 147 - `GameStore`: still stores callback hooks, though over time those could become 148 a more explicit event sink. 149 150 The important design rule: `AppServices` can know about everything, but 151 everything should not require new business logic inside `AppServices`. 152 153 So I would treat the current class as acceptable but past the point where new 154 responsibilities should keep landing there. The next time you touch sync 155 lifecycle, share acceptance, reset, or file import, extract that domain then. I 156 would not do a big architecture-only rewrite unless you already have bugs around 157 startup ordering or sync callbacks.