6 files changed, 83 insertions, 77 deletions

diff --git a/planetwars-server/src/db/bots.rs b/planetwars-server/src/db/bots.rs
index a0a31b0..cf8bbb5 100644
--- a/planetwars-server/src/db/bots.rs
+++ b/planetwars-server/src/db/bots.rs
@@ -5,7 +5,7 @@ use crate::schema::{bot_versions, bots};
 use chrono;
 
 #[derive(Insertable)]
-#[table_name = "bots"]
+#[diesel(table_name = bots)]
 pub struct NewBot<'a> {
     pub owner_id: Option<i32>,
     pub name: &'a str,
@@ -19,29 +19,29 @@ pub struct Bot {
     pub active_version: Option<i32>,
 }
 
-pub fn create_bot(new_bot: &NewBot, conn: &PgConnection) -> QueryResult<Bot> {
+pub fn create_bot(new_bot: &NewBot, conn: &mut PgConnection) -> QueryResult<Bot> {
     diesel::insert_into(bots::table)
         .values(new_bot)
         .get_result(conn)
 }
 
-pub fn find_bot(id: i32, conn: &PgConnection) -> QueryResult<Bot> {
+pub fn find_bot(id: i32, conn: &mut PgConnection) -> QueryResult<Bot> {
     bots::table.find(id).first(conn)
 }
 
-pub fn find_bots_by_owner(owner_id: i32, conn: &PgConnection) -> QueryResult<Vec<Bot>> {
+pub fn find_bots_by_owner(owner_id: i32, conn: &mut PgConnection) -> QueryResult<Vec<Bot>> {
     bots::table
         .filter(bots::owner_id.eq(owner_id))
         .get_results(conn)
 }
 
-pub fn find_bot_by_name(name: &str, conn: &PgConnection) -> QueryResult<Bot> {
+pub fn find_bot_by_name(name: &str, conn: &mut 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,
+    conn: &mut PgConnection,
 ) -> QueryResult<(Bot, BotVersion)> {
     bots::table
         .inner_join(bot_versions::table.on(bots::active_version.eq(bot_versions::id.nullable())))
@@ -49,26 +49,28 @@ pub fn find_bot_with_version_by_name(
         .first(conn)
 }
 
-pub fn all_active_bots_with_version(conn: &PgConnection) -> QueryResult<Vec<(Bot, BotVersion)>> {
+pub fn all_active_bots_with_version(
+    conn: &mut 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>> {
+pub fn find_all_bots(conn: &mut PgConnection) -> QueryResult<Vec<Bot>> {
     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>> {
+pub fn find_active_bots(conn: &mut PgConnection) -> QueryResult<Vec<Bot>> {
     bots::table
         .filter(bots::active_version.is_not_null())
         .get_results(conn)
 }
 
 #[derive(Insertable)]
-#[table_name = "bot_versions"]
+#[diesel(table_name = bot_versions)]
 pub struct NewBotVersion<'a> {
     pub bot_id: Option<i32>,
     pub code_bundle_path: Option<&'a str>,
@@ -86,7 +88,7 @@ pub struct BotVersion {
 
 pub fn create_bot_version(
     new_bot_version: &NewBotVersion,
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<BotVersion> {
     diesel::insert_into(bot_versions::table)
         .values(new_bot_version)
@@ -96,7 +98,7 @@ pub fn create_bot_version(
 pub fn set_active_version(
     bot_id: i32,
     version_id: Option<i32>,
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<()> {
     diesel::update(bots::table.filter(bots::id.eq(bot_id)))
         .set(bots::active_version.eq(version_id))
@@ -104,13 +106,13 @@ pub fn set_active_version(
     Ok(())
 }
 
-pub fn find_bot_version(version_id: i32, conn: &PgConnection) -> QueryResult<BotVersion> {
+pub fn find_bot_version(version_id: i32, conn: &mut PgConnection) -> QueryResult<BotVersion> {
     bot_versions::table
         .filter(bot_versions::id.eq(version_id))
         .first(conn)
 }
 
-pub fn find_bot_versions(bot_id: i32, conn: &PgConnection) -> QueryResult<Vec<BotVersion>> {
+pub fn find_bot_versions(bot_id: i32, conn: &mut PgConnection) -> QueryResult<Vec<BotVersion>> {
     bot_versions::table
         .filter(bot_versions::bot_id.eq(bot_id))
         .get_results(conn)
diff --git a/planetwars-server/src/db/maps.rs b/planetwars-server/src/db/maps.rs
index dffe4fd..8972461 100644
--- a/planetwars-server/src/db/maps.rs
+++ b/planetwars-server/src/db/maps.rs
@@ -3,7 +3,7 @@ use diesel::prelude::*;
 use crate::schema::maps;
 
 #[derive(Insertable)]
-#[table_name = "maps"]
+#[diesel(table_name = maps)]
 pub struct NewMap<'a> {
     pub name: &'a str,
     pub file_path: &'a str,
@@ -16,20 +16,20 @@ pub struct Map {
     pub file_path: String,
 }
 
-pub fn create_map(new_map: NewMap, conn: &PgConnection) -> QueryResult<Map> {
+pub fn create_map(new_map: NewMap, conn: &mut PgConnection) -> QueryResult<Map> {
     diesel::insert_into(maps::table)
         .values(new_map)
         .get_result(conn)
 }
 
-pub fn find_map(id: i32, conn: &PgConnection) -> QueryResult<Map> {
+pub fn find_map(id: i32, conn: &mut PgConnection) -> QueryResult<Map> {
     maps::table.find(id).get_result(conn)
 }
 
-pub fn find_map_by_name(name: &str, conn: &PgConnection) -> QueryResult<Map> {
+pub fn find_map_by_name(name: &str, conn: &mut PgConnection) -> QueryResult<Map> {
     maps::table.filter(maps::name.eq(name)).first(conn)
 }
 
-pub fn list_maps(conn: &PgConnection) -> QueryResult<Vec<Map>> {
+pub fn list_maps(conn: &mut PgConnection) -> QueryResult<Vec<Map>> {
     maps::table.get_results(conn)
 }
diff --git a/planetwars-server/src/db/matches.rs b/planetwars-server/src/db/matches.rs
index 1dded43..bfec892 100644
--- a/planetwars-server/src/db/matches.rs
+++ b/planetwars-server/src/db/matches.rs
@@ -1,9 +1,6 @@
 pub use crate::db_types::MatchState;
 use chrono::NaiveDateTime;
 use diesel::associations::BelongsTo;
-use diesel::pg::Pg;
-use diesel::query_builder::BoxedSelectStatement;
-use diesel::query_source::{AppearsInFromClause, Once};
 use diesel::sql_types::*;
 use diesel::{
     BelongingToDsl, ExpressionMethods, JoinOnDsl, NullableExpressionMethods, QueryDsl, RunQueryDsl,
@@ -18,7 +15,7 @@ use super::bots::{Bot, BotVersion};
 use super::maps::Map;
 
 #[derive(Insertable)]
-#[table_name = "matches"]
+#[diesel(table_name = matches)]
 pub struct NewMatch<'a> {
     pub state: MatchState,
     pub log_path: &'a str,
@@ -27,7 +24,7 @@ pub struct NewMatch<'a> {
 }
 
 #[derive(Insertable)]
-#[table_name = "match_players"]
+#[diesel(table_name = match_players)]
 pub struct NewMatchPlayer {
     /// id of the match this player is in
     pub match_id: i32,
@@ -38,7 +35,7 @@ pub struct NewMatchPlayer {
 }
 
 #[derive(Queryable, Identifiable)]
-#[table_name = "matches"]
+#[diesel(table_name = matches)]
 pub struct MatchBase {
     pub id: i32,
     pub state: MatchState,
@@ -50,8 +47,8 @@ pub struct MatchBase {
 }
 
 #[derive(Queryable, Identifiable, Associations, Clone)]
-#[primary_key(match_id, player_id)]
-#[belongs_to(MatchBase, foreign_key = "match_id")]
+#[diesel(primary_key(match_id, player_id))]
+#[diesel(belongs_to(MatchBase, foreign_key = match_id))]
 pub struct MatchPlayer {
     pub match_id: i32,
     pub player_id: i32,
@@ -65,9 +62,9 @@ pub struct MatchPlayerData {
 pub fn create_match(
     new_match_base: &NewMatch,
     new_match_players: &[MatchPlayerData],
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<MatchData> {
-    conn.transaction(|| {
+    conn.transaction(|conn| {
         let match_base = diesel::insert_into(matches::table)
             .values(new_match_base)
             .get_result::<MatchBase>(conn)?;
@@ -101,7 +98,7 @@ pub struct MatchData {
 /// Add player information to MatchBase instances
 fn fetch_full_match_data(
     matches: Vec<MatchBase>,
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<Vec<FullMatchData>> {
     let map_ids: HashSet<i32> = matches.iter().filter_map(|m| m.map_id).collect();
 
@@ -140,8 +137,8 @@ fn fetch_full_match_data(
 }
 
 // TODO: this method should disappear
-pub fn list_matches(amount: i64, conn: &PgConnection) -> QueryResult<Vec<FullMatchData>> {
-    conn.transaction(|| {
+pub fn list_matches(amount: i64, conn: &mut PgConnection) -> QueryResult<Vec<FullMatchData>> {
+    conn.transaction(|conn| {
         let matches = matches::table
             .filter(matches::state.eq(MatchState::Finished))
             .order_by(matches::created_at.desc())
@@ -164,17 +161,32 @@ pub fn list_public_matches(
     amount: i64,
     before: Option<NaiveDateTime>,
     after: Option<NaiveDateTime>,
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<Vec<FullMatchData>> {
-    conn.transaction(|| {
+    conn.transaction(|conn| {
         // TODO: how can this common logic be abstracted?
-        let query = matches::table
+        let mut query = matches::table
             .filter(matches::state.eq(MatchState::Finished))
             .filter(matches::is_public.eq(true))
             .into_boxed();
 
-        let matches =
-            select_matches_page(query, amount, before, after).get_results::<MatchBase>(conn)?;
+        // TODO: how to remove this duplication?
+        query = match (before, after) {
+            (None, None) => query.order_by(matches::created_at.desc()),
+            (Some(before), None) => query
+                .filter(matches::created_at.lt(before))
+                .order_by(matches::created_at.desc()),
+            (None, Some(after)) => query
+                .filter(matches::created_at.gt(after))
+                .order_by(matches::created_at.asc()),
+            (Some(before), Some(after)) => query
+                .filter(matches::created_at.lt(before))
+                .filter(matches::created_at.gt(after))
+                .order_by(matches::created_at.desc()),
+        };
+        query = query.limit(amount);
+
+        let matches = query.get_results::<MatchBase>(conn)?;
         fetch_full_match_data(matches, conn)
     })
 }
@@ -185,7 +197,7 @@ pub fn list_bot_matches(
     amount: i64,
     before: Option<NaiveDateTime>,
     after: Option<NaiveDateTime>,
-    conn: &PgConnection,
+    conn: &mut PgConnection,
 ) -> QueryResult<Vec<FullMatchData>> {
     let mut query = matches::table
         .filter(matches::state.eq(MatchState::Finished))
@@ -211,22 +223,8 @@ pub fn list_bot_matches(
         };
     }
 
-    let matches =
-        select_matches_page(query, amount, before, after).get_results::<MatchBase>(conn)?;
-    fetch_full_match_data(matches, conn)
-}
-
-fn select_matches_page<QS>(
-    query: BoxedSelectStatement<'static, matches::SqlType, QS, Pg>,
-    amount: i64,
-    before: Option<NaiveDateTime>,
-    after: Option<NaiveDateTime>,
-) -> BoxedSelectStatement<'static, matches::SqlType, QS, Pg>
-where
-    QS: AppearsInFromClause<matches::table, Count = Once>,
-{
-    // TODO: this is not nice. Replace this with proper cursor logic.
-    match (before, after) {
+    // TODO: how to remove this duplication?
+    query = match (before, after) {
         (None, None) => query.order_by(matches::created_at.desc()),
         (Some(before), None) => query
             .filter(matches::created_at.lt(before))
@@ -238,8 +236,11 @@ where
             .filter(matches::created_at.lt(before))
             .filter(matches::created_at.gt(after))
             .order_by(matches::created_at.desc()),
-    }
-    .limit(amount)
+    };
+    query = query.limit(amount);
+
+    let matches = query.get_results::<MatchBase>(conn)?;
+    fetch_full_match_data(matches, conn)
 }
 
 // TODO: maybe unify this with matchdata?
@@ -270,8 +271,8 @@ impl BelongsTo<MatchBase> for FullMatchPlayerData {
     }
 }
 
-pub fn find_match(id: i32, conn: &PgConnection) -> QueryResult<FullMatchData> {
-    conn.transaction(|| {
+pub fn find_match(id: i32, conn: &mut PgConnection) -> QueryResult<FullMatchData> {
+    conn.transaction(|conn| {
         let match_base = matches::table.find(id).get_result::<MatchBase>(conn)?;
 
         let map = match match_base.map_id {
@@ -298,7 +299,7 @@ pub fn find_match(id: i32, conn: &PgConnection) -> QueryResult<FullMatchData> {
     })
 }
 
-pub fn find_match_base(id: i32, conn: &PgConnection) -> QueryResult<MatchBase> {
+pub fn find_match_base(id: i32, conn: &mut PgConnection) -> QueryResult<MatchBase> {
     matches::table.find(id).get_result::<MatchBase>(conn)
 }
 
@@ -306,7 +307,7 @@ pub enum MatchResult {
     Finished { winner: Option<i32> },
 }
 
-pub fn save_match_result(id: i32, result: MatchResult, conn: &PgConnection) -> QueryResult<()> {
+pub fn save_match_result(id: i32, result: MatchResult, conn: &mut PgConnection) -> QueryResult<()> {
     let MatchResult::Finished { winner } = result;
 
     diesel::update(matches::table.find(id))
@@ -320,17 +321,20 @@ pub fn save_match_result(id: i32, result: MatchResult, conn: &PgConnection) -> Q
 
 #[derive(QueryableByName)]
 pub struct BotStatsRecord {
-    #[sql_type = "Text"]
+    #[diesel(sql_type = Text)]
     pub opponent: String,
-    #[sql_type = "Text"]
+    #[diesel(sql_type = Text)]
     pub map: String,
-    #[sql_type = "Nullable<Bool>"]
+    #[diesel(sql_type = Nullable<Bool>)]
     pub win: Option<bool>,
-    #[sql_type = "Int8"]
+    #[diesel(sql_type = Int8)]
     pub count: i64,
 }
 
-pub fn fetch_bot_stats(bot_name: &str, db_conn: &PgConnection) -> QueryResult<Vec<BotStatsRecord>> {
+pub fn fetch_bot_stats(
+    bot_name: &str,
+    db_conn: &mut PgConnection,
+) -> QueryResult<Vec<BotStatsRecord>> {
     diesel::sql_query(
         "
 SELECT opponent, map, win, COUNT(*) as count
diff --git a/planetwars-server/src/db/ratings.rs b/planetwars-server/src/db/ratings.rs
index 8262fed..0a510d4 100644
--- a/planetwars-server/src/db/ratings.rs
+++ b/planetwars-server/src/db/ratings.rs
@@ -10,7 +10,7 @@ pub struct Rating {
     pub rating: f64,
 }
 
-pub fn get_rating(bot_id: i32, db_conn: &PgConnection) -> QueryResult<Option<f64>> {
+pub fn get_rating(bot_id: i32, db_conn: &mut PgConnection) -> QueryResult<Option<f64>> {
     ratings::table
         .filter(ratings::bot_id.eq(bot_id))
         .select(ratings::rating)
@@ -18,7 +18,7 @@ pub fn get_rating(bot_id: i32, db_conn: &PgConnection) -> QueryResult<Option<f64
         .optional()
 }
 
-pub fn set_rating(bot_id: i32, rating: f64, db_conn: &PgConnection) -> QueryResult<usize> {
+pub fn set_rating(bot_id: i32, rating: f64, db_conn: &mut PgConnection) -> QueryResult<usize> {
     diesel::insert_into(ratings::table)
         .values(Rating { bot_id, rating })
         .on_conflict(ratings::bot_id)
@@ -40,7 +40,7 @@ pub struct RankedBot {
     pub rating: f64,
 }
 
-pub fn get_bot_ranking(db_conn: &PgConnection) -> QueryResult<Vec<RankedBot>> {
+pub fn get_bot_ranking(db_conn: &mut PgConnection) -> QueryResult<Vec<RankedBot>> {
     bots::table
         .left_join(users::table)
         .inner_join(ratings::table)
diff --git a/planetwars-server/src/db/sessions.rs b/planetwars-server/src/db/sessions.rs
index a91d954..f8108cc 100644
--- a/planetwars-server/src/db/sessions.rs
+++ b/planetwars-server/src/db/sessions.rs
@@ -6,7 +6,7 @@ use diesel::{insert_into, prelude::*, Insertable, RunQueryDsl};
 use rand::{self, Rng};
 
 #[derive(Insertable)]
-#[table_name = "sessions"]
+#[diesel(table_name = sessions)]
 struct NewSession {
     token: String,
     user_id: i32,
@@ -19,7 +19,7 @@ pub struct Session {
     pub token: String,
 }
 
-pub fn create_session(user: &User, conn: &PgConnection) -> Session {
+pub fn create_session(user: &User, conn: &mut PgConnection) -> Session {
     let new_session = NewSession {
         token: gen_session_token(),
         user_id: user.id,
@@ -31,7 +31,7 @@ pub fn create_session(user: &User, conn: &PgConnection) -> Session {
         .unwrap()
 }
 
-pub fn find_user_by_session(token: &str, conn: &PgConnection) -> QueryResult<(Session, User)> {
+pub fn find_user_by_session(token: &str, conn: &mut PgConnection) -> QueryResult<(Session, User)> {
     sessions::table
         .inner_join(users::table)
         .filter(sessions::token.eq(&token))
diff --git a/planetwars-server/src/db/users.rs b/planetwars-server/src/db/users.rs
index 9676dae..60cc20a 100644
--- a/planetwars-server/src/db/users.rs
+++ b/planetwars-server/src/db/users.rs
@@ -11,7 +11,7 @@ pub struct Credentials<'a> {
 }
 
 #[derive(Insertable)]
-#[table_name = "users"]
+#[diesel(table_name = users)]
 pub struct NewUser<'a> {
     pub username: &'a str,
     pub password_hash: &'a [u8],
@@ -50,7 +50,7 @@ pub fn hash_password(password: &str) -> (Vec<u8>, [u8; 32]) {
     (hash, salt)
 }
 
-pub fn create_user(credentials: &Credentials, conn: &PgConnection) -> QueryResult<User> {
+pub fn create_user(credentials: &Credentials, conn: &mut PgConnection) -> QueryResult<User> {
     let (hash, salt) = hash_password(&credentials.password);
 
     let new_user = NewUser {
@@ -63,19 +63,19 @@ pub fn create_user(credentials: &Credentials, conn: &PgConnection) -> QueryResul
         .get_result::<User>(conn)
 }
 
-pub fn find_user(user_id: i32, db_conn: &PgConnection) -> QueryResult<User> {
+pub fn find_user(user_id: i32, db_conn: &mut PgConnection) -> QueryResult<User> {
     users::table
         .filter(users::id.eq(user_id))
         .first::<User>(db_conn)
 }
 
-pub fn find_user_by_name(username: &str, db_conn: &PgConnection) -> QueryResult<User> {
+pub fn find_user_by_name(username: &str, db_conn: &mut PgConnection) -> QueryResult<User> {
     users::table
         .filter(users::username.eq(username))
         .first::<User>(db_conn)
 }
 
-pub fn set_user_password(credentials: Credentials, db_conn: &PgConnection) -> QueryResult<()> {
+pub fn set_user_password(credentials: Credentials, db_conn: &mut PgConnection) -> QueryResult<()> {
     let (hash, salt) = hash_password(&credentials.password);
 
     let n_changes = diesel::update(users::table.filter(users::username.eq(&credentials.username)))
@@ -91,7 +91,7 @@ pub fn set_user_password(credentials: Credentials, db_conn: &PgConnection) -> Qu
     }
 }
 
-pub fn authenticate_user(credentials: &Credentials, db_conn: &PgConnection) -> Option<User> {
+pub fn authenticate_user(credentials: &Credentials, db_conn: &mut PgConnection) -> Option<User> {
     find_user_by_name(credentials.username, db_conn)
         .optional()
         .unwrap()