Add rejection tests for set target sequence number action

Author: davidsteiner

crates/hotfix/src/session.rs

@@ -574,7 +574,11 @@ where
                     .try_set_next_target_seq_num(&mut self.ctx, seq_num)
                     .await;
                 if let Err(ref err) = response {
-                    warn!(?err, seq_num = seq_num.get(), "SetNextTargetSeqNum rejected");
+                    warn!(
+                        ?err,
+                        seq_num = seq_num.get(),
+                        "SetNextTargetSeqNum rejected"
+                    );
                 }
                 if responder.send(response).is_err() {
                     error!("failed to respond to SetNextTargetSeqNum request");

crates/hotfix/src/session/state.rs

@@ -250,12 +250,7 @@ impl SessionState {
         }
     }
 
-    /// Set the next expected target sequence number. Only permitted while
-    /// `Disconnected` — any other state returns `InvalidState`.
-    ///
-    /// The store stores "last seen" (see `inbound::on_sequence_reset` passing
-    /// `end - 1`), so we subtract 1 to make `next_target_seq_number()` return
-    /// `seq_num`. `NonZeroU64` guarantees the subtraction is safe.
+    /// Set the next expected target sequence number.
     pub(crate) async fn try_set_next_target_seq_num<A, S>(
         &self,
         ctx: &mut SessionCtx<A, S>,
@@ -265,12 +260,18 @@ impl SessionState {
         A: Application,
         S: MessageStore,
     {
+        // Only permitted while `Disconnected` — any other state returns `InvalidState`.
         match self {
-            SessionState::Disconnected(_) => ctx
-                .store
-                .set_target_seq_number(seq_num.get() - 1)
-                .await
-                .map_err(SetNextTargetSeqNumError::from),
+            SessionState::Disconnected(_) => {
+                // The store stores "last seen" (see `inbound::on_sequence_reset` passing `end - 1`),
+                // so we subtract 1 to make `next_target_seq_number()` return `seq_num`.
+                let target_seq_num = seq_num.get() - 1;
+
+                ctx.store
+                    .set_target_seq_number(target_seq_num)
+                    .await
+                    .map_err(SetNextTargetSeqNumError::from)
+            }
             _ => Err(SetNextTargetSeqNumError::InvalidState {
                 current: self.as_status(),
             }),

crates/hotfix/tests/session_test_cases/admin_request_tests.rs

@@ -2,7 +2,7 @@ use crate::common::actions::when;
 use crate::common::assertions::{assert_msg_type, then};
 use crate::common::cleanup::finally;
 use crate::common::setup::given_an_active_session;
-use hotfix::session::Status;
+use hotfix::session::{SetNextTargetSeqNumError, Status};
 use hotfix_message::Part;
 use hotfix_message::fix44::{MsgType, RESET_SEQ_NUM_FLAG};
 use std::num::NonZeroU64;
@@ -94,3 +94,220 @@ async fn test_set_next_target_seq_num_while_disconnected() {
         .expect("session info");
     assert_eq!(info.next_target_seq_number, 42);
 }
+
+/// Rejection: while `Active`, SetNextTargetSeqNum is refused and the store is
+/// untouched.
+#[tokio::test]
+async fn test_set_next_target_seq_num_rejected_while_active() {
+    let (session, mut counterparty) = crate::common::setup::given_an_active_session().await;
+
+    let info_before = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+
+    let result = session
+        .session_handle()
+        .set_next_target_seq_num(NonZeroU64::new(42).expect("42 is non-zero"))
+        .await;
+
+    assert!(
+        matches!(
+            result,
+            Err(SetNextTargetSeqNumError::InvalidState {
+                current: Status::Active
+            })
+        ),
+        "expected InvalidState{{ current: Active }}, got: {result:?}"
+    );
+
+    let info_after = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+    assert_eq!(
+        info_after.next_target_seq_number, info_before.next_target_seq_number,
+        "target sequence number should not change on rejection"
+    );
+
+    crate::common::cleanup::finally(&session, &mut counterparty)
+        .disconnect()
+        .await;
+}
+
+/// Rejection: while `AwaitingLogon` (we've sent our Logon, peer hasn't responded),
+/// SetNextTargetSeqNum is refused and the store is untouched.
+#[tokio::test]
+async fn test_set_next_target_seq_num_rejected_while_awaiting_logon() {
+    let (session, mut counterparty) = crate::common::setup::given_a_connected_session().await;
+
+    // wait for our outbound Logon so we're deterministically in AwaitingLogon
+    crate::common::assertions::then(&mut counterparty)
+        .receives(|msg| {
+            crate::common::assertions::assert_msg_type(msg, hotfix_message::fix44::MsgType::Logon)
+        })
+        .await;
+
+    let info_before = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+
+    let result = session
+        .session_handle()
+        .set_next_target_seq_num(NonZeroU64::new(42).expect("42 is non-zero"))
+        .await;
+
+    assert!(
+        matches!(
+            result,
+            Err(SetNextTargetSeqNumError::InvalidState {
+                current: Status::AwaitingLogon
+            })
+        ),
+        "expected InvalidState{{ current: AwaitingLogon }}, got: {result:?}"
+    );
+
+    let info_after = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+    assert_eq!(
+        info_after.next_target_seq_number,
+        info_before.next_target_seq_number
+    );
+
+    crate::common::cleanup::finally(&session, &mut counterparty)
+        .disconnect()
+        .await;
+}
+
+/// Rejection: while `AwaitingLogout` (we've sent our Logout and are waiting
+/// for the peer's reply), SetNextTargetSeqNum is refused and the store is
+/// untouched.
+#[tokio::test]
+async fn test_set_next_target_seq_num_rejected_while_awaiting_logout() {
+    use crate::common::actions::when;
+    use crate::common::assertions::{assert_msg_type, then};
+    use hotfix::message::logout::Logout;
+    use hotfix_message::fix44::MsgType;
+
+    let (session, mut counterparty) = crate::common::setup::given_an_active_session().await;
+
+    // initiate logout from our side — we stay in AwaitingLogout until the peer replies
+    when(&session).requests_disconnect().await;
+    then(&mut counterparty)
+        .receives(|msg| assert_msg_type(msg, MsgType::Logout))
+        .await;
+
+    let info_before = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+
+    let result = session
+        .session_handle()
+        .set_next_target_seq_num(NonZeroU64::new(42).expect("42 is non-zero"))
+        .await;
+
+    assert!(
+        matches!(
+            result,
+            Err(SetNextTargetSeqNumError::InvalidState {
+                current: Status::AwaitingLogout
+            })
+        ),
+        "expected InvalidState{{ current: AwaitingLogout }}, got: {result:?}"
+    );
+
+    let info_after = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+    assert_eq!(
+        info_after.next_target_seq_number,
+        info_before.next_target_seq_number
+    );
+
+    // let the peer reply so the session cleans up (do NOT call finally().disconnect()
+    // — we're already in logout).
+    when(&mut counterparty)
+        .sends_message(Logout::default())
+        .await;
+    then(&mut counterparty).gets_disconnected().await;
+}
+
+/// Rejection: while `AwaitingResend` (we detected a gap and asked the peer to
+/// resend), SetNextTargetSeqNum is refused and the store is untouched.
+#[tokio::test]
+async fn test_set_next_target_seq_num_rejected_while_awaiting_resend() {
+    use crate::common::actions::when;
+    use crate::common::test_messages::TestMessage;
+
+    let (mut session, mut counterparty) = crate::common::setup::given_an_active_session().await;
+
+    // create a gap so the session transitions to AwaitingResend
+    when(&mut counterparty)
+        .has_previously_sent(TestMessage::dummy_execution_report())
+        .await;
+    when(&mut counterparty)
+        .sends_message(TestMessage::dummy_execution_report())
+        .await;
+
+    crate::common::assertions::then(&mut session)
+        .status_changes_to(Status::AwaitingResend {
+            begin: 2,
+            end: 3,
+            attempts: 1,
+        })
+        .await;
+    crate::common::assertions::then(&mut counterparty)
+        .receives(|msg| {
+            crate::common::assertions::assert_msg_type(
+                msg,
+                hotfix_message::fix44::MsgType::ResendRequest,
+            )
+        })
+        .await;
+
+    let info_before = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+
+    let result = session
+        .session_handle()
+        .set_next_target_seq_num(NonZeroU64::new(42).expect("42 is non-zero"))
+        .await;
+
+    assert!(
+        matches!(
+            result,
+            Err(SetNextTargetSeqNumError::InvalidState {
+                current: Status::AwaitingResend { .. }
+            })
+        ),
+        "expected InvalidState{{ current: AwaitingResend{{..}} }}, got: {result:?}"
+    );
+
+    let info_after = session
+        .session_handle()
+        .get_session_info()
+        .await
+        .expect("session info");
+    assert_eq!(
+        info_after.next_target_seq_number,
+        info_before.next_target_seq_number
+    );
+
+    crate::common::cleanup::finally(&session, &mut counterparty)
+        .disconnect()
+        .await;
+}