Improve validations for MarketToLimitOrder (#2584)

Author: nicolad

crates/model/src/orders/market_to_limit.rs

@@ -15,8 +15,12 @@
 
 use std::ops::{Deref, DerefMut};
 
+use anyhow;
 use indexmap::IndexMap;
-use nautilus_core::{UUID4, UnixNanos};
+use nautilus_core::{
+    UUID4, UnixNanos,
+    correctness::{FAILED, check_predicate_false},
+};
 use rust_decimal::Decimal;
 use serde::{Deserialize, Serialize};
 use ustr::Ustr;
@@ -33,7 +37,7 @@ use crate::{
         StrategyId, Symbol, TradeId, TraderId, Venue, VenueOrderId,
     },
     orders::OrderError,
-    types::{Currency, Money, Price, Quantity},
+    types::{Currency, Money, Price, Quantity, quantity::check_positive_quantity},
 };
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -52,7 +56,7 @@ pub struct MarketToLimitOrder {
 impl MarketToLimitOrder {
     /// Creates a new [`MarketToLimitOrder`] instance.
     #[allow(clippy::too_many_arguments)]
-    pub fn new(
+    pub fn new_checked(
         trader_id: TraderId,
         strategy_id: StrategyId,
         instrument_id: InstrumentId,
@@ -75,8 +79,21 @@ impl MarketToLimitOrder {
         tags: Option<Vec<Ustr>>,
         init_id: UUID4,
         ts_init: UnixNanos,
-    ) -> Self {
-        // TODO: Implement new_checked and check quantity positive, add error docs.
+    ) -> anyhow::Result<Self> {
+        check_positive_quantity(quantity, "quantity")?;
+
+        if let Some(disp) = display_qty {
+            check_positive_quantity(disp, "display_qty")?;
+            check_predicate_false(disp > quantity, "`display_qty` may not exceed `quantity`")?;
+        }
+
+        if time_in_force == TimeInForce::Gtd {
+            check_predicate_false(
+                expire_time.unwrap_or_default() == 0,
+                "Condition failed: `expire_time` is required for `GTD` order",
+            )?;
+        }
+
         let init_order = OrderInitialized::new(
             trader_id,
             strategy_id,
@@ -112,13 +129,66 @@ impl MarketToLimitOrder {
             exec_spawn_id,
             tags,
         );
-        Self {
+
+        Ok(Self {
             core: OrderCore::new(init_order),
             price: None, // Price will be determined on fill
             expire_time,
             is_post_only: post_only,
             display_qty,
-        }
+        })
+    }
+
+    #[allow(clippy::too_many_arguments)]
+    pub fn new(
+        trader_id: TraderId,
+        strategy_id: StrategyId,
+        instrument_id: InstrumentId,
+        client_order_id: ClientOrderId,
+        order_side: OrderSide,
+        quantity: Quantity,
+        time_in_force: TimeInForce,
+        expire_time: Option<UnixNanos>,
+        post_only: bool,
+        reduce_only: bool,
+        quote_quantity: bool,
+        display_qty: Option<Quantity>,
+        contingency_type: Option<ContingencyType>,
+        order_list_id: Option<OrderListId>,
+        linked_order_ids: Option<Vec<ClientOrderId>>,
+        parent_order_id: Option<ClientOrderId>,
+        exec_algorithm_id: Option<ExecAlgorithmId>,
+        exec_algorithm_params: Option<IndexMap<Ustr, Ustr>>,
+        exec_spawn_id: Option<ClientOrderId>,
+        tags: Option<Vec<Ustr>>,
+        init_id: UUID4,
+        ts_init: UnixNanos,
+    ) -> Self {
+        Self::new_checked(
+            trader_id,
+            strategy_id,
+            instrument_id,
+            client_order_id,
+            order_side,
+            quantity,
+            time_in_force,
+            expire_time,
+            post_only,
+            reduce_only,
+            quote_quantity,
+            display_qty,
+            contingency_type,
+            order_list_id,
+            linked_order_ids,
+            parent_order_id,
+            exec_algorithm_id,
+            exec_algorithm_params,
+            exec_spawn_id,
+            tags,
+            init_id,
+            ts_init,
+        )
+        .expect(FAILED)
     }
 }
 
@@ -404,7 +474,7 @@ impl Order for MarketToLimitOrder {
     }
 
     fn set_liquidity_side(&mut self, liquidity_side: LiquiditySide) {
-        self.liquidity_side = Some(liquidity_side)
+        self.liquidity_side = Some(liquidity_side);
     }
 
     fn would_reduce_only(&self, side: PositionSide, position_qty: Quantity) -> bool {
@@ -444,3 +514,61 @@ impl From<OrderInitialized> for MarketToLimitOrder {
         )
     }
 }
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests
+////////////////////////////////////////////////////////////////////////////////
+#[cfg(test)]
+mod tests {
+    use rstest::rstest;
+
+    use crate::{
+        enums::{OrderSide, OrderType, TimeInForce},
+        instruments::{CurrencyPair, stubs::*},
+        orders::builder::OrderTestBuilder,
+        types::Quantity,
+    };
+
+    #[rstest]
+    fn ok(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketToLimit)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .quantity(Quantity::from(1))
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(
+        expected = "Condition failed: invalid `Quantity` for 'quantity' not positive, was 0"
+    )]
+    fn quantity_zero(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketToLimit)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .quantity(Quantity::from(0)) // Invalid: zero quantity
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(expected = "Condition failed: `expire_time` is required for `GTD` order")]
+    fn gtd_without_expire(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketToLimit)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .quantity(Quantity::from(1))
+            .time_in_force(TimeInForce::Gtd) // Missing expire_time
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(expected = "`display_qty` may not exceed `quantity`")]
+    fn display_qty_gt_quantity(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketToLimit)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .quantity(Quantity::from(1))
+            .display_qty(Quantity::from(2)) // Invalid: display > quantity
+            .build();
+    }
+}