Change cancel_order method to use OrderRef instead of `OrderCance…

…lling`

core/src/exchanges/general/order/cancel.rs

@@ -1,13 +1,11 @@
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use futures::future::join_all;
 use itertools::Itertools;
 use mmb_domain::events::EventSourceType;
 use mmb_domain::market::ExchangeErrorType;
 use mmb_domain::order::pool::OrderRef;
 use mmb_domain::order::snapshot::Amount;
-use mmb_domain::order::snapshot::{
-    ClientOrderId, ExchangeOrderId, OrderCancelling, OrderInfo, OrderStatus,
-};
+use mmb_domain::order::snapshot::{ClientOrderId, ExchangeOrderId, OrderInfo, OrderStatus};
 use mmb_utils::cancellation_token::CancellationToken;
 use tokio::sync::oneshot;
 
@@ -76,11 +74,7 @@ impl Exchange {
                     self.exchange_account_id
                 );
 
-                let order_to_cancel = order
-                    .to_order_cancelling()
-                    .ok_or_else(|| anyhow!("Unable to convert order to order_to_cancel"))?;
-                let order_cancellation_outcome =
-                    self.cancel_order(order_to_cancel, cancellation_token).await;
+                let order_cancellation_outcome = self.cancel_order(order, cancellation_token).await;
 
                 log::info!(
                     "Submitted order cancellation {client_order_id} {exchange_order_id:?} on {}: {order_cancellation_outcome:?}", self.exchange_account_id);
