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, pub name: String, + pub active_version: Option, } pub fn create_bot(new_bot: &NewBot, conn: &PgConnection) -> QueryResult { @@ -38,11 +39,34 @@ pub fn find_bot_by_name(name: &str, conn: &PgConnection) -> QueryResult { 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> { + 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> { - // 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> { + 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 set_active_version( + bot_id: i32, + version_id: Option, + 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 { + bot_versions::table + .filter(bot_versions::id.eq(version_id)) + .first(conn) +} + pub fn find_bot_versions(bot_id: i32, conn: &PgConnection) -> QueryResult> { bot_versions::table .filter(bot_versions::bot_id.eq(bot_id)) .get_results(conn) } - -pub fn active_bot_version(bot_id: i32, conn: &PgConnection) -> QueryResult { - bot_versions::table - .filter(bot_versions::bot_id.eq(bot_id)) - .order(bot_versions::created_at.desc()) - .first(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, @@ -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, 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 = { + 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, db_pool: DbPool) { } } -async fn play_ranking_match(config: Arc, selected_bots: Vec, db_pool: DbPool) { - let db_conn = db_pool.get().await.expect("could not get db pool"); +async fn play_ranking_match( + config: Arc, + 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>, 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, Extension(pool): Extension, @@ -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(¶ms.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, name -> Text, + active_version -> Nullable, } } @@ -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));