Migrate region storage from UserDefaults to disk

[mosliem]

Summary

Closes #629

Migrates region data out of UserDefaults into proper disk-based
storage, with the following structure:

• Downloaded/server regions → Application Support/Regions/default-regions.json
• Custom regions → Documents/custom-regions/<id>.json (one file per region — synced via iCloud and accessible through Files.app)
• Currently selected region → UserDefaults (identifier only, not the full object)

Changes

OBAKitCore

• New RegionsFileStorage.swift — RegionsFileStorageProtocol + concrete
  RegionsFileStorage implementation. All directory URL construction uses
  fileManager.url(for:in:appropriateFor:create:) (throws, no force-unwraps).
  Corrupted custom region files are individually logged and skipped so other
  regions still load.
• Updated RegionsService.swift:
  • Accepts a fileStorage: RegionsFileStorageProtocol parameter (defaults to
    RegionsFileStorage() — no breaking change to existing call sites)
  • currentRegion now stores only the regionIdentifier (Int) in UserDefaults;
    the full Region is looked up at read time via find(id:), preventing stale
    object data on disk
  • currentRegion setter compares identifiers directly to avoid a disk read on
    every location update
  • One-time transparent migration from legacy UserDefaults format to disk on
    first launch; legacy keys are removed after migration so it runs only once

OBAKitTests

• New RegionsFileStorageTests.swift
• Updated RegionsServiceTests.swift

Video

region.migration.mov

[aaronbrethorst]

Hey Mohamed, I like the direction of this PR — moving region data out of UserDefaults into proper file storage is the right architectural call, and the protocol/mock design makes it cleanly testable. The TemporaryDirectoryFileManager subclass for test isolation is a nice approach. Before we can merge this, I will need you to make a couple changes:

Critical

1. Migration deletes legacy UserDefaults keys even when the save fails (RegionsService.swift:297, 314, 322): In migrateFromUserDefaultsIfNeeded, userDefaults.removeObject(forKey:) runs unconditionally — regardless of whether saveDefaultRegions threw or whether try? PropertyListDecoder().decode(...) returned nil. If the disk write fails (out of storage, sandbox error) or the decode fails (model schema changed between versions), the legacy data is permanently deleted from UserDefaults and nothing was written to disk. On the next launch, the app silently falls back to bundled regions and the user's previously-stored region list is gone forever.

   The fix: only call removeObject inside the do block after saveDefaultRegions succeeds. For the decode failure case, either leave the legacy key intact (so migration retries on next launch) or at minimum log the raw data before deleting. The same pattern applies to all three migration blocks (default regions, custom regions, current region).

2. currentRegion getter triggers synchronous disk I/O on every access (RegionsService.swift:155-159, 200, 233-234): The getter calls find(id:), which falls back to customRegions (line 200). customRegions is a computed property that calls fileStorage.loadCustomRegions() — which does contentsOfDirectory + Data(contentsOf:) for every file — on every single property access. currentRegion is read from MapViewController, StopViewController, BookmarksViewController, SearchInteractor, and many other hot paths (including updateCurrentRegionFromLocation, which fires on every location update). For any user with a custom region selected, this means synchronous disk I/O on every location callback. Even for users with a standard region, find(id:) still falls through to the disk read when the identifier isn't found in the in-memory regions array.

   The fix: cache custom regions in memory (invalidate on add/delete), or cache the resolved currentRegion object and invalidate when regions changes or when the UserDefaults identifier changes.

Important

3. loadCustomRegions error handling is asymmetric with loadDefaultRegions (RegionsFileStorage.swift:18 vs 25): loadDefaultRegions() throws errors to the caller. loadCustomRegions() silently swallows all errors (URL resolution failure, directory listing failure) and returns []. The caller cannot distinguish "no custom regions exist" from "the Documents directory is inaccessible." Since saveCustomRegion and deleteCustomRegion both throw, loadCustomRegions should also throw for total-failure conditions. Per-file decode errors can still be caught and skipped internally (the compactMap pattern is fine for partial failures).

4. No test for the migration failure path (RegionsServiceTests.swift): The three migration tests only cover the success path. There's no test verifying behavior when saveDefaultRegions throws during migration — which is exactly the scenario from Critical issue #1. A test that configures MockRegionsFileStorage to throw on save, populates a legacy UserDefaults key, and asserts the legacy key is NOT deleted would catch this data-loss bug and prevent regressions.

Fit and Finish

5. currentRegion setter silently ignores nil (RegionsService.swift:162): guard let newValue else { return } means you can never deselect a region. The getter can return nil (when no identifier is stored, or when find(id:) returns nil because the region was deleted), but the setter refuses nil. This asymmetry was pre-existing, but it's worth a brief doc comment on the property explaining the contract.

6. Throwing computed properties (RegionsFileStorage.swift:49-71): var defaultRegionsFileURL: URL { get throws } is valid Swift but unusual — most Swift codebases express fallible URL construction as functions. Consider converting to func defaultRegionsFileURL() throws -> URL for consistency with common Swift conventions.

Thanks again, and I look forward to merging this change.