@@ -92,53 +86,61 @@ impl Exchange {
 
     pub async fn cancel_order(
         &self,
-        order: OrderCancelling,
+        order: &OrderRef,
         cancellation_token: CancellationToken,
     ) -> Option<CancelOrderResult> {
-        let exchange_order_id = order.exchange_order_id.clone();
-        let order_cancellation_outcome = self.cancel_order_core(order, cancellation_token).await;
-
-        // Option is returning when cancel_order_core is stopped by CancellationToken
-        // So appropriate Handler was already called in a fallback
-        if let Some(ref cancel_outcome) = order_cancellation_outcome {
-            match &cancel_outcome.outcome {
-                RequestResult::Success(client_order_id) => self.handle_cancel_order_succeeded(
-                    Some(client_order_id),
-                    &exchange_order_id,
-                    cancel_outcome.filled_amount,
-                    cancel_outcome.source_type,
-                ),
-                RequestResult::Error(error) => {
-                    if error.error_type != ExchangeErrorType::ParsingError {
-                        self.handle_cancel_order_failed(
-                            &exchange_order_id,
-                            error.clone(),
-                            cancel_outcome.source_type,
-                        );
-                    }
+        match order.exchange_order_id() {
+            Some(exchange_order_id) => {
+                let order_cancellation_outcome = self
+                    .cancel_order_core(order, &exchange_order_id, cancellation_token)
+                    .await;
+
+                // Option is returning when cancel_order_core is stopped by CancellationToken
+                // So appropriate Handler was already called in a fallback
+                if let Some(ref cancel_outcome) = order_cancellation_outcome {
+                    match &cancel_outcome.outcome {
+                        RequestResult::Success(client_order_id) => self
+                            .handle_cancel_order_succeeded(
+                                Some(client_order_id),
+                                &exchange_order_id,
+                                cancel_outcome.filled_amount,
+                                cancel_outcome.source_type,
+                            ),
+                        RequestResult::Error(error) => {
+                            if error.error_type != ExchangeErrorType::ParsingError {
+                                self.handle_cancel_order_failed(
+                                    &exchange_order_id,
+                                    error.clone(),
+                                    cancel_outcome.source_type,
+                                );
+                            }
+                        }
+                    };
                 }
-            };
-        }
 
-        order_cancellation_outcome
+                order_cancellation_outcome
+            }
+            None => {
+                log::warn!("Missing exchange_order_id in cancelling order");
+                None
+            }
+        }
     }
 
     async fn cancel_order_core(
         &self,
-        // TODO Here has to be common Order (or OrderRef) cause it's more natural way:
-        // When user want to cancel_order he already has that order data somewhere
-        order: OrderCancelling,
+        order: &OrderRef,
+        exchange_order_id: &ExchangeOrderId,
         cancellation_token: CancellationToken,
     ) -> Option<CancelOrderResult> {
-        let exchange_order_id = order.exchange_order_id.clone();
         let (tx, mut websocket_event_receiver) = oneshot::channel();
 
         // TODO insert is not analog of C# GetOrAd!
         // Here has to be entry().or_insert()
         self.order_cancellation_events
             .insert(exchange_order_id.clone(), (tx, None));
 
-        let cancel_order_future = self.exchange_client.cancel_order(order);
+        let cancel_order_future = self.exchange_client.cancel_order(order, exchange_order_id);
 
         tokio::select! {
             cancel_order_result = cancel_order_future => {

core/src/exchanges/general/test_helper.rs

@@ -34,9 +34,9 @@ use mmb_domain::market::{
     CurrencyCode, CurrencyId, CurrencyPair, ExchangeAccountId, SpecificCurrencyPair,
 };
 use mmb_domain::order::pool::{OrderRef, OrdersPool};
-use mmb_domain::order::snapshot::{Amount, Price};
+use mmb_domain::order::snapshot::{Amount, ExchangeOrderId, Price};
 use mmb_domain::order::snapshot::{
-    ClientOrderId, OrderCancelling, OrderInfo, OrderRole, OrderSide, OrderSnapshot, OrderType,
+    ClientOrderId, OrderInfo, OrderRole, OrderSide, OrderSnapshot, OrderType,
 };
 use mmb_domain::position::{ActivePosition, ClosedPosition};
 use parking_lot::RwLock;
@@ -65,7 +65,11 @@ impl ExchangeClient for TestClient {
         unimplemented!("doesn't need in UT")
     }
 
-    async fn cancel_order(&self, _order: OrderCancelling) -> CancelOrderResult {
+    async fn cancel_order(
+        &self,
+        _order: &OrderRef,
+        _exchange_order_id: &ExchangeOrderId,
+    ) -> CancelOrderResult {
         unimplemented!("doesn't need in UT")
     }
 

core/src/exchanges/traits.rs

@@ -26,7 +26,7 @@ use mmb_domain::market::{
 use mmb_domain::order::pool::{OrderRef, OrdersPool};
 use mmb_domain::order::snapshot::Price;
 use mmb_domain::order::snapshot::{
-    ClientOrderId, ExchangeOrderId, OrderCancelling, OrderInfo, OrderInfoExtensionData, OrderSide,
+    ClientOrderId, ExchangeOrderId, OrderInfo, OrderInfoExtensionData, OrderSide,
 };
 use mmb_domain::position::{ActivePosition, ClosedPosition};
 use mmb_utils::DateTime;
@@ -90,7 +90,13 @@ impl From<anyhow::Error> for ExchangeError {
 pub trait ExchangeClient: Support {
     async fn create_order(&self, order: &OrderRef) -> CreateOrderResult;
 
-    async fn cancel_order(&self, order: OrderCancelling) -> CancelOrderResult;
+    /// There is an `ExchangeOrderId` as additional argument cause it's an `Option` in `OrderRef`
+    /// And there is no point to check if it's `Some(value)` cause it already must be checked in core
+    async fn cancel_order(
+        &self,
+        order: &OrderRef,
+        exchange_order_id: &ExchangeOrderId,
+    ) -> CancelOrderResult;
 
     async fn cancel_all_orders(&self, currency_pair: CurrencyPair) -> Result<()>;
 

core_tests/Cargo.toml

@@ -16,6 +16,7 @@ mmb_core = { path = "../core" }
 mmb_domain = { path = "../domain" }
 mmb_utils = { path = "../mmb_utils" }
 
+parking_lot = { version = "0.12", features = ["serde"]}
 rust_decimal = { version = "1", features = ["maths"]}
 rust_decimal_macros = "1"
 

core_tests/src/order.rs

@@ -1,7 +1,7 @@
 use mmb_core::exchanges::general::exchange::Exchange;
 use mmb_core::exchanges::general::exchange::RequestResult;
 use mmb_domain::market::ExchangeAccountId;
-use mmb_domain::order::pool::OrderRef;
+use mmb_domain::order::pool::{OrderRef, OrdersPool};
 use mmb_domain::order::snapshot::Amount;
 use mmb_domain::order::snapshot::*;
 
@@ -15,6 +15,7 @@ use tokio::time::Duration;
 use mmb_domain::market::CurrencyPair;
 use mmb_domain::order::snapshot::Price;
 use mmb_utils::infrastructure::with_timeout;
+use parking_lot::RwLock;
 use std::sync::Arc;
 
 /// This struct needed for creating an orders in tests.
@@ -119,25 +120,41 @@ impl OrderProxy {
     }
 
     pub async fn cancel_order_or_fail(&self, order_ref: &OrderRef, exchange: Arc<Exchange>) {
-        let header = self.make_header();
-        let exchange_order_id = order_ref.exchange_order_id().expect("in test");
-        let order_to_cancel = OrderCancelling {
-            header: header.clone(),
-            exchange_order_id,
-            extension_data: order_ref.fn_ref(|s| s.extension_data.clone()),
-        };
-
         order_ref.fn_mut(|order| order.set_status(OrderStatus::Canceling, Utc::now()));
 
         let cancel_outcome = exchange
-            .cancel_order(order_to_cancel, CancellationToken::default())
+            .cancel_order(order_ref, CancellationToken::default())
             .await
             .expect("in test");
 
         if let RequestResult::Success(gotten_client_order_id) = cancel_outcome.outcome {
             assert_eq!(gotten_client_order_id, self.client_order_id);
         }
     }
+
+    pub fn created_order_ref_stub(&self, orders_pool: Arc<OrdersPool>) -> OrderRef {
+        let props = OrderSimpleProps::new(
+            Utc::now(),
+            Some(self.price),
+            Some(OrderRole::Maker),
+            Some("1234567890".into()),
+            Default::default(),
+            Default::default(),
+            OrderStatus::Created,
+            None,
+        );
+
+        let snapshot = OrderSnapshot::new(
+            self.make_header(),
+            props,
+            OrderFills::default(),
+            OrderStatusHistory::default(),
+            SystemInternalOrderProps::default(),
+            None,
+        );
+
+        orders_pool.add_snapshot_initial(Arc::new(RwLock::new(snapshot)))
+    }
 }
 
 pub struct OrderProxyBuilder {

domain/src/order/pool.rs

@@ -5,7 +5,7 @@ use crate::order::snapshot::{
     Amount, ClientOrderId, ExchangeOrderId, OrderHeader, OrderInfoExtensionData, OrderSimpleProps,
     OrderSnapshot, OrderStatus,
 };
-use crate::order::snapshot::{OrderCancelling, OrderRole, OrderSide, OrderType};
+use crate::order::snapshot::{OrderRole, OrderSide, OrderType};
 use dashmap::DashMap;
 use mmb_database::impl_event;
 use mmb_utils::DateTime;
@@ -97,25 +97,6 @@ impl OrderRef {
     pub fn get_fills(&self) -> (Vec<OrderFill>, Amount) {
         self.fn_ref(|order| (order.fills.fills.clone(), order.fills.filled_amount))
     }
-
-    pub fn to_order_cancelling(&self) -> Option<OrderCancelling> {
-        self.fn_ref(|order| {
-            order
-                .props
-                .exchange_order_id
-                .as_ref()
-                .map(|exchange_order_id| OrderCancelling {
-                    header: order.header.clone(),
-                    exchange_order_id: exchange_order_id.clone(),
-                    extension_data: order.extension_data.clone(),
-                })
-        })
-    }
-
-    #[cfg(test)]
-    pub fn new(snapshot: Arc<RwLock<OrderSnapshot>>) -> Self {
-        Self(snapshot)
-    }
 }
 
 #[derive(Debug)]

domain/src/order/snapshot.rs

@@ -474,13 +474,6 @@ pub struct OrderCreating {
     pub price: Price,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct OrderCancelling {
-    pub header: Arc<OrderHeader>,
-    pub exchange_order_id: ExchangeOrderId,
-    pub extension_data: Option<Box<dyn OrderInfoExtensionData>>,
-}
-
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct OrderSnapshot {
     pub header: Arc<OrderHeader>,

exchanges/binance/src/binance.rs

@@ -773,19 +773,20 @@ impl Binance {
     #[named]
     pub(super) async fn request_cancel_order(
         &self,
-        order: OrderCancelling,
+        order: &OrderRef,
+        exchange_order_id: &ExchangeOrderId,
     ) -> Result<RestResponse, ExchangeError> {
-        let specific_currency_pair = self.get_specific_currency_pair(order.header.currency_pair);
+        let specific_currency_pair = self.get_specific_currency_pair(order.currency_pair());
 
         let path = self.get_uri_path("/fapi/v1/order", "/api/v3/order");
         let mut builder = UriBuilder::from_path(path);
         builder.add_kv("symbol", specific_currency_pair);
-        builder.add_kv("orderId", &order.exchange_order_id);
+        builder.add_kv("orderId", exchange_order_id);
         self.add_authentification(&mut builder);
 
         let uri = builder.build_uri(self.hosts.rest_uri_host(), true);
 
-        let log_args = format!("Cancel order for {}", order.header.client_order_id);
+        let log_args = format!("Cancel order for {}", order.client_order_id());
         self.rest_client
             .delete(uri, function_name!(), log_args)
             .await

exchanges/binance/src/exchange_client.rs

@@ -33,15 +33,15 @@ impl ExchangeClient for Binance {
         }
     }
 
-    async fn cancel_order(&self, order: OrderCancelling) -> CancelOrderResult {
-        let order_header = order.header.clone();
-
-        match self.request_cancel_order(order).await {
-            Ok(_) => CancelOrderResult::succeed(
-                order_header.client_order_id.clone(),
-                EventSourceType::Rest,
-                None,
-            ),
+    async fn cancel_order(
+        &self,
+        order: &OrderRef,
+        exchange_order_id: &ExchangeOrderId,
+    ) -> CancelOrderResult {
+        match self.request_cancel_order(order, exchange_order_id).await {
+            Ok(_) => {
+                CancelOrderResult::succeed(order.client_order_id(), EventSourceType::Rest, None)
+            }
             Err(err) => CancelOrderResult::failed(err, EventSourceType::Rest),
         }
     }