server: ensure first/last name nullable, make avatar long blob in DB

Fixes #474, #486.

server/src/domain/sql_migrations.rs

@@ -11,7 +11,7 @@ use tracing::{info, instrument, warn};
 
 use super::sql_tables::LAST_SCHEMA_VERSION;
 
-#[derive(Iden, PartialEq, Eq, Debug, Serialize, Deserialize, Clone)]
+#[derive(Iden, PartialEq, Eq, Debug, Serialize, Deserialize, Clone, Copy)]
 pub enum Users {
     Table,
     UserId,
@@ -27,7 +27,7 @@ pub enum Users {
     Uuid,
 }
 
-#[derive(Iden, PartialEq, Eq, Debug, Serialize, Deserialize, Clone)]
+#[derive(Iden, PartialEq, Eq, Debug, Serialize, Deserialize, Clone, Copy)]
 pub enum Groups {
     Table,
     GroupId,
@@ -36,7 +36,7 @@ pub enum Groups {
     Uuid,
 }
 
-#[derive(Iden)]
+#[derive(Iden, Clone, Copy)]
 pub enum Memberships {
     Table,
     UserId,
@@ -334,6 +334,132 @@ pub async fn upgrade_to_v1(pool: &DbConnection) -> std::result::Result<(), sea_o
     Ok(())
 }
 
+async fn replace_column<I: Iden + Copy + 'static, const N: usize>(
+    pool: &DbConnection,
+    table_name: I,
+    column_name: I,
+    mut new_column: ColumnDef,
+    update_values: [Statement; N],
+) -> anyhow::Result<()> {
+    // Update the definition of a column (in a compatible way). Due to Sqlite, this is more complicated:
+    //  - rename the column to a temporary name
+    //  - create the column with the new definition
+    //  - copy the data from the temp column to the new one
+    //  - update the new one if there are changes needed
+    //  - drop the old one
+    let builder = pool.get_database_backend();
+    pool.transaction::<_, (), sea_orm::DbErr>(move |transaction| {
+        Box::pin(async move {
+            #[derive(Iden)]
+            enum TempTable {
+                TempName,
+            }
+            transaction
+                .execute(
+                    builder.build(
+                        Table::alter()
+                            .table(table_name)
+                            .rename_column(column_name, TempTable::TempName),
+                    ),
+                )
+                .await?;
+            transaction
+                .execute(
+                    builder.build(Table::alter().table(table_name).add_column(&mut new_column)),
+                )
+                .await?;
+            transaction
+                .execute(
+                    builder.build(
+                        Query::update()
+                            .table(table_name)
+                            .value(column_name, Expr::col((table_name, TempTable::TempName))),
+                    ),
+                )
+                .await?;
+            for statement in update_values {
+                transaction.execute(statement).await?;
+            }
+            transaction
+                .execute(
+                    builder.build(
+                        Table::alter()
+                            .table(table_name)
+                            .drop_column(TempTable::TempName),
+                    ),
+                )
+                .await?;
+            Ok(())
+        })
+    })
+    .await?;
+    Ok(())
+}
+
+async fn migrate_to_v2(pool: &DbConnection) -> anyhow::Result<()> {
+    let builder = pool.get_database_backend();
+    // Allow nulls in DisplayName, and change empty string to null.
+    replace_column(
+        pool,
+        Users::Table,
+        Users::DisplayName,
+        ColumnDef::new(Users::DisplayName)
+            .string_len(255)
+            .to_owned(),
+        [builder.build(
+            Query::update()
+                .table(Users::Table)
+                .value(Users::DisplayName, Option::<String>::None)
+                .cond_where(Expr::col(Users::DisplayName).eq("")),
+        )],
+    )
+    .await?;
+    Ok(())
+}
+
+async fn migrate_to_v3(pool: &DbConnection) -> anyhow::Result<()> {
+    let builder = pool.get_database_backend();
+    // Allow nulls in First and LastName. Users who created their DB in 0.4.1 have the not null constraint.
+    replace_column(
+        pool,
+        Users::Table,
+        Users::FirstName,
+        ColumnDef::new(Users::FirstName).string_len(255).to_owned(),
+        [builder.build(
+            Query::update()
+                .table(Users::Table)
+                .value(Users::FirstName, Option::<String>::None)
+                .cond_where(Expr::col(Users::FirstName).eq("")),
+        )],
+    )
+    .await?;
+    replace_column(
+        pool,
+        Users::Table,
+        Users::LastName,
+        ColumnDef::new(Users::LastName).string_len(255).to_owned(),
+        [builder.build(
+            Query::update()
+                .table(Users::Table)
+                .value(Users::LastName, Option::<String>::None)
+                .cond_where(Expr::col(Users::LastName).eq("")),
+        )],
+    )
+    .await?;
+    // Change Avatar from binary to blob(long), because for MySQL this is 64kb.
+    replace_column(
+        pool,
+        Users::Table,
+        Users::Avatar,
+        ColumnDef::new(Users::Avatar)
+            .blob(sea_query::BlobSize::Long)
+            .to_owned(),
+        [],
+    )
+    .await?;
+    Ok(())
+}
+
 pub async fn migrate_from_version(
     pool: &DbConnection,
     version: SchemaVersion,
@@ -346,68 +472,13 @@ pub async fn migrate_from_version(
         std::cmp::Ordering::Equal => return Ok(()),
         std::cmp::Ordering::Greater => anyhow::bail!("DB version downgrading is not supported"),
     }
-    let builder = pool.get_database_backend();
     if version < SchemaVersion(2) {
-        // Drop the not_null constraint on display_name. Due to Sqlite, this is more complicated:
+        migrate_to_v2(pool).await?;
-        //  - rename the display_name column to a temporary name
-        //  - create the display_name column without the constraint
-        //  - copy the data from the temp column to the new one
-        //  - update the new one to replace empty strings with null
-        //  - drop the old one
-        pool.transaction::<_, (), sea_orm::DbErr>(|transaction| {
-            Box::pin(async move {
-                #[derive(Iden)]
-                enum TempUsers {
-                    TempDisplayName,
-                }
-                transaction
-                    .execute(
-                        builder.build(
-                            Table::alter()
-                                .table(Users::Table)
-                                .rename_column(Users::DisplayName, TempUsers::TempDisplayName),
-                        ),
-                    )
-                    .await?;
-                transaction
-                    .execute(
-                        builder.build(
-                            Table::alter()
-                                .table(Users::Table)
-                                .add_column(ColumnDef::new(Users::DisplayName).string_len(255)),
-                        ),
-                    )
-                    .await?;
-                transaction
-                    .execute(builder.build(Query::update().table(Users::Table).value(
-                        Users::DisplayName,
-                        Expr::col((Users::Table, TempUsers::TempDisplayName)),
-                    )))
-                    .await?;
-                transaction
-                    .execute(
-                        builder.build(
-                            Query::update()
-                                .table(Users::Table)
-                                .value(Users::DisplayName, Option::<String>::None)
-                                .cond_where(Expr::col(Users::DisplayName).eq("")),
-                        ),
-                    )
-                    .await?;
-                transaction
-                    .execute(
-                        builder.build(
-                            Table::alter()
-                                .table(Users::Table)
-                                .drop_column(TempUsers::TempDisplayName),
-                        ),
-                    )
-                    .await?;
-                Ok(())
-            })
-        })
-        .await?;
     }
+    if version < SchemaVersion(3) {
+        migrate_to_v3(pool).await?;
+    }
+    let builder = pool.get_database_backend();
     pool.execute(
         builder.build(
             Query::update()

server/src/domain/sql_tables.rs

@@ -21,7 +21,7 @@ impl From<SchemaVersion> for Value {
     }
 }
 
-pub const LAST_SCHEMA_VERSION: SchemaVersion = SchemaVersion(2);
+pub const LAST_SCHEMA_VERSION: SchemaVersion = SchemaVersion(3);
 
 pub async fn init_table(pool: &DbConnection) -> anyhow::Result<()> {
     let version = {
@@ -100,21 +100,21 @@ mod tests {
         let sql_pool = get_in_memory_db().await;
         sql_pool
             .execute(raw_statement(
-                r#"CREATE TABLE users ( user_id TEXT, display_name TEXT, creation_date TEXT);"#,
+                r#"CREATE TABLE users ( user_id TEXT, display_name TEXT, first_name TEXT NOT NULL, last_name TEXT, avatar BLOB, creation_date TEXT);"#,
             ))
             .await
             .unwrap();
         sql_pool
             .execute(raw_statement(
-                r#"INSERT INTO users (user_id, display_name, creation_date)
+                r#"INSERT INTO users (user_id, display_name, first_name, creation_date)
-                       VALUES ("bôb", "", "1970-01-01 00:00:00")"#,
+                       VALUES ("bôb", "", "", "1970-01-01 00:00:00")"#,
             ))
             .await
             .unwrap();
         sql_pool
             .execute(raw_statement(
-                r#"INSERT INTO users (user_id, display_name, creation_date)
+                r#"INSERT INTO users (user_id, display_name, first_name, creation_date)
-                       VALUES ("john", "John Doe", "1971-01-01 00:00:00")"#,
+                       VALUES ("john", "John Doe", "John", "1971-01-01 00:00:00")"#,
             ))
             .await
             .unwrap();
@@ -142,11 +142,12 @@ mod tests {
         #[derive(FromQueryResult, PartialEq, Eq, Debug)]
         struct SimpleUser {
             display_name: Option<String>,
+            first_name: Option<String>,
             uuid: Uuid,
         }
         assert_eq!(
             SimpleUser::find_by_statement(raw_statement(
-                r#"SELECT display_name, uuid FROM users ORDER BY display_name"#
+                r#"SELECT display_name, first_name, uuid FROM users ORDER BY display_name"#
             ))
             .all(&sql_pool)
             .await
@@ -154,10 +155,12 @@ mod tests {
             vec![
                 SimpleUser {
                     display_name: None,
+                    first_name: None,
                     uuid: crate::uuid!("a02eaf13-48a7-30f6-a3d4-040ff7c52b04")
                 },
                 SimpleUser {
                     display_name: Some("John Doe".to_owned()),
+                    first_name: Some("John".to_owned()),
                     uuid: crate::uuid!("986765a5-3f03-389e-b47b-536b2d6e1bec")
                 }
             ]