[#4797] Fall back to the event identifier when a sequencing policy resolves nothing

[schananas]

Fixes #4797

What changed

SequencingPolicy.sequenceIdentifierFor documents an empty result as normal, but SimpleEventHandlingComponent unwrapped it with Optional.get(), under a @SuppressWarnings("OptionalGetWithoutIsPresent"). Both call sites now fall back to the event identifier.

Two shipped policies legitimately answer empty, so they threw NoSuchElementException on every event and the processor delivered nothing at all, for ever: NoOpSequencingPolicy, and SequentialPerAggregatePolicy on any store that does not set the legacy aggregate identifier.

The old behaviour was not silent: DefaultWorkPackageEventFilter catches, PropagatingErrorHandler rethrows, and WorkPackage.canHandle:321 logs "Error while detecting whether event can be handled in Work Package ... Aborting Work Package" with the Optional.get stack trace, then aborts. The permanent stall is the defect; the silence is not.

Why this fallback

FullConcurrencyPolicy already returns Optional.of(message.identifier()). A per-event identifier is how the framework encodes "no sequencing", so falling back to it makes an empty Optional mean the same thing as the policy that explicitly asks for no sequencing.

A warning when the fallback fires

SimpleEventHandlingComponent now warns once per component, naming the policy class, when a policy resolves no identifier and the fallback is used. Without it, explicitly configuring SequentialPerAggregatePolicy on a DCB store goes from a loud abort to silently getting one sequence per event -- the user asked for per-aggregate ordering and gets none.

NoOpSequencingPolicy is exempt: an empty answer there is the user's deliberate declaration that they want no sequencing, and a warning that fires on correct configuration gets muted wholesale, burying the case it exists for. Recognising it by type follows MessagingConfigurationDefaults:345, which already does !(commandSequencingPolicy instanceof NoOpSequencingPolicy) for the same reason, and NoOpSequencingPolicy's own Javadoc sanctions it: "Infrastructure components may decide upon this sequencing policy being present bypassing sequencing infrastructure at all."

The wired default never returns empty, so it never warns -- asserted by a test, not just reasoned.

Tests

PolicyCannotDetermineAnIdentifier covers all three branches, including a plain-EventHandler component, the shape a real annotated projection takes. FallbackToEventIdentifierIsLogged covers the warning: fires once for SequentialPerAggregatePolicy with no aggregate identifier, silent for NoOpSequencingPolicy, silent for the wired default.

Against the true pre-fix production file: 6 errors, all NoSuchElementException: No value present. The default-policy test correctly stays green in both directions, as a regression guard should.

One detail worth knowing if anyone tries to shrink the diff: the orElse to orElseGet change is load-bearing on its own. Optional.orElse evaluates its argument eagerly, so the old code threw even when a nested component had already answered.

Targeted run, no failures:

Class	tests
SimpleEventHandlingComponentSequencingPolicyTest	13
SimpleEventHandlingComponentTest	7
SegmentMatcherTest	3
SequentialPerAggregatePolicyTest	3

No existing test needed updating; nothing had encoded the old throwing behaviour.

For the reviewer

The behaviour change worth a second opinion: per-event identifiers mean "no sequencing", so events under NoOpSequencingPolicy may now be handled concurrently and spread across segments, where before they were handled not at all. A total read-side stall is not a contract worth preserving, but the chosen default is a judgement call.

This does not fix the separate problem that SequentialPerAggregatePolicy resolves nothing on a DCB store. With this change it degrades quietly to per-event identifiers instead of throwing. That degradation is #4803.

Merge note

bug/4803/default-sequencing-policy-dcb-degradation-docs touches the same two files in different regions. Both derive from the same base blobs off origin/main, so no conflict is expected, but merge one before rebasing the other.

[MateuszNaKodach]

Good finding! Wow. I think we can merge that :)