Work in progress

webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/checks/migration/MigrateEventLogEnforceSchema.scala

@@ -39,11 +39,12 @@ package bootstrap.liftweb.checks.migration
 
 import bootstrap.liftweb.BootstrapChecks
 import bootstrap.liftweb.BootstrapLogger
-import com.normation.errors.IOResult
 import com.normation.rudder.db.Doobie
 import com.normation.zio._
 import doobie.implicits._
 import doobie.util.fragment.Fragment
+import zio.Fiber
+import zio.URIO
 import zio.interop.catz._
 
 /*
@@ -57,55 +58,143 @@ class MigrateEventLogEnforceSchema(
 
   import doobie._
 
+  def table: Fragment = Fragment.const("eventlog")
+
   val msg: String = "eventLog columns that should be not null (eventtype, principal, severity, data)"
 
   override def description: String =
     "Check if eventtype, principal, severity, data have a not null constraint, otherwise migrate these columns"
 
+  private val defaultEventType = Fragment.const("''")
   private val defaultSeverity  = Fragment.const("100")
   private val defaultPrincipal = Fragment.const("'unknown'")
+  private val defaultData      = Fragment.const("''")
 
-  private def alterTableStatement: IOResult[Unit] = {
-    val sql = {
-      sql"""
+  def migrationStatement: Fragment = {
+    val statement = sql"""
         -- Alter the EventLog schema for the 'eventType' column
-        update EventLog set eventType = '' where eventType is null;
-        alter table EventLog
-        alter column eventType set default '',
-        alter column eventType set not null;
+        DO $$$$
+        DECLARE
+          -- the table and column to add non null constraint to, and to set default value when recovering from null
+          -- WARNING: table name and column name are case sensitive, needs to be lowercase if not quoted during column creation
+          tablename text := '${table}';
+          migrationcolumn text := 'eventtype';
+          defaultvalue text := ${defaultEventType};
+        BEGIN
+          EXECUTE (WITH any_nullable_column AS 
+            (SELECT DISTINCT column_name, table_name, is_nullable
+             FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = tablename and column_name = migrationcolumn and is_nullable = 'YES')
+            SELECT format('
+                update %1$$I set %2$$I = %3$$L where %2$$I is null;
+                alter table %1$$I
+                alter column %2$$I set default %3$$L,
+                alter column %2$$I set not null;
+              ',
+              tablename,
+              migrationcolumn,
+              defaultvalue
+            )
+          );
+        END
+        $$$$;
 
         -- Alter the EventLog schema for the 'principal' column
-        update EventLog set principal = ${defaultPrincipal} where principal is null;
-        alter table EventLog
-        alter column principal set default ${defaultPrincipal},
-        alter column principal set not null;
+        DO $$$$
+        DECLARE
+          tablename text := '${table}';
+          migrationcolumn text := 'principal';
+          defaultvalue text := ${defaultPrincipal};
+        BEGIN
+          EXECUTE (WITH any_nullable_column AS 
+            (SELECT DISTINCT column_name, table_name, is_nullable
+             FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = tablename and column_name = migrationcolumn and is_nullable = 'YES')
+            SELECT format('
+                update %1$$I set %2$$I = %3$$L where %2$$I is null;
+                alter table %1$$I
+                alter column %2$$I set default %3$$L,
+                alter column %2$$I set not null;
+              ',
+              tablename,
+              migrationcolumn,
+              defaultvalue
+            )
+          );
+        END
+        $$$$;
+
 
         -- Alter the EventLog schema for the 'severity' column
-        update EventLog set severity = ${defaultSeverity} where severity is null;
-        alter table EventLog
-        alter column severity set default ${defaultSeverity},
-        alter column severity set not null;
+        DO $$$$
+        DECLARE
+          tablename text := '${table}';
+          migrationcolumn text := 'severity';
+          defaultvalue text := ${defaultSeverity};
+        BEGIN
+          EXECUTE (WITH any_nullable_column AS 
+            (SELECT DISTINCT column_name, table_name, is_nullable
+             FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = tablename and column_name = migrationcolumn and is_nullable = 'YES')
+            SELECT format('
+                update %1$$I set %2$$I = %3$$L where %2$$I is null;
+                alter table %1$$I
+                alter column %2$$I set default %3$$L,
+                alter column %2$$I set not null;
+              ',
+              tablename,
+              migrationcolumn,
+              defaultvalue
+            )
+          );
+        END
+        $$$$;
 
         -- Alter the EventLog schema for the 'data' column
-        update EventLog set data = '' where data is null;
-        alter table EventLog
-        alter column data set default '',
-        alter column data set not null;
+        DO $$$$
+        DECLARE
+          tablename text := '${table}';
+          migrationcolumn text := 'data';
+          defaultvalue text := ${defaultData};
+        BEGIN
+          EXECUTE (WITH any_nullable_column AS 
+            (SELECT DISTINCT column_name, table_name, is_nullable
+             FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = tablename and column_name = migrationcolumn and is_nullable = 'YES')
+            SELECT format('
+                update %1$$I set %2$$I = %3$$L where %2$$I is null;
+                alter table %1$$I
+                alter column %2$$I set default %3$$L,
+                alter column %2$$I set not null;
+              ',
+              tablename,
+              migrationcolumn,
+              defaultvalue
+            )
+          );
+        END
+        $$$$;
       """
-    }
-
-    transactIOResult(s"Error with 'EventLog' table migration")(xa => sql.update.run.transact(xa)).unit
+    statement
   }
 
-  override def checks(): Unit = {
+  val migrateAsync: URIO[Any, Fiber.Runtime[Nothing, Unit]] = {
     val prog = {
       for {
-        _ <- alterTableStatement
+        _ <- transactIOResult(s"Error with 'EventLog' table migration")(xa => migrationStatement.update.run.transact(xa)).unit
         _ <- BootstrapLogger.info(s"Migrated ${msg}")
       } yield ()
     }
 
-    prog.catchAll(err => BootstrapLogger.error(s"Error when trying to migrate ${msg}: ${err.fullMsg}")).forkDaemon.runNow
+    prog
+      .catchAll(err => {
+        BootstrapLogger.error(
+          s"Non-fatal error when trying to migrate ${msg}." +
+          s"\nThis is a data check and it should not alter the behavior of Rudder." +
+          s"\nPlease contact the development team with the following information to help resolving the issue : ${err.fullMsg}"
+        )
+      })
+      .forkDaemon
+  }
+
+  override def checks(): Unit = {
+    migrateAsync.runNow
   }
 
 }

webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateEventLogEnforceSchema.scala

@@ -0,0 +1,117 @@
+package bootstrap.liftweb.checks.migration
+
+import com.normation.rudder.db.{DBCommon, Doobie}
+import com.normation.zio.UnsafeRun
+import doobie.syntax.all._
+import doobie.util.fragment
+import doobie.util.fragments.whereAnd
+import org.junit.runner.RunWith
+import org.specs2.runner.JUnitRunner
+import zio.interop.catz._
+
+import scala.util.{Failure, Success, Try}
+
+@RunWith(classOf[JUnitRunner])
+class TestMigrateEventLogEnforceSchema extends DBCommon {
+  import TestMigrateEventLogEnforceSchema._
+
+  override def cleanDb(): Unit = {
+    doobie.transactRunEither(
+      sql"""
+        DROP TABLE IF EXISTS $tempTable;
+        DROP SEQUENCE IF EXISTS eventLogIdSeq_temp;
+        DROP INDEX IF EXISTS eventType_idx_temp;
+        DROP INDEX IF EXISTS creationDate_idx_temp;
+        DROP INDEX IF EXISTS eventlog_fileFormat_idx_temp;
+      """.update.run.transact(_)
+    ) match {
+      case Right(_) => ()
+      case Left(ex) => throw ex
+    }
+  }
+
+  // The previous schema, with the renamed table for this test
+  // We add need to know for sure the initial state and the final state of the migrated table,
+  // so we need to define the previous schema without conflicting with the current one (with all values renamed)
+  private val previousSchemaDDL = sql"""
+    CREATE SEQUENCE eventLogIdSeq_temp START 1;
+    
+    CREATE TABLE $tempTable (
+      id             integer PRIMARY KEY  DEFAULT nextval('eventLogIdSeq_temp')
+    , creationDate   timestamp with time zone NOT NULL DEFAULT 'now'
+    , severity       integer
+    , causeId        integer
+    , modificationId text
+    , principal      text
+    , reason         text
+    , eventType      text
+    , data           xml
+    );
+    
+    CREATE INDEX eventType_idx_temp ON $tempTable (eventType);
+    CREATE INDEX creationDate_idx_temp ON $tempTable (creationDate);
+    CREATE INDEX eventlog_fileFormat_idx_temp ON $tempTable (((((xpath('/entry//@fileFormat',data))[1])::text)));
+  """
+
+  override def initDb(): Unit = {
+    super.initDb()
+    doobie.transactRunEither(previousSchemaDDL.update.run.transact(_)) match {
+      case Right(_) => ()
+      case Left(ex) => throw ex
+    }
+  }
+
+  "MigrateEventLogEnforceSchema" should {
+
+    "migrate all columns successfully" in {
+      lazy val migrationEventLogRepository = new MigrateEventLogEnforceSchemaTempTable(doobie)
+
+      migrationEventLogRepository.checks()
+
+      doobie.transactRunEither(
+        (sql"""
+        SELECT DISTINCT column_name, table_name, is_nullable
+        FROM INFORMATION_SCHEMA.COLUMNS 
+      """ ++ whereAnd(
+            fr"table_name = 'eventlog'",
+            fr"column_name in ('eventtype', 'principal', 'severity', 'data')",
+            fr"is_nullable = 'YES'"
+          ))
+          .query[(String, String, String)]
+          .option
+          .transact(_)
+      ) match {
+        case Right(res) => res must beNone
+        case Left(ex)   =>
+          ko(
+            s"The migration of 'EventLog' table does not add NOT NULL constraint to all columns with error : ${ex.getMessage}"
+          )
+      }
+    }
+
+    "report an error message when it fails" in {
+      lazy val migrationEventLogRepository = new MigrateEventLogEnforceSchemaTempTable(doobie, Some(sql"some invalid query"))
+
+      Try(migrationEventLogRepository.migrateAsync.runNow.join.runNow) match {
+        case Failure(ex) => ex.getMessage must contain("Please contact the development team with the following information to help resolving the issue : ${err.fullMsg}")
+        case Success(()) => ko("The migration is supposed to throw an error, but it did not")
+      }
+
+    }
+  }
+}
+
+object TestMigrateEventLogEnforceSchema {
+  // this table is setup and tear down for the lifetime of this test execution
+  private val tempTable = fragment.Fragment.const("eventlog_migratetest")
+
+  private class MigrateEventLogEnforceSchemaTempTable(doobie: Doobie, overrideStatement: Option[fragment.Fragment] = None)
+      extends MigrateEventLogEnforceSchema(doobie) {
+    override def table              = tempTable
+    override def migrationStatement = {
+      val default = super.migrationStatement
+      overrideStatement.getOrElse(default)
+    }
+  }
+
+}