Feedback updates

docs/database_migration.md

@@ -7,10 +7,10 @@ NOTE: [pgloader](https://github.com/dimitri/pgloader) is a tool that can easily
 The process is as follows:
 
 1. Create empty schema on target database
-2. Dump existing values
+2. Stop/pause LLDAP and dump existing values
 3. Sanitize for target DB (not always required)
 4. Insert data into target
-5. Change LLDAP config to new target
+5. Change LLDAP config to new target and restart
 
 The steps below assume you already have PostgreSQL or MySQL set up with an empty database for LLDAP to use.
 
@@ -28,39 +28,62 @@ If it succeeds, you can proceed to the next step.
 
 ## Create a dump of existing data
 
-We want to dump all existing values to some file. The dump should consist just INSERT
+We want to dump (almost) all existing values to some file - the exception being the `metadata` table. Be sure to stop/pause LLDAP during this step, as some
+databases (SQLite in this example) will give an error if LLDAP is in the middle of a write. The dump should consist just INSERT
 statements. There are various ways to do this, but a simple enough way is filtering a
 whole database dump. For example:
 
 ```
-sqlite3 /path/to/lldap/config/users.db .dump | grep "^INSERT" > /path/to/dump.sql
+sqlite3 /path/to/lldap/config/users.db .dump | grep "^INSERT" | grep -v "^INSERT INTO metadata" > /path/to/dump.sql
 ```
 
-## Sanitize data (if needed)
+## Sanitize data
 
 Some databases might use different formats for some data - for example, PostgreSQL uses
-a different syntax for hex strings than SQLite.
+a different syntax for hex strings than SQLite. We also want to make sure inserts are done in
+a transaction in case one of the statements fail.
 
 ### To PostgreSQL
 
 PostgreSQL uses a different hex string format. The command below should switch SQLite
-format to PostgreSQL format.
+format to PostgreSQL format, and wrap it all in a transaction:
 
 ```
-sed -i -r "s/X'([[:xdigit:]]+'[^'])/'\\\x\\1/g" /path/to/dump.sql
+sed -i -r -e "s/X'([[:xdigit:]]+'[^'])/'\\\x\\1/g" \
+-e '1s/^/BEGIN;\n/' \
+-e '$aCOMMIT;' /path/to/dump.sql
+```
+
+### To MySQL
+
+MySQL mostly cooperates, but it gets some errors if you don't escape the `groups` table. Run the
+following command to wrap all table names in backticks for good measure, and wrap the inserts in
+a transaction:
+
+```
+sed -i -r -e 's/^INSERT INTO ([a-zA-Z0-9_]+) /INSERT INTO `\1` /' \
+-e '1s/^/START TRANSACTION;\n/' \
+-e '$aCOMMIT;' /path/to/dump.sql
 ```
 
 ## Insert data
 
-Insert the data generated from the previous step into the target database.
+Insert the data generated from the previous step into the target database. If you encounter errors,
+you may need to manually tweak your dump, or make changed in LLDAP and recreate the dump.
 
 ### PostgreSQL
 
 `psql -d <database> -U <username> -W < /path/to/dump.sql`
 
+or 
+
+`psql -d <database> -U <username> -W -f /path/to/dump.sql`
+
 ### MySQL
 
-`mysql -u < -p <database> < /path/to/dump.sql`
+`mysql -u <username> -p <database> < /path/to/dump.sql`
+
+NOTE: MySQL will limit avatar size to 64Kb. You may need to remove avatars for some users. (Untested: you could probably change the column type to LONGBLOB )
 
 ## Switch to new database
 

server/src/infra/cli.rs

@@ -28,7 +28,7 @@ pub enum Command {
     SendTestEmail(TestEmailOpts),
     /// Create database schema.
     #[clap(name = "create_schema")]
-    CreateSchema(CreateSchemaOpts),
+    CreateSchema(RunOpts),
 }
 
 #[derive(Debug, Parser, Clone)]
@@ -77,6 +77,10 @@ pub struct RunOpts {
     #[clap(long, env = "LLDAP_HTTP_URL")]
     pub http_url: Option<String>,
 
+    /// Database connection URL
+    #[clap(short, long, env = "LLDAP_DATABASE_URL")]
+    pub database_url: Option<String>,
+
     #[clap(flatten)]
     pub smtp_opts: SmtpOpts,
 
@@ -97,13 +101,6 @@ pub struct TestEmailOpts {
     pub smtp_opts: SmtpOpts,
 }
 
-#[derive(Debug, Parser, Clone)]
-pub struct CreateSchemaOpts {
-    /// Database connection URL
-    #[clap(short, long, env = "LLDAP_CREATE_SCHEMA_DATABASE_URL")]
-    pub database_url: String,
-}
-
 #[derive(Debug, Parser, Clone)]
 #[clap(next_help_heading = Some("LDAPS"))]
 pub struct LdapsOpts {

server/src/infra/configuration.rs

@@ -209,6 +209,10 @@ impl ConfigOverrider for RunOpts {
         if let Some(url) = self.http_url.as_ref() {
             config.http_url = url.to_string();
         }
+
+        if let Some(database_url) = self.database_url.as_ref() {
+            config.database_url = database_url.to_string();
+        }
         self.smtp_opts.override_config(config);
         self.ldaps_opts.override_config(config);
     }

server/src/main.rs

@@ -200,15 +200,19 @@ async fn create_schema(database_url: String) -> Result<()> {
     };
     domain::sql_tables::init_table(&sql_pool)
         .await
-        .context("while creating the tables")?;
+        .context("while creating base tables")?;
+    infra::jwt_sql_tables::init_table(&sql_pool).await.context("while creating jwt tables")?;
     Ok(())
 }
 
-fn create_schema_command(opts: CreateSchemaOpts) -> Result<()> {
+fn create_schema_command(opts: RunOpts) -> Result<()> {
     debug!("CLI: {:#?}", &opts);
+    let config = infra::configuration::init(opts)?;
+    infra::logging::init(&config)?;
+    let database_url = config.database_url;
 
     actix::run(
-        create_schema(opts.database_url)
+        create_schema(database_url)
             .unwrap_or_else(|e| error!("Could not create schema: {:#}", e)),
     )?;