Improve output triggers when logged to file

Hmm write on rebuild AND reap on destruct

diff --git a/CRM/Logging/Schema.php b/CRM/Logging/Schema.php
index 356aaf8fae3ef3bc5b68c151d668e4620e909d14..04868e2b006410ae68dce199d924662329e816ee 100644 (file)
--- 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 de0b5a9de9d7b1b6b28f0ec0cfe26ecb125eb2f2..b1ac987d532fe8bcc811ebe444dd6cd88d4d058c 100644 (file)
--- 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 0b0f47894f3a34800f9f54a2745518ece453ab1e..5c3ae3ba5620b5f1ad74b5609cbdba1bb13a0b69 100644 (file)
--- 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);
+  }
+
 }