Skip to content

[#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing#4295

Merged
smcvb merged 14 commits into
mainfrom
worktree-feat/override-append-condition
May 15, 2026
Merged

[#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing#4295
smcvb merged 14 commits into
mainfrom
worktree-feat/override-append-condition

Conversation

@MateuszNaKodach
Copy link
Copy Markdown
Contributor

@MateuszNaKodach MateuszNaKodach commented Mar 16, 2026

This pull request resolves #4291.

@MateuszNaKodach MateuszNaKodach self-assigned this Mar 16, 2026
@MateuszNaKodach MateuszNaKodach added Type: Feature Use to signal an issue is completely new to the project. Priority 1: Must Highest priority. A release cannot be made if this issue isn’t resolved. labels Mar 16, 2026
@MateuszNaKodach MateuszNaKodach added this to the Release 5.2.0 milestone Mar 16, 2026
@MateuszNaKodach MateuszNaKodach changed the title [#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing {WIP} [#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing Mar 16, 2026
@sonarqubecloud
Copy link
Copy Markdown

@smcvb smcvb modified the milestones: Release 5.2.0, Release 5.1.0 Mar 17, 2026
@MateuszNaKodach MateuszNaKodach marked this pull request as ready for review March 18, 2026 14:18
@MateuszNaKodach MateuszNaKodach requested a review from a team as a code owner March 18, 2026 14:18
@MateuszNaKodach MateuszNaKodach requested review from hatzlj, hjohn and smcvb and removed request for a team March 18, 2026 14:18
@MateuszNaKodach MateuszNaKodach changed the title {WIP} [#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing [#4291] feat(eventsourcing): EventStoreTransaction - allow overriding AppendCondition calculated from sourcing Mar 18, 2026
…ng `AppendCondition` calculated from sourcing

Allow users to control the AppendCondition used at commit time, beyond
what automatic sourcing provides. This enables two primary use cases:

1. Appending without sourcing — enforcing uniqueness constraints without
   first sourcing events (e.g., ensuring a course name is unique by
   checking that no matching event exists since ORIGIN).

2. Narrowing (or broadening) the append condition — sourcing broad
   criteria for state but restricting which events are considered
   conflicting at commit time.

Production changes:

- AppendCondition: add replacingCriteria(EventCriteria) abstract method
  that preserves the consistency marker while replacing the criteria.
  Named "replacingCriteria" rather than "withCriteria" (as originally
  planned) because the interface already has a static factory method
  withCriteria(EventCriteria) and Java prohibits a static and abstract
  instance method with the same signature on a sealed interface.

- NoAppendCondition: both wither methods (withMarker, replacingCriteria)
  now return DefaultAppendCondition instead of throwing. withMarker
  previously threw UnsupportedOperationException, but havingAnyTag()
  with a real marker is a valid, useful condition meaning "any event
  after this marker is a conflict." This makes NoAppendCondition
  composable (e.g., AppendCondition.none().withMarker(m) works).

- DefaultAppendCondition: implement replacingCriteria mirroring the
  existing withMarker pattern (returns this if criteria unchanged).

- EventStoreTransaction: add overrideAppendCondition(UnaryOperator)
  interface method. Input is AppendCondition.none() when no sourcing
  happened; returning none() bypasses conflict detection. Multiple
  calls compose (each receives the previous call's output).

- DefaultEventStoreTransaction: implement override using a ResourceKey
  stored in ProcessingContext (consistent with appendConditionKey,
  eventQueueKey, appendPositionKey). Uses updateResource for atomic
  composition. Override applied in attachAppendEventsStep after marker
  resolution but before appendEvents, with DEBUG logging.

- InterceptingEventStore: delegate overrideAppendCondition through
  InterceptingEventStoreTransaction.

Test coverage:

- DefaultAppendConditionTest: 2 tests for replacingCriteria
- NoAppendConditionTest: 3 tests replacing the old throwing test,
  verifying composability of withMarker and replacingCriteria
- DefaultEventStoreTransactionTest: 7 unit tests covering override
  without sourcing, after sourcing, chaining, criteria replacement,
  bypass, normal flow, and null rejection
- StorageEngineBackedEventStoreTestSuite: 3 integration tests covering
  append-without-sourcing with conflict detection, narrowed criteria
  avoiding false conflicts, and bypass via none()
… integration tests

Replace generic CourseUpdated with CourseCreated, StudentSubscribedToCourse, and
StudentUnsubscribedFromCourse event types to better illustrate real-world use cases:

- shouldAppendWithOverriddenConditionWithoutSourcing: course name uniqueness
- narrowedCriteriaShouldAvoidFalseConflict: subscription/unsubscription narrowing
- overrideReturningNoneShouldBypassConflictDetection: duplicate course name bypass

Add sourceCount() helper to avoid CourseUpdated-specific payload conversion.
… vs Aggregate hierarchy

The OverrideAppendCondition integration tests use DCB-specific patterns
(ORIGIN-based uniqueness, criteria narrowing by event type, conflict bypass)
that are not supported by aggregate-based storage engines. The aggregate-based
JPA engine uses sequence-number conflict detection via unique constraints,
ignoring AppendCondition criteria entirely.

Introduce a test suite hierarchy:

  StorageEngineBackedEventStoreTestSuite        (common: sourcing, streaming, tokens, conflicts)
  ├── DcbBasedStorageEngineBackedEventStoreTestSuite   (DCB: overrideAppendCondition tests)
  │   ├── InMemoryStorageEngineBackedEventStoreTest
  │   └── AxonServerStorageEngineBackedEventStoreIT
  └── AggregateBasedStorageEngineBackedEventStoreTestSuite  (aggregate: placeholder)
      └── AggregateBasedJpaStorageEngineBackedEventStoreIT

The 3 OverrideAppendCondition tests and DCB event records (CourseCreated,
StudentSubscribedToCourse, StudentUnsubscribedFromCourse) move from the base
suite to DcbBasedStorageEngineBackedEventStoreTestSuite. The base suite
exposes eventStore, awaitLatch, and RESOLVER as protected for subclass access.
…efault method and improve `NoAppendCondition` composability

- Updated `replacingCriteria` in `AppendCondition` to a default implementation returning `DefaultAppendCondition`.
- Modified `NoAppendCondition` to support composable operations like `withMarker` and `replacingCriteria`.
- Adjusted documentation and method references for consistency and clarity.
…e() and improve test coverage

Production: resolveAppendCondition now treats a null return from the
override operator as AppendCondition.none(), bypassing conflict detection
rather than propagating a NullPointerException to appendEvents.

Test fixes:
- Remove isInstanceOf(DefaultAppendCondition.class) assertions from
  NoAppendConditionTest — tests behavior via API (marker/criteria), not
  internal implementation types
- Pre-populate events in overrideAfterSourcingReceivesDerivedCondition
  and overrideReplacingCriteriaPreservesMarker so sourcing produces a
  non-ORIGIN marker, making the marker assertions meaningful
- Add overrideReturningNullIsTreatedAsNone test verifying the null
  handling
@MateuszNaKodach MateuszNaKodach force-pushed the worktree-feat/override-append-condition branch from 1a70409 to 4ffce43 Compare May 1, 2026 09:55
@MateuszNaKodach MateuszNaKodach requested a review from a team as a code owner May 1, 2026 09:55
Copy link
Copy Markdown
Contributor

@smcvb smcvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bunch of detail arguments to cover before we merge. Perhaps most importantly, I think this change would be important for #4388 as well. I stated we should change where this is merged in...but we can also simply port this entire PR once it's merged. I'll leave that choice up to you, @MateuszNaKodach.

/**
* Bypassing conflict detection — creating two courses with the same name.
* <p>
* Demonstrates that returning {@link AppendCondition#none()} from the override completely
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test isn't unique to DCB, right? If you'd have an aggregate-based store, you can also have AppendCondition#none.

@MateuszNaKodach MateuszNaKodach requested a review from smcvb May 6, 2026 13:18
smcvb added 6 commits May 15, 2026 14:25
Use non-sentence style per standard

#4291
Rename to replaceCriteria. It is a request to replace, not the fact replacing is already taking place

#4291
Implement as default and throw Unsupported exception. Chances are slim users have implemented this, but they should if they want to use it. So, throw

#4291
Not reuse criteria(). We'd want this method not to change because criteria() gets altered

#4291
Move override tests to base test suite. Tests that are DCB-specific should have an assumption to cover them.

#4291
Remove redundant import

#4291
Copy link
Copy Markdown
Contributor

@smcvb smcvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concerns have been addressed, hence I'm approving this pull request.

@smcvb smcvb enabled auto-merge May 15, 2026 13:29
@sonarqubecloud
Copy link
Copy Markdown

@smcvb smcvb merged commit abe103a into main May 15, 2026
8 checks passed
@smcvb smcvb deleted the worktree-feat/override-append-condition branch May 15, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority 1: Must Highest priority. A release cannot be made if this issue isn’t resolved. Type: Feature Use to signal an issue is completely new to the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EventStoreTransaction - allow overriding AppendCondition calculated from sourcing

5 participants