From a84fd60179698b8fb009cc11ce3d492bce765770 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 24 Sep 2021 18:46:42 +1000 Subject: [PATCH 1/2] Add failing test A test that shows, that if we save two CFDs in the database, we fail to correctly load the second CFD by order-uuid. --- daemon/src/db.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/daemon/src/db.rs b/daemon/src/db.rs index 1ff78b2..40135f4 100644 --- a/daemon/src/db.rs +++ b/daemon/src/db.rs @@ -472,6 +472,52 @@ mod tests { assert_eq!(cfd, cfd_from_db) } + #[tokio::test] + async fn test_insert_and_load_cfd_by_order_id_multiple() { + let pool = setup_test_db().await; + let mut conn = pool.acquire().await.unwrap(); + + let order = Order::from_default_with_price(Usd(dec!(10000)), Origin::Theirs).unwrap(); + + let cfd = Cfd::new( + order.clone(), + Usd(dec!(1000)), + CfdState::OutgoingOrderRequest { + common: CfdStateCommon { + transition_timestamp: SystemTime::now(), + }, + }, + ); + + let order_id = order.id; + + insert_order(&order, &mut conn).await.unwrap(); + insert_cfd(cfd.clone(), &mut conn).await.unwrap(); + + let cfd_from_db = load_cfd_by_order_id(order_id, &mut conn).await.unwrap(); + assert_eq!(cfd, cfd_from_db); + + let order = Order::from_default_with_price(Usd(dec!(10000)), Origin::Theirs).unwrap(); + + let cfd = Cfd::new( + order.clone(), + Usd(dec!(1000)), + CfdState::OutgoingOrderRequest { + common: CfdStateCommon { + transition_timestamp: SystemTime::now(), + }, + }, + ); + + let order_id = order.id; + + insert_order(&order, &mut conn).await.unwrap(); + insert_cfd(cfd.clone(), &mut conn).await.unwrap(); + + let cfd_from_db = load_cfd_by_order_id(order_id, &mut conn).await.unwrap(); + assert_eq!(cfd, cfd_from_db); + } + #[tokio::test] async fn test_insert_new_cfd_state() { let pool = setup_test_db().await; From 703a669ec045fc157972ef4793562f1af629bfdc Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 24 Sep 2021 19:01:19 +1000 Subject: [PATCH 2/2] Make the test pass The problem was that the restriction on the JOIN clause does not properly filter the complete set, but actually results in wrongly joining the CFDs. This resulted in two CFDs returned, where one was wrongly assembled (the id of the first one, but the data of the second one). In a cases where the wrong one is the first one in the list of returned CFDs we would run into an error when inserting a new state, because we would try to alter the state of a wrong CFD. Fixed by joining on the id and filtering for the order.uuid in the where clause. Note: I also tested to add the `and orders.uuid = ?` restriction to the first join clause (in addition to the id clause) which might theoretically be more optimized. That would work as well, but I found it harder to reason about it. As part of the overall where clause one can see the restriction to exactly this value more clearly. --- daemon/sqlx-data.json | 60 +++++++++++++++++++++---------------------- daemon/src/db.rs | 3 ++- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/daemon/sqlx-data.json b/daemon/sqlx-data.json index dc97e7a..7e1a6d7 100644 --- a/daemon/sqlx-data.json +++ b/daemon/sqlx-data.json @@ -124,36 +124,8 @@ ] } }, - "8e7571250da58b12f5884f17656e5966957c7798ea029c701a4fc43fd613f015": { - "query": "\n select\n id\n from cfds\n where order_uuid = ?;\n ", - "describe": { - "columns": [ - { - "name": "id", - "ordinal": 0, - "type_info": "Int64" - } - ], - "parameters": { - "Right": 1 - }, - "nullable": [ - true - ] - } - }, - "a464a1feb12abadff8bfd5b2b3b7362f3846869c0702944b21737eff8f420be5": { - "query": "\n insert into cfd_states (\n cfd_id,\n state\n ) values (?, ?);\n ", - "describe": { - "columns": [], - "parameters": { - "Right": 2 - }, - "nullable": [] - } - }, - "a8454a2e4288b58d625f10722c6e23a51d5d17e6bb7af9685bcdbe64546969fa": { - "query": "\n select\n cfds.id as cfd_id,\n orders.uuid as order_id,\n orders.initial_price as price,\n orders.min_quantity as min_quantity,\n orders.max_quantity as max_quantity,\n orders.leverage as leverage,\n orders.trading_pair as trading_pair,\n orders.position as position,\n orders.origin as origin,\n orders.liquidation_price as liquidation_price,\n orders.creation_timestamp as creation_timestamp,\n orders.term as term,\n cfds.quantity_usd as quantity_usd,\n cfd_states.state as state\n from cfds as cfds\n inner join orders as orders on cfds.order_uuid = ?\n inner join cfd_states as cfd_states on cfd_states.cfd_id = cfds.id\n where cfd_states.state in (\n select\n state\n from cfd_states\n where cfd_id = cfds.id\n order by id desc\n limit 1\n )\n ", + "62c8df2dde7a305757a070149a7066faf15da1ef2d6c6fc4c0bd83e385e4750e": { + "query": "\n select\n cfds.id as cfd_id,\n orders.uuid as order_id,\n orders.initial_price as price,\n orders.min_quantity as min_quantity,\n orders.max_quantity as max_quantity,\n orders.leverage as leverage,\n orders.trading_pair as trading_pair,\n orders.position as position,\n orders.origin as origin,\n orders.liquidation_price as liquidation_price,\n orders.creation_timestamp as creation_timestamp,\n orders.term as term,\n cfds.quantity_usd as quantity_usd,\n cfd_states.state as state\n from cfds as cfds\n inner join orders as orders on cfds.order_id = orders.id\n inner join cfd_states as cfd_states on cfd_states.cfd_id = cfds.id\n where cfd_states.state in (\n select\n state\n from cfd_states\n where cfd_id = cfds.id\n order by id desc\n limit 1\n )\n and orders.uuid = ?\n ", "describe": { "columns": [ { @@ -248,6 +220,34 @@ ] } }, + "8e7571250da58b12f5884f17656e5966957c7798ea029c701a4fc43fd613f015": { + "query": "\n select\n id\n from cfds\n where order_uuid = ?;\n ", + "describe": { + "columns": [ + { + "name": "id", + "ordinal": 0, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + } + }, + "a464a1feb12abadff8bfd5b2b3b7362f3846869c0702944b21737eff8f420be5": { + "query": "\n insert into cfd_states (\n cfd_id,\n state\n ) values (?, ?);\n ", + "describe": { + "columns": [], + "parameters": { + "Right": 2 + }, + "nullable": [] + } + }, "f3dce76f316212c91cb3402b0cef00f1c9adbef8519c54e9bdbd373aab946209": { "query": "\n select * from orders where uuid = ?;\n ", "describe": { diff --git a/daemon/src/db.rs b/daemon/src/db.rs index 40135f4..74a0126 100644 --- a/daemon/src/db.rs +++ b/daemon/src/db.rs @@ -263,7 +263,7 @@ pub async fn load_cfd_by_order_id( cfds.quantity_usd as quantity_usd, cfd_states.state as state from cfds as cfds - inner join orders as orders on cfds.order_uuid = ? + inner join orders as orders on cfds.order_id = orders.id inner join cfd_states as cfd_states on cfd_states.cfd_id = cfds.id where cfd_states.state in ( select @@ -273,6 +273,7 @@ pub async fn load_cfd_by_order_id( order by id desc limit 1 ) + and orders.uuid = ? "#, order_uuid )