Skip to content

Give InMemoryModelStorage partial-update and relationship-default semantics - #30

Open
colemancda wants to merge 13 commits into
masterfrom
fix/InMemoryModelStorage-relationship-default
Open

Give InMemoryModelStorage partial-update and relationship-default semantics#30
colemancda wants to merge 13 commits into
masterfrom
fix/InMemoryModelStorage-relationship-default

Conversation

@colemancda

@colemancda colemancda commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

InMemoryModelStorage currently behaves very differently from the SQL/Core Data
backends for both attributes and to-many relationships, in ways that make it
awkward to use as a genuine drop-in ModelStorage. Neither of those backends
ever leaves a row partially formed:

  • fetch never materializes attributes. A SQL column binds NULL for
    anything an INSERT doesn't provide, and a Core Data managed object always
    has a value (nil for an unset optional) for every attribute its model
    declares. InMemoryModelStorage returns exactly what insert was given, so
    an attribute nobody has ever explicitly set to .null (rather than simply
    never provided) decoded as keyNotFound instead of the absence it actually
    represents.
  • fetch never materializes to-many relationships. Neither backend
    actually stores a to-many value on the row that owns it — a one-to-many
    (whose inverse is a to-one foreign key, e.g. a Site's
    parkingReservations, backed by ParkingReservation.site) is answered by
    scanning the destination table for rows whose foreign key points back, and a
    many-to-many (whose inverse is also to-many) by a join table either side can
    add a link to. A row whose to-many relationship key was never explicitly
    set — which includes every one-to-many, since it's supposed to be entirely
    computed and never assigned — either decoded as an empty collection
    regardless of what actually pointed to it, or threw keyNotFound if nothing
    had ever touched that key.
  • insert fully replaces a row. A SQL ON CONFLICT DO UPDATE (or a Core
    Data managed object) only touches the columns/relationships a given write
    actually provides — anything omitted is left as-is. InMemoryModelStorage.insert
    instead swapped in the whole ModelData unconditionally, so re-inserting a
    row from a write path that doesn't restate every relationship silently wiped
    whatever that relationship previously held.

These are real correctness gaps, not just decode ergonomics — the relationship
one produces silently wrong (empty) data, and the insert one is silent data
loss on any partial update.

Separately, this also migrates the whole test suite off XCTest onto Swift
Testing. Running swift test on Linux was crashing with:

Could not cast value of type '(CoreModelTests.InMemoryViewContextTests) -> @Swift.MainActor () throws -> ()' to '(CoreModelTests.InMemoryViewContextTests) -> () throws -> ()'

— the legacy XCTest runner failing to dynamically dispatch a @MainActor-isolated
test method on Linux. This was pre-existing on master, unrelated to the storage
fix above, and moving off XCTest entirely eliminates the whole crash class.

Changes

  • fetch(_:for:) / fetch(_:) now materialize:
    • Every declared attribute the row doesn't already have a value for, as
      .null.
    • Every declared to-many relationship:
      • One-to-many (inverse is .toOne): derived live by scanning the
        destination entity's rows for a foreign key pointing back, exactly the
        way SQL/Core Data compute it.
      • Many-to-many (inverse is also .toMany): unions whatever this row
        already records with anything the destination rows record pointing
        back, approximating join-table semantics without adding one.
      • To-one relationships are untouched — an absent required reference is a
        real problem, not a default.
  • insert(_:) now merges into an existing row (provided keys overwrite,
    omitted keys are preserved) instead of replacing it outright.
  • All 11 XCTest-based test files migrated to Swift Testing (@Suite/@Test/
    #expect). A couple of notes on the conversion:
    • @Test/@Suite can't be combined with a declaration-level @available
      the two CoreData-gated suites that needed macOS 12+ guard each test body
      with a runtime if #available/guard #available instead.
    • One CoreData observation test relied on RunLoop.main.run(until:) pumping
      the real main run loop, which XCTest's synchronous main-thread execution
      guaranteed but Swift Testing's concurrency model doesn't; replaced with a
      short async poll.

Test plan

  • swift test on macOS — 125 tests in 18 suites, all pass (no test lost
    relative to the pre-migration count; verified per-file during the
    conversion)
  • swift test on Linux (swift:latest, matching CI) — 91 tests in 11
    suites (CoreData-gated suites don't build there), all pass, no crash
  • Verified against a downstream consumer with a multi-entity relationship
    graph (a User related to four child entities via one-to-many
    relationships, each with several optional attributes): fetching/caching
    a profile, inserting one of each related entity, then
    re-fetching/re-caching the profile a second time now round-trips
    correctly — the derived relationships correctly reflect the related
    rows and survive the second insert, and fetching a just-created row with
    unset optional attributes or no related rows yet no longer throws
    keyNotFound.

@github-code-quality

github-code-quality Bot commented Aug 1, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: Swift

Swift / code-coverage/llvm-cov

The overall coverage in commit 03a45d3 in the fix/InMemoryModelSto... branch is 95%. The coverage in commit 476c6eb in the master branch is 96%.

Show a code coverage summary of the most impacted files.
File master 476c6eb fix/InMemoryModelSto... 03a45d3 +/-
Sources/CoreMod...ryStorage.swift 100% 92% -8%

Updated August 01, 2026 17:27 UTC

@colemancda
colemancda force-pushed the fix/InMemoryModelStorage-relationship-default branch from 5b9f727 to b5ffe20 Compare August 1, 2026 16:37
… behavior as SQL/Core Data

insert() now merges into an existing row instead of replacing it outright, so
providing only some attributes/relationships preserves whatever the row already
had for the rest — the same "only touch the columns you provided" semantics a
SQL ON CONFLICT DO UPDATE or a Core Data managed object gives for free. Without
this, re-inserting a row from a batch that doesn't touch every relationship
(e.g. a catalog sync that doesn't restate reservations written by an unrelated
flow) silently wiped those links.

fetch() now materializes every attribute and to-many relationship the schema
declares, the way a SQL row or a Core Data managed object does automatically on
read:

- An attribute nobody has ever explicitly set to .null (rather than simply
  never provided) previously decoded as keyNotFound instead of the absence it
  actually represents — a SQL column binds NULL for anything an INSERT doesn't
  provide, and a Core Data managed object is never partially formed.
- A to-many relationship is never actually stored on the row that owns it in
  the first place on either backend. A one-to-many (whose inverse is a to-one
  foreign key) is answered by scanning the destination table for rows whose
  foreign key points back, and a many-to-many (whose inverse is also to-many)
  by a join table either side can add a link to. A row whose to-many key was
  never explicitly set — which includes every one-to-many, since it's supposed
  to be entirely computed and never assigned — always decoded as keyNotFound
  regardless of what actually pointed to it. Both are now derived correctly.
  To-one relationships are left alone: an absent required reference is a real
  problem, not a default.
@colemancda
colemancda force-pushed the fix/InMemoryModelStorage-relationship-default branch from b5ffe20 to bd071ce Compare August 1, 2026 16:44
… compile under Embedded Swift

The closure-based merge overload does dynamic casting internally, disallowed
by Embedded's restrictions. Replaced with explicit loops that do the same
thing.
@Test/@suite can't be combined with a declaration-level @available, so each
test guards its body with a runtime if #available instead of the
declaration carrying the attribute directly.

Also replaced the RunLoop.main.run(until:) spin in the observation test with
a short async poll: it relied on XCTest's synchronous main-thread execution
to pump the real run loop, which Swift Testing's concurrency model doesn't
guarantee the same way.
The compound-operator and string-comparison-helper tests needed extra care:
#expect wraps its argument in a macro-rewritten closure that doesn't always
preserve the contextual type inference plain XCTAssertEqual calls had, so
array literals like [.caseInsensitive] passed to Set<...>-typed parameters
failed to infer their element type, and a && b == c parsed with the wrong
operator precedence (== binds tighter than &&). Fixed by hoisting the
Set<Comparison.Option> literals into typed local lets and adding explicit
parens around the compound-operator comparisons.
XCTAssertThrowsError without a specific error check becomes
#expect(throws: (any Error).self); with a case check it becomes a do/catch
with Issue.record on the wrong-error/no-error paths.
Renamed testInferAttributeType to inferAttributeTypeMapping: stripping the
'test' prefix would have made the @test function's own name collide with the
free function it calls (inferAttributeType(from:)), which Swift resolves to
the instance method over the module-level function inside its own body.
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.

1 participant