changeset 145:2a36a889eaec

merge
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 12 Mar 2020 12:11:10 +0100
parents 740d9829f52e (diff) 33467fb6aee8 (current diff)
children 6ff3728528f3
files
diffstat 6 files changed, 157 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/MySQL/MySQLDatabase.cpp	Fri Mar 06 14:20:49 2020 +0100
+++ b/Framework/MySQL/MySQLDatabase.cpp	Thu Mar 12 12:11:10 2020 +0100
@@ -420,6 +420,36 @@
   }
 
 
+  bool MySQLDatabase::DoesTriggerExist(MySQLTransaction& transaction,
+                                       const std::string& name)
+  {
+    if (mysql_ == NULL)
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+    }
+
+    if (!IsValidDatabaseIdentifier(name))
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_ParameterOutOfRange);
+    }
+  
+    Query query("SELECT COUNT(*) FROM information_schema.TRIGGERS "
+                "WHERE TRIGGER_NAME = ${trigger}", true);
+    query.SetType("trigger", ValueType_Utf8String);
+    
+    MySQLStatement statement(*this, query);
+
+    Dictionary args;
+    args.SetUtf8Value("trigger", name);
+
+    std::auto_ptr<IResult> result(statement.Execute(transaction, args));
+    return (!result->IsDone() &&
+            result->GetFieldsCount() == 1 &&
+            result->GetField(0).GetType() == ValueType_Integer64 &&
+            dynamic_cast<const Integer64Value&>(result->GetField(0)).GetValue() == 1);            
+  }
+
+
   void MySQLDatabase::Execute(const std::string& sql,
                               bool arobaseSeparator)
   {
--- a/Framework/MySQL/MySQLDatabase.h	Fri Mar 06 14:20:49 2020 +0100
+++ b/Framework/MySQL/MySQLDatabase.h	Thu Mar 12 12:11:10 2020 +0100
@@ -88,6 +88,9 @@
     bool DoesDatabaseExist(MySQLTransaction& transaction,
                            const std::string& name);
 
+    bool DoesTriggerExist(MySQLTransaction& transaction,
+                          const std::string& name);
+
     virtual Dialect GetDialect() const
     {
       return Dialect_MySQL;
--- a/MySQL/NEWS	Fri Mar 06 14:20:49 2020 +0100
+++ b/MySQL/NEWS	Thu Mar 12 12:11:10 2020 +0100
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+* Report issue/solution and prevent the start of Orthanc if the MySQL user is
+  not allowed to run the "CREATE TRIGGER" command (missing "SUPER" privilege)
 * Implementation of new extensions: LookupResourceAndParent and GetAllMetadata
 * Added an advisory lock to avoid race conditions during database setup
 * Added "MaximumConnectionRetries" & "ConnectionRetryInterval" to configure 
--- a/MySQL/Plugins/CreateInstance.sql	Fri Mar 06 14:20:49 2020 +0100
+++ b/MySQL/Plugins/CreateInstance.sql	Thu Mar 12 12:11:10 2020 +0100
@@ -1,3 +1,5 @@
+DROP PROCEDURE IF EXISTS CreateInstance;
+
 CREATE PROCEDURE CreateInstance(
        IN patient TEXT,
        IN study TEXT,
--- a/MySQL/Plugins/GetLastChangeIndex.sql	Fri Mar 06 14:20:49 2020 +0100
+++ b/MySQL/Plugins/GetLastChangeIndex.sql	Thu Mar 12 12:11:10 2020 +0100
@@ -1,13 +1,17 @@
-CREATE TABLE GlobalIntegers(
+CREATE TABLE IF NOT EXISTS GlobalIntegers(
        property INTEGER PRIMARY KEY,
        value BIGINT
        );
 
 
+DELETE FROM GlobalIntegers WHERE property = 0;
+
 INSERT INTO GlobalIntegers
 SELECT 0, COALESCE(MAX(seq), 0) FROM Changes;
 
 
+DROP TRIGGER IF EXISTS ChangeAdded;
+
 CREATE TRIGGER ChangeAdded
 AFTER INSERT ON Changes
 FOR EACH ROW
--- a/MySQL/Plugins/MySQLIndex.cpp	Fri Mar 06 14:20:49 2020 +0100
+++ b/MySQL/Plugins/MySQLIndex.cpp	Thu Mar 12 12:11:10 2020 +0100
@@ -35,6 +35,16 @@
 
 namespace OrthancDatabases
 {
+  static void ThrowCannotCreateTrigger()
+  {
+    LOG(ERROR) << "The MySQL user is not allowed to create triggers => 2 possible solutions:";
+    LOG(ERROR) << "  1- Give the SUPER privilege to the MySQL database user, or";
+    LOG(ERROR) << "  2- Run \"set global log_bin_trust_function_creators=1;\" as MySQL root user.";
+    LOG(ERROR) << "Once you are done, drop and recreate the MySQL database";
+    throw Orthanc::OrthancException(Orthanc::ErrorCode_Database,
+                                    "Need to fix the MySQL permissions for \"CREATE TRIGGER\"");
+  }
+  
   IDatabase* MySQLIndex::OpenInternal()
   {
     uint32_t expectedVersion = 6;
@@ -74,102 +84,173 @@
 
     {
       MySQLDatabase::TransientAdvisoryLock lock(*db, MYSQL_LOCK_DATABASE_SETUP);
-      MySQLTransaction t(*db);
 
-      db->Execute("ALTER DATABASE " + parameters_.GetDatabase() + 
-                  " CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", false);
-    
-      if (!db->DoesTableExist(t, "Resources"))
+      /**
+       * In a first transaction, we create the tables. Such a
+       * transaction cannot be rollback: "The CREATE TABLE statement
+       * in InnoDB is processed as a single transaction. This means
+       * that a ROLLBACK from the user does not undo CREATE TABLE
+       * statements the user made during that transaction."
+       * https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
+       *
+       * As a consequence, we delay the initial population of the
+       * tables in a sequence of transactions below. This solves the
+       * error message "MySQL plugin is incompatible with database
+       * schema version: 0" that was reported in the forum:
+       * https://groups.google.com/d/msg/orthanc-users/OCFFkm1qm0k/Mbroy8VWAQAJ
+       **/      
       {
-        std::string query;
+        MySQLTransaction t(*db);
+        
+        db->Execute("ALTER DATABASE " + parameters_.GetDatabase() + 
+                    " CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci", false);
 
-        Orthanc::EmbeddedResources::GetFileResource
-          (query, Orthanc::EmbeddedResources::MYSQL_PREPARE_INDEX);
-        db->Execute(query, true);
+        // This is the first table to be created
+        if (!db->DoesTableExist(t, "GlobalProperties"))
+        {
+          std::string query;
+          
+          Orthanc::EmbeddedResources::GetFileResource
+            (query, Orthanc::EmbeddedResources::MYSQL_PREPARE_INDEX);
+          db->Execute(query, true);
+        }
 
-        SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion, expectedVersion);
-        SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, 1);
+        t.Commit();
       }
 
-      if (!db->DoesTableExist(t, "Resources"))
-      {
-        LOG(ERROR) << "Corrupted MySQL database";
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);        
-      }
+      /**
+       * This is the sequence of transactions that initially populate
+       * the database. WARNING - As table creation cannot be rollback,
+       * don't forget to add "IF NOT EXISTS" if some table must be
+       * created below this point (in order to recover from failed
+       * transaction).
+       **/
 
       int version = 0;
-      if (!LookupGlobalIntegerProperty(version, *db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion) ||
-          version != 6)
+
       {
-        LOG(ERROR) << "MySQL plugin is incompatible with database schema version: " << version;
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
+        MySQLTransaction t(*db);
+
+        // This is the last table to be created
+        if (!db->DoesTableExist(t, "PatientRecyclingOrder"))
+        {
+          LOG(ERROR) << "Corrupted MySQL database";
+          throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);        
+        }
+
+        // This is the last item to be created
+        if (!db->DoesTriggerExist(t, "PatientAdded"))
+        {
+          ThrowCannotCreateTrigger();
+        }
+
+        if (!LookupGlobalIntegerProperty(version, *db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion))
+        {
+          SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabaseSchemaVersion, expectedVersion);
+          SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, 1);
+          version = expectedVersion;
+        }
+
+        if (version != 6)
+        {
+          LOG(ERROR) << "MySQL plugin is incompatible with database schema version: " << version;
+          throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
+        }
+
+        t.Commit();
       }
 
-      int revision;
-      if (!LookupGlobalIntegerProperty(revision, *db, t, Orthanc::GlobalProperty_DatabasePatchLevel))
+      int revision = 0;
+
       {
-        revision = 1;
-        SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+        MySQLTransaction t(*db);
+
+        if (!LookupGlobalIntegerProperty(revision, *db, t, Orthanc::GlobalProperty_DatabasePatchLevel))
+        {
+          revision = 1;
+          SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+        }
+
+        t.Commit();
       }
 
       if (revision == 1)
       {
+        MySQLTransaction t(*db);
+        
         // The serialization of jobs as a global property can lead to
         // very long values => switch to the LONGTEXT type that can
         // store up to 4GB:
         // https://stackoverflow.com/a/13932834/881731
         db->Execute("ALTER TABLE GlobalProperties MODIFY value LONGTEXT", false);
-
+        
         revision = 2;
         SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+
+        t.Commit();
       }
 
       if (revision == 2)
-      {
+      {        
+        MySQLTransaction t(*db);
+
         // Install the "GetLastChangeIndex" extension
         std::string query;
 
         Orthanc::EmbeddedResources::GetFileResource
           (query, Orthanc::EmbeddedResources::MYSQL_GET_LAST_CHANGE_INDEX);
         db->Execute(query, true);
+
+        if (!db->DoesTriggerExist(t, "ChangeAdded"))
+        {
+          ThrowCannotCreateTrigger();
+        }
         
         revision = 3;
         SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+
+        t.Commit();
       }
-
+      
       if (revision == 3)
       {
+        MySQLTransaction t(*db);
+
         // Reconfiguration of "Metadata" from TEXT type (up to 64KB)
         // to the LONGTEXT type (up to 4GB). This might be important
         // for applications such as the Osimis Web viewer that stores
         // large amount of metadata.
         // http://book.orthanc-server.com/faq/features.html#central-registry-of-metadata-and-attachments
         db->Execute("ALTER TABLE Metadata MODIFY value LONGTEXT", false);
-
+        
         revision = 4;
         SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+
+        t.Commit();
       }
-
+      
       if (revision == 4)
       {
+        MySQLTransaction t(*db);
+        
         // Install the "CreateInstance" extension
         std::string query;
-
+        
         Orthanc::EmbeddedResources::GetFileResource
           (query, Orthanc::EmbeddedResources::MYSQL_CREATE_INSTANCE);
         db->Execute(query, true);
-
+        
         revision = 5;
         SetGlobalIntegerProperty(*db, t, Orthanc::GlobalProperty_DatabasePatchLevel, revision);
+
+        t.Commit();
       }
-
+      
       if (revision != 5)
       {
         LOG(ERROR) << "MySQL plugin is incompatible with database schema revision: " << revision;
         throw Orthanc::OrthancException(Orthanc::ErrorCode_Database);        
       }
-
-      t.Commit();
     }
 
     /**