diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php index 858c693813a1..98bec45c62b5 100644 --- a/CRM/Logging/Schema.php +++ b/CRM/Logging/Schema.php @@ -31,6 +31,14 @@ * @copyright CiviCRM LLC (c) 2004-2019 */ class CRM_Logging_Schema { + + /** + * Default storage engine for log tables + * + * @var string + */ + const ENGINE = 'InnoDB'; + private $logs = []; private $tables = []; @@ -58,7 +66,7 @@ class CRM_Logging_Schema { /** * Specifications of all log table including - * - engine (default is archive, if not set.) + * - engine (default is InnoDB, if not set.) * - engine_config, a string appended to the engine type. * For INNODB space can be saved with 'ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=4' * - indexes (default is none and they cannot be added unless engine is innodb. If they are added and @@ -302,23 +310,31 @@ public function fixSchemaDifferences($enableLogging = FALSE) { * and also implements the engine change defined by the hook (i.e. INNODB). * * Note changing engine & adding hook-defined indexes, but not changing back - * to ARCHIVE if engine has not been deliberately set (by hook) and not dropping - * indexes. Sysadmin will need to manually intervene to revert to defaults. + * to INNODB if engine has not been deliberately set (by hook) and not + * dropping indexes. Sysadmin will need to manually intervene to revert to + * defaults. * * @param array $params - * 'updateChangedEngineConfig' - update if the engine config changes, default FALSE + * 'updateChangedEngineConfig' - update if the engine config changes? + * 'forceEngineMigration' - force engine upgrade from ARCHIVE to InnoDB? * * @return int $updateTablesCount + * @throws \CiviCRM_API3_Exception */ - public function updateLogTableSchema($params = []) { - isset($params['updateChangedEngineConfig']) ? NULL : $params['updateChangedEngineConfig'] = FALSE; - + public function updateLogTableSchema($params) { $updateLogConn = FALSE; $updatedTablesCount = 0; foreach ($this->logs as $mainTable => $logTable) { $alterSql = []; $tableSpec = $this->logTableSpec[$mainTable]; - $engineChanged = isset($tableSpec['engine']) && (strtoupper($tableSpec['engine']) != $this->getEngineForLogTable($logTable)); + $currentEngine = strtoupper($this->getEngineForLogTable($logTable)); + if (!isset($tableSpec['engine']) && $currentEngine == 'ARCHIVE' && $params['forceEngineMigration']) { + // table uses ARCHIVE engine (the previous default) and no one set an + // alternative engine via hook_civicrm_alterLogTables => force change to + // new default + $tableSpec['engine'] = self::ENGINE; + } + $engineChanged = isset($tableSpec['engine']) && (strtoupper($tableSpec['engine']) != $currentEngine); $engineConfigChanged = isset($tableSpec['engine_config']) && (strtoupper($tableSpec['engine_config']) != $this->getEngineConfigForLogTable($logTable)); if ($engineChanged || ($engineConfigChanged && $params['updateChangedEngineConfig'])) { $alterSql[] = "ENGINE=" . $tableSpec['engine'] . " " . CRM_Utils_Array::value('engine_config', $tableSpec); @@ -762,7 +778,7 @@ private function createLogTableFor($table) { // - prepend the name with log_ // - drop AUTO_INCREMENT columns // - drop non-column rows of the query (keys, constraints, etc.) - // - set the ENGINE to the specified engine (default is archive or if archive is disabled or nor installed INNODB) + // - set the ENGINE to the specified engine (default is INNODB) // - add log-specific columns (at the end of the table) $mysqlEngines = []; $engines = CRM_Core_DAO::executeQuery("SHOW ENGINES"); @@ -771,11 +787,10 @@ private function createLogTableFor($table) { $mysqlEngines[] = $engines->Engine; } } - $logEngine = in_array('ARCHIVE', $mysqlEngines) ? 'ARCHIVE' : 'INNODB'; $query = preg_replace("/^CREATE TABLE `$table`/i", "CREATE TABLE `{$this->db}`.log_$table", $query); $query = preg_replace("/ AUTO_INCREMENT/i", '', $query); $query = preg_replace("/^ [^`].*$/m", '', $query); - $engine = strtoupper(CRM_Utils_Array::value('engine', $this->logTableSpec[$table], $logEngine)); + $engine = strtoupper(CRM_Utils_Array::value('engine', $this->logTableSpec[$table], self::ENGINE)); $engine .= " " . CRM_Utils_Array::value('engine_config', $this->logTableSpec[$table]); $query = preg_replace("/^\) ENGINE=[^ ]+ /im", ') ENGINE=' . $engine . ' ', $query); diff --git a/api/v3/System.php b/api/v3/System.php index 35238776820f..54faf069a1a3 100644 --- a/api/v3/System.php +++ b/api/v3/System.php @@ -413,6 +413,13 @@ function _civicrm_api3_system_updatelogtables_spec(&$params) { 'title' => 'Update Engine Config if changed?', 'description' => 'By default, we only update if the ENGINE has changed, set this to TRUE to update if the ENGINE_CONFIG has changed.', 'type' => CRM_Utils_Type::T_BOOLEAN, + 'api.default' => FALSE, + ]; + $params['forceEngineMigration'] = [ + 'title' => 'Force storage engine to upgrade to InnoDB?', + 'description' => 'Older versions of CiviCRM used the ARCHIVE engine by default. Set this to TRUE to migrate the engine to the new default.', + 'type' => CRM_Utils_Type::T_BOOLEAN, + 'api.default' => FALSE, ]; } diff --git a/tests/phpunit/CRM/Logging/LoggingTest.php b/tests/phpunit/CRM/Logging/LoggingTest.php index 62d242320112..05ed23a17dcc 100644 --- a/tests/phpunit/CRM/Logging/LoggingTest.php +++ b/tests/phpunit/CRM/Logging/LoggingTest.php @@ -47,8 +47,11 @@ public function testMultilingualAlterSchemaLogging() { $query = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE `log_civicrm_option_value`", array(), TRUE, NULL, FALSE, FALSE); $query->fetch(); $create = explode("\n", $query->Create_Table); - $this->assertTrue(in_array(" `logging_test` int(11) DEFAULT '0'", $create)); - $create = explode("\n", $query->Create_Table); + // MySQL may return "DEFAULT 0" or "DEFAULT '0'" depending on version + $this->assertTrue( + in_array(" `logging_test` int(11) DEFAULT '0'", $create) + || in_array(" `logging_test` int(11) DEFAULT 0", $create) + ); CRM_Core_DAO::executeQuery("ALTER TABLE `civicrm_option_value` DROP COLUMN `logging_test`", array(), FALSE, NULL, FALSE, FALSE); $query = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE `log_civicrm_option_value`", array(), TRUE, NULL, FALSE, FALSE); $query->fetch(); @@ -58,7 +61,9 @@ public function testMultilingualAlterSchemaLogging() { \Civi::$statics['CRM_Logging_Schema']['columnSpecs'] = array(); CRM_Core_I18n_Schema::rebuildMultilingualSchema($locales); $logging->fixSchemaDifferencesFor('civicrm_option_value', array(), TRUE); - $this->assertTrue(in_array(" `logging_test` int(11) DEFAULT '0'", $create)); + $this->assertTrue( + in_array(" `logging_test` int(11) DEFAULT '0'", $create) + || in_array(" `logging_test` int(11) DEFAULT 0", $create)); $logging->disableLogging(); } diff --git a/tests/phpunit/CRM/Logging/SchemaTest.php b/tests/phpunit/CRM/Logging/SchemaTest.php index efdd8ccb2b5c..f7fcdb8e51ae 100644 --- a/tests/phpunit/CRM/Logging/SchemaTest.php +++ b/tests/phpunit/CRM/Logging/SchemaTest.php @@ -37,13 +37,63 @@ public function testQueryRewrite($query, $expectedQuery) { $this->assertEquals($expectedQuery, CRM_Logging_Schema::fixTimeStampAndNotNullSQL($query)); } + /** + * Test log tables are created as InnoDB by default + */ public function testLogEngine() { $schema = new CRM_Logging_Schema(); $schema->enableLogging(); $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + while ($log_table->fetch()) { + $this->assertRegexp('/ENGINE=InnoDB/', $log_table->Create_Table); + } + } + + /** + * Test that the log table engine can be changed via hook to e.g. MyISAM + */ + public function testHookLogEngine() { + $this->hookClass->setHook('civicrm_alterLogTables', array($this, 'alterLogTables')); + $schema = new CRM_Logging_Schema(); + $schema->enableLogging(); + $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + while ($log_table->fetch()) { + $this->assertRegexp('/ENGINE=MyISAM/', $log_table->Create_Table); + } + } + + /** + * Test that existing log tables with ARCHIVE engine are converted to InnoDB + * + * @throws \Exception + */ + public function testArchiveEngineConversion() { + $schema = new CRM_Logging_Schema(); + $schema->enableLogging(); + // change table to ARCHIVE + CRM_Core_DAO::executeQuery("ALTER TABLE log_civicrm_acl ENGINE ARCHIVE"); + $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); while ($log_table->fetch()) { $this->assertRegexp('/ENGINE=ARCHIVE/', $log_table->Create_Table); } + // engine should not change by default + $schema->updateLogTableSchema(['updateChangedEngineConfig' => FALSE, 'forceEngineMigration' => FALSE]); + $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + while ($log_table->fetch()) { + $this->assertRegexp('/ENGINE=ARCHIVE/', $log_table->Create_Table); + } + // update with forceEngineMigration should convert to InnoDB + $schema->updateLogTableSchema(['updateChangedEngineConfig' => FALSE, 'forceEngineMigration' => TRUE]); + $log_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE log_civicrm_acl"); + while ($log_table->fetch()) { + $this->assertRegexp('/ENGINE=InnoDB/', $log_table->Create_Table); + } + } + + public function alterLogTables(&$logTableSpec) { + foreach (array_keys($logTableSpec) as $tableName) { + $logTableSpec[$tableName]['engine'] = 'MyISAM'; + } } /** @@ -139,7 +189,7 @@ public function testColumnInfo() { ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci"); $schema = new CRM_Logging_Schema(); $schema->enableLogging(); - $schema->updateLogTableSchema(); + $schema->updateLogTableSchema(['updateChangedEngineConfig' => FALSE, 'forceEngineMigration' => FALSE]); $ci = \Civi::$statics['CRM_Logging_Schema']['columnSpecs']['civicrm_test_column_info']; $this->assertEquals('test_id', $ci['test_id']['COLUMN_NAME']);