Group the event store storage by Bounded Context - #1673
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9892f1d834
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR isolates DefaultEventStore persistence per Bounded Context by assigning the event store’s underlying record storage to a StorageGroup derived from the context name, preventing storage backends that key by record spec from conflating multiple contexts into a single physical store.
Changes:
- Pass a context-derived
StorageGroupwhen creating the event store record storage inDefaultEventStore. - Extend
StorageGroupwithof(BoundedContextName)and update its documentation to cover context-scoped grouping. - Add tests asserting the
(group, recordType)identity reaching theStorageFactory.createRecordStorage(..)seam, plus a smallStorageGroupfactory spec.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps published snapshot version. |
| server/src/main/kotlin/io/spine/server/storage/StorageGroup.kt | Adds of(BoundedContextName) for context-based grouping and updates KDoc. |
| server/src/main/java/io/spine/server/event/store/DefaultEventStore.java | Creates event store record storage under a context-derived StorageGroup. |
| server/src/test/kotlin/io/spine/server/storage/StorageGroupSpec.kt | Verifies StorageGroup factories for entity class and context name. |
| server/src/test/kotlin/io/spine/server/event/store/EventStoreIdentitySpec.kt | Asserts event store storage identity is grouped by context and survives SystemAwareStorageFactory wrapping. |
| server/src/test/kotlin/io/spine/server/entity/storage/HistoryStorageIdentitySpec.kt | Minor punctuation fix in KDoc. |
| docs/dependencies/pom.xml | Updates generated dependencies POM version. |
| docs/dependencies/dependencies.md | Updates generated dependency report version/timestamp. |
| .agents/tasks/event-store-context-prefix.md | Task notes / follow-up plan (not reviewed per repo guidelines). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1673 +/- ##
==========================================
- Coverage 87.87% 87.82% -0.05%
==========================================
Files 1067 1067
Lines 22466 22468 +2
Branches 1090 1090
==========================================
- Hits 19741 19733 -8
- Misses 2346 2354 +8
- Partials 379 381 +2 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`DefaultEventStore` now creates its record storage under a `StorageGroup` named after the context, via the new `StorageGroup.of(BoundedContextName)`. The event stores of all contexts of an application store records of the same type, so the group is what keeps the event log of each context in its own physical storage in vendors which map equal record specifications to one table or kind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Codex review on the PR flagged that JDBC's dot-replacement table naming aliases distinct group names (`Sales.EU` vs `Sales_EU`). The encoding fix belongs to the planned `jdbc-storage` step; `StorageGroup`'s KDoc now states the injective-mapping requirement as the SPI contract, and the task plan details a collision-free scheme preserving existing table names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…me)` Normalization of the group name is deliberately a vendor concern: naming rules differ per backend (SQL identifiers vs. Datastore kinds), and any framework-level normalization would alias distinct names for all backends at once. The factory KDoc now says so explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1b6de3e to
5084bf8
Compare
This PR makes the event store of each Bounded Context land in its own physical
storage when the application runs on a storage backend which keys physical
storages by the record specification, such as
jdbc-storageorgcloud-jvm.Problem
All Bounded Contexts of an application share one
StorageFactory.DefaultEventStorecreated its record storage with noStorageGroup, andits record specification is the same for every context (
EventId→Event).Backends deriving the physical storage identity from the specification alone
therefore put the events of all contexts into one table
(
spine_core_Eventinjdbc-storage) or one Datastore kind(
spine.core.Eventingcloud-jvm):persistEvents())intermingle in one physical store;
CatchUpProcessreads viaEventStore.read(..)filtered by event type —when two contexts share an event proto type, catch-up in one context replays
the events of the other;
InMemoryStorageFactoryis unaffected only by accident: eachcreateRecordStorage(..)call returns a fresh, instance-isolated storage.This is why tests never caught the issue.
Fix
DefaultEventStorenow creates its record storage under aStorageGroupnamed after the Bounded Context, via the new
StorageGroup.of(BoundedContextName)factory:StorageGroup's documented purpose is exactly this: differentiatingstorages holding records of the same type, so a vendor does not conflate
them — per-entity histories already use it.
jdbc-storagederives theBilling_Eventtable,gcloud-jvmtheBilling-Eventkind, through their existing grouped-naming paths.InMemoryStorageFactoryignores groups — no behavior change in tests.The follow-up work in
jdbc-storageandgcloud-jvm(custom-name /custom-layout API for context-grouped storages, migration notes) is planned in
.agents/tasks/event-store-context-prefix.mdand will be addressed in separate PRs after this one lands.
Migration note (release notes material)
On group-honoring backends the event log moves to a new per-context physical
storage. Existing deployments keep their data in the old shared storage
(
spine_core_Event/ kindspine.core.Event); operators split it by thetypecolumn/property using each context's event-type set. Detailed guidanceis part of the planned
jdbc-storage/gcloud-jvmfollow-ups.Testing
EventStoreIdentitySpecasserts, at thecreateRecordStorage(..)vendor seam, that the event store arrives with the
(group, recordType)identity
(<context name>, Event), that two contexts produce two distinctidentities, and that the group passes through
SystemAwareStorageFactoryintact.
StorageGroupSpec(aClassTest) covers bothof(..)factories andnull-checks the static API.
./gradlew build dokkaGeneratepasses locally, including thebehavioral
InMemoryDefaultEventStoreTestand the delivery/catch-up suites.🤖 Generated with Claude Code