Skip to content

Conversation

@justSmK
Copy link
Contributor

@justSmK justSmK commented Sep 25, 2025

… refactor loader; add contract tests

- Add `DataBaseLoading` protocol (`loadPersistentContainer` / `makeInMemoryContainer` / `destroy`).
- Make `DataBaseLoader` conform to the protocol.
- Startup flow: on-disk load → destroy+retry → in-memory fallback; no fatalError on loader init.
- DI: register `DataBaseLoading`; use `StubLoader` on init failure (assertionFailure in DEBUG).

No Core Data model changes; no public API changes beyond introducing the protocol.
…ack; remove fatalError in injection

- Add DatabaseRepository protocol:
  - Lifecycle/limits: `limit`, `lifeLimitDate`, `deprecatedLimit`, `onObjectsDidChange`.
  - Metadata: `infoUpdateVersion`, `installVersion`, `instanceId`.
  - CRUD: `create(event:)`, `readEvent(by:)`, `update(event:)`, `delete(event:)`.
  - Queries/maintenance: `query(fetchLimit:retryDeadline:)`, `removeDeprecatedEventsIfNeeded()`, `countDeprecatedEvents()`, `erase()`, `countEvents()`.
  - Convenience default: `query(fetchLimit:)` uses `Constants.Database.retryDeadline`.
- Make the existing Core Data repository conform to DatabaseRepository.
- DI/Composition: depend on `DatabaseRepository` instead of a concrete class.
  - On construction failure, inject a safe Noop `StubDatabaseRepository` (logs via MindboxLogger; `assertionFailure` in DEBUG).
- Remove `fatalError` from repository injection paths to avoid startup crashes and allow graceful degradation.
Notes
- No Core Data model changes.
- Public API impact limited to adding the DatabaseRepository protocol and conformances.
- DatabaseLoader changes and tests will follow in separate commits.
…a salvage, destroy+retry, in-memory fallback

- Add low-disk gate: compute free space via filesystem attributes and bail out to in-memory store if `freeSize < diskSpaceRepairThreshold` (300 MB) without touching on-disk store.
- Implement read-only metadata salvage (`NSReadOnlyPersistentStoreOption: true`) to preserve `install`, `infoUpdate`, `instanceId` before any destructive action; re-apply metadata to the new/retried store.
- Define clear startup flow:
  1. try loadPersistentStores() → return on success;
  2. on failure → salvage metadata;
  3. if low disk → in-memory fallback (no destroy);
  4. else destroy on-disk store and retry loading;
  5. if retry fails → in-memory fallback.
- Centralize Core Data store options in `applyStandardOptions`: synchronous add, `NSFileProtectionNone`, automatic/inferred migration.
- `destroy()` uses `destroyPersistentStore` (iOS 15+ / legacy path for older OSes) and logs before/after states.
- Keep `StubLoader` for DI fallback; no `fatalError` paths in loader code.
…; add `NoopDatabaseRepository` & retry semantics coverage

- Migrate tests from concrete `MBDatabaseRepository` to the `DatabaseRepository` protocol
  - Update test fixtures/factories to accept `DatabaseRepository` instead of `MBDatabaseRepository`

- Add a dedicated test suite for `NoopDatabaseRepository`
  - Verify CRUD methods are no-ops and never throw
  - Assert `countEvents()` and `countDeprecatedEvents()` return `0`
  - Assert metadata getters return `nil` and setters are safe (no crash, no side effects)
  - Ensure `erase()` is a no-op and safe to call repeatedly

- Update `Event` contract in tests to cover retry support
  - Extend fixtures with `retryTimestamp: TimeInterval`
  - Add computed `isRetry` and pin its behavior:
    - `isRetry == false` when `retryTimestamp == 0` (new/default events)
    - `isRetry == true` when `retryTimestamp != 0` (after update)
  - Use a fixed clock/deterministic timestamps to avoid flakiness

- Notes
  - Test-only changes; no production code or Core Data model changes
  - Prepares the suite for the new loader/fallback flow by decoupling tests from the concrete repository
… `UserDefaults.standard` when `UserDefaults(suiteName:)` returns `nil`

- Avoids a hard crash in production while still surfacing the problem in debug.
- Keeps the app functional if the App Group identifier is missing/misconfigured.
- Adds a guard around UserDefaults(suiteName:) and returns `.standard` on failure.
@justSmK justSmK changed the title WMSDK-518: Fatal error MBDatabaseRepository WMSDK-518: Fix fatal error MBDatabaseRepository Sep 25, 2025
- Add `DatabaseLoaderFlowTests` with Spy subclass to cover all branches:
  - straight success (no salvage/destroy),
  - load failure + low disk → in-memory + metadata applied,
  - load failure + enough disk → destroy+retry + metadata applied,
  - repair failure → in-memory fallback.
@justSmK justSmK force-pushed the feature/WMSDK-518-fatalError-MBDatabaseRepository branch from 2a0444f to fa3d5e7 Compare September 25, 2025 19:08
@justSmK justSmK requested review from Vailence and enotniy September 25, 2025 19:08
…bserver lifecycle and refactor `DatabaseRepository` injection

- Add memory-warning handling to `MBDatabaseRepository` that’s enabled **only** for the
  `NSInMemoryStoreType` store.
- Subscribe to `UIApplication.didReceiveMemoryWarningNotification` (`queue: .main`) in `init`
  and remove the observer in `deinit`.
- On warning:
  - Create a background context and aggressively prune all `CDEvent` rows
    (`includesPropertyValues = false`), then `save()` and `reset()` the context.
  - Fire `onObjectsDidChange` on the **main** queue after pruning completes.
- Guard against re-entrancy with `isPruningOnWarning`; reset the flag on the main queue
  once the prune finishes.
- No-op for SQLite-backed stores (warning is ignored there).
- Keep the main thread lightweight: the heavy work is performed on a background context;
  main is used only to enqueue the job and deliver the callback.

Tests:
- Add `MBDatabaseRepositoryMemoryWarningTests` covering:
  - `test_InMemory_PrunesAll_OnMemoryWarning` — in-memory store is fully pruned on warning.
  - `test_SQLite_IgnoresMemoryWarning` — SQLite store ignores warning.
  - `test_InMemory_PruneIsIdempotent_WhenWarningsBurst` — multiple warnings don’t double-prune.

Notes:
- No public API changes. Behavior is active only when the SDK runs with the in-memory fallback,
  reducing OOM risk by shedding data under pressure.
@justSmK justSmK force-pushed the feature/WMSDK-518-fatalError-MBDatabaseRepository branch from 3aa672a to 326c311 Compare September 26, 2025 07:40
@justSmK justSmK requested a review from Vailence September 29, 2025 07:34
@justSmK justSmK requested a review from Vailence September 30, 2025 09:53
@justSmK justSmK merged commit 98fe421 into develop Sep 30, 2025
5 checks passed
@justSmK justSmK deleted the feature/WMSDK-518-fatalError-MBDatabaseRepository branch September 30, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants