From a07f7ac3896af018adefaf73b038a49eb5a6eb5f Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Mon, 20 Mar 2023 22:26:38 +0100 Subject: [PATCH] server: ensure first/last name nullable, make avatar long blob in DB Fixes #474, #486. --- server/src/domain/sql_migrations.rs | 197 +++++++++++++++++++--------- server/src/domain/sql_tables.rs | 17 ++- 2 files changed, 144 insertions(+), 70 deletions(-) diff --git a/server/src/domain/sql_migrations.rs b/server/src/domain/sql_migrations.rs index 7be7b7f..bbbcf17 100644 --- a/server/src/domain/sql_migrations.rs +++ b/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( + 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::::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::::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::::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: - // - 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::::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?; + migrate_to_v2(pool).await?; } + if version < SchemaVersion(3) { + migrate_to_v3(pool).await?; + } + let builder = pool.get_database_backend(); pool.execute( builder.build( Query::update() diff --git a/server/src/domain/sql_tables.rs b/server/src/domain/sql_tables.rs index b61fd93..fa152f5 100644 --- a/server/src/domain/sql_tables.rs +++ b/server/src/domain/sql_tables.rs @@ -21,7 +21,7 @@ impl From 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) - VALUES ("bôb", "", "1970-01-01 00:00:00")"#, + r#"INSERT INTO users (user_id, display_name, first_name, creation_date) + 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) - VALUES ("john", "John Doe", "1971-01-01 00:00:00")"#, + r#"INSERT INTO users (user_id, display_name, first_name, creation_date) + 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, + first_name: Option, 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") } ]