From 0a0896208e53671328b9fe0a43f8d53860465ea1 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 2 Jun 2021 15:13:32 +1200 Subject: [PATCH] Improve output triggers when logged to file Hmm write on rebuild AND reap on destruct --- CRM/Logging/Schema.php | 4 +-- Civi/Core/SqlTriggers.php | 51 +++++++++++++++++++++++----- tests/phpunit/api/v3/LoggingTest.php | 38 +++++++++++++++++++++ 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php index 356aaf8fae..04868e2b00 100644 --- a/CRM/Logging/Schema.php +++ b/CRM/Logging/Schema.php @@ -230,7 +230,7 @@ AND (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString ) * * @param string $tableName */ - public function dropTriggers($tableName = NULL) { + public function dropTriggers($tableName = NULL): void { /** @var \Civi\Core\SqlTriggers $sqlTriggers */ $sqlTriggers = Civi::service('sql_triggers'); $dao = new CRM_Core_DAO(); @@ -244,7 +244,7 @@ AND (TABLE_NAME LIKE 'log_civicrm_%' $nonStandardTableNameString ) // Sort the table names so the sql output is consistent for those sites // loading it asynchronously (using the setting 'logging_no_trigger_permission') - asort($tableNames); + ksort($tableNames); foreach ($tableNames as $table) { $validName = CRM_Core_DAO::shortenSQLName($table, 48, TRUE); diff --git a/Civi/Core/SqlTriggers.php b/Civi/Core/SqlTriggers.php index de0b5a9de9..b1ac987d53 100644 --- a/Civi/Core/SqlTriggers.php +++ b/Civi/Core/SqlTriggers.php @@ -24,7 +24,14 @@ class SqlTriggers { * * @var string|null */ - private $file = NULL; + private $file; + + /** + * Queries written to file when the class is destructed. + * + * @var array + */ + private $enqueuedQueries = []; /** * Build a list of triggers via hook and add them to (err, reconcile them @@ -52,6 +59,7 @@ class SqlTriggers { // now create the set of new triggers $this->createTriggers($info, $tableName); + $this->writeEnqueuedQueriesToFile(); } /** @@ -141,7 +149,7 @@ class SqlTriggers { // Sort tables alphabetically in order to output in a consistent order // for sites that like to diff this output over time // (ie. with the logging_no_trigger_permission setting in place). - asort($triggers); + ksort($triggers); // now spit out the sql foreach ($triggers as $tableName => $tables) { if ($onlyTableName != NULL && $onlyTableName != $tableName) { @@ -202,12 +210,8 @@ class SqlTriggers { ['expires' => 0] ); } - - $buf = "\n"; - $buf .= "DELIMITER //\n"; - $buf .= \CRM_Core_DAO::composeQuery($triggerSQL, $params) . " //\n"; - $buf .= "DELIMITER ;\n"; - file_put_contents($this->getFile(), $buf, FILE_APPEND); + $query = \CRM_Core_DAO::composeQuery($triggerSQL, $params); + $this->enqueuedQueries[$query] = $query; } else { \CRM_Core_DAO::executeQuery($triggerSQL, $params, TRUE, NULL, FALSE, FALSE); @@ -226,4 +230,35 @@ class SqlTriggers { return $this->file; } + /** + * Write queries to file when the class is destructed. + * + * Note this is already written out for a full rebuild but it is + * possible (at least in terms of what is public) to call drop & create + * separately so this ensures they are output. + */ + public function __destruct() { + $this->writeEnqueuedQueriesToFile(); + } + + /** + * Write queries queued for write-to-file. + */ + protected function writeEnqueuedQueriesToFile(): void { + if (!empty($this->enqueuedQueries) && $this->getFile()) { + $buf = "DELIMITER //\n"; + foreach ($this->enqueuedQueries as $query) { + if (strpos($query, 'CREATE TRIGGER') === 0) { + // The create triggers are long so put spaces between them. For the drops + // condensed is more readable. + $buf .= "\n"; + } + $buf .= $query . " //\n"; + } + $buf .= "DELIMITER ;\n"; + file_put_contents($this->getFile(), $buf, FILE_APPEND); + $this->enqueuedQueries = []; + } + } + } diff --git a/tests/phpunit/api/v3/LoggingTest.php b/tests/phpunit/api/v3/LoggingTest.php index 0b0f47894f..5c3ae3ba56 100644 --- a/tests/phpunit/api/v3/LoggingTest.php +++ b/tests/phpunit/api/v3/LoggingTest.php @@ -37,6 +37,7 @@ class api_v3_LoggingTest extends CiviUnitTestCase { protected function tearDown(): void { $this->quickCleanup(['civicrm_email', 'civicrm_address']); parent::tearDown(); + Civi::settings()->set('logging_no_trigger_permission', FALSE); $this->callAPISuccess('Setting', 'create', ['logging' => FALSE]); $schema = new CRM_Logging_Schema(); $schema->dropAllLogTables(); @@ -492,4 +493,41 @@ class api_v3_LoggingTest extends CiviUnitTestCase { } } + /** + * Test the output under logging_no_trigger_permission. + * + * The logging_no_trigger_permission setting causes the trigger sql + * to be output to a file rather than run. It is for situations + * where the db user does not have adequate permissions (Super permission + * is required when replication is enabled. + * + * This tests the output of that file. + */ + public function testTriggerOutput(): void { + Civi::settings()->set('logging_no_trigger_permission', TRUE); + Civi::settings()->set('logging', TRUE); + /* @var \Civi\Core\SqlTriggers $sqlTriggers */ + $sqlTriggers = Civi::service('sql_triggers'); + $fileName = $sqlTriggers->getFile(); + $triggerOutPut = file_get_contents($fileName); + $this->assertStringStartsWith('DELIMITER // +DROP FUNCTION', $triggerOutPut); + $this->assertStringContainsString('DROP TRIGGER IF EXISTS civicrm_activity_before_insert // +DROP TRIGGER IF EXISTS civicrm_activity_before_update // +DROP TRIGGER IF EXISTS civicrm_activity_before_delete // +DROP TRIGGER IF EXISTS civicrm_activity_after_insert // +DROP TRIGGER IF EXISTS civicrm_activity_after_update // +DROP TRIGGER IF EXISTS civicrm_activity_after_delete // +DROP TRIGGER IF EXISTS civicrm_activity_contact_before_insert // +DROP TRIGGER IF EXISTS civicrm_activity_contact_before_update // +DROP TRIGGER IF EXISTS civicrm_activity_contact_before_delete // +DROP TRIGGER IF EXISTS civicrm_activity_contact_after_insert // +DROP TRIGGER IF EXISTS civicrm_activity_contact_after_update // +DROP TRIGGER IF EXISTS civicrm_activity_contact_after_delete //', $triggerOutPut); + + $this->assertStringEndsWith('END // +DELIMITER ; +', $triggerOutPut); + } + } -- 2.25.1