store active version id in bots table

10 files changed, 88 insertions, 30 deletions

diff --git a/planetwars-server/migrations/2022-07-23-131936_bot_active_version/down.sql b/planetwars-server/migrations/2022-07-23-131936_bot_active_version/down.sql
new file mode 100644
index 0000000..444fe51
--- /dev/null
+++ b/planetwars-server/migrations/2022-07-23-131936_bot_active_version/down.sql
@@ -0,0 +1,2 @@
+-- This file should undo anything in `up.sql`
+ALTER TABLE bots DROP COLUMN active_version;
\ No newline at end of file
diff --git a/planetwars-server/migrations/2022-07-23-131936_bot_active_version/up.sql b/planetwars-server/migrations/2022-07-23-131936_bot_active_version/up.sql
new file mode 100644
index 0000000..8ea398b
--- /dev/null
+++ b/planetwars-server/migrations/2022-07-23-131936_bot_active_version/up.sql
@@ -0,0 +1,12 @@
+-- Your SQL goes here
+ALTER TABLE bots ADD COLUMN active_version INTEGER REFERENCES bot_versions(id);
+
+-- set most recent bot verison as active
+UPDATE bots
+SET active_version = most_recent.id
+FROM (
+    SELECT DISTINCT ON (bot_id) id, bot_id
+    FROM bot_versions
+    ORDER BY bot_id, created_at DESC
+    ) most_recent
+WHERE bots.id = most_recent.bot_id;
\ No newline at end of file
diff --git a/planetwars-server/src/db/bots.rs b/planetwars-server/src/db/bots.rs
index a112a9a..a0a31b0 100644
--- a/planetwars-server/src/db/bots.rs
+++ b/planetwars-server/src/db/bots.rs
@@ -16,6 +16,7 @@ pub struct Bot {
     pub id: i32,
     pub owner_id: Option<i32>,
     pub name: String,
+    pub active_version: Option<i32>,
 }
 
 pub fn create_bot(new_bot: &NewBot, conn: &PgConnection) -> QueryResult<Bot> {
@@ -38,11 +39,34 @@ pub fn find_bot_by_name(name: &str, conn: &PgConnection) -> QueryResult<Bot> {
     bots::table.filter(bots::name.eq(name)).first(conn)
 }
 
+pub fn find_bot_with_version_by_name(
+    bot_name: &str,
+    conn: &PgConnection,
+) -> QueryResult<(Bot, BotVersion)> {
+    bots::table
+        .inner_join(bot_versions::table.on(bots::active_version.eq(bot_versions::id.nullable())))
+        .filter(bots::name.eq(bot_name))
+        .first(conn)
+}
+
+pub fn all_active_bots_with_version(conn: &PgConnection) -> QueryResult<Vec<(Bot, BotVersion)>> {
+    bots::table
+        .inner_join(bot_versions::table.on(bots::active_version.eq(bot_versions::id.nullable())))
+        .get_results(conn)
+}
+
 pub fn find_all_bots(conn: &PgConnection) -> QueryResult<Vec<Bot>> {
-    // TODO: filter out bots that cannot be run (have no valid code bundle associated with them)
     bots::table.get_results(conn)
 }
 
+/// Find all bots that have an associated active version.
+/// These are the bots that can be run.
+pub fn find_active_bots(conn: &PgConnection) -> QueryResult<Vec<Bot>> {
+    bots::table
+        .filter(bots::active_version.is_not_null())
+        .get_results(conn)
+}
+
 #[derive(Insertable)]
 #[table_name = "bot_versions"]
 pub struct NewBotVersion<'a> {
@@ -69,15 +93,25 @@ pub fn create_bot_version(
         .get_result(conn)
 }
 
-pub fn find_bot_versions(bot_id: i32, conn: &PgConnection) -> QueryResult<Vec<BotVersion>> {
+pub fn set_active_version(
+    bot_id: i32,
+    version_id: Option<i32>,
+    conn: &PgConnection,
+) -> QueryResult<()> {
+    diesel::update(bots::table.filter(bots::id.eq(bot_id)))
+        .set(bots::active_version.eq(version_id))
+        .execute(conn)?;
+    Ok(())
+}
+
+pub fn find_bot_version(version_id: i32, conn: &PgConnection) -> QueryResult<BotVersion> {
     bot_versions::table
-        .filter(bot_versions::bot_id.eq(bot_id))
-        .get_results(conn)
+        .filter(bot_versions::id.eq(version_id))
+        .first(conn)
 }
 
-pub fn active_bot_version(bot_id: i32, conn: &PgConnection) -> QueryResult<BotVersion> {
+pub fn find_bot_versions(bot_id: i32, conn: &PgConnection) -> QueryResult<Vec<BotVersion>> {
     bot_versions::table
         .filter(bot_versions::bot_id.eq(bot_id))
-        .order(bot_versions::created_at.desc())
-        .first(conn)
+        .get_results(conn)
 }
diff --git a/planetwars-server/src/modules/bot_api.rs b/planetwars-server/src/modules/bot_api.rs
index 3bc002c..3efb1c2 100644
--- a/planetwars-server/src/modules/bot_api.rs
+++ b/planetwars-server/src/modules/bot_api.rs
@@ -104,10 +104,9 @@ impl pb::bot_api_service_server::BotApiService for BotApiServer {
 
         let match_request = req.get_ref();
 
-        let opponent_bot = db::bots::find_bot_by_name(&match_request.opponent_name, &conn)
-            .map_err(|_| Status::not_found("opponent not found"))?;
-        let opponent_bot_version = db::bots::active_bot_version(opponent_bot.id, &conn)
-            .map_err(|_| Status::not_found("no opponent version found"))?;
+        let (opponent_bot, opponent_bot_version) =
+            db::bots::find_bot_with_version_by_name(&match_request.opponent_name, &conn)
+                .map_err(|_| Status::not_found("opponent not found"))?;
 
         let player_key = gen_alphanumeric(32);
 
diff --git a/planetwars-server/src/modules/bots.rs b/planetwars-server/src/modules/bots.rs
index 5513539..4aba168 100644
--- a/planetwars-server/src/modules/bots.rs
+++ b/planetwars-server/src/modules/bots.rs
@@ -5,6 +5,7 @@ use diesel::{PgConnection, QueryResult};
 use crate::{db, util::gen_alphanumeric, GlobalConfig};
 
 /// Save a string containing bot code as a code bundle.
+/// If a bot was provided, set the saved bundle as its active version.
 pub fn save_code_string(
     bot_code: &str,
     bot_id: Option<i32>,
@@ -22,5 +23,11 @@ pub fn save_code_string(
         code_bundle_path: Some(&bundle_name),
         container_digest: None,
     };
-    db::bots::create_bot_version(&new_code_bundle, conn)
+    let version = db::bots::create_bot_version(&new_code_bundle, conn)?;
+    // Leave this coupled for now - this is how the behaviour was bevore.
+    // It would be cleaner to separate version setting and bot selection, though.
+    if let Some(bot_id) = bot_id {
+        db::bots::set_active_version(bot_id, Some(version.id), conn)?;
+    }
+    Ok(version)
 }
diff --git a/planetwars-server/src/modules/ranking.rs b/planetwars-server/src/modules/ranking.rs
index a9f6419..d508d6c 100644
--- a/planetwars-server/src/modules/ranking.rs
+++ b/planetwars-server/src/modules/ranking.rs
@@ -1,3 +1,4 @@
+use crate::db::bots::BotVersion;
 use crate::{db::bots::Bot, DbPool, GlobalConfig};
 
 use crate::db;
@@ -22,12 +23,12 @@ pub async fn run_ranker(config: Arc<GlobalConfig>, db_pool: DbPool) {
         .expect("could not get database connection");
     loop {
         interval.tick().await;
-        let bots = db::bots::find_all_bots(&db_conn).unwrap();
+        let bots = db::bots::all_active_bots_with_version(&db_conn).unwrap();
         if bots.len() < 2 {
             // not enough bots to play a match
             continue;
         }
-        let selected_bots: Vec<Bot> = {
+        let selected_bots: Vec<(Bot, BotVersion)> = {
             let mut rng = &mut rand::thread_rng();
             bots.choose_multiple(&mut rng, 2).cloned().collect()
         };
@@ -36,15 +37,16 @@ pub async fn run_ranker(config: Arc<GlobalConfig>, db_pool: DbPool) {
     }
 }
 
-async fn play_ranking_match(config: Arc<GlobalConfig>, selected_bots: Vec<Bot>, db_pool: DbPool) {
-    let db_conn = db_pool.get().await.expect("could not get db pool");
+async fn play_ranking_match(
+    config: Arc<GlobalConfig>,
+    selected_bots: Vec<(Bot, BotVersion)>,
+    db_pool: DbPool,
+) {
     let mut players = Vec::new();
-    for bot in &selected_bots {
-        let version = db::bots::active_bot_version(bot.id, &db_conn)
-            .expect("could not get active bot version");
+    for (bot, bot_version) in selected_bots {
         let player = MatchPlayer::BotVersion {
-            bot: Some(bot.clone()),
-            version,
+            bot: Some(bot),
+            version: bot_version,
         };
         players.push(player);
     }
diff --git a/planetwars-server/src/modules/registry.rs b/planetwars-server/src/modules/registry.rs
index 3f6dad2..297404a 100644
--- a/planetwars-server/src/modules/registry.rs
+++ b/planetwars-server/src/modules/registry.rs
@@ -397,7 +397,10 @@ async fn put_manifest(
         code_bundle_path: None,
         container_digest: Some(&content_digest),
     };
-    db::bots::create_bot_version(&new_version, &db_conn).expect("could not save bot version");
+    let version =
+        db::bots::create_bot_version(&new_version, &db_conn).expect("could not save bot version");
+    db::bots::set_active_version(bot.id, Some(version.id), &db_conn)
+        .expect("could not update bot version");
 
     Ok(Response::builder()
         .status(StatusCode::CREATED)
diff --git a/planetwars-server/src/routes/bots.rs b/planetwars-server/src/routes/bots.rs
index 9ddb109..fc180d8 100644
--- a/planetwars-server/src/routes/bots.rs
+++ b/planetwars-server/src/routes/bots.rs
@@ -167,8 +167,9 @@ pub async fn get_my_bots(
         .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)
 }
 
+/// List all active bots
 pub async fn list_bots(conn: DatabaseConnection) -> Result<Json<Vec<Bot>>, StatusCode> {
-    bots::find_all_bots(&conn)
+    bots::find_active_bots(&conn)
         .map(Json)
         .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)
 }
diff --git a/planetwars-server/src/routes/demo.rs b/planetwars-server/src/routes/demo.rs
index 69838f3..dad9453 100644
--- a/planetwars-server/src/routes/demo.rs
+++ b/planetwars-server/src/routes/demo.rs
@@ -28,8 +28,7 @@ pub struct SubmitBotResponse {
     pub match_data: ApiMatch,
 }
 
-/// submit python code for a bot, which will face off
-/// with a demo bot. Return a played match.
+/// Submit bot code and opponent name to play a match
 pub async fn submit_bot(
     Json(params): Json<SubmitBotParams>,
     Extension(pool): Extension<ConnectionPool>,
@@ -41,10 +40,9 @@ pub async fn submit_bot(
         .opponent_name
         .unwrap_or_else(|| DEFAULT_OPPONENT_NAME.to_string());
 
-    let opponent_bot =
-        db::bots::find_bot_by_name(&opponent_name, &conn).map_err(|_| StatusCode::BAD_REQUEST)?;
-    let opponent_bot_version = db::bots::active_bot_version(opponent_bot.id, &conn)
-        .map_err(|_| StatusCode::BAD_REQUEST)?;
+    let (opponent_bot, opponent_bot_version) =
+        db::bots::find_bot_with_version_by_name(&opponent_name, &conn)
+            .map_err(|_| StatusCode::BAD_REQUEST)?;
 
     let player_bot_version = save_code_string(&params.code, None, &conn, &config)
         // TODO: can we recover from this?
diff --git a/planetwars-server/src/schema.rs b/planetwars-server/src/schema.rs
index 0606ac4..5993115 100644
--- a/planetwars-server/src/schema.rs
+++ b/planetwars-server/src/schema.rs
@@ -22,6 +22,7 @@ table! {
         id -> Int4,
         owner_id -> Nullable<Int4>,
         name -> Text,
+        active_version -> Nullable<Int4>,
     }
 }
 
@@ -82,7 +83,6 @@ table! {
     }
 }
 
-joinable!(bot_versions -> bots (bot_id));
 joinable!(bots -> users (owner_id));
 joinable!(match_players -> bot_versions (bot_version_id));
 joinable!(match_players -> matches (match_id));