From d7ea71508585721ba07c9a75baa27926b15737dd Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 17 Mar 2016 06:38:27 +1300 Subject: [PATCH] CRM-18193 add routine for converting log tables to support new log_conn_id format & hook spec i --- CRM/Logging/Schema.php | 96 ++++++++++++++++++++++++++-- api/v3/System.php | 11 ++++ tests/phpunit/api/v3/LoggingTest.php | 91 ++++++++++++++++++++++++-- 3 files changed, 188 insertions(+), 10 deletions(-) diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php index 47c761392a..b84f21640d 100644 --- a/CRM/Logging/Schema.php +++ b/CRM/Logging/Schema.php @@ -59,6 +59,8 @@ class CRM_Logging_Schema { /** * Specifications of all log table including * - engine (default is archive, 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 * engine is not set to innodb an exception will be thrown since quiet acquiescence is easier to miss). * - exceptions (by default those stored in $this->exceptions are included). These are @@ -287,6 +289,82 @@ AND (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString ) CRM_Core_DAO::triggerRebuild(NULL, TRUE); } + /** + * Update log tables structure. + * + * This function updates log tables to have the log_conn_id type of varchar + * and also implements any engine change to INNODB defined by the hooks. + * + * 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. + */ + public function updateLogTableSchema() { + $updateLogConn = FALSE; + foreach ($this->logs as $mainTable => $logTable) { + $alterSql = array(); + $tableSpec = $this->logTableSpec[$mainTable]; + if (isset($tableSpec['engine']) && strtoupper($tableSpec['engine']) != $this->getEngineForLogTable($logTable)) { + $alterSql[] = "ENGINE=" . $tableSpec['engine'] . " " . CRM_Utils_Array::value('engine_config', $tableSpec); + if (!empty($tableSpec['indexes'])) { + $indexes = $this->getIndexesForTable($logTable); + foreach ($tableSpec['indexes'] as $indexName => $indexSpec) { + if (!in_array($indexName, $indexes)) { + if (is_array($indexSpec)) { + $indexSpec = implode(" , ", $indexSpec); + } + $alterSql[] = "ADD INDEX {$indexName}($indexSpec)"; + } + } + } + } + $columns = $this->columnSpecsOf($logTable); + if (empty($columns['log_conn_id'])) { + throw new Exception($logTable . print_r($columns, TRUE)); + } + if ($columns['log_conn_id']['DATA_TYPE'] != 'varchar' || $columns['log_conn_id']['LENGTH'] != 17) { + $alterSql[] = "MODIFY log_conn_id VARCHAR(17)"; + $updateLogConn = TRUE; + } + if (!empty($alterSql)) { + CRM_Core_DAO::executeQuery("ALTER TABLE {$this->db}.{$logTable} " . implode(', ', $alterSql)); + } + } + if ($updateLogConn) { + civicrm_api3('Setting', 'create', array('logging_uniqueid_date' => date('Y-m-d H:i:s'))); + } + } + + /** + * Get the engine for the given table. + * + * @param string $table + * + * @return string + */ + public function getEngineForLogTable($table) { + return strtoupper(CRM_Core_DAO::singleValueQuery(" + SELECT ENGINE FROM information_schema.tables WHERE TABLE_NAME = %1 + AND table_schema = %2 + ", array(1 => array($table, 'String'), 2 => array($this->db, 'String')))); + } + + /** + * Get all the indexes in the table. + * + * @param string $table + * + * @return array + */ + public function getIndexesForTable($table) { + return CRM_Core_DAO::executeQuery(" + SELECT constraint_name + FROM information_schema.key_column_usage + WHERE table_schema = %2 AND table_name = %1", + array(1 => array($table, 'String'), 2 => array($this->db, 'String')) + )->fetchAll(); + } + /** * Add missing (potentially specified) log table columns for the given table. * @@ -485,7 +563,7 @@ AND (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString ) private function columnSpecsOf($table) { static $columnSpecs = array(), $civiDB = NULL; - if (empty($columnSpecs)) { + if (empty($columnSpecs) || !isset($columnSpecs[$table])) { if (!$civiDB) { $dao = new CRM_Contact_DAO_Contact(); $civiDB = $dao->_database; @@ -583,6 +661,15 @@ WHERE table_schema IN ('{$this->db}', '{$civiDB}')"; return $diff; } + /** + * Getter for logTableSpec. + * + * @return array + */ + public function getLogTableSpec() { + return $this->logTableSpec; + } + /** * Create a log table with schema mirroring the given table’s structure and seeding it with the given table’s contents. * @@ -614,12 +701,13 @@ COLS; // - prepend the name with log_ // - drop AUTO_INCREMENT columns // - drop non-column rows of the query (keys, constraints, etc.) - // - set the ENGINE to ARCHIVE + // - set the ENGINE to the specified engine (default is archive) // - add log-specific columns (at the end of the table) $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], 'ARCHIVE')); + $engine .= " " . CRM_Utils_Array::value('engine_config', $this->logTableSpec[$table]); $query = preg_replace("/^\) ENGINE=[^ ]+ /im", ') ENGINE=' . $engine . ' ', $query); // log_civicrm_contact.modified_date for example would always be copied from civicrm_contact.modified_date, @@ -634,8 +722,8 @@ COLS; CRM_Core_DAO::executeQuery("INSERT INTO `{$this->db}`.log_$table ($columns, log_conn_id, log_user_id, log_action) SELECT $columns, @uniqueID, @civicrm_user_id, 'Initialization' FROM {$table}", CRM_Core_DAO::$_nullArray, TRUE, NULL, FALSE, FALSE); $this->tables[] = $table; - if(empty($this->logs)) { - civicrm_api3('Setting', 'create', array('logging_uniqueid_date' => 'now')); + if (empty($this->logs)) { + civicrm_api3('Setting', 'create', array('logging_uniqueid_date' => date('Y-m-d H:i:s'))); civicrm_api3('Setting', 'create', array('logging_all_tables_uniquid' => 1)); } $this->logs[$table] = "log_$table"; diff --git a/api/v3/System.php b/api/v3/System.php index b2f4ce44fc..6187cce4cc 100644 --- a/api/v3/System.php +++ b/api/v3/System.php @@ -389,3 +389,14 @@ function _civicrm_api3_system_get_whitelist($whitelistFile) { ); return $whitelist; } + +/** + * Update log table structures. + * + * This updates the engine type if defined in the hook and changes the field type + * for log_conn_id to reflect CRM-18193. + */ +function civicrm_api3_system_updatelogtables() { + $schema = new CRM_Logging_Schema(); + $schema->updateLogTableSchema(); +} diff --git a/tests/phpunit/api/v3/LoggingTest.php b/tests/phpunit/api/v3/LoggingTest.php index 24bd12d5c6..2b70a35e63 100644 --- a/tests/phpunit/api/v3/LoggingTest.php +++ b/tests/phpunit/api/v3/LoggingTest.php @@ -63,10 +63,15 @@ class api_v3_LoggingTest extends CiviUnitTestCase { $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); $this->assertLoggingEnabled(TRUE); $this->checkLogTableCreated(); - $this->checkTriggersCreated(); + $this->checkTriggersCreated(TRUE); // Create a contact to make sure they aren't borked. $this->individualCreate(); $this->assertTrue($this->callAPISuccessGetValue('Setting', array('name' => 'logging', 'group' => 'core'))); + $this->assertEquals(1, $this->callAPISuccessGetValue('Setting', array('name' => 'logging_all_tables_uniquid', 'group' => 'core'))); + $this->assertEquals( + date('Y-m-d'), + date('Y-m-d', strtotime($this->callAPISuccessGetValue('Setting', array('name' => 'logging_uniqueid_date', 'group' => 'core')))) + ); $this->callAPISuccess('Setting', 'create', array('logging' => FALSE)); $this->assertEquals(0, $this->callAPISuccessGetValue('Setting', array('name' => 'logging', 'group' => 'core'))); @@ -80,12 +85,66 @@ class api_v3_LoggingTest extends CiviUnitTestCase { $this->hookClass->setHook('civicrm_alterLogTables', array($this, 'innodbLogTableSpec')); $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); $this->checkINNODBLogTableCreated(); - $this->checkTriggersCreated(); + $this->checkTriggersCreated(TRUE); // Create a contact to make sure they aren't borked. $this->individualCreate(); $this->callAPISuccess('Setting', 'create', array('logging' => FALSE)); } + /** + * Check responsible creation when old structure log table exists. + * + * When an existing table exists NEW tables will have the varchar type for log_conn_id. + * + * Existing tables will be unchanged, and the trigger will use log_conn_id + * rather than uniqueId to be consistent across the tables. + * + * The settings for unique id will not be set. + */ + public function testEnableLoggingLegacyLogTableExists() { + $this->createLegacyStyleContactLogTable(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + $this->checkTriggersCreated(FALSE); + $this->assertEquals(0, $this->callAPISuccessGetValue('Setting', array('name' => 'logging_all_tables_uniquid', 'group' => 'core'))); + $this->assertEmpty($this->callAPISuccessGetValue('Setting', array('name' => 'logging_uniqueid_date', 'group' => 'core'))); + } + + /** + * Check we can update legacy log tables using the api function. + */ + public function testUpdateLegacyLogTable() { + $this->createLegacyStyleContactLogTable(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + $this->callAPISuccess('System', 'updatelogtables', array()); + $this->checkLogTableCreated(); + $this->checkTriggersCreated(TRUE); + $this->assertEquals(0, $this->callAPISuccessGetValue('Setting', array('name' => 'logging_all_tables_uniquid', 'group' => 'core'))); + $this->assertEquals( + date('Y-m-d'), + date('Y-m-d', strtotime($this->callAPISuccessGetValue('Setting', array('name' => 'logging_uniqueid_date', 'group' => 'core')))) + ); + } + + /** + * Check we can update legacy log tables using the api function. + */ + public function testUpdateLogTableHookINNODB() { + $this->createLegacyStyleContactLogTable(); + $this->callAPISuccess('Setting', 'create', array('logging' => TRUE)); + $this->hookClass->setHook('civicrm_alterLogTables', array($this, 'innodbLogTableSpec')); + $this->callAPISuccess('System', 'updatelogtables', array()); + $this->checkINNODBLogTableCreated(); + $this->checkTriggersCreated(TRUE); + // Make sure that the absence of a hook specifying INNODB does not cause revert to archive. + // Only a positive action, like specifying ARCHIVE in a hook should trigger a change back to archive. + $this->hookClass->setHook('civicrm_alterLogTables', array()); + $schema = new CRM_Logging_Schema(); + $spec = $schema->getLogTableSpec(); + $this->assertEquals(array(), $spec['civicrm_contact']); + $this->callAPISuccess('System', 'updatelogtables', array()); + $this->checkINNODBLogTableCreated(); + } + /** * Use a hook to declare an INNODB engine for the contact log table. * @@ -93,7 +152,8 @@ class api_v3_LoggingTest extends CiviUnitTestCase { */ public function innodbLogTableSpec(&$logTableSpec) { $logTableSpec['civicrm_contact'] = array( - 'engine' => 'INNODB', + 'engine' => 'InnoDB', + 'engine_config' => 'ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=4', 'indexes' => array( 'index_id' => 'id', 'index_log_conn_id' => 'log_conn_id', @@ -111,7 +171,7 @@ class api_v3_LoggingTest extends CiviUnitTestCase { $this->assertEquals('log_civicrm_contact', $dao->Table); $tableField = 'Create_Table'; $this->assertContains('`log_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,', $dao->$tableField); - $this->assertContains('`log_conn_id` varchar(17) COLLATE utf8_unicode_ci DEFAULT NULL,', $dao->$tableField); + $this->assertContains('`log_conn_id` varchar(17)', $dao->$tableField); return $dao->$tableField; } @@ -121,17 +181,23 @@ class api_v3_LoggingTest extends CiviUnitTestCase { protected function checkINNODBLogTableCreated() { $createTableString = $this->checkLogTableCreated(); $this->assertContains('ENGINE=InnoDB', $createTableString); + $this->assertContains('ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=4', $createTableString); $this->assertContains('KEY `index_id` (`id`),', $createTableString); } /** * Check the triggers were created and look OK. */ - protected function checkTriggersCreated() { + protected function checkTriggersCreated($unique) { $dao = CRM_Core_DAO::executeQuery("SHOW TRIGGERS LIKE 'civicrm_contact'"); while ($dao->fetch()) { if ($dao->Timing == 'After') { - $this->assertContains('@uniqueID', $dao->Statement); + if ($unique) { + $this->assertContains('@uniqueID', $dao->Statement); + } + else { + $this->assertContains('CONNECTION_ID()', $dao->Statement); + } } } } @@ -147,4 +213,17 @@ class api_v3_LoggingTest extends CiviUnitTestCase { $this->assertTrue($schema->isEnabled() === $expected); } + /** + * Create the contact log table with log_conn_id as an integer. + */ + protected function createLegacyStyleContactLogTable() { + CRM_Core_DAO::executeQuery(" + CREATE TABLE log_civicrm_contact + (log_conn_id INT NULL, log_user_id INT NULL, log_date timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP) + ENGINE=ARCHIVE + (SELECT c.*, CURRENT_TIMESTAMP as log_date, 'Initialize' as 'log_action' + FROM civicrm_contact c) + "); + } + } -- 2.25.1