feat: add SessionStateListener callback when a PossDup message with too-low MsgSeqNum is discarded#1259
Conversation
…oo-low MsgSeqNum is discarded Fixes quickfix-j#1254
|
@mvanhorn thanks for the PR, will have a look... |
Wrap the raw session.getSessionID() argument in eq() so it is not mixed with the messageCaptor.capture() matcher in the same verify() call, which raised InvalidUseOfMatchersException. Add the ArgumentMatchers.eq import.
|
Thanks @chrjohn. Fixed in Verified locally on JDK 21: the four |
| throw new SessionException(text); | ||
| } | ||
| return validatePossDup(msg); | ||
| final boolean validPossDup = validatePossDup(msg); |
There was a problem hiding this comment.
@mvanhorn this assignment is not used at all. Maybe we can move this block to line 1836?
Moreover, I propose to remove the return value of doTargetTooLow() since it is not used anywhere.
Summary
SessionStateListenergains an observe-only callback,onPossDupMessageDiscarded(SessionID, Message), fired when a PossDupFlag=Y message is discarded by the session layer. The spec-mandated discard behavior (FIX session-layer test case 4.5.1(e)) is completely unchanged; applications that need to see these messages, like the GPW WATS case in #1254, can now observe them without forking the engine.Why this matters
@chrjohn confirmed in #1254 that the discard is spec-correct and will not change, and proposed exactly this: "some kind of handler or callback that is called on reception of a PossDup message ... I have nothing against putting in callbacks for messages that arrive in the engine anyway." The reporter confirmed a callback covers his use case and is currently running a private fork hack. The new method is a default on
SessionStateListener(all existing methods are defaults;onResendRequestSatisfiedis precedent for callbacks with context), invoked via the existingstateListenermultiplexer at the discard sites indoTargetTooLow()and the expected-seqnumvalidatePossDupfailure branch inverify(). The javadoc states the message was not processed by the application and must not be mutated.Testing
SessionTestgains cases for the too-low PossDup discard (callback fires, message untouched), the expected-seqnum OrigSendingTime failure (callback fires after the session Reject, target sequence advanced accordingly), and the no-callback path for normal messages. No Java runtime was available in the implementation environment, so CI is the verification run for the suite.Fixes #1254