From 668409e76d8cc7797fe627b2e2c3d0223b3db684 Mon Sep 17 00:00:00 2001
From: Ilion Beyst <ilion.beyst@gmail.com>
Date: Wed, 13 Jul 2022 19:36:07 +0200
Subject: refactor: unify match save and spawn

---
 planetwars-server/src/modules/bot_api.rs |  8 ++++----
 planetwars-server/src/modules/matches.rs | 32 ++++++++++++++++++--------------
 planetwars-server/src/modules/ranking.rs | 12 +++++-------
 planetwars-server/src/routes/demo.rs     | 10 +++++-----
 4 files changed, 32 insertions(+), 30 deletions(-)

(limited to 'planetwars-server')

diff --git a/planetwars-server/src/modules/bot_api.rs b/planetwars-server/src/modules/bot_api.rs
index 962b33d..0ee9357 100644
--- a/planetwars-server/src/modules/bot_api.rs
+++ b/planetwars-server/src/modules/bot_api.rs
@@ -122,10 +122,10 @@ impl pb::bot_api_service_server::BotApiService for BotApiServer {
                 version: opponent_bot_version,
             },
         ]);
-        let created_match = run_match
-            .store_in_database(&conn)
-            .expect("failed to save match");
-        run_match.spawn(self.conn_pool.clone());
+        let (created_match, _) = run_match
+            .run(self.conn_pool.clone())
+            .await
+            .expect("failed to create match");
 
         Ok(Response::new(pb::CreatedMatch {
             match_id: created_match.base.id,
diff --git a/planetwars-server/src/modules/matches.rs b/planetwars-server/src/modules/matches.rs
index 0496db7..6caa8c2 100644
--- a/planetwars-server/src/modules/matches.rs
+++ b/planetwars-server/src/modules/matches.rs
@@ -19,7 +19,6 @@ const PYTHON_IMAGE: &str = "python:3.10-slim-buster";
 pub struct RunMatch {
     log_file_name: String,
     players: Vec<MatchPlayer>,
-    match_id: Option<i32>,
 }
 
 pub enum MatchPlayer {
@@ -38,7 +37,6 @@ impl RunMatch {
         RunMatch {
             log_file_name,
             players,
-            match_id: None,
         }
     }
 
@@ -62,10 +60,24 @@ impl RunMatch {
         }
     }
 
-    pub fn store_in_database(&mut self, db_conn: &PgConnection) -> QueryResult<MatchData> {
-        // don't store the same match twice
-        assert!(self.match_id.is_none());
+    pub async fn run(
+        self,
+        conn_pool: ConnectionPool,
+    ) -> QueryResult<(MatchData, JoinHandle<MatchOutcome>)> {
+        let match_data = {
+            // TODO: it would be nice to get an already-open connection here when possible.
+            // Maybe we need an additional abstraction, bundling a connection and connection pool?
+            let db_conn = conn_pool.get().await.expect("could not get a connection");
+            self.store_in_database(&db_conn)?
+        };
 
+        let runner_config = self.into_runner_config();
+        let handle = tokio::spawn(run_match_task(conn_pool, runner_config, match_data.base.id));
+
+        Ok((match_data, handle))
+    }
+
+    fn store_in_database(&self, db_conn: &PgConnection) -> QueryResult<MatchData> {
         let new_match_data = db::matches::NewMatch {
             state: db::matches::MatchState::Playing,
             log_path: &self.log_file_name,
@@ -81,15 +93,7 @@ impl RunMatch {
             })
             .collect::<Vec<_>>();
 
-        let match_data = db::matches::create_match(&new_match_data, &new_match_players, db_conn)?;
-        self.match_id = Some(match_data.base.id);
-        Ok(match_data)
-    }
-
-    pub fn spawn(self, pool: ConnectionPool) -> JoinHandle<MatchOutcome> {
-        let match_id = self.match_id.expect("match must be saved before running");
-        let runner_config = self.into_runner_config();
-        tokio::spawn(run_match_task(pool, runner_config, match_id))
+        db::matches::create_match(&new_match_data, &new_match_players, db_conn)
     }
 }
 
diff --git a/planetwars-server/src/modules/ranking.rs b/planetwars-server/src/modules/ranking.rs
index 7147b98..1c35394 100644
--- a/planetwars-server/src/modules/ranking.rs
+++ b/planetwars-server/src/modules/ranking.rs
@@ -48,14 +48,12 @@ async fn play_ranking_match(selected_bots: Vec<Bot>, db_pool: DbPool) {
         players.push(player);
     }
 
-    let mut run_match = RunMatch::from_players(players);
-    run_match
-        .store_in_database(&db_conn)
-        .expect("could not store match in db");
-    run_match
-        .spawn(db_pool.clone())
+    let (_, handle) = RunMatch::from_players(players)
+        .run(db_pool.clone())
         .await
-        .expect("running match failed");
+        .expect("failed to run match");
+    // wait for match to complete, so that only one ranking match can be running
+    let _outcome = handle.await;
 }
 
 fn recalculate_ratings(db_conn: &PgConnection) -> QueryResult<()> {
diff --git a/planetwars-server/src/routes/demo.rs b/planetwars-server/src/routes/demo.rs
index f9929f7..5ff02c7 100644
--- a/planetwars-server/src/routes/demo.rs
+++ b/planetwars-server/src/routes/demo.rs
@@ -46,7 +46,7 @@ pub async fn submit_bot(
         // TODO: can we recover from this?
         .expect("could not save bot code");
 
-    let mut run_match = RunMatch::from_players(vec![
+    let run_match = RunMatch::from_players(vec![
         MatchPlayer::BotVersion {
             bot: None,
             version: player_bot_version.clone(),
@@ -56,10 +56,10 @@ pub async fn submit_bot(
             version: opponent_bot_version.clone(),
         },
     ]);
-    let match_data = run_match
-        .store_in_database(&conn)
-        .expect("failed to save match");
-    run_match.spawn(pool.clone());
+    let (match_data, _) = run_match
+        .run(pool.clone())
+        .await
+        .expect("failed to run match");
 
     // TODO: avoid clones
     let full_match_data = FullMatchData {
-- 
cgit v1.2.